Batch 2026-07-28-gate-hygiene-t1: 5 issue(s) (Batch_2026-07-27_225225) - #183
Merged
Conversation
) (#169) * feat(review-loops): type REVIEW_SCHEMA.issues and wire quality/test loops to structured findings Type REVIEW_SCHEMA.issues.items with severity/summary/recommendation (one field looser than CHALLENGE_SCHEMA.findings: recommendation stays optional), keeping `issues` out of REVIEW_SCHEMA.required and `id` out of the schema entirely. Add normalizeFindings(raw, source) to turn a reviewer's `issues` array into engine-assigned {id, severity, summary, recommendation} findings, returning null when the reviewer omitted the key so callers fall back to today's prose path byte-for-byte. Add findingsBlock() as the single renderer feeding every fix stage: null -> today's prose, non-empty -> the id-prefixed work list plus prose kept as context, empty array -> an explicit no-findings line with the prose still rendered underneath. Wire the quality loop and test loop: a changes_requested review whose `issues` normalizes to a present, empty array is now treated as nothing-to-fix (returns approved/{ok:true}, increments the new ctx.metrics.findings_empty_exits counter, skips the fix stage) instead of degrading the loop toward a fix stage with nothing to fix. Fix prompts are now fed from findingsBlock() and ask implementers to prefix each fixes_applied entry with the id of the finding it resolves. Reviewer prompts gain wording tying the `issues` array to the verdict. Adds normalizeFindings/findingsBlock unit tests, and both a present-empty- array scenario and its mirror image (issues omitted entirely, which must NOT be treated as empty) to the quality-loop and test-loop suites, plus a regression proving the existing issues:['x'] fixtures still trigger their fix stage with the new rendered finding line. Part of #162. * refactor(review-loops): dedupe the issues-vs-comments reviewer prompt line runQualityLoop and runTestLoop repeated the same three-line "every concern goes in `issues`" instruction verbatim. Extract it to ISSUES_ASK, matching the existing HANDOFF_ASK/COMMIT_SHA_ASK shared-prompt-line pattern, so the two call sites can't drift. Synced via lint-engine.js --fix per the LOCKSTEP-EDIT rule. Part of #162. * feat(review-loops): wire the pr-review merge gate to structured findings Type the pr-review merge gate (reviewAndMerge) into the same normalizeFindings()/findingsBlock() machinery task 1 gave the internal quality/test loops (issue #162): - Add ISSUES_ASK to the spec-review and code-review prompts, tying the verdict to a non-empty `issues` array the same way quality review and test validation already do. - Normalize each reviewer's issues under a distinct source ('spec-i'+iter, 'code-i'+iter) before concatenating for recordGateOutcome, since both reviews land in one tally via parallel() and model-chosen/shared ids would collide. - Add nothingToFix(r, f): true when a reviewer approved outright or requested changes while naming zero findings. prReviewClean (both approved) stays the only path that sets approved = true. When both reviewers have nothing to fix but the pair isn't clean, break WITHOUT approving, tallied as 'carried-unresolved' (computeGateYield hardcodes exactly four disposition keys) and counted in ctx.metrics.findings_empty_exits, landing on the existing needs_human. - Add a haltReason local so fail()'s message is accurate for both the cap-reached and the early-empty-exit paths. - Feed pr-fix from findingsBlock per reviewer (with the prose comments kept below as context) and add the fixes_applied id-prefix instruction. - Rewrite the recordGateOutcome doc comment: 'accepted' no longer claims to be the same condition that ends the loop, 'carried-unresolved' now covers both the iteration-cap and the empty-exit path, and the stale "severity stays zero" NOTE is replaced since REVIEW_SCHEMA.issues is now typed and gate_findings['pr-review'].severity reports real counts. Adds 7 integration tests to tests/pr-review-gate.test.js covering the empty-exit break, the mixed empty/real-findings case, the omitted-issues fallback, both null-reviewer death paths, and non-zero severity tallying. node scripts/lint-engine.js --fix keeps .claude/workflows/ticketmill.js in lockstep. * refactor(review-loops): dedupe the fixes_applied id-prefix ask across fix stages The "prefix each fixes_applied entry with the id it resolves" instruction was copy-pasted verbatim (only the example id differed) into the quality-fix, test-quality-fix, and pr-fix prompts added by issue #162. Extract it into fixesAppliedIdAsk(example), matching the existing ISSUES_ASK/COMMIT_SHA_ASK/ HANDOFF_ASK shared-prompt-line convention. * docs(architecture): document review findings as a load-bearing artifact Add docs/architecture/gate-hygiene.md covering the typed REVIEW_SCHEMA.issues shape (one field looser than CHALLENGE_SCHEMA.findings), the engine-assigned id scheme, the null-vs-empty findings distinction, the three loop predicates and why the merge gate's differs, the empty-findings exit landing on needs_human as carried-unresolved, the findings_empty_exits counter, and the fixes_applied id echo. Documents the baseline shift in computeGateYield's escaped-defect signal now that gate_findings['pr-review'].count carries real data. Add the gate-hygiene.md row to index.md's file map (authored text, outside the provenance fixture's 6-line tracked segment). Precisely reword docs/architecture/CLAUDE.md:3 to distinguish the ten split-derived pages from pages added since, leaving the other two "ten files" references (:49, :83, both describing the provenance fixture's still-exactly-ten scope) untouched. Add a gate-hygiene.md row to the "What lives here" table and a pointer to the now-stale, hash-frozen metrics.md:81-84 passage this page supersedes. Copy CLAUDE.md over AGENTS.md per the freeze pair's parity note. docs/architecture/pipeline.md, metrics.md, and failure-semantics.md are untouched — their prose is hash-frozen by tests/architecture-provenance.test.js. Refs #162 * test(gate-findings): isolate nothingToFix's approved-branch from its empty-array branch Every existing fixture with an approved reviewer also carried issues: [], which independently satisfies nothingToFix's second branch, so no test could tell `r.result === 'approved' ||` apart from `f.length === 0`. Add direct unit tests for nothingToFix() (mirroring the normalizeFindings/findingsBlock units) plus a reviewAndMerge() integration case pairing an approval that carries a non-empty issues array with a changes_requested/issues:[] partner, asserting bothNothingToFix still halts early as carried-unresolved without running pr-fix. Verified by temporarily deleting the approved-branch clause: all three new assertions fail against the mutant and pass against HEAD. * fix(review-loops): close the empty-findings verification gap in PR review Code review on PR #169 flagged that the quality-loop and test-loop empty-findings exits (a changes_requested verdict with zero structured issues, converted to a clean pass) were silent — recorded only in ctx.metrics and a pushDecision() note, neither of which reaches the batch PR body. Push a VERIFY_SKIPS entry at both exits, the same mechanism already used for capped challenges, the skipped test loop, and the skipped browser check. The pr-review merge gate's equivalent exit was already visible via needs_human and is untouched. Also: ISSUES_ASK's doc comment now names all four REVIEW_SCHEMA prompts that consume it (was missing spec review and code review), and findingsBlock() falls back to fallbackLabel (the reviewer's summary) instead of dropping straight to "(none)" when a non-null findings array pairs with empty comments, restoring the fallback chain the pre-#162 prompt had. * docs(review-loops): fix escaped-defect premise and dedupe findings headings Code review iteration 2 (PR #169) found two minor issues: gate-hygiene.md claimed gate_findings['pr-review'].count carried no real signal before #162, but the merge gate already fed real counts into it whenever a reviewer put a concern in issues rather than comments — reword the three affected passages (gate-hygiene.md, ticketmill.js's recordGateOutcome NOTE, the CLAUDE.md/AGENTS.md freeze-pair note) to "already populated, but only incidentally — now guaranteed and schema-backed". Also demote findingsBlock()'s internal headings from ## to ### so they nest under the pr-fix prompt's own "## Spec review" / "## Code review" wrapper headings instead of colliding with them. * docs(issue-162): document the empty-findings VERIFY_SKIPS visibility fix The quality loop and test loop empty-findings exits (a changes_requested verdict with zero structured issues, treated as clean) were silent before a follow-up code review fixed it: they now push a VERIFY_SKIPS entry, the same mechanism already used for capped challenges and skipped test loops, so they surface in the batch PR's Verification Gaps section. gate-hygiene.md covered the findings_empty_exits counter but not this visibility change; add it to the same section. Refs #162
… (#172) * feat(observability): tally the quality gate and bound its cap line runQualityLoop was the only capped gate that never called recordGateOutcome or reported gate_findings, and its cap-exhaustion exit ('degraded') was silently swallowed by both callers. Add one recordGateOutcome(ctx, 'quality', ...) call per loop iteration (dismissed on simplify/review agent death, accepted on approval, carried-unresolved on the empty-findings clean exit, and carried-unresolved/re-litigated on changes_requested depending on whether the cap was hit), plus a single rolled-up VERIFY_SKIPS line per issue when the cap is exhausted without approval or agent death (runQualityLoop runs once per task plus once per PR-fix round, so the line rewrites in place instead of duplicating). Also add a computeGateYield footnote noting quality's larger denominator versus pr-review, and correct three stale doc comments: the recordGateOutcome header/dismissed/bound/carried-unresolved clauses now cover quality, and the FRICTION_WEIGHTS quality_degrades comment now says it counts agent death inside the loop, not cap exhaustion, pointing readers at gate_findings.quality['carried-unresolved'] instead. Refs #163 * test(observability): cover the quality gate's tally, cap line, and no-op guarantees Extend tests/quality-loop.test.js with one test per recordGateOutcome disposition branch from task 1's map (simplify-death and review-death -> dismissed, approved -> accepted, changes_requested-before-cap -> re-litigated), a typed mixed-severity issues array proving non-zero gate_findings.quality.severity (mirrors pr-review-gate.test.js:390), the exact invariant sum(gate_findings.quality.disposition) === ctx.metrics.quality_iters (including across a later mid-loop death), a fully capped loop asserting disposition {'re-litigated': 4, 'carried-unresolved': 1} / result 'degraded' / quality_degrades 0 / exactly one VERIFY_SKIPS entry, two capped scopes on one ctx rolling up to exactly one entry naming both, and a converging loop pushing none. Add a carried-unresolved assertion to the existing empty-findings-exit test. Extend tests/friction-fields.test.js to prove a capped quality loop leaves contrarian_capped false and unresolved_count 0 -- the quality cap writes to gate_findings, never to ctx.unresolved. Extend tests/gate-yield.test.js to prove a 'quality' key tallies into by_gate without perturbing escaped_defects (neither an early gate nor the escape gate), and that the quality-denominator footnote renders only when a quality key is present. Extend tests/gate-findings.test.js to prove a 'quality' key tallies independently of the pre-existing approach/plan buckets on the same ctx. Full profile test_command (node --check, lint-engine, bash -n, manifest JSON parse, node --test, setup-worktree.test.sh) is green: 641/641 unit tests, 32/32 worktree-script cases. Refs #163 * docs(gate-hygiene): document the quality gate's disposition tally and cap Adds a "## The quality gate" section to gate-hygiene.md covering the five-branch disposition map runQualityLoop now records through recordGateOutcome, the sum(disposition) === quality_iters invariant, why the empty-findings exit tallies carried-unresolved but returns approved (computeGateYield's accepted:dismissed ratio and the approach contrarian gate's carried-unresolved precedent), what quality_degrades does and doesn't count, and the one-line-per-issue VERIFY_SKIPS cap roll-up. Extends the page's provenance section to also name metrics.md:114 ("rolls the three gates' gate_findings tallies"), which goes stale now that a fourth gate (quality) routinely appears in the rollup, and can't be corrected in place because architecture-provenance.test.js hashes metrics.md's one tracked segment — the entire file — verbatim. Extends the "frozen passage" section in both docs/architecture/AGENTS.md and docs/architecture/CLAUDE.md with identical text (copied byte for byte per the freeze-pair convention), and updates the gate-hygiene.md file-map row in index.md, AGENTS.md, and CLAUDE.md to reflect the page's broadened scope. Issue #163 task 3. * docs(gate-hygiene): fix quality_degrades undercount and provenance overclaims Code review for PR #172 found gate-hygiene.md describing quality_degrades as counting only two agent deaths (simplify, review) when the fix agent's death also increments it; fix all three deaths in both places that made the claim. Also corrects two provenance passages (gate-hygiene.md and its AGENTS.md/CLAUDE.md freeze pair) that described metrics.md's 329-of-332-line tracked segment as "the entire file," and reworks the escaped-defects header in computeGateYield so it names the gates the check actually consults instead of claiming every earlier gate missed the finding. * docs(gate-hygiene): fix the provenance passage's own off-by-one The "329 lines starting at its first heading, running to the end of the 332-line file" wording implied a segment that literally spans lines 5-332 on disk, which is 328 lines, not 329. The tracked segment's true length includes a trailing blank line that writeLines() strips at write time and architecture-provenance.test.js pads back in for hashing -- it never exists on disk. Say so directly: 329 tracked lines, 328 of them visible on disk (5-332), plus the one stripped trailing blank. * fix(observability): correct the quality cap comment's degrade-branch count The cap-exhaustion comment claimed "both degrade branches above" but the loop has three (simplify, review, fix) — a leftover from before the quality_degrades undercount fix added the third branch.
…on (#174) * fix(contrarian): make recommendation optional on challenge findings (#164) CHALLENGE_SCHEMA.findings.items.required drops 'recommendation', keeping it as a declared string property: a required fix proposal is prompt elaboration of the finding shape, not a control-flow need, and a published measurement tied that elaboration to rejecting correct code 26.2% -> 73.2% of the time. A required proposal also makes a finding cheaper to emit than to withhold, fighting these same prompts' stated acceptance condition that zero critical/major findings is the expected, unremarkable case. Widened from the issue body's two named prompt lines to three: all three "findings ARRAY" contrarian-prompt occurrences (consolidation gate, approach gate, plan gate) feed CHALLENGE_SCHEMA, so relaxing the schema without the consolidation line would leave that gate contradicting itself. Also: rewrites the now-false "one field looser" REVIEW_SCHEMA comment (the two item schemas require the same two fields as of this change, while `issues` staying out of REVIEW_SCHEMA.required and `id` never being in either schema remain true and load-bearing); cross-references #164 from the issue #162 NOTE block whose severity/summary-required, recommendation- optional description of parity only becomes literally true now; and updates all three persona anchors in templates/agents/contrarian.md (Core Principle, the Recommendation bullet, the anti-pattern list) to drop the unconditional fix-proposal demand while keeping the substantive-reasoning and concrete- failure-mode bar. templates/agents/contrarian.md and its engine-owned twin .claude/agents/contrarian.md are byte-identical before this change and deliberately drift after it: the twin is out of scope for this issue and is left untouched, deferred as a separate note. No render-site edits: all six 'f.recommendation || ''' sites already render undefined and '' identically, which is exactly the acceptance criterion. Tests: gate-findings.test.js gains a structural assertion (via harness.readGlobal) that CHALLENGE_SCHEMA.findings.items.required is exactly [severity, summary] and recommendation stays a declared property — needed because tests/harness.js stubs agent() and never validates opts.schema, so no harness test can otherwise exercise the real validator. contrarian- cap.test.js gains one recommendation-omitted case per gate (approach, plan) asserting byte-identical rendering at both the cap-out unresolved line and the prior iteration's re-evaluate/re-plan prompt, plus a findings-free case that locks in pre-existing behavior (findings was already outside CHALLENGE_SCHEMA's top-level required) rather than proving anything new. * docs(architecture): correct gate-hygiene.md's now-false CHALLENGE_SCHEMA parity claim Issue #164 made CHALLENGE_SCHEMA.findings.items and REVIEW_SCHEMA.issues.items require the same two fields (severity, summary), with recommendation optional on both — but gate-hygiene.md still documented the old asymmetry as deliberate design ("one field looser... a finding without a recommendation is an unfinished argument"). Rewrote "The typed shape, and why it's one field looser than CHALLENGE_SCHEMA" (heading included) to state the new parity, why the contrarian gate gave up its extra required field (a mandatory fix proposal is the prompt shape a published measurement tied to inflating rejection of correct work, and it fought these same prompts' stated acceptance condition that zero critical/major findings is expected), and that the reviewer-side half of the original reasoning survives unchanged. Notes the parity is schema-level only (ISSUES_ASK's prose still asks for a recommendation), that all three CHALLENGE_SCHEMA prompt lines were reworded (not just the two the issue body named), and that the absent-vs-empty-string distinction is invisible at all five challenge render sites — distinct from findingsBlock() on the REVIEW_SCHEMA side, which also prefixes the engine-assigned id. Also touched the :17 one-line reference so it no longer reads as reviewer-only now that the split is shared. Did not touch pipeline.md/metrics.md/failure-semantics.md (hash-frozen), AGENTS.md/CLAUDE.md (byte-identical freeze pair, neither row mentions this asymmetry), or index.md (gate-hygiene.md's row already exists). Refs #164
…unt (#176) * fix(friction): pool the quality stage denominator across scopes quality_iters is a run-wide aggregate (once per task plus once per PR-fix round) but was compared against a single loop's cap (MAX_QUALITY_ITERATIONS), so a multi-task issue that cleared quality on iteration 2 of every task could saturate to 1.0 -- indistinguishable from an issue whose every quality loop actually exhausted the cap. Add a quality_scopes counter (incremented once per runQualityLoop invocation, not per iteration) and pool it into computeFriction's quality-stage cap via a new multiScopeField map, which also documents task_review_attempts and browser_iters as multi-scope aggregates not yet counted this way. A metrics blob with no quality_scopes field falls back to scopes=1, so pre-existing data scores unchanged. Corrects computeFriction's header comment, which claimed a uniform min(1, iters/cap) ratio across all seven stages and that a first-pass clean run scores 0 regardless of stage count -- both false given every stage field is written with += 1 semantics, so a first-try pass has always cost 1/cap. Refs #165 * test(friction): cover the pooled quality denominator, driver invariants, and per-call scope counter tests/friction.test.js: multi-scope all-capped saturates at 1.0, the dilution case (6 iters / 3 scopes) scores 0.4 instead of saturating, all-first-iteration (3/3) scores 0.2 with a comment recording that both sanctioned formulas agree on 0.2 (not the acceptance criterion's unsatisfiable literal "0"), single-scope behavior is unchanged, a metrics blob with no quality_scopes key scores as scopes=1, appending a first-try scope deliberately lowers the ratio (5/5 -> 6/10), a saturating fixture proves value/cap can exceed 1 while contribution clamps to it, scopes stays null on task-review/browser drivers, and both driver invariants (stage: contribution === min(1, value/cap) and cap === baseCap*(scopes ?? 1); signal: value*weight === contribution, no cap/scopes keys) hold across a mixed fixture. tests/compose-friction-churn.test.js: the :113 pass-through test gains a multi-scope quality fixture so the new driver keys survive composition unchanged. tests/quality-loop.test.js: quality_scopes assertions added to the existing two-calls, fully-capped-loop, and STOP-tripped scenarios. tests/harness.test.js: a new parity test regex-reads the metrics literal out of workflows/ticketmill.js and asserts its key set equals Object.keys(harness.freshMetrics()), giving the hand-sync contract teeth. Refs #165 * docs(gate-hygiene): document the pooled quality friction denominator Extends the quality-gate section of docs/architecture/gate-hygiene.md with quality_scopes as quality_iters' companion invocation counter, and adds "The friction denominator: pooled, not worst-scope" covering why pooled (mean) was chosen over worst-scope (max) — a max relocates this issue's opening complaint rather than fixing it — the accepted non-monotonicity cost and why dilution is safe (cap exhaustion is carried by ctx.quality_caps and gate_findings disposition, not quality_degrades), acceptance criterion 1's literal "scores 0" being unmet by both sanctioned formulas (both score 0.2), the corrected driver invariant pair, and why the other six stage drivers carry scopes: null rather than 1. docs/architecture/metrics.md is hash-frozen and stays byte-for-byte unmodified; a third provenance entry supersedes metrics.md:13-14 (now true of six stages, not seven) and notes metrics.md:15-17's "scores 0" claim was already inaccurate independent of this change. CLAUDE.md gains the matching frozen-passage paragraph, copied verbatim to AGENTS.md per the freeze-pair convention. index.md and CLAUDE.md's What-lives-here row get a refreshed gate-hygiene.md parenthetical. Issue #165. * fix(observability): correct computeFriction's multi-scope comments The module header claimed six of the seven capped stages run at most once per issue, and the multiScopeField comment called browser's scope unit "per-iteration". Both are wrong: task-review and browser are multi-scope aggregates in exactly the same sense quality is (only approach/plan/test/pr-review are genuinely single-scope), and browser's scope is the phase (implement/pre-merge), not the iteration -- iteration is the numerator. Reword both comments to match gate-hygiene.md and the code they sit beside. * docs(gate-hygiene): resync iter-counting line citations after i2 header edit commit 09c5113 added 3 net lines to computeFriction's module header comment, shifting the seven `= iter`/`++` statements gate-hygiene.md cites as evidence for "iteration counting starts at 1, not 0" by +3 each. No functional change. * fix(observability): drop the last "six single-scope stages" mislabel Code review (iteration 1) found a third surviving instance of a claim 09c5113 already corrected in two other places: task-review and browser are multi-scope aggregates without a counter yet, not single-scope stages. Reword the computeFriction comment, the matching test title, and resync harness.js's stale :5127 line citation to :5130.
…#166) (#177) * feat(gate-state): add pure gate-state substrate, schemas, and unit suite Task 1 of issue #166: the pure JS layer for durable per-issue gate state, mirroring the CONSOLIDATION_* marker subsystem end to end. Adds GATE_STATE_TITLE/GATE_STATE_SCHEMA plus buildGateStatePayload/ buildGateStateComment/parseGateStateComment (title-gated, fence-extracted JSON, canonical marker as the last line, never throws), parseGateStateProbeRow (structurally rules out a truncated read being misread as absence), selectGateState (found/absent/malformed/read-failed with explicit trust-before-last-wins and a falsifiable-absent cross-check against prior-work evidence), isTrustedGateStateAuthor (self_login primary, claim_authors fallback restricted to fresh-or-batch-matching claims), deriveRunEpoch, diffGateStateIntent (match/mismatch/superseded), and attachGateStateBlocks. Declares RUN_EPOCH beside ENGINE_OWNED and wires it into __seed. Adds GATE_STATE_PROBE_SCHEMA/GATE_STATE_VERIFY_SCHEMA and four optional gate-state fields on PREFLIGHT_SCHEMA. All above the TICKETMILL-TEST-HARNESS-SPLIT marker; no write/read call sites wired yet (later tasks). tests/gate-state.test.js: 33 new unit tests covering the build/parse round trip (incl. apostrophe/newline-bearing free text), every parse rejection path, all four select states with both falsifiable-absent branches, positional trust-before-last-wins selection, trust rules, staleness, and all three diff verdicts. node --test: 678 passed, 0 failed. Ran scripts/lint-engine.js --fix per the LOCKSTEP-EDIT rule, syncing .claude/workflows/ticketmill.js. * fix(gate-state): treat empty blocks with total>0 as read-failed selectGateState only routed to 'read-failed' on zero blocks when total was also 0 and hasPriorWork was true. A self-contradictory probe result (zero blocks but total>0) fell through to 'absent' regardless of prior-work evidence, silently presenting a truncated/corrupted read as genuine absence -- the exact failure mode this design exists to prevent. Check total>0 unconditionally, before hasPriorWork. Also fixes a backwards doc reference (CONSOLIDATION_MEMBER_TITLE/ CONSOLIDATION_GROUP_TITLE are defined below GATE_STATE_TITLE, not above) and adds a covering unit test. * feat(gate-state): add postGateState and its four write boundaries Adds the non-fatal postGateState(ctx, boundary) helper (issue #166 task 2), modeled on the cap-note-plan/cap-note-approach stages: stageOpts('probe'), NOTE_SCHEMA, exactly one try, log-only on a dead agent or posted!==true. Posts the durable "## Gate State" comment via `gh issue comment <n> --repo <r> --body-file -` fed by a QUOTED heredoc, deliberately breaking from postConsolidationMarkers' `--body "..."` idiom so free text pulled from ctx.settled (which may carry apostrophes/backticks/$) reaches gh as literal bytes instead of being handed to the shell for interpolation. Wires it at four boundaries in implementIssue()/reviewAndMerge(): - 'approach', once after the approach-gate loop closes (covers all four of its break exits, before the plan stage's own fail() returns). - 'plan', once after the plan-gate loop closes (same four-break coverage, before IMPLEMENT). - 'pr-review-iN', once per pr-review iteration, right after recordGateOutcome(ctx, 'pr-review', ...) — kept inside the loop since reviewAndMerge returns from inside it at the nothing-to-fix and cap-reached breaks. - 'pr-review-iN-aborted', immediately above the reviewer-death `return fail(ctx, 'needs_human', ...)` — the only boundary a process_pr resume can reach, since that path calls reviewAndMerge directly and never runs the approach/plan loops. ctx.gate_state_intent is set only when posted===true; every other outcome sets ctx.gate_state_post_failed = boundary and pushes a ctx.deferred note, so a routine non-fatal post failure can never look like corruption to Task 4's future verify sweep. Threaded through fail(), the success return, and the resume_point==='skip' return (always null there — shape totality, no boundary can fire on that path). Updates tests/pr-review-gate.test.js's scripted responders/key-order assertions for the new in-loop and aborted gate-state calls, and adds tests/gate-state-post.test.js covering non-fatal failure in both directions, the pinned posting idiom, one-post-per-iteration, approach-then-plan ordering, a dead post changing no loop outcome, the process_pr-resume aborted-boundary scenario, a STOP trip posting nothing new for the iteration that never runs, and skip-path shape totality. node --test tests/*.test.js: 690/690 green. Lockstep copy (.claude/workflows/ticketmill.js) kept in sync; scripts/lint-engine.js clean. Ref #166 * refactor(gate-state-post-test): dedupe the gate-state-key filter into one helper Five tests repeated the same keys.filter(function (k) { return k.indexOf('gate-state-') === 0 }) inline; extract it to a gateStateKeys() helper next to the file's existing stageKeyOf() helper. No behavior change — 11/11 in this file, 690/690 overall. Ref #166 * feat(gate-state): add fetchGateStateBlocks probe, Select-time join, and RUN_EPOCH Wires the durable per-issue gate-state READ path (issue #166 task 3): - fetchGateStateBlocks(issueNumbers, priorWorkByIssue): a READ-ONLY, DRY_RUN-safe probe mirroring fetchConsolidationMarkers' shape but pinning the claim probe's deterministic jq idiom (one gh command per issue, jq computes the exact {total, blocks} return shape) instead of a bare `gh issue view --json comments`, so a truncated read is structurally a parse failure, never a fake absence. Chunked at MAX_GATE_STATE_PROBE_CHUNK (5) issues per agent call; a dead chunk marks only its own issues read-failed via synthesized stub rows, never silently dropping them. self_login is reduced across chunks (first non-empty wins). Logs one line per issue naming found/absent/malformed/read-failed, plus a distinct greppable line for the falsifiable-absent case (zero blocks with prior-work evidence). - attachGateStateBlocks extended to a real join: it now takes the probe's raw rowsByIssue + reduced self_login and always computes the four PREFLIGHT_SCHEMA gate-state fields fresh, never trusting whatever the preflight object already carried for them (clobbers a hallucinated agent-supplied value the same way attachEngineOwnedIntentional never trusts an agent-asserted regime). - Wired at Select immediately after attachEngineOwnedIntentional, threading each preflight's own pr_number/worktree_exists/resume_point through as priorWork so the falsifiable-absent rule is evidence-driven. - RUN_EPOCH assigned right after outcomeGradeR/revisitRiskR are awaited, via the existing pure deriveRunEpoch over whichever probe's `now` is available; logs loudly when both are unavailable. Tests: tests/gate-state-read.test.js covers the chunking/dead-chunk/ self_login-reduction/truncated-stdout/partial-coverage behavior; the pre-existing single-arg attachGateStateBlocks test in tests/gate-state.test.js is updated to the new always-clobber contract. Full suite 704/704 green, lockstep in sync. Refs #166 * refactor(gate-state): dedupe hasPriorWork into hasGateStatePriorWork Task 3's fetchGateStateBlocks diagnostic log recomputed the exact three-condition prior-work check selectGateState already has inline. Extract it into one shared pure helper, hasGateStatePriorWork, so the falsifiable-absent logic exists in one place. Refs #166 * fix(gate-state): don't mislabel a total>0 corrupted read as unexpected-absent fetchGateStateBlocks' diagnostic-log branch fired whenever hasGateStatePriorWork(pw) was true, without checking parsed.total === 0 -- so a genuinely corrupted/truncated read (total>0, zero matching blocks, which selectGateState always treats as read-failed) that happened to coincide with prior-work evidence got logged as "absent (unexpected: ...)" instead of a plain read-failed. Require parsed.total === 0 in the branch condition so only the true falsifiable-absent case gets the "unexpected absent" wording. Updates the gate-state-read.test.js case that pinned the buggy behavior with jqRow(3, []) to use jqRow(0, []) (the genuine falsifiable-absent case) and adds a new case asserting the total>0 corrupted-read path logs plain read-failed, never the unexpected-absent wording. * feat(gate-state): add Report-phase verifyGateState self-validation sweep Task 4 of issue #166: a single Report-phase stage, chunked at MAX_GATE_STATE_PROBE_CHUNK like fetchGateStateBlocks, that proves post -> GitHub -> read -> parse for the durable gate-state comment without ever showing the verifying agent the payload it's checked against (the prompt carries only issue numbers plus the same jq-pinned per-issue read Task 3 uses). JS runs parseGateStateProbeRow -> parseGateStateComment -> diffGateStateIntent against each result's gate_state_intent and logs one of six outcomes: match, mismatch, superseded, read-failed, post-failed, no-intent. Non-fatal end to end (advisory logging only, never mutates a result's status); wrapped in try/catch at the call site, placed before the token/friction rollups. Also updates GATE_STATE_VERIFY_SCHEMA (declared but unused by Task 1) from its original per-issue shape to the chunked rows[] shape this sweep actually needs, mirroring GATE_STATE_PROBE_SCHEMA. tests/gate-state-verify.test.js covers all six outcomes, a fully dead verify stage, one dead chunk of two leaving the other intact, and that the prompt never embeds the intent payload. * refactor(gate-state): dedupe verifyGateState/fetchGateStateBlocks probe plumbing fetchGateStateBlocks and the new Report-phase verifyGateState sweep independently built the same chunk list, pinned the same jq idiom verbatim, generated the same dead-chunk stub rows, and normalized rows the same way. Extract chunkGateStateIssues, gateStateProbeCommandLine, deadGateStateChunkRows, and normalizeGateStateRow as shared helpers so the read-side probe and its self-validation sweep can't drift apart. No behavior change; 718/718 tests pass, lint-engine confirms lockstep. * docs(gate-state): document the durable per-issue gate-state substrate Append a section to docs/architecture/gate-hygiene.md covering issue #166's "## Gate State" comment: why the payload is fenced JSON rather than consolidation's flat key:value lines, the four write boundaries (and the recorded decision to leave the STOP.tripped exit without one), append-only positional last-wins and the idempotence it buys for free, the four-state read contract and why `absent` must be falsifiable, the jq-pinned read idiom versus fetchConsolidationMarkers's bare read, trust-before-last-wins selection, the intent-only-on-success rule and the post-failed sweep outcome, the self_login/claim_authors trust model, RUN_EPOCH's derivation from existing wall-clock reads, seeded_from as a consumer-less discriminator, the group-identity gap that leaves group_id/members riding in every payload, and the deviation from the issue body's preflight-step wording toward a separate chunked probe. Refreshes the gate-hygiene.md summary row in index.md and the byte-identical AGENTS.md/CLAUDE.md pair. pipeline.md, metrics.md, and failure-semantics.md are untouched (hash-frozen by tests/architecture-provenance.test.js). node --test tests/*.test.js: 718/718 green; node scripts/lint-engine.js clean. * fix(gate-state): fix jq total, order supersession on write_seq, backfill missing probe rows Code review on PR #177 found four state-machine defects in the gate-state substrate: - gateStateProbeCommandLine's jq counted ALL comments into `total`, so any issue with one ordinary comment and zero gate-state blocks reported read-failed instead of absent -- the common case, not an edge case. `total` now runs through the same title-gated filter `blocks` uses. - diffGateStateIntent ordered same-run supersession on `epoch`, but RUN_EPOCH is assigned once per run and is identical on every boundary a run posts, so 'superseded' was unreachable in production. Added GATE_STATE_WRITE_SEQ, a monotonic per-run write counter embedded on the payload as `write_seq`, and moved ordering onto it. - fetchGateStateBlocks' diagnostic log misread a live chunk's response that simply omitted a queried issue as absent rather than read-failed. Missing issues are now backfilled with the same read-failed stub a dead chunk's issues get. - verifyGateState's mismatch/read-failed outcomes only logged; they now also push a VERIFY_SKIPS entry so a failed self-validation reaches the batch PR's Verification Gaps section. Also fixes a stale doc comment (verifyGateState claimed a concurrent run's write could report 'superseded'; diffGateStateIntent's own run check forbids that) and reconciles gate-hygiene.md's `total` wording. Validation: node --check, lint-engine (clean, 2 lockstep pairs in sync), node --test (723/723 pass, 5 new). * fix(gate-state): gate the suspicious-absent log on an actually successful read Every hard read failure (dead chunk, non-zero gh exit, truncated stdout) also produced blocks.length===0 and total===0, so it printed the same "absent (unexpected: ...)" line as a genuine falsifiable-absent read. On a resume, where hasGateStatePriorWork is true for exactly the issues this substrate serves, that made read failures indistinguishable from suspicious absences. Add the readOk = row.exit_ok === true && parsed.ok === true guard, mirroring attachGateStateBlocks' own definition, so only a genuine successful-but-empty read prints the suspicious line and every read failure falls through to a plain "read-failed". Add prior-work-bearing test variants for the dead-chunk, omitted-row, non-zero-exit, and truncated-stdout cases. Also fix the stale "advisory only (log lines, never a result mutation)" call-site comment on the verify sweep (it now also pushes to VERIFY_SKIPS) and rewrap one over-length prose line in gate-hygiene.md. Addresses PR #177 code review iteration 2 (issue #166). * test(gate-state): assert GATE_STATE_WRITE_SEQ actually advances across two real posts The "approach then plan, in order" test only checked stage-call ordering, never the posted payloads' write_seq values -- a regression reverting the GATE_STATE_WRITE_SEQ ++ (added in an earlier PR-review round to fix same-run write ordering, since RUN_EPOCH is identical across all boundaries in a run) to a static value would have passed every test in the suite. Extract the real heredoc body postGateState sends to gh from the captured agent prompts and parse it with the real parseGateStateComment, then assert the plan boundary's write_seq is exactly one more than the approach boundary's -- proving the module-level counter, not a fixture, drove both values. Confirmed this fails (1/11) against a version of the increment reverted to a static write_seq: 1.
This was referenced Jul 28, 2026
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
1 participant
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.
Closes #16
Closes #21
Closes #22
Closes #24
Closes #25
Closes #28
Closes #29
Closes #30
Results
(no consolidation groups completed this run — every issue merged as its own unit)
(all verification gates ran; no gaps section needed)
Per-issue PRs were squash-merged into Batch_2026-07-19_200908 with full review trails on each issue.
Merge Auto-Resolution
No CONFLICTING PRs this run — nothing to auto-resolve.
Token Usage
Run total (output tokens, via budget.spent()): 950592
approximate - overlapping concurrent stages over-count and do NOT reconcile to the run total.
(A single shared monotonic counter cannot be split per concurrent call — agent() returns schema content only, no per-call usage — so simultaneous issues each see the same counter movement.)