Review custody for chg-dispatched reviews (US-47.5)

.claude/skills/chain-of-custody/SKILL.md

@@ -22,10 +22,10 @@ gated, so escalating to a bigger key never clears a 403 there:
 
 | Endpoint | Gate |
 |----------|------|
-| `verify` / `reject` / `force_unclaim` / `verify_all` | `exact_role: :orchestrator` (`story_verification_controller.ex:28-29`) — a superadmin key CANNOT verify |
-| `contract` / `report` | `exact_role: [:agent, :orchestrator]` (`story_status_controller.ex:26-30`) — a human user key CANNOT report |
+| `verify` / `reject` / `force_unclaim` / `verify_all` | `exact_role: :orchestrator` (`story_verification_controller.ex:30-31`) — a superadmin key CANNOT verify |
+| `contract` / `report` | `exact_role: [:agent, :orchestrator]` (`story_status_controller.ex:48-52`) — a human user key CANNOT report |
 | `claim` / `start` / `request-review` / `unclaim` / `renew-claim` | `exact_role: :agent` (`story_status_controller.ex`) |
-| `review-complete` | `exact_role: [:orchestrator, :user]` (`review_record_controller.ex:22-23`) — an agent key never reaches the controller |
+| `review-complete` | `exact_role: [:orchestrator, :user]` (`review_record_controller.ex:24-25`) — an agent key never reaches the controller |
 
 **Anti-pattern: never "normalize" an `exact_role` custody gate to `role:`.** That is exactly what
 would let one high-privilege key both implement and report/verify.
@@ -42,7 +42,7 @@ Human-rooted operations additionally pass `RequireHumanAnchor`
 
 It is not a per-request WebAuthn check — WebAuthn is how a tenant EARNS `:human_anchored`. Keying on
 tier rather than role is what closes the self-minted-key bypass. **The plug is in NO router pipeline —
-it is opt-in per controller** (e.g. `story_verification_controller.ex:35`, `dispatch_controller.ex:16`,
+it is opt-in per controller** (e.g. `story_verification_controller.ex:37`, `dispatch_controller.ex:16`,
 `artifact_report_controller.ex:30`), so a new human-rooted endpoint gets NO trust-tier gate unless you
 add the plug line yourself.
 
@@ -96,8 +96,8 @@ caller's lineage is always resolved SERVER-SIDE from the authenticating key
   `get_dispatch_lineage/2`) fails **CLOSED**, and the `assigned_agent_id`
   equality check runs IN ADDITION to the lineage comparison rather than being short-circuited by it.
   `verifier_dispatch_id` is written only by the assign-verifier flow (`assign_rotating_verifier/3`,
-  `progress.ex:732-779`); that write is result-checked, and a failure flags `verifier_needed` plus a
-  `verifier_not_assigned` audit event (`flag_verifier_needed/5`, `progress.ex:784`) instead of
+  `progress.ex:755-802`); that write is result-checked, and a failure flags `verifier_needed` plus a
+  `verifier_not_assigned` audit event (`flag_verifier_needed/5`, `progress.ex:807`) instead of
   silently leaving the field nil. `request-review` is OPTIONAL, so most stories reach verify with no
   verifier dispatch — the CALLER-lineage step is what keeps that path lineage-gated.
 - **report** — `validate_not_self_report/3`. `nil` caller blocked
@@ -115,13 +115,36 @@ caller's lineage is always resolved SERVER-SIDE from the authenticating key
   user-role key also has no `agent_id` and permitting on nil alone let an ANCESTOR of the
   implementer review its own subtree; then the reviewer's lineage; then plain equality. That nil
   permit has THREE parts, all of which must change
-  together: (a) the `exact_role: [:orchestrator, :user]` plug (`review_record_controller.ex:22-23`),
+  together: (a) the `exact_role: [:orchestrator, :user]` plug (`review_record_controller.ex:24-25`),
   which 403s an agent key before the controller runs — so the `:agent` branch of the controller cond
   is unreachable today; (b) `LoopctlWeb.ReviewRecordController.create/2`
-  (`lib/loopctl_web/controllers/review_record_controller.ex:92-116`), which rejects an orchestrator
+  (`lib/loopctl_web/controllers/review_record_controller.ex:125-151`), which rejects an orchestrator
   key with no `agent_id` and passes a literal `nil` (there is NO sentinel) for a user key; (c) the
   `Progress` permit itself.
 
+**chg-dispatched reviews (US-47.5) — `Loopctl.Changes.ReviewCustody`.** On a story with a change
+binding, review-complete additionally demands (after the three gates above, never instead) a
+reviewer whose LIVE lineage is a STRICT descendant of the chg root of the source that delivered
+the change (`chg_reviewer_required`; the root key too), a `commit_sha` that is an applied
+`submitted` revision of the CURRENT binding (`commit_sha_not_bound`), and one verdict per reviewer
+dispatch — a unique index, not a read (`chg_review_already_recorded`). Its mirror: a caller whose
+MINTING lineage is rooted at any registered `chg_root_dispatch_id` (live or revoked source, either
+mode) is refused `chg_root_cannot_verify` on verify / reject / verify-all / bulk verify+reject /
+backfill / bulk mark-complete and `chg_root_review_only` on claim / bulk claim / report, before
+any story is read. MINTING, because an expired chg key's live lineage is `[]` and reject admits
+`[]`. Neither refusal records a custody violation. The lineage gate always passes a chg
+reviewer, so it proves only that chg-server MINTED the key — separation of the job from the
+implementer is chg-server's to construct (PRD §6). A landing whose revision (`event.sha`) no chg
+review names is escalated over the control-only `:review_missing` in the landed event's own
+transaction (`EventEffects.after_move/5`), FAIL CLOSED: a refused hold rolls the landing back
+so chg redelivers it. The verify half is ONE function every path to `verified` calls —
+verify, verify-all, bulk verify, backfill, bulk mark-complete —
+`ReviewCustody.ensure_landed_review/3`: on a chg story it refuses `review_missing_escalated`
+while the stage is escalated over `:review_missing`, and `chg_review_not_for_landed_revision`
+unless a chg review names the LANDED revision (the `sha` of the newest applied `landed` event
+of the current binding). Escalation in general does NOT block verify; only this edge does.
+Neither refusal records a custody violation.
+
 **Custody-orphan predicates are the ONLY guard for the never-dispatched case.** The DB CHECK
 `stories_reported_done_requires_agent` is satisfied whenever `implementer_dispatch_id IS NULL`, which
 is exactly the orphaned shape — do not delete `custody_orphaned?/1` or `custody_unattributed?/1` as
@@ -248,9 +271,9 @@ correct behavior; do not add a workaround.
   a read-then-write.
 
 **Enforcement is conditional — this is the deprecation seam.** `Progress.maybe_consume_cap/6`
-(`progress.ex:540-563`) is what actually gates the custody ops: a `nil` `cap_id` is rejected with
+(`progress.ex:563-586`) is what actually gates the custody ops: a `nil` `cap_id` is rejected with
 `:missing_capability` **only for tenants that have an audit key** (`tenant_has_audit_key?/1`,
-`progress.ex:812-817`); a pre-v2 (keyless) tenant returns `{:ok, :pre_v2_tenant}` and the operation
+`progress.ex:835-840`); a pre-v2 (keyless) tenant returns `{:ok, :pre_v2_tenant}` and the operation
 proceeds with NO capability at all. So L1 strength is per-tenant. A REJECTED cap is split by
 `cap_refusal/4`: only `:invalid_signature` / `:replay` surface as `{:cap_rejected, _}`, which
 FallbackController answers with a plain 403 — it halts NOTHING and counts toward nothing (see the
@@ -297,7 +320,7 @@ exist, but every one descends from the implementer's root — the single-root te
 not a shortage; its remedy is the operator minting an independently-rooted verifier tree.
 
 **Empty-lineage caveat, in BOTH directions.** When the implementer dispatch cannot be loaded,
-`assign_rotating_verifier/3` passes `[]` (`progress.ex:732-736`), and with `[]` the rejection is
+`assign_rotating_verifier/3` passes `[]` (`progress.ex:755-759`), and with `[]` the rejection is
 inert — selection can then pick a same-lineage (even the implementer's own) dispatch. The verify-time
 comparison is fail-closed on an empty lineage, so this is caught at verify rather than at selection;
 do not "simplify" either half.

CHANGELOG.md

@@ -81,6 +81,30 @@ All notable changes to loopctl are documented here.
   Migration `20261019160000_add_change_stage_mapping` adds `story_stages.trunk_verified_sha`
   and the `change_revert_issues` table; additive, no deploy ordering. Each chained stage
   retraction now also names `release_id` and `trunk_verified_sha` under `retracted`.
+- **Review custody for chg-dispatched reviews (US-47.5).** On a story bound to a chg change,
+  `POST /api/v1/stories/:id/review-complete` accepts the review only from a key whose dispatch
+  is a strict descendant of the intake source's `chg_root_dispatch_id` (409
+  `chg_reviewer_required`, chg-server's root key included), requires a new body field
+  `commit_sha` naming a submitted revision of the story's current change (422
+  `commit_sha_not_bound`), and records one verdict per reviewer dispatch (409
+  `chg_review_already_recorded`, held by a unique index). These run after the existing custody
+  gates, whose codes are unchanged; on any other story `commit_sha` is ignored and not stored.
+  Review records now serialise `commit_sha` and `reviewer_dispatch_id`. A chg story that
+  reaches `merged` with no such review for its landed revision is escalated in the same
+  transaction over a new control-only edge `review_missing` (`merged -> escalated`; the merge
+  stands) and waits for a human; if that escalation cannot be written the whole `landed` event
+  rolls back (503 `busy` on contention, else 500 `change_event_failed`, logged and counted as
+  `[:loopctl, :changes, :review_hold_refused]`) and chg redelivers it, so a merge never commits
+  without its hold. Such a story is never verified: `POST /stories/:id/verify`,
+  `/epics/:id/verify-all`, `/stories/bulk/verify`, `/stories/:id/backfill` and
+  `/stories/bulk/mark-complete` refuse a story bound to a chg change with 409
+  `review_missing_escalated` while it is escalated over `review_missing` (a human resolves it
+  first), and with 409 `chg_review_not_for_landed_revision` unless a chg review names its
+  LANDED revision — the revision commit of the newest applied `landed` event of its current
+  change (verify-all and the bulk paths report either per story). Stories with no change
+  binding are answered exactly as before. Migration `20261019170000_add_chg_review_custody_to_review_records`
+  adds the two nullable columns, a CHECK that they are set together, and the unique index;
+  additive, no deploy ordering. MCP: loopctl-mcp-server 3.3.0.
 - **Intake-source mode `chg` (US-47.2).** `POST /api/v1/intake/sources` and `PATCH
   /api/v1/intake/sources/:id` accept `mode: "chg"` beside `pr`, plus two fields a `chg`
   source must carry: `change_events_secret` (the HMAC key chg-server will sign its event
@@ -419,6 +443,18 @@ All notable changes to loopctl are documented here.
 
 ### Changed
 
+- **chg-server's keys never certify or implement (US-47.5).** A key whose MINTING lineage
+  (revoked or not) is rooted at a dispatch any intake source of the tenant registers as
+  `chg_root_dispatch_id` — live or revoked source, either mode — is refused 409
+  `chg_root_cannot_verify` by `POST /stories/:id/verify`, `/reject` (exempt from
+  `caller_lineage_required` only), `/backfill`, `POST /epics/:id/verify-all` and
+  `POST /stories/bulk/verify`, `/bulk/reject` and `/bulk/mark-complete` (the bulk paths and
+  verify-all refuse the whole call), and 409 `chg_root_review_only` by `POST /stories/:id/claim`,
+  `/report` and `/bulk/claim`. No other caller's answer changes.
+- **`POST /api/v1/stories/:id/review-complete` answers `caller_lineage_required` as the
+  documented 409.** The controller had no clause for it, so a key no dispatch minted reviewing
+  dispatch-minted work got a 500.
+
 - **The app's HTTP listener no longer serves HTTP/2.** Bandit now closes a connection that
   opens with the HTTP/2 preface, which keeps CVE-2026-105659 (HIGH, Bandit's HTTP/2 stack,
   no patched release) unreachable. Fly is unaffected: its proxy already speaks HTTP/1.1 to

CLAUDE.md

@@ -124,10 +124,10 @@ does NOT clear a 403 there:
 
 | Endpoint | Gate | Consequence |
 |----------|------|-------------|
-| `verify` / `reject` / `force_unclaim` / `verify_all` | `exact_role: :orchestrator` (`lib/loopctl_web/controllers/story_verification_controller.ex:28-29`) | a `:user` or `:superadmin` key CANNOT verify |
-| `contract` / `report` | `exact_role: [:agent, :orchestrator]` (`lib/loopctl_web/controllers/story_status_controller.ex:26-30`) | a human `:user` key CANNOT report |
-| `claim` / `start` / `request-review` / `unclaim` | `exact_role: :agent` (`:32-33`) | orchestrator/user keys cannot claim or start |
-| `review-complete` | `exact_role: [:orchestrator, :user]` (`lib/loopctl_web/controllers/review_record_controller.ex:22-23`) | an `:agent` key is 403'd BEFORE any controller logic runs |
+| `verify` / `reject` / `force_unclaim` / `verify_all` | `exact_role: :orchestrator` (`lib/loopctl_web/controllers/story_verification_controller.ex:30-31`) | a `:user` or `:superadmin` key CANNOT verify |
+| `contract` / `report` | `exact_role: [:agent, :orchestrator]` (`lib/loopctl_web/controllers/story_status_controller.ex:48-52`) | a human `:user` key CANNOT report |
+| `claim` / `start` / `request-review` / `unclaim` | `exact_role: :agent` (`:58-60`) | orchestrator/user keys cannot claim or start |
+| `review-complete` | `exact_role: [:orchestrator, :user]` (`lib/loopctl_web/controllers/review_record_controller.ex:24-25`) | an `:agent` key is 403'd BEFORE any controller logic runs |
 
 **Anti-pattern: never convert an `exact_role` custody gate to `role:`.** That is precisely what
 lets one high-privilege key both implement and report/verify — the failure the product exists to
@@ -176,9 +176,9 @@ in order:
    story that lacks a verifier dispatch.
 
 `verifier_dispatch_id` is written only by the assign-verifier flow
-(`assign_rotating_verifier/3`, `progress.ex:732-779`); that write is checked, and a failure
+(`assign_rotating_verifier/3`, `progress.ex:755-802`); that write is checked, and a failure
 flags `verifier_needed` plus a `verifier_not_assigned` audit event
-(`flag_verifier_needed/5`, `progress.ex:784`) rather than silently leaving the field nil.
+(`flag_verifier_needed/5`, `progress.ex:807`) rather than silently leaving the field nil.
 Because `request-review` is OPTIONAL, a story often reaches verify with no verifier dispatch
 at all — step 3 is what keeps that path lineage-gated instead of a bare agent-id inequality.
 
@@ -203,11 +203,11 @@ user-role key also carries no `agent_id`, and permitting on nil alone let an ANC
 implementer rubber-stamp its own subtree without the lineage ever being read. That permit has
 THREE parts which must change together:
 1. the `exact_role: [:orchestrator, :user]` plug
-   (`lib/loopctl_web/controllers/review_record_controller.ex:22-23`), which 403s an `:agent` key
+   (`lib/loopctl_web/controllers/review_record_controller.ex:24-25`), which 403s an `:agent` key
    before any controller code runs — so the `:agent` branch of the controller cond below is
    unreachable today;
 2. `LoopctlWeb.ReviewRecordController.create/2`
-   (`lib/loopctl_web/controllers/review_record_controller.ex:92-116`), which rejects an
+   (`lib/loopctl_web/controllers/review_record_controller.ex:125-151`), which rejects an
    orchestrator key carrying no `agent_id` and passes a literal `nil` (there is NO "human
    operator" sentinel) for a user key;
 3. the `Progress` permit itself.
@@ -231,6 +231,46 @@ and whole-map-replaced by `PATCH /api/v1/stories/:id`** — one ordinary request
 marker and restored the claim → force-unclaim → backfill-to-verified launder. Never add
 `:lifecycle_entered_at` to a changeset `cast` list.
 
+**chg-dispatched reviews (US-47.5)** — `Loopctl.Changes.ReviewCustody`
+(`lib/loopctl/changes/review_custody.ex`). A story with a change binding is a chg story, and
+its chg root is the `chg_root_dispatch_id` of the one intake source that delivered the bound
+change's events (any other count: no root, nobody qualifies). On such a story
+`record_review/4` runs `chg_review/4` (`lib/loopctl/progress.ex:2458`) AFTER
+`validate_not_self_review/3`, never instead: the reviewer's LIVE lineage must be a STRICT
+descendant of the root (`chg_reviewer_required`, the root's own key included), then
+`commit_sha` must be the sha of an APPLIED `submitted` event of the CURRENT binding
+(`commit_sha_not_bound`); one reviewer dispatch records one verdict, held by the unique index
+`review_records_reviewer_dispatch_uidx` (`chg_review_already_recorded`). On any other story
+`commit_sha` is not stored, so a review recorded before binding never counts as a chg review.
+The other half: a caller whose MINTING lineage (`Dispatches.minting_lineage_for_api_key/2`,
+revoked or not — an expired chg key's live lineage is `[]`, which reject would accept) is
+rooted at a `chg_root_dispatch_id` ANY intake source of the tenant names, live or revoked,
+either mode, is refused `chg_root_cannot_verify` by verify, reject, verify-all, bulk
+verify/reject, backfill and bulk mark-complete (`refuse_chg_root_verifier/2`,
+`progress.ex:2611`; `BulkOperations.refuse_chg_root/3`, `bulk_operations.ex:389`) and
+`chg_root_review_only` by claim, bulk claim and report (`refuse_chg_root_actor/3`,
+`progress.ex:272`). Both halves read the lineage through ONE function,
+`ReviewCustody.caller_lineage/2` (minting lineage, else the call's live one), and the
+controllers resolve it through ONE helper, `LoopctlWeb.CustodyLineage.chg_root_opts/1`. The refusal is judged before any story is read, so no other caller's
+answer changes, and it records no custody violation. What the chg gate does NOT prove: every
+chg reviewer key sits under chg's root and outside every implementer's chain, so lineage
+always passes it — loopctl proves chg-server minted the key, and chg-server is trusted to run
+the job apart from the implementer (PRD §6). A landing with no chg review for its landed
+revision (`event.sha`, the revision commit — never `data.merge_sha`) is escalated in the
+landed event's own transaction over the control-only edge `:review_missing`
+(`EventEffects.after_move/5`, `lib/loopctl/changes/event_effects.ex:493`), and that hold
+fails CLOSED: a refused escalation rolls the whole landing back (`:busy`, or
+`change_event_failed`, logged and counted) so chg redelivers it — the merge never commits
+without its hold. Such a story is never verified: every path that reaches `verified` —
+verify and verify-all through `chg_landed_review/2` (`progress.ex:2698`), bulk verify
+through `process_verify/3` (`bulk_operations.ex:421`), backfill and bulk mark-complete
+through `guard_backfill/2` (`progress.ex:2792`) — calls one function,
+`ReviewCustody.ensure_landed_review/3` (`lib/loopctl/changes/review_custody.ex:215`). It refuses a chg story with
+`review_missing_escalated` while its stage is escalated over `:review_missing` (a human
+resolves it) and with `chg_review_not_for_landed_revision` unless a chg review names its
+LANDED revision: the `sha` of the newest APPLIED `landed` event of its current binding. A
+story with no binding passes untouched.
+
 **The MCP server must NEVER hold both implementer and reviewer keys in the same process.** The 409 errors are the system working correctly — do not add workarounds.
 
 ### Before Changing Any Role Requirement