Skip to content

Durable per-issue gate state on the issue (substrate) #166

Closed

Description

@aaddrick

Problem

Nothing about an issue's gate state survives a run boundary. processIssue builds a fresh ctx every time:

  • settled: [] at :4821
  • metrics: { ..., pr_review_iters: 0, ... } at :4846

On the process_pr resume path at :4871, the run does worktree setup, pushes one "Resumed Run" decision, and calls reviewAndMerge(ctx), which starts its loop at iter = 1 against MAX_PR_REVIEW_ITERATIONS. Nothing reads a prior count, and settledBlock(ctx) renders empty into both review prompts at :4619 and :4639.

So a resumed run judges the PR with no record of what the contrarian gates adjudicated, and draws a fresh review budget. The reviewers are told to read issue and PR comments, which is a prose narrative, not a record.

runs.jsonl cannot fix this. buildLedgerLine at :5613 carries sixteen fields and gate_findings is not among them; the full per-issue detail lives in logs/<logs_dir>/runs/<run_tag>.json, which the engine never reads back. More decisively, logs/ is gitignored and the mill skill documents runs.jsonl as a per-host local artifact, so it evaporates in the cross-host resume case this exists to serve.

Change

This issue ships the substrate only. No consumer.

  1. Write. Post a structured block, behind the existing marker convention <!-- ticketmill <repo>#<issue> -->, carrying { schema, issue, gate_budgets, settled }. Two boundaries are enough: once when the plan gate resolves, and once per pr-review iteration. Append-only, read last-wins, mirroring the outcomes.jsonl contract that diffOutcomeGrades already implements.
  2. Read. Add one step to the preflight probe (:6905-6949) returning the raw blocks as strings, and a field on PREFLIGHT_SCHEMA (:478) to carry them.
  3. Parse. All parsing is pure JS above the TICKETMILL-TEST-HARNESS-SPLIT marker at :6522. The agent returns raw text and never decides, matching the OUTCOMES_SCHEMA.prior_ledger_lines contract and the PIN documented in docs/architecture/metrics.md under "A quality-fix iteration closed a PIN violation".

Two constraints

The fail-open must be visible. The preflight probe runs at M.probe (haiku, low effort, :352), already carries six numbered steps, and on death returns a hardcoded default at :6947. Three states must be distinguishable downstream: block found and parsed, block genuinely absent (a first run), and read failed. A read failure that silently presents as "absent" would hand the eventual consumer a fresh budget, which is the exact silent cap exhaustion this substrate exists to close. Log the failure case.

Self-validate the write. Because the consumer lands a tier later, this issue would otherwise ship an unexercised write path into GitHub comments. Include a same-run read-back that logs a mismatch, so the substrate is proven before anything depends on it.

If a new agent stage key is added, add it to the M map at :351. stageOpts at :412 defaults unknown keys to { model: 'sonnet' }, so an unregistered key silently runs at sonnet.

Acceptance criteria

  • The block is written at both boundaries and re-read across a simulated resume.
  • The parser is unit-tested above the split marker with no gh or git in the loop.
  • Found, absent, and read-failed are distinguishable, and read-failed logs.
  • Malformed JSON, a truncated block, and a block carrying a different issue number all fail open to today's behaviour rather than throwing.
  • A run with no prior block produces byte-identical behaviour to today.
  • The write is idempotent enough that a re-run does not corrupt last-wins reads.
  • node --test stays green. Add a dedicated test file; extend tests/decision-records.test.js if the settled ledger shape changes.

Out of scope

Nothing consumes the parsed state in this issue. Seeding ctx.metrics and ctx.settled, and enforcing an exhausted budget, is the follow-up that depends on this.

Documentation target

Document in CHANGELOG.md and in docs/architecture/gate-hygiene.md (create it if absent), adding its row to docs/architecture/index.md's file map.

Do NOT edit docs/architecture/pipeline.md, metrics.md, or failure-semantics.md. Their moved prose is hash-frozen by tests/architecture-provenance.test.js, and since the tech-docs stage runs at :4707, after the last test-loop execution, a frozen-prose edit surfaces only as a red CI check on the batch PR. New pages are authored text and are explicitly allowed; see docs/architecture/CLAUDE.md.

Activity

  1. added
    gate-hygieneGate hygiene: findings, caps, and per-problem budgets (sequenced)
    on Jul 28, 2026
  2. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Ticketmill Claimed

    batch: Batch_2026-07-27_225225
    run: 2026-07-27-gate-hygiene-t1
    host: tanngrisnir
    started: 1785207412
    Queued in an active ticketmill run — will be processed even if work has not visibly started (concurrency queue). Claims older than 12h are stale.

  3. self-assigned this
    on Jul 28, 2026
  4. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Starting Automated Processing

    Batch branch: Batch_2026-07-27_225225 (integration target; final human-reviewed PR goes Batch_2026-07-27_225225 -> main)

    Pipeline:

    • Research
    • Evaluate + Contrarian
    • Plan + Contrarian
    • Tasks with quality loops
    • Tests
    • Docs
    • PR
    • Review
    • Merge
  5. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Evaluation: Best Path

    Complexity: complex. Additive and consumer-free, but it ships durable cross-run state a later tier will trust to gate merges, and its worst failure mode is silent: a read failure presenting as "no prior block" hands the consumer a fresh budget, which is the exact hole this exists to close.

    Recommended approach

    Model the whole thing on the consolidation-marker machinery already in the engine, rather than inventing a new mechanism. That code (CONSOLIDATION_GROUP_TITLE, buildConsolidationGroupComment, parseConsolidationGroupComment, CONSOLIDATION_MARKER_PROBE_SCHEMA, postConsolidationMarkers) is the same problem already solved once: write a machine-readable block into an issue comment behind the canonical scope-guard marker, probe it back at Select, parse it in pure JS above the split marker. Follow it and this issue adds no new architectural idea.

    1. Shape. A title-gated comment: first line exactly ## Gate State (a new GATE_STATE_TITLE constant, same gating role as the two consolidation titles), a one-line human summary, then the payload in a fenced JSON block inside a <details> wrapper, then the canonical <!-- ticketmill <repo>#<issue> --> line last. JSON rather than consolidation's flat key: value lines because settled[] is an array of five-field objects with free text; flattening it would mean inventing an encoding. <details> because up to four of these land per issue per run and the trail is read by humans and by contrarian gates.

    Payload: { schema: 1, issue, run, gate_budgets, settled }. schema so a future shape change fails closed instead of half-parsing. run (RUN_TAG) is human/debug only and never the ordering key: last-wins is positional over comment order, so no wall-clock is needed anywhere (the sandbox forbids Date.now() and scripts/lint-engine.js enforces that).

    2. Write, at two boundaries. After the plan gate resolves, on all three exits (accepted, cap-reached, contrarian-died), and once per pr-review iteration right after recordGateOutcome(ctx, 'pr-review', ...). Append-only; a re-run appends a newer block that supersedes, so idempotence falls out of last-wins rather than needing an exists-check. Posting is non-fatal on every path, mirroring the cap-note-plan stage: if (!r || !r.posted) log(...) and continue.

    No new M key. Call stage(ctx, 'gate-state-plan', ..., stageOpts('probe'), ...) — a distinct stage() key gives per-stage token attribution while stageOpts('probe') gives haiku/low, which sidesteps the unregistered-key-runs-at-sonnet hazard entirely. (Checked: gate-state-* matches no REWORK_STAGE_PREFIXES entry, so computeReworkTax classifies it as first-pass, correctly.)

    3. Self-validating write. Fold the read-back into the same agent call: post the body verbatim, then re-read the issue's comments and return the raw body of the last comment whose first line is the title. Schema { posted, readback }. JS parses readback and compares it to the object it intended to write, logging a mismatch. One round trip, and it proves the full post -> GitHub -> read -> parse loop in the same run, which is what makes shipping this without a consumer safe.

    4. Read. One step appended to the preflight probe, kept last and short so it does not crowd the six numbered steps already in that haiku prompt. It returns two judgment-free facts and no decision: gate_state_blocks (raw comment bodies, verbatim, most recent last) and gate_state_read_ok (literally: did the gh call exit 0). Both are added to PREFLIGHT_SCHEMA. This keeps the PIN intact — the agent observes, JS decides — the same split OUTCOMES_SCHEMA.prior_ledger_lines plus now already uses.

    5. Parse, pure JS above TICKETMILL-TEST-HARNESS-SPLIT. Three functions: buildGateStateComment(repo, issue, payload), parseGateStateComment(body) (title-gated, fence-extracted, JSON.parse in try/catch, cross-checks the embedded issue number and schema, returns null on any mismatch), and readGateState(blocks, readOk, issue), the reducer that folds them into one state.

    The fail-open, made visible

    readGateState returns four distinguishable states, not three:

    condition state logs
    readOk, blocks present, parse clean found no
    readOk, zero blocks absent no
    !readOk read_failed yes
    readOk, blocks present, every parse fails malformed yes

    Everything except found fails open to today's behaviour; everything except absent logs. The fourth state costs one branch and keeps "we could not read it" apart from "we could not understand it," which are different bugs.

    Two lines carry almost all the risk here, and both must positively assert failure rather than infer absence from silence:

    • The hardcoded default returned when the preflight probe dies must set gate_state_read_ok: false. A dead probe is read_failed, never absent.
    • The post-probe normalization (beside the existing r.predicted_files = Array.isArray(...) ? ... : []) must default a missing gate_state_read_ok to false, not true.

    Why this and not the alternatives

    runs.jsonl is ruled out by the issue itself and the ruling is right: gitignored, host-local, and buildLedgerLine's sixteen fields do not include gate state. A git-tracked state file on the branch dies with the worktree and pollutes the PR diff. Labels cannot carry structure, and editing the issue body mutates human-authored text and destroys append-only. The issue comment is the only substrate that is durable, cross-host, append-only, and already load-bearing for this exact purpose.

    Scope discipline

    Nothing reads the parsed state. ctx.metrics and ctx.settled stay untouched, so a run with no prior block takes byte-identical decisions to today; the only deltas are two extra posts and one extra probe step. Seeding and enforcement are #168's.

    Docs go to docs/architecture/gate-hygiene.md, which #162's tier already created, plus CHANGELOG.md; index.md's row gets its description widened. pipeline.md, metrics.md, and failure-semantics.md stay untouched (hash-frozen). Both engine copies ship together via node scripts/lint-engine.js --fix. New tests land in tests/gate-state.test.js, driving all three pure functions through tests/harness.js with no gh or git in the loop.

    One correction to the issue text

    The line numbers in the body are stale. #162, #163, and #164 have already landed on this batch branch and the engine has grown from 7761 to 8027 lines, so :4821, :4846, :6905-6949, :478, and :6522 all point at the wrong code now. Anchor every edit on a symbol, never a line number.

  6. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Contrarian: Approach Challenge (Iteration 1)

    Verdict: needs_rework — the architecture is right, the rework is bounded to four specific corrections. Mirroring the consolidation-marker subsystem is the correct call and I am not challenging it.

    Strengths (steel-man)

    Reusing CONSOLIDATION_* end to end is the strongest part of this. Verified in the tree: fetchConsolidationMarkers -> parseConsolidationGroupComment -> healGroups is exactly "durable machine-readable state on an issue comment, parsed in pure JS above the split marker," and it is already reviewed, tested (tests/consolidation.test.js), and idiomatic here. JSON over consolidation's flat key: value lines is right for a five-field array with free text. The no-wall-clock/positional-last-wins choice is forced and correct (scripts/lint-engine.js:68 really does reject Date.now(). The stage(ctx, 'gate-state-plan', ..., stageOpts('probe'), ...) trick is sound: stage() at :1787 takes opts explicitly and never re-looks-up M[key], so no unregistered-key-at-sonnet hazard exists, and REWORK_STAGE_PREFIXES (:5425) has no gate-state prefix, so rework-tax classification stays correct. Four states instead of three is a genuine improvement over the issue text. And pre really does reach processIssue(pre) (:5074) intact, so the follow-up tier can read the new fields without further plumbing.

    Findings

    [Major] The self-validating write is unfalsifiable as designed.
    Folding post + read-back into one agent call means the agent that reports readback already holds the intended body verbatim in its own prompt. Failure scenario: gh issue comment fails, or GitHub mangles the fenced block, and the agent returns the body it was handed. JS compares intended vs readback, they match, no mismatch is ever logged, and the constraint that exists specifically to prove an unexercised write path certifies a broken one. This is not hypothetical laziness — echoing available context is the cheap path for a haiku stage. Recommendation: split it, exactly as the precedent does (postConsolidationMarkers writes; fetchConsolidationMarkers is a separate read probe). Second call, prompt containing only the issue number and the title to match, never the payload; return the comment id plus raw body. Then a match actually proves the round trip.

    [Major] "Absent" is not falsifiable, and the obvious fetch silently truncates.
    Checked on this repo just now: gh api repos/aaddrick/ticketmill/issues/162/comments --jq length returns 30, while gh issue view 162 --json comments returns 31. The REST default is one page of 30 ascending, so on any issue past 30 comments a bare gh api .../comments returns the oldest 30, exits 0, and finds no gate-state block. That is gate_state_read_ok: true with zero blocks — reported as absent, which is the precise silent fail-open the issue calls a hard constraint. Ticketmill issues cross 30 comments routinely (#162 already has). Recommendation: (a) pin the exact command in the probe prompt — gh issue view <n> --repo <r> --json comments, the same call fetchConsolidationMarkers uses — never a bare gh api; (b) have the probe also return the total comment count, so JS can catch the impossible case (a ticketmill-claimed issue always carries at least two trail comments, so blocks.length === 0 && total === 0 means the read never happened) and classify it read_failed rather than trusting a model-attested boolean alone.

    [Major] aaddrick/ticketmill is public, so the block is attacker-writable and nothing checks the author.
    Confirmed "visibility":"PUBLIC". Any GitHub user can post a comment whose first line is ## Gate State with a well-formed payload naming the right issue and schema: 1. Positional last-wins means the newest such comment wins outright — no adjudication stands between it and the consumer. Once #168 seeds ctx.settled and ctx.metrics from it, forged free text lands verbatim in settledBlock() (:1874), which tells every downstream gate that re-litigating without new evidence "is itself a process failure," and forged gate_budgets can present a fresh or an exhausted PR-review budget. The consolidation precedent has no author filter either, but its markers only seed a proposal an opus gate plus a contrarian re-adjudicate; this substrate is designed to be trusted directly by JS. The trust delta is the whole point. Recommendation: carry author_login and author_association per block on the probe schema (gh exposes both) and filter in JS to the repo owner before parsing; drop and log anything else as malformed. Also re-apply settleDecision's own slice() caps (400/300/4x160) at parse time so a hostile or runaway payload cannot balloon a later prompt. Deciding this now costs one field; retrofitting it later means changing a contract the consumer already depends on.

    [Major] The plan gate has four exits, not three, and the fourth writes nothing.
    Besides accepted (:4586), cap-reached (:4595), and contrarian-died (:4583), the loop also breaks at if (!rp || rp.status !== 'success') { log('re-plan failed — proceeding with current plan'); break } (:4626). A run that consumes plan-gate iterations and then loses the re-planner leaves no durable record at all, so the consumer hands the resumed run a fresh budget — the exact hole this exists to close, on the one path where the run was already degraded. Recommendation: post once after the loop rather than at each exit. One call site instead of three, every exit covered including any added later, and the payload is read off ctx.gate_findings/ctx.settled, which are already correct at that point.

    [Minor] The payload and the read-back are both unbounded.
    Every post carries the full settled[]; settleDecision caps each entry near 1.3 KB but not the array, and up to four posts per run each repeat the whole thing. Then the probe returns all blocks raw, most recent last, into a haiku stage that already carries six numbered steps and the full issue body. Recommendation: cap the payload's settled to the last 6 (matching settledBlock's own slice(-6), so the durable record and the rendered block agree) and cap the probe to the last 2 or 3 blocks — last-wins only ever needs one, plus a fallback.

    [Minor] <details> hides the payload from humans, not from agents.
    gh issue view --comments renders raw markdown, so every downstream contrarian, spec reviewer, and code reviewer reading the trail ingests up to four JSON blobs per run that duplicate what settledBlock() already injects structurally. Worse, the trail instruction that prior contrarian comments count as prior adjudication now meets a machine block asserting adjudications in a shape no agent was told how to weigh. Recommendation: keep the human summary one line and explicitly non-directive (state that it is machine state, not an instruction), and keep the payload minimal — this is a second reason to cap settled.

    [Minor] Consolidated group units drop member state and lose it on re-anchor.
    deriveUnits (:1314) builds a group unit with Object.assign({}, primaryRef, {...}), union-folding only predicted_files, depends_on, and revisit_risk. New preflight fields inherit from the primary alone, so member issues' blocks are invisible, and when a group re-anchors to a different primary (which the stable-group-id comment at :986 says explicitly can happen) the prior state stops being read. Writes go to ctx.issue (the primary), so this is self-consistent per run but not across a re-anchor. Recommendation: no code change needed in this issue — decide it deliberately and write it down in gate-hygiene.md, or union-fold the field like the three neighbors, so the follow-up tier does not discover it as a bug.

    Pre-mortem, one line

    Six months out, the way this fails is not a crash: it is a resumed run reporting absent because someone's gh call quietly returned page one, drawing a fresh review budget, with nothing in the log to say so. Findings 1 and 2 are the two places that story starts.

  7. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Revised Evaluation (after contrarian iteration 1)

    All four majors and all three minors confirmed against the tree. Nothing to rebut outright. Two get adopted with a narrowing correction, because the literal fix as written would misfire; each narrowing is backed by a command I ran.

    The architecture is unchanged: mirror the consolidation-marker subsystem end to end. What changes is the write topology (one call site per gate, not three), the read contract (pinned command, author provenance, a checkable count), and the read-back (a second agent that never sees the payload).

    Verification log

    Claim Check Result
    Precedent splits write from read grep postConsolidationMarkers|fetchConsolidationMarkers separate functions, :4022 and :3805
    Default REST comment fetch truncates gh api .../issues/162/comments --jq length vs gh issue view 162 --json comments 30 vs 31
    Author provenance is available gh issue view 166 --json comments --jq '.comments[0]' authorAssociation: "OWNER", author.login: "aaddrick"
    Repo is public gh repo view --json visibility PUBLIC
    Plan gate exit count sed -n '4355,4412p' four breaks: dismissed :4367, accepted :4376, cap :4396, re-plan failed :4408
    settledBlock renders 6 :1856 ctx.settled.slice(-6)
    settleDecision caps :1847-1849 400 / 300 / 4 x 160
    deriveUnits union-folds three fields :1289-1292 predicted_files, depends_on, revisit_risk; everything else is Object.assign({}, primaryRef, ...)

    1. Split the read-back (major, adopted as written)

    The self-validating write was unfalsifiable: one agent holding the payload in its own prompt can echo it without a round trip. Adopt the precedent's shape exactly.

    • Write call posts the body verbatim under stageOpts('probe'), returns posted.
    • Verify call is a separate stage() whose prompt carries only the issue number, the literal title ## Gate State, and the pinned fetch command. It never sees the payload. It returns comment_id plus the LAST such comment's raw body.
    • JS compares parseGateStateComment(body) against the object it intended to write, canonicalized through JSON.stringify, and requires a non-null comment_id. A mismatch logs. Nothing throws.

    Run the verify once per issue per run, guarded by a ctx.gate_state_verified flag, on the first successful post at whichever boundary comes first. That covers the process_pr resume path at :4871, where implementIssue never runs and the plan-gate write therefore never happens. Cost is one extra haiku probe per issue.

    2. Falsifiable absence (major, adopted with a narrowing)

    The pinned command goes into the probe prompt verbatim, exactly as fetchConsolidationMarkers:3809 pins it: gh issue view <n> --repo <repo> --json comments. Never a bare gh api. The 30-vs-31 measurement above is the entire reason.

    The probe also returns gate_state_total_comments, so the boolean stops being the only evidence.

    Narrowing. The rule "zero blocks with zero total means the read never ran" is a false positive on a genuinely fresh issue. Preflight runs at :6905; toClaim posts the claim comment at :7240, after. A first-run issue with no comments legitimately reports zero. So the impossible-case test is conditioned on evidence of prior processing, which the probe already returns in the same object:

    read_ok === false || total_comments < 0                -> read_failed (log)
    blocks empty && total_comments === 0 && priorWork(pre) -> read_failed (log)
       priorWork = pr_number != null || worktree_exists || branch || resume_point !== 'implement'
    

    A previously processed issue always carries trail comments (claim, "Starting Automated Processing", gate comments), so zero total against prior-work evidence is arithmetically impossible and means the read never ran. On a fresh issue it stays absent, silently, which is correct. This is a cross-field consistency check inside one structured output rather than a bare model attestation: strictly better than the boolean, with no false-positive log noise.

    3. Author provenance (major, adopted with a portability correction)

    The repo is PUBLIC, the block is last-wins, and the design had no author check. Any user could open a comment titled ## Gate State carrying an exhausted budget and a later tier would trust it. Fixed at the schema, not the parser.

    Each block carries { body, author_login, author_association }. JS filters before parsing; a rejected block is counted and logged as malformed with its login, never parsed.

    Correction to the proposed rule. "Filter to the repo owner" is right for this repo and wrong for the engine. Ticketmill is repo-agnostic by design and profile-driven, so a deployment running the mill under a bot or a write-access collaborator would see every block rejected and degrade to permanent absent: a real regression dressed as a security fix. The correct boundary is write access, which is exactly what authorAssociation encodes.

    accept: OWNER | MEMBER | COLLABORATOR
    reject: CONTRIBUTOR | FIRST_TIME_CONTRIBUTOR | FIRST_TIMER | MANNEQUIN | NONE
    

    A forger without push rights lands in the reject set. author_login rides along anyway, so tightening to owner-only later is a one-line change with no contract churn, which is the challenge's own stated reason for adding the field now.

    Rejected: viewerDidAuthor, also on the payload and tempting. It is relative to whichever token runs gh, so it reads false on the second host, and cross-host resume is the entire point of this issue.

    parseGateStateComment re-applies settleDecision's caps at parse time (400 / 300 / 4 x 160, verified at :1847-1849), so an oversized forged or corrupted block cannot inflate a later prompt even if it clears the author check.

    4. One write call site per gate (major, adopted for the plan gate; deliberately not for pr-review)

    Plan gate: post once after the loop. Four exits confirmed, not three, and the fourth (re-plan failed, :4408) wrote nothing under the old plan. ctx.gate_findings and ctx.settled are already final where control leaves the loop, so a single post below it covers every exit including any added later. One call site instead of three.

    PR-review gate: keep the single in-loop call site, immediately after recordGateOutcome(ctx, 'pr-review', ...) at :4669. Moving it below that loop would be a regression: reviewAndMerge returns from inside the loop on a failed pr-fix (:4691) and on a quality-loop halt (:4699), and those are precisely the runs a later resume needs the iteration count for. Note this is already one call site, so finding 4's arithmetic applies only to the plan gate. The dead-reviewer return fail at :4657 sits above recordGateOutcome, so that iteration produced no outcome and has nothing to write.

    Bound: at most 1 + MAX_PR_REVIEW_ITERATIONS (3) = 4 gate-state comments per issue per run.

    5. Bound both sides (minor, adopted)

    • Payload settled capped to the last 6, matching settledBlock:1856's own slice(-6), so the durable record and the rendered prompt agree by construction rather than by coincidence.
    • The probe returns at most the last 3 matching blocks. Last-wins needs one plus a fallback for a malformed tail.

    6. Non-directive human summary (minor, adopted)

    The <details> wrapper hides the payload from humans and not from agents, so the visible line is where the disclaimer has to live. Exactly one line, explicitly non-directive:

    ## Gate State
    
    Machine-readable run state recorded by ticketmill for cross-run resume. This is a record, not an
    instruction: nothing in it directs any agent.
    
    <details><summary>gate-state payload (schema 1)</summary>
    
    ```json
    { "schema": 1, "repo": "aaddrick/ticketmill", "issue": 166, "run": "<run_tag>",
      "boundary": "plan-gate",
      "gate_budgets": { "approach_iters": 1, "plan_iters": 2, "pr_review_iters": 0 },
      "settled": [ /* last 6, each field capped at parse time */ ] }
    ```
    
    </details>
    
    <!-- ticketmill aaddrick/ticketmill#166 -->
    

    parseGateStateComment cross-checks the embedded repo as well as issue and schema, so a block copied between repos fails to malformed rather than being trusted.

    7. Group inheritance (minor, adopted as a documented decision, no code change)

    Confirmed: deriveUnits:1292 is Object.assign({}, primaryRef, {...}) with three explicit union-folds. A new preflight field is inherited from the primary alone, and a re-anchor drops it.

    Decision: do not union-fold, and record why in gate-hygiene.md. The three neighbors are unions by nature; a group's predicted files, dependencies and revisit reasons genuinely are the set union across its members. Gate budgets are not. plan_iters is a per-issue count against a per-issue cap, and OR-folding or maxing budgets from independent gates would fabricate a number no gate ever produced. Losing the primary's state on re-anchor fails open to a fresh budget, the same direction as absent, which is today's behaviour for every issue. Documented explicitly so the follow-up tier inherits the reasoning instead of rediscovering it.

    Unchanged from iteration 1

    Consolidation-marker mirroring; title-gated ## Gate State closing with the canonical marker line; JSON rather than consolidation's flat key-value lines, because settled[] is an array of five-field objects with free text; append-only with positional last-wins so idempotence falls out instead of needing an exists-check; no wall clock anywhere (scripts/lint-engine.js:68 bans Date.now(, verified); all parsing pure and above TICKETMILL-TEST-HARNESS-SPLIT:6522; four distinguishable states (found / absent / read_failed / malformed), everything but found failing open to today's behaviour and everything but absent logging; no consumer, so ctx.metrics and ctx.settled stay untouched and a run with no prior block takes byte-identical decisions.

    No new M key: the write and the verify both pass stageOpts('probe') explicitly, so the :412 sonnet-default hazard the issue warns about cannot fire. Stage keys gate-state-plan, gate-state-pr-i<n>, gate-state-verify match no entry in REWORK_STAGE_PREFIXES:5168, so they do not pollute computeReworkTax.

    New tests/gate-state.test.js driven through tests/harness.js, covering round trip, malformed JSON, truncated fence, wrong issue number, wrong repo, wrong schema version, non-write-access author, read_ok:false, the zero-total-with-prior-work impossible case, the zero-total-fresh-issue case staying absent, and cap enforcement at parse time. No gh, no git. Ship both engine copies via node scripts/lint-engine.js --fix. Docs: new docs/architecture/gate-hygiene.md, its row in docs/architecture/index.md, and CHANGELOG.md. pipeline.md, metrics.md and failure-semantics.md stay untouched; they are hash-frozen by tests/architecture-provenance.test.js.

    Residual risks

    1. The verify call can fail benignly (agent death, gh hiccup) and log a mismatch on a write that actually landed. Non-fatal by construction: it degrades to today's zero validation, never worse.
    2. authorAssociation is GitHub-computed. If a deployment's poster somehow falls outside the accept set, gate state reads permanently absent, but logged as malformed with the login every time, so it is loud rather than silent.
    3. Four comments per issue per run is real noise on a long-lived issue. The <details> wrapper and the 6-entry settled cap keep each one small, and the probe only ever reads the last 3.
  8. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Contrarian: Approach Challenge (Iteration 2)

    Verdict: needs_rework — three majors, all of them narrow. The architecture stands; two of the three are re-adjudications of iteration-1 fixes that I measured and found to misfire, and I am citing the measurements.

    Strengths (steel-man)

    The iteration-1 rework landed. Verified in the tree at HEAD (5715792):

    Iteration-1 finding Fix as revised Check
    Unfalsifiable read-back Split into a second stage() that never sees the payload Precedent confirmed: fetchConsolidationMarkers:4020 is a separate probe from postConsolidationMarkers:4238
    Truncating fetch Pin gh issue view <n> --repo <r> --json comments I tested the pinned command against a 397-comment issue (`gh issue view 75107 --repo golang/go --json comments --jq '.comments
    Four plan-gate exits One post below the loop Confirmed: :4583 dismissed, :4591 accepted, :4601 cap, :4614+:4626 re-plan-failed; loop ends before IMPLEMENT
    priorWork(pre) fields exist pr_number, worktree_exists, branch, resume_point All four are in PREFLIGHT_SCHEMA:487-497
    Single probe fallback to patch One call site PREFLIGHT_SCHEMA is used once, :7201, with the died-default at :7213
    Parse-time caps could false-alarm the verify — Not a problem: settleDecision:1866 is the only writer to ctx.settled, and it applies 400/300/4x160 on write, so re-applying at parse is idempotent. No spurious mismatch

    Keeping the pr-review write inside the loop is right for the reason given (reviewAndMerge returns from inside it), and I confirmed there is no ctx.settled writer that bypasses the caps. Also confirmed the engine has no require/child_process at all — every shell call goes through an agent — which makes the verify stage load-bearing rather than ceremonial.

    Findings

    [Major] authorAssociation is not a write-access proxy, and the accept set rejects exactly the deployments it was adopted to protect.
    The owner-only filter was narrowed to OWNER|MEMBER|COLLABORATOR because "a deployment running the mill under a bot or a write-access collaborator would see every block rejected." Measured on live GitHub just now:

    repo:home-assistant/core -> author github-actions, authorAssociation "NONE"   (x5 issues)
                                author epenet (core maintainer),  "CONTRIBUTOR"
    

    github-actions lands in the reject set. And MEMBER is only returned for publicly visible org membership, so an org maintainer with private membership reads CONTRIBUTOR — also rejected. The narrowing therefore produces the identical regression it was adopted to prevent, one class over: a bot-run or private-membership deployment writes blocks its own reader discards, reads permanently absent, and logs its own writes as malformed forever. Fail-closed, permanent, and self-inflicted. Recommendation: stop inferring trust from association. Derive the trusted login in band — the same issue already carries ## Ticketmill Claimed written by this deployment's own identity, and the probe is already fetching every comment; trust blocks whose author_login matches the claim/marker comment's author, with the OWNER|MEMBER|COLLABORATOR set as an additional accept, never as the sole gate. That inherits exactly the trust the claim protocol already has (no new surface), needs no config, and works under any identity. Keep both fields on the schema either way.

    [Major] The payload has no staleness anchor, and "no wall clock" is a self-imposed choice, not a constraint.
    Iteration 1 endorsed positional last-wins as "forced and correct." New evidence: scripts/lint-engine.js:68 bans Date.now() in JS only, and the engine already sources real epochs from agents in three places — date +%s written into the claim body (:7533), date +%Y-%m-%d_%H%M%S for the batch branch (:6925), and the injected now param on gradeFromObservation (:6488). It even acts on one: CLAIM_STALE_SECONDS:292 is 12h. So a timestamp was available and was not taken. The consequence: a block from a run abandoned three months ago is positionally indistinguishable from one written five minutes ago, and run cannot substitute — RUN_TAG:269 is A.run_label || A.date || 'run', which defaults to the literal string run. Failure scenario: an issue caps its plan gate at 3, halts, is reopened months later in a new batch; the consumer reads plan_iters: 3 and grants the fresh attempt zero plan-gate iterations. That fails closed — the one direction this design promises never to fail. Recommendation: carry an epoch in the payload sourced the way the claim comment already does ($(date +%s) inside the write agent's command), plus batch. Ordering stays positional; the epoch is only ever a staleness bound for the consumer. Per the plan's own argument for adding author_login now, a field added later is a contract change on a consumer that already depends on it.

    [Major] The verify validates the first post; last-wins consumes the last.
    The bound is up to 4 posts per issue per run (1 plan + 3 pr-review), ctx.gate_state_verified fires on the first successful one, and the reader takes the newest block. So in every multi-post run — precisely the long ones a resume exists for — the block that gets consumed is the one that was never checked. And the checking matters here specifically because JS cannot post: the engine has no child_process, so a haiku agent must reproduce a fenced JSON body verbatim through shell quoting (free text in settled[] routinely contains apostrophes). A mangle on post 4 is invisible. Recommendation: verify the last write, not the first. Cheapest shape that keeps one probe: leave the flag unset and instead re-run the verify after any post that supersedes a verified one, capped at 2 per issue (plan boundary + final pr-review boundary); at stageOpts('probe') that is one extra haiku call, against a pipeline that already spends opus on three contrarian iterations.

    [Minor] The malformed-tail fallback returns found with stale data and logs nothing.
    Iteration-1 minor 5 was adopted as "the probe returns the last 3 blocks — last-wins needs one plus a fallback for a malformed tail." As specified, a run where the newest block is malformed and the previous one parses returns state found, silently, carrying a strictly older budget. That is a fifth condition the four-state table does not cover, and it violates hard constraint (a) more sharply than absent does, because it is confidently wrong rather than empty. Recommendation: log every skipped block and return a stale: true (or found_fallback) marker alongside found, so "I used an older block" is distinguishable from "I used the newest block."

    [Minor] Bolting the read onto the preflight probe diverges from the precedent it claims to mirror.
    fetchConsolidationMarkers is its own probe over the whole candidate set in one call. The plan instead appends a 7th step to the per-issue preflight at :7201 — the probe that decides resume_point (skip/process_pr/implement) for every issue in every batch, already carrying six numbered steps at haiku/low. Adding an instruction to return up to three raw JSON blobs both bloats the most routing-critical probe in the pipeline and costs N calls where the precedent costs one. Recommendation: a separate fetchGateStateBlocks(issueNumbers) probe alongside the preflight Promise.all, shaped like fetchConsolidationMarkers. The routing probe stays byte-identical, the new read's failures are isolated to its own read_ok, and PREFLIGHT_SCHEMA stays untouched (attach the result onto pre in JS, where attachEngineOwnedIntentional:3023 already shows the pattern).

    [Minor] The approach-gate budget is still lost on the path just above the plan gate.
    Adopted finding 4 moved the write below the plan-gate loop, which is right. But implementIssue returns at :4542 (plan agent died) and :4543 (planning failed), both after the approach gate has burned up to 3 opus contrarian iterations and before any write exists. A resume re-runs the approach gate on a fresh budget — the same hole, one level up. Recommendation: either move the single write to just after the approach-gate loop and re-post it after the plan-gate loop (two calls, both boundaries covered), or record the gap explicitly in gate-hygiene.md as accepted scope. Do not leave it undecided.

    Pre-mortem

    The way this fails in six months is not the write path. It is a third-party deployment that installed the plugin, ran it under a CI identity, and has been logging malformed on its own comments every run since — reading absent forever while believing it has durable state. Finding 1 is that story; finding 2 is the same story with the sign flipped, where the state is read and is silently three months old.

  9. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Revised Evaluation (after contrarian iteration 2)

    All three majors and all three minors are confirmed against the tree and live GitHub, with one correction to the evidence behind finding 1 that does not change its conclusion. Two remedies are adopted as proposed; one is adopted in diagnosis but implemented by a cheaper mechanism, argued below. I also found an error in my own prior approach (the docs target already exists) and corrected it.

    Finding 1 (major): association is not a write-access proxy. CONFIRMED, with an evidence correction.

    Measured on live GitHub just now:

    comment author repo author_association
    github-actions[bot] actions/runner#3661 CONTRIBUTOR
    k8s-ci-robot kubernetes/kubernetes#120000 CONTRIBUTOR
    gopherbot golang/go#75107 CONTRIBUTOR
    bors, rustbot rust-lang/rust#120000 COLLABORATOR
    nodejs-github-bot nodejs/node#50000 COLLABORATOR

    Correction: I could not reproduce NONE for github-actions[bot]; it measured CONTRIBUTOR. The conclusion is unaffected, because CONTRIBUTOR is outside the OWNER|MEMBER|COLLABORATOR accept set either way, so a write-access bot identity is rejected by the narrowed gate. I did not independently verify the private-org-membership half of the finding; it is not load-bearing, since the bot half alone establishes that association is not a write-access proxy.

    Adopted. Derive the trusted login in band. The probe already fetches every comment, so it also returns claim_authors: the login of every comment whose first line is exactly ## Ticketmill Claimed (or LEGACY_CLAIM_TITLE). That comment is written by this deployment's own gh identity at :7533, and it survives the run, because the claims-release sweep at :7671 removes only the label, never the comment. Verified on this issue, where the claim comment is present and authored by the same identity every pipeline comment carries.

    Accept a block when author_login is in claim_authors OR author_association is in {OWNER, MEMBER, COLLABORATOR}. Association is demoted to a secondary accept, never the sole gate. If both miss, the block is skipped with reason untrusted_author, logged, and the read degrades to absent, which is today's behaviour.

    Accepted risk, stated plainly: anyone can post a ## Ticketmill Claimed comment and self-bootstrap into claim_authors. This does not widen the existing surface, because a forged claim already makes real runs skip the issue outright at :7528, which is a strictly worse denial of service than seeding a block nothing reads. Blast radius stays bounded by the repo/issue/schema cross-checks, the parse-time cap re-application, and the fact that this issue ships no consumer.

    Finding 2 (major): no staleness anchor, and the no-wall-clock rule was self-imposed. CONFIRMED.

    scripts/lint-engine.js:68 matches the literal Date.now( and :82 the argless new Date(). Both are source-text bans on the engine JS, not bans on an agent shelling date. The engine already sources real time from agents in four places: :7000 and :7098 (date -u +%Y-%m-%dT%H:%M:%SZ, returned verbatim as now), :7533 (the claim's started: $(date +%s)), and :3237/:3241 (the worktree lock).

    Adopted, with the sourcing simplified. Rather than embedding $(date +%s) inside the write agent's printf (JSON quoting inside a shell heredoc is exactly where a payload gets mangled), the new read probe returns now verbatim using the :7000/:7098 idiom, and JS computes RUN_EPOCH through the existing pure toEpochMs helper at :6377. That probe runs once per run at Select, strictly before any write, so the epoch costs zero extra calls.

    The payload gains epoch and batch. Ordering stays positional; the epoch is only ever a staleness bound for the consumer. The epoch is run scoped, so every block a run writes shares one value, which matches the claim comment's run-scoped started: exactly. If the probe dies, epoch is null and the consumer must treat an unbounded block as stale.

    No threshold is enforced here, because this issue ships no consumer. gate-hygiene.md documents the intended bound as CLAIM_STALE_SECONDS (12h, :292) so the consumer tier inherits one staleness policy rather than inventing a second.

    Finding 3 (major): the verify validates a block that gets superseded. CONFIRMED, remedy improved.

    MAX_PR_REVIEW_ITERATIONS = 3 at :293, and with the approach boundary added below there are up to five posts per issue. A one-shot first-post verify validates the smallest, earliest payload while last-wins consumes the largest, latest one. The gap is real: settled and gate_budgets both grow across the run, so a truncation or escaping failure that only appears at size is exactly what the first-post verify cannot see.

    Diagnosis adopted. Remedy changed, and I believe strictly improved. The finding proposes re-verifying after any superseding post, capped at 2 per issue. I inspected the exits and that cap cannot reliably land on the last post: reviewAndMerge leaves its loop through three breaks (prReviewClean, bothNothingToFix, capReached) and two return fail(...) paths (pr-fix failure, quality halt). At post time, inside the loop, the code cannot know that this iteration was the last one, so a cap-2 in-loop scheme spends its second verify on iteration 1 or 2 and still misses the final block on the return-fail exits, which are precisely the needs_human halts a human resumes from.

    Instead: one run-level sweep at Report, verifyGateState(results), shaped like the claims-release sweep at :7671 and like fetchConsolidationMarkers. It re-reads each issue's newest ## Gate State block and compares it against the intent JS recorded. That is one haiku probe per run rather than up to 2N, and it always sees the truly final block for every issue on every exit path.

    Plumbing: ctx.gate_state_intent (the canonical JSON.stringify of the last payload posted) rides out on both processIssue exits exactly the way gate_findings and settled already do in fail() at :2897 and on the success return. The verify prompt still carries only the issue number, the literal title, and the pinned fetch, never the payload; it returns comment_id plus raw body and JS does the comparison. The write/read split is preserved.

    Accepted risk: a mismatch is now discovered after merge rather than mid-run. That is acceptable because nothing consumes the block in this issue, which is the entire point of shipping the substrate first, and the mismatch lands in the log and the run report.

    Finding 4 (minor): the malformed-tail fallback is a silent fifth state. CONFIRMED, adopted.

    readGateState returns { state, payload, stale, skipped }. state stays the four-value table (found | absent | read_failed | malformed), stale: true marks "the newest block was unusable and I fell back to an older one", and skipped lists every rejected block with a reason (untrusted_author, malformed_json, wrong_issue, wrong_repo, wrong_schema). Every skip is logged. "I used an older block" is now distinguishable from "I used the newest block".

    Finding 5 (minor): a 7th preflight step is not the precedent it claims. CONFIRMED, adopted.

    fetchConsolidationMarkers at :4021 is its own probe over the whole candidate set in one call, and attachEngineOwnedIntentional at :3023 attaches a derived field onto each preflight in JS with a non-mutating Object.assign map. Both precedents fit better than inflating the routing prompt.

    New fetchGateStateBlocks(issueNumbers) at stageOpts('probe'), pinned to gh issue view <n> --repo <r> --json comments (never a bare gh api; the pinned form paginates, verified at 397 comments). It returns per issue: now, read_ok, total_comments, blocks (the last 3 whose first line is exactly ## Gate State, each as {body, author_login, author_association}), and claim_authors. A new attachGateStateBlocks(preflights, byIssue) mirrors attachEngineOwnedIntentional. PREFLIGHT_SCHEMA at :478 and the routing prompt are untouched.

    This deviates from the issue's literal wording ("add one step to the preflight probe, and a field on PREFLIGHT_SCHEMA"). It preserves the stated contract (the agent returns raw text and never decides) and every acceptance criterion, and it costs one call instead of N inflated prompts. Flagged in the PR body as a deliberate deviation.

    Finding 6 (minor): the approach-gate budget is lost above the plan gate. CONFIRMED, and it is three paths, not two.

    Between the approach loop and the plan loop there are three return fail(...) exits: plan agent died, planR.status !== 'success', and !tasks.length. On all three the approach gate's consumed iterations are never written, so a resume redraws the opus-tier budget.

    Decided, not deferred: adopt option (a). Write once immediately after the approach-gate loop, above those three returns. Boundaries become three:

    1. after the approach-gate loop, boundary: 'approach'
    2. once after the plan-gate loop, boundary: 'plan', covering all four of its exits (all four are breaks, re-verified)
    3. once per pr-review iteration right after recordGateOutcome(ctx, 'pr-review', ...) at :4903, boundary: 'pr-review-iN', kept inside the loop because reviewAndMerge returns from inside it

    Max five posts per issue. Editing one comment in place would have removed the noise, but the issue text prescribes append-only with last-wins, so that trade is not mine to make here.

    Correction to my own prior approach

    I claimed docs/architecture/gate-hygiene.md was new and needed an index.md row. Both are wrong: the file exists at 453 lines with eleven sections, and docs/architecture/index.md:15 already carries its row. I append a ## Durable gate state section only, and add no index row. tests/architecture-provenance.test.js and tests/docs-links.test.js both run to confirm the append is clean.

    Unchanged from iteration 1

    The write/read split, the pinned fetch, the single post below the plan-gate loop covering all four exits, the priorWork fields, and parse-time cap re-application (settleDecision at :1866 is the only ctx.settled writer and caps on write, so re-application cannot false-alarm the verify) all stand as verified.

    Tests

    New tests/gate-state.test.js through tests/harness.js, no gh and no git: found, absent, read_failed, malformed-newest with stale fallback, all-malformed, untrusted author skipped while a claim-author block is accepted, wrong issue, wrong repo, oversized settled truncated to the 400/300/4x160 caps, epoch: null tolerated, and a build-then-parse round trip that is byte-stable through JSON.stringify. Both engine copies synced with node scripts/lint-engine.js --fix.

  10. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Contrarian: Approach Challenge (Iteration 3)

    Verdict: needs_rework — one major, four minors. The major re-opens an adjudicated decision, and I am citing the new evidence the settled-decisions contract requires: the rebuttal that closed it is false for the cheapest version of the attack. Everything else from iteration 2 has landed.

    Strengths (steel-man)

    The iteration-2 rework is real. I verified each remedy against the tree at 5715792:

    Iteration-2 finding Remedy as revised Check I ran
    Approach-gate budget lost above the plan gate Write immediately after the approach loop, above the three return fail(...) at :4541/:4542/:4545 Read :4401-:4489: all four approach-loop exits are breaks (contrarian died, accepted, cap, re-evaluate failed). One post after the loop covers every one. Confirmed
    Verify validates a superseded block Run-level verifyGateState(results) at Report phase('Report') runs after runPool on every terminal state: STOP.tripped fills not_started results and returns (:5288) rather than throwing, and a per-unit throw is isolated at :5296. The sweep really does see each issue's final block on every exit path, at one call instead of 2N
    Preflight bloat Separate fetchGateStateBlocks + attachGateStateBlocks fetchConsolidationMarkers:4021 is a whole-set probe; attachEngineOwnedIntentional:3023 is a non-mutating attach. Both precedents fit
    Epoch Probe returns now, JS derives via toEpochMs:6382 Confirmed pure; confirmed lint-engine.js:68/:82 bans only the JS source text, not an agent shelling date
    Malformed tail {state, payload, stale, skipped}, every skip logged Adopted as written

    Two extra checks came back clean and I am not raising them: settled capped at 6 matches settledBlock:1874's own slice(-6), and settleDecision fires at most twice per issue (:4444 approach, :4589 plan), so nothing is truncated today. And results does carry per-issue ctx fields out of both exits (fail():2902, success return :5068), so gate_state_intent rides out exactly as claimed.

    Findings

    [Major] claim_authors is self-bootstrappable at zero cost, and the rebuttal that closed this decision does not hold for a stale forged claim.

    • Assumption challenged: "anyone can post a ## Ticketmill Claimed comment and self-bootstrap into claim_authors. This does not widen the existing surface, because a forged claim already makes real runs skip the issue outright at :7528, which is a strictly worse denial of service."
    • New evidence: that holds only for a fresh forgery. The claim prompt at :7528-:7532 honors a foreign claim only when batch != TARGET AND (now - started) < CLAIM_STALE_SECONDS; a claim with started: 0 is stale, so the third branch fires and the real run claims the issue and proceeds normally. Step 4's race check likewise yields only to a claim that "is fresh". A stale forged claim therefore causes no DoS at all — no cost, no symptom — while still putting its author permanently into claim_authors, because the proposed rule collects the login of every comment whose first line is CLAIM_TITLE (:290), with no age or batch filter. The trade the rebuttal priced (pay a DoS to gain trust) is not the trade an attacker faces.
    • Failure scenario: the repo is PUBLIC (verified iteration 1). Attacker posts one stale claim comment, then a well-formed ## Gate State block with the right repo/issue/schema. It is the newest block; positional last-wins takes it; the author filter — the only thing standing between a public comment and JS-consumed state, which is iteration 1's adopted point — accepts it. When the follow-up tier seeds ctx.metrics/ctx.settled, forged gate_budgets present an exhausted PR-review budget and forged settled[] free text lands verbatim in settledBlock(), telling every downstream gate that re-litigating it "is itself a process failure."
    • Impact: the trust contract freezes now and a later tier depends on it — the same "decide it now, retrofitting is a contract change" argument the plan itself used to adopt author_login and epoch.
    • Recommendation: derive the trusted identity out of band from the deployment's own token: gh api user --jq .login, one extra line in the probe already being written. I ran it here and it returns aaddrick. It needs no config, works for any human or bot PAT, and is not forgeable by a commenter; it is a single-object endpoint, so the plan's "never a bare gh api" rule (which is about comment-list pagination) does not apply. Keep claim_authors as a fallback for tokens where gh api user 403s (GitHub App installation tokens), but restrict it there to claims that are fresh (< CLAIM_STALE_SECONDS) or whose batch: equals this run's TARGET, so a zero-cost stale forgery grants nothing.

    [Minor] The payload carries no group identity, so the documented fail-open is undiagnosable.
    I am not re-raising the settled decision itself: iteration 1 section 7 adjudicated "do not union-fold; a re-anchor loses the primary's state and fails open to a fresh budget," and I accept that reasoning. What is new is context around it. consolidationEnabled:1165 is profile.consolidation !== false, so grouping is on by default and this repo's profile does not disable it; and postConsolidationMarkers:4238 — the precedent this design mirrors "end to end" — posts a marker on every member issue, not only the primary. Gate state posts only on ctx.issue, and the payload has no group_id/primary/members, so a consumer cannot tell a singleton's block from a three-issue unit's, nor "re-anchored, state sits on the old primary" from "never written." Recommendation: add group_id and members to the payload (cheap now, a contract change later) and state the member-issue gap in gate-hygiene.md beside the no-union-fold reasoning.

    [Minor] The Report sweep will report a false mismatch under a concurrent unclaimed run.
    Claims fail open: the claim stage proceeds unclaimed when its agent dies, and a foreign claim flips resume_point only when it is fresh. Two runs can therefore both write blocks on one issue. verifyGateState compares ctx.gate_state_intent against the newest block, so a foreign run's later post reads as a corruption alarm that is not one — and that alarm is the entire product of this stage. Recommendation: select the newest block whose run and batch match this run before comparing, and report "superseded by another run" as its own outcome rather than as a mismatch.

    [Minor] Two small holes in the epoch plumbing as specified.
    The probe returns now per issue, but RUN_EPOCH is a single run-scoped value — which element wins is unspecified, and per-issue values will differ across a Promise.all. And toEpochMs:6382 returns NaN on an unparseable string, which JSON.stringify silently renders as null, indistinguishable from "the probe died" — the one case the design wants a consumer to treat as unbounded. Recommendation: reduce explicitly (first finite value, or return now once at the top level of the probe result rather than per issue) and guard with Number.isFinite, so epoch is either a real number or an explicit, logged null.

    [Minor] Pin the posting idiom rather than inheriting --body.
    postConsolidationMarkers:4247 hands the agent a verbatim body with "gh issue comment ... --body", but every consolidation body is flat key: value with no free text. Gate-state bodies carry settled[] free text — apostrophes are routine — inside a JSON fence, and the Report-time verify exists precisely because that is where a mangle happens. Recommendation: pin gh issue comment <n> --repo <r> --body-file - with a quoted heredoc (<<'GATESTATE_EOF') in the prompt, so no shell interpolation touches the payload at all.

    Pre-mortem

    The failure I would bet on six months out is no longer mechanical: the epoch, the trust field, the last-write verify and the state table have all been argued to a good place. It is that this substrate ships advertised as trusted directly by JS, the follow-up tier consumes it on that promise, and the only gate protecting it turns out to be openable by any account willing to post two comments on a public issue — one of which the engine deliberately ignores.

  11. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Proceeding After Contrarian Cap (Approach)

    Iteration-2's remedies all landed and I verified each against the tree at 5715792: all four approach-loop exits are breaks (so one post after the loop covers them), phase('Report') runs on every terminal path including STOP.tripped and per-unit throws (so the run-level verify really does see each issue's final block), the separate probe matches fetchConsolidationMarkers/attachEngineOwnedIntentional, and toEpochMs plus the lint-engine bans are as described. One major remains: the adopted claim_authors trust rule can be self-bootstrapped for free, because the claim protocol ignores a STALE forged claim entirely (no DoS, the rebuttal's whole premise) while the proposed rule still whitelists its author permanently. Four minors cover missing group identity in the payload, a false-positive mismatch alarm under a concurrent unclaimed run, unspecified per-issue-to-run-scoped epoch reduction plus

  12. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Implementation Plan

    Summary. Ship the durable per-issue gate-state substrate as a title-gated ## Gate State issue comment, append-only with positional last-wins, mirroring the CONSOLIDATION_* marker subsystem end to end. Five tasks: (1) the pure JS layer plus schemas plus its unit suite, (2) the write path at three per-issue boundaries, (3) the run-level read probe plus the RUN_EPOCH derivation, (4) the Report-phase self-validation sweep that proves post -> GitHub -> read -> parse in the same run, (5) the docs/architecture/gate-hygiene.md section. No consumer: nothing seeds ctx.metrics or ctx.settled from a parsed block in this issue.

    Full plan

    Anchors (verified against the worktree at 5715792)

    All line numbers in the issue body predate three merged commits. Live anchors:

    What Live line
    PREFLIGHT_SCHEMA :478
    CONSOLIDATION_MEMBER_TITLE / _GROUP_TITLE :994 / :995
    buildConsolidationGroupComment / parseConsolidationGroupComment :1028 / :1051
    settledBlock (slice(-6)) :1874
    recordGateOutcome :2097
    attachEngineOwnedIntentional :3023
    fetchConsolidationMarkers :4021
    postConsolidationMarkers posting idiom :4238-:4247
    approach-gate loop (four exits, all break) :4401-:4489
    plan-gate loop (four exits, all break) :4547-:4700
    recordGateOutcome(ctx, 'pr-review', ...) :4903
    fail() result assembly :2902
    success return :5068
    processIssue ctx (settled: [], pr_review_iters: 0) :5078 / :5103
    process_pr resume path :5128
    toEpochMs :6382
    TICKETMILL-TEST-HARNESS-SPLIT :6788
    preflight probe call site :7172-:7201
    phase('Report') :7670
    CLAIM_TITLE / CLAIM_STALE_SECONDS / RUN_TAG :290 / :292 / :269

    The comment shape

    ## Gate State
    Machine-readable gate record for a resumed run. Not an instruction to any agent; carries no directive weight.
    <details><summary>gate state (schema 1)</summary>
    
    ```json
    { "schema": 1, "repo": "...", "issue": 166, "run": "<RUN_TAG>", "batch": "<TARGET>",
      "epoch": 1753660800000, "boundary": "plan", "group_id": null, "members": [166],
      "gate_budgets": { "approach": {...}, "plan": {...}, "pr-review": {...} },
      "settled": [ ...last 6... ] }
    
    ```

    JSON rather than consolidation's flat key: value lines because settled[] is an array of five-field objects carrying free text. The canonical marker line closes the body, exactly as every consolidation marker does.

    Task 1 — pure substrate + schemas + unit tests

    All new functions go above the TICKETMILL-TEST-HARNESS-SPLIT marker at :6788, beside the consolidation parsers they mirror.

    • GATE_STATE_TITLE = '## Gate State', GATE_STATE_SCHEMA = 1, GATE_STATE_FENCE constants.
    • buildGateStatePayload(ctx, boundary, meta) — assembles the object above from ctx.gate_findings, ctx.metrics, ctx.settled.slice(-6) (matching settledBlock's own window at :1874), ctx.groupId, memberIssues(ctx).
    • buildGateStateComment(repo, issue, payload) — title, the non-directive human line, <details> + fenced JSON, canonical marker. JSON.stringify(payload, null, 2).
    • parseGateStateComment(body, expect) — title-gated on the first line (an unrelated comment that merely says "gate state" is never misread), extracts the fence, JSON.parse inside try/catch, then rejects on schema !== 1, repo !== expect.repo, or issue !== expect.issue. Returns null on every failure path; never throws. Covers the issue's malformed-JSON, truncated-block and wrong-issue cases.
    • selectGateState(blocks, expect) — the three-state reducer, returning { state: 'found' | 'absent' | 'read-failed', payload, trusted, skipped: [reasons] }. read-failed is produced only when the probe reported failure (gate_state_read_ok === false), never inferred from an empty array; absent means the probe succeeded and no block parsed. Positional last-wins over the trusted subset; every rejected block contributes one entry to skipped so a caller can log why.
    • isTrustedGateStateAuthor(login, trust) — contrarian major. Primary rule: login === trust.self_login, where self_login comes out of band from the deployment's own token (gh api user --jq .login), which a commenter cannot forge. Fallback for tokens where that 403s: trust.claim_authors, but only authors of a claim that is fresh (< CLAIM_STALE_SECONDS) or whose batch: equals this run's TARGET, so a zero-cost stale forged claim grants nothing.
    • deriveRunEpoch(now) — contrarian minor. toEpochMs(now) guarded by Number.isFinite; returns a real number or an explicit null (never a NaN that JSON.stringify silently renders as null). Caller logs the null case.
    • diffGateStateIntent(intent, parsed) — canonical compare through JSON.stringify of both sides with sorted keys; returns 'match' | 'mismatch' | 'superseded'. Contrarian minor: superseded when the newest block's run/batch do not match this run's, so a concurrent unclaimed run's later post is not reported as corruption.
    • attachGateStateBlocks(preflights, byIssue) — non-mutating attach, shaped like attachEngineOwnedIntentional at :3023.
    • Schemas: GATE_STATE_PROBE_SCHEMA (blocks[] {issue, body, author_login, author_association}, read_ok, total_comments, self_login, now) and GATE_STATE_VERIFY_SCHEMA ({issue, comment_id, body, read_ok}). Add optional gate_state_blocks, gate_state_read_ok, gate_state_trust to PREFLIGHT_SCHEMA at :478 — the issue asked for a field on PREFLIGHT_SCHEMA to carry the raw blocks, and it gets one; the population route differs (see task 3) and that deviation is written down in the docs task.
    • No new agent stage key is needed: every new agent call uses stageOpts('probe'), an existing key in the M map at :352, so the stageOpts default-to-sonnet trap the issue warns about cannot fire.
    • tests/gate-state.test.js (new), harness-driven: build/parse round trip; a body with an apostrophe-bearing settled entry; malformed JSON; a truncated fence; a block carrying a different issue number; a block carrying a different repo; a wrong schema; found/absent/read-failed all distinguishable; last-wins across three blocks; untrusted author rejected and stale-claim author rejected; deriveRunEpoch on a good string, on garbage, and on undefined; diffGateStateIntent match/mismatch/superseded.
    • Run node scripts/lint-engine.js --fix in the same commit (profile verify_notes LOCKSTEP-EDIT rule; .claude/workflows/ticketmill.js is a mechanical mirror of the source this issue targets, enforced by test_command). Then node --test.

    Task 2 — the write path

    • postGateState(ctx, boundary), a non-fatal helper next to the cap-note stages it is modeled on. Builds the payload, stores it on ctx.gate_state_intent (last write wins in memory too), and posts through stageOpts('probe') with NOTE_SCHEMA and 1 retry, exactly like cap-note-plan. A dead agent or posted !== true only log()s — no path may fail an issue.
    • Contrarian minor, pinned idiom. The prompt pins gh issue comment <n> --repo <r> --body-file - fed by a quoted heredoc (<<'GATESTATE_EOF'), so no shell interpolation touches a payload that routinely carries apostrophes inside free text. postConsolidationMarkers at :4247 uses --body because its bodies are flat key: value with no free text; that inheritance is deliberately broken here.
    • Three boundaries, at most five posts per issue:
      1. approach — immediately after the approach-gate loop closes at :4489, above the three return fail(...) exits at :4541/:4542/:4545. All four loop exits are breaks, so one post covers every one.
      2. plan — once after the plan-gate loop at :4700. Same four-break structure.
      3. pr-review-iN — inside the pr-review loop, right after recordGateOutcome(ctx, 'pr-review', ...) at :4903. Deliberately not moved below the loop: reviewAndMerge returns from inside it at the both-nothing-to-fix and cap-reached breaks, and those are exactly the runs a resume needs the count for.
    • Thread gate_state_intent out on results: add it to fail()'s object at :2902, the success return at :5068, and the resume_point === 'skip' return, so the Report sweep can see each issue's last intended block on every terminal path.
    • Tests: extend tests/pr-review-gate.test.js (or a scenario block in tests/gate-state.test.js) driving reviewAndMerge through the stubbed-agent harness to assert one gate-state stage per pr-review iteration, that a dead gate-state stage does not change the loop's outcome, and that ctx.gate_state_intent reflects the last boundary.
    • Lockstep sync + node --test.

    Task 3 — the read path

    • fetchGateStateBlocks(issueNumbers) at stageOpts('probe'), shaped like fetchConsolidationMarkers at :4021: ONE call over the whole set, read-only, safe under DRY_RUN. Pins gh issue view <n> --repo <r> --json comments per issue — never a bare gh api for the comment list, which paginates. Returns the last 3 matching blocks per issue as {body, author_login, author_association}, plus read_ok, total_comments, and two run-scoped top-level facts: self_login (gh api user --jq .login, a single-object endpoint, so the no-bare-gh api rule about pagination does not apply) and now (date -u +%Y-%m-%dT%H:%M:%SZ returned verbatim — the exact idiom already at :7000 and :7098).
    • Contrarian minor, epoch plumbing. now is returned once at the top level of the probe result, not per issue, so there is no unspecified per-issue reduction. RUN_EPOCH = deriveRunEpoch(probe.now) at module scope near the Select call site; a null is logged loudly. No Date.now() anywhere — scripts/lint-engine.js bans it and ordering is comment position, never wall clock.
    • Wire at Select immediately after attachEngineOwnedIntentional at :7221: preflights = attachGateStateBlocks(preflights, blocks). A dead probe yields gate_state_read_ok: false on every preflight, which selectGateState turns into read-failed, and the wiring logs one line per issue in that state. That is the issue's visible-fail-open constraint: found / absent / read-failed are three distinct values and only the third logs.
    • Per-issue log line naming the state and the count of skipped blocks, so a first run (absent) and a broken read (read-failed) never look the same in the transcript.
    • Tests: attach is non-mutating and total; a dead probe marks every preflight read-failed; a probe returning blocks for a subset leaves the rest absent; a run whose probe returns nothing produces preflights byte-identical to today's plus the three new fields.
    • Lockstep sync + node --test.

    Task 4 — the Report-phase self-validation sweep

    • verifyGateState(results) called from phase('Report') at :7670, before the token/friction rollups. One stage, one round trip, for the whole run rather than 2N.
    • The prompt carries only the issue numbers, the literal ## Gate State title, and the pinned fetch command — never the payload, so the agent cannot echo back what it was told and pass a comparison it never performed. It returns comment_id plus the raw body per issue.
    • JS then runs parseGateStateComment(body, expect) and diffGateStateIntent(result.gate_state_intent, parsed), logging one of match / mismatch / superseded / read-failed / no-intent per issue. A mismatch is the loud outcome the issue asks for and the whole product of this stage; superseded covers the concurrent-unclaimed-run case rather than raising a false alarm. Non-fatal throughout: the sweep never changes a result's status.
    • phase('Report') runs on every terminal state — STOP.tripped fills not_started results and returns rather than throwing, and a per-unit throw is isolated — so the sweep sees each issue's final block on every exit path.
    • Tests: verifyGateState's pure comparison half over fixtures for each of the five outcomes; a dead verify stage is a no-op.
    • Lockstep sync + node --test.

    Task 5 — documentation

    • Append a section to docs/architecture/gate-hygiene.md (the file exists; do not create it) covering: the comment shape and why JSON rather than flat lines; the three boundaries and why the pr-review post stays inside the loop; append-only positional last-wins; the three-state fail-open contract and which state logs; the trust model (self_login primary, restricted claim_authors fallback) and the stale-forged-claim reasoning behind it; the epoch derivation and the CLAIM_STALE_SECONDS (12h) bound documented as the intended staleness window with no threshold enforced at this tier; the group-identity gap — blocks post only on ctx.issue while postConsolidationMarkers posts on every member, so a re-anchored primary fails open to a fresh budget, which is why group_id/members ride in the payload; and the deviation from the issue body's "add a step to the preflight probe" (a separate whole-set probe was adopted; PREFLIGHT_SCHEMA still carries the fields).
    • Update the gate-hygiene.md row description in docs/architecture/index.md, and the matching rows in docs/architecture/AGENTS.md and docs/architecture/CLAUDE.md (byte-identical parity pair).
    • Do not touch pipeline.md, metrics.md, or failure-semantics.md — hash-frozen by tests/architecture-provenance.test.js, and the tech-docs stage runs after the last test-loop execution, so a frozen-prose edit surfaces only as a red CI check on the batch PR.
    • Do not hand-edit CHANGELOG.md or .claude-plugin/plugin.json: .claude/ticketmill.json declares a release block, so the batch release stage owns both. Every recent per-issue commit follows that split.
    • node --test (this repo's tests/docs-links.test.js, readme-docs-census.test.js and architecture-split.test.js all police doc edits).

    Out of scope, restated

    Nothing seeds ctx.metrics or ctx.settled from a parsed block, and no budget is enforced from one. That is the follow-up tier that depends on this substrate.

  13. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Task List

    • 1. Pure gate-state substrate, schemas, and unit suite — ticketmill-implementer
      Above the TICKETMILL-TEST-HARNESS-SPLIT marker (:6788), beside the consolidation parsers: GATE_STATE_TITLE/GATE_STATE_SCHEMA constants, buildGateStatePayload, buildGateStateComment, parseGateStateComment (title-gated, fence-extracted, try/catch, rejects wrong schema/repo/issue, never throws), selectGateState returning {state: 'found'|'absent'|'read-failed', payload, trusted, skipped} with read-failed only from an explicit probe failure, isTrustedGateStateAuthor (primary self_login from gh api user; claim_authors fallback restricted to fresh-or-batch-matching claims), deriveRunEpoch (toEpochMs + Number.isFinite, explicit null never NaN), diffGateStateIntent (match/mismatch/superseded), attachGateStateBlocks (non-mutating, shaped like attachEngineOwnedIntentional:3023). Add GATE_STATE_PROBE_SCHEMA and GATE_STATE_VERIFY_SCHEMA, plus optional gate_state_blocks/gate_state_read_ok/gate_state_trust on PREFLIGHT_SCHEMA:478. New tests/gate-state.test.js covering round trip, apostrophe-bearing free text, malformed JSON, truncated fence, wrong issue, wrong repo, wrong schema, the three states, last-wins, untrusted and stale-claim authors, epoch guards, and all three diff verdicts. node scripts/lint-engine.js --fix in the same commit; node --test green.

    • 2. Write path: postGateState plus the three boundary call sites — ticketmill-implementer
      Non-fatal postGateState(ctx, boundary) modeled on cap-note-plan (stageOpts('probe'), NOTE_SCHEMA, 1 retry, log-only on failure), pinning gh issue comment <n> --repo <r> --body-file - fed by a quoted heredoc so no shell interpolation touches apostrophe-bearing payload text. Call at three boundaries: after the approach-gate loop (:4489, above the three return fail(...) at :4541/:4542/:4545), after the plan-gate loop (:4700), and inside the pr-review loop right after recordGateOutcome(ctx, 'pr-review', ...) at :4903 (kept in-loop because reviewAndMerge returns from inside it). Store the intended object on ctx.gate_state_intent and thread it out through fail():2902, the success return :5068, and the skip return. Harness tests: one gate-state stage per pr-review iteration, a dead stage changes no outcome, gate_state_intent reflects the last boundary. Lockstep sync; node --test green.

    • 3. Read path: fetchGateStateBlocks probe, Select wiring, and RUN_EPOCH — ticketmill-implementer
      New whole-set read-only probe shaped like fetchConsolidationMarkers:4021, pinning gh issue view <n> --repo <r> --json comments (never a bare gh api for comment lists), returning per issue the last 3 matching blocks as {body, author_login, author_association} plus read_ok/total_comments, and run-scoped top-level self_login (gh api user --jq .login) and now (date -u +%Y-%m-%dT%H:%M:%SZ verbatim, the idiom at :7000/:7098). Derive RUN_EPOCH = deriveRunEpoch(probe.now) once, logging a null. Wire attachGateStateBlocks at Select right after attachEngineOwnedIntentional:7221, and log one line per issue naming found/absent/read-failed so a broken read never reads as a first run. A dead probe marks every preflight read-failed. Tests: attach is total and non-mutating, dead probe, partial coverage, and a no-block run leaving preflights otherwise unchanged. Lockstep sync; node --test green.

    • 4. Report-phase self-validation sweep — ticketmill-implementer
      verifyGateState(results) called from phase('Report'):7670 before the rollups: one stage for the whole run whose prompt carries only the issue numbers, the literal ## Gate State title, and the pinned fetch command — never the payload — returning comment_id and the raw body per issue. JS then parses and compares against each result's gate_state_intent, logging match/mismatch/superseded/read-failed/no-intent; superseded (block whose run/batch are not this run's) is its own outcome so a concurrent unclaimed run is not reported as corruption. Non-fatal end to end; never changes a result's status. Tests over fixtures for all five outcomes plus a dead verify stage. Lockstep sync; node --test green.

    • 5. Document the substrate in docs/architecture/gate-hygiene.md — ticketmill-implementer
      Append a section to the existing gate-hygiene.md: comment shape and why JSON rather than consolidation's flat lines; the three boundaries and why the pr-review post stays inside the loop; append-only positional last-wins; the three-state fail-open and which state logs; the trust model and the stale-forged-claim reasoning behind preferring self_login; the epoch derivation with CLAIM_STALE_SECONDS (12h) documented as the intended bound with no threshold enforced at this tier; the group-identity gap (blocks post only on ctx.issue while postConsolidationMarkers:4238 posts per member) and why group_id/members ride in the payload; and the deviation from the issue body's "step on the preflight probe" wording. Refresh the gate-hygiene.md row in docs/architecture/index.md and the parity pair AGENTS.md/CLAUDE.md. Do not touch pipeline.md, metrics.md, failure-semantics.md (hash-frozen), and do not hand-edit CHANGELOG.md or .claude-plugin/plugin.json (the batch release stage owns both per profile.release). node --test green.

  14. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Contrarian: Plan Stress Test (Iteration 1)

    Verdict: needs_rework — three majors, all narrow and all mechanical. The architecture is settled and I am not reopening it. Two of the three majors are things the approach chain adopted and the task set then dropped; the third is a harness-split placement error that would break Task 2's own required tests.

    Misfiled-comment check: every comment in this trail carries the marker <!-- ticketmill aaddrick/ticketmill#166 -->. Nothing misfiled, nothing deleted.

    Strengths (steel-man)

    The decomposition is right. Five tasks split cleanly along the pure/impure line the harness enforces, each ends at a green node --test, and the sequencing (substrate, write, read, verify, docs) matches the dependency order of everything except the one symbol flagged below. I re-verified the anchors that matter and they hold: attachEngineOwnedIntentional:3023, fetchConsolidationMarkers:4021, the approach loop closing at :4489 with the three return fail(...) at :4541/:4542/:4545, recordGateOutcome(ctx, 'pr-review', ...) at :4903, toEpochMs:6382, the split marker at :6788, phase('Report'):7670. memberIssues:1091 and ctx.groupId both exist, so group_id/members in the payload cost nothing new. The skipped return at :5116 really does assemble a result object, so threading gate_state_intent through it is a one-field edit. And I checked the token-accounting worry and dropped it: reconcile_error is pool-scoped (see aggregateTokens' poolSpend doc), so a Report-time sweep lands in orchestration_overhead beside the existing claims-release agent and cannot suppress the rework-tax rollup.

    Findings

    [Major] RUN_EPOCH is declared below the harness split, and Task 2 needs it before Task 3 exists.

    • Assumption challenged: that "module scope near the Select call site" is a usable home for a value the pure layer reads.
    • Evidence: tests/harness.js truncates the source at TICKETMILL-TEST-HARNESS-SPLIT:6788 and evaluates only what precedes it. Task 3 places RUN_EPOCH = deriveRunEpoch(probe.now) at the Select wiring (:7221), below the cut. buildGateStatePayload (Task 1, above the cut) reads it. Every harness test that drives postGateState, including Task 2's own required "one gate-state stage per pr-review iteration", throws ReferenceError: RUN_EPOCH is not defined. __seed:5342 cannot rescue it: it only reassigns bindings that already exist above the cut.
    • Second problem, same symbol: Task 2 lands before Task 3, so at Task 2's commit boundary the binding does not exist at all and node --test cannot be green as that task requires.
    • Impact: either Task 2 ships with epoch omitted, silently reverting iteration 2's adopted staleness anchor, or Task 2 is unbuildable as written.
    • Recommendation: follow the existing precedent exactly. ENGINE_OWNED is let ENGINE_OWNED = [] at :409, assigned at Select (:6879), seeded via __seed:5353. Move let RUN_EPOCH = null into Task 1 beside it, add if ('RUN_EPOCH' in o) RUN_EPOCH = o.RUN_EPOCH to __seed, and leave Task 3 with the assignment only.

    [Major] A failed post still sets ctx.gate_state_intent, so the Report sweep cries "mismatch" on the one path Task 2 designs for.

    • Assumption challenged: that gate_state_intent and "what is actually on the issue" stay in step.
    • Evidence: Task 2 says postGateState "builds the payload, stores it on ctx.gate_state_intent ... and posts", and "a dead agent or posted !== true only log()s". On a benign post failure the intent advances while the newest block stays at the previous boundary. Task 4 then compares intent against the newest block and logs mismatch.
    • Failure scenario: a run reaches pr-review-i3, the haiku post agent dies with STAGE_TRIES exhausted, and the pr-review-i2 block is still newest. Report announces a mismatch on an issue where nothing was mangled. The plan calls mismatch "the loud outcome the issue asks for and the whole product of this stage", so the single signal this issue ships is dominated by its own expected non-fatal path.
    • Impact: worse than noise. It conflates "a post did not land" with "GitHub mangled the payload", opposite bugs, and it trains the reader to ignore the alarm before any consumer exists.
    • Recommendation: set ctx.gate_state_intent only when posted === true, and record ctx.gate_state_post_failed = boundary on the failure path. Task 4 then reports post-failed as its own outcome beside match/mismatch/superseded/read-failed/no-intent, the same shape the superseded fix already established for the concurrent-run false positive.

    [Major] The falsifiability check for absent was adopted at iteration 1 and appears in no task.

    • Assumption challenged: that pinning gh issue view is sufficient to make absent falsifiable.
    • Evidence: Task 3 has the probe return total_comments per issue. No task consumes it. Task 1 specifies selectGateState(blocks, expect) with read-failed produced "only when the probe reported failure (gate_state_read_ok === false)", and neither the signature nor the test list mentions total_comments, priorWork, or the impossible case. The revised evaluation adopted the opposite in writing: "blocks empty && total_comments === 0 && priorWork(pre) -> read_failed (log)", plus "the probe also returns gate_state_total_comments, so the boolean stops being the only evidence". As planned, the sole evidence for absent is one model-attested boolean again.
    • Failure scenario, concrete and new: fetchGateStateBlocks is ONE haiku/low call over the whole candidate set, and it must read every comment of every candidate. This trail alone is 45KB and Make review findings the load-bearing artifact #162 already carries 31 comments. On a ten-issue batch the agent reads several hundred KB to extract a handful of blocks. The realistic degradation is not a non-zero exit, it is a truncated or skipped read that still returns read_ok: true with an empty blocks array. That reports as absent, which is precisely the silent fail-open the issue names as hard constraint (a).
    • Impact: the issue's own acceptance criterion is not met by the task set, and the failure is invisible in the transcript because absent does not log.
    • Recommendation: give selectGateState the third argument it needs ({read_ok, total_comments} plus the priorWork fields already on the preflight: pr_number, worktree_exists, branch, resume_point) and implement the adopted rule. Add the two cases the evaluation named to Task 1's test list: zero blocks with zero total against prior-work evidence classifies read-failed; zero blocks on a genuinely fresh issue stays absent. Separately, bound the probe, either chunking fetchGateStateBlocks or instructing it to read only the tail of each comment list, so the read does not silently degrade at batch size.

    [Minor] selectGateState quietly drops the malformed state and the stale fallback marker.
    Iteration 2's finding 4 was confirmed and adopted as {state, payload, stale, skipped} over a four-value table, with "I used an older block" distinguishable from "I used the newest block". Task 1 returns three states and no stale. An all-malformed read now reports absent, indistinguishable from a first run, and a malformed-tail fallback reports plain found carrying an older budget. skipped does preserve the raw information, so this is naming rather than loss, but the contract freezes here and a consumer reading state alone gets it wrong. Recommendation: restore malformed and stale as the evaluation adopted them. Two branches and two fixtures.

    [Minor] The :4700 anchor for the plan boundary points into the wrong loop.
    The plan-gate loop closes at :4629 and :4631 is // ---- IMPLEMENT. Line :4700 is inside the per-task implement loop, in the reviewer died — accepting provisionally branch. A literal reading of Task 2 posts a gate-state comment once per task review instead of once per issue. The prose ("once after the plan-gate loop") is right and the number is wrong, but the anchor table claims verification: fix it to :4630 or anchor on the // ---- IMPLEMENT comment.

    [Minor] Two of the three boundaries have no test.
    Task 2's test list covers only the pr-review loop. The approach boundary is the call site that cost iterations 2 and 3 a finding each: it must sit above the three return fail(...) at :4541/:4542/:4545, and its entire purpose is the run where planning dies. Nothing asserts that. Recommendation: add two harness cases to Task 2, one where the plan stage returns null and exactly one approach block is still posted, and one reaching IMPLEMENT that posts approach then plan in that order.

    [Minor] Nothing in the payload says whether this run's budgets are cumulative.
    Every block this tier writes comes from a ctx built fresh (settled: [], pr_review_iters: 0 at :5078/:5104), because there is no consumer yet. Under positional last-wins those blocks supersede richer ones, and a consumer cannot tell a block whose budgets were seeded from a prior block from one that started at zero. It fails open, so nothing breaks now, but the contract freezes here and this is the same "add the field now, retrofitting is a contract change" argument the chain used for author_login, epoch, and group_id. Recommendation: add seeded_from (the run/epoch of the block this ctx was seeded from, null when it was not) to the payload in Task 1.

    [Minor] Pin "the exact full body, never an excerpt" in both agent prompts.
    Task 1 correctly title-gates on the first line and notes that "an unrelated comment that merely says 'gate state' is never misread", which is true for a verbatim body. But this trail already contains two comments quoting a ## Gate State block with a fenced JSON payload naming the real repo and issue, authored by the same trusted identity, and #168's trail will contain more. The defense holds only while the probe returns whole bodies. fetchConsolidationMarkers:4026 pins "its exact full body" for exactly this reason; Task 3 says only "raw verbatim bodies". Recommendation: copy the precedent's wording into both the read and the verify prompts, additionally require the canonical marker to be the last non-empty line in parseGateStateComment, and add one Task 1 fixture built from a comment that quotes the block shape, asserting it parses to null.

    Pre-mortem

    The way this ships broken is not the design, which is now well argued. It is that the write half lands, the read half reports absent on every issue because the whole-set probe quietly truncated, and the only stage that could have caught it is busy logging mismatch on issues whose last post simply did not land. Both halves fail open, both are silent by construction, and the three approach iterations spent on making failure visible are undone by two omissions in the task list rather than by any decision anyone actually made.

  15. 38 remaining items

  16. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Test Validation (iteration 2)

    PASS. Full suite green (737/737 node --test, lint-engine lockstep clean). Independently re-verified the two fixes made since iteration 1's requested change: mutated write_seq: ++GATE_STATE_WRITE_SEQ to a static value — exactly the new assertion in gate-state-post.test.js's "approach then plan" test fails (10/11); mutated readOk in the diagnostic-log branch to always-true — exactly the 4 new "...WITH prior-work evidence" variants in gate-state-read.test.js fail (16/20), confirming both are genuinely load-bearing, not decorative.

    pr-review-gate.test.js's stage-key-order assertions were updated in lockstep with the new gate-state-pr-review-i*/-aborted boundary posts and strengthened, not weakened (now asserts the aborted boundary fires and the non-aborted one doesn't on a dead reviewer). gateStateProbeCommandLine's jq total-vs-blocks filter parity and the VERIFY_SKIPS push on mismatch/read-failed (with a negative test for match/superseded) are both covered through real control flow, not string snapshots.

    No hollow assertions, no green-by-omission gaps found against this diff.

  17. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Implemented via PR #177, squash-merged into batch branch `Batch_2026-07-27_225225` (88bba37). This does not close this issue: the batch branch has not merged to `main` yet, so closure happens when a human merges the batch PR (`Batch_2026-07-27_225225` -> `main`), whose body carries the `Closes #166` reference.

    Follow-ups filed from review history: #180, #181, #182.

  18. aaddrick commented on Jul 28, 2026

    @aaddrick
    OwnerAuthor

    Landed on main in batch PR #183 (v0.2.0), commit 0ac64ac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

gate-hygieneGate hygiene: findings, caps, and per-problem budgets (sequenced)

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions