toasterson/Anima

Watch

Fork 0

PR-backed document work items advance past the working AI gate and can never obtain the approval merge demands #752

Open

toasterson wants to merge 4 commits from claude/wi-01a0c921-pr-backed-document-work-items-advance-pa into main

toasterson commented

2026-09-22 12:40:55 +00:00

Owner

A document-producing WI advances working → testing on its linked findings Document. Correct for an artifact-only finding — no diff, the Document is the deliverable.

Wrong when the item also carries a canonical PR. It reaches testing with no working-phase review, and approve_and_merge then refuses it:

working-phase AI gate has not approved current PR head <sha>

The review-dispatch query selects only current_phase = 'working', so once advanced the item can never obtain that approval — permanently unmergeable with no supported route forward.

Observed: Akh-Medu WI-225 — PR #238 open, non-draft, mergeable, CI green on exact head 3827e22, machine_cleared: true, yet ai_gate_cleared: false, review_rounds: 0, last_reviewed_diff_hash stale at e0640660.

Why the lifecycle gate never fired

The advance does not reach check_gate. scheduler.rs::reconcile_deliverable_wis calls WorkItemRepo::advance_to_testing directly once a linked Document exists. I found this the hard way: a first cut that changed only the lifecycle gate compiled, kept every existing test green, and did not fix the bug — the new regressions caught it.

Fix — two sites, same rule

scheduler.rs — the reconcile holds a PR-backed document item in working, the only phase where a reviewer can still be scheduled, until the working-stage AI gate approves its current head. A lookup failure declines to advance rather than advancing blind.
work_item_lifecycle.rs — the deliverable-head gate's exemption changes from "kind produces a document" to "artifact-only" (document kind and no PR). Scoped to that one gate: an early return would also have skipped the local exact-head-CI check and the exit-bar evaluation below it.

This withholds an advance. It never grants an approval, never fabricates a Document, and leaves artifact-only closure exactly as before.

Recovery for already-stuck items

Return them testing → working through the supported phase write; normal review then applies against the correct working phase. No cross-phase verdict plumbing, no operator gate verb, no approval bypass.

Verification — real Postgres

deliverable_exit_bar_test 9 passed:

PR-backed document WI cannot advance before current-head approval
advances once that exact head is approved
an approval pinned to a superseded head does not satisfy the gate (reviewed e0640660 / head 3827e22 — WI-225's actual shape)
an item returned testing → working is held until its current head is approved, then advances
the three pre-existing tests still pass, including a no-PR document WI advancing on its Document alone

Adjacent suites green: document_deliverable_test 8, stale_gate_test 16, ai_gate_bypass_test 5.

The diff is three files / 264 lines with no rustfmt churn — cargo fmt -p anima-server rewrites 61 files of pre-existing repo drift, so it was deliberately not run.

Gates not pre-cleared; fresh exact-head review and CI required.

🤖 Generated with Claude Code

A document-producing WI advances `working → testing` on its linked findings Document. Correct for an **artifact-only** finding — no diff, the Document *is* the deliverable. Wrong when the item **also carries a canonical PR**. It reaches `testing` with no working-phase review, and `approve_and_merge` then refuses it: > `working-phase AI gate has not approved current PR head <sha>` The review-dispatch query selects only `current_phase = 'working'`, so once advanced the item can **never** obtain that approval — permanently unmergeable with no supported route forward. **Observed:** Akh-Medu WI-225 — PR #238 open, non-draft, **mergeable**, CI **green on exact head `3827e22`**, `machine_cleared: true`, yet `ai_gate_cleared: false`, `review_rounds: 0`, `last_reviewed_diff_hash` stale at `e0640660`. ## Why the lifecycle gate never fired The advance does not reach `check_gate`. `scheduler.rs::reconcile_deliverable_wis` calls `WorkItemRepo::advance_to_testing` **directly** once a linked Document exists. I found this the hard way: a first cut that changed only the lifecycle gate compiled, kept every existing test green, and did **not** fix the bug — the new regressions caught it. ## Fix — two sites, same rule - **`scheduler.rs`** — the reconcile holds a PR-backed document item in `working`, the only phase where a reviewer can still be scheduled, until the working-stage AI gate approves its **current** head. A lookup failure declines to advance rather than advancing blind. - **`work_item_lifecycle.rs`** — the deliverable-head gate's exemption changes from *"kind produces a document"* to *"artifact-only"* (document kind **and** no PR). Scoped to that one gate: an early return would also have skipped the local exact-head-CI check and the exit-bar evaluation below it. This **withholds an advance**. It never grants an approval, never fabricates a Document, and leaves artifact-only closure exactly as before. ## Recovery for already-stuck items Return them `testing → working` through the supported phase write; normal review then applies against the correct working phase. No cross-phase verdict plumbing, no operator gate verb, no approval bypass. ## Verification — real Postgres `deliverable_exit_bar_test` **9 passed**: - PR-backed document WI cannot advance before current-head approval - advances once that exact head is approved - an approval pinned to a **superseded** head does not satisfy the gate (reviewed `e0640660` / head `3827e22` — WI-225's actual shape) - an item returned `testing → working` is held until its current head is approved, then advances - the three pre-existing tests still pass, including a **no-PR document WI advancing on its Document alone** Adjacent suites green: `document_deliverable_test` 8, `stale_gate_test` 16, `ai_gate_bypass_test` 5. The diff is three files / 264 lines with no rustfmt churn — `cargo fmt -p anima-server` rewrites 61 files of pre-existing repo drift, so it was deliberately not run. Gates not pre-cleared; fresh exact-head review and CI required. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

toasterson added 1 commit

2026-09-22 12:40:55 +00:00

WI-225: a PR-backed document work item must clear the working AI gate 56640beaf6

A document-producing WI advances `working → testing` on its linked findings
Document. That is right for an artifact-only finding: there is no diff, and
the Document is the deliverable.

It is wrong when the item also carries a canonical PR. The item reached
`testing` with no working-phase review, and `approve_and_merge` then refused
it with "working-phase AI gate has not approved current PR head". The
review-dispatch query selects only `current_phase = 'working'`, so once
advanced the item could never obtain that approval. WI-225 sat in exactly
that state: green CI on its exact head 3827e22, a mergeable PR #238, and no
supported route to the approval merge demanded.

The advance was not going through the lifecycle gate at all. The scheduler's
deliverable reconcile calls `WorkItemRepo::advance_to_testing` directly after
confirming a linked Document, bypassing `check_gate`. Two sites therefore
needed the same rule, and they now agree:

  - scheduler.rs: the reconcile holds a PR-backed document item in `working`
    — the only phase where a reviewer can still be scheduled — until the
    working-stage AI gate has approved its CURRENT head. A lookup failure
    declines to advance rather than advancing blind.
  - work_item_lifecycle.rs: the deliverable-head gate's exemption changes
    from "kind produces a document" to "artifact-only" — a document item with
    no PR, as before. The skip stays scoped to that one gate; an early return
    would also have skipped the local exact-head-CI check and the exit-bar
    evaluation below it.

This withholds an advance. It never grants an approval, never fabricates a
Document, and leaves artifact-only closure exactly as it was. Items already
stuck in `testing` recover by being returned to `working` through the
supported phase write, after which normal review applies against the correct
working phase — no cross-phase verdict plumbing, and no operator gate verb.

Tests, against real Postgres (9 passed in this file):
  - a PR-backed document WI cannot advance before current-head approval
  - it does advance once that exact head is approved
  - an approval pinned to a superseded head does not satisfy the gate
    (reviewed e0640660 / head 3827e22 — WI-225's actual shape)
  - an item returned from `testing` to `working` is held until its current
    head is approved, then advances
The three pre-existing tests still pass, including the no-PR document WI
advancing on its Document alone.

Adjacent suites green: document_deliverable_test 8, stale_gate_test 16,
ai_gate_bypass_test 5.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

toasterson added 1 commit

2026-09-22 13:11:35 +00:00

fix(scheduler): route the deliverable reconcile through the lifecycle service 222f72b78e

Review of the first WI-225 fix found two defects in the scheduler-local
gate it introduced, both now removed with the helper itself.

TOCTOU. `pr_backed_document_awaiting_review` read the PR head, the
workflow's gate configuration and the pinned approval in three separate
unlocked `pool` queries, then advanced with a bare
`WorkItemRepo::advance_to_testing` UPDATE. A head published between the
read and the write was invisible, so the item advanced carrying an
approval for a head that was no longer current — the exact state WI-225
was found in.

Weaker than the authoritative check. The helper accepted an approval from
ANY working AI phase. The lifecycle SQL pins it to `MAX(phase_order)`
among the working AI phases at or before the current phase, so on a
workflow with more than one, an approval obtained at an earlier phase let
the item walk straight past a later gate.

Both are fixed by deleting the duplicate and calling
`WorkItemLifecycleService::advance_satisfied_exit_bar`, which opens one
transaction, takes the implementation-allocation lock and the work-item
row lock, recomputes the exit bar (including the linked-Document override
for document kinds) and runs the authoritative `check_gate`. The
reconcile now agrees with `effective_pipeline_gates_for_head` — the query
`approve_and_merge` evaluates — by construction rather than by copy.

The service advances one graph phase per call, and the `working` stage is
several phases wide since the quality-gate inversion (Implementation → CI
→ AI Review). `advance_to_testing` jumped that whole span in one UPDATE,
skipping those phases outright; the reconcile now walks it, re-evaluating
the gate at every hop, and stops as soon as the item leaves `working` —
so it never runs an item past `testing`.

Tests (11/11, `deliverable_exit_bar_test`, already a registered database
target):

- `an_earlier_working_phase_approval_does_not_satisfy_a_later_gate`
  publishes a graph version with two AI-gated working phases (the seeded
  graph has one, and phases referenced by a version are structurally
  immutable), approves the earlier one for the item's exact current head,
  and asserts the item is held.
- `a_head_published_during_the_advance_is_not_bypassed` holds the
  allocation lock across a publication while the reconcile runs, and
  asserts the new head is observed.
- Both fail against the previous commit's scheduler, reaching `testing`
  where they must stay in `working`.
- `an_item_returned_to_working_is_held_until_its_current_head_is_approved`
  no longer writes `current_phase` by hand. It reaches `testing` through
  the real reconcile and returns through
  `WorkItemLifecycleService::transition` — the call `set_work_item_phase`
  makes — and asserts the return is recorded in phase history.

Also re-indents the `artifact_only_document` block from the previous
commit; no logic change there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

toasterson added 2 commits

2026-10-01 20:08:36 +00:00

Merge remote-tracking branch 'origin/main' into codex/wi883-review-gate-20261001 b042dc1a26

fix: keep document reconciliation within reviewed working gates 46b98fbd06

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin claude/wi-01a0c921-pr-backed-document-work-items-advance-pa:claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

git switch claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

git switch claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

git rebase main

git switch main

git merge --ff-only claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

git switch claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

git rebase main

git switch main

git merge --no-ff claude/wi-01a0c921-pr-backed-document-work-items-advance-pa

git switch main

git merge --squash claude/wi-01a0c921-pr-backed-document-work-items-advance-pa