feat(server): Phase 1-2: add runtime control version history#173
Open
lan17 wants to merge 5 commits intofeature/control-phase-0from
Open
feat(server): Phase 1-2: add runtime control version history#173lan17 wants to merge 5 commits intofeature/control-phase-0from
lan17 wants to merge 5 commits intofeature/control-phase-0from
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is Phase 1 of the control lifecycle work. It takes
control_versionsfrom migration-only/backfill state into active runtime use by recording new version rows on control mutations and exposing version history through read APIs.This PR is stacked on top of #172.
Please review it against
feature/control-phase-0, not againstmain.Design doc and implementation plan: https://gist.github.com/lan17/6a08282243576f096626bb10996c024b
What changed
The server now records a version row whenever a control is created, updated, patched, or soft-deleted. Those writes happen in the same transaction as the control mutation, so the live control row and its latest audit snapshot stay in sync.
To keep that logic in one place, this PR introduces a
ControlServicethat owns active-control lookup, version creation, and version-history reads. The control endpoints now use that service instead of each path reaching into the database on its own, and the agent/policy association endpoints use the same service for active-control existence checks.This PR also adds two version history endpoints:
GET /api/v1/controls/{control_id}/versionsGET /api/v1/controls/{control_id}/versions/{version_num}Those endpoints follow the repo's cursor-pagination conventions. The list endpoint returns summaries only, while the detail endpoint returns the full stored snapshot for audit and diffing.
The shared API models, Python SDK wrapper, and generated TypeScript SDK were updated to match the new endpoints and response shapes.
Review follow-up
A review-loop pass found a concurrency hole in version number allocation and one misleading error mapping on
PATCH /controls/{id}. This branch now serializes version creation with a row-level control lock and only reportsCONTROL_NAME_CONFLICTfor actual control-name uniqueness failures.Why this shape
Phase 0 created the schema and backfilled history, but runtime writes still were not participating in version tracking. That left
control_versionsuseful for migration cleanup, but not yet trustworthy as the live audit trail.This PR closes that gap before the later store/clone work. It also establishes
ControlServiceas the boundary for control persistence concerns, which keeps the next phase from duplicating versioning and lookup logic again.Reviewer notes
The important areas to look at are:
ControlServicetransaction behavior, row locking, and snapshot shapeValidation
make checkmake openapi-spec-checkmake sdk-ts-generatemake sdk-ts-overlay-testmake sdk-ts-name-check