Phase 3: live results, spec corrections, Sonnet 5.5 for all passes

docs/build-plan.md

@@ -55,7 +55,7 @@ Tasks:
 Owner:
 
 - [x] Owner: create a scratch source repo in the org for the V tests, separate from claude-review-test ([claude-review-p0-source](https://github.com/OperationCode/claude-review-p0-source)).
-- [x] Owner: Anthropic Console: a dedicated workspace with a hard monthly spend limit and a service account. Create the production federation rule as in spec "Auth and cost": pattern mode, subject prefix `repo:OperationCode@10479364/*`, scope `workspace:developer`, the review workspace only, and the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back; if the Console rejects the placeholder, stop and resolve the order of rule ID and SHA first (spec V26 a). Its ID goes into the central code; phase 3 sets the candidate SHA. No API key. Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created; the Console accepted the placeholder SHA (V26 a). The read-back is not recorded yet; do it before V26 b.
+- [ ] Owner: Anthropic Console: a dedicated workspace with a hard monthly spend limit and a service account. Create the production federation rule as in spec "Auth and cost": pattern mode, subject prefix `repo:OperationCode@10479364/*`, scope `workspace:developer`, the review workspace only, and the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back; if the Console rejects the placeholder, stop and resolve the order of rule ID and SHA first (spec V26 a). Its ID goes into the central code; phase 3 sets the candidate SHA. No API key. Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created; the Console accepted the placeholder SHA (V26 a partial). The read-back output is not recorded, so this item stays open; the later V26 b exchange does not replace it.
 - [ ] Owner: the test workspace (reused for evals, spec P2-1), and a federation rule for `workflow_dispatch` runs in `OperationCode/.github` whose job uses the environment `eval`: pattern mode, the same subject prefix, exact claims `repository_owner_id`, `repository_id`, `event_name` and `environment`, scope `workspace:developer`, that workspace only (spec "Auth and cost"). Its IDs are in `evals.py`. The rule cannot name `eval.yml`: any `.github` workflow that uses `eval` can match, so the owner's approval and the workflow unit test (1c) hold it to `eval.yml`. Test it as in spec V27 before phase 2.
 - [ ] Owner: on `.github`, create the GitHub environment `eval`: the owner is a required reviewer for every run, and any branch may deploy to it. "Prevent self-review" is off temporarily (owner, 2026-10-26), so that the coordinator, which runs with the owner's token, can approve eval runs it started from reviewed branches; the owner turns it on again in phase 5. Admin bypass stays on. During phase 2 the coordinator's approval stands in for the owner's under the owner's delegation. Before approving a run, check the run that waits for approval: the workflow file and commit it runs, every job in it that uses `eval`, and their effective permissions (no GitHub write). Also check that the PR does not change `eval.yml`, any other workflow file, the eval code, the prompts, the fixtures or the federation setup in a harmful way, and that `test.yml` passed. The approval is the control; the residual risk is an approver who approves without reading. Exception to the no-execute rule (K-4): evals of PRs to this repo may run that PR's code, only after an owner approves the `eval` environment run. Needed before phase 2.
 - [x] Owner: a test workspace with a very small spend limit for V9.
@@ -64,7 +64,7 @@ Owner:
 
 Exit criteria:
 
-- [x] V6 shows that the newest run on the head SHA decides. If not, building stops. (The queued cell is still open.)
+- [x] V6 shows that the newest run on the head SHA (among runs at the pinned SHA, see spec P3-3) decides. If not, building stops. (The queued cell is still open.)
 - [x] V2 and V3 pass. If not, building stops.
 - [x] V4, V5, V9, V13, V16, V18 and V24 have results in the spec. A fallback in the spec covers each one that failed.
 
@@ -92,7 +92,7 @@ Depends on: none for most tasks; V18 for `collect.py`, V16 for the push epoch in
 
 ### 1b. Analyze and publish
 
-- [x] `analyze.py`: one Messages API call per pass, no tools, `claude-opus-5-5`, explicit `effort` and `max_tokens`, structured output with the stripped schema and a full schema check in code (V5), fallback, at most 2 retries, a request deadline, stale check before the calls. The spend limit error (HTTP 400 `invalid_request_error`, "workspace API usage limits") is an outage with its own clear reason, not a request bug (V9). Always writes exactly three pass records, and exits 0 when it wrote them. The pass record has the optional `meta` field (reason, model, stop reason, schema digest, token usage). Federation goes through the SDK's `WorkloadIdentityCredentials`; placeholder IDs make it refuse to call, and the `release`-marked test (`pytest -m release`) fails on them; phase 3 set the production IDs. Tests use a fake client.
+- [x] `analyze.py`: one Messages API call per pass, no tools, the model set per pass in code (`claude-sonnet-5-5` on every pass, see spec P3-4), explicit `effort` and `max_tokens`, structured output with the stripped schema and a full schema check in code (V5), fallback, at most 2 retries, a request deadline, stale check before the calls. The spend limit error (HTTP 400 `invalid_request_error`, "workspace API usage limits") is an outage with its own clear reason, not a request bug (V9). Always writes exactly three pass records, and exits 0 when it wrote them. The pass record has the optional `meta` field (reason, model, stop reason, schema digest, token usage). Federation goes through the SDK's `WorkloadIdentityCredentials`; placeholder IDs make it refuse to call, and the `release`-marked test (`pytest -m release`) fails on them; phase 3 set the production IDs. Tests use a fake client.
 - [x] `publish.py`: escaping, inline `COMMENT` review, sticky summary, final check, exit code (0 only for `pass`). It never approves, requests changes, merges, closes, labels or edits the PR (spec decision K-1). A summary over the comment limit is cut in the spec's order, or fails closed with the full record in the gate job log. The summary shows token counts, no currency.
 
 Depends on: V3, V5, V9 (analyze); V6, V16, V24 (publish).
@@ -151,22 +151,27 @@ Exit criteria:
 Run the spec's workflow acceptance tests under a test org ruleset that requires the central workflow from `OperationCode/.github` at the candidate release SHA and targets only claude-review-test. Only one ruleset requires the workflow there at a time.
 
 - [x] Production IDs in `analyze.py` and the release tests in CI (the candidate S1). R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) and the production service account are set; `test.yml` runs `pytest tests/unit -m release`.
-- [x] V26 a: R1 created at the placeholder SHA and accepted by the Console (2026-10-27); R2 `claude-review-p3-r2` created the same way, inert until the release simulation.
+- [ ] V26 a (partial): the owner created R1 at the placeholder SHA and the Console accepted it (2026-10-27); R2 `claude-review-p3-r2` was created the same way, inert until the release simulation. No read-back output is recorded for R1 or R2. Remaining: read R1 and R2 back.
 - [x] P3-2: an over-long string no longer fails a pass. Release eval run 40517202830 at S1 matched 12 of 13: one supply-chain quote went over 300 characters, which the API cannot enforce. Analyze now cuts every string over its schema `maxLength` (summary, file, quote, title, explanation) to that limit and keeps the finding (spec P3-2). Fix round 1: a cut file path ends in "/", so it names no changed file; when the file, title or quote is at its limit, the inline comment key covers every shown field, so a cut never hides a distinct explanation. Fix round 2: a redaction can make two titles equal far below the limit, so the key of every finding covers every shown field, at any length. Fix round 3: each inline comment carries the D14 key and the full key, both of the redacted shown text; one run dedupes by full key, a later run skips a D14 key already on the PR and shows the finding in full in the summary when its full key is new. Fix round 4: an earlier comment counts only when its text checks out (the D14 skip needs the inline layout, path, side and title; "see the earlier comment" needs the exact body at the same line), and an unverified high or critical finding is never cut from the summary. A candidate with this change needs its own eval.
-- [ ] V26 b, before the acceptance tests: after the owner sets the production rule to the candidate SHA, a run at that SHA gets a token and a run at another SHA is denied.
-- [ ] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`.
-- [ ] A fork PR from an account outside the org, including a draft that later becomes ready. The review starts with no approval click (V8 b).
-- [ ] V8 a: a fork PR whose own workflow has a job named `oc-claude-review` that starts before the required run.
-- [ ] V11: retarget from a non-default branch.
-- [ ] V12: repo-level Actions settings.
-- [ ] V22 a and b: pin behind main, then a pin move with PRs open.
-- [ ] V23: a same-path workflow file in the target repo.
-- [ ] V25: the pilot bypass. Maintain and Admin can merge past a failed run, Write cannot, and GitHub records each bypass. Record the role `actor_id` values, and whether a direct push to the default branch is blocked.
-- [ ] The rest of the acceptance list in the spec's "Testing" section, including the outage process. Design change (spec P3-1): the invalid federation rule case runs live with a denied exchange; the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit.
+- [x] V26 b, before the acceptance tests: after the owner sets the production rule to the candidate SHA, a run at that SHA gets a token and a run at another SHA is denied. Run 40560632129 at S1 got a token (2026-10-27, 18:31Z); run 40561015054 at the probe SHA was denied, and the Console shows 6 rows `Failure` `match_claim_value_mismatch` on R1 at 18:33:49Z (owner).
+- [ ] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`. S2 `9752ba7` (PR 10 into `p3-sim-base`) and M2 `1cefe42` both got a token (runs 40585724589 and 40587964524); the rule accepted SHAs that are only on non-default branches. Partial: accepted at S2 and M2 (token inferred through R2); R2 Console rows were not read, no R2 read-back output is recorded, and step 5a was not run for R2/M2. Remaining: R2 read-backs at the placeholder, S2 and M2, and a denied exchange at M2 before the R2 edit.
+- [x] Release eval at S1 (`3aa7ae9`): run 40528717338, 13 of 13 matched.
+- [x] M1 tree check: M1 `590a387` and S1 have the same tree, and the R1 exchange works at M1 (run 40585413003) after the denied run at M1 (run 40583290922).
+- [ ] Release simulation (R2, S2, M2): steps 1 to 6 of "Each release", never merged to main. Partial: steps ran and S2 and M2 got tokens; no R2 read-backs and no step 5a deny for R2/M2. Remaining: those two parts.
+- [ ] A fork PR from an account outside the org, including a draft that later becomes ready. The review starts with no approval click (V8 b). NOT YET RUN, pending account X.
+- [ ] V8 a: a fork PR whose own workflow has a job named `oc-claude-review` that starts before the required run. NOT YET RUN, pending account X. A same-repo version by an org admin passed (PR 36, run 40572853203 started first; the box stayed blocked).
+- [x] V11: retarget from a non-default branch. The retarget starts no run and the box stays blocked until a reopen run passes (PR 29, run 40561903948); an old pass on the same head counts (PR 30).
+- [x] V12: repo-level Actions settings. With Actions disabled, and with a restrictive allowed-actions list, the required run still ran (runs 40562372022 and 40562688524). Settings restored.
+- [x] V22 a and b: pin behind main, then a pin move with PRs open. `job.workflow_sha` equaled the pin in 33 of 33 attempts. After a forward pin move the old passes do not count; after a move back they count again (spec "Pinned SHA only"; PRs 28 and 37, 21:32 to 21:40Z).
+- [ ] V23: a same-path workflow file in the target repo. NOT YET RUN, pending account X. A same-repo version by an org admin passed (PR 35, run 40572849014).
+- [ ] V25 (NOT YET RUN, pending account X): the pilot bypass. Maintain and Admin can merge past a failed run, Write cannot, and GitHub records each bypass. Record the role `actor_id` values, and whether a direct push to the default branch is blocked.
+- [x] The rest of the acceptance list in the spec's "Testing" section that needs no account X, except the four unit-test-only cases listed in the next item (approver removed, edited override, artifact removed, gate cancel after inline comments), including the outage process (spec "Phase 3 results"). Design change (spec P3-1): the invalid federation rule case ran live with two denied exchanges (runs 40561015054 and 40583290922); the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit (ruling Q7).
+- [ ] The acceptance cases that need account X: V6 f as a Write user, and the web UI "Re-run all jobs" button on a ruleset-started run (NOT YET RUN; the override cases ran with the REST re-run). Also not run, covered by unit tests: gate cancelled after inline comments with a refused summary write, approver role removed, edited override, artifact upload removed.
+- [x] Sonnet 5.5 on every pass (spec P3-4). Eval at 302882f: runs 40589761270 and 40590079545 each matched 13 of 13 with no failed pass; the Opus reference run 40528717338 also matched 13 of 13. Total tokens 0.97x and 0.98x of Opus, output tokens 0.71x and 0.76x, eval wall time 84 s and 82 s against 138 s. The switch takes effect at the next release: the production rule pins a SHA, so merging does not change production.
 
 Owner:
 
-- [ ] Owner: set `job_workflow_ref` of the production federation rule to the candidate release SHA. The candidate's code must name that rule ID (spec "Auth and cost", "Each release").
+- [x] Owner: set `job_workflow_ref` of the production federation rule to the candidate release SHA. Done 2026-10-27 (S1, then M1 `590a387`; the run at M1 got a token; no read-back recorded). The candidate's code must name that rule ID (spec "Auth and cost", "Each release").
 - [ ] Owner: an account outside the org opens the fork PRs.
 - [ ] Owner: test users with Write, Maintain and Admin on claude-review-test for V25.
 
@@ -190,6 +195,7 @@ Exit criteria:
 - [ ] Start a run on each open PR in the pilot repos (push, or close and reopen).
 - [ ] Each week, record false positives, overrides, bypasses, failed runs by reason, egress blocks and cost per run. Keep cost numbers (any currency amount) in the owner's private notes and the Anthropic Console, never in this public repo. Public text may give ratios and timing only.
 - [ ] Fix false positives in prompts or code. Each fix goes through phase 2 again, then ships as in "Each release" below.
+- [ ] Keep the fixture eval results as an artifact: `eval.yml` uploads none today, so a release eval's per-case results exist only in the job log (phase 2 run 40445885087 and phase 3 run 40528717338 were read from logs). Ships as in "Each release".
 - [ ] Plan publication before writing: bounded continuation comments, pointed to from the sticky summary, hold every summary-only finding (changed keys, unverified, outside the diff) when one comment cannot. Then no explanation is cut from the PR comments and summary capacity no longer fails a run whose computed result is `pass` (spec "Publishing (code)", size limit residual; review r-p3-cut-final Important 1). Keep the fail-closed check until this path is complete. Ships as in "Each release".
 
 Depends on: phase 3.
@@ -243,8 +249,9 @@ One federation rule allows one SHA, and every release is validated at its final
 - [ ] Put the R2 ID in the central code. That commit is the release candidate S2.
 - [ ] Owner: set `job_workflow_ref` of R2 to S2 and read it back.
 - [ ] Validate S2, for every release: unit tests pass, the owner-approved fixture eval passes, and a run under the scratch test ruleset pinned to S2 gets a token through R2.
-- [ ] Merge. If the merge makes a new SHA, check that its tree equals S2 (`git diff` is empty, so the content and rule ID are the same), then the owner sets R2 to that SHA and reads it back. If the trees differ, treat the merge SHA as a new candidate and go back to the step that sets R2. That SHA is the release SHA.
+- [ ] Merge. If the merge makes a new SHA, check that its tree equals S2 (`git diff` is empty, so the content and rule ID are the same). If the trees differ, treat the merge SHA as a new candidate and go back to the step that sets R2. That SHA is the release SHA. Phase 3 learned: every squash merge makes a new SHA, so one release takes two Console edits per rule, the candidate SHA and then the merge SHA.
+- [ ] Before the second Console edit, pin the scratch test ruleset to the merge SHA and run once: the exchange must be denied, because the rule still names the candidate SHA (phase 3: run 40583290922). Then the owner sets R2 to the merge SHA and reads it back.
 - [ ] Pin the scratch test ruleset to the release SHA and do one token exchange through R2. Any new candidate, for example a fix commit after a failed validation, repeats from the step that sets R2 to the candidate SHA.
 - [ ] Owner: move the pin of every ruleset that requires the workflow (the pilot ruleset, the org ruleset, or both) to the release SHA, then read each back: `repository_id`, path, `sha`, no `ref`.
-- [ ] Start a new run on each open PR (push, or close and reopen). Do not re-run old runs: after R1 is deleted, a re-run at the old SHA fails auth.
+- [ ] Start a new run on each open PR (push, or close and reopen). Do not re-run old runs: after R1 is deleted, a re-run at the old SHA fails auth. Do this after every pin move, also a move back: the box counts only runs at the pinned SHA, so a move back revives older passes at that SHA (spec "Pinned SHA only").
 - [ ] Owner: delete R1 only when each new run has completed at the release SHA (`job.workflow_sha`); a queued run does not count (V22).