docs: lock pillar 4 design (release management)

[ellen-goc]

Summary

Locks the pillar-4 design into docs/pillars/04-release-management.md, following the pillar-2 (#13) precedent, and adds the README index entry. Implementation lands in weyucou/ibuki-backend as a new backend/releases/ app — no code in this repo.

The design was re-verified against weyucou/ibuki-backend@main (9ec13c7) — tenants/models.py, specs/models.py, proposals/models.py, proposals/states.py, mcp_server/{tools,context,auth}.py, conformance/schemas.py, pyproject.toml, .github/workflows/ci.yml. Three grounded facts shape it:

• There is no Project model — tenant is the only scope.
• The cut-eligible unit is a locked specs.SpecVersion, not a Proposal. ProposalState has no locked member, and specs/models.py names this issue as the consumer of locked_at.
• The lock step is pillar-4 scope and the critical path. The ibuki.lock stub is labelled "(pillar 4)" in mcp_server/tools.py, and nothing in pillar 3 writes a SpecVersion.

The doc locks:

• Data model — Release + ReleaseItem, with ReleaseItem.spec_version = PROTECT closing the retention gap specs/models.py flags as pillar-4 scope.
• Manifest hashing — SHA-256 over canonical JSON, stored verbatim in manifest_json so verification re-hashes stored bytes.
• Versioning — semver / date / sequential as pure functions, configured by a new Tenant.versioning_scheme field (Django admin, matching judge_model and check_token_budget — no MCP setter).
• Concurrency — select_for_update(nowait=True) on the tenant row, with UniqueConstraint(tenant, version) as backstop.
• Immutability — application layer, mirroring the shipped ProposalEvent append-only and SpecVersion lock patterns; DB grants stay deferred to ibuki-backend#14.
• MCP surface — lock / cut_release / get_release / list_releases, ReleaseErrorCode, and cut_by derived server-side from the authenticated ApiToken.
• Child split — four PR-able slices for ibuki-backend, plus the AC→test map.

Two design points are resolved beyond the epic text, both grounded in the shipped code:

1. release_id is a pre-generated UUID. The manifest contains its own release_id and is hashed before insert, so a DB-assigned id would force a follow-up UPDATE — which the append-only guard rejects. A UUID also keeps one tenant's cut volume from being inferred off a shared integer sequence.
2. ALREADY_LOCKED is added to the error codes. SpecVersion.lock() raises SpecLockedError on an already-locked row, so the path exists whether or not it is named; unnamed it would surface as an internal error.

Open items carried forward

• DECISION 1 (blocks child 4 only) — proposals.Proposal persists no payload, verified in proposals/models.py, so an accepted proposal cannot currently be turned back into spec content. Recommendation (a): ibuki-backend#18 persists the accepted payload.
• DECISION 3 — date-scheme exhaustion past -Z. Recommendation: fail with VERSION_EXHAUSTED.
• Cut-window watermark (child 3) — selecting on locked_at > predecessor.cut_at can drop a lock that stamps locked_at before a cut but commits after it: the cut's query cannot see it, and the next cut's filter excludes it. Reachable under Postgres' default READ COMMITTED whenever a lock and a cut overlap. The doc records a recommended fix and asks for a TransactionTestCase reproducing the interleave.

DECISION 2 (name= override strictness) needs no separate answer — the issue's own acceptance criteria mandate validating a supplied name against the tenant's scheme and rejecting a mismatch with VERSION_MISMATCH.

Closes #10 is not appropriate yet — #10 stays open until the four ibuki-backend children are filed and merged. This PR addresses the design-locking portion.

Refs #10, ibuki-backend#18, ibuki-backend#14.

Test plan

• pre-commit run --all-files passes (JSON Schema metaschema, reference template, gitleaks)
• Every backend fact in the doc read from origin/main at 9ec13c7 — no claims from memory
• README link to docs/pillars/04-release-management.md resolves; in-doc anchors (#what-grounding-changed, #open-items) match their headings
• No code changes; nothing to run

[ellen-goc]

✅ CI green — Slack notification sent.

[ellen-goc]

PR Review — #18 vs issue #10 (pillar 4, release management)

Scope check: this PR locks the design only; it correctly does not claim Closes #10. Issue #10 is complexity:complex (an epic) and stays open until the four ibuki-backend children are filed and merged. Reviewed accordingly: the deliverable is the design-locking portion, not the twelve implementation ACs.

Grounding verification — passed

The PR's central claim is that every backend fact was read from weyucou/ibuki-backend@main (9ec13c7) rather than from memory. I re-read the same files at that SHA. Every claim holds:

Claim in the doc	Verified
No Project model; tenant is the only scope	✅ tenants/models.py ships Tenant + ApiToken only
ProposalState has no locked member; accepted is terminal	✅ proposals/states.py — ACCEPTED maps to frozenset()
Proposal persists no payload	✅ proposals/models.py — tenant, feature_id, supersedes, scope_in, scope_in_normalised, state, round, timestamps
SpecVersion.locked_at; Spec→SpecVersion is CASCADE; retention is pillar-4 scope	✅ specs/models.py docstring and spec FK comment say exactly that
SpecVersion.lock() raises SpecLockedError on an already-locked row	✅ — so ALREADY_LOCKED is a real path, not a speculative one
Spec.add_version() wraps read-then-insert, guarded by unique_spec_version	✅
lock / cut_release stubs labelled "(pillar 4)"	✅ mcp_server/tools.py build_mcp()
ORM-touching MCP tools must be async (SynchronousOnlyOperation)	✅ recorded in the get_spec docstring
context.py binds tenant only; no current_token()	✅ — auth.py resolves ApiToken then passes only api_token.tenant into tenant_context()
CheckErrorCode is a StrEnum with SCREAMING values	✅ conformance/schemas.py:63
spec_payload returns {found, spec} — absent is well-formed	✅ specs/loader.py ABSENT_SPEC_PAYLOAD
tenants is at 0003_tenant_check_token_budget, so 0004 is next	✅
ibuki-backend#18 owns submit/respond; the repo is at #31	✅ #18 = "MCP wiring: ibuki.submit/respond/list_escalated (pillar 3, child 4/4)"; #31 is the last merged PR

All twelve ACs in #10 are represented in What this pillar delivers and mapped in the AC→test table. README index entry resolves, in-doc anchors (#what-grounding-changed, #open-items) match their headings. CI green (schema validation + gitleaks). Structure follows the pillar-2 (#13) precedent.

Findings

1. get_release(release_id) has no stated tenant scoping — the one IDOR-shaped gap in an otherwise closed surface.

The MCP table specifies { release_id } → { found, release } and the doc states no tool accepts a tenant argument, but it never says the lookup is filtered by current_tenant(), nor which code applies when a caller passes another tenant's release_id. Both candidate codes exist in ReleaseErrorCode (RELEASE_NOT_FOUND, TENANT_MISMATCH) and the TENANT_MISMATCH row in the lock table is scoped to proposals only. This is exactly the case the shipped code treats explicitly — _get_spec_by_id() checks spec.tenant_id != tenant.pk, emits a tool.tenant_mismatch warning, and returns a distinct payload, precisely because an id is global while a name lookup is tenant-scoped in the query.

Child 4 implements from this doc. Suggest locking it here: get_release filters on tenant=current_tenant() and returns {"found": false, "release": null} for a release it does not own (not-found rather than TENANT_MISMATCH, so the response does not confirm the id exists), with a tool.tenant_mismatch warning log matching the get_spec precedent. The AC→test map has test_list_releases_excludes_other_tenant but no get_release equivalent — worth a matching row.

2. The locked cut_release flow embeds the query that open item 3 says is defective, and the locked data model has nowhere to put the fix.

Step 3 of cut_release flow specifies locked_at__gt=predecessor.cut_at with no inline caveat; open item 3, ~130 lines later, explains that this exact predicate permanently drops any lock that stamps locked_at before a cut but commits after it. A child-3 author reading the flow section as the locked contract will implement the known defect.

The sequencing consequence is sharper: the recommended fix stores max(item.locked_at) as a watermark, but the locked Release model has no such field — it carries release_id, tenant, version, scheme, cut_at, cut_by, predecessor, manifest_hash, manifest_json. Child 1 is "unblocked today, can start immediately" and owns 0001_initial, so as written it will ship a migration that child 3 must then amend. Either resolve open item 3 before child 1 files its migration, or add the nullable watermark column to the locked data model now and let child 3 populate it. At minimum, mark step 3 as provisional and cross-link open item 3 from it.

3. The tenant row as the cut mutex has a false-positive path.

Tenant.objects.select_for_update(nowait=True) fails on any concurrent row lock, not just another cut. A Django admin save of judge_model or check_token_budget — both shipped, both edited through admin, which is also how this design sets versioning_scheme — takes an exclusive lock on the same row and would surface to a concurrent cut as CONCURRENT_CUT, an error whose contract says "another cut is in flight". Worth either naming the coupling in the concurrency rationale, or keying the mutex on something cut-specific (pg_advisory_xact_lock on the tenant id, or a lock on the predecessor Release row). The UniqueConstraint(tenant, version) backstop is unaffected either way.

4. The DB-grants deferral cites a different issue than the shipped code does.

The doc points twice at ibuki-backend#14 and calls it "the same deferral proposals/models.py already documents". Both proposals/models.py and specs/models.py actually defer to #4 ("AWS deployment: Fargate + Postgres"); #14 is its 2/2 split ("apply stacks, migrate, smoke-test"). #14 is a defensible owner for the grants, but "same deferral" overstates it — either cite #4 or note that #14 is the split that inherits it.

Nits

• list_releases pages on cut_at with no tiebreaker. Per-tenant cuts are serialized by the row lock so ties are near-impossible, but (cut_at, release_id) as the cursor costs nothing and removes the class of bug.
• VersioningScheme in releases/versioning.py imported by tenants/models.py is import-cycle-safe as the doc argues, but it does invert app layering — releases depends on tenants, not the reverse. Since child 2 owns both versioning.py and tenants/0004, siting the enum in tenants/ (or a shared module) would keep the dependency one-directional at no cost.
• The pillar-2 doc's abridged data model still lists Tenant(..., versioning_scheme, ...) and a Project model, both of which this PR's grounding contradicts. Not this PR's defect, but pillar 2's doc is now stale on two points a reader may cross-reference.

Definition of Done

Applying the done checklist to the design-lock deliverable this PR actually carries:

Item	Result
Acceptance criteria met	✅ for the design-lock scope — all 12 ACs of #10 are represented and mapped to named tests; implementation ACs remain open in the children, as the PR states
Unit tests cover new logic	n/a — docs only, no code in this repo
CI green	✅ schema validation + gitleaks both passed on c5384a6
Documentation updated	✅ new pillar doc + README index entry
Code review approved	pending — this review
No known regressions	✅ additive, 308 insertions, 0 deletions
Security checklist	n/a for a docs PR; note finding 1, which is a security-relevant gap in the designed surface rather than in shipped code

Epic gate: #10 is complexity:complex and must not enter in-progress or go to askcc develop — it is split into the four complexity:medium children first, which this PR's child split defines but does not file. That remains the next action.

Verdict

Approve in substance — the grounding is real and I could not find a claim in it that the shipped tree contradicts, which is not the usual outcome for a re-grounded design doc. Recommend addressing 1 and 2 before merge (both are additive: one contract line plus a test-map row, and one field plus a cross-link), and 3/4 before the children are filed. None of the four makes the doc wrong today; all four get more expensive once child issues are cut from it.

[ellen-goc]

Review response — findings 1 and 2 addressed in fe0a6f4

• Finding 1 (get_release tenant scoping): contract line added to the MCP surface section — get_release/list_releases filter on tenant=current_tenant(), cross-tenant release_id returns {"found": false, "release": null} (not-found, so the response does not confirm the id exists) with a tool.tenant_mismatch warning log per the _get_spec_by_id() precedent. Matching AC→test map row added: test_get_release_excludes_other_tenant.
• Finding 2 (defective step-3 predicate baked into the locked flow): flow step 3 is now marked Provisional with a cross-link to open item 3, and the nullable lock_watermark column is added to the locked Release model so child 1 ships it in 0001_initial and child 3 populates it — no migration amendment needed.

Findings 3 (cut-mutex false positive) and 4 (DB-grants deferral citation) plus the nits remain open for the child-filing step, per the review's own sequencing. With 1 and 2 landed this is mergeable from my side — leaving the merge to a human reviewer per policy.

[ellen-goc]

✅ CI green — Slack notification sent.

[ellen-goc]

PR review — #18 against issue #10

Verdict: comment (blocking concern on one locked design element). Scope, grounding and AC coverage all hold up; one recommended fix baked into the locked data model does not close the defect it is aimed at.

Reviewed as a design-lock PR, not a delivery of #10 — the PR correctly declines Closes #10, and #10 is complexity:complex (an epic), which per the Definition of Done never enters in-progress itself and closes only when its children close.

Verification performed

Every backend claim in the doc was re-read from weyucou/ibuki-backend@origin/main (9ec13c7, confirmed as current HEAD). Confirmed:

Claim	Evidence
No Project model; Spec unique on (tenant, name)	specs/models.py — unique_tenant_spec_name
ProposalState has no locked; terminal accept is accepted	proposals/states.py
specs/models.py names weyucou/ibuki#10 as the consumer, and labels retention past spec deletion pillar-4 scope	module docstring + SpecVersion.spec CASCADE comment
ibuki.lock / ibuki.cut_release stubs labelled "(pillar 4)"	mcp_server/tools.py build_mcp()
Proposal persists no payload	proposals/models.py — tenant, feature_id, supersedes, scope_in, scope_in_normalised, state, round, timestamps only
Spec.add_version() + SpecVersion.lock() ship; lock() raises SpecLockedError when already locked	specs/models.py
ProposalEvent append-only + DB-grant deferral documented	proposals/models.py docstring
Tenant.judge_model / Tenant.check_token_budget, neither with an MCP setter; tenants at 0003_tenant_check_token_budget	tenants/models.py, tenants/migrations/
CheckErrorCode is a StrEnum with SCREAMING values	conformance/schemas.py:63
spec_payload absent-is-well-formed precedent	specs/loader.py
auth.py resolves ApiToken but context.py binds tenant only	mcp_server/auth.py, mcp_server/context.py
hypothesis is a dev dependency; Postgres 17 in CI	pyproject.toml:47, .github/workflows/ci.yml:64

Doc hygiene: README link resolves, in-doc anchors #what-grounding-changed / #open-items match their headings, section structure follows the pillar-2 precedent. CI green (schema validation + gitleaks).

All 12 acceptance criteria in #10 are addressed and traced in the testing-strategy table, plus two rows beyond them (cross-tenant release_id, hash order-independence).

1. Blocking — the recommended watermark fix does not close the defect (open item 3, and the locked data model)

The defect is stated correctly, but max(item.locked_at) as the next cut's lower bound only rescues the case where the late-committing lock also carries the latest locked_at. That is the opposite of the case that produces the bug:

• t0 — lock A stamps locked_at = t0, transaction still open
• t1 — lock B stamps locked_at = t1, commits
• t2 — cut #1 runs. Sees B only. lock_watermark = t1, cut_at = t2
• t3 — lock A commits; row is now visible with locked_at = t0
• cut #2 filters locked_at > t1 → A (t0) is still excluded, permanently

A lock that commits late almost always stamped earlier than the locks that overtook it, so the watermark leaves the common interleave unfixed while adding a column.

The robust form is a set difference rather than a time window — bundle every locked SpecVersion for the tenant that no ReleaseItem references yet:

SpecVersion.objects.select_related("spec").filter(
    spec__tenant=tenant,
    locked_at__isnull=False,
    release_items__isnull=True,      # needs related_name on ReleaseItem.spec_version
).order_by("locked_at", "pk")

Correct under any commit order, and UniqueConstraint(release, spec_version) is already the backstop. Note the tempting alternative — a released_in / consumed flag on SpecVersion — is not available: SpecVersion.save() rejects every write to a locked row, so the anti-join has to live on the releases side.

This matters now rather than at child 3, because the doc locks lock_watermark into the data model with child 1 shipping it in 0001_initial. Either drop the column from the locked model and leave the selection strategy open for child 3, or switch the recommendation to the anti-join and drop the column in the same edit. Ordering, cut_at semantics and the manifest contract are unaffected either way.

2. Two grounded claims that do not match the tree

• Proposal is not unique on (tenant, feature_id). "What grounding changed" asserts it, but proposals/models.py Meta carries only ordering and the proposals_scope_norm_gin index — there is no UniqueConstraint anywhere in the app. The conclusion it supports (tenant-only scoping, no Project) is unaffected; the supporting claim should be corrected, since the section's value is that every line in it was verified.
• The cross-tenant get_release behaviour does not match the _get_spec_by_id() precedent. The shipped handler returns {"error": "tenant_mismatch"} for another tenant's spec_id; the doc specifies {"found": false, "release": null} for another tenant's release_id and calls it a match. Only the tool.tenant_mismatch warning log matches — the response shape is the opposite. The doc's choice is the better one (it does not confirm the id exists), so state it as a deliberate divergence and file a follow-up to align get_spec, rather than describing it as precedent-following. As written, the MCP surface ships two different answers to the same question.

3. Structured-error translation needs a placement note (child 3)

Both nowait=True and UniqueConstraint(tenant, version) surface as exceptions that abort the Postgres transaction. Catching them inside transaction.atomic() and returning CONCURRENT_CUT / VERSION_TAKEN raises TransactionManagementError instead — the translation has to happen outside the block. Worth one line, since the doc names both translations without saying where they happen. Related: the nowait failure should be discriminated on pgcode 55P03 (lock_not_available) rather than a bare DatabaseError, or a genuine database outage gets reported to the caller as a concurrent cut.

4. Error codes without a test row

ReleaseErrorCode is locked with nine values, but the testing-strategy table names tests only for the AC-derived ones. ALREADY_LOCKED, PROPOSAL_NOT_ACCEPTED, TENANT_MISMATCH (lock path), VERSION_TAKEN and VERSION_EXHAUSTED have no row. Add a row each, or a line assigning them to a child — an enumerated code with no named test tends to arrive as an unstructured 500.

Nits

• tenants/models.py importing VersioningScheme from releases/versioning.py inverts app layering: a base app gains a dependency on a pillar app. The doc's cycle argument is sound and it will work, but backend/commons already exists as the shared home and would avoid the inversion.
• list_releases keyset pagination on cut_at alone: no unique constraint backs that column. The tenant row lock makes a collision unlikely in practice, but a (cut_at, release_id) cursor removes the skip/duplicate edge case for free.
• The doc says ibuki-backend "is at #31"; it is now at #32. Immaterial — the children take the next free numbers.

Summary

The grounding pass is thorough and holds on every point I could check against the shipped tree, and the doc is a materially better artifact than the epic body it replaces. Item 1 is the one thing worth fixing before merge, because it is locked into a migration child 1 ships. Items 2–4 are edits to the doc's own text. Once item 1 is resolved, this is ready to merge and the four children can be filed.