diff --git a/.claude/workflows/ticketmill.js b/.claude/workflows/ticketmill.js index 94ad6ae..a725de3 100644 --- a/.claude/workflows/ticketmill.js +++ b/.claude/workflows/ticketmill.js @@ -318,6 +318,12 @@ const MAX_TOUCH_FILES = 100 // comment already sets for gating efficiency metrics; not a correctness input, // only a display/trust-flag threshold. const MAX_RECONCILE_ERROR_FOR_TRUST = 0.05 +// gate-state read (issue #166 task 3): fetchGateStateBlocks issues ONE agent +// call per chunk of at most this many issues (belt-and-braces — a dead chunk's +// agent call only takes its own chunk's issues down with it; surviving chunks +// still report). Not a correctness input: the per-issue jq command inside a +// chunk is fully independent of every other issue in it. +const MAX_GATE_STATE_PROBE_CHUNK = 5 // churn analytics (issue #89): a file appearing in >= this many DISTINCT // issues' ctx.changed_files within one run is a cross-issue hotspot (computeChurn) — // 2 is the smallest number that actually means "more than one issue collided on @@ -408,6 +414,27 @@ let VERIFY_SKIPS = [] // human-visible verification gaps -> batch PR b // PROFILE.engine_owned_globs, and PROFILE.lockstep_installed_paths respectively. let ENGINE_OWNED = [] let LOCKSTEP_INSTALLED_PATHS = [] +// RUN_EPOCH (issue #166): populated at Select from a probe-returned `date -u` +// string via the pure deriveRunEpoch/toEpochMs (below the TICKETMILL-TEST- +// HARNESS-SPLIT marker) -- the sandbox has no Date.now()/argless `new Date()`, +// so this is the run's only wall-clock anchor. null until Select assigns it; +// selectGateState treats a null run epoch as unknown age, which reads as +// stale, never as fresh (see gateStateEpochStale). +let RUN_EPOCH = null +// GATE_STATE_WRITE_SEQ (issue #166 PR #177 review): a per-run, module-level +// monotonic write counter -- NOT a clock and NOT Date.now()/Math.random() (a +// plain incrementing int is deterministic across a resume/replay the same way +// every other module-level counter in this file is). RUN_EPOCH is assigned +// ONCE at Select and is therefore IDENTICAL across every boundary a run +// posts, so it cannot order two same-run writes against each other -- +// diffGateStateIntent's 'superseded' verdict needs something that varies +// write to write. postGateState() increments this once per call and embeds +// it on the payload as `write_seq`; call order is write order, since a given +// issue's boundaries always post sequentially within that issue's own await +// chain. Never reset mid-run. diffGateStateIntent still gates supersession on +// `intent.run === actual.run` first, since two payloads from different runs +// never share counter provenance. +let GATE_STATE_WRITE_SEQ = 0 function stageOpts(key) { const base = M[key] || { model: 'sonnet' } @@ -507,6 +534,20 @@ const PREFLIGHT_SCHEMA = { // and deriveUnits() for how they're threaded onto the unit shape. predicted_files: { type: 'array', items: { type: 'string' } }, depends_on: { type: 'array', items: { type: 'integer' } }, + // OPTIONAL gate-state read (issue #166, Task 3's fetchGateStateBlocks probe): + // fail-open the same way predicted_files/depends_on do -- attachGateStateBlocks + // (below the TICKETMILL-TEST-HARNESS-SPLIT marker) normalizes a preflight + // missing any of these four to their fail-open default so selectGateState never + // sees an undefined key. gate_state_blocks carries raw verbatim comment bodies + // (most recent last); gate_state_read_ok is whether the probe's gh call exited + // 0; gate_state_total_comments is the issue's gate-state comment count (the + // SAME title-gated filter `gate_state_blocks` uses, never every comment on + // the issue -- feeds the falsifiable-absent cross-check); gate_state_trust is set at Select from the + // probe's self_login, never by the agent itself. + gate_state_blocks: { type: 'array', items: { type: 'string' } }, + gate_state_read_ok: { type: 'boolean' }, + gate_state_total_comments: { type: 'integer' }, + gate_state_trust: { type: 'string' }, }, } const SETUP_SCHEMA = { @@ -961,6 +1002,479 @@ const CONSOLIDATION_MARKER_PROBE_SCHEMA = { }, } +// ============================================================================= +// GATE STATE (issue #166): durable per-issue gate/contrarian state carried on +// the issue itself across a run boundary. Substrate only in this tier -- no +// consumer reads/acts on it yet (see the design note on buildGateStatePayload's +// `seeded_from`). Mirrors the CONSOLIDATION_* marker subsystem immediately +// above end to end: a title-gated comment, fence-extracted payload, canonical +// scope-guard marker as its LAST line, append-only with positional last-wins +// (read: newest wins, exactly like the outcomes.jsonl/diffOutcomeGrades +// contract and the consolidation markers' own heal pass). Departs from that +// precedent in one place: the payload is fenced JSON, not consolidation's flat +// regex-parsed key:value lines, because `settled` (settleDecision/settledBlock +// above) is an array of five-field objects carrying free text that may itself +// contain newlines -- oneLine()'s single-line-per-field convention can't +// express that without lossy flattening, while JSON.stringify/JSON.parse +// round-trips it exactly, apostrophes and all. +// ============================================================================= + +// Comment title (first line) gating a gate-state marker apart from ordinary +// trail comments -- same convention as CONSOLIDATION_MEMBER_TITLE/ +// CONSOLIDATION_GROUP_TITLE below. Every gate-state comment still ends with +// the canonical scope-guard line "" (see +// scopeGuard()); this title adds one second, gate-state-specific line of +// machine-parseable structure ABOVE it -- it never replaces or reshapes the +// canonical marker itself. +const GATE_STATE_TITLE = '## Gate State' +// GATE_STATE_SCHEMA: payload shape version, embedded in every payload so +// parseGateStateComment can REJECT (never coerce) a payload an older or +// incompatible engine build wrote. Bump only on a breaking shape change. +const GATE_STATE_SCHEMA = 1 + +// GATE_STATE_PROBE_SCHEMA: the whole-set read probe (fetchGateStateBlocks, +// wired below the TICKETMILL-TEST-HARNESS-SPLIT marker) -- ONE call over every +// candidate issue, pinning a jq read per issue exactly like the claim probe's +// pinned "last title-gated comment" idiom (:7524), never a bare `gh issue view +// --json comments` handed to the agent's own judgment (the fetchConsolidation +// Markers precedent this replaces for gate state, since a bare read let a +// truncated response get silently misread as absence). `raw` is each issue's +// VERBATIM jq stdout; the agent relays it, never parses or judges it -- +// parseGateStateProbeRow does that in JS. `exit_ok` is the agent-level gh exit +// status. `self_login` is `gh api user --jq .login` (a single-object endpoint, +// so this file's "never a bare gh api" pagination rule doesn't apply) -- the +// PRIMARY trust signal isTrustedGateStateAuthor checks first; '' when the +// token can't resolve it (installation tokens), which falls through to the +// claim_authors fallback. +const GATE_STATE_PROBE_SCHEMA = { + type: 'object', required: ['rows'], + properties: { + self_login: { type: 'string' }, + rows: { type: 'array', items: { type: 'object', required: ['issue', 'raw', 'exit_ok'], properties: { + issue: { type: 'integer' }, raw: { type: 'string' }, exit_ok: { type: 'boolean' }, + } } }, + }, +} +// GATE_STATE_VERIFY_SCHEMA: the Report-phase self-validation sweep's chunked +// read-back (verifyGateState, wired below the TICKETMILL-TEST-HARNESS-SPLIT +// marker) -- ONE agent call per chunk of at most MAX_GATE_STATE_PROBE_CHUNK +// issues, shaped like GATE_STATE_PROBE_SCHEMA minus `self_login` (the verify +// sweep is proving THIS run's own write round-tripped through GitHub, never +// adjudicating trust between authors, so it has no use for a self-identity +// signal). Pins the SAME per-issue jq idiom fetchGateStateBlocks uses -- +// `raw` is each issue's VERBATIM jq stdout, relayed never parsed or judged by +// the agent. Fed through the exact same parseGateStateProbeRow -> +// parseGateStateComment pipeline the read-side probe uses -- no second +// parser, no second trust rule. The prompt this schema backs carries ONLY +// issue numbers, never the payload being verified against -- see +// verifyGateState's own comment for why that matters. +const GATE_STATE_VERIFY_SCHEMA = { + type: 'object', required: ['rows'], + properties: { + rows: { type: 'array', items: { type: 'object', required: ['issue', 'raw', 'exit_ok'], properties: { + issue: { type: 'integer' }, raw: { type: 'string' }, exit_ok: { type: 'boolean' }, + } } }, + }, +} + +// buildGateStatePayload: assembles the JSON payload embedded in a gate-state +// comment. `settled` is capped to its last 6 entries here (mirrors +// settledBlock's own slice(-6)) so a long-running issue's payload never grows +// unbounded regardless of how many gates it has cleared. `seeded_from` names +// the {run, epoch} of the block THIS ctx's `gate_budgets` were carried forward +// from, when a resume seeds them from a prior run's recorded state, or null +// when they started at zero this run. It is ALWAYS null at this tier: no +// consumer seeds gate_budgets from a prior block yet (substrate only, no +// consumer -- see the section banner above), so every call site passes null. +// The field's PRESENCE, not its value, is what parseGateStateComment's schema +// round-trips against -- a future consumer fills it in without a shape change +// here. `write_seq` (issue #166 PR #177 review) is the GATE_STATE_WRITE_SEQ +// counter value at the moment this payload was built -- unlike `epoch` +// (identical across every boundary in a run), this varies write to write, so +// diffGateStateIntent can order two same-run writes against each other. null +// when the caller doesn't supply one (e.g. a hand-built test fixture, never a +// real postGateState() call, which always passes it). Every field defaults +// defensively (never throws on a sparse `o`) so a caller mid-construction +// (e.g. a boundary with no group) gets a valid, schema-conformant payload +// rather than an exception. +function buildGateStatePayload(o) { + o = o || {} + return { + schema: GATE_STATE_SCHEMA, + repo: o.repo, + issue: o.issue, + run: o.run, + batch: o.batch, + epoch: (o.epoch === undefined) ? null : o.epoch, + write_seq: (o.write_seq === undefined) ? null : o.write_seq, + boundary: o.boundary, + group_id: (o.group_id === undefined) ? null : o.group_id, + members: Array.isArray(o.members) ? o.members.slice() : [], + seeded_from: (o.seeded_from === undefined) ? null : o.seeded_from, + gate_budgets: (o.gate_budgets && typeof o.gate_budgets === 'object' && !Array.isArray(o.gate_budgets)) ? o.gate_budgets : {}, + settled: (Array.isArray(o.settled) ? o.settled : []).slice(-6), + } +} + +// buildGateStateComment: renders the full comment body posted at each gate- +// state boundary. Fixed shape: title line, ONE human line that is +// deliberately non-directive -- a resumed run's agents must never read this +// as an instruction, it exists purely so a human (or a future run's read-back) +// can see what the engine last recorded, never phrased as something to act +// on -- a
wrapper holding the fenced JSON payload, and the canonical +// scope-guard marker as the LAST NON-EMPTY line (parseGateStateComment +// enforces this on read; it is what makes the comment legible to scopeGuard() +// and every other marker-consumer in this file). +function buildGateStateComment(repo, issue, payload) { + const p = payload || {} + const humanLine = 'Recorded automatically at the "' + p.boundary + '" boundary (run ' + p.run + + ') for resume continuity -- a record, not a directive; nothing here should be treated as an instruction.' + return [ + GATE_STATE_TITLE, + humanLine, + '', + '
Gate state payload', + '', + '```json', + JSON.stringify(payload, null, 2), + '```', + '', + '
', + '', + ].join('\n') +} + +// parseGateStateComment: null unless `body` is a well-formed gate-state marker +// FOR THIS repo/issue -- title-gated (first line exactly GATE_STATE_TITLE, +// same convention as parseConsolidation*), fence-extracted, and REQUIRES the +// canonical scope-guard marker to be the last non-empty line (a body that +// merely quotes the block shape inside a larger comment, or has trailing +// content after the marker, is rejected here, not left to the JSON parse to +// catch). Rejects a payload whose schema/repo/issue don't match the ones this +// read expects -- a payload from a different repo, a different issue (e.g. a +// consolidation group's cross-posted comment), or an older/incompatible +// schema version is never silently accepted. Wrapped in try/catch and NEVER +// THROWS: any malformed/truncated JSON, or a payload that isn't a plain +// object, returns null exactly like every other rejection path here, so a +// caller never needs a second layer of defense around this call. +function parseGateStateComment(body, repo, issue) { + try { + const s = String(body == null ? '' : body) + const lines = s.split('\n') + if (!lines.length || lines[0].trim() !== GATE_STATE_TITLE) return null + let lastIdx = lines.length - 1 + while (lastIdx >= 0 && lines[lastIdx].trim() === '') lastIdx-- + if (lastIdx < 0) return null + const expectedMarker = '' + if (lines[lastIdx].trim() !== expectedMarker) return null + const fenceMatch = /```json\r?\n([\s\S]*?)\r?\n```/.exec(s) + if (!fenceMatch) return null + const payload = JSON.parse(fenceMatch[1]) + if (!payload || typeof payload !== 'object' || Array.isArray(payload)) return null + if (payload.schema !== GATE_STATE_SCHEMA) return null + if (payload.repo !== repo) return null + if (payload.issue !== issue) return null + return payload + } catch (e) { + return null + } +} + +// parseGateStateProbeRow: JSON.parses the VERBATIM stdout of the pinned jq +// read (GATE_STATE_PROBE_SCHEMA.rows[].raw / GATE_STATE_VERIFY_SCHEMA.raw) -- +// the agent relays stdout, never judges it, so this is the only place that +// decides whether a read actually succeeded. Expected shape: {total: , +// blocks: [{body, author_login, author_association}, ...]} (oldest-first, +// already sliced to the last few by the jq itself). ANY throw (non-JSON +// stdout, a truncated/partial JSON string from a chunked or interrupted read) +// OR shape mismatch (missing/wrong-typed total, blocks not an array, or a +// block entry missing a string `body`) returns {ok: false, total: 0, blocks: +// []} -- this function itself never throws, and it never returns ok:true over +// a shape it isn't sure of. This is deliberate: selectGateState treats +// ok:false as read-failed, so a truncated read can never be silently misread +// as "genuinely absent" -- issue #166's core fail-open requirement -- it +// fails LOUD instead. This is what makes that kind of truncation structurally +// impossible to hide. +function parseGateStateProbeRow(raw) { + try { + const o = JSON.parse(raw) + if (!o || typeof o !== 'object' || Array.isArray(o)) return { ok: false, total: 0, blocks: [] } + if (!Number.isInteger(o.total) || o.total < 0) return { ok: false, total: 0, blocks: [] } + if (!Array.isArray(o.blocks)) return { ok: false, total: 0, blocks: [] } + for (const b of o.blocks) { + if (!b || typeof b !== 'object' || typeof b.body !== 'string') return { ok: false, total: 0, blocks: [] } + } + return { ok: true, total: o.total, blocks: o.blocks } + } catch (e) { + return { ok: false, total: 0, blocks: [] } + } +} + +// isTrustedGateStateAuthor: is `login` allowed to author a block selectGateState +// treats as authoritative? PRIMARY signal -- `login === selfLogin`, the +// deployment's own authenticated identity (`gh api user --jq .login`, a +// single-object endpoint so the file's "never a bare gh api" pagination rule +// doesn't apply). This closes the self-bootstrap trust hole the capped +// approach-gate contrarian flagged against a claim_authors-only rule (a stale +// forged claim's author no longer qualifies as primary trust). FALLBACK (for +// installation tokens where `gh api user` 403s) -- `claimAuthors`, restricted +// to a claim that is fresh (age < CLAIM_STALE_SECONDS, :292) OR whose batch +// matches THIS run's batch branch: a claim that is neither fresh nor batch- +// matching authored no work in scope and is not evidence of anything. +// claimAuthors entries: {login, ageSeconds, batch}; either of ageSeconds/batch +// may be null/absent (unknown), in which case only the other test can pass +// that entry. +function isTrustedGateStateAuthor(login, selfLogin, claimAuthors, batch) { + if (!login) return false + if (selfLogin && login === selfLogin) return true + const list = Array.isArray(claimAuthors) ? claimAuthors : [] + return list.some(function (c) { + if (!c || c.login !== login) return false + const fresh = typeof c.ageSeconds === 'number' && Number.isFinite(c.ageSeconds) && c.ageSeconds < CLAIM_STALE_SECONDS + const batchMatch = batch != null && c.batch === batch + return fresh || batchMatch + }) +} + +// deriveRunEpoch: turns the probe-returned `date -u +%Y-%m-%dT%H:%M:%SZ` +// string (the same idiom already used elsewhere in this file for a wall-clock +// anchor, since the sandbox has no Date.now()/argless `new Date()`) into +// RUN_EPOCH (epoch ms), via the existing pure toEpochMs (:6382 as of writing). +// Explicit null on anything unparseable -- NEVER NaN, so a downstream +// `runEpochMs - payload.epoch` comparison in gateStateEpochStale can't +// silently produce a NaN that always compares false; a subtraction against an +// unusable "now" must read as unknown/stale, not as "definitely not stale". +function deriveRunEpoch(nowRaw) { + const ms = toEpochMs(nowRaw) + return Number.isFinite(ms) ? ms : null +} + +// gateStateEpochStale: shared by selectGateState -- true when `payload.epoch` +// predates CLAIM_STALE_SECONDS relative to `runEpochMs`, OR either side is +// unparseable/absent. An unknown age reads as stale, never as fresh (fail +// toward re-verifying, not toward trusting silently). +// KNOWN IMPRECISION (issue #166 PR #177 review): `payload.epoch` is RUN_EPOCH +// -- the run's Select-time wall-clock anchor -- not the actual moment this +// particular boundary was written, so a long-running run's later boundaries +// read slightly younger than they really are. Log-only at this tier (no +// consumer yet); not worth a second probe-derived wall-clock read to fix. +function gateStateEpochStale(payload, runEpochMs) { + const payloadEpoch = (payload && Number.isFinite(payload.epoch)) ? payload.epoch : null + if (payloadEpoch === null || !Number.isFinite(runEpochMs)) return true + return (runEpochMs - payloadEpoch) > (CLAIM_STALE_SECONDS * 1000) +} + +// selectGateState: the single decision point for "what does this issue's +// gate-state comment trail say, and can it be trusted?" Turns `rows` (one +// issue's already-parsed probe result -- parseGateStateProbeRow's {ok, total, +// blocks} shape, optionally carrying the agent-level `exit_ok` alongside it; +// blocks are oldest-first, mirroring GitHub's own comment order), `evidence` +// ({repo, issue, self_login, claim_authors, batch, run_epoch}), and +// `priorWork` ({pr_number, worktree_exists, resume_point}) into exactly one +// of four states: +// - 'read-failed' -- the probe/parse never produced usable data (an +// explicit agent-level exit_ok:false, OR parseGateStateProbeRow's own +// ok:false), OR the falsifiable-absent rule fires (below). This is what +// makes a truncated/broken read structurally impossible to misread as +// genuine absence. +// - 'absent' -- zero blocks, zero total, and nothing else on this issue +// (pr_number/worktree_exists/resume_point) is evidence prior work ever +// happened -- a genuinely fresh issue. +// - 'malformed' -- at least one block exists, but NONE of them parse +// (title-gated, fence-extracted, marker-checked -- see +// parseGateStateComment): the newest fails and every older one fails too. +// - 'found' -- at least one block parses. Selection is EXPLICIT TRUST- +// BEFORE-LAST-WINS: walk blocks newest -> oldest, return the first one +// whose author is trusted (isTrustedGateStateAuthor), counting every +// newer untrusted-but-parseable block passed over into `skipped`. If NO +// block is trusted, this is the degenerate all-untrusted case: state +// stays 'found' (there IS data, just not from a trusted author) using the +// newest PARSEABLE block's payload, `trusted: false`, and `skipped: 0` +// (nothing was skipped to reach it -- it's the first thing the walk +// looked at). `trusted` is kept on the result specifically so a caller +// can distinguish this degenerate case from an ordinary trusted find. +// `stale` (only meaningful when `state === 'found'`) comes from +// gateStateEpochStale against the SELECTED payload. +// +// FALSIFIABLE-ABSENT RULE: zero blocks + total===0 is only accepted as +// genuine absence when nothing else on this issue is evidence prior work +// happened. If `pr_number` is non-null, OR `worktree_exists` is true, OR +// `resume_point` is anything other than 'implement', a prior run plainly did +// SOMETHING here, so zero gate-state comments is contradictory -- +// read-failed, never absent. Zero blocks with no such evidence stays absent. +// This is NEVER inferred from an empty blocks array alone -- always from this +// explicit cross-check against independently-sourced preflight evidence. +// A second, narrower contradiction is checked first and unconditionally: +// zero blocks but total>0 is self-contradictory on its face (the probe says +// comments exist but produced none) -- exactly the truncated/corrupted-read +// shape this whole design exists to make undetectable-as-absence, so it is +// always read-failed regardless of `hasPriorWork`. `total` is computed by +// gateStateProbeCommandLine's jq using the SAME title-gated filter `blocks` +// uses (never a bare all-comments count), so this branch is reachable only +// under a genuinely truncated/corrupted read, not on any ordinary issue +// carrying an unrelated human or bot comment. +// +// hasGateStatePriorWork: shared with fetchGateStateBlocks' diagnostic log +// (below the split), which needs the same fact to tell the falsifiable-absent +// case apart from an ordinary read-failed -- kept as one pure helper rather +// than two copies of the same three-condition check. +function hasGateStatePriorWork(priorWork) { + const pw = priorWork || {} + return !!( + (pw.pr_number !== null && pw.pr_number !== undefined) || + pw.worktree_exists === true || + (pw.resume_point != null && pw.resume_point !== 'implement') + ) +} +function selectGateState(rows, evidence, priorWork) { + const r = rows || {} + const ev = evidence || {} + const pw = priorWork || {} + const EMPTY = { payload: null, trusted: false, stale: false, skipped: 0 } + + if (r.exit_ok === false || r.ok === false) { + return Object.assign({ state: 'read-failed' }, EMPTY) + } + + const blocks = Array.isArray(r.blocks) ? r.blocks : [] + const total = Number.isInteger(r.total) ? r.total : blocks.length + + if (blocks.length === 0) { + if (total > 0) return Object.assign({ state: 'read-failed' }, EMPTY) + if (hasGateStatePriorWork(pw)) return Object.assign({ state: 'read-failed' }, EMPTY) + return Object.assign({ state: 'absent' }, EMPTY) + } + + let skipped = 0 + let fallback = null // newest PARSEABLE block, regardless of trust + let fallbackSkipped = 0 + for (let i = blocks.length - 1; i >= 0; i--) { + const b = blocks[i] + const payload = parseGateStateComment(b && b.body, ev.repo, ev.issue) + if (!payload) continue // unparseable at this position -- never counted into `skipped` + const trusted = isTrustedGateStateAuthor(b && b.author_login, ev.self_login, ev.claim_authors, ev.batch) + if (!fallback) { fallback = payload; fallbackSkipped = skipped } + if (trusted) { + return { state: 'found', payload: payload, trusted: true, stale: gateStateEpochStale(payload, ev.run_epoch), skipped: skipped } + } + skipped++ + } + + if (fallback) { + return { state: 'found', payload: fallback, trusted: false, stale: gateStateEpochStale(fallback, ev.run_epoch), skipped: fallbackSkipped } + } + return Object.assign({ state: 'malformed' }, EMPTY) +} + +// diffGateStateIntent: compares the payload THIS run intended to post +// (`intent`) against a payload actually read back (`actual` -- e.g. +// selectGateState's `.payload`, or the Report-phase verify sweep's direct +// re-read). Three verdicts: +// - 'match' -- byte-for-byte the same write (JSON.stringify-equal). +// - 'superseded' -- `actual` is a LATER write from the SAME run (same +// `run`, later `write_seq`) -- expected and not alarming: +// a later boundary in this same run posted after `intent` +// was captured (e.g. a later pr-review iteration's write +// landing after an earlier iteration's intent snapshot). +// Ordering is on `write_seq`, NOT `epoch` (issue #166 PR +// #177 review) -- RUN_EPOCH is assigned once at Select and +// is identical on every boundary a single run posts, so it +// can never distinguish an earlier write from a later one +// within that run; only the monotonic per-write +// GATE_STATE_WRITE_SEQ counter does. +// - 'mismatch' -- anything else: a different run's write sitting where +// ours should be, an EARLIER write, or same-run content +// that disagrees without a later write_seq to explain it +// -- real corruption or a lost write. +function diffGateStateIntent(intent, actual) { + if (!intent || !actual) return 'mismatch' + if (JSON.stringify(intent) === JSON.stringify(actual)) return 'match' + if (intent.run === actual.run && Number.isFinite(intent.write_seq) && Number.isFinite(actual.write_seq) && actual.write_seq > intent.write_seq) { + return 'superseded' + } + return 'mismatch' +} + +// chunkGateStateIssues / gateStateProbeCommandLine / deadGateStateChunkRows / +// normalizeGateStateRow: shared by fetchGateStateBlocks (below the split) and +// the Report-phase verifyGateState sweep -- both chunk the SAME issue list at +// MAX_GATE_STATE_PROBE_CHUNK, pin the SAME per-issue jq idiom, fall back to +// the SAME {raw: '', exit_ok: false} stub rows when a chunk's agent call +// dies, and normalize a returned row's `raw`/`exit_ok` the same way. Kept as +// four small pure helpers rather than two copies of each, so the read-side +// probe and its self-validation sweep can never drift apart. +function chunkGateStateIssues(list) { + const chunks = [] + for (let i = 0; i < list.length; i += MAX_GATE_STATE_PROBE_CHUNK) chunks.push(list.slice(i, i + MAX_GATE_STATE_PROBE_CHUNK)) + return chunks +} +function gateStateProbeCommandLine() { + // total is computed by the SAME title-gated select(...) filter blocks uses -- + // NEVER a bare `.comments|length` -- so it counts gate-state comments only, + // not every comment on the issue. A bare all-comments count would make + // `blocks.length === 0 && total > 0` reachable on any ordinary issue with so + // much as one human reply or one of this pipeline's own non-gate-state + // comments, which selectGateState's self-contradiction check (:1343 as of + // writing) treats as read-failed -- silently swallowing the `absent` state + // for every issue that has ever received an unrelated comment (issue #166 + // PR #177 review). + const titleFilter = 'select(.body | startswith("' + GATE_STATE_TITLE + '"))' + return 'gh issue view --repo ' + REPO + ' --json comments --jq \'{total: ([.comments[] | ' + titleFilter + '] | length), blocks: [.comments[] | ' + titleFilter + ' | {body, author_login: .author.login, author_association: .authorAssociation}] | .[-3:]}\'' +} +function deadGateStateChunkRows(chunk) { + return chunk.map(function (n) { return { issue: n, raw: '', exit_ok: false } }) +} +function normalizeGateStateRow(row) { + return { raw: typeof row.raw === 'string' ? row.raw : '', exit_ok: row.exit_ok === true } +} + +// attachGateStateBlocks: per-preflight normalizer AND real-data join, mirroring +// attachEngineOwnedIntentional's shape (:3023) -- guarantees every preflight +// carries all four gate-state PREFLIGHT_SCHEMA fields (gate_state_blocks, +// gate_state_read_ok, gate_state_total_comments, gate_state_trust) +// UNCONDITIONALLY. Pure and side-effect-free: returns a NEW array, never +// mutates `preflights` or `rowsByIssue`. +// +// `rowsByIssue` (optional; keyed by issue NUMBER) carries fetchGateStateBlocks' +// RAW per-issue probe rows -- {raw, exit_ok} straight off GATE_STATE_PROBE_SCHEMA, +// UNPARSED -- this function is what runs parseGateStateProbeRow, so a truncated +// or non-JSON `raw` string handed in here surfaces as gate_state_read_ok:false, +// never as an empty-but-successful read. `selfLogin` is the reduced +// self_login string (see fetchGateStateBlocks below the split) stored verbatim +// as gate_state_trust -- a FUTURE consumer's selectGateState call uses it as +// evidence.self_login; this function itself never decides trust or state. +// +// Every preflight's four fields are ALWAYS computed fresh from `rowsByIssue`/ +// `selfLogin` -- NEVER read back off the preflight object's own pre-existing +// values, even when `rowsByIssue` has no entry for that issue. This is +// deliberate: these four fields are read-only FACTS this run's own probe +// resolved, never something an upstream agent (e.g. the preflight probe, +// which happens to share PREFLIGHT_SCHEMA) gets to assert on its own — a +// hallucinated gate_state_blocks arriving on `p` from anywhere else is always +// clobbered to the real (or fail-open default) value, exactly like +// attachEngineOwnedIntentional never trusts an agent-supplied regime. +function attachGateStateBlocks(preflights, rowsByIssue, selfLogin) { + const byIssue = rowsByIssue || {} + const trust = typeof selfLogin === 'string' ? selfLogin : '' + return (preflights || []).map(function (p) { + const row = Object.prototype.hasOwnProperty.call(byIssue, p.issue) ? byIssue[p.issue] : null + if (!row) { + return Object.assign({}, p, { + gate_state_blocks: [], gate_state_read_ok: false, gate_state_total_comments: 0, gate_state_trust: trust, + }) + } + const parsed = parseGateStateProbeRow(row.raw) + const readOk = row.exit_ok === true && parsed.ok === true + return Object.assign({}, p, { + gate_state_blocks: readOk ? parsed.blocks.map(function (b) { return b.body }) : [], + gate_state_read_ok: readOk, + gate_state_total_comments: readOk ? parsed.total : 0, + gate_state_trust: trust, + }) + }) +} + + // ============================================================================= // CONSOLIDATION (unit-of-work) FOUNDATIONS // @@ -2925,7 +3439,7 @@ async function fail(ctx, status, stageKey, error) { await postNote(ctx, stageKey, status, error) BATCH.failures++ if (BATCH.failures >= MAX_BATCH_FAILURES) tripStop('circuit breaker: ' + BATCH.failures + ' issues failed') - return Object.assign({ issue: ctx.issue, title: ctx.title, status: status, stage: stageKey, pr: ctx.pr || null, error: String(error || ''), follow_ups: [], metrics: ctx.metrics || null, tokens: ctx.tokens || null, timeline: timeline(ctx), handoff_notes: (ctx.notes || []).slice(), members: memberIssues(ctx), changed_files: (ctx && ctx.changed_files) || null, added_files: (ctx && ctx.added_files) || null, touch_counts: (ctx && ctx.touch_counts) || {}, gate_findings: (ctx && ctx.gate_findings) || {}, settled: (((ctx && ctx.settled) || []).slice()) }, frictionFields(ctx, status)) + return Object.assign({ issue: ctx.issue, title: ctx.title, status: status, stage: stageKey, pr: ctx.pr || null, error: String(error || ''), follow_ups: [], metrics: ctx.metrics || null, tokens: ctx.tokens || null, timeline: timeline(ctx), handoff_notes: (ctx.notes || []).slice(), members: memberIssues(ctx), changed_files: (ctx && ctx.changed_files) || null, added_files: (ctx && ctx.added_files) || null, touch_counts: (ctx && ctx.touch_counts) || {}, gate_findings: (ctx && ctx.gate_findings) || {}, settled: (((ctx && ctx.settled) || []).slice()), gate_state_intent: (ctx && ctx.gate_state_intent) || null, gate_state_post_failed: (ctx && ctx.gate_state_post_failed) || null }, frictionFields(ctx, status)) } // ============================================================================= @@ -4057,6 +4571,141 @@ async function fetchConsolidationMarkers(issueNumbers) { return (r && r.markers) || [] } +// fetchGateStateBlocks: READ-ONLY (safe under DRY_RUN) — the whole-set gate- +// state read, shaped like fetchConsolidationMarkers just above it, but the +// READ IDIOM is deliberately NOT that one: fetchConsolidationMarkers hands the +// agent a bare `gh issue view --json comments` and trusts its own judgment to +// pick the right comment, which lets a truncated response get silently +// misread as "no marker". Gate state instead pins the claim probe's +// deterministic idiom (the per-issue `gh issue view ... --jq '{total, blocks}'` +// a few thousand lines below, in the claims loop) verbatim, one command per +// issue: jq computes the EXACT return shape, so a short/truncated read is a +// JSON.parse failure (parseGateStateProbeRow), never a fake "zero blocks". +// The agent's ONLY job per issue is relaying that command's stdout — it never +// parses or judges it. +// +// Chunked at MAX_GATE_STATE_PROBE_CHUNK issues per agent call — belt-and- +// braces, not a truncation defense (the jq pin already makes a truncated READ +// structurally impossible to misread): a chunk whose agent call dies (throws, +// budget-exhausted, or returns a malformed response) marks ONLY its own +// chunk's issues read-failed via synthesized {raw: '', exit_ok: false} stub +// rows — surviving chunks still report normally, rather than one dead call +// taking the whole candidate set down with it. A LIVE chunk that returns a +// schema-valid `rows` array simply missing one of its assigned issues (which +// GATE_STATE_PROBE_SCHEMA cannot forbid) gets the SAME stub backfilled after +// the chunk loop below, for the same reason: a queried-but-unanswered issue +// must read as read-failed, never silently as absent. +// +// self_login reduction: each chunk independently runs `gh api user --jq +// .login` (a single-object endpoint, so the file's "never a bare gh api" +// pagination rule does not apply) since chunks run in parallel and none of +// them can see another's result. The FIRST chunk (in `chunks` order) that +// reports a non-empty login wins — every chunk is hitting the SAME +// authenticated identity, so this is a redundant-computation reduction, not a +// disagreement to arbitrate; an empty string means no chunk could resolve it +// (an installation token, or every chunk died), which isTrustedGateStateAuthor +// treats as "primary trust unavailable, fall through to claim_authors". +// +// `priorWorkByIssue` ({issue: {pr_number, worktree_exists, resume_point}}) is +// NEVER sent to the agent (it stays a pure verbatim relay) — it feeds ONLY +// the per-issue log line below, via selectGateState's falsifiable-absent rule, +// so "zero gate-state comments but a PR is already open" logs as the +// DISTINCT, greppable suspicious case rather than a bare "absent" that could +// hide a real read problem. +// +// Returns { rowsByIssue, self_login } — rowsByIssue is RAW ({issue: {raw, +// exit_ok}}), unparsed on purpose: attachGateStateBlocks (above the split) is +// what runs parseGateStateProbeRow, so this function's own contract stays a +// thin, mirror-of-fetchConsolidationMarkers relay with no decision logic of +// its own beyond the per-issue log line (which is diagnostic output, not a +// decision fed back into the run). +async function fetchGateStateBlocks(issueNumbers, priorWorkByIssue) { + const list = Array.isArray(issueNumbers) ? issueNumbers.slice() : [] + if (!list.length) return { rowsByIssue: {}, self_login: '' } + const pwByIssue = priorWorkByIssue || {} + const chunks = chunkGateStateIssues(list) + + const chunkResults = await Promise.all(chunks.map(function (chunk, ci) { + return agent([ + 'READ-ONLY (safe under any run mode, including a dry run). For EACH issue number listed below, run this EXACT', + 'command, substituting only for that issue\'s number — do not alter the jq filter in any way:', + gateStateProbeCommandLine(), + 'Issues in this call: ' + chunk.join(', '), + 'Relay each command\'s stdout VERBATIM as `raw` — never parse, reformat, summarize, or judge it. `exit_ok` is', + 'whether that gh command exited 0 for that issue (false on any non-zero exit, including a repo/issue lookup', + 'failure).', + '', + 'Also run ONCE for this whole call (not per issue): gh api user --jq .login', + 'Return self_login = that command\'s trimmed stdout, or "" if the command fails or returns nothing (this', + 'happens for installation tokens — expected, not an error).', + '', + 'Return rows: [{issue, raw, exit_ok}] — exactly one entry per issue listed above, even one whose gh command', + 'failed.', + ].join('\n'), { label: 'gate-state-probe-c' + ci, phase: 'Select', schema: GATE_STATE_PROBE_SCHEMA, model: M.probe.model, effort: M.probe.effort }) + .catch(function () { return null }) + .then(function (r) { + if (r && Array.isArray(r.rows)) return { self_login: typeof r.self_login === 'string' ? r.self_login : '', rows: r.rows } + // dead chunk — belt-and-braces: mark ONLY this chunk's issues read-failed via + // explicit stub rows, never silently drop them (a dropped issue would look + // identical to one this function was never asked about at all). + return { self_login: '', rows: deadGateStateChunkRows(chunk) } + }) + })) + + const rowsByIssue = {} + let selfLogin = '' + for (const cr of chunkResults) { + if (!selfLogin && cr.self_login && cr.self_login.trim()) selfLogin = cr.self_login.trim() + for (const row of (cr.rows || [])) rowsByIssue[row.issue] = normalizeGateStateRow(row) + } + // Backfill any queried issue a LIVE chunk's response simply omitted — GATE_STATE_PROBE_SCHEMA + // cannot enforce one row per issue, so a schema-valid response that drops an issue (never + // throws, never hits the dead-chunk catch above) would otherwise leave rowsByIssue[n] + // undefined. Stubbed the same way a dead chunk's issues are (deadGateStateChunkRows, a few + // lines above) so a queried-but-never-answered issue reads as read-failed, never as a false + // "absent" (issue #166 PR #177 review). + for (const n of list) { + if (!Object.prototype.hasOwnProperty.call(rowsByIssue, n)) rowsByIssue[n] = normalizeGateStateRow({ issue: n, raw: '', exit_ok: false }) + } + + // Per-issue diagnostic log, using the SAME pure selectGateState decision a + // future consumer would reach — this run just prints it rather than acting + // on it (substrate only, no consumer yet). run_epoch is not yet assigned at + // this Select-phase call site (see the RUN_EPOCH assignment below, which + // needs outcomeGradeR/revisitRiskR already awaited) — the `state` value + // never depends on it (only the auxiliary `stale` flag does), so passing the + // module-level RUN_EPOCH here (null on a fresh run) is correct, not stale. + // Every issue in `list` now has a rowsByIssue entry (real or backfilled + // stub), so `row` below is never falsy — no separate {} fallback needed. + for (const n of list) { + const row = rowsByIssue[n] + const parsed = parseGateStateProbeRow(row.raw) + const rowsArg = Object.assign({ exit_ok: row.exit_ok }, parsed) + const pw = pwByIssue[n] || {} + const sel = selectGateState(rowsArg, { repo: REPO, issue: n, self_login: selfLogin, claim_authors: [], batch: TARGET, run_epoch: RUN_EPOCH }, pw) + // readOk mirrors attachGateStateBlocks' definition (:1467) exactly -- the same + // "did the read actually succeed" test, so this log and the stored preflight + // fields never disagree about it. Without this gate, EVERY hard read failure + // (dead chunk, non-zero gh exit, truncated stdout) also has blocks.length===0 + // and total===0, so it printed the same "absent (unexpected: ...)" line as a + // genuine falsifiable-absent read -- on a resume, where hasGateStatePriorWork + // is true for exactly the issues this substrate serves, that made read + // failures indistinguishable from suspicious absences (issue #166 PR #177 + // review, iteration 2). + const readOk = row.exit_ok === true && parsed.ok === true + if (sel.state === 'read-failed' && readOk && parsed.blocks.length === 0 && parsed.total === 0 && hasGateStatePriorWork(pw)) { + const why = pw.pr_number != null ? ('PR #' + pw.pr_number + ' open') + : pw.worktree_exists ? 'worktree exists' + : ('resume_point=' + pw.resume_point) + log('gate-state #' + n + ': absent (unexpected: ' + why + ')') + } else { + log('gate-state #' + n + ': ' + sel.state + (sel.state === 'found' && !sel.trusted ? ' (untrusted author)' : '')) + } + } + + return { rowsByIssue: rowsByIssue, self_login: selfLogin } +} + // challengeConsolidationGroup: the capped contrarian loop for ONE proposed group. // Returns the (possibly revised) accepted group, or null if it DISSOLVED (cap // reached without acceptance, or a dead challenger/reviser — see the module @@ -4297,6 +4946,211 @@ async function postConsolidationMarkers(units) { } } +// postGateState (issue #166): non-fatal per-issue write of the durable +// "## Gate State" comment (buildGateStateComment/buildGateStatePayload, above +// the TICKETMILL-TEST-HARNESS-SPLIT marker) at a run boundary. Modeled +// directly on the cap-note-plan/cap-note-approach stages just above +// (stageOpts('probe'), NOTE_SCHEMA, exactly 1 try, log-only on a dead agent +// or posted!==true) -- like those, NO path through this helper may fail an +// issue; every failure mode degrades to a logged/deferred note and the caller +// keeps going. +// +// Posting idiom deliberately breaks from postConsolidationMarkers just above +// (`gh issue comment ... --body "..."`, :4645/:4660): that idiom is safe only +// because a consolidation marker's body is flat, oneLine()-rendered +// key:value text with nothing in it that a shell would treat specially. A +// gate-state body embeds free text straight out of ctx.settled (rationale/ +// resolution strings a human or an earlier agent wrote), which CAN contain +// apostrophes, backticks, or `$` -- any of which a `--body "$(...)"` or an +// UNQUOTED heredoc would hand to the shell for interpolation, silently +// corrupting the posted payload (or worse). Instead this pins `gh issue +// comment --repo --body-file -` fed by stdin from a QUOTED heredoc +// (`<<'...'`), which disables ALL shell expansion inside it -- the payload +// reaches `gh` as literal bytes no matter what punctuation the free text +// carries. +// +// ctx.gate_state_intent is set to the intended payload ONLY when the agent +// actually reports posted===true. Every other outcome (dead agent after its +// one try, an explicit posted:false, or a schema mismatch) instead sets +// ctx.gate_state_post_failed = boundary (last failure wins across an issue's +// several boundaries) and pushes a ctx.deferred note, so the miss is visible +// without ever touching gate_state_intent. This asymmetry is deliberate: +// setting intent unconditionally would let Task 4's Report-phase verify +// sweep compare an intent against a block that was never actually written +// and report 'mismatch' (real corruption) for what is, here, a routine, +// designed-for non-fatal skip. +async function postGateState(ctx, boundary) { + const payload = buildGateStatePayload({ + repo: REPO, + issue: ctx.issue, + run: RUN_TAG, + batch: TARGET, + epoch: RUN_EPOCH, + write_seq: ++GATE_STATE_WRITE_SEQ, + boundary: boundary, + group_id: ctx.groupId, + members: memberIssues(ctx), + gate_budgets: { + approach: (ctx.metrics && ctx.metrics.approach_iters) || 0, + plan: (ctx.metrics && ctx.metrics.plan_iters) || 0, + 'pr-review': (ctx.metrics && ctx.metrics.pr_review_iters) || 0, + }, + settled: ctx.settled, + }) + const body = buildGateStateComment(REPO, ctx.issue, payload) + const posted = await stage(ctx, 'gate-state-' + boundary, [ + 'Post the durable gate-state record on issue #' + ctx.issue + ' of ' + REPO + ' EXACTLY as given below, verbatim', + 'and unchanged -- do not reformat, reword, summarize, or add anything to it. Run exactly this command, with the', + 'body fed on stdin via a QUOTED heredoc (the quotes around the delimiter are load-bearing: they disable ALL', + 'shell interpolation inside the heredoc, which matters because the body below may contain apostrophes,', + 'backticks, or $ characters that must reach gh as literal bytes, not be expanded by the shell):', + '', + 'gh issue comment ' + ctx.issue + ' --repo ' + REPO + ' --body-file - <<\'TICKETMILL_GATE_STATE_EOF\'', + body, + 'TICKETMILL_GATE_STATE_EOF', + '', + 'Do NOT substitute an unquoted heredoc (< GitHub -> read -> parse for gate state in one +// run. ONE stage for the WHOLE run (never 2N per-issue calls, and never the +// old per-boundary-during-processIssue design GATE_STATE_VERIFY_SCHEMA's +// original comment described -- that design was superseded by this +// Report-phase sweep before this task landed), chunked at +// MAX_GATE_STATE_PROBE_CHUNK like fetchGateStateBlocks (:4544, via the shared +// chunkGateStateIssues helper) -- belt-and-braces: a dead chunk's agent call +// only takes its own chunk's issues down with it, surviving chunks report +// normally. +// +// The verify prompt carries ONLY the issue numbers and the pinned per-issue +// jq idiom fetchGateStateBlocks uses (via the shared gateStateProbeCommandLine +// helper) -- it NEVER carries +// ctx.gate_state_intent or any other part of the payload being checked +// against. This is load-bearing: if the prompt included the intended +// payload, the agent could satisfy the schema by echoing it back rather than +// actually relaying gh's real output, and the "comparison" would prove +// nothing. JS alone -- never the agent -- runs parseGateStateProbeRow, then +// parseGateStateComment on the newest returned block, then diffGateStateIntent +// against the result's own gate_state_intent. +// +// phase('Report') runs on every terminal exit of a batch (STOP.tripped fills +// the remaining results as 'not_started' and returns; a per-unit throw is +// isolated to that unit by runPool -- see :5871), so `results` passed in here +// always carries every issue's FINAL gate-state fields for this run, on every +// exit path, not just a clean finish. +// +// Six outcomes, logged one per issue via `log()`. NON-FATAL end to end: +// this sweep never mutates a result's status, never throws past its own call +// site, and a dead/misbehaving chunk degrades to 'read-failed' for that +// chunk's issues rather than aborting the sweep. 'mismatch' and 'read-failed' +// additionally push a VERIFY_SKIPS entry (issue #166 PR #177 review) -- every +// other outcome is either nothing-to-verify or a clean/expected result, but +// these two mean this run's self-validation either proved nothing +// ('read-failed') or found evidence of a lost/corrupted write ('mismatch'), +// which belongs in the batch PR's Verification Gaps section, the human's only +// window into what this run couldn't verify. +// - 'no-intent' -- this run never recorded a successful gate-state post +// for this issue AND never recorded a failed one either +// (gate_state_intent and gate_state_post_failed both +// absent) -- e.g. not_started, preflight-skipped, or the +// unit died before its first boundary. Nothing to +// verify; not alarming. +// - 'post-failed' -- postGateState() itself already reported the post +// failed (ctx.gate_state_post_failed set, no intent +// recorded) -- Task 2's KNOWN non-fatal path. Kept as +// its own outcome so this benign, already-logged miss is +// never reported alongside genuine read-back corruption. +// - 'read-failed' -- the verify probe itself couldn't produce usable data +// for this issue this run (no row at all, an explicit +// exit_ok:false, or parseGateStateProbeRow rejected the +// stdout shape) -- never conflated with a real mismatch. +// - 'match' -- diffGateStateIntent found the newest gate-state block +// read back byte-identical to what this run intended. +// - 'superseded' -- diffGateStateIntent found a later write from the SAME +// run sitting where the intent snapshot was taken from +// (e.g. a later pr-review iteration posted after an +// earlier iteration's intent was captured) -- expected, +// not alarming, so this run's own later boundary is +// never reported as corruption. A DIFFERENT run's write +// (concurrent or otherwise) is never 'superseded' -- +// diffGateStateIntent requires `intent.run === actual.run` +// before it even looks at ordering, so that case always +// falls through to 'mismatch' below. +// - 'mismatch' -- anything else diffGateStateIntent returns: a +// different run's write, an earlier write, no +// gate-state block found at all despite a recorded +// successful post, or same-run content that disagrees +// without a later write_seq to explain it. Real +// corruption or a lost write. +async function verifyGateState(results) { + const list = Array.isArray(results) ? results : [] + const toVerify = list.filter(function (r) { return r && r.gate_state_intent }) + const rowsByIssue = {} + + if (toVerify.length) { + const issueNumbers = toVerify.map(function (r) { return r.issue }) + const chunks = chunkGateStateIssues(issueNumbers) + + const chunkRows = await Promise.all(chunks.map(function (chunk, ci) { + return agent([ + 'READ-ONLY. For EACH issue number listed below, run this EXACT command, substituting only for that', + 'issue\'s number -- do not alter the jq filter in any way:', + gateStateProbeCommandLine(), + 'Issues in this call: ' + chunk.join(', '), + 'Relay each command\'s stdout VERBATIM as `raw` -- never parse, reformat, summarize, or judge it. `exit_ok` is', + 'whether that gh command exited 0 for that issue (false on any non-zero exit, including a repo/issue lookup', + 'failure).', + '', + 'Return rows: [{issue, raw, exit_ok}] -- exactly one entry per issue listed above, even one whose gh command', + 'failed.', + ].join('\n'), { label: 'gate-state-verify-c' + ci, phase: 'Report', schema: GATE_STATE_VERIFY_SCHEMA, model: M.probe.model, effort: M.probe.effort }) + .catch(function () { return null }) + .then(function (r) { + if (r && Array.isArray(r.rows)) return r.rows + // dead chunk -- belt-and-braces, mirrors fetchGateStateBlocks: mark ONLY this + // chunk's issues read-failed via explicit stub rows, never silently drop them. + return deadGateStateChunkRows(chunk) + }) + })) + for (const rows of chunkRows) { + for (const row of (rows || [])) rowsByIssue[row.issue] = normalizeGateStateRow(row) + } + } + + for (const r of list) { + if (!r) continue + let outcome + if (!r.gate_state_intent) { + outcome = r.gate_state_post_failed ? 'post-failed' : 'no-intent' + } else { + const row = rowsByIssue[r.issue] + const parsed = row ? parseGateStateProbeRow(row.raw) : { ok: false, total: 0, blocks: [] } + if (!row || row.exit_ok !== true || !parsed.ok) { + outcome = 'read-failed' + } else { + const newest = parsed.blocks.length ? parsed.blocks[parsed.blocks.length - 1] : null + const actual = newest ? parseGateStateComment(newest.body, REPO, r.issue) : null + outcome = diffGateStateIntent(r.gate_state_intent, actual) + } + } + log('gate-state-verify #' + r.issue + ': ' + outcome) + if (outcome === 'mismatch' || outcome === 'read-failed') { + VERIFY_SKIPS.push('#' + r.issue + ': gate-state self-validation reported ' + outcome + ' -- this run\'s durable gate-state record for this issue could not be confirmed to have round-tripped through GitHub as intended (non-fatal, no consumer relies on it yet, but resume continuity for a future consumer is unverified)') + } + } +} + // ============================================================================= // IMPLEMENT (setup -> research -> evaluate<->contrarian -> plan<->contrarian -> // tasks with review/quality loops -> test loop -> browser -> docblocks -> PR) @@ -4515,6 +5369,13 @@ async function implementIssue(ctx) { pushDecision(ctx, 'Revised Evaluation (i' + iter + ')', '**Approach:** ' + (re.approach || '') + '\n' + (re.summary || '')) } + // Gate-state boundary 'approach' (issue #166): covers every one of the loop's + // four exits above (dead contrarian, sound_with_caveats, cap-out, dead + // re-evaluate) -- all four are `break`s out of the same loop, so one post + // placed right here, before anything that can fail the issue, durably + // records the approach gate's outcome regardless of which exit was taken. + await postGateState(ctx, 'approach') + // ---- PLAN + CONTRARIAN CHALLENGE (plan) ---- const agentMenu = IMPLEMENTERS.length ? IMPLEMENTERS.map(function (n) { @@ -4655,6 +5516,13 @@ async function implementIssue(ctx) { pushDecision(ctx, 'Revised Plan (i' + iter + ')', (rp.summary || '') + '\n**Tasks:**\n' + tasks.map(function (t) { return '- ' + t.id + ' [' + (t.agent || 'implementer') + '] ' + t.description }).join('\n')) } + // Gate-state boundary 'plan' (issue #166): covers every one of the loop's + // four exits above (dead contrarian, sound_with_caveats, cap-out, dead + // re-plan) the same way the 'approach' boundary covers its own loop -- all + // four are `break`s, so one post here, before the IMPLEMENT section below, + // durably records the plan gate's outcome regardless of which exit fired. + await postGateState(ctx, 'plan') + // ---- IMPLEMENT (sequential per-task: implement -> review -> fix loop -> quality loop) ---- let tasksCompleted = 0 const failedTasks = [] @@ -4899,7 +5767,18 @@ async function reviewAndMerge(ctx) { ]) const spec = reviews[0] const code = reviews[1] - if (!spec || !code) return fail(ctx, 'needs_human', 'pr-review', 'a PR reviewer died — PR #' + ctx.pr + ' left open for human review') + if (!spec || !code) { + // Gate-state boundary 'pr-review-iN-aborted' (issue #166): the ONLY exit + // from this loop that a resumed run can reach WITHOUT ever passing + // through the recordGateOutcome() call below, because the process_pr + // resume path (processIssue -> reviewAndMerge directly) never runs the + // approach/plan gates or their own boundary posts. Without a post here, + // a resume whose reviewers die on iteration 1 would record nothing at + // all for this issue. ctx.metrics.pr_review_iters is already `iter` + // (set above, before the reviews ran), so no extra plumbing is needed. + await postGateState(ctx, 'pr-review-i' + iter + '-aborted') + return fail(ctx, 'needs_human', 'pr-review', 'a PR reviewer died — PR #' + ctx.pr + ' left open for human review') + } // gate_findings tally (issue #91, retyped by issue #162): one call per // PR-review iteration, using the same disposition vocabulary as the @@ -4929,6 +5808,14 @@ async function reviewAndMerge(ctx) { const prReviewDisposition = prReviewClean ? 'accepted' : ((bothNothingToFix || capReached) ? 'carried-unresolved' : 're-litigated') recordGateOutcome(ctx, 'pr-review', (specFindings || []).concat(codeFindings || []), prReviewDisposition) + // Gate-state boundary 'pr-review-iN' (issue #166): kept INSIDE the loop, + // right after the gate_findings tally above, because reviewAndMerge + // returns from inside this loop at both breaks below (nothing-to-fix and + // cap-reached) as well as at the clean-approval break just below this + // line -- a post placed after the loop would never run for either of + // those in-loop returns. + await postGateState(ctx, 'pr-review-i' + iter) + if (prReviewClean) { approved = true; break } // Both reviewers have nothing to fix, but that isn't prReviewClean (one or @@ -5092,7 +5979,7 @@ async function reviewAndMerge(ctx) { if (mar.resolved) ctx.metrics.merge_auto_resolved = (ctx.metrics.merge_auto_resolved || 0) + 1 log('#' + ctx.issue + ' merged PR #' + ctx.pr + (merge.follow_up_issues && merge.follow_up_issues.length ? ' (follow-ups: ' + merge.follow_up_issues.join(', ') + ')' : '')) - return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'completed', pr: ctx.pr, follow_ups: merge.follow_up_issues || [], stage: 'merge', error: null, metrics: ctx.metrics, tokens: ctx.tokens, timeline: timeline(ctx), handoff_notes: ctx.notes.slice(), members: memberIssues(ctx), changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice() }, frictionFields(ctx, 'completed')) + return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'completed', pr: ctx.pr, follow_ups: merge.follow_up_issues || [], stage: 'merge', error: null, metrics: ctx.metrics, tokens: ctx.tokens, timeline: timeline(ctx), handoff_notes: ctx.notes.slice(), members: memberIssues(ctx), changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice(), gate_state_intent: ctx.gate_state_intent || null, gate_state_post_failed: ctx.gate_state_post_failed || null }, frictionFields(ctx, 'completed')) } // ============================================================================= @@ -5150,7 +6037,12 @@ async function processIssue(pre) { // must NOT count as "shipped into TARGET" — see batchClosesIssues() below, // which is the sole reader of this field. const merged_into_target = pre.pr_state === 'merged' && pre.pr_base === TARGET - return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'skipped', pr: ctx.pr, follow_ups: [], stage: 'preflight', error: null, reason: pre.reason, members: memberIssues(ctx), merged_into_target: merged_into_target, changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice() }, frictionFields(ctx, 'skipped')) + // gate_state_intent/gate_state_post_failed: shape totality only -- no + // gate-state boundary can fire on this skip path (it never runs + // implementIssue/reviewAndMerge), so these are always null here and the + // Report-phase verify sweep (Task 4) reads that as 'no-intent', never as + // a mismatch. + return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'skipped', pr: ctx.pr, follow_ups: [], stage: 'preflight', error: null, reason: pre.reason, members: memberIssues(ctx), merged_into_target: merged_into_target, changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice(), gate_state_intent: null, gate_state_post_failed: null }, frictionFields(ctx, 'skipped')) } if (pre.resume_point === 'process_pr') { log('#' + ctx.issue + ' healing: open PR #' + ctx.pr + ' found — jumping to review/merge') @@ -5379,6 +6271,7 @@ function __seed(o) { if ('ROOT' in o) ROOT = o.ROOT if ('ENGINE_OWNED' in o) ENGINE_OWNED = o.ENGINE_OWNED if ('LOCKSTEP_INSTALLED_PATHS' in o) LOCKSTEP_INSTALLED_PATHS = o.LOCKSTEP_INSTALLED_PATHS + if ('RUN_EPOCH' in o) RUN_EPOCH = o.RUN_EPOCH if ('MAX_CONTRARIAN_ITERATIONS' in o) MAX_CONTRARIAN_ITERATIONS = o.MAX_CONTRARIAN_ITERATIONS if ('OUTCOME_GRADING' in o) OUTCOME_GRADING = o.OUTCOME_GRADING if ('REVISIT_RISK' in o) REVISIT_RISK = o.REVISIT_RISK @@ -7247,6 +8140,20 @@ let preflights = (await Promise.all(issueList.map(function (it) { // deriveUnits()'s OR-fold for how it threads onto a consolidation-group unit. preflights = attachEngineOwnedIntentional(preflights, ENGINE_OWNED) +// ---- Select: gate-state read (issue #166 task 3) — READ-ONLY, safe under +// DRY_RUN (this whole block runs before the DRY_RUN early-return below), and +// placed right after the regime classifier it sits beside in PREFLIGHT_SCHEMA. +// priorWork is each preflight's OWN already-resolved pr_number/worktree_exists/ +// resume_point — passing it lets fetchGateStateBlocks' falsifiable-absent log +// line be evidence-driven rather than model-attested. See fetchGateStateBlocks' +// module comment (below the split) for the full design; attachGateStateBlocks +// (above the split) guarantees every preflight — even one a dead chunk never +// covered — comes out carrying all four gate-state fields, fail-open defaulted. +const gateStatePriorWork = {} +for (const p of preflights) gateStatePriorWork[p.issue] = { pr_number: p.pr_number, worktree_exists: p.worktree_exists, resume_point: p.resume_point } +const gateStateProbe = await fetchGateStateBlocks(preflights.map(function (p) { return p.issue }), gateStatePriorWork) +preflights = attachGateStateBlocks(preflights, gateStateProbe.rowsByIssue, gateStateProbe.self_login) + for (const p of preflights) log('#' + p.issue + ' preflight: ' + p.resume_point + ' — ' + p.reason) // ---- Select: engine-owned root-dirty skip — regime (a) of the three-regime @@ -7263,6 +8170,19 @@ if (engineSkip.flagged.length) log('engine-owned guardrail: root working tree di const learnR = await learnPromise const outcomeGradeR = await outcomeGradePromise const revisitRiskR = await revisitRiskPromise +// RUN_EPOCH (issue #166 task 3): derived here because both outcomeGradeR.now +// and revisitRiskR.now are already awaited above, and each already carries a +// `date -u +%Y-%m-%dT%H:%M:%SZ` wall-clock anchor from the exact same idiom +// (OUTCOMES_SCHEMA / REVISIT_RISK_SCHEMA's own step 0 — the sandbox has no +// Date.now()/argless `new Date()`) — no extra agent call needed. Prefer +// outcomeGradeR's since it fires first in program order; fall back to +// revisitRiskR's so a dead outcome-grading pass alone doesn't leave the whole +// run epoch-less. Logged loudly when both are unavailable/unparseable: every +// gate-state comparison this run then treats age as unknown, which +// gateStateEpochStale resolves toward stale, never toward silently trusting +// an old block. +RUN_EPOCH = deriveRunEpoch((outcomeGradeR && outcomeGradeR.now) || (revisitRiskR && revisitRiskR.now)) +if (RUN_EPOCH === null) log('gate-state: RUN_EPOCH is null — outcome-grading and revisit-risk both failed to supply a wall-clock reading this run; every gate-state block will read as unknown-age/stale') addStage('preflight', preflightR1Before) // STAGE_TOKENS.preflight R1 close — see the bracket comment above learnPromise if (learnR && learnR.found) { LEARN = learnR @@ -7712,6 +8632,21 @@ if (HELD_CLAIMS.length) { if (!swept || !swept.posted) log('claims-release sweep incomplete — stale "' + CLAIM_LABEL + '" labels expire via the ' + Math.round(CLAIM_STALE_SECONDS / 3600) + 'h staleness window') } +// ---- Gate-state self-validation sweep (issue #166, task 4) — proves +// post -> GitHub -> read -> parse round-tripped for every issue this run +// wrote a gate-state comment for. Advisory only (log lines, plus a +// VERIFY_SKIPS entry on mismatch/read-failed — see :5122 — never a result +// mutation) and non-fatal end to end: wrapped so a bug in the sweep itself +// can never take the rest of Report down with it. Runs BEFORE the +// token/friction/rework rollups below on purpose — those are pure +// JS-computed aggregations over `results` that must never depend on this +// sweep's (agent-backed, best-effort) outcome. ---- +try { + await verifyGateState(results) +} catch (e) { + log('gate-state-verify sweep threw (non-fatal): ' + String((e && e.message) || e).slice(0, 200)) +} + // ---- Token usage: JS-computed aggregation (no LLM math), injected verbatim below ---- const TOKEN_AGG = aggregateTokens(results, spentTokens(), CONCURRENCY, STAGE_TOKENS, POOL_SPEND) diff --git a/docs/architecture/AGENTS.md b/docs/architecture/AGENTS.md index ce1d04d..668d970 100644 --- a/docs/architecture/AGENTS.md +++ b/docs/architecture/AGENTS.md @@ -68,7 +68,7 @@ to ship. | `invocation-and-guardrails.md` | Invocation, the sandbox lint, and the engine-owned path guardrail. | | `branching-and-merge.md` | The batch-branch model, release stage, and merge auto-resolve. | | `metrics.md` | Friction and churn, rework tax, gate yield, and outcome grading. | -| `gate-hygiene.md` | Typed review findings, engine-assigned ids, the three loop predicates, and gate-outcome tallying (the quality gate's disposition map, its cap, the pooled friction denominator, and its supersession of `metrics.md:13-14` and `:114`). Authored text, added after the split; not tracked in the provenance fixture, the same as the `AGENTS.md`/`CLAUDE.md` row below. | +| `gate-hygiene.md` | Typed review findings, engine-assigned ids, the three loop predicates, gate-outcome tallying (the quality gate's disposition map, its cap, the pooled friction denominator, and its supersession of `metrics.md:13-14` and `:114`), and the durable per-issue "## Gate State" comment (write boundaries, the four-state read contract, and the trust model). Authored text, added after the split; not tracked in the provenance fixture, the same as the `AGENTS.md`/`CLAUDE.md` row below. | | `failure-semantics.md` | How the run fails, halts, and resumes (two segments, emitted out of source order: the short bullet list first, the incident-derived-machinery table second). | | `cost-and-tokens.md` | Token tracking, cost estimation, and the token_budget guard. | | `scheduling.md` | Claims interop, the consolidation gate, and lane scheduling. | diff --git a/docs/architecture/CLAUDE.md b/docs/architecture/CLAUDE.md index ce1d04d..668d970 100644 --- a/docs/architecture/CLAUDE.md +++ b/docs/architecture/CLAUDE.md @@ -68,7 +68,7 @@ to ship. | `invocation-and-guardrails.md` | Invocation, the sandbox lint, and the engine-owned path guardrail. | | `branching-and-merge.md` | The batch-branch model, release stage, and merge auto-resolve. | | `metrics.md` | Friction and churn, rework tax, gate yield, and outcome grading. | -| `gate-hygiene.md` | Typed review findings, engine-assigned ids, the three loop predicates, and gate-outcome tallying (the quality gate's disposition map, its cap, the pooled friction denominator, and its supersession of `metrics.md:13-14` and `:114`). Authored text, added after the split; not tracked in the provenance fixture, the same as the `AGENTS.md`/`CLAUDE.md` row below. | +| `gate-hygiene.md` | Typed review findings, engine-assigned ids, the three loop predicates, gate-outcome tallying (the quality gate's disposition map, its cap, the pooled friction denominator, and its supersession of `metrics.md:13-14` and `:114`), and the durable per-issue "## Gate State" comment (write boundaries, the four-state read contract, and the trust model). Authored text, added after the split; not tracked in the provenance fixture, the same as the `AGENTS.md`/`CLAUDE.md` row below. | | `failure-semantics.md` | How the run fails, halts, and resumes (two segments, emitted out of source order: the short bullet list first, the incident-derived-machinery table second). | | `cost-and-tokens.md` | Token tracking, cost estimation, and the token_budget guard. | | `scheduling.md` | Claims interop, the consolidation gate, and lane scheduling. | diff --git a/docs/architecture/gate-hygiene.md b/docs/architecture/gate-hygiene.md index b05421a..b81bf51 100644 --- a/docs/architecture/gate-hygiene.md +++ b/docs/architecture/gate-hygiene.md @@ -518,6 +518,334 @@ same issue, run twice, can report a different quality contribution for reasons that have nothing to do with how hard it fought. Compare quality friction only within reports generated by the same version of this engine. +## Durable per-issue gate state + +Issue #166 gave every issue a durable record of its own gate/contrarian +history, carried on the issue itself so it survives a run boundary. +`processIssue` builds a fresh `ctx` on every invocation — `settled: []`, +`ctx.metrics.pr_review_iters: 0`, and so on — and the `process_pr` resume +path (a healed run that finds an open PR and jumps straight to +`reviewAndMerge(ctx)`) never runs the approach or plan gates at all. Before +this issue, neither loss was visible anywhere a human or a later run could +read it back: `runs.jsonl` is gitignored/host-local and doesn't carry +`gate_findings`, and the full per-issue detail under `logs//runs/` +is never read back by the engine. This section covers the mechanism that +fixes that: a title-gated `## Gate State` issue comment, mirroring the +`CONSOLIDATION_*` marker subsystem end to end (title-gated, fence-extracted, +append-only, closed with the canonical scope-guard marker). It is substrate +only — nothing reads or acts on the recorded state yet; see `seeded_from` +below for the shape that leaves for a future consumer. + +### The comment shape: JSON, not consolidation's flat lines + +`buildGateStateComment` renders a fixed shape — the `GATE_STATE_TITLE` line, +one deliberately non-directive human summary line, a `
`-wrapped +fenced JSON payload, and the canonical `` +marker as the last non-empty line, exactly the structure `parseGateStateComment` +requires on read. That payload departs from the consolidation markers' +own convention in one specific way: consolidation markers are flat, +`oneLine()`-rendered `key: value` text, parsed back out with a regex. +Gate state can't use that shape, because `settled` — the same array +`settleDecision`/`settledBlock` already maintain on `ctx`, capped here to +its last 6 entries the same way `settledBlock` caps its own render — is an +array of five-field objects (`topic`, `gate`, `decision`, `why`, `rejected`) +carrying free text a human or an earlier agent wrote. That text can contain +apostrophes, backticks, or newlines; `oneLine()`'s single-line-per-field +convention can't express it without lossy flattening. `JSON.stringify`/ +`JSON.parse` round-trips it exactly instead, so `buildGateStatePayload` +assembles a real object (`schema`, `repo`, `issue`, `run`, `batch`, `epoch`, +`write_seq`, `boundary`, `group_id`, `members`, `seeded_from`, `gate_budgets`, +`settled`) +and the comment fences it as JSON rather than trying to force it through +consolidation's flat format. + +### Four write boundaries, and the one that deliberately has none + +`postGateState(ctx, boundary)` is called at four points in `processIssue`'s +implementation path, each a place a gate has just resolved and nothing has +failed the issue yet: + +- **`'approach'`**, once, right after the approach-gate contrarian loop. All + four of that loop's exits (dead contrarian, `sound_with_caveats`, cap-out, + a dead re-evaluate) are `break`s out of the same loop, so one post placed + immediately after it durably records the approach gate's outcome + regardless of which exit fired. +- **`'plan'`**, once, right after the plan-gate contrarian loop, for the + same reason — its own four exits are all `break`s into the same + post-loop line. +- **`'pr-review-i' + iter`**, once per merge-gate iteration, called from + *inside* `reviewAndMerge`'s review loop, immediately after that + iteration's `recordGateOutcome(ctx, 'pr-review', ...)` call and before + the clean-approval, nothing-to-fix, and cap-reached branches that follow + it. It stays inside the loop deliberately: those three per-iteration + exits are the loop's own exits, and this is the one line in the loop body + every one of them passes through with `iter` — the value the boundary + name is built from — still in scope. A post moved to after the loop + would not reliably see that per-iteration state. +- **`'pr-review-i' + iter + '-aborted'`**, once, at the `return + fail(ctx, 'needs_human', ...)` a dead PR reviewer takes — the `!spec || + !code` branch. This is its own boundary, not a reuse of the in-loop one + above, because it is the *only* exit from the review loop a resumed run + can reach without ever having gone through `recordGateOutcome` for this + iteration: the `process_pr` resume path calls `reviewAndMerge(ctx)` + directly, so the approach and plan gates — and their own boundary posts + — never run on a resume. Without a dedicated post here, a resumed run + whose reviewers die on iteration 1 would record nothing at all for the + issue. `ctx.metrics.pr_review_iters` is already `iter` at this point (set + at the top of the loop body, before the reviews are dispatched), so no + extra plumbing is needed to name the boundary correctly. + +A fifth candidate exit — `if (STOP.tripped) return fail(...)` at the very +top of the review loop — deliberately gets no post of its own. This is a +recorded decision, not an oversight: the STOP check runs *before* +`ctx.metrics.pr_review_iters = iter` is assigned, so it splits into exactly +two cases. On iteration 1, the check can only fire before that assignment, +so `pr_review_iters` is still its ctx-init value of 0 and no agent has +acted this iteration — there is nothing this boundary could record that the +`'plan'` boundary immediately before this loop hasn't already captured. +On iteration 2 or later, the state is already durable: the *previous* +iteration's in-loop `'pr-review-i' + iter` post already recorded that +iteration's `recordGateOutcome` result before this iteration ever started. +Either way, a post at the STOP exit would be redundant with a boundary that +already fired. + +### Append-only, positional last-wins: idempotence without an exists-check + +Gate-state comments are never edited or deleted — every boundary just posts +a new one. Reading a live issue's history means walking its gate-state +comments *positionally*, newest to oldest, and taking the first usable one +— the same append-only/last-wins contract the `outcomes.jsonl`/ +`diffOutcomeGrades` pipeline and the consolidation markers' own heal pass +already use. This is what makes the write side idempotent for free: a +retried or re-run boundary that posts the same payload a second time +doesn't need to check whether an equivalent comment already exists first, +because whichever copy is newest on the thread is the one a reader will +select. Consolidation's own marker posts *do* need an "SKIP if one already +exists" check in their prompt, because a marker's meaning is binary +presence (is this issue in a group or not); a gate-state comment's meaning +is a value that can legitimately change between posts (an iteration count, +a settled list), so there's no equivalent presence check to make, and none +is attempted. + +### Four states on read, and why `absent` must be falsifiable + +`selectGateState(rows, evidence, priorWork)` is the single decision point +for "what does this issue's gate-state trail say, and can it be trusted?" +It resolves to exactly one of four states: + +- **`found`** — at least one comment parses. Selection walks blocks newest + to oldest and returns the first one whose author is trusted (see below); + every newer, parseable-but-untrusted block passed over on the way counts + into `skipped`. +- **`malformed`** — at least one comment exists on the thread, but none of + them parse (wrong title, missing scope-guard marker, unparseable fence). +- **`absent`** — zero gate-state comments, and nothing else about the issue + is evidence prior work happened. +- **`read-failed`** — the probe or the parse never produced usable data at + all. + +Only `read-failed` gets a distinct log path built specifically to make a +broken read impossible to mistake for a clean one: `fetchGateStateBlocks`'s +per-issue diagnostic line and `verifyGateState`'s per-issue outcome line +both name it explicitly, separately from `found`/`absent`/`malformed`, +because it is the one state that means the run learned nothing reliable +about this issue's history rather than learning that the history is empty +or broken. + +`absent` is the state that most needs to be earned rather than assumed. +Zero blocks and a zero `total` looks, on its own, exactly like a genuinely +fresh issue that has never reached a gate. But it looks *identical* to a +truncated or failed read on an issue that has real history — and the only +way to tell those two apart is to check something the gate-state read +itself has no access to: whether this issue shows other independent +evidence of prior work. `hasGateStatePriorWork(priorWork)` makes that check +explicit — a non-null `pr_number`, a `worktree_exists` of `true`, or a +`resume_point` other than `'implement'` — any one of these means a prior +run plainly did *something* here, so zero gate-state comments next to any +of them is contradictory, not confirmatory. `selectGateState` treats that +combination as `read-failed`, never `absent`. Absence is only accepted as +genuine when zero blocks/zero `total` is *not* contradicted by that +independent evidence. A second, narrower check runs first and +unconditionally, ahead of the prior-work cross-check: zero blocks with a +nonzero `total` is self-contradictory on its face (the probe reports +gate-state comments exist but produced none) — the exact shape a truncated +or corrupted read would take — so it is always `read-failed` regardless of +prior-work evidence. `total` is computed by the SAME title-gated jq filter +`blocks` uses (never a bare count of every comment on the issue), so this +branch is reachable only under a genuinely truncated or corrupted read — +not, as an earlier build of this jq idiom let happen, on any ordinary issue +that had simply received one unrelated human or bot comment. + +### The jq-pinned read idiom, not `fetchConsolidationMarkers`'s bare read + +`fetchGateStateBlocks` sits right next to `fetchConsolidationMarkers` in +the source and is shaped like it — one whole-set probe over every candidate +issue — but deliberately does not copy its read idiom. `fetchConsolidationMarkers` +hands the agent a bare `gh issue view --repo --json comments` and +trusts the agent's own judgment to pick out the right comment; a truncated +or partial response can get silently read as "no marker" with nothing to +catch the difference. Gate state instead pins the same deterministic +"last title-gated comment" idiom the claim probe already uses +(`gh issue view --repo --json comments --jq '{total, blocks}'`, +computed by `gateStateProbeCommandLine()`, where both `total` and `blocks` +run through the identical `select(.body | startswith("## Gate State"))` +filter): jq, not the agent, computes the exact return shape. The agent's +only job is relaying that command's stdout verbatim as `raw` — it never +parses or judges it. `parseGateStateProbeRow` +is what actually decides whether a read succeeded, and it is built so a +truncated or non-JSON `raw` string can never validate: any shape mismatch — +missing/wrong-typed `total`, `blocks` not an array, a block missing a +string `body` — returns the same `{ok: false, total: 0, blocks: []}` a +JSON parse failure would, which `selectGateState` reads as `read-failed`, +never as zero blocks. This is the structural fix the falsifiable-absent +rule above builds on: because the agent never selects or summarizes, a +truncated read has no path to presenting itself as an absent one. The same +pinned idiom, via the shared `gateStateProbeCommandLine`/`chunkGateStateIssues` +helpers, backs `verifyGateState`'s Report-phase read-back too — one idiom, +two call sites, so the read-side probe and its self-validation sweep can't +drift apart. + +### Trust before last-wins + +Selection is not simply "take the newest parseable comment." Within a +single issue's blocks, `selectGateState` walks newest to oldest and returns +the first block whose author is trusted, via `isTrustedGateStateAuthor` — +not the first block that merely parses. Every newer block that parses but +fails the trust check is passed over and counted into `skipped`, so a +caller can tell "the newest record was trusted" apart from "the newest +usable record we found was actually several posts back." If no block on +the issue is authored by a trusted identity, selection falls back to the +newest block that parses at all (the degenerate all-untrusted case), +returned with `trusted: false` and `skipped: 0` — there's real data, just +not from a source this run is willing to vouch for on its own, and the +caller is left able to tell the two cases apart rather than treating them +identically. + +### `intent-only-on-success`, and the `post-failed` sweep outcome it enables + +`postGateState` sets `ctx.gate_state_intent` to the payload it just built +only when the posting agent reports `posted === true`. Every other +outcome — a dead agent after its one retry, an explicit `posted: false`, a +schema mismatch — instead sets `ctx.gate_state_post_failed = boundary` (the +last such failure wins across an issue's several boundaries) and pushes a +`ctx.deferred` note, without ever touching `gate_state_intent`. This +asymmetry is what makes `verifyGateState`'s Report-phase sweep meaningful: +if intent were set unconditionally, a routine, already-logged posting miss +would look, to `diffGateStateIntent`, exactly like real corruption — an +intended payload with nothing on GitHub to match it. Because intent is only +recorded on a confirmed post, the sweep can distinguish the two outcomes +cleanly: `'post-failed'` (no intent recorded, but a failure was) reports +the benign, already-known miss; `'mismatch'` is reserved for a case where +this run genuinely believed it posted successfully and the read-back +disagrees. + +### The trust model: `self_login` primary, a bounded claim fallback + +`isTrustedGateStateAuthor(login, selfLogin, claimAuthors, batch)` checks two +signals, in order. The primary signal is identity: `login === selfLogin`, +where `selfLogin` is this deployment's own authenticated GitHub identity +(`gh api user --jq .login`, resolved once per probe chunk and reduced to +the first non-empty result). This is what closes a self-bootstrap trust +hole a capped approach-gate contrarian challenge flagged against a +`claim_authors`-only rule: without a primary signal independent of claims, +a stale, forged, or simply mistaken claim comment's author would still +count as trusted evidence. The fallback — for installation tokens where +`gh api user` 403s and `selfLogin` can't resolve — is `claimAuthors`, but +restricted: a claiming login only counts if its claim is either fresh +(`ageSeconds < CLAIM_STALE_SECONDS`) or matches this run's own batch +branch. A claim that is neither fresh nor batch-matching authored no work +in the scope of this run and is not evidence of anything current — trusting +it would let a stale or forged claim from an unrelated run's history stand +in for real authorship. + +### `RUN_EPOCH`: derived, never a new clock + +Nothing in this subsystem calls a wall clock of its own. `scripts/lint-engine.js` +forbids `Date.now()` outright (it breaks resume inside the Workflow-tool +sandbox), so `RUN_EPOCH` is derived from wall-clock reads two other probes +already make: `deriveRunEpoch((outcomeGradeR && outcomeGradeR.now) || +(revisitRiskR && revisitRiskR.now))`, run once at Select immediately after +both are awaited. Both `.now` values are a probe-returned `date -u ++%Y-%m-%dT%H:%M:%SZ` string, the same idiom the outcome-grading and +revisit-risk probes already used before this issue — no new probe call was +added purely to get a clock reading. `deriveRunEpoch` turns that string +into epoch milliseconds via the existing `toEpochMs`, or explicit `null` on +anything unparseable (deliberately never `NaN`, so a downstream +`gateStateEpochStale` subtraction against an unusable "now" reads as +unknown/stale rather than silently comparing false). `CLAIM_STALE_SECONDS` +(12h, the same constant the claim-staleness window already uses) is +documented here as the *intended* staleness bound for a gate-state block's +`epoch` — `gateStateEpochStale` computes it — but nothing at this tier +reads or enforces that `stale` flag. It's substrate for a future consumer, +carried through so one doesn't need a shape change to start acting on it. + +### `write_seq`: orders same-run writes; `epoch` can't + +`RUN_EPOCH` is assigned once at Select and is therefore identical across +every boundary a single run posts — it answers "how old is this run's data" +(what `gateStateEpochStale` needs), not "which of two same-run writes came +later" (what `diffGateStateIntent`'s `'superseded'` verdict needs). Those are +different questions with different answers: two boundaries from the same run +always carry the same `epoch`, so an epoch comparison between them is always +a tie, never an order. `GATE_STATE_WRITE_SEQ` is a separate module-level +counter — not a clock, a plain incrementing integer — that `postGateState` +bumps once per call and embeds on the payload as `write_seq`. Call order is +write order (a single issue's boundaries always post sequentially within +that issue's own `await` chain), so `diffGateStateIntent` orders same-run +writes on `write_seq`, never `epoch`. + +### `seeded_from`: a discriminator with no reader yet + +`buildGateStatePayload`'s `seeded_from` field names the `{run, epoch}` of +the gate-state block a resumed run's `gate_budgets` were carried forward +from, distinguishing a cumulative count (seeded from a prior run's +recorded state) from a fresh one (this run's budgets started at zero). It +is always `null` at this tier: no consumer seeds `gate_budgets` from a +prior block yet, so every call site passes `null`, and only the field's +presence in the schema — not its value — is what `parseGateStateComment` +round-trips against. A future consumer fills it in without needing a shape +change here. + +### The group-identity gap + +A gate-state comment posts only on `ctx.issue` — the group's current +primary — at each boundary, never on every live member the way +`postConsolidationMarkers` posts a marker on every member of a +materialized group (a group marker on the primary, a member marker on each +other live member). That asymmetry leaves a real gap: a group's logical +primary can move on re-anchor (the original primary closing or dropping +out, promoting a different member), and a re-anchored primary's own issue +thread carries no gate-state history of its own — the accumulated +`approach`/`plan`/`pr-review` iteration counts live only on the *old* +primary's thread. A resumed run on the newly-anchored primary reads +`absent` (or, if other evidence of prior work exists, `read-failed`) +rather than finding continuity, and fails open to a fresh budget rather +than inheriting the group's real history. This is a known, undocumented- +until-now gap at this tier, not a bug fixed here — it's why `group_id` and +`members` ride in every payload regardless: so a reader (today, a human; +later, a real consumer) can at least see which issues a surviving primary's +recorded state was speaking for, even when the full trail isn't reachable +from wherever the group currently anchors. + +### A probe, not a preflight step + +The issue that proposed this work described the read side as "add a step +to the preflight probe." What shipped instead is a separate, chunked, +whole-set probe (`fetchGateStateBlocks`, called once at Select over every +candidate issue, chunked at `MAX_GATE_STATE_PROBE_CHUNK`) rather than an +extra instruction folded into the existing per-issue preflight agent call. +The deviation is deliberate: the jq-pinned read idiom this subsystem needs +(see above) is a deterministic command with a fixed, verifiable return +shape, which fits a purpose-built probe far better than one more ask +layered onto a preflight prompt that already has several unrelated jobs. +`PREFLIGHT_SCHEMA` still carries the four gate-state fields the issue's +wording anticipated (`gate_state_blocks`, `gate_state_read_ok`, +`gate_state_total_comments`, `gate_state_trust`) — but they are written +unconditionally by `attachGateStateBlocks`, in JS, joining the probe's raw +per-issue rows onto each preflight after the fact, and always clobbering +any value an upstream agent might have hallucinated onto those same field +names. The preflight *agent* is never asked to produce them. + ## Provenance: the frozen passage this page supersedes `docs/architecture/metrics.md:81-84` — the "Completing the gate findings diff --git a/docs/architecture/index.md b/docs/architecture/index.md index 73723cf..1273401 100644 --- a/docs/architecture/index.md +++ b/docs/architecture/index.md @@ -12,7 +12,7 @@ plain JavaScript; every unit of actual work is a schema-validated subagent call. | [invocation-and-guardrails.md](invocation-and-guardrails.md) | Invocation, the sandbox lint, and the engine-owned path guardrail. | | [branching-and-merge.md](branching-and-merge.md) | The batch-branch model, release stage, and merge auto-resolve. | | [metrics.md](metrics.md) | Friction and churn, rework tax, gate yield, and outcome grading. | -| [gate-hygiene.md](gate-hygiene.md) | Typed review findings, engine-assigned ids, the absent-vs-empty distinction, the three loop predicates, and gate-outcome tallying (the quality gate's disposition map, its cap, the pooled friction denominator, and its supersession of `metrics.md:13-14` and `:114`). | +| [gate-hygiene.md](gate-hygiene.md) | Typed review findings, engine-assigned ids, the absent-vs-empty distinction, the three loop predicates, gate-outcome tallying (the quality gate's disposition map, its cap, the pooled friction denominator, and its supersession of `metrics.md:13-14` and `:114`), and the durable per-issue "## Gate State" comment (write boundaries, the four-state read contract, and the trust model). | | [failure-semantics.md](failure-semantics.md) | How the run fails, halts, and resumes. | | [cost-and-tokens.md](cost-and-tokens.md) | Token tracking, cost estimation, and the token_budget guard. | | [scheduling.md](scheduling.md) | Claims interop, the consolidation gate, and lane scheduling. | diff --git a/tests/gate-state-post.test.js b/tests/gate-state-post.test.js new file mode 100644 index 0000000..6467134 --- /dev/null +++ b/tests/gate-state-post.test.js @@ -0,0 +1,381 @@ +'use strict' + +// Integration tests for the durable per-issue gate-state WRITE path (issue +// #166, task 2): postGateState() itself, and its four call sites wired into +// implementIssue()/reviewAndMerge() (the approach-gate loop, the plan-gate +// loop, the in-loop pr-review post right after recordGateOutcome, and the +// pr-review-death "aborted" post that covers the process_pr resume path). +// +// tests/gate-state.test.js already proves the pure layer this helper is +// built on (buildGateStatePayload/buildGateStateComment/parseGateStateComment +// etc.) in isolation; these tests instead drive postGateState() and its call +// sites through the real control flow (loaded via tests/harness.js, same +// pattern as tests/contrarian-cap.test.js and tests/pr-review-gate.test.js), +// proving: +// - postGateState is non-fatal in isolation, in both directions (a dead +// agent, and an explicit posted:false), and sets ctx.gate_state_intent / +// ctx.gate_state_post_failed correctly for each. +// - a dead gate-state stage never changes what the SURROUNDING loop does +// (stage()'s own retry-then-swallow behavior is what makes this true; +// this proves postGateState doesn't accidentally re-throw or branch on +// the failure). +// - the 'approach' boundary posts exactly once, even when the very next +// stage (plan) dies. +// - a run that clears both gates posts 'approach' then 'plan', in that +// order, before IMPLEMENT. +// - the process_pr resume path (reviewAndMerge called directly, never +// through the approach/plan loops) posts exactly ONE +// 'pr-review-i1-aborted' block when both reviewers die on iteration 1, +// and nothing else -- the scenario boundary 4 exists for. +// - a STOP trip between pr-review iterations posts nothing new for the +// iteration that never runs. + +const test = require('node:test') +const assert = require('node:assert/strict') +const harness = require('./harness') + +const REPO = 'aaddrick/ticketmill-fixture' +const TARGET = 'Batch_2026-07-27_fixture' + +function seed(context, overrides) { + context.__seed(Object.assign({ PROFILE: {}, REPO: REPO, TARGET: TARGET }, overrides)) +} + +function stageKeyOf(call) { + const label = (call.opts && call.opts.label) || '' + return label.slice(label.indexOf(':') + 1) +} + +function gateStateKeys(keys) { + return keys.filter(function (k) { return k.indexOf('gate-state-') === 0 }) +} + +// Pulls the literal heredoc body back out of a postGateState prompt (the +// exact text between the two TICKETMILL_GATE_STATE_EOF markers -- see +// postGateState's prompt construction in workflows/ticketmill.js) and parses +// it with the real parseGateStateComment, so a test can assert on the ACTUAL +// payload a real postGateState() call built (including its GATE_STATE_WRITE_SEQ- +// derived write_seq), not a hand-crafted fixture. +function extractGateStatePayload(context, prompt, repo, issue) { + const m = /<<'TICKETMILL_GATE_STATE_EOF'\n([\s\S]*?)\nTICKETMILL_GATE_STATE_EOF/.exec(String(prompt)) + assert.ok(m, 'expected a TICKETMILL_GATE_STATE_EOF heredoc body in the prompt:\n' + prompt) + const payload = context.parseGateStateComment(m[1], repo, issue) + assert.ok(payload, 'expected the extracted heredoc body to parse as a valid gate-state comment') + return payload +} + +// ---- postGateState() in isolation ---- + +test('postGateState: a dead gate-state agent sets gate_state_post_failed (never gate_state_intent), pushes a ctx.deferred note, and returns falsy', async function () { + const context = harness.boot() + seed(context) + harness.installScriptedAgent(context, function () { return null }) // dead every time + + const ctx = harness.makeCtx({ issue: 10 }) + const posted = await context.postGateState(ctx, 'approach') + + assert.ok(!posted, 'a dead agent must not report a live post') + assert.strictEqual(ctx.gate_state_intent, undefined) + assert.strictEqual(ctx.gate_state_post_failed, 'approach') + assert.strictEqual(ctx.deferred.length, 1) + assert.ok(ctx.deferred[0].includes('approach'), 'expected the deferred note to name the boundary: ' + ctx.deferred[0]) +}) + +test('postGateState: an explicit posted:false leaves gate_state_intent unset and records gate_state_post_failed the same as a dead agent', async function () { + const context = harness.boot() + seed(context) + harness.installScriptedAgent(context, function () { return { posted: false } }) + + const ctx = harness.makeCtx({ issue: 11 }) + const posted = await context.postGateState(ctx, 'plan') + + assert.deepStrictEqual(posted, { posted: false }) + assert.strictEqual(ctx.gate_state_intent, undefined) + assert.strictEqual(ctx.gate_state_post_failed, 'plan') + assert.strictEqual(ctx.deferred.length, 1) +}) + +test('postGateState: posted:true sets gate_state_intent to the built payload and never touches gate_state_post_failed', async function () { + const context = harness.boot() + seed(context) + harness.installScriptedAgent(context, function () { return { posted: true } }) + + const ctx = harness.makeCtx({ issue: 12, groupId: null }) + const posted = await context.postGateState(ctx, 'pr-review-i2') + + assert.deepStrictEqual(posted, { posted: true }) + assert.strictEqual(ctx.gate_state_post_failed, undefined) + assert.ok(ctx.gate_state_intent, 'expected gate_state_intent to be set') + assert.strictEqual(ctx.gate_state_intent.repo, REPO) + assert.strictEqual(ctx.gate_state_intent.issue, 12) + assert.strictEqual(ctx.gate_state_intent.batch, TARGET) + assert.strictEqual(ctx.gate_state_intent.boundary, 'pr-review-i2') + assert.deepStrictEqual(JSON.parse(JSON.stringify(ctx.gate_state_intent.members)), [12]) + assert.strictEqual(ctx.deferred.length, 0) +}) + +test('postGateState: pins `gh issue comment --repo --body-file -` fed by a QUOTED heredoc, never an unquoted heredoc or --body "$(...)"', async function () { + const context = harness.boot() + seed(context) + let seenPrompt = null + harness.installScriptedAgent(context, function (prompt) { seenPrompt = String(prompt); return { posted: true } }) + + const ctx = harness.makeCtx({ issue: 13 }) + await context.postGateState(ctx, 'approach') + + assert.ok(seenPrompt.includes('gh issue comment 13 --repo ' + REPO + ' --body-file - <<\'TICKETMILL_GATE_STATE_EOF\''), + 'expected the pinned --body-file heredoc command in the prompt:\n' + seenPrompt) + // The actual pinned COMMAND line must not itself be the --body "$(...)" form + // (the prompt's own cautionary prose legitimately mentions that string, so + // this checks the command line specifically, not the prompt as a whole). + assert.ok(!seenPrompt.includes('gh issue comment 13 --repo ' + REPO + ' --body "$('), 'the pinned command must not use a --body "$(...)" form') +}) + +// ---- 'approach' boundary: implementIssue(), approach-gate loop ---- + +test('implementIssue: a run whose plan stage dies still posts exactly one gate-state "approach" block (posted before the plan stage runs)', async function () { + const context = harness.boot() + seed(context) + + harness.installScriptedAgent(context, function (prompt, opts) { + const label = (opts && opts.label) || '' + if (label === '20:setup') return { status: 'success', worktree: '/tmp/fixture-worktree', branch: 'issue-20-fixture' } + if (label === '20:research') return { status: 'success', context: { issue_title: 'Fixture', issue_body: 'req', related_files: [], dependencies: [], prior_work: '' } } + if (label === '20:evaluate') return { status: 'success', approach: 'do the thing', rationale: 'because', complexity: 'trivial', risks: [], alternatives_rejected: [], summary: 'initial evaluation' } + if (label === '20:challenge-approach-i1') return { verdict: 'sound_with_caveats', summary: 'fine', findings: [] } + if (label === '20:gate-state-approach') return { posted: true } + // plan agent dies -> implementIssue returns fail(ctx,'halted','plan',...) immediately after. + if (label === '20:plan') return null + if (label === '20:halt-note-plan') return { posted: true } + throw new Error('unexpected stage label: ' + label) + }) + + const ctx = harness.makeCtx({ issue: 20 }) + const result = await context.implementIssue(ctx) + + assert.strictEqual(result.status, 'halted') + assert.strictEqual(result.stage, 'plan') + + const keys = context.agent.calls.map(stageKeyOf) + const gateStateCalls = gateStateKeys(keys) + assert.deepStrictEqual(gateStateCalls, ['gate-state-approach']) + assert.ok(ctx.gate_state_intent, 'the approach boundary must have posted successfully') + assert.strictEqual(ctx.gate_state_intent.boundary, 'approach') +}) + +test('implementIssue: a dead gate-state-approach post changes no loop outcome — the plan stage still runs normally right after it', async function () { + const context = harness.boot() + seed(context) + + harness.installScriptedAgent(context, function (prompt, opts) { + const label = (opts && opts.label) || '' + if (label === '25:setup') return { status: 'success', worktree: '/tmp/fixture-worktree', branch: 'issue-25-fixture' } + if (label === '25:research') return { status: 'success', context: { issue_title: 'Fixture', issue_body: 'req', related_files: [], dependencies: [], prior_work: '' } } + if (label === '25:evaluate') return { status: 'success', approach: 'do the thing', rationale: 'because', complexity: 'trivial', risks: [], alternatives_rejected: [], summary: 'initial evaluation' } + if (label === '25:challenge-approach-i1') return { verdict: 'sound_with_caveats', summary: 'fine', findings: [] } + // The gate-state post dies (STAGE_TRIES-exhausting null); postGateState + // must swallow this and let the loop's own break stand. + if (label === '25:gate-state-approach') return null + // Plan runs regardless -- the dead gate-state stage above must not have + // halted, skipped, or altered the approach gate's own outcome. + if (label === '25:plan') return { status: 'error', error: 'stop test here (gate-state death is what is under test)' } + if (label === '25:halt-note-plan') return { posted: true } + throw new Error('unexpected stage label: ' + label) + }) + + const ctx = harness.makeCtx({ issue: 25 }) + const result = await context.implementIssue(ctx) + + // The plan stage ran (and failed on its OWN scripted error) -- proving the + // dead gate-state post did not short-circuit the approach loop's normal + // break-and-continue. + assert.strictEqual(result.status, 'failed') + assert.strictEqual(result.stage, 'plan') + assert.strictEqual(result.error, 'stop test here (gate-state death is what is under test)') + + // The approach gate itself still settled normally (unaffected by the dead post). + assert.strictEqual(ctx.settled.length, 1) + assert.strictEqual(ctx.settled[0].gate, 'approach challenge i1') + + // The dead post is recorded, not silently absorbed. + assert.strictEqual(ctx.gate_state_intent, undefined) + assert.strictEqual(ctx.gate_state_post_failed, 'approach') +}) + +// ---- 'approach' then 'plan', in order, before IMPLEMENT ---- + +test('implementIssue: a run that clears both gates posts gate-state "approach" then "plan", in that order, before the first task runs', async function () { + const context = harness.boot() + seed(context) + + const prompts = {} + harness.installScriptedAgent(context, function (prompt, opts) { + const label = (opts && opts.label) || '' + if (label === '21:setup') return { status: 'success', worktree: '/tmp/fixture-worktree', branch: 'issue-21-fixture' } + if (label === '21:research') return { status: 'success', context: { issue_title: 'Fixture', issue_body: 'req', related_files: [], dependencies: [], prior_work: '' } } + if (label === '21:evaluate') return { status: 'success', approach: 'do the thing', rationale: 'because', complexity: 'trivial', risks: [], alternatives_rejected: [], summary: 'initial evaluation' } + if (label === '21:challenge-approach-i1') return { verdict: 'sound_with_caveats', summary: 'fine', findings: [] } + if (label === '21:gate-state-approach') { prompts.approach = prompt; return { posted: true } } + if (label === '21:plan') return { status: 'success', summary: 'planned', tasks: [{ id: 1, description: 'Implement the fixture feature', agent: 'implementer' }], task_list_markdown: '' } + if (label === '21:challenge-plan-i1') return { verdict: 'sound_with_caveats', summary: 'fine', findings: [] } + if (label === '21:gate-state-plan') { prompts.plan = prompt; return { posted: true } } + // Fail the first task immediately so the test doesn't have to script the + // rest of IMPLEMENT — only the ordering of the two gate-state posts above it is under test. + if (label === '21:task-1-implement') return { status: 'error', summary: 'forced failure', error: 'stop test here' } + if (label === '21:halt-note-implement') return { posted: true } + throw new Error('unexpected stage label: ' + label) + }) + + const ctx = harness.makeCtx({ issue: 21 }) + const result = await context.implementIssue(ctx) + + assert.strictEqual(result.status, 'failed') + assert.strictEqual(result.stage, 'implement') + + const keys = context.agent.calls.map(stageKeyOf) + const gateStateCalls = gateStateKeys(keys) + assert.deepStrictEqual(gateStateCalls, ['gate-state-approach', 'gate-state-plan']) + // Both boundaries land BEFORE the first task-implement stage. + assert.ok(keys.indexOf('gate-state-plan') < keys.indexOf('task-1-implement')) + + // GATE_STATE_WRITE_SEQ (module-level, ++'d on every real postGateState() + // call) must have actually advanced across these two same-run writes, not + // just been present. RUN_EPOCH is identical for both (same run), so + // write_seq is the ONLY thing diffGateStateIntent can use to order two + // same-run boundaries against each other -- a regression that reverted the + // increment to a static value (e.g. always null, or always the same + // number) would leave every other assertion in this suite green while + // silently breaking same-run ordering. + const approachPayload = extractGateStatePayload(context, prompts.approach, REPO, 21) + const planPayload = extractGateStatePayload(context, prompts.plan, REPO, 21) + assert.strictEqual(typeof approachPayload.write_seq, 'number', 'expected a real numeric write_seq, not null/undefined') + assert.strictEqual(planPayload.write_seq, approachPayload.write_seq + 1, + 'expected the module-level GATE_STATE_WRITE_SEQ counter to advance by exactly 1 between the two real postGateState() calls in this run') +}) + +// ---- exactly one gate-state stage per pr-review iteration ---- + +test('reviewAndMerge: posts exactly one gate-state stage per pr-review iteration, in iteration order, across three iterations', async function () { + const context = harness.boot() + seed(context, { TEST_CMD: null }) + + const CHANGES_REQUESTED = { result: 'changes_requested', comments: 'needs work', issues: [{ severity: 'critical', summary: 'security hole' }], recommended_fix_agent: null, summary: 'needs work' } + const APPROVED = { result: 'approved', comments: '', issues: [], recommended_fix_agent: null, summary: 'fine' } + const FIX_OK = { status: 'success', commit: 'deadbeef', files_changed: [], fixes_applied: ['fixed it'], summary: 'fixed', error: null } + const SIMPLIFY_OK = { status: 'success', commit: null, files_changed: [], summary: 'nothing to simplify' } + const QUALITY_APPROVED = { result: 'approved', comments: '', issues: [], recommended_fix_agent: null, summary: 'clean' } + + harness.installScriptedAgent(context, function (prompt, opts) { + const label = (opts && opts.label) || '' + if (label === '26:spec-review-i1') return APPROVED + if (label === '26:code-review-i1') return CHANGES_REQUESTED + if (label === '26:gate-state-pr-review-i1') return { posted: true } + if (label === '26:pr-fix-i1') return FIX_OK + if (label === '26:simplify-pr-fix-i1-i1') return SIMPLIFY_OK + if (label === '26:quality-review-pr-fix-i1-i1') return QUALITY_APPROVED + if (label === '26:spec-review-i2') return APPROVED + if (label === '26:code-review-i2') return CHANGES_REQUESTED + if (label === '26:gate-state-pr-review-i2') return { posted: true } + if (label === '26:pr-fix-i2') return FIX_OK + if (label === '26:simplify-pr-fix-i2-i1') return SIMPLIFY_OK + if (label === '26:quality-review-pr-fix-i2-i1') return QUALITY_APPROVED + // iteration 3 = MAX_PR_REVIEW_ITERATIONS -> the cap breaks WITHOUT a fix stage. + if (label === '26:spec-review-i3') return APPROVED + if (label === '26:code-review-i3') return CHANGES_REQUESTED + if (label === '26:gate-state-pr-review-i3') return { posted: true } + throw new Error('unexpected stage label: ' + label) + }) + + const ctx = harness.makeCtx({ issue: 26, pr: 260 }) + const result = await context.reviewAndMerge(ctx) + + assert.strictEqual(result.status, 'needs_human') + assert.strictEqual(ctx.metrics.pr_review_iters, 3) + + const keys = context.agent.calls.map(stageKeyOf) + const gateStateCalls = gateStateKeys(keys) + assert.deepStrictEqual(gateStateCalls, ['gate-state-pr-review-i1', 'gate-state-pr-review-i2', 'gate-state-pr-review-i3']) +}) + +// ---- boundary 4: process_pr resume, reviewers die on iteration 1 ---- + +test('processIssue: a process_pr resume whose reviewers both die on iteration 1 posts exactly one "pr-review-i1-aborted" gate-state block and no other', async function () { + const context = harness.boot() + seed(context, { ROOT: '/tmp/ticketmill-fixture-root' }) + + harness.installScriptedAgent(context, function (prompt, opts) { + const label = (opts && opts.label) || '' + if (label === '22:setup-for-review') return { status: 'success', worktree: '/tmp/fixture-worktree', branch: 'issue-22-fixture' } + if (label === '22:spec-review-i1') return null // reviewer died + if (label === '22:code-review-i1') return { result: 'approved', comments: '', issues: [], recommended_fix_agent: null, summary: 'fine' } + if (label === '22:gate-state-pr-review-i1-aborted') return { posted: true } + if (label === '22:halt-note-pr-review') return { posted: true } + throw new Error('unexpected stage label: ' + label) + }) + + const pre = { issue: 22, title: 'Fixture', branch: '', pr_number: 220, resume_point: 'process_pr', reason: 'open PR found on resume' } + const result = await context.processIssue(pre) + + assert.strictEqual(result.status, 'needs_human') + assert.strictEqual(result.stage, 'pr-review') + + const keys = context.agent.calls.map(stageKeyOf) + const gateStateCalls = gateStateKeys(keys) + // Exactly the aborted boundary -- never 'gate-state-approach'/'gate-state-plan' + // (this resume path never runs implementIssue at all), and never the + // in-loop 'gate-state-pr-review-i1' (the reviewers never both returned). + assert.deepStrictEqual(gateStateCalls, ['gate-state-pr-review-i1-aborted']) + assert.strictEqual(result.gate_state_intent.boundary, 'pr-review-i1-aborted') +}) + +// ---- STOP trip between pr-review iterations ---- + +test('reviewAndMerge: STOP trips during iteration 1 posts nothing new for the iteration-2 boundary that never runs', async function () { + const context = harness.boot() + seed(context, { TEST_CMD: null }) + + const CHANGES_REQUESTED = { result: 'changes_requested', comments: 'needs work', issues: [{ severity: 'major', summary: 'thing to fix' }], recommended_fix_agent: null, summary: 'needs a fix' } + + harness.installScriptedAgent(context, function (prompt, opts) { + const label = (opts && opts.label) || '' + if (label === '23:spec-review-i1') return { result: 'approved', comments: '', issues: [], recommended_fix_agent: null, summary: 'fine' } + if (label === '23:code-review-i1') return CHANGES_REQUESTED + if (label === '23:gate-state-pr-review-i1') return { posted: true } + if (label === '23:pr-fix-i1') return { status: 'success', commit: 'deadbeef', files_changed: [], fixes_applied: ['fixed it'], summary: 'fixed', error: null } + if (label === '23:simplify-pr-fix-i1-i1') return { status: 'success', commit: null, files_changed: [], summary: 'nothing to simplify' } + if (label === '23:quality-review-pr-fix-i1-i1') { + // Trip STOP as a side effect right after the last stage of iteration 1 — + // iteration 2 must see it at the loop's own STOP check, before any + // review or gate-state stage for i2 runs. + harness.readGlobal(context, 'STOP.tripped = true; STOP.reason = "test stop"') + return { result: 'approved', comments: '', issues: [], recommended_fix_agent: null, summary: 'clean' } + } + throw new Error('unexpected stage label: ' + label) + }) + + const ctx = harness.makeCtx({ issue: 23, pr: 230 }) + const result = await context.reviewAndMerge(ctx) + + assert.strictEqual(result.status, 'halted') + assert.strictEqual(result.stage, 'pr-review') + + const keys = context.agent.calls.map(stageKeyOf) + const gateStateCalls = gateStateKeys(keys) + assert.deepStrictEqual(gateStateCalls, ['gate-state-pr-review-i1']) + assert.ok(!keys.includes('spec-review-i2'), 'iteration 2 must never start once STOP has tripped') +}) + +// ---- shape totality on the resume_point==='skip' return ---- + +test('processIssue: the resume_point==="skip" return always carries gate_state_intent/gate_state_post_failed as null (no boundary can fire on this path)', async function () { + const context = harness.boot() + seed(context) + harness.installScriptedAgent(context, function () { throw new Error('no agent call should happen on the skip path') }) + + const pre = { issue: 24, title: 'Fixture', resume_point: 'skip', reason: 'already merged', pr_state: 'open', pr_base: null } + const result = await context.processIssue(pre) + + assert.strictEqual(result.status, 'skipped') + assert.strictEqual(result.gate_state_intent, null) + assert.strictEqual(result.gate_state_post_failed, null) +}) diff --git a/tests/gate-state-read.test.js b/tests/gate-state-read.test.js new file mode 100644 index 0000000..94ec0b2 --- /dev/null +++ b/tests/gate-state-read.test.js @@ -0,0 +1,354 @@ +'use strict' + +// Integration tests for the durable per-issue gate-state READ path (issue +// #166, task 3): fetchGateStateBlocks() (the whole-set, jq-pinned, chunked +// probe) and attachGateStateBlocks()'s real-data join (extended in this task +// to accept the probe's raw rowsByIssue + self_login, beyond task 1's +// defaulting-only single-arg shape). +// +// tests/gate-state.test.js already proves the pure layer this is built on +// (parseGateStateProbeRow, selectGateState, isTrustedGateStateAuthor, ...) in +// isolation; these tests instead drive fetchGateStateBlocks() through the +// real control flow (loaded via tests/harness.js, same pattern as +// tests/gate-state-post.test.js), proving: +// - attachGateStateBlocks is total (every preflight gets all four fields) +// and non-mutating. +// - attachGateStateBlocks clobbers a hallucinated agent-supplied +// gate_state_blocks — both when real join data exists for that issue +// (real data wins) and when it does not (safe defaults win) — these four +// fields are NEVER read back off the preflight's own pre-existing value. +// - a dead probe (every chunk's agent call dies) marks every issue +// read-failed (via synthesized exit_ok:false stub rows, never a silent +// drop). +// - one dead chunk of three leaves the other two chunks' issues intact. +// - truncated/non-JSON jq stdout (agent succeeded, jq output is garbage) +// yields read-failed, never a fake "absent". +// - partial coverage (an issue outside the queried issueNumbers set +// entirely) leaves that issue at attachGateStateBlocks' safe defaults. +// - a probe returning nothing (empty issueNumbers) leaves preflights' +// OTHER fields byte-identical. +// - the self_login reduction across chunks picks the FIRST non-empty +// login, in chunk order, ignoring later ones. + +const test = require('node:test') +const assert = require('node:assert/strict') +const harness = require('./harness') + +const REPO = 'aaddrick/ticketmill-fixture' +const TARGET = 'Batch_2026-07-27_fixture' + +function seed(context, overrides) { + context.__seed(Object.assign({ PROFILE: {}, REPO: REPO, TARGET: TARGET }, overrides)) +} + +function jqRow(total, blocks) { + return JSON.stringify({ total: total, blocks: blocks || [] }) +} + +// ---- attachGateStateBlocks: total, non-mutating, clobbers hallucinated fields ---- + +test('attachGateStateBlocks: total and non-mutating over a real rowsByIssue join', function () { + const context = harness.boot() + const preflights = [ + { issue: 1, pr_number: null }, + { issue: 2, pr_number: null }, + ] + const rowsByIssue = { + 1: { raw: jqRow(3, [{ body: 'not a gate-state comment', author_login: 'someone', author_association: 'NONE' }]), exit_ok: true }, + } + const attached = harness.normalize(context.attachGateStateBlocks(preflights, rowsByIssue, 'ticketmill-bot')) + + assert.strictEqual(attached.length, 2) + // issue 1: covered, valid read, zero matching blocks (comment didn't title-match, but + // parseGateStateProbeRow doesn't re-filter -- the jq already did; this fixture's single + // "block" entry is just illustrative of the shape, total/blocks pass through as given) + assert.strictEqual(attached[0].gate_state_read_ok, true) + assert.strictEqual(attached[0].gate_state_total_comments, 3) + assert.strictEqual(attached[0].gate_state_trust, 'ticketmill-bot') + // issue 2: not in rowsByIssue at all -- fail-open defaults + assert.deepStrictEqual(attached[1], { + issue: 2, pr_number: null, + gate_state_blocks: [], gate_state_read_ok: false, gate_state_total_comments: 0, gate_state_trust: 'ticketmill-bot', + }) + // non-mutating: original preflight objects never gained the new keys + assert.strictEqual(Object.prototype.hasOwnProperty.call(preflights[0], 'gate_state_blocks'), false) + assert.strictEqual(Object.prototype.hasOwnProperty.call(preflights[1], 'gate_state_blocks'), false) +}) + +test('attachGateStateBlocks: clobbers a hallucinated agent-supplied gate_state_blocks with real join data', function () { + const context = harness.boot() + const preflights = [ + { issue: 1, gate_state_blocks: ['a hallucinated block the agent invented'], gate_state_read_ok: true, gate_state_total_comments: 99, gate_state_trust: 'not-a-real-login' }, + ] + const rowsByIssue = { 1: { raw: jqRow(1, [{ body: 'real body', author_login: 'ticketmill-bot', author_association: 'OWNER' }]), exit_ok: true } } + const attached = harness.normalize(context.attachGateStateBlocks(preflights, rowsByIssue, 'ticketmill-bot')) + + assert.deepStrictEqual(attached[0].gate_state_blocks, ['real body']) + assert.strictEqual(attached[0].gate_state_total_comments, 1) + assert.strictEqual(attached[0].gate_state_trust, 'ticketmill-bot') +}) + +test('attachGateStateBlocks: clobbers a hallucinated agent-supplied gate_state_blocks even with NO join data (never falls back to the preflight\'s own value)', function () { + const context = harness.boot() + const preflights = [ + { issue: 1, gate_state_blocks: ['a hallucinated block'], gate_state_read_ok: true, gate_state_total_comments: 7, gate_state_trust: 'someone-else' }, + ] + const attached = harness.normalize(context.attachGateStateBlocks(preflights, {}, '')) + + assert.deepStrictEqual(attached[0].gate_state_blocks, []) + assert.strictEqual(attached[0].gate_state_read_ok, false) + assert.strictEqual(attached[0].gate_state_total_comments, 0) + assert.strictEqual(attached[0].gate_state_trust, '') +}) + +test('attachGateStateBlocks: partial coverage -- an issue outside rowsByIssue entirely gets the safe defaults', function () { + const context = harness.boot() + const preflights = [{ issue: 1 }, { issue: 2 }, { issue: 3 }] + const rowsByIssue = { + 1: { raw: jqRow(0, []), exit_ok: true }, + 2: { raw: jqRow(0, []), exit_ok: true }, + // issue 3 never queried at all (e.g. fetchGateStateBlocks was called with issueNumbers=[1,2]) + } + const attached = harness.normalize(context.attachGateStateBlocks(preflights, rowsByIssue, 'bot')) + assert.strictEqual(attached[0].gate_state_read_ok, true) + assert.strictEqual(attached[1].gate_state_read_ok, true) + assert.deepStrictEqual(attached[2], { issue: 3, gate_state_blocks: [], gate_state_read_ok: false, gate_state_total_comments: 0, gate_state_trust: 'bot' }) +}) + +test('attachGateStateBlocks: truncated jq stdout yields gate_state_read_ok:false (read-failed), never a fake successful-empty read', function () { + const context = harness.boot() + const preflights = [{ issue: 1 }] + const rowsByIssue = { 1: { raw: '{"total": 4, "blocks": [{"body": "trun', exit_ok: true } } // truncated mid-stream + const attached = harness.normalize(context.attachGateStateBlocks(preflights, rowsByIssue, '')) + assert.strictEqual(attached[0].gate_state_read_ok, false) + assert.strictEqual(attached[0].gate_state_total_comments, 0) + assert.deepStrictEqual(attached[0].gate_state_blocks, []) +}) + +test('attachGateStateBlocks: a probe returning nothing (no rowsByIssue, no self_login) leaves every OTHER preflight field byte-identical', function () { + const context = harness.boot() + const preflights = [{ issue: 1, title: 'fixture', pr_number: 5, resume_point: 'process_pr' }] + const attached = harness.normalize(context.attachGateStateBlocks(preflights)) + assert.strictEqual(attached[0].issue, 1) + assert.strictEqual(attached[0].title, 'fixture') + assert.strictEqual(attached[0].pr_number, 5) + assert.strictEqual(attached[0].resume_point, 'process_pr') + assert.strictEqual(attached[0].gate_state_read_ok, false) +}) + +// ---- fetchGateStateBlocks: chunking, dead-chunk isolation, self_login reduction ---- + +test('fetchGateStateBlocks: a fully dead probe (every chunk agent call dies) marks every issue read-failed via explicit exit_ok:false stub rows', async function () { + const context = harness.boot() + seed(context) + harness.installScriptedAgent(context, function () { return null }) // every chunk call dies + + const result = await context.fetchGateStateBlocks([1, 2, 3], {}) + const rows = harness.normalize(result.rowsByIssue) + assert.strictEqual(result.self_login, '') + for (const n of [1, 2, 3]) { + assert.deepStrictEqual(rows[n], { raw: '', exit_ok: false }, 'issue #' + n + ' must be an explicit read-failed stub, not silently absent') + } +}) + +test('fetchGateStateBlocks: a fully dead probe WITH prior-work evidence logs plain read-failed, never the suspicious-absent wording (issue #166 PR #177 review, iteration 2)', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { return null }) // every chunk call dies + + await context.fetchGateStateBlocks([2], { 2: { pr_number: 99, worktree_exists: false, resume_point: 'review' } }) + const line = logs.find(function (l) { return l.indexOf('#2') !== -1 }) + assert.ok(line, 'expected a log line for issue #2: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1, 'a dead chunk with prior-work evidence must still log read-failed: ' + line) + assert.ok(line.indexOf('absent') === -1, 'a dead chunk must never be logged as absent, even with prior-work evidence: ' + line) +}) + +test('fetchGateStateBlocks: a non-zero gh exit for one issue WITH prior-work evidence logs plain read-failed, never the suspicious-absent wording (issue #166 PR #177 review, iteration 2)', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 2, raw: '', exit_ok: false }] } // gh exited non-zero for this issue + }) + + await context.fetchGateStateBlocks([2], { 2: { pr_number: 99, worktree_exists: false, resume_point: 'review' } }) + const line = logs.find(function (l) { return l.indexOf('#2') !== -1 }) + assert.ok(line, 'expected a log line for issue #2: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1, 'a non-zero gh exit with prior-work evidence must still log read-failed: ' + line) + assert.ok(line.indexOf('absent') === -1, 'a non-zero gh exit must never be logged as absent, even with prior-work evidence: ' + line) +}) + +test('fetchGateStateBlocks: one dead chunk of three leaves the other two chunks\' issues intact', async function () { + const context = harness.boot() + seed(context) + // 11 issues -> chunks of [5, 5, 1] at MAX_GATE_STATE_PROBE_CHUNK=5 -- three chunks. + const issues = [] + for (let i = 1; i <= 11; i++) issues.push(i) + harness.installScriptedAgent(context, function (prompt, opts) { + if (opts.label === 'gate-state-probe-c1') return null // the middle chunk (issues 6-10) dies + const chunkIssues = opts.label === 'gate-state-probe-c0' ? issues.slice(0, 5) : issues.slice(10, 11) + return { self_login: 'ticketmill-bot', rows: chunkIssues.map(function (n) { return { issue: n, raw: jqRow(0, []), exit_ok: true } }) } + }) + + const result = await context.fetchGateStateBlocks(issues, {}) + const rows = harness.normalize(result.rowsByIssue) + for (const n of [1, 2, 3, 4, 5]) assert.strictEqual(rows[n].exit_ok, true, 'issue #' + n + ' (surviving chunk 0) must be intact') + for (const n of [6, 7, 8, 9, 10]) assert.deepStrictEqual(rows[n], { raw: '', exit_ok: false }, 'issue #' + n + ' (dead chunk 1) must be read-failed') + assert.strictEqual(rows[11].exit_ok, true, 'issue #11 (surviving chunk 2) must be intact') +}) + +test('fetchGateStateBlocks: a live chunk that returns rows for some but not all of its assigned issues never lets the missing issue read as absent', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + // Schema-valid response (GATE_STATE_PROBE_SCHEMA can't enforce one row per + // issue) that simply omits issue 2's row -- distinct from a dead chunk, + // which this file's other tests already cover via a null/throwing agent. + return { self_login: 'bot', rows: [{ issue: 1, raw: jqRow(0, []), exit_ok: true }] } + }) + + const result = await context.fetchGateStateBlocks([1, 2], {}) + const rows = harness.normalize(result.rowsByIssue) + assert.deepStrictEqual(rows[2], { raw: '', exit_ok: false }, 'issue #2 must be backfilled as an explicit read-failed stub, never silently missing') + const line = logs.find(function (l) { return l.indexOf('#2') !== -1 }) + assert.ok(line, 'expected a log line for issue #2: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1 && line.indexOf('absent') === -1, 'issue #2 must log read-failed, never absent: ' + line) +}) + +test('fetchGateStateBlocks: a live chunk that omits an issue\'s row WITH prior-work evidence still logs plain read-failed, never the suspicious-absent wording (issue #166 PR #177 review, iteration 2)', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 1, raw: jqRow(0, []), exit_ok: true }] } // omits issue 2's row + }) + + await context.fetchGateStateBlocks([1, 2], { 2: { pr_number: 99, worktree_exists: false, resume_point: 'review' } }) + const line = logs.find(function (l) { return l.indexOf('#2') !== -1 }) + assert.ok(line, 'expected a log line for issue #2: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1, 'an omitted row with prior-work evidence must still log read-failed: ' + line) + assert.ok(line.indexOf('absent') === -1, 'an omitted row must never be logged as absent, even with prior-work evidence: ' + line) +}) + +test('fetchGateStateBlocks: truncated jq stdout (agent succeeded, jq output is garbage) logs read-failed, never absent', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 42, raw: '{"total": 2, "blocks": [{"bo', exit_ok: true }] } + }) + + await context.fetchGateStateBlocks([42], {}) + const line = logs.find(function (l) { return l.indexOf('#42') !== -1 }) + assert.ok(line, 'expected a log line naming issue #42: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1, 'expected "read-failed", got: ' + line) + assert.ok(line.indexOf('absent') === -1, 'must never log a truncated read as absent: ' + line) +}) + +test('fetchGateStateBlocks: truncated jq stdout WITH prior-work evidence logs plain read-failed, never the suspicious-absent wording (issue #166 PR #177 review, iteration 2)', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 42, raw: '{"total": 2, "blocks": [{"bo', exit_ok: true }] } + }) + + await context.fetchGateStateBlocks([42], { 42: { pr_number: 99, worktree_exists: false, resume_point: 'review' } }) + const line = logs.find(function (l) { return l.indexOf('#42') !== -1 }) + assert.ok(line, 'expected a log line for issue #42: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1, 'truncated stdout with prior-work evidence must still log read-failed: ' + line) + assert.ok(line.indexOf('absent') === -1, 'truncated stdout must never be logged as absent, even with prior-work evidence: ' + line) +}) + +test('fetchGateStateBlocks: a genuinely empty issue (valid read, zero blocks, zero total, no prior-work evidence) logs absent', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 7, raw: jqRow(0, []), exit_ok: true }] } + }) + + await context.fetchGateStateBlocks([7], { 7: { pr_number: null, worktree_exists: false, resume_point: 'implement' } }) + const line = logs.find(function (l) { return l.indexOf('#7') !== -1 }) + assert.ok(line && line.indexOf('absent') !== -1 && line.indexOf('unexpected') === -1, 'expected a plain "absent" line: ' + JSON.stringify(logs)) +}) + +test('fetchGateStateBlocks: zero blocks, zero total, WITH prior-work evidence (an open PR) logs the distinct suspicious-absent line, not a bare read-failed', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 9, raw: jqRow(0, []), exit_ok: true }] } // total===0, zero gate-state blocks -- the falsifiable-absent case + }) + + await context.fetchGateStateBlocks([9], { 9: { pr_number: 123, worktree_exists: false, resume_point: 'process_pr' } }) + const line = logs.find(function (l) { return l.indexOf('#9') !== -1 }) + assert.ok(line, 'expected a log line for issue #9: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('unexpected') !== -1 && line.indexOf('PR #123') !== -1, 'expected the distinct greppable suspicious-case string: ' + line) +}) + +test('fetchGateStateBlocks: zero blocks but total>0 (a corrupted/truncated read) WITH prior-work evidence still logs a plain read-failed, never the suspicious-absent wording', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 10, raw: jqRow(3, []), exit_ok: true }] } // total>0, zero gate-state blocks -- self-contradictory read + }) + + await context.fetchGateStateBlocks([10], { 10: { pr_number: 123, worktree_exists: false, resume_point: 'process_pr' } }) + const line = logs.find(function (l) { return l.indexOf('#10') !== -1 }) + assert.ok(line, 'expected a log line for issue #10: ' + JSON.stringify(logs)) + assert.ok(line.indexOf('read-failed') !== -1, 'expected the plain "read-failed" line: ' + line) + assert.ok(line.indexOf('unexpected') === -1, 'a total>0 corrupted read must never be logged as "absent (unexpected...)": ' + line) +}) + +test('fetchGateStateBlocks: partial coverage -- an issue outside the queried issueNumbers set gets no row and no log line at all', async function () { + const context = harness.boot() + seed(context) + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + harness.installScriptedAgent(context, function () { + return { self_login: 'bot', rows: [{ issue: 1, raw: jqRow(0, []), exit_ok: true }] } + }) + + const result = await context.fetchGateStateBlocks([1], {}) + assert.strictEqual(Object.prototype.hasOwnProperty.call(harness.normalize(result.rowsByIssue), '2'), false) + assert.strictEqual(logs.some(function (l) { return l.indexOf('#2') !== -1 }), false) +}) + +test('fetchGateStateBlocks: a probe called with no issues at all returns empty and makes no agent call', async function () { + const context = harness.boot() + seed(context) + const agentStub = harness.installScriptedAgent(context, function () { return { self_login: 'bot', rows: [] } }) + + const result = await context.fetchGateStateBlocks([], {}) + assert.deepStrictEqual(harness.normalize(result.rowsByIssue), {}) + assert.strictEqual(result.self_login, '') + assert.strictEqual(agentStub.calls.length, 0) +}) + +test('fetchGateStateBlocks: self_login reduction picks the FIRST non-empty login across chunks, in chunk order', async function () { + const context = harness.boot() + seed(context) + const issues = [] + for (let i = 1; i <= 11; i++) issues.push(i) // three chunks: c0 (1-5), c1 (6-10), c2 (11) + harness.installScriptedAgent(context, function (prompt, opts) { + const login = opts.label === 'gate-state-probe-c0' ? '' : opts.label === 'gate-state-probe-c1' ? 'first-real-login' : 'second-real-login' + const n = opts.label === 'gate-state-probe-c0' ? issues.slice(0, 5) : opts.label === 'gate-state-probe-c1' ? issues.slice(5, 10) : issues.slice(10, 11) + return { self_login: login, rows: n.map(function (x) { return { issue: x, raw: jqRow(0, []), exit_ok: true } }) } + }) + + const result = await context.fetchGateStateBlocks(issues, {}) + assert.strictEqual(result.self_login, 'first-real-login') +}) diff --git a/tests/gate-state-verify.test.js b/tests/gate-state-verify.test.js new file mode 100644 index 0000000..ed122d5 --- /dev/null +++ b/tests/gate-state-verify.test.js @@ -0,0 +1,334 @@ +'use strict' + +// Integration tests for the durable per-issue gate-state Report-phase +// self-validation sweep (issue #166, task 4): verifyGateState(results). +// +// tests/gate-state.test.js already proves the pure layer this is built on +// (parseGateStateProbeRow, parseGateStateComment, diffGateStateIntent, +// buildGateStatePayload/buildGateStateComment) in isolation; these tests +// instead drive verifyGateState() through the real control flow (loaded via +// tests/harness.js, same pattern as tests/gate-state-read.test.js), proving: +// - all six outcomes are logged correctly: no-intent, post-failed, +// read-failed, match, superseded, mismatch. +// - a fully dead verify stage (every chunk's agent call dies) logs every +// issue-with-an-intent as read-failed, never silently skipped. +// - one dead chunk of two leaves the other chunk's issue intact. +// - the prompt handed to the agent NEVER carries the payload being +// verified against — only issue numbers and the pinned jq command — so +// a scripted agent has no way to "cheat" the comparison. +// - the sweep never makes an agent call at all when nothing has an intent +// (every result is no-intent/post-failed). +// - 'mismatch' and 'read-failed' each push a VERIFY_SKIPS entry naming the +// issue (the batch PR's Verification Gaps section); 'match'/'superseded' +// never do. + +const test = require('node:test') +const assert = require('node:assert/strict') +const harness = require('./harness') + +const REPO = 'aaddrick/ticketmill-fixture' +const TARGET = 'Batch_2026-07-27_fixture' + +function seed(context, overrides) { + context.__seed(Object.assign({ PROFILE: {}, REPO: REPO, TARGET: TARGET }, overrides)) +} + +function jqRaw(blocks) { + return JSON.stringify({ total: blocks.length, blocks: blocks }) +} + +function block(body) { + return { body: body, author_login: 'ticketmill-bot', author_association: 'OWNER' } +} + +function makeResult(context, issue, overrides) { + return Object.assign({ issue: issue, title: 'fixture', gate_state_intent: null, gate_state_post_failed: null }, overrides) +} + +function intentFor(context, issue, run, epoch, boundary, writeSeq) { + return context.buildGateStatePayload({ repo: REPO, issue: issue, run: run || 'run-1', batch: TARGET, epoch: (epoch === undefined ? 1000 : epoch), boundary: boundary || 'plan', write_seq: (writeSeq === undefined ? 1 : writeSeq) }) +} + +function bodyFor(context, issue, payload) { + return context.buildGateStateComment(REPO, issue, payload) +} + +function logsOf(context) { + const logs = [] + context.log = function (msg) { logs.push(String(msg)) } + return logs +} + +function outcomeLine(logs, issue) { + return logs.find(function (l) { return l.indexOf('#' + issue + ':') !== -1 }) +} + +// ---- no-intent / post-failed: no probe call needed ---- + +test('verifyGateState: a result with neither gate_state_intent nor gate_state_post_failed logs no-intent and makes no agent call', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const agentStub = harness.installScriptedAgent(context, function () { return null }) + + const results = [makeResult(context, 1)] + await context.verifyGateState(results) + + assert.strictEqual(agentStub.calls.length, 0, 'no-intent/post-failed-only results must never trigger a probe call') + assert.ok(outcomeLine(logs, 1).indexOf('no-intent') !== -1, outcomeLine(logs, 1)) +}) + +test('verifyGateState: gate_state_post_failed set (no intent) logs post-failed, distinct from no-intent', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + harness.installScriptedAgent(context, function () { return null }) + + const results = [makeResult(context, 2, { gate_state_post_failed: 'approach' })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 2).indexOf('post-failed') !== -1, outcomeLine(logs, 2)) +}) + +// ---- match / superseded / mismatch: probe returns real data ---- + +test('verifyGateState: newest block round-trips byte-identical to the intent -- logs match', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const payload = intentFor(context, 3) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 3, raw: jqRaw([block(bodyFor(context, 3, payload))]), exit_ok: true }] } + }) + + const results = [makeResult(context, 3, { gate_state_intent: payload })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 3).indexOf('match') !== -1 && outcomeLine(logs, 3).indexOf('mismatch') === -1, outcomeLine(logs, 3)) +}) + +test('verifyGateState: newest block is a LATER write from the SAME run -- logs superseded, not mismatch', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const intent = intentFor(context, 4, 'run-1', 1000, 'pr-review-i1', 1) + const later = intentFor(context, 4, 'run-1', 1000, 'pr-review-i2', 2) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 4, raw: jqRaw([block(bodyFor(context, 4, intent)), block(bodyFor(context, 4, later))]), exit_ok: true }] } + }) + + const results = [makeResult(context, 4, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 4).indexOf('superseded') !== -1, outcomeLine(logs, 4)) +}) + +test('verifyGateState: newest block is a DIFFERENT run\'s write -- logs mismatch, real corruption never hidden as superseded', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const intent = intentFor(context, 5, 'run-1', 1000) + const otherRun = intentFor(context, 5, 'concurrent-run', 2000) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 5, raw: jqRaw([block(bodyFor(context, 5, otherRun))]), exit_ok: true }] } + }) + + const results = [makeResult(context, 5, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 5).indexOf('mismatch') !== -1, outcomeLine(logs, 5)) +}) + +test('verifyGateState: an intent recorded but zero gate-state blocks read back -- logs mismatch (a lost write), never silently absent', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const intent = intentFor(context, 6) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 6, raw: jqRaw([]), exit_ok: true }] } + }) + + const results = [makeResult(context, 6, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 6).indexOf('mismatch') !== -1, outcomeLine(logs, 6)) +}) + +// ---- read-failed: probe/parse itself is unusable ---- + +test('verifyGateState: exit_ok:false on the probe row logs read-failed', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const intent = intentFor(context, 7) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 7, raw: '', exit_ok: false }] } + }) + + const results = [makeResult(context, 7, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 7).indexOf('read-failed') !== -1, outcomeLine(logs, 7)) +}) + +// ---- mismatch/read-failed reach VERIFY_SKIPS -- the batch PR's Verification +// Gaps section is the human's only window into what this run's gate-state +// self-validation couldn't confirm ---- + +test('verifyGateState: a mismatch outcome pushes a VERIFY_SKIPS entry naming the issue', async function () { + const context = harness.boot() + seed(context) + const intent = intentFor(context, 5, 'run-1', 1000) + const otherRun = intentFor(context, 5, 'concurrent-run', 2000) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 5, raw: jqRaw([block(bodyFor(context, 5, otherRun))]), exit_ok: true }] } + }) + + const results = [makeResult(context, 5, { gate_state_intent: intent })] + await context.verifyGateState(results) + + const skips = Array.from(harness.readGlobal(context, 'VERIFY_SKIPS')) + assert.ok(skips.some(function (s) { return s.indexOf('#5') !== -1 && s.indexOf('mismatch') !== -1 }), 'expected a VERIFY_SKIPS entry for issue #5: ' + JSON.stringify(skips)) +}) + +test('verifyGateState: a read-failed outcome pushes a VERIFY_SKIPS entry naming the issue', async function () { + const context = harness.boot() + seed(context) + const intent = intentFor(context, 7) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 7, raw: '', exit_ok: false }] } + }) + + const results = [makeResult(context, 7, { gate_state_intent: intent })] + await context.verifyGateState(results) + + const skips = Array.from(harness.readGlobal(context, 'VERIFY_SKIPS')) + assert.ok(skips.some(function (s) { return s.indexOf('#7') !== -1 && s.indexOf('read-failed') !== -1 }), 'expected a VERIFY_SKIPS entry for issue #7: ' + JSON.stringify(skips)) +}) + +test('verifyGateState: match and superseded outcomes never push a VERIFY_SKIPS entry', async function () { + const context = harness.boot() + seed(context) + const matchPayload = intentFor(context, 3) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 3, raw: jqRaw([block(bodyFor(context, 3, matchPayload))]), exit_ok: true }] } + }) + + const results = [makeResult(context, 3, { gate_state_intent: matchPayload })] + await context.verifyGateState(results) + + const skips = Array.from(harness.readGlobal(context, 'VERIFY_SKIPS')) + assert.strictEqual(skips.some(function (s) { return s.indexOf('#3') !== -1 }), false, 'a clean match must never surface as a verification gap: ' + JSON.stringify(skips)) +}) + +test('verifyGateState: truncated/non-JSON jq stdout (agent succeeded, jq output is garbage) logs read-failed, never a fake match/mismatch', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const intent = intentFor(context, 8) + harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 8, raw: '{"total": 1, "blocks": [{"bo', exit_ok: true }] } + }) + + const results = [makeResult(context, 8, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 8).indexOf('read-failed') !== -1, outcomeLine(logs, 8)) +}) + +test('verifyGateState: a fully dead verify stage (every chunk agent call dies) logs read-failed for every issue with an intent, none silently dropped', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + harness.installScriptedAgent(context, function () { return null }) + + const results = [1, 2, 3].map(function (n) { return makeResult(context, n, { gate_state_intent: intentFor(context, n) }) }) + await context.verifyGateState(results) + + for (const n of [1, 2, 3]) { + const line = outcomeLine(logs, n) + assert.ok(line, 'expected a log line for issue #' + n) + assert.ok(line.indexOf('read-failed') !== -1, line) + } +}) + +test('verifyGateState: one dead chunk of two leaves the other chunk\'s issue intact', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + // MAX_GATE_STATE_PROBE_CHUNK is 5 -- 6 issues-with-intent -> two chunks: c0 (1-5), c1 (6). + const issues = [1, 2, 3, 4, 5, 6] + const intents = {} + for (const n of issues) intents[n] = intentFor(context, n) + + harness.installScriptedAgent(context, function (prompt, opts) { + if (opts.label === 'gate-state-verify-c0') return null // first chunk dies + return { rows: [6].map(function (n) { return { issue: n, raw: jqRaw([block(bodyFor(context, n, intents[n]))]), exit_ok: true } }) } + }) + + const results = issues.map(function (n) { return makeResult(context, n, { gate_state_intent: intents[n] }) }) + await context.verifyGateState(results) + + for (const n of [1, 2, 3, 4, 5]) assert.ok(outcomeLine(logs, n).indexOf('read-failed') !== -1, 'dead chunk 0 issue #' + n + ': ' + outcomeLine(logs, n)) + assert.ok(outcomeLine(logs, 6).indexOf('match') !== -1, 'surviving chunk 1 issue #6: ' + outcomeLine(logs, 6)) +}) + +// ---- the prompt never carries the payload being verified against ---- + +test('verifyGateState: the agent prompt carries only issue numbers and the pinned jq command -- never the intent payload', async function () { + const context = harness.boot() + seed(context) + const intent = intentFor(context, 9, 'run-1', 1000, 'plan') + const agentStub = harness.installScriptedAgent(context, function () { + return { rows: [{ issue: 9, raw: jqRaw([block(bodyFor(context, 9, intent))]), exit_ok: true }] } + }) + + const results = [makeResult(context, 9, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.strictEqual(agentStub.calls.length, 1) + const prompt = agentStub.calls[0].prompt + // the intent's JSON-serialized settled/gate_budgets payload must never appear verbatim + // in the prompt text -- only its issue number and the fixed jq idiom may. + assert.ok(prompt.indexOf(JSON.stringify(intent)) === -1, 'prompt must never embed the intent payload') + assert.ok(prompt.indexOf('"schema"') === -1, 'prompt must never embed payload JSON keys') + assert.ok(prompt.indexOf('gh issue view --repo ' + REPO) !== -1, 'prompt must carry the pinned jq idiom') + assert.ok(prompt.indexOf('Issues in this call: 9') !== -1, 'prompt must carry the issue number') +}) + +test('verifyGateState: nothing to verify (every result no-intent/post-failed) makes zero agent calls', async function () { + const context = harness.boot() + seed(context) + const agentStub = harness.installScriptedAgent(context, function () { return { rows: [] } }) + + const results = [ + makeResult(context, 1), + makeResult(context, 2, { gate_state_post_failed: 'plan' }), + null, // runPool can leave holes-shaped defensively -- must never throw + ] + await context.verifyGateState(results) + + assert.strictEqual(agentStub.calls.length, 0) +}) + +// ---- a dead sweep call site never disturbs unrelated Report-phase work ---- +// (verifyGateState itself is exercised above; the Report-phase call site wraps +// it in try/catch -- see workflows/ticketmill.js's phase('Report') section -- +// so a throw inside this function cannot be reached from outside it without +// reconstructing the whole Report phase. What matters at this layer is that +// verifyGateState() itself never throws even when handed maximally hostile +// input, which the `null` entry in the previous test, and the malformed rows +// below, already cover.) + +test('verifyGateState: a chunk response with a non-array `rows` field is treated the same as a dead chunk (read-failed), never throws', async function () { + const context = harness.boot() + seed(context) + const logs = logsOf(context) + const intent = intentFor(context, 10) + harness.installScriptedAgent(context, function () { return { rows: 'not-an-array' } }) + + const results = [makeResult(context, 10, { gate_state_intent: intent })] + await context.verifyGateState(results) + + assert.ok(outcomeLine(logs, 10).indexOf('read-failed') !== -1, outcomeLine(logs, 10)) +}) diff --git a/tests/gate-state.test.js b/tests/gate-state.test.js new file mode 100644 index 0000000..61139aa --- /dev/null +++ b/tests/gate-state.test.js @@ -0,0 +1,479 @@ +'use strict' + +// Unit tests for the durable per-issue gate-state substrate (issue #166, +// task 1): the pure build/parse/select/trust/epoch/diff layer above the +// TICKETMILL-TEST-HARNESS-SPLIT marker. Substrate only -- nothing here +// exercises a WRITE (postGateState, task 2). attachGateStateBlocks' real-data +// join and its interaction with fetchGateStateBlocks (task 3's Select-time +// wiring) are covered separately in tests/gate-state-read.test.js; only its +// bare defaulting shape (no join data at all) is proven here. +// +// Covers: build/parse round trip (incl. apostrophe/newline-bearing free +// text), every parseGateStateComment rejection path (malformed JSON, +// truncated fence, wrong issue/repo/schema, a comment that merely quotes the +// shape inside a larger body, marker-not-last), parseGateStateProbeRow on +// truncated/non-JSON stdout, all four selectGateState states (including both +// falsifiable-absent branches), positional trust-before-last-wins selection +// (an older trusted block beating an untrusted newer one, with `skipped` +// counted), isTrustedGateStateAuthor's self_login/claim_authors rules, +// staleness (both the epoch-guard and the CLAIM_STALE_SECONDS threshold), and +// all three diffGateStateIntent verdicts. + +const test = require('node:test') +const assert = require('node:assert/strict') +const harness = require('./harness') + +const REPO = 'aaddrick/ticketmill-fixture' + +// ---- buildGateStatePayload / buildGateStateComment / parseGateStateComment ---- + +test('build/parse round trip: buildGateStatePayload -> buildGateStateComment -> parseGateStateComment reproduces the payload exactly', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ + repo: REPO, issue: 42, run: 'run-1', batch: 'dev', epoch: 1700000000000, + boundary: 'plan', group_id: null, members: [42], seeded_from: null, + gate_budgets: { approach: 1, plan: 2 }, + settled: [{ topic: 'topic A', gate: 'plan', decision: 'decided', why: 'evidence', rejected: ['alt 1'] }], + }) + const body = context.buildGateStateComment(REPO, 42, payload) + const parsed = context.parseGateStateComment(body, REPO, 42) + + assert.notStrictEqual(parsed, null) + harness.assertVmEqual(parsed, harness.normalize(payload)) + // title-gated, marker as last line, human line present and not the payload itself + assert.strictEqual(body.split('\n')[0], '## Gate State') + assert.strictEqual(body.trim().split('\n').pop(), '') +}) + +test('build/parse round trip preserves apostrophes and embedded newlines in settled free text (JSON, unlike the flat-line consolidation format, does not need oneLine())', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ + repo: REPO, issue: 1, run: 'run-1', batch: 'dev', epoch: 1, boundary: 'approach', + settled: [{ + topic: "don't re-litigate this", + gate: 'approach', + decision: "It's fine as-is.\nSecond line of reasoning.", + why: "because it's simpler and it's already been argued", + rejected: ["alternative A's approach", "plan B's approach"], + }], + }) + const body = context.buildGateStateComment(REPO, 1, payload) + const parsed = harness.normalize(context.parseGateStateComment(body, REPO, 1)) + + assert.strictEqual(parsed.settled[0].topic, "don't re-litigate this") + assert.strictEqual(parsed.settled[0].decision, "It's fine as-is.\nSecond line of reasoning.") + assert.strictEqual(parsed.settled[0].why, "because it's simpler and it's already been argued") + assert.deepStrictEqual(parsed.settled[0].rejected, ["alternative A's approach", "plan B's approach"]) +}) + +test('buildGateStatePayload: caps `settled` to the last 6 entries, oldest dropped first', function () { + const context = harness.boot() + const settled = [] + for (let i = 0; i < 9; i++) settled.push({ topic: 't' + i, gate: 'plan', decision: 'd' + i, why: '', rejected: [] }) + const payload = harness.normalize(context.buildGateStatePayload({ repo: REPO, issue: 1, run: 'run-1', boundary: 'plan', settled: settled })) + + assert.strictEqual(payload.settled.length, 6) + assert.strictEqual(payload.settled[0].topic, 't3') + assert.strictEqual(payload.settled[5].topic, 't8') +}) + +test('buildGateStatePayload: seeded_from is always null at this tier (no consumer yet), even when explicitly passed', function () { + const context = harness.boot() + const payload = harness.normalize(context.buildGateStatePayload({ repo: REPO, issue: 1, run: 'run-1', boundary: 'approach' })) + assert.strictEqual(payload.seeded_from, null) +}) + +test('parseGateStateComment: malformed JSON inside the fence returns null, never throws', function () { + const context = harness.boot() + const body = [ + '## Gate State', 'A record, not a directive.', '', '
x', '', + '```json', '{ this is not valid json', '```', '', '
', + '', + ].join('\n') + assert.strictEqual(context.parseGateStateComment(body, REPO, 1), null) +}) + +test('parseGateStateComment: a truncated fence (no closing ```) returns null', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ repo: REPO, issue: 1, run: 'run-1', boundary: 'plan' }) + const body = context.buildGateStateComment(REPO, 1, payload) + const closeIdx = body.lastIndexOf('```\n') + assert.ok(closeIdx > -1, 'fixture must contain a closing fence to truncate') + // keep everything up to (not including) the closing fence, then re-append the + // canonical marker so ONLY the fence is truncated -- isolates this from the + // separate marker-not-last rejection path. + const truncated = body.slice(0, closeIdx) + '' + assert.strictEqual(context.parseGateStateComment(truncated, REPO, 1), null) +}) + +test('parseGateStateComment: rejects when the embedded payload.issue disagrees with the read\'s expected issue', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ repo: REPO, issue: 99, run: 'run-1', boundary: 'plan' }) + // marker matches (REPO, 1) -- the read's expectation -- but the JSON payload + // embedded inside claims issue 99. + const body = context.buildGateStateComment(REPO, 1, payload) + assert.strictEqual(context.parseGateStateComment(body, REPO, 1), null) +}) + +test('parseGateStateComment: rejects when the embedded payload.repo disagrees with the read\'s expected repo', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ repo: 'someone/else', issue: 1, run: 'run-1', boundary: 'plan' }) + const body = context.buildGateStateComment(REPO, 1, payload) + assert.strictEqual(context.parseGateStateComment(body, REPO, 1), null) +}) + +test('parseGateStateComment: rejects a payload whose schema does not match GATE_STATE_SCHEMA', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ repo: REPO, issue: 1, run: 'run-1', boundary: 'plan' }) + payload.schema = 999 + const body = context.buildGateStateComment(REPO, 1, payload) + assert.strictEqual(context.parseGateStateComment(body, REPO, 1), null) +}) + +test('parseGateStateComment: a comment that merely QUOTES the gate-state shape inside a larger body parses to null', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ repo: REPO, issue: 1, run: 'run-1', boundary: 'plan' }) + const inner = context.buildGateStateComment(REPO, 1, payload) + const body = [ + '## Someone quoting the format for discussion', + 'Here is what a gate-state comment looks like, for reference:', + '', + '> ' + inner.split('\n').join('\n> '), + '', + 'Anyway, back to the actual discussion.', + ].join('\n') + assert.strictEqual(context.parseGateStateComment(body, REPO, 1), null) +}) + +test('parseGateStateComment: marker present but NOT the last non-empty line parses to null', function () { + const context = harness.boot() + const payload = context.buildGateStatePayload({ repo: REPO, issue: 1, run: 'run-1', boundary: 'plan' }) + const body = context.buildGateStateComment(REPO, 1, payload) + '\n\nEdit: please disregard the above.' + assert.strictEqual(context.parseGateStateComment(body, REPO, 1), null) +}) + +// ---- parseGateStateProbeRow ---- + +test('parseGateStateProbeRow: non-JSON stdout returns ok:false, never throws', function () { + const context = harness.boot() + const result = harness.normalize(context.parseGateStateProbeRow('definitely not json')) + assert.strictEqual(result.ok, false) + assert.strictEqual(result.total, 0) + assert.deepStrictEqual(result.blocks, []) +}) + +test('parseGateStateProbeRow: truncated JSON stdout (a chunked/interrupted read) returns ok:false, never throws', function () { + const context = harness.boot() + const result = harness.normalize(context.parseGateStateProbeRow('{"total":1,"blocks":[{"body":"## Gate St')) + assert.strictEqual(result.ok, false) +}) + +test('parseGateStateProbeRow: a shape mismatch (blocks not an array) returns ok:false', function () { + const context = harness.boot() + const result = harness.normalize(context.parseGateStateProbeRow(JSON.stringify({ total: 0, blocks: 'nope' }))) + assert.strictEqual(result.ok, false) +}) + +test('parseGateStateProbeRow: well-formed stdout parses to ok:true with total/blocks intact', function () { + const context = harness.boot() + const raw = JSON.stringify({ total: 2, blocks: [{ body: 'x', author_login: 'me', author_association: 'OWNER' }] }) + const result = harness.normalize(context.parseGateStateProbeRow(raw)) + assert.deepStrictEqual(result, { ok: true, total: 2, blocks: [{ body: 'x', author_login: 'me', author_association: 'OWNER' }] }) +}) + +// ---- gateStateProbeCommandLine: the pinned jq idiom ---- + +test('gateStateProbeCommandLine: `total` is counted by the SAME title-gated filter `blocks` uses, never a bare all-comments count', function () { + const context = harness.boot() + const line = context.gateStateProbeCommandLine() + // A bare `.comments|length` would count EVERY comment on the issue, not + // just gate-state ones -- making `blocks.length === 0 && total > 0` + // reachable on any issue that has received so much as one unrelated human + // or bot comment, which selectGateState's self-contradiction check treats + // as read-failed, silently swallowing the `absent` state for the common + // case (issue #166 PR #177 review). + assert.ok(line.indexOf('.comments|length') === -1, 'total must not be a bare unfiltered comment count: ' + line) + const titleFilterCount = (line.match(/select\(\.body \| startswith\("## Gate State"\)\)/g) || []).length + assert.strictEqual(titleFilterCount, 2, 'the gate-state title filter must back BOTH total and blocks: ' + line) +}) + +// ---- selectGateState: four states ---- + +test('selectGateState: found / absent / malformed / read-failed are all distinguishable', function () { + const context = harness.boot() + const issue = 1 + const payload = context.buildGateStatePayload({ repo: REPO, issue: issue, run: 'run-1', boundary: 'plan', epoch: 1000 }) + const body = context.buildGateStateComment(REPO, issue, payload) + + const found = context.selectGateState( + { ok: true, total: 1, blocks: [{ body: body, author_login: 'bot' }] }, + { repo: REPO, issue: issue, self_login: 'bot', run_epoch: 1000 }, + {}, + ) + assert.strictEqual(found.state, 'found') + + const absent = context.selectGateState( + { ok: true, total: 0, blocks: [] }, + { repo: REPO, issue: issue, self_login: 'bot' }, + { pr_number: null, worktree_exists: false, resume_point: 'implement' }, + ) + assert.strictEqual(absent.state, 'absent') + + const malformed = context.selectGateState( + { ok: true, total: 1, blocks: [{ body: 'not a gate-state comment at all', author_login: 'bot' }] }, + { repo: REPO, issue: issue, self_login: 'bot' }, + {}, + ) + assert.strictEqual(malformed.state, 'malformed') + + const readFailedExplicit = context.selectGateState({ ok: false, total: 0, blocks: [] }, { repo: REPO, issue: issue }, {}) + assert.strictEqual(readFailedExplicit.state, 'read-failed') + + const readFailedExitNotOk = context.selectGateState({ exit_ok: false, ok: true, total: 0, blocks: [] }, { repo: REPO, issue: issue }, {}) + assert.strictEqual(readFailedExitNotOk.state, 'read-failed') + + const states = [found.state, absent.state, malformed.state, readFailedExplicit.state] + assert.strictEqual(new Set(states).size, 4, 'all four states must be pairwise distinct') +}) + +test('selectGateState: falsifiable-absent rule -- zero blocks WITH prior-work evidence (pr_number) is read-failed, never absent', function () { + const context = harness.boot() + const result = context.selectGateState( + { ok: true, total: 0, blocks: [] }, + { repo: REPO, issue: 1 }, + { pr_number: 7, worktree_exists: false, resume_point: 'implement' }, + ) + assert.strictEqual(result.state, 'read-failed') +}) + +test('selectGateState: falsifiable-absent rule -- zero blocks with NO prior-work evidence stays absent', function () { + const context = harness.boot() + const result = context.selectGateState( + { ok: true, total: 0, blocks: [] }, + { repo: REPO, issue: 1 }, + { pr_number: null, worktree_exists: false, resume_point: 'implement' }, + ) + assert.strictEqual(result.state, 'absent') +}) + +test('selectGateState: zero blocks but total>0 is read-failed even with NO prior-work evidence -- a self-contradictory probe result is never absence', function () { + const context = harness.boot() + const result = context.selectGateState( + { ok: true, total: 3, blocks: [] }, + { repo: REPO, issue: 1 }, + { pr_number: null, worktree_exists: false, resume_point: 'implement' }, + ) + assert.strictEqual(result.state, 'read-failed') +}) + +test('selectGateState: falsifiable-absent rule also fires on worktree_exists / a non-implement resume_point alone', function () { + const context = harness.boot() + const byWorktree = context.selectGateState( + { ok: true, total: 0, blocks: [] }, { repo: REPO, issue: 1 }, + { pr_number: null, worktree_exists: true, resume_point: 'implement' }, + ) + assert.strictEqual(byWorktree.state, 'read-failed') + + const byResumePoint = context.selectGateState( + { ok: true, total: 0, blocks: [] }, { repo: REPO, issue: 1 }, + { pr_number: null, worktree_exists: false, resume_point: 'process_pr' }, + ) + assert.strictEqual(byResumePoint.state, 'read-failed') +}) + +// ---- selectGateState: positional last-wins + trust-before-last-wins ---- + +test('selectGateState: positional last-wins across three trusted blocks -- the newest wins, zero skipped', function () { + const context = harness.boot() + const issue = 1 + function comment(boundary, epoch) { + return context.buildGateStateComment(REPO, issue, context.buildGateStatePayload({ repo: REPO, issue: issue, run: 'run-1', boundary: boundary, epoch: epoch })) + } + const result = context.selectGateState( + { + ok: true, total: 3, + blocks: [ + { body: comment('approach', 1000), author_login: 'bot' }, + { body: comment('plan', 2000), author_login: 'bot' }, + { body: comment('pr-review-i1', 3000), author_login: 'bot' }, + ], + }, + { repo: REPO, issue: issue, self_login: 'bot', run_epoch: 3000 }, + {}, + ) + assert.strictEqual(result.state, 'found') + assert.strictEqual(harness.normalize(result.payload).boundary, 'pr-review-i1') + assert.strictEqual(result.skipped, 0) +}) + +test('selectGateState: an older TRUSTED block is selected over an untrusted newest one, with `skipped` counted', function () { + const context = harness.boot() + const issue = 1 + function comment(run, epoch) { + return context.buildGateStateComment(REPO, issue, context.buildGateStatePayload({ repo: REPO, issue: issue, run: run, boundary: 'plan', epoch: epoch })) + } + const result = context.selectGateState( + { + ok: true, total: 3, + blocks: [ + { body: comment('run-trusted', 1000), author_login: 'me' }, + { body: comment('run-stranger-a', 2000), author_login: 'stranger' }, + { body: comment('run-stranger-b', 3000), author_login: 'stranger' }, + ], + }, + { repo: REPO, issue: issue, self_login: 'me', run_epoch: 3000 }, + {}, + ) + assert.strictEqual(result.state, 'found') + assert.strictEqual(result.trusted, true) + assert.strictEqual(result.skipped, 2) + assert.strictEqual(harness.normalize(result.payload).run, 'run-trusted') +}) + +test('selectGateState: the degenerate all-untrusted case still returns found (there IS data), with trusted:false and skipped:0', function () { + const context = harness.boot() + const issue = 1 + function comment(run, epoch) { + return context.buildGateStateComment(REPO, issue, context.buildGateStatePayload({ repo: REPO, issue: issue, run: run, boundary: 'plan', epoch: epoch })) + } + const result = context.selectGateState( + { + ok: true, total: 2, + blocks: [ + { body: comment('run-a', 1000), author_login: 'stranger-1' }, + { body: comment('run-b', 2000), author_login: 'stranger-2' }, + ], + }, + { repo: REPO, issue: issue, self_login: 'me', run_epoch: 2000 }, + {}, + ) + assert.strictEqual(result.state, 'found') + assert.strictEqual(result.trusted, false) + assert.strictEqual(result.skipped, 0) + assert.strictEqual(harness.normalize(result.payload).run, 'run-b') // newest parseable, fallback +}) + +// ---- isTrustedGateStateAuthor ---- + +test('isTrustedGateStateAuthor: self_login match trusts regardless of claim_authors', function () { + const context = harness.boot() + assert.strictEqual(context.isTrustedGateStateAuthor('me', 'me', [], 'dev'), true) +}) + +test('isTrustedGateStateAuthor: no login is never trusted', function () { + const context = harness.boot() + assert.strictEqual(context.isTrustedGateStateAuthor('', 'me', [], 'dev'), false) + assert.strictEqual(context.isTrustedGateStateAuthor(null, 'me', [], 'dev'), false) +}) + +test('isTrustedGateStateAuthor: a claim author that is neither fresh nor batch-matching is untrusted', function () { + const context = harness.boot() + const staleSeconds = harness.readGlobal(context, 'CLAIM_STALE_SECONDS') + const claimAuthors = [{ login: 'someone', ageSeconds: staleSeconds + 1, batch: 'other-branch' }] + assert.strictEqual(context.isTrustedGateStateAuthor('someone', null, claimAuthors, 'dev'), false) +}) + +test('isTrustedGateStateAuthor: a fresh claim (age < CLAIM_STALE_SECONDS) is trusted even off-batch', function () { + const context = harness.boot() + const claimAuthors = [{ login: 'someone', ageSeconds: 10, batch: 'other-branch' }] + assert.strictEqual(context.isTrustedGateStateAuthor('someone', null, claimAuthors, 'dev'), true) +}) + +test('isTrustedGateStateAuthor: a batch-matching claim is trusted even when stale (a stale forged claim off-batch is not)', function () { + const context = harness.boot() + const staleSeconds = harness.readGlobal(context, 'CLAIM_STALE_SECONDS') + const claimAuthors = [{ login: 'someone', ageSeconds: staleSeconds + 1, batch: 'dev' }] + assert.strictEqual(context.isTrustedGateStateAuthor('someone', null, claimAuthors, 'dev'), true) + + const staleOffBatch = [{ login: 'someone', ageSeconds: staleSeconds + 1, batch: 'other-branch' }] + assert.strictEqual(context.isTrustedGateStateAuthor('someone', null, staleOffBatch, 'dev'), false) +}) + +// ---- epoch / staleness ---- + +test('deriveRunEpoch: parses a probe-returned `date -u` ISO string into epoch ms', function () { + const context = harness.boot() + assert.strictEqual(context.deriveRunEpoch('2024-01-01T00:00:00Z'), Date.parse('2024-01-01T00:00:00Z')) +}) + +test('deriveRunEpoch: unparseable/absent input returns explicit null, never NaN', function () { + const context = harness.boot() + assert.strictEqual(context.deriveRunEpoch('not a date'), null) + assert.strictEqual(context.deriveRunEpoch(undefined), null) + assert.strictEqual(context.deriveRunEpoch(null), null) +}) + +test('gateStateEpochStale: epoch guards -- a null run epoch or a null/missing payload epoch reads as stale, never fresh', function () { + const context = harness.boot() + assert.strictEqual(context.gateStateEpochStale({ epoch: 1000 }, null), true) + assert.strictEqual(context.gateStateEpochStale({}, 5000), true) + assert.strictEqual(context.gateStateEpochStale(null, 5000), true) +}) + +test('gateStateEpochStale / selectGateState: stale flips true once age exceeds CLAIM_STALE_SECONDS', function () { + const context = harness.boot() + const staleSeconds = harness.readGlobal(context, 'CLAIM_STALE_SECONDS') + + assert.strictEqual(context.gateStateEpochStale({ epoch: 0 }, staleSeconds * 1000), false) // exactly at the boundary: not yet stale + assert.strictEqual(context.gateStateEpochStale({ epoch: 0 }, staleSeconds * 1000 + 1), true) // one ms past it: stale + + const issue = 1 + const payload = context.buildGateStatePayload({ repo: REPO, issue: issue, run: 'run-1', boundary: 'plan', epoch: 0 }) + const body = context.buildGateStateComment(REPO, issue, payload) + const result = context.selectGateState( + { ok: true, total: 1, blocks: [{ body: body, author_login: 'me' }] }, + { repo: REPO, issue: issue, self_login: 'me', run_epoch: staleSeconds * 1000 + 1 }, + {}, + ) + assert.strictEqual(result.state, 'found') + assert.strictEqual(result.stale, true) +}) + +// ---- diffGateStateIntent ---- + +test('diffGateStateIntent: verdicts are keyed off write_seq (a monotonic per-run write counter), never epoch -- match, superseded, mismatch', function () { + const context = harness.boot() + const intent = { schema: 1, repo: REPO, issue: 1, run: 'run-1', batch: 'dev', epoch: 1000, write_seq: 1, boundary: 'plan', group_id: null, members: [1], seeded_from: null, gate_budgets: {}, settled: [] } + + const identical = Object.assign({}, intent) + assert.strictEqual(context.diffGateStateIntent(intent, identical), 'match') + + const laterSameRun = Object.assign({}, intent, { boundary: 'pr-review-i1', write_seq: 2 }) + assert.strictEqual(context.diffGateStateIntent(intent, laterSameRun), 'superseded') + + // Same run, LATER epoch but the SAME write_seq -- epoch alone must never + // establish ordering. In production every boundary in a single run shares + // one RUN_EPOCH; only write_seq varies write to write. + const laterEpochSameSeq = Object.assign({}, intent, { epoch: 5000 }) + assert.strictEqual(context.diffGateStateIntent(intent, laterEpochSameSeq), 'mismatch') + + const differentRun = Object.assign({}, intent, { run: 'run-2', write_seq: 2 }) + assert.strictEqual(context.diffGateStateIntent(intent, differentRun), 'mismatch') + + const earlierSameRun = Object.assign({}, intent, { write_seq: 0 }) + assert.strictEqual(context.diffGateStateIntent(intent, earlierSameRun), 'mismatch') + + assert.strictEqual(context.diffGateStateIntent(null, identical), 'mismatch') + assert.strictEqual(context.diffGateStateIntent(intent, null), 'mismatch') +}) + +// ---- attachGateStateBlocks (bare defaulting shape only -- see +// tests/gate-state-read.test.js for the real rowsByIssue join, the +// hallucination-clobbering behavior, and fetchGateStateBlocks itself) ---- + +test('attachGateStateBlocks: with no join data at all, writes all four gate-state fields unconditionally to their fail-open defaults, without mutating the input', function () { + const context = harness.boot() + const preflights = [ + { issue: 1 }, + // even a preflight that already carries these fields (e.g. a hallucinating + // agent) gets them fully overridden -- these four facts are NEVER read back + // off the preflight's own pre-existing value, only from real join data. + { issue: 2, gate_state_blocks: ['x'], gate_state_read_ok: true, gate_state_total_comments: 5, gate_state_trust: 'primary' }, + ] + const attached = harness.normalize(context.attachGateStateBlocks(preflights)) + + assert.deepStrictEqual(attached[0], { issue: 1, gate_state_blocks: [], gate_state_read_ok: false, gate_state_total_comments: 0, gate_state_trust: '' }) + assert.deepStrictEqual(attached[1], { issue: 2, gate_state_blocks: [], gate_state_read_ok: false, gate_state_total_comments: 0, gate_state_trust: '' }) + // non-mutating: the original host-realm objects never gained the new keys + assert.strictEqual(Object.prototype.hasOwnProperty.call(preflights[0], 'gate_state_blocks'), false) +}) diff --git a/tests/pr-review-gate.test.js b/tests/pr-review-gate.test.js index f47296c..7bf41b1 100644 --- a/tests/pr-review-gate.test.js +++ b/tests/pr-review-gate.test.js @@ -66,6 +66,7 @@ const MERGE_OK = { status: 'merged', follow_up_issues: [], error: null } const FIX_OK = { status: 'success', commit: 'deadbeef', files_changed: [], fixes_applied: ['addressed review feedback'], summary: 'fixed', error: null } const SIMPLIFY_OK = { status: 'success', commit: null, files_changed: [], summary: 'nothing to simplify' } const QUALITY_REVIEW_APPROVED = { result: 'approved', comments: '', issues: [], recommended_fix_agent: null, summary: 'clean' } +const GATE_STATE_POSTED = { posted: true } test('reviewAndMerge(): a clean pr-review approval on iteration 1 records an "accepted" disposition in ctx.gate_findings["pr-review"], tallying both reviewers\' issues', async function () { const context = harness.boot() @@ -76,6 +77,7 @@ test('reviewAndMerge(): a clean pr-review approval on iteration 1 records an "ac // 'approved' can still carry nit-level issues — the disposition is driven // by .result alone, not by issues being empty. 'code-review-i1': Object.assign({}, APPROVED_REVIEW, { issues: [{ severity: 'minor', summary: 'nit: naming' }] }), + 'gate-state-pr-review-i1': GATE_STATE_POSTED, 'changed-files-probe': CHANGED_FILES_PROBE_OK, merge: MERGE_OK, }) @@ -105,12 +107,14 @@ test('reviewAndMerge(): a changes-requested iteration followed by a clean approv // ---- iteration 1: code review requests changes (spec is fine) ---- 'spec-review-i1': APPROVED_REVIEW, 'code-review-i1': { result: 'changes_requested', comments: 'tighten error handling', issues: [{ severity: 'major', summary: 'unhandled rejection' }], recommended_fix_agent: null, summary: 'needs a fix' }, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, 'pr-fix-i1': FIX_OK, 'simplify-pr-fix-i1-i1': SIMPLIFY_OK, 'quality-review-pr-fix-i1-i1': QUALITY_REVIEW_APPROVED, // ---- iteration 2: both approve ---- 'spec-review-i2': APPROVED_REVIEW, 'code-review-i2': APPROVED_REVIEW, + 'gate-state-pr-review-i2': GATE_STATE_POSTED, // pr-fix-i1 (FIX_OK) posted a commit, so reviewAndMerge()'s commit-sha // probe (issue #79, Layer 2) dispatches once for the whole issue, before // changed-files-probe — see the probeCommitShas() call site. @@ -131,10 +135,13 @@ test('reviewAndMerge(): a changes-requested iteration followed by a clean approv // Accumulated, not overwritten: one 're-litigated' (iter 1) + one 'accepted' (iter 2). assert.deepStrictEqual(JSON.parse(JSON.stringify(g.disposition)), { 're-litigated': 1, accepted: 1 }) + // One gate-state stage per pr-review iteration (issue #166 task 2), posted + // right after each iteration's recordGateOutcome — in this scenario that's + // exactly two: 'gate-state-pr-review-i1' then 'gate-state-pr-review-i2'. const keys = context.agent.calls.map(stageKeyOf) assert.deepStrictEqual(keys, [ - 'spec-review-i1', 'code-review-i1', 'pr-fix-i1', 'simplify-pr-fix-i1-i1', 'quality-review-pr-fix-i1-i1', - 'spec-review-i2', 'code-review-i2', 'commit-sha-probe', 'changed-files-probe', 'merge', + 'spec-review-i1', 'code-review-i1', 'gate-state-pr-review-i1', 'pr-fix-i1', 'simplify-pr-fix-i1-i1', 'quality-review-pr-fix-i1-i1', + 'spec-review-i2', 'code-review-i2', 'gate-state-pr-review-i2', 'commit-sha-probe', 'changed-files-probe', 'merge', ]) }) @@ -147,20 +154,24 @@ test('reviewAndMerge(): exhausting MAX_PR_REVIEW_ITERATIONS without approval rec installScriptedResponder(context, { 'spec-review-i1': APPROVED_REVIEW, 'code-review-i1': CHANGES_REQUESTED, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, 'pr-fix-i1': FIX_OK, 'simplify-pr-fix-i1-i1': SIMPLIFY_OK, 'quality-review-pr-fix-i1-i1': QUALITY_REVIEW_APPROVED, 'spec-review-i2': APPROVED_REVIEW, 'code-review-i2': CHANGES_REQUESTED, + 'gate-state-pr-review-i2': GATE_STATE_POSTED, 'pr-fix-i2': FIX_OK, 'simplify-pr-fix-i2-i1': SIMPLIFY_OK, 'quality-review-pr-fix-i2-i1': QUALITY_REVIEW_APPROVED, - // Final iteration: the loop records the outcome and breaks WITHOUT running - // a pr-fix/quality-loop stage (see reviewAndMerge()'s - // `if (iter === MAX_PR_REVIEW_ITERATIONS) break` right after - // recordGateOutcome) — deliberately left unscripted below to prove that. + // Final iteration: the loop records the outcome (and its gate-state post) + // and breaks WITHOUT running a pr-fix/quality-loop stage (see + // reviewAndMerge()'s `if (capReached) break` right after + // recordGateOutcome/postGateState) — pr-fix-i3 etc. are deliberately left + // unscripted below to prove that. 'spec-review-i3': APPROVED_REVIEW, 'code-review-i3': CHANGES_REQUESTED, + 'gate-state-pr-review-i3': GATE_STATE_POSTED, }) const ctx = harness.makeCtx({ issue: 32, pr: 320 }) @@ -200,6 +211,7 @@ test('reviewAndMerge(): both reviewers changes_requested with issues:[] breaks e installScriptedResponder(context, { 'spec-review-i1': EMPTY_CHANGES_REQUESTED, 'code-review-i1': EMPTY_CHANGES_REQUESTED, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, // pr-fix-i1/merge deliberately unscripted below — proving neither runs. }) @@ -215,10 +227,12 @@ test('reviewAndMerge(): both reviewers changes_requested with issues:[] breaks e assert.strictEqual(g.count, 0) assert.deepStrictEqual(JSON.parse(JSON.stringify(g.disposition)), { 'carried-unresolved': 1 }) - // fail() posts a best-effort halt note (halt-note-pr-review) — the only other - // stage that runs alongside the two reviews; pr-fix-i1 and merge must not. + // The gate-state post runs INSIDE the loop, right after recordGateOutcome — + // before the bothNothingToFix early break — so it fires here too, even + // though pr-fix/merge never run. fail() posts a best-effort halt note + // (halt-note-pr-review) after that. const keys = context.agent.calls.map(stageKeyOf) - assert.deepStrictEqual(keys, ['spec-review-i1', 'code-review-i1', 'halt-note-pr-review']) + assert.deepStrictEqual(keys, ['spec-review-i1', 'code-review-i1', 'gate-state-pr-review-i1', 'halt-note-pr-review']) for (const shouldNotRun of ['pr-fix-i1', 'merge']) { assert.ok(!keys.includes(shouldNotRun), 'stage "' + shouldNotRun + '" must not run; ran: ' + keys.join(', ')) } @@ -246,6 +260,7 @@ test('reviewAndMerge(): spec approved-with-a-nit + code changes_requested-with-i installScriptedResponder(context, { 'spec-review-i1': APPROVED_WITH_NIT, 'code-review-i1': EMPTY_CHANGES_REQUESTED, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, // pr-fix-i1/merge deliberately unscripted below — proving neither runs. }) @@ -264,7 +279,7 @@ test('reviewAndMerge(): spec approved-with-a-nit + code changes_requested-with-i assert.deepStrictEqual(JSON.parse(JSON.stringify(g.disposition)), { 'carried-unresolved': 1 }) const keys = context.agent.calls.map(stageKeyOf) - assert.deepStrictEqual(keys, ['spec-review-i1', 'code-review-i1', 'halt-note-pr-review']) + assert.deepStrictEqual(keys, ['spec-review-i1', 'code-review-i1', 'gate-state-pr-review-i1', 'halt-note-pr-review']) for (const shouldNotRun of ['pr-fix-i1', 'merge']) { assert.ok(!keys.includes(shouldNotRun), 'stage "' + shouldNotRun + '" must not run; ran: ' + keys.join(', ')) } @@ -280,11 +295,13 @@ test('reviewAndMerge(): one reviewer with issues:[] and the other with real find installScriptedResponder(context, { 'spec-review-i1': EMPTY_CHANGES_REQUESTED, 'code-review-i1': CONCRETE_FINDING, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, 'pr-fix-i1': FIX_OK, 'simplify-pr-fix-i1-i1': SIMPLIFY_OK, 'quality-review-pr-fix-i1-i1': QUALITY_REVIEW_APPROVED, 'spec-review-i2': APPROVED_REVIEW, 'code-review-i2': APPROVED_REVIEW, + 'gate-state-pr-review-i2': GATE_STATE_POSTED, 'commit-sha-probe': COMMIT_PROBE_OK, 'changed-files-probe': CHANGED_FILES_PROBE_OK, merge: MERGE_OK, @@ -322,11 +339,13 @@ test('reviewAndMerge(): both reviewers changes_requested with `issues` omitted e installScriptedResponder(context, { 'spec-review-i1': OMITTED_ISSUES, 'code-review-i1': OMITTED_ISSUES, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, 'pr-fix-i1': FIX_OK, 'simplify-pr-fix-i1-i1': SIMPLIFY_OK, 'quality-review-pr-fix-i1-i1': QUALITY_REVIEW_APPROVED, 'spec-review-i2': APPROVED_REVIEW, 'code-review-i2': APPROVED_REVIEW, + 'gate-state-pr-review-i2': GATE_STATE_POSTED, 'commit-sha-probe': COMMIT_PROBE_OK, 'changed-files-probe': CHANGED_FILES_PROBE_OK, merge: MERGE_OK, @@ -356,6 +375,7 @@ test('reviewAndMerge(): a null spec reviewer fails needs_human at stage "pr-revi installScriptedResponder(context, { 'spec-review-i1': null, 'code-review-i1': APPROVED_REVIEW, + 'gate-state-pr-review-i1-aborted': GATE_STATE_POSTED, }) const ctx = harness.makeCtx({ issue: 36, pr: 360 }) @@ -365,6 +385,12 @@ test('reviewAndMerge(): a null spec reviewer fails needs_human at stage "pr-revi assert.strictEqual(result.stage, 'pr-review') const keys = context.agent.calls.map(stageKeyOf) assert.ok(!keys.includes('merge'), 'merge must never run when the spec reviewer died; ran: ' + keys.join(', ')) + // A dead reviewer is exactly the "aborted" gate-state boundary (issue #166 + // task 2, site 4) — this is the resume-covering post, distinct from the + // in-loop 'gate-state-pr-review-i1' post that only fires after a clean + // spec+code pair. + assert.ok(keys.includes('gate-state-pr-review-i1-aborted'), 'expected the aborted gate-state boundary to post; ran: ' + keys.join(', ')) + assert.ok(!keys.includes('gate-state-pr-review-i1'), 'the non-aborted in-loop boundary must not also fire; ran: ' + keys.join(', ')) }) test('reviewAndMerge(): a null code reviewer fails needs_human at stage "pr-review" without ever running merge', async function () { @@ -374,6 +400,7 @@ test('reviewAndMerge(): a null code reviewer fails needs_human at stage "pr-revi installScriptedResponder(context, { 'spec-review-i1': APPROVED_REVIEW, 'code-review-i1': null, + 'gate-state-pr-review-i1-aborted': GATE_STATE_POSTED, }) const ctx = harness.makeCtx({ issue: 37, pr: 370 }) @@ -383,6 +410,7 @@ test('reviewAndMerge(): a null code reviewer fails needs_human at stage "pr-revi assert.strictEqual(result.stage, 'pr-review') const keys = context.agent.calls.map(stageKeyOf) assert.ok(!keys.includes('merge'), 'merge must never run when the code reviewer died; ran: ' + keys.join(', ')) + assert.ok(keys.includes('gate-state-pr-review-i1-aborted'), 'expected the aborted gate-state boundary to post; ran: ' + keys.join(', ')) }) // ---- issue #162 task 2: severity is no longer permanently zero ---- @@ -399,11 +427,13 @@ test('reviewAndMerge(): a typed mixed-severity issues array makes gate_findings[ 'code-review-i1': { result: 'changes_requested', comments: '', issues: [ { severity: 'major', summary: 'missing input validation', recommendation: 'validate before use' }, ], recommended_fix_agent: null, summary: 'one code finding' }, + 'gate-state-pr-review-i1': GATE_STATE_POSTED, 'pr-fix-i1': FIX_OK, 'simplify-pr-fix-i1-i1': SIMPLIFY_OK, 'quality-review-pr-fix-i1-i1': QUALITY_REVIEW_APPROVED, 'spec-review-i2': APPROVED_REVIEW, 'code-review-i2': APPROVED_REVIEW, + 'gate-state-pr-review-i2': GATE_STATE_POSTED, 'commit-sha-probe': COMMIT_PROBE_OK, 'changed-files-probe': CHANGED_FILES_PROBE_OK, merge: MERGE_OK, diff --git a/workflows/ticketmill.js b/workflows/ticketmill.js index 94ad6ae..a725de3 100644 --- a/workflows/ticketmill.js +++ b/workflows/ticketmill.js @@ -318,6 +318,12 @@ const MAX_TOUCH_FILES = 100 // comment already sets for gating efficiency metrics; not a correctness input, // only a display/trust-flag threshold. const MAX_RECONCILE_ERROR_FOR_TRUST = 0.05 +// gate-state read (issue #166 task 3): fetchGateStateBlocks issues ONE agent +// call per chunk of at most this many issues (belt-and-braces — a dead chunk's +// agent call only takes its own chunk's issues down with it; surviving chunks +// still report). Not a correctness input: the per-issue jq command inside a +// chunk is fully independent of every other issue in it. +const MAX_GATE_STATE_PROBE_CHUNK = 5 // churn analytics (issue #89): a file appearing in >= this many DISTINCT // issues' ctx.changed_files within one run is a cross-issue hotspot (computeChurn) — // 2 is the smallest number that actually means "more than one issue collided on @@ -408,6 +414,27 @@ let VERIFY_SKIPS = [] // human-visible verification gaps -> batch PR b // PROFILE.engine_owned_globs, and PROFILE.lockstep_installed_paths respectively. let ENGINE_OWNED = [] let LOCKSTEP_INSTALLED_PATHS = [] +// RUN_EPOCH (issue #166): populated at Select from a probe-returned `date -u` +// string via the pure deriveRunEpoch/toEpochMs (below the TICKETMILL-TEST- +// HARNESS-SPLIT marker) -- the sandbox has no Date.now()/argless `new Date()`, +// so this is the run's only wall-clock anchor. null until Select assigns it; +// selectGateState treats a null run epoch as unknown age, which reads as +// stale, never as fresh (see gateStateEpochStale). +let RUN_EPOCH = null +// GATE_STATE_WRITE_SEQ (issue #166 PR #177 review): a per-run, module-level +// monotonic write counter -- NOT a clock and NOT Date.now()/Math.random() (a +// plain incrementing int is deterministic across a resume/replay the same way +// every other module-level counter in this file is). RUN_EPOCH is assigned +// ONCE at Select and is therefore IDENTICAL across every boundary a run +// posts, so it cannot order two same-run writes against each other -- +// diffGateStateIntent's 'superseded' verdict needs something that varies +// write to write. postGateState() increments this once per call and embeds +// it on the payload as `write_seq`; call order is write order, since a given +// issue's boundaries always post sequentially within that issue's own await +// chain. Never reset mid-run. diffGateStateIntent still gates supersession on +// `intent.run === actual.run` first, since two payloads from different runs +// never share counter provenance. +let GATE_STATE_WRITE_SEQ = 0 function stageOpts(key) { const base = M[key] || { model: 'sonnet' } @@ -507,6 +534,20 @@ const PREFLIGHT_SCHEMA = { // and deriveUnits() for how they're threaded onto the unit shape. predicted_files: { type: 'array', items: { type: 'string' } }, depends_on: { type: 'array', items: { type: 'integer' } }, + // OPTIONAL gate-state read (issue #166, Task 3's fetchGateStateBlocks probe): + // fail-open the same way predicted_files/depends_on do -- attachGateStateBlocks + // (below the TICKETMILL-TEST-HARNESS-SPLIT marker) normalizes a preflight + // missing any of these four to their fail-open default so selectGateState never + // sees an undefined key. gate_state_blocks carries raw verbatim comment bodies + // (most recent last); gate_state_read_ok is whether the probe's gh call exited + // 0; gate_state_total_comments is the issue's gate-state comment count (the + // SAME title-gated filter `gate_state_blocks` uses, never every comment on + // the issue -- feeds the falsifiable-absent cross-check); gate_state_trust is set at Select from the + // probe's self_login, never by the agent itself. + gate_state_blocks: { type: 'array', items: { type: 'string' } }, + gate_state_read_ok: { type: 'boolean' }, + gate_state_total_comments: { type: 'integer' }, + gate_state_trust: { type: 'string' }, }, } const SETUP_SCHEMA = { @@ -961,6 +1002,479 @@ const CONSOLIDATION_MARKER_PROBE_SCHEMA = { }, } +// ============================================================================= +// GATE STATE (issue #166): durable per-issue gate/contrarian state carried on +// the issue itself across a run boundary. Substrate only in this tier -- no +// consumer reads/acts on it yet (see the design note on buildGateStatePayload's +// `seeded_from`). Mirrors the CONSOLIDATION_* marker subsystem immediately +// above end to end: a title-gated comment, fence-extracted payload, canonical +// scope-guard marker as its LAST line, append-only with positional last-wins +// (read: newest wins, exactly like the outcomes.jsonl/diffOutcomeGrades +// contract and the consolidation markers' own heal pass). Departs from that +// precedent in one place: the payload is fenced JSON, not consolidation's flat +// regex-parsed key:value lines, because `settled` (settleDecision/settledBlock +// above) is an array of five-field objects carrying free text that may itself +// contain newlines -- oneLine()'s single-line-per-field convention can't +// express that without lossy flattening, while JSON.stringify/JSON.parse +// round-trips it exactly, apostrophes and all. +// ============================================================================= + +// Comment title (first line) gating a gate-state marker apart from ordinary +// trail comments -- same convention as CONSOLIDATION_MEMBER_TITLE/ +// CONSOLIDATION_GROUP_TITLE below. Every gate-state comment still ends with +// the canonical scope-guard line "" (see +// scopeGuard()); this title adds one second, gate-state-specific line of +// machine-parseable structure ABOVE it -- it never replaces or reshapes the +// canonical marker itself. +const GATE_STATE_TITLE = '## Gate State' +// GATE_STATE_SCHEMA: payload shape version, embedded in every payload so +// parseGateStateComment can REJECT (never coerce) a payload an older or +// incompatible engine build wrote. Bump only on a breaking shape change. +const GATE_STATE_SCHEMA = 1 + +// GATE_STATE_PROBE_SCHEMA: the whole-set read probe (fetchGateStateBlocks, +// wired below the TICKETMILL-TEST-HARNESS-SPLIT marker) -- ONE call over every +// candidate issue, pinning a jq read per issue exactly like the claim probe's +// pinned "last title-gated comment" idiom (:7524), never a bare `gh issue view +// --json comments` handed to the agent's own judgment (the fetchConsolidation +// Markers precedent this replaces for gate state, since a bare read let a +// truncated response get silently misread as absence). `raw` is each issue's +// VERBATIM jq stdout; the agent relays it, never parses or judges it -- +// parseGateStateProbeRow does that in JS. `exit_ok` is the agent-level gh exit +// status. `self_login` is `gh api user --jq .login` (a single-object endpoint, +// so this file's "never a bare gh api" pagination rule doesn't apply) -- the +// PRIMARY trust signal isTrustedGateStateAuthor checks first; '' when the +// token can't resolve it (installation tokens), which falls through to the +// claim_authors fallback. +const GATE_STATE_PROBE_SCHEMA = { + type: 'object', required: ['rows'], + properties: { + self_login: { type: 'string' }, + rows: { type: 'array', items: { type: 'object', required: ['issue', 'raw', 'exit_ok'], properties: { + issue: { type: 'integer' }, raw: { type: 'string' }, exit_ok: { type: 'boolean' }, + } } }, + }, +} +// GATE_STATE_VERIFY_SCHEMA: the Report-phase self-validation sweep's chunked +// read-back (verifyGateState, wired below the TICKETMILL-TEST-HARNESS-SPLIT +// marker) -- ONE agent call per chunk of at most MAX_GATE_STATE_PROBE_CHUNK +// issues, shaped like GATE_STATE_PROBE_SCHEMA minus `self_login` (the verify +// sweep is proving THIS run's own write round-tripped through GitHub, never +// adjudicating trust between authors, so it has no use for a self-identity +// signal). Pins the SAME per-issue jq idiom fetchGateStateBlocks uses -- +// `raw` is each issue's VERBATIM jq stdout, relayed never parsed or judged by +// the agent. Fed through the exact same parseGateStateProbeRow -> +// parseGateStateComment pipeline the read-side probe uses -- no second +// parser, no second trust rule. The prompt this schema backs carries ONLY +// issue numbers, never the payload being verified against -- see +// verifyGateState's own comment for why that matters. +const GATE_STATE_VERIFY_SCHEMA = { + type: 'object', required: ['rows'], + properties: { + rows: { type: 'array', items: { type: 'object', required: ['issue', 'raw', 'exit_ok'], properties: { + issue: { type: 'integer' }, raw: { type: 'string' }, exit_ok: { type: 'boolean' }, + } } }, + }, +} + +// buildGateStatePayload: assembles the JSON payload embedded in a gate-state +// comment. `settled` is capped to its last 6 entries here (mirrors +// settledBlock's own slice(-6)) so a long-running issue's payload never grows +// unbounded regardless of how many gates it has cleared. `seeded_from` names +// the {run, epoch} of the block THIS ctx's `gate_budgets` were carried forward +// from, when a resume seeds them from a prior run's recorded state, or null +// when they started at zero this run. It is ALWAYS null at this tier: no +// consumer seeds gate_budgets from a prior block yet (substrate only, no +// consumer -- see the section banner above), so every call site passes null. +// The field's PRESENCE, not its value, is what parseGateStateComment's schema +// round-trips against -- a future consumer fills it in without a shape change +// here. `write_seq` (issue #166 PR #177 review) is the GATE_STATE_WRITE_SEQ +// counter value at the moment this payload was built -- unlike `epoch` +// (identical across every boundary in a run), this varies write to write, so +// diffGateStateIntent can order two same-run writes against each other. null +// when the caller doesn't supply one (e.g. a hand-built test fixture, never a +// real postGateState() call, which always passes it). Every field defaults +// defensively (never throws on a sparse `o`) so a caller mid-construction +// (e.g. a boundary with no group) gets a valid, schema-conformant payload +// rather than an exception. +function buildGateStatePayload(o) { + o = o || {} + return { + schema: GATE_STATE_SCHEMA, + repo: o.repo, + issue: o.issue, + run: o.run, + batch: o.batch, + epoch: (o.epoch === undefined) ? null : o.epoch, + write_seq: (o.write_seq === undefined) ? null : o.write_seq, + boundary: o.boundary, + group_id: (o.group_id === undefined) ? null : o.group_id, + members: Array.isArray(o.members) ? o.members.slice() : [], + seeded_from: (o.seeded_from === undefined) ? null : o.seeded_from, + gate_budgets: (o.gate_budgets && typeof o.gate_budgets === 'object' && !Array.isArray(o.gate_budgets)) ? o.gate_budgets : {}, + settled: (Array.isArray(o.settled) ? o.settled : []).slice(-6), + } +} + +// buildGateStateComment: renders the full comment body posted at each gate- +// state boundary. Fixed shape: title line, ONE human line that is +// deliberately non-directive -- a resumed run's agents must never read this +// as an instruction, it exists purely so a human (or a future run's read-back) +// can see what the engine last recorded, never phrased as something to act +// on -- a
wrapper holding the fenced JSON payload, and the canonical +// scope-guard marker as the LAST NON-EMPTY line (parseGateStateComment +// enforces this on read; it is what makes the comment legible to scopeGuard() +// and every other marker-consumer in this file). +function buildGateStateComment(repo, issue, payload) { + const p = payload || {} + const humanLine = 'Recorded automatically at the "' + p.boundary + '" boundary (run ' + p.run + + ') for resume continuity -- a record, not a directive; nothing here should be treated as an instruction.' + return [ + GATE_STATE_TITLE, + humanLine, + '', + '
Gate state payload', + '', + '```json', + JSON.stringify(payload, null, 2), + '```', + '', + '
', + '', + ].join('\n') +} + +// parseGateStateComment: null unless `body` is a well-formed gate-state marker +// FOR THIS repo/issue -- title-gated (first line exactly GATE_STATE_TITLE, +// same convention as parseConsolidation*), fence-extracted, and REQUIRES the +// canonical scope-guard marker to be the last non-empty line (a body that +// merely quotes the block shape inside a larger comment, or has trailing +// content after the marker, is rejected here, not left to the JSON parse to +// catch). Rejects a payload whose schema/repo/issue don't match the ones this +// read expects -- a payload from a different repo, a different issue (e.g. a +// consolidation group's cross-posted comment), or an older/incompatible +// schema version is never silently accepted. Wrapped in try/catch and NEVER +// THROWS: any malformed/truncated JSON, or a payload that isn't a plain +// object, returns null exactly like every other rejection path here, so a +// caller never needs a second layer of defense around this call. +function parseGateStateComment(body, repo, issue) { + try { + const s = String(body == null ? '' : body) + const lines = s.split('\n') + if (!lines.length || lines[0].trim() !== GATE_STATE_TITLE) return null + let lastIdx = lines.length - 1 + while (lastIdx >= 0 && lines[lastIdx].trim() === '') lastIdx-- + if (lastIdx < 0) return null + const expectedMarker = '' + if (lines[lastIdx].trim() !== expectedMarker) return null + const fenceMatch = /```json\r?\n([\s\S]*?)\r?\n```/.exec(s) + if (!fenceMatch) return null + const payload = JSON.parse(fenceMatch[1]) + if (!payload || typeof payload !== 'object' || Array.isArray(payload)) return null + if (payload.schema !== GATE_STATE_SCHEMA) return null + if (payload.repo !== repo) return null + if (payload.issue !== issue) return null + return payload + } catch (e) { + return null + } +} + +// parseGateStateProbeRow: JSON.parses the VERBATIM stdout of the pinned jq +// read (GATE_STATE_PROBE_SCHEMA.rows[].raw / GATE_STATE_VERIFY_SCHEMA.raw) -- +// the agent relays stdout, never judges it, so this is the only place that +// decides whether a read actually succeeded. Expected shape: {total: , +// blocks: [{body, author_login, author_association}, ...]} (oldest-first, +// already sliced to the last few by the jq itself). ANY throw (non-JSON +// stdout, a truncated/partial JSON string from a chunked or interrupted read) +// OR shape mismatch (missing/wrong-typed total, blocks not an array, or a +// block entry missing a string `body`) returns {ok: false, total: 0, blocks: +// []} -- this function itself never throws, and it never returns ok:true over +// a shape it isn't sure of. This is deliberate: selectGateState treats +// ok:false as read-failed, so a truncated read can never be silently misread +// as "genuinely absent" -- issue #166's core fail-open requirement -- it +// fails LOUD instead. This is what makes that kind of truncation structurally +// impossible to hide. +function parseGateStateProbeRow(raw) { + try { + const o = JSON.parse(raw) + if (!o || typeof o !== 'object' || Array.isArray(o)) return { ok: false, total: 0, blocks: [] } + if (!Number.isInteger(o.total) || o.total < 0) return { ok: false, total: 0, blocks: [] } + if (!Array.isArray(o.blocks)) return { ok: false, total: 0, blocks: [] } + for (const b of o.blocks) { + if (!b || typeof b !== 'object' || typeof b.body !== 'string') return { ok: false, total: 0, blocks: [] } + } + return { ok: true, total: o.total, blocks: o.blocks } + } catch (e) { + return { ok: false, total: 0, blocks: [] } + } +} + +// isTrustedGateStateAuthor: is `login` allowed to author a block selectGateState +// treats as authoritative? PRIMARY signal -- `login === selfLogin`, the +// deployment's own authenticated identity (`gh api user --jq .login`, a +// single-object endpoint so the file's "never a bare gh api" pagination rule +// doesn't apply). This closes the self-bootstrap trust hole the capped +// approach-gate contrarian flagged against a claim_authors-only rule (a stale +// forged claim's author no longer qualifies as primary trust). FALLBACK (for +// installation tokens where `gh api user` 403s) -- `claimAuthors`, restricted +// to a claim that is fresh (age < CLAIM_STALE_SECONDS, :292) OR whose batch +// matches THIS run's batch branch: a claim that is neither fresh nor batch- +// matching authored no work in scope and is not evidence of anything. +// claimAuthors entries: {login, ageSeconds, batch}; either of ageSeconds/batch +// may be null/absent (unknown), in which case only the other test can pass +// that entry. +function isTrustedGateStateAuthor(login, selfLogin, claimAuthors, batch) { + if (!login) return false + if (selfLogin && login === selfLogin) return true + const list = Array.isArray(claimAuthors) ? claimAuthors : [] + return list.some(function (c) { + if (!c || c.login !== login) return false + const fresh = typeof c.ageSeconds === 'number' && Number.isFinite(c.ageSeconds) && c.ageSeconds < CLAIM_STALE_SECONDS + const batchMatch = batch != null && c.batch === batch + return fresh || batchMatch + }) +} + +// deriveRunEpoch: turns the probe-returned `date -u +%Y-%m-%dT%H:%M:%SZ` +// string (the same idiom already used elsewhere in this file for a wall-clock +// anchor, since the sandbox has no Date.now()/argless `new Date()`) into +// RUN_EPOCH (epoch ms), via the existing pure toEpochMs (:6382 as of writing). +// Explicit null on anything unparseable -- NEVER NaN, so a downstream +// `runEpochMs - payload.epoch` comparison in gateStateEpochStale can't +// silently produce a NaN that always compares false; a subtraction against an +// unusable "now" must read as unknown/stale, not as "definitely not stale". +function deriveRunEpoch(nowRaw) { + const ms = toEpochMs(nowRaw) + return Number.isFinite(ms) ? ms : null +} + +// gateStateEpochStale: shared by selectGateState -- true when `payload.epoch` +// predates CLAIM_STALE_SECONDS relative to `runEpochMs`, OR either side is +// unparseable/absent. An unknown age reads as stale, never as fresh (fail +// toward re-verifying, not toward trusting silently). +// KNOWN IMPRECISION (issue #166 PR #177 review): `payload.epoch` is RUN_EPOCH +// -- the run's Select-time wall-clock anchor -- not the actual moment this +// particular boundary was written, so a long-running run's later boundaries +// read slightly younger than they really are. Log-only at this tier (no +// consumer yet); not worth a second probe-derived wall-clock read to fix. +function gateStateEpochStale(payload, runEpochMs) { + const payloadEpoch = (payload && Number.isFinite(payload.epoch)) ? payload.epoch : null + if (payloadEpoch === null || !Number.isFinite(runEpochMs)) return true + return (runEpochMs - payloadEpoch) > (CLAIM_STALE_SECONDS * 1000) +} + +// selectGateState: the single decision point for "what does this issue's +// gate-state comment trail say, and can it be trusted?" Turns `rows` (one +// issue's already-parsed probe result -- parseGateStateProbeRow's {ok, total, +// blocks} shape, optionally carrying the agent-level `exit_ok` alongside it; +// blocks are oldest-first, mirroring GitHub's own comment order), `evidence` +// ({repo, issue, self_login, claim_authors, batch, run_epoch}), and +// `priorWork` ({pr_number, worktree_exists, resume_point}) into exactly one +// of four states: +// - 'read-failed' -- the probe/parse never produced usable data (an +// explicit agent-level exit_ok:false, OR parseGateStateProbeRow's own +// ok:false), OR the falsifiable-absent rule fires (below). This is what +// makes a truncated/broken read structurally impossible to misread as +// genuine absence. +// - 'absent' -- zero blocks, zero total, and nothing else on this issue +// (pr_number/worktree_exists/resume_point) is evidence prior work ever +// happened -- a genuinely fresh issue. +// - 'malformed' -- at least one block exists, but NONE of them parse +// (title-gated, fence-extracted, marker-checked -- see +// parseGateStateComment): the newest fails and every older one fails too. +// - 'found' -- at least one block parses. Selection is EXPLICIT TRUST- +// BEFORE-LAST-WINS: walk blocks newest -> oldest, return the first one +// whose author is trusted (isTrustedGateStateAuthor), counting every +// newer untrusted-but-parseable block passed over into `skipped`. If NO +// block is trusted, this is the degenerate all-untrusted case: state +// stays 'found' (there IS data, just not from a trusted author) using the +// newest PARSEABLE block's payload, `trusted: false`, and `skipped: 0` +// (nothing was skipped to reach it -- it's the first thing the walk +// looked at). `trusted` is kept on the result specifically so a caller +// can distinguish this degenerate case from an ordinary trusted find. +// `stale` (only meaningful when `state === 'found'`) comes from +// gateStateEpochStale against the SELECTED payload. +// +// FALSIFIABLE-ABSENT RULE: zero blocks + total===0 is only accepted as +// genuine absence when nothing else on this issue is evidence prior work +// happened. If `pr_number` is non-null, OR `worktree_exists` is true, OR +// `resume_point` is anything other than 'implement', a prior run plainly did +// SOMETHING here, so zero gate-state comments is contradictory -- +// read-failed, never absent. Zero blocks with no such evidence stays absent. +// This is NEVER inferred from an empty blocks array alone -- always from this +// explicit cross-check against independently-sourced preflight evidence. +// A second, narrower contradiction is checked first and unconditionally: +// zero blocks but total>0 is self-contradictory on its face (the probe says +// comments exist but produced none) -- exactly the truncated/corrupted-read +// shape this whole design exists to make undetectable-as-absence, so it is +// always read-failed regardless of `hasPriorWork`. `total` is computed by +// gateStateProbeCommandLine's jq using the SAME title-gated filter `blocks` +// uses (never a bare all-comments count), so this branch is reachable only +// under a genuinely truncated/corrupted read, not on any ordinary issue +// carrying an unrelated human or bot comment. +// +// hasGateStatePriorWork: shared with fetchGateStateBlocks' diagnostic log +// (below the split), which needs the same fact to tell the falsifiable-absent +// case apart from an ordinary read-failed -- kept as one pure helper rather +// than two copies of the same three-condition check. +function hasGateStatePriorWork(priorWork) { + const pw = priorWork || {} + return !!( + (pw.pr_number !== null && pw.pr_number !== undefined) || + pw.worktree_exists === true || + (pw.resume_point != null && pw.resume_point !== 'implement') + ) +} +function selectGateState(rows, evidence, priorWork) { + const r = rows || {} + const ev = evidence || {} + const pw = priorWork || {} + const EMPTY = { payload: null, trusted: false, stale: false, skipped: 0 } + + if (r.exit_ok === false || r.ok === false) { + return Object.assign({ state: 'read-failed' }, EMPTY) + } + + const blocks = Array.isArray(r.blocks) ? r.blocks : [] + const total = Number.isInteger(r.total) ? r.total : blocks.length + + if (blocks.length === 0) { + if (total > 0) return Object.assign({ state: 'read-failed' }, EMPTY) + if (hasGateStatePriorWork(pw)) return Object.assign({ state: 'read-failed' }, EMPTY) + return Object.assign({ state: 'absent' }, EMPTY) + } + + let skipped = 0 + let fallback = null // newest PARSEABLE block, regardless of trust + let fallbackSkipped = 0 + for (let i = blocks.length - 1; i >= 0; i--) { + const b = blocks[i] + const payload = parseGateStateComment(b && b.body, ev.repo, ev.issue) + if (!payload) continue // unparseable at this position -- never counted into `skipped` + const trusted = isTrustedGateStateAuthor(b && b.author_login, ev.self_login, ev.claim_authors, ev.batch) + if (!fallback) { fallback = payload; fallbackSkipped = skipped } + if (trusted) { + return { state: 'found', payload: payload, trusted: true, stale: gateStateEpochStale(payload, ev.run_epoch), skipped: skipped } + } + skipped++ + } + + if (fallback) { + return { state: 'found', payload: fallback, trusted: false, stale: gateStateEpochStale(fallback, ev.run_epoch), skipped: fallbackSkipped } + } + return Object.assign({ state: 'malformed' }, EMPTY) +} + +// diffGateStateIntent: compares the payload THIS run intended to post +// (`intent`) against a payload actually read back (`actual` -- e.g. +// selectGateState's `.payload`, or the Report-phase verify sweep's direct +// re-read). Three verdicts: +// - 'match' -- byte-for-byte the same write (JSON.stringify-equal). +// - 'superseded' -- `actual` is a LATER write from the SAME run (same +// `run`, later `write_seq`) -- expected and not alarming: +// a later boundary in this same run posted after `intent` +// was captured (e.g. a later pr-review iteration's write +// landing after an earlier iteration's intent snapshot). +// Ordering is on `write_seq`, NOT `epoch` (issue #166 PR +// #177 review) -- RUN_EPOCH is assigned once at Select and +// is identical on every boundary a single run posts, so it +// can never distinguish an earlier write from a later one +// within that run; only the monotonic per-write +// GATE_STATE_WRITE_SEQ counter does. +// - 'mismatch' -- anything else: a different run's write sitting where +// ours should be, an EARLIER write, or same-run content +// that disagrees without a later write_seq to explain it +// -- real corruption or a lost write. +function diffGateStateIntent(intent, actual) { + if (!intent || !actual) return 'mismatch' + if (JSON.stringify(intent) === JSON.stringify(actual)) return 'match' + if (intent.run === actual.run && Number.isFinite(intent.write_seq) && Number.isFinite(actual.write_seq) && actual.write_seq > intent.write_seq) { + return 'superseded' + } + return 'mismatch' +} + +// chunkGateStateIssues / gateStateProbeCommandLine / deadGateStateChunkRows / +// normalizeGateStateRow: shared by fetchGateStateBlocks (below the split) and +// the Report-phase verifyGateState sweep -- both chunk the SAME issue list at +// MAX_GATE_STATE_PROBE_CHUNK, pin the SAME per-issue jq idiom, fall back to +// the SAME {raw: '', exit_ok: false} stub rows when a chunk's agent call +// dies, and normalize a returned row's `raw`/`exit_ok` the same way. Kept as +// four small pure helpers rather than two copies of each, so the read-side +// probe and its self-validation sweep can never drift apart. +function chunkGateStateIssues(list) { + const chunks = [] + for (let i = 0; i < list.length; i += MAX_GATE_STATE_PROBE_CHUNK) chunks.push(list.slice(i, i + MAX_GATE_STATE_PROBE_CHUNK)) + return chunks +} +function gateStateProbeCommandLine() { + // total is computed by the SAME title-gated select(...) filter blocks uses -- + // NEVER a bare `.comments|length` -- so it counts gate-state comments only, + // not every comment on the issue. A bare all-comments count would make + // `blocks.length === 0 && total > 0` reachable on any ordinary issue with so + // much as one human reply or one of this pipeline's own non-gate-state + // comments, which selectGateState's self-contradiction check (:1343 as of + // writing) treats as read-failed -- silently swallowing the `absent` state + // for every issue that has ever received an unrelated comment (issue #166 + // PR #177 review). + const titleFilter = 'select(.body | startswith("' + GATE_STATE_TITLE + '"))' + return 'gh issue view --repo ' + REPO + ' --json comments --jq \'{total: ([.comments[] | ' + titleFilter + '] | length), blocks: [.comments[] | ' + titleFilter + ' | {body, author_login: .author.login, author_association: .authorAssociation}] | .[-3:]}\'' +} +function deadGateStateChunkRows(chunk) { + return chunk.map(function (n) { return { issue: n, raw: '', exit_ok: false } }) +} +function normalizeGateStateRow(row) { + return { raw: typeof row.raw === 'string' ? row.raw : '', exit_ok: row.exit_ok === true } +} + +// attachGateStateBlocks: per-preflight normalizer AND real-data join, mirroring +// attachEngineOwnedIntentional's shape (:3023) -- guarantees every preflight +// carries all four gate-state PREFLIGHT_SCHEMA fields (gate_state_blocks, +// gate_state_read_ok, gate_state_total_comments, gate_state_trust) +// UNCONDITIONALLY. Pure and side-effect-free: returns a NEW array, never +// mutates `preflights` or `rowsByIssue`. +// +// `rowsByIssue` (optional; keyed by issue NUMBER) carries fetchGateStateBlocks' +// RAW per-issue probe rows -- {raw, exit_ok} straight off GATE_STATE_PROBE_SCHEMA, +// UNPARSED -- this function is what runs parseGateStateProbeRow, so a truncated +// or non-JSON `raw` string handed in here surfaces as gate_state_read_ok:false, +// never as an empty-but-successful read. `selfLogin` is the reduced +// self_login string (see fetchGateStateBlocks below the split) stored verbatim +// as gate_state_trust -- a FUTURE consumer's selectGateState call uses it as +// evidence.self_login; this function itself never decides trust or state. +// +// Every preflight's four fields are ALWAYS computed fresh from `rowsByIssue`/ +// `selfLogin` -- NEVER read back off the preflight object's own pre-existing +// values, even when `rowsByIssue` has no entry for that issue. This is +// deliberate: these four fields are read-only FACTS this run's own probe +// resolved, never something an upstream agent (e.g. the preflight probe, +// which happens to share PREFLIGHT_SCHEMA) gets to assert on its own — a +// hallucinated gate_state_blocks arriving on `p` from anywhere else is always +// clobbered to the real (or fail-open default) value, exactly like +// attachEngineOwnedIntentional never trusts an agent-supplied regime. +function attachGateStateBlocks(preflights, rowsByIssue, selfLogin) { + const byIssue = rowsByIssue || {} + const trust = typeof selfLogin === 'string' ? selfLogin : '' + return (preflights || []).map(function (p) { + const row = Object.prototype.hasOwnProperty.call(byIssue, p.issue) ? byIssue[p.issue] : null + if (!row) { + return Object.assign({}, p, { + gate_state_blocks: [], gate_state_read_ok: false, gate_state_total_comments: 0, gate_state_trust: trust, + }) + } + const parsed = parseGateStateProbeRow(row.raw) + const readOk = row.exit_ok === true && parsed.ok === true + return Object.assign({}, p, { + gate_state_blocks: readOk ? parsed.blocks.map(function (b) { return b.body }) : [], + gate_state_read_ok: readOk, + gate_state_total_comments: readOk ? parsed.total : 0, + gate_state_trust: trust, + }) + }) +} + + // ============================================================================= // CONSOLIDATION (unit-of-work) FOUNDATIONS // @@ -2925,7 +3439,7 @@ async function fail(ctx, status, stageKey, error) { await postNote(ctx, stageKey, status, error) BATCH.failures++ if (BATCH.failures >= MAX_BATCH_FAILURES) tripStop('circuit breaker: ' + BATCH.failures + ' issues failed') - return Object.assign({ issue: ctx.issue, title: ctx.title, status: status, stage: stageKey, pr: ctx.pr || null, error: String(error || ''), follow_ups: [], metrics: ctx.metrics || null, tokens: ctx.tokens || null, timeline: timeline(ctx), handoff_notes: (ctx.notes || []).slice(), members: memberIssues(ctx), changed_files: (ctx && ctx.changed_files) || null, added_files: (ctx && ctx.added_files) || null, touch_counts: (ctx && ctx.touch_counts) || {}, gate_findings: (ctx && ctx.gate_findings) || {}, settled: (((ctx && ctx.settled) || []).slice()) }, frictionFields(ctx, status)) + return Object.assign({ issue: ctx.issue, title: ctx.title, status: status, stage: stageKey, pr: ctx.pr || null, error: String(error || ''), follow_ups: [], metrics: ctx.metrics || null, tokens: ctx.tokens || null, timeline: timeline(ctx), handoff_notes: (ctx.notes || []).slice(), members: memberIssues(ctx), changed_files: (ctx && ctx.changed_files) || null, added_files: (ctx && ctx.added_files) || null, touch_counts: (ctx && ctx.touch_counts) || {}, gate_findings: (ctx && ctx.gate_findings) || {}, settled: (((ctx && ctx.settled) || []).slice()), gate_state_intent: (ctx && ctx.gate_state_intent) || null, gate_state_post_failed: (ctx && ctx.gate_state_post_failed) || null }, frictionFields(ctx, status)) } // ============================================================================= @@ -4057,6 +4571,141 @@ async function fetchConsolidationMarkers(issueNumbers) { return (r && r.markers) || [] } +// fetchGateStateBlocks: READ-ONLY (safe under DRY_RUN) — the whole-set gate- +// state read, shaped like fetchConsolidationMarkers just above it, but the +// READ IDIOM is deliberately NOT that one: fetchConsolidationMarkers hands the +// agent a bare `gh issue view --json comments` and trusts its own judgment to +// pick the right comment, which lets a truncated response get silently +// misread as "no marker". Gate state instead pins the claim probe's +// deterministic idiom (the per-issue `gh issue view ... --jq '{total, blocks}'` +// a few thousand lines below, in the claims loop) verbatim, one command per +// issue: jq computes the EXACT return shape, so a short/truncated read is a +// JSON.parse failure (parseGateStateProbeRow), never a fake "zero blocks". +// The agent's ONLY job per issue is relaying that command's stdout — it never +// parses or judges it. +// +// Chunked at MAX_GATE_STATE_PROBE_CHUNK issues per agent call — belt-and- +// braces, not a truncation defense (the jq pin already makes a truncated READ +// structurally impossible to misread): a chunk whose agent call dies (throws, +// budget-exhausted, or returns a malformed response) marks ONLY its own +// chunk's issues read-failed via synthesized {raw: '', exit_ok: false} stub +// rows — surviving chunks still report normally, rather than one dead call +// taking the whole candidate set down with it. A LIVE chunk that returns a +// schema-valid `rows` array simply missing one of its assigned issues (which +// GATE_STATE_PROBE_SCHEMA cannot forbid) gets the SAME stub backfilled after +// the chunk loop below, for the same reason: a queried-but-unanswered issue +// must read as read-failed, never silently as absent. +// +// self_login reduction: each chunk independently runs `gh api user --jq +// .login` (a single-object endpoint, so the file's "never a bare gh api" +// pagination rule does not apply) since chunks run in parallel and none of +// them can see another's result. The FIRST chunk (in `chunks` order) that +// reports a non-empty login wins — every chunk is hitting the SAME +// authenticated identity, so this is a redundant-computation reduction, not a +// disagreement to arbitrate; an empty string means no chunk could resolve it +// (an installation token, or every chunk died), which isTrustedGateStateAuthor +// treats as "primary trust unavailable, fall through to claim_authors". +// +// `priorWorkByIssue` ({issue: {pr_number, worktree_exists, resume_point}}) is +// NEVER sent to the agent (it stays a pure verbatim relay) — it feeds ONLY +// the per-issue log line below, via selectGateState's falsifiable-absent rule, +// so "zero gate-state comments but a PR is already open" logs as the +// DISTINCT, greppable suspicious case rather than a bare "absent" that could +// hide a real read problem. +// +// Returns { rowsByIssue, self_login } — rowsByIssue is RAW ({issue: {raw, +// exit_ok}}), unparsed on purpose: attachGateStateBlocks (above the split) is +// what runs parseGateStateProbeRow, so this function's own contract stays a +// thin, mirror-of-fetchConsolidationMarkers relay with no decision logic of +// its own beyond the per-issue log line (which is diagnostic output, not a +// decision fed back into the run). +async function fetchGateStateBlocks(issueNumbers, priorWorkByIssue) { + const list = Array.isArray(issueNumbers) ? issueNumbers.slice() : [] + if (!list.length) return { rowsByIssue: {}, self_login: '' } + const pwByIssue = priorWorkByIssue || {} + const chunks = chunkGateStateIssues(list) + + const chunkResults = await Promise.all(chunks.map(function (chunk, ci) { + return agent([ + 'READ-ONLY (safe under any run mode, including a dry run). For EACH issue number listed below, run this EXACT', + 'command, substituting only for that issue\'s number — do not alter the jq filter in any way:', + gateStateProbeCommandLine(), + 'Issues in this call: ' + chunk.join(', '), + 'Relay each command\'s stdout VERBATIM as `raw` — never parse, reformat, summarize, or judge it. `exit_ok` is', + 'whether that gh command exited 0 for that issue (false on any non-zero exit, including a repo/issue lookup', + 'failure).', + '', + 'Also run ONCE for this whole call (not per issue): gh api user --jq .login', + 'Return self_login = that command\'s trimmed stdout, or "" if the command fails or returns nothing (this', + 'happens for installation tokens — expected, not an error).', + '', + 'Return rows: [{issue, raw, exit_ok}] — exactly one entry per issue listed above, even one whose gh command', + 'failed.', + ].join('\n'), { label: 'gate-state-probe-c' + ci, phase: 'Select', schema: GATE_STATE_PROBE_SCHEMA, model: M.probe.model, effort: M.probe.effort }) + .catch(function () { return null }) + .then(function (r) { + if (r && Array.isArray(r.rows)) return { self_login: typeof r.self_login === 'string' ? r.self_login : '', rows: r.rows } + // dead chunk — belt-and-braces: mark ONLY this chunk's issues read-failed via + // explicit stub rows, never silently drop them (a dropped issue would look + // identical to one this function was never asked about at all). + return { self_login: '', rows: deadGateStateChunkRows(chunk) } + }) + })) + + const rowsByIssue = {} + let selfLogin = '' + for (const cr of chunkResults) { + if (!selfLogin && cr.self_login && cr.self_login.trim()) selfLogin = cr.self_login.trim() + for (const row of (cr.rows || [])) rowsByIssue[row.issue] = normalizeGateStateRow(row) + } + // Backfill any queried issue a LIVE chunk's response simply omitted — GATE_STATE_PROBE_SCHEMA + // cannot enforce one row per issue, so a schema-valid response that drops an issue (never + // throws, never hits the dead-chunk catch above) would otherwise leave rowsByIssue[n] + // undefined. Stubbed the same way a dead chunk's issues are (deadGateStateChunkRows, a few + // lines above) so a queried-but-never-answered issue reads as read-failed, never as a false + // "absent" (issue #166 PR #177 review). + for (const n of list) { + if (!Object.prototype.hasOwnProperty.call(rowsByIssue, n)) rowsByIssue[n] = normalizeGateStateRow({ issue: n, raw: '', exit_ok: false }) + } + + // Per-issue diagnostic log, using the SAME pure selectGateState decision a + // future consumer would reach — this run just prints it rather than acting + // on it (substrate only, no consumer yet). run_epoch is not yet assigned at + // this Select-phase call site (see the RUN_EPOCH assignment below, which + // needs outcomeGradeR/revisitRiskR already awaited) — the `state` value + // never depends on it (only the auxiliary `stale` flag does), so passing the + // module-level RUN_EPOCH here (null on a fresh run) is correct, not stale. + // Every issue in `list` now has a rowsByIssue entry (real or backfilled + // stub), so `row` below is never falsy — no separate {} fallback needed. + for (const n of list) { + const row = rowsByIssue[n] + const parsed = parseGateStateProbeRow(row.raw) + const rowsArg = Object.assign({ exit_ok: row.exit_ok }, parsed) + const pw = pwByIssue[n] || {} + const sel = selectGateState(rowsArg, { repo: REPO, issue: n, self_login: selfLogin, claim_authors: [], batch: TARGET, run_epoch: RUN_EPOCH }, pw) + // readOk mirrors attachGateStateBlocks' definition (:1467) exactly -- the same + // "did the read actually succeed" test, so this log and the stored preflight + // fields never disagree about it. Without this gate, EVERY hard read failure + // (dead chunk, non-zero gh exit, truncated stdout) also has blocks.length===0 + // and total===0, so it printed the same "absent (unexpected: ...)" line as a + // genuine falsifiable-absent read -- on a resume, where hasGateStatePriorWork + // is true for exactly the issues this substrate serves, that made read + // failures indistinguishable from suspicious absences (issue #166 PR #177 + // review, iteration 2). + const readOk = row.exit_ok === true && parsed.ok === true + if (sel.state === 'read-failed' && readOk && parsed.blocks.length === 0 && parsed.total === 0 && hasGateStatePriorWork(pw)) { + const why = pw.pr_number != null ? ('PR #' + pw.pr_number + ' open') + : pw.worktree_exists ? 'worktree exists' + : ('resume_point=' + pw.resume_point) + log('gate-state #' + n + ': absent (unexpected: ' + why + ')') + } else { + log('gate-state #' + n + ': ' + sel.state + (sel.state === 'found' && !sel.trusted ? ' (untrusted author)' : '')) + } + } + + return { rowsByIssue: rowsByIssue, self_login: selfLogin } +} + // challengeConsolidationGroup: the capped contrarian loop for ONE proposed group. // Returns the (possibly revised) accepted group, or null if it DISSOLVED (cap // reached without acceptance, or a dead challenger/reviser — see the module @@ -4297,6 +4946,211 @@ async function postConsolidationMarkers(units) { } } +// postGateState (issue #166): non-fatal per-issue write of the durable +// "## Gate State" comment (buildGateStateComment/buildGateStatePayload, above +// the TICKETMILL-TEST-HARNESS-SPLIT marker) at a run boundary. Modeled +// directly on the cap-note-plan/cap-note-approach stages just above +// (stageOpts('probe'), NOTE_SCHEMA, exactly 1 try, log-only on a dead agent +// or posted!==true) -- like those, NO path through this helper may fail an +// issue; every failure mode degrades to a logged/deferred note and the caller +// keeps going. +// +// Posting idiom deliberately breaks from postConsolidationMarkers just above +// (`gh issue comment ... --body "..."`, :4645/:4660): that idiom is safe only +// because a consolidation marker's body is flat, oneLine()-rendered +// key:value text with nothing in it that a shell would treat specially. A +// gate-state body embeds free text straight out of ctx.settled (rationale/ +// resolution strings a human or an earlier agent wrote), which CAN contain +// apostrophes, backticks, or `$` -- any of which a `--body "$(...)"` or an +// UNQUOTED heredoc would hand to the shell for interpolation, silently +// corrupting the posted payload (or worse). Instead this pins `gh issue +// comment --repo --body-file -` fed by stdin from a QUOTED heredoc +// (`<<'...'`), which disables ALL shell expansion inside it -- the payload +// reaches `gh` as literal bytes no matter what punctuation the free text +// carries. +// +// ctx.gate_state_intent is set to the intended payload ONLY when the agent +// actually reports posted===true. Every other outcome (dead agent after its +// one try, an explicit posted:false, or a schema mismatch) instead sets +// ctx.gate_state_post_failed = boundary (last failure wins across an issue's +// several boundaries) and pushes a ctx.deferred note, so the miss is visible +// without ever touching gate_state_intent. This asymmetry is deliberate: +// setting intent unconditionally would let Task 4's Report-phase verify +// sweep compare an intent against a block that was never actually written +// and report 'mismatch' (real corruption) for what is, here, a routine, +// designed-for non-fatal skip. +async function postGateState(ctx, boundary) { + const payload = buildGateStatePayload({ + repo: REPO, + issue: ctx.issue, + run: RUN_TAG, + batch: TARGET, + epoch: RUN_EPOCH, + write_seq: ++GATE_STATE_WRITE_SEQ, + boundary: boundary, + group_id: ctx.groupId, + members: memberIssues(ctx), + gate_budgets: { + approach: (ctx.metrics && ctx.metrics.approach_iters) || 0, + plan: (ctx.metrics && ctx.metrics.plan_iters) || 0, + 'pr-review': (ctx.metrics && ctx.metrics.pr_review_iters) || 0, + }, + settled: ctx.settled, + }) + const body = buildGateStateComment(REPO, ctx.issue, payload) + const posted = await stage(ctx, 'gate-state-' + boundary, [ + 'Post the durable gate-state record on issue #' + ctx.issue + ' of ' + REPO + ' EXACTLY as given below, verbatim', + 'and unchanged -- do not reformat, reword, summarize, or add anything to it. Run exactly this command, with the', + 'body fed on stdin via a QUOTED heredoc (the quotes around the delimiter are load-bearing: they disable ALL', + 'shell interpolation inside the heredoc, which matters because the body below may contain apostrophes,', + 'backticks, or $ characters that must reach gh as literal bytes, not be expanded by the shell):', + '', + 'gh issue comment ' + ctx.issue + ' --repo ' + REPO + ' --body-file - <<\'TICKETMILL_GATE_STATE_EOF\'', + body, + 'TICKETMILL_GATE_STATE_EOF', + '', + 'Do NOT substitute an unquoted heredoc (< GitHub -> read -> parse for gate state in one +// run. ONE stage for the WHOLE run (never 2N per-issue calls, and never the +// old per-boundary-during-processIssue design GATE_STATE_VERIFY_SCHEMA's +// original comment described -- that design was superseded by this +// Report-phase sweep before this task landed), chunked at +// MAX_GATE_STATE_PROBE_CHUNK like fetchGateStateBlocks (:4544, via the shared +// chunkGateStateIssues helper) -- belt-and-braces: a dead chunk's agent call +// only takes its own chunk's issues down with it, surviving chunks report +// normally. +// +// The verify prompt carries ONLY the issue numbers and the pinned per-issue +// jq idiom fetchGateStateBlocks uses (via the shared gateStateProbeCommandLine +// helper) -- it NEVER carries +// ctx.gate_state_intent or any other part of the payload being checked +// against. This is load-bearing: if the prompt included the intended +// payload, the agent could satisfy the schema by echoing it back rather than +// actually relaying gh's real output, and the "comparison" would prove +// nothing. JS alone -- never the agent -- runs parseGateStateProbeRow, then +// parseGateStateComment on the newest returned block, then diffGateStateIntent +// against the result's own gate_state_intent. +// +// phase('Report') runs on every terminal exit of a batch (STOP.tripped fills +// the remaining results as 'not_started' and returns; a per-unit throw is +// isolated to that unit by runPool -- see :5871), so `results` passed in here +// always carries every issue's FINAL gate-state fields for this run, on every +// exit path, not just a clean finish. +// +// Six outcomes, logged one per issue via `log()`. NON-FATAL end to end: +// this sweep never mutates a result's status, never throws past its own call +// site, and a dead/misbehaving chunk degrades to 'read-failed' for that +// chunk's issues rather than aborting the sweep. 'mismatch' and 'read-failed' +// additionally push a VERIFY_SKIPS entry (issue #166 PR #177 review) -- every +// other outcome is either nothing-to-verify or a clean/expected result, but +// these two mean this run's self-validation either proved nothing +// ('read-failed') or found evidence of a lost/corrupted write ('mismatch'), +// which belongs in the batch PR's Verification Gaps section, the human's only +// window into what this run couldn't verify. +// - 'no-intent' -- this run never recorded a successful gate-state post +// for this issue AND never recorded a failed one either +// (gate_state_intent and gate_state_post_failed both +// absent) -- e.g. not_started, preflight-skipped, or the +// unit died before its first boundary. Nothing to +// verify; not alarming. +// - 'post-failed' -- postGateState() itself already reported the post +// failed (ctx.gate_state_post_failed set, no intent +// recorded) -- Task 2's KNOWN non-fatal path. Kept as +// its own outcome so this benign, already-logged miss is +// never reported alongside genuine read-back corruption. +// - 'read-failed' -- the verify probe itself couldn't produce usable data +// for this issue this run (no row at all, an explicit +// exit_ok:false, or parseGateStateProbeRow rejected the +// stdout shape) -- never conflated with a real mismatch. +// - 'match' -- diffGateStateIntent found the newest gate-state block +// read back byte-identical to what this run intended. +// - 'superseded' -- diffGateStateIntent found a later write from the SAME +// run sitting where the intent snapshot was taken from +// (e.g. a later pr-review iteration posted after an +// earlier iteration's intent was captured) -- expected, +// not alarming, so this run's own later boundary is +// never reported as corruption. A DIFFERENT run's write +// (concurrent or otherwise) is never 'superseded' -- +// diffGateStateIntent requires `intent.run === actual.run` +// before it even looks at ordering, so that case always +// falls through to 'mismatch' below. +// - 'mismatch' -- anything else diffGateStateIntent returns: a +// different run's write, an earlier write, no +// gate-state block found at all despite a recorded +// successful post, or same-run content that disagrees +// without a later write_seq to explain it. Real +// corruption or a lost write. +async function verifyGateState(results) { + const list = Array.isArray(results) ? results : [] + const toVerify = list.filter(function (r) { return r && r.gate_state_intent }) + const rowsByIssue = {} + + if (toVerify.length) { + const issueNumbers = toVerify.map(function (r) { return r.issue }) + const chunks = chunkGateStateIssues(issueNumbers) + + const chunkRows = await Promise.all(chunks.map(function (chunk, ci) { + return agent([ + 'READ-ONLY. For EACH issue number listed below, run this EXACT command, substituting only for that', + 'issue\'s number -- do not alter the jq filter in any way:', + gateStateProbeCommandLine(), + 'Issues in this call: ' + chunk.join(', '), + 'Relay each command\'s stdout VERBATIM as `raw` -- never parse, reformat, summarize, or judge it. `exit_ok` is', + 'whether that gh command exited 0 for that issue (false on any non-zero exit, including a repo/issue lookup', + 'failure).', + '', + 'Return rows: [{issue, raw, exit_ok}] -- exactly one entry per issue listed above, even one whose gh command', + 'failed.', + ].join('\n'), { label: 'gate-state-verify-c' + ci, phase: 'Report', schema: GATE_STATE_VERIFY_SCHEMA, model: M.probe.model, effort: M.probe.effort }) + .catch(function () { return null }) + .then(function (r) { + if (r && Array.isArray(r.rows)) return r.rows + // dead chunk -- belt-and-braces, mirrors fetchGateStateBlocks: mark ONLY this + // chunk's issues read-failed via explicit stub rows, never silently drop them. + return deadGateStateChunkRows(chunk) + }) + })) + for (const rows of chunkRows) { + for (const row of (rows || [])) rowsByIssue[row.issue] = normalizeGateStateRow(row) + } + } + + for (const r of list) { + if (!r) continue + let outcome + if (!r.gate_state_intent) { + outcome = r.gate_state_post_failed ? 'post-failed' : 'no-intent' + } else { + const row = rowsByIssue[r.issue] + const parsed = row ? parseGateStateProbeRow(row.raw) : { ok: false, total: 0, blocks: [] } + if (!row || row.exit_ok !== true || !parsed.ok) { + outcome = 'read-failed' + } else { + const newest = parsed.blocks.length ? parsed.blocks[parsed.blocks.length - 1] : null + const actual = newest ? parseGateStateComment(newest.body, REPO, r.issue) : null + outcome = diffGateStateIntent(r.gate_state_intent, actual) + } + } + log('gate-state-verify #' + r.issue + ': ' + outcome) + if (outcome === 'mismatch' || outcome === 'read-failed') { + VERIFY_SKIPS.push('#' + r.issue + ': gate-state self-validation reported ' + outcome + ' -- this run\'s durable gate-state record for this issue could not be confirmed to have round-tripped through GitHub as intended (non-fatal, no consumer relies on it yet, but resume continuity for a future consumer is unverified)') + } + } +} + // ============================================================================= // IMPLEMENT (setup -> research -> evaluate<->contrarian -> plan<->contrarian -> // tasks with review/quality loops -> test loop -> browser -> docblocks -> PR) @@ -4515,6 +5369,13 @@ async function implementIssue(ctx) { pushDecision(ctx, 'Revised Evaluation (i' + iter + ')', '**Approach:** ' + (re.approach || '') + '\n' + (re.summary || '')) } + // Gate-state boundary 'approach' (issue #166): covers every one of the loop's + // four exits above (dead contrarian, sound_with_caveats, cap-out, dead + // re-evaluate) -- all four are `break`s out of the same loop, so one post + // placed right here, before anything that can fail the issue, durably + // records the approach gate's outcome regardless of which exit was taken. + await postGateState(ctx, 'approach') + // ---- PLAN + CONTRARIAN CHALLENGE (plan) ---- const agentMenu = IMPLEMENTERS.length ? IMPLEMENTERS.map(function (n) { @@ -4655,6 +5516,13 @@ async function implementIssue(ctx) { pushDecision(ctx, 'Revised Plan (i' + iter + ')', (rp.summary || '') + '\n**Tasks:**\n' + tasks.map(function (t) { return '- ' + t.id + ' [' + (t.agent || 'implementer') + '] ' + t.description }).join('\n')) } + // Gate-state boundary 'plan' (issue #166): covers every one of the loop's + // four exits above (dead contrarian, sound_with_caveats, cap-out, dead + // re-plan) the same way the 'approach' boundary covers its own loop -- all + // four are `break`s, so one post here, before the IMPLEMENT section below, + // durably records the plan gate's outcome regardless of which exit fired. + await postGateState(ctx, 'plan') + // ---- IMPLEMENT (sequential per-task: implement -> review -> fix loop -> quality loop) ---- let tasksCompleted = 0 const failedTasks = [] @@ -4899,7 +5767,18 @@ async function reviewAndMerge(ctx) { ]) const spec = reviews[0] const code = reviews[1] - if (!spec || !code) return fail(ctx, 'needs_human', 'pr-review', 'a PR reviewer died — PR #' + ctx.pr + ' left open for human review') + if (!spec || !code) { + // Gate-state boundary 'pr-review-iN-aborted' (issue #166): the ONLY exit + // from this loop that a resumed run can reach WITHOUT ever passing + // through the recordGateOutcome() call below, because the process_pr + // resume path (processIssue -> reviewAndMerge directly) never runs the + // approach/plan gates or their own boundary posts. Without a post here, + // a resume whose reviewers die on iteration 1 would record nothing at + // all for this issue. ctx.metrics.pr_review_iters is already `iter` + // (set above, before the reviews ran), so no extra plumbing is needed. + await postGateState(ctx, 'pr-review-i' + iter + '-aborted') + return fail(ctx, 'needs_human', 'pr-review', 'a PR reviewer died — PR #' + ctx.pr + ' left open for human review') + } // gate_findings tally (issue #91, retyped by issue #162): one call per // PR-review iteration, using the same disposition vocabulary as the @@ -4929,6 +5808,14 @@ async function reviewAndMerge(ctx) { const prReviewDisposition = prReviewClean ? 'accepted' : ((bothNothingToFix || capReached) ? 'carried-unresolved' : 're-litigated') recordGateOutcome(ctx, 'pr-review', (specFindings || []).concat(codeFindings || []), prReviewDisposition) + // Gate-state boundary 'pr-review-iN' (issue #166): kept INSIDE the loop, + // right after the gate_findings tally above, because reviewAndMerge + // returns from inside this loop at both breaks below (nothing-to-fix and + // cap-reached) as well as at the clean-approval break just below this + // line -- a post placed after the loop would never run for either of + // those in-loop returns. + await postGateState(ctx, 'pr-review-i' + iter) + if (prReviewClean) { approved = true; break } // Both reviewers have nothing to fix, but that isn't prReviewClean (one or @@ -5092,7 +5979,7 @@ async function reviewAndMerge(ctx) { if (mar.resolved) ctx.metrics.merge_auto_resolved = (ctx.metrics.merge_auto_resolved || 0) + 1 log('#' + ctx.issue + ' merged PR #' + ctx.pr + (merge.follow_up_issues && merge.follow_up_issues.length ? ' (follow-ups: ' + merge.follow_up_issues.join(', ') + ')' : '')) - return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'completed', pr: ctx.pr, follow_ups: merge.follow_up_issues || [], stage: 'merge', error: null, metrics: ctx.metrics, tokens: ctx.tokens, timeline: timeline(ctx), handoff_notes: ctx.notes.slice(), members: memberIssues(ctx), changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice() }, frictionFields(ctx, 'completed')) + return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'completed', pr: ctx.pr, follow_ups: merge.follow_up_issues || [], stage: 'merge', error: null, metrics: ctx.metrics, tokens: ctx.tokens, timeline: timeline(ctx), handoff_notes: ctx.notes.slice(), members: memberIssues(ctx), changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice(), gate_state_intent: ctx.gate_state_intent || null, gate_state_post_failed: ctx.gate_state_post_failed || null }, frictionFields(ctx, 'completed')) } // ============================================================================= @@ -5150,7 +6037,12 @@ async function processIssue(pre) { // must NOT count as "shipped into TARGET" — see batchClosesIssues() below, // which is the sole reader of this field. const merged_into_target = pre.pr_state === 'merged' && pre.pr_base === TARGET - return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'skipped', pr: ctx.pr, follow_ups: [], stage: 'preflight', error: null, reason: pre.reason, members: memberIssues(ctx), merged_into_target: merged_into_target, changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice() }, frictionFields(ctx, 'skipped')) + // gate_state_intent/gate_state_post_failed: shape totality only -- no + // gate-state boundary can fire on this skip path (it never runs + // implementIssue/reviewAndMerge), so these are always null here and the + // Report-phase verify sweep (Task 4) reads that as 'no-intent', never as + // a mismatch. + return Object.assign({ issue: ctx.issue, title: ctx.title, status: 'skipped', pr: ctx.pr, follow_ups: [], stage: 'preflight', error: null, reason: pre.reason, members: memberIssues(ctx), merged_into_target: merged_into_target, changed_files: ctx.changed_files, added_files: ctx.added_files, touch_counts: ctx.touch_counts, gate_findings: ctx.gate_findings, settled: (ctx.settled || []).slice(), gate_state_intent: null, gate_state_post_failed: null }, frictionFields(ctx, 'skipped')) } if (pre.resume_point === 'process_pr') { log('#' + ctx.issue + ' healing: open PR #' + ctx.pr + ' found — jumping to review/merge') @@ -5379,6 +6271,7 @@ function __seed(o) { if ('ROOT' in o) ROOT = o.ROOT if ('ENGINE_OWNED' in o) ENGINE_OWNED = o.ENGINE_OWNED if ('LOCKSTEP_INSTALLED_PATHS' in o) LOCKSTEP_INSTALLED_PATHS = o.LOCKSTEP_INSTALLED_PATHS + if ('RUN_EPOCH' in o) RUN_EPOCH = o.RUN_EPOCH if ('MAX_CONTRARIAN_ITERATIONS' in o) MAX_CONTRARIAN_ITERATIONS = o.MAX_CONTRARIAN_ITERATIONS if ('OUTCOME_GRADING' in o) OUTCOME_GRADING = o.OUTCOME_GRADING if ('REVISIT_RISK' in o) REVISIT_RISK = o.REVISIT_RISK @@ -7247,6 +8140,20 @@ let preflights = (await Promise.all(issueList.map(function (it) { // deriveUnits()'s OR-fold for how it threads onto a consolidation-group unit. preflights = attachEngineOwnedIntentional(preflights, ENGINE_OWNED) +// ---- Select: gate-state read (issue #166 task 3) — READ-ONLY, safe under +// DRY_RUN (this whole block runs before the DRY_RUN early-return below), and +// placed right after the regime classifier it sits beside in PREFLIGHT_SCHEMA. +// priorWork is each preflight's OWN already-resolved pr_number/worktree_exists/ +// resume_point — passing it lets fetchGateStateBlocks' falsifiable-absent log +// line be evidence-driven rather than model-attested. See fetchGateStateBlocks' +// module comment (below the split) for the full design; attachGateStateBlocks +// (above the split) guarantees every preflight — even one a dead chunk never +// covered — comes out carrying all four gate-state fields, fail-open defaulted. +const gateStatePriorWork = {} +for (const p of preflights) gateStatePriorWork[p.issue] = { pr_number: p.pr_number, worktree_exists: p.worktree_exists, resume_point: p.resume_point } +const gateStateProbe = await fetchGateStateBlocks(preflights.map(function (p) { return p.issue }), gateStatePriorWork) +preflights = attachGateStateBlocks(preflights, gateStateProbe.rowsByIssue, gateStateProbe.self_login) + for (const p of preflights) log('#' + p.issue + ' preflight: ' + p.resume_point + ' — ' + p.reason) // ---- Select: engine-owned root-dirty skip — regime (a) of the three-regime @@ -7263,6 +8170,19 @@ if (engineSkip.flagged.length) log('engine-owned guardrail: root working tree di const learnR = await learnPromise const outcomeGradeR = await outcomeGradePromise const revisitRiskR = await revisitRiskPromise +// RUN_EPOCH (issue #166 task 3): derived here because both outcomeGradeR.now +// and revisitRiskR.now are already awaited above, and each already carries a +// `date -u +%Y-%m-%dT%H:%M:%SZ` wall-clock anchor from the exact same idiom +// (OUTCOMES_SCHEMA / REVISIT_RISK_SCHEMA's own step 0 — the sandbox has no +// Date.now()/argless `new Date()`) — no extra agent call needed. Prefer +// outcomeGradeR's since it fires first in program order; fall back to +// revisitRiskR's so a dead outcome-grading pass alone doesn't leave the whole +// run epoch-less. Logged loudly when both are unavailable/unparseable: every +// gate-state comparison this run then treats age as unknown, which +// gateStateEpochStale resolves toward stale, never toward silently trusting +// an old block. +RUN_EPOCH = deriveRunEpoch((outcomeGradeR && outcomeGradeR.now) || (revisitRiskR && revisitRiskR.now)) +if (RUN_EPOCH === null) log('gate-state: RUN_EPOCH is null — outcome-grading and revisit-risk both failed to supply a wall-clock reading this run; every gate-state block will read as unknown-age/stale') addStage('preflight', preflightR1Before) // STAGE_TOKENS.preflight R1 close — see the bracket comment above learnPromise if (learnR && learnR.found) { LEARN = learnR @@ -7712,6 +8632,21 @@ if (HELD_CLAIMS.length) { if (!swept || !swept.posted) log('claims-release sweep incomplete — stale "' + CLAIM_LABEL + '" labels expire via the ' + Math.round(CLAIM_STALE_SECONDS / 3600) + 'h staleness window') } +// ---- Gate-state self-validation sweep (issue #166, task 4) — proves +// post -> GitHub -> read -> parse round-tripped for every issue this run +// wrote a gate-state comment for. Advisory only (log lines, plus a +// VERIFY_SKIPS entry on mismatch/read-failed — see :5122 — never a result +// mutation) and non-fatal end to end: wrapped so a bug in the sweep itself +// can never take the rest of Report down with it. Runs BEFORE the +// token/friction/rework rollups below on purpose — those are pure +// JS-computed aggregations over `results` that must never depend on this +// sweep's (agent-backed, best-effort) outcome. ---- +try { + await verifyGateState(results) +} catch (e) { + log('gate-state-verify sweep threw (non-fatal): ' + String((e && e.message) || e).slice(0, 200)) +} + // ---- Token usage: JS-computed aggregation (no LLM math), injected verbatim below ---- const TOKEN_AGG = aggregateTokens(results, spentTokens(), CONCURRENCY, STAGE_TOKENS, POOL_SPEND)