Repository navigation
C5-1a: tech admin, governance and staff on the contract, with finer admin scopes (ADR 0009) - #49
Merged
Merged
Conversation
…ons, settings anchors The admin group's wire contract migration (C5-1a): rfcs gains created_at and updated_at NOT NULL, revision, and the RFC- number sequence with its backfill in (created_at, id) order; staff and api_keys gain what their lists and preconditions read; admin_revisions anchors each singleton settings resource's If-Match revision. Down file reverses it; the test proves the backfills on legacy rows, idempotence, and down then up.
ADR 0002's second known limit narrowed with the admin group's conversion: the admin module's second path segment names an area scope (admin:settings, admin:staff, admin:modules) that replaces the coarse read and write on that area's routes, and the users module's writes are named users:grants. Exact match, no implication; the coarse admin scopes stay on the routes no area declares. ValidScopeGrammar exposes the whole grantable set so C5-2a's mint validation and the auth core cannot disagree; the census test holds the areas against the routes in both directions. ADR 0002's known limits row now points at this record.
The machine key surface keeps its mint (ADR 0002; grammar validation is C5-2a's) and gains the wire rules: the key list is the cursor envelope with include=total and scopes that never read back null, a mint answers 201 with Location, revoking an unknown key is a 404 (a typo no longer looks like success), and the mint and revoke write their audit row and key.created or key.revoked in one transaction. The AI and routing settings become documents with a revision anchored in admin_revisions (migration 095): GET carries the revision and ETag, PUT and DELETE take If-Match or a body revision (428 and 409 otherwise), and the save, its audit row and admin_settings.saved are one transaction. The module writes the settings rows itself, in the transaction; the AI stack's key stores stay the read side and pick a saved key up within their cache TTL. Wire tests pin the envelope, the strict query posture, the revision rules and the finer admin:settings scope (a coarse admin key is refused and audited); the tx proofs roll a failed event or audit write back and run three contenders at pool 4 with the gated saturation test.
… anchored The staff list is the cursor envelope with an active filter and include=total, ordered by created_at and id (the alphabetical order is a listed change; the desk sorts client side). A create answers 201 with Location, the revision and modules never null, and a duplicate email or staff number is a 409 naming the field instead of a 500. Updates, module grants and revokes take If-Match or a body revision: the modules list is part of the staff document, so a grant moves its revision; idempotent re-grants and re-revokes write nothing. The module catalog list is the envelope with each flag's revision, and the kill switch takes the same precondition, writing module.enabled or module.disabled when it flips. Every mutation carries its audit row and its outbox event in one transaction. Wire tests pin the shapes, the strict query posture, the revision rules, the vocabulary of grantable modules and the finer admin:staff and admin:modules scopes; the tx proofs roll failed event writes back and run three contenders at pool 4 with the gated saturation test.
…tions
The RFC list is the cursor envelope with a status filter and include=total,
and a legacy NULL content row no longer 500s it (the seed's rows read back
with content null). A create answers 201 with Location, an RFC- number from
the sequence, the generated content, the revision and the ETag; the create,
its audit row and rfc.created are one transaction. Updates replace the
editable fields on If-Match or the body revision (428, 409) and refuse
status, pointing at the transitions route; the lifecycle itself moves to
POST /rfcs/{id}/transitions (draft to review, review to approved or
rejected, rejected reopened), the forbidden edges 409
invalid_state_transition, each edge writing its event. The seed's one
'published' status becomes 'approved' (outside the vocabulary, same
meaning). Wire tests pin the shapes, the filter, the cursor walk, the
idempotent replay, the machine key scope and the lifecycle; the tx proofs
roll failed event writes back (the number abandoned, never reused) and run
three contenders at pool 4 with the gated saturation test.
The admin and governance fragments describe the converted modules exactly (the exposure scan route keeps its pending shape); the new transitions route joins the census; the generated client regenerates. Every golden group of the three modules is re-recorded onto the new shapes with steps pinning the validation 400s, the filters, the revision preconditions, the refused transitions, the idempotent no-ops, the finer admin scopes (a settings key reaches the settings, a coarse key is refused and audited) and the events feed; the integration golden is untouched. One row per route and behaviour in CONTRACT-CHANGES.md.
The tech admin service walks the cursor envelopes, sends the revision it read as If-Match on every settings save, roster update, grant, revoke and kill switch flip, retries a stale write once on the fresh revision, and keeps the UI's stable error prefixes. The governance service lists summaries, edits on the revision and moves status through the transitions route; the dashboard table shows the RFC number in place of the problem statement (the list no longer drags the body), and a legacy null content renders empty. The staff access tests' fake backend serves the new wire.
…nd the proofs the recipe asks for The recipe audit's gaps closed on the three modules: the settings saves and the grant, revoke and kill switch writes re-read their response before the audit row and event so the event is the transaction's last statement; a staff PUT whose body sets no field is now the grants' idempotent no-op (nothing written, revision unmoved) instead of an event without a revision move; the modules list honors the include it declares (total, unknown names refused); the kill switch's response is re-read inside its transaction; RevokeKey lost its dead RowsAffected branch. New proofs: idempotent replay and reuse on the key mint and the staff create; failing event and audit writes roll the revoke, the settings delete, the staff grant and the kill switch back; the machine key scope refusal writes its audit row on all three modules; governance's rejected and reopen edges, transition preconditions and list strictness are pinned; the pool 4 concurrent creates prove one event each; the role guard coverage and the disable preserves grants invariant the old handler tests carried are restored as wire facts; migration 095's api_keys backfill assertion actually runs; the machinekey tests compile against the converted techadmin service.
…-admin # Conflicts: # docs/refactor/CONTRACT-CHANGES.md
…nverted contract The admin spec finishes the item's Playwright step: a minted key shown once and revoked (the desk's confirm dialogs accepted so the DELETE fires), the AI settings saved on their revision and removed again, the staff roster's AI_LM grant and the kill switch on their own revisions (the seeded roster is all active, so the member is asserted by name, not by state), and a governance RFC through its lifecycle: drafted in the desk form, listed with its RFC- number, opened, edited on its revision with a stale write refused 409, transitioned to review through the transitions route, and served by the status filter. The number cells match without anchors (the template pads their text) and the form is driven by placeholder (its labels are not wired).
…tes one too The admin group's conversion (C5-1a) writes a key.created audit row on the minted key, so the smoke check that read every audit row for the key's entity saw two rows and failed its exact string compare. The check now pins the refusal row itself: action key.scope_refused attributed to the key's id, which is the fact it names.
…tation table The census test built its areas from FinerAdminScopes(), the very map it judged, and accepted the coarse admin:write for every route outside a declared area, so a deleted area, a stray area and a route added under /api/v1/admin without a scope decision all passed. The test now holds a hand written table of every census route under /api/v1/admin and /api/v1/users with the scope a machine key must hold for its method, or the user only marker, and fails on a census route missing from the table, on a table row matching no census route, on a route resolving to a scope other than its row's, and on a finer scope the middleware declares that no row expects. Review round 1 P2-1 (PR 49).
…cks and the routing precondition The pool 4 race started with the anchor rows absent, so the losing racers blocked on the INSERT that creates the row and read the bumped revision by accident; the FOR UPDATE in LockRevision and LockModuleRevision was never exercised and removing it passed the suite. The settings race now creates the anchor first (one save) and races three writers at the current revision, and the staff pool 4 test races one module flag the same way (the anchor seeded by one real toggle), both expecting exactly one winner. The routing settings had no precondition coverage at all: the wire test now takes the save and the delete through 428 without a precondition, 409 on a stale one and the successful write. Review round 1 P2-2 (PR 49).
A machine key holding admin:staff carries no JWT claims, so the grant route's requesterSub stored an empty granted_by and the module_grants row lost who granted (the audit row attributed to the key; the grant did not). requesterSub now falls back to the machine key id from the request context and stores the key:<id> principal, the same one the idempotency layer keys on. A wire test grants through a real minted key and holds the row's granted_by to that principal. Review round 1 P3-1 (PR 49).
…opes The contract table said no stored key changes reach, which is true of the seed and not of a deployed database that minted keys with admin:read, admin:write or users:write. The new row tells an operator those keys lose reach on the settings, staff and modules areas and the branch grants on upgrade and must be re-minted with the finer names. Review round 1 P3-2 (PR 49).
…st fixtures The three new wire test files read and delete events_outbox rows but never called testutil.LockOutboxTables(t), so go test ./... running packages in parallel against one database intermittently lost rows to other tests and the wire test broke. Take the lock as the first line of each newFixture, before RequireDB, exactly as the other outbox-touching fixtures and the PR's own tx_test files do.
…e enum A database seeded before this PR holds rfcs rows with status values outside the contract's enum (the base seed wrote 'published', a value no route can produce). Step 1b maps every non enum value to 'approved' (the same meaning as 'published', inside the vocabulary) and pins the enum with a CHECK constraint, guarded like the number constraint so a second apply stays a no-op. The down file's 'does not come back' list records the unmapped legacy value is gone; the rows whose status was outside the enum stay mapped, because the down has no way to know 'published' was the original. The CONTRACT-CHANGES row now states the real state: a new seed sits on the lifecycle, and a pre-PR seed lands on it after the migration. TestMigration095_RemapsLegacyRfcStatusAndPinsTheEnum inserts a 'published' row and an 'archived' odd row, asserts the backfill maps them both to 'approved' and the CHECK constraint refuses a new 'published' insert, and proves the second apply is a no-op.
For /api/v1/admin//settings/ai (or /api/v1/admin with nothing after, or a trailing slash) the second segment is empty, no area matches, and the required scope became admin:read or admin:write. The mux answers 307 on the doubled slash and a 404 on the bare prefix, so no handler is reached and the wider reach never lands; but the guarantee 'coarse scopes reach only routes no area declares' (ADR 0009 section 5) then depends on the router cleaning the path. Refuse it: when the module is admin and the second segment is empty, RequiredScopeForPath returns '', false, the most restrictive answer, so the coarse scope is never offered as a fallback. TestMachineKeyFinerAdminScopes gains the doubled slash and bare prefix cases for the coarse read, the coarse write, and the union of the two.
The line 'ALTER TABLE rfcs ALTER COLUMN number DROP DEFAULT' fails on a second run with 'column number of relation rfcs does not exist' because every other line in the file uses IF EXISTS. Down files are applied by hand, so a retry after a partial failure stops mid-file. Drop the line: dropping the column drops its default, so the step is redundant. The test applies the down twice on a migrated database and asserts the second apply is a no-op.
The route lists the users (subs) holding a branch but sits under the branches segment, so a key with branches:read reaches it. The v1 contract stays as it is; moving the route under the users segment (so users:read admits it and branches:read does not) is a later item.
…-admin Brings PR 47 (C5-1b). Two conflicts: the model shape test keeps this branch's governance RFC and RFCSummary bindings and takes refactor/v1's renamed millwork Option and configurator Rule and Preset; openapi.yaml is reassembled from the merged fragments (fresh: 291 paths, 370 operations, 463 schemas). Pending, conformance, census and the generated TypeScript are fresh; governance, staff, techadmin, millwork, configurator, crm, project, apicontract, middleware, characterization and serve pass with -race. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-admin Brings PR 53 (CI pulls its images through a Docker Hub mirror), ci.yml only, disjoint from this branch's files, so the hosted checks can run past the Docker Hub rate limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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.
What this is
Item C5-1a (security class), the admin group of cycle 5's module conversions:
core/internal/techadmin, core/internal/staff and core/internal/governance onto
docs/refactor/MODULE-RECIPE.md and ADR 0001, plus the finer admin scopes ADR
0002's second known limit and ADR 0007 section 5.5 assign to C5-1, written up
as docs/adr/0009-finer-admin-scopes.md. AI load management itself lives in
core/internal/integrations (ailm), not in these modules, so its scoped key
move is not in this pull request.
The modules
mint on one 400 with every field, the delete distinguishing 400/404/204 with
key.revoked and its audit row in one transaction; the AI and routing settings
are documents with a revision anchored in admin_revisions (migration 095):
If-Match or the body revision on every write (428, 409), the settings
answered with the new revision and ETag, admin_settings.saved/deleted in the
write's transaction.
on every row; creates, updates, module grants and revokes take the
precondition, an idempotent no-op (a re-grant, a re-revoke, an update whose
body sets no field) writes nothing; the kill switch is its own document on
its own revision anchor and disabling deletes no grant.
in (created_at, id) order), a status filter on the lowercase vocabulary, the
transitions route with the draft/review/approved/rejected lifecycle
(rejected reopens to draft), each edge its own event, all writes with the
revision precondition and the event as the transaction's last statement.
ADR 0009 (the finer admin scopes)
admin:settings, admin:staff and admin:modules replace the coarse read/write on
their areas' routes (the scope is the second path segment under /api/v1/admin,
admitting every method on the area), users:grants names the users module's
writes, exact match with no implication, the coarse admin scopes stay on the
exposure scan trigger and the key routes stay user only. ValidScopeGrammar
exposes the grantable set for C5-2a's mint validation; the census test holds
the areas against the routes in both directions and the refusal audit row
names the finer scope. ADR 0002's known limits row points here.
The audit pass on top (this branch's own review of itself)
Event-last ordering fixed on the settings saves and the grant/revoke/kill
switch writes (the response re-read moves before the sinks); a no-field staff
PUT writes nothing instead of an event without a revision move; the modules
list honors the include it declares; the kill switch re-reads inside its
transaction; new proofs: idempotent replay and 422 reuse on the mint and the
staff create, failing event/audit writes rolling back every write kind on all
three modules, the machine key refusal audit row on all three, the rejected
and reopen edges, transition preconditions (428, weak If-Match, disagreement),
list strictness and the empty [] page, one event each at pool 4, the role
guard and the disable-preserves-grants invariant restored as wire facts.
Merged
origin/refactor/v1 merged as a merge commit (C2-2, C3-1 and several security
items); generated files regenerated (no drift), migration 095 keeps its number
(no collision), ROUTES.txt recounted at 365 routes.
Gates