From b1f78301dbc848832161c8ca32c82de2e848f469 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 11 Nov 2026 17:49:08 -0800 Subject: [PATCH] The lander must accept each owner-approved gate change before a PR merges (GST-2491) (#4304) * Let an owner recorded approval close a gate finding in unattended ship (GST-2491) An unattended /ship logs every gate finding open and nothing could close it, so a PR with an owner-approved gate change could never finish. gstack-approvals gate records the owner disposition outside the run; it closes only that fingerprint on a review row whose range holds the head, and finish, the receipt, the PR body and READY all carry it. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep the pinned review brief unchanged; document the resume steps for a dispositioned gate finding (GST-2491) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Bind a gate disposition to its gate change and show it as kept to the rest of /ship (GST-2491) A disposition now names the finding's gate_id as well as its fingerprint, so a later loosening of the same line stays open. gstack-review-read shows a closed gate finding as kept with the disposition's approval id, so the existing --blocking-open, --gate-open, draft and Gate edits rules carry it without prose changes, and an unreadable approval record applies no disposition instead of failing the reader. Review-Finding: p1/red-team#1 reader=red-team Review-Finding: s11/outside-voice#1 reader=outside-voice Review-Finding: s11/outside-voice#2 reader=outside-voice Review-Finding: s11/adversarial-native#1 reader=adversarial-native Review-Finding: s11/adversarial-native#2 reader=adversarial-native Review-Finding: s11/adversarial-native#4 reader=adversarial-native Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * List the Gate disposition line in the receipt and prose troubleshooting entries (GST-2491) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Require the lander to accept each owner gate disposition before merging (GST-2491) Land and a new read-only verify --pin re-derive the dispositions the approval store applies at the pin and merge only with --accept-gate-disposition for each one. The gate writer refuses while any worktree holds an open /ship run on the branch, under a lock; a row counts only in the writer's full shape with its digest; receipt write needs --body when a disposition applies; ready and land cross-check the receipt and body against the store. Supersedes #4299. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Regenerate the result-code anchors for the gate-disposition refusal (GST-2491) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Always record the receipt's gate dispositions and refuse a store that disagrees at merge (GST-2491) The receipt always carries gate_dispositions (an empty list when none apply) and write refuses on an unreadable store or a git error. Land, verify and ready refuse a receipt id the merge-time store lacks, naming the likely wrong state root or mirror, and a field-less receipt beside an applied row. A git ancestry error while deriving dispositions refuses instead of reading as not applied. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Count a disposition whose head a rewrite removed as not applied, and expect the receipt to end with gate_dispositions (GST-2491) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Trim and check the gate disposition recorder name, and let a parked run's ended lease stop blocking it (GST-2491) Review-Finding: s11/outside-voice#3 reader=outside-voice Review-Finding: s11/outside-voice#2 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-approvals | 37 ++- bin/gstack-doctor-components.sh | 1 + bin/gstack-review-read | 16 +- bin/gstack-ship-queue | 29 +- bin/gstack-ship-receipt | 9 +- ...ved-gate-changes-finish-unattended-ship.md | 4 + docs/ship-policy.md | 98 ++++++ docs/troubleshooting.md | 62 +++- lib/approval-record.ts | 2 + lib/gate-dispositions.ts | 222 ++++++++++++++ lib/headless-artifacts.ts | 3 + lib/plan-record.ts | 2 +- lib/result-codes-release.ts | 5 + lib/review-evidence.ts | 14 +- lib/ship-preflight.ts | 6 +- lib/ship-queue-land-direct.ts | 44 ++- lib/ship-queue.ts | 6 +- lib/ship-receipt-prose.ts | 4 +- lib/ship-receipt.ts | 4 +- lib/ship-release.ts | 17 +- scripts/lib/windows-curation.ts | 4 + ship/sections/pr-body.md | 4 +- ship/sections/pr-body.md.tmpl | 4 +- test/gstack-ship-receipt-docs.test.ts | 2 +- test/gstack-ship-receipt.test.ts | 4 +- test/ship-gate-dispositions.test.ts | 289 ++++++++++++++++++ test/ship-queue-land-direct.test.ts | 192 ++++++++++++ ...d-gate-changes-finish-unattended-ship.json | 4 + 28 files changed, 1050 insertions(+), 38 deletions(-) create mode 100644 changelog.d/owner-approved-gate-changes-finish-unattended-ship.md create mode 100644 lib/gate-dispositions.ts create mode 100644 test/ship-gate-dispositions.test.ts create mode 100644 test/size-ledger/ship/owner-approved-gate-changes-finish-unattended-ship.json diff --git a/bin/gstack-approvals b/bin/gstack-approvals index 3db93bf07..2cf9ff732 100755 --- a/bin/gstack-approvals +++ b/bin/gstack-approvals @@ -10,6 +10,8 @@ * gstack-approvals plan-check --plan * gstack-approvals rule --finding --head --reason --words --by [--json] * gstack-approvals rulings [--branch ] + * gstack-approvals gate --finding --gate-id --head --reason --words --by [--json] + * gstack-approvals dispositions [--branch ] * gstack-approvals lane-exception --head --files --reason --words --by [--json] * gstack-approvals list [--json] [--branch ] * gstack-approvals sync [--branch ] @@ -30,6 +32,7 @@ import { GSTACK_ACTOR, openShipRun, RULED_NOTE_SCOPE, ruledNoteRow, ruledNotes, import { resultLine } from '../lib/result-codes'; import { parsePlanLine, publicPlanLine, resolvePlan } from '../lib/plan-record'; import { TEST_LANE_EXCEPTION_SCOPE, testLaneExceptionRow } from '../lib/test-lane-exception'; +import { GATE_DISPOSITION_SCOPE, GATE_SOURCE, gateDispositionRow, gateDispositions, gateDispositionText, ONE_LINE, recordGateDisposition, validGateId } from '../lib/gate-dispositions'; const HELP = `gstack-approvals — the per-thread approval record (capy/skill/gstack/captain.md) @@ -43,6 +46,14 @@ const HELP = `gstack-approvals — the per-thread approval record (capy/skill/gs counts that finding closed on a fix-check whose range_base..commit holds the head, and the receipt and PR body list it; refused inside this checkout's open /ship run rulings the "Ruled note: …" lines the Pre-Landing Review and the PR body carry + gate --finding --gate-id --head + --reason --words --by + the owner's disposition of a gate finding an unattended /ship logged open (docs/ship-policy.md#gate-dispositions); + it closes that fingerprint and gate_id on a review row whose range_base..commit holds the head (a first pass: its + commit), and gstack-review-read shows it kept; + finish prints it, and the receipt, PR body and READY carry it; refused while any worktree holds an open /ship run on + the branch; the lander accepts each one with gstack-ship-queue land|verify --accept-gate-disposition + dispositions the "Gate disposition: …" lines the Pre-Landing Review and the PR body carry lane-exception --head --files --reason --words --by the owner's grant that a CI-carried lane counts although exactly those files are in no run's tree.json @@ -69,7 +80,7 @@ Example: ${exitTableHelp()}`; -const VALUE_FLAGS = new Set(['--ask', '--words', '--by', '--scope', '--pending', '--outcome', '--date', '--source', '--branch', '--autoplan', '--stack', '--finding', '--reason', '--head', '--files', '--plan']); +const VALUE_FLAGS = new Set(['--ask', '--words', '--by', '--scope', '--pending', '--outcome', '--date', '--source', '--branch', '--autoplan', '--stack', '--finding', '--reason', '--head', '--files', '--plan', '--gate-id']); function parse(args: string[]): { flags: Record; bools: Set; error?: string } { const flags: Record = {}; @@ -146,7 +157,7 @@ function main(argv: string[]): number { const json = bools.has('--json'); const recorded = (where: StoreLocation) => `${where.file}${where.mirror ? ` mirror=${where.mirror}` : ''}`; - if (cmd === 'add' || cmd === 'import' || cmd === 'rule' || cmd === 'lane-exception') { + if (cmd === 'add' || cmd === 'import' || cmd === 'rule' || cmd === 'lane-exception' || cmd === 'gate') { let rows; if (cmd === 'lane-exception') { const invalid = (message: string) => fail(cmd, { code: 'APPROVAL_RECORD_INVALID', message }); @@ -174,8 +185,25 @@ function main(argv: string[]): number { const built = buildRow({ ...ruledNoteRow(finding, head.stdout.trim(), reason), words: flags['--words'] ?? '', by: flags['--by'] ?? '', date: flags['--date'] }, loc.branch); if ('code' in built) return fail(cmd, built); rows = [built]; + } else if (cmd === 'gate') { + const invalid = (message: string) => fail(cmd, { code: 'APPROVAL_RECORD_INVALID', message }); + const finding = (flags['--finding'] ?? '').trim(), gateId = (flags['--gate-id'] ?? '').trim(), reason = (flags['--reason'] ?? '').trim(), words = (flags['--words'] ?? '').trim(), by = (flags['--by'] ?? '').trim(); + if (!finding || /\s/.test(finding) || !validGateId(gateId) || !reason) return invalid('gate needs --finding , --gate-id , both without spaces (a finding with no gate_id, or a literal undefined, cannot be dispositioned), and --reason '); + if (!words) return invalid('gate needs --words : a gate disposition is the owner\'s, in the owner\'s words'); + if (!ONE_LINE.test(reason) || !ONE_LINE.test(words) || !ONE_LINE.test(by)) return invalid('--reason, --words and --by are one line each: no newline or control character'); + const head = spawnSync('git', ['rev-parse', '--verify', '--quiet', `${flags['--head'] ?? ''}^{commit}`], { encoding: 'utf8', timeout: 10_000 }); + if (!flags['--head'] || head.status !== 0) return invalid('gate needs --head '); + if (GSTACK_ACTOR.test(by)) return invalid('--by names gstack; a gate disposition is the owner\'s, so --by names who recorded it'); + const built = buildRow({ ...gateDispositionRow(finding, gateId, head.stdout.trim(), reason), words, by, date: flags['--date'], source: GATE_SOURCE }, loc.branch); + if ('code' in built) return fail(cmd, built); + const res = recordGateDisposition(process.cwd(), loc, built); + if ('code' in res) return fail(cmd, res); + if (json) process.stdout.write(JSON.stringify({ added: res.added.map(r => r.id), skipped: res.skipped, file: loc.file, mirror: loc.mirror ?? null }) + '\n'); + else process.stdout.write(`APPROVAL_RECORDED: ${built.id}${res.skipped ? ' (already present)' : ''} ${recorded(loc)}\n`); + return res.mirrorError ? fail(cmd, res.mirrorError) : EXIT.ok; } else if (cmd === 'add') { if (RULED_NOTE_SCOPE.test(flags['--scope'] ?? '')) return fail(cmd, { code: 'APPROVAL_RECORD_INVALID', message: 'a ruled_note scope is recorded with gstack-approvals rule' }); + if (GATE_DISPOSITION_SCOPE.test(flags['--scope'] ?? '') || (flags['--source'] ?? '').trim() === GATE_SOURCE) return fail(cmd, { code: 'APPROVAL_RECORD_INVALID', message: 'a gate_disposition scope is recorded with gstack-approvals gate' }); if (TEST_LANE_EXCEPTION_SCOPE.test(flags['--scope'] ?? '')) return fail(cmd, { code: 'APPROVAL_RECORD_INVALID', message: 'a test_lane_exception scope is recorded with gstack-approvals lane-exception' }); const built = buildRow({ ask: flags['--ask'] ?? '', words: flags['--words'] ?? '', by: flags['--by'] ?? '', scope: flags['--scope'] ?? '', pending: flags['--pending'], outcome: flags['--outcome'], date: flags['--date'], source: flags['--source'] }, loc.branch); if ('code' in built) return fail(cmd, built); @@ -230,6 +258,11 @@ function main(argv: string[]): number { process.stdout.write(notes.length ? notes.map(n => `Ruled note: ${ruledNoteText(n)}\n`).join('') : 'RULED_NOTES: none\n'); return EXIT.ok; } + if (cmd === 'dispositions') { + const ds = gateDispositions(readRecord(loc).rows, loc.branch); + process.stdout.write(ds.length ? ds.map(d => `Gate disposition: ${gateDispositionText(d)}\n`).join('') : 'GATE_DISPOSITIONS: none\n'); + return EXIT.ok; + } if (cmd === 'sync') { const s = syncRecord(loc); process.stdout.write(`APPROVAL_SYNCED: rows=${s.rows} ${recorded(loc)}\n`); diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index e84015ad7..157afdf0d 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -355,6 +355,7 @@ gstack_result_anchor() { QUEUE_LAND_BASE_BUSY) printf '%s' 'queue-land-base-busy' ;; QUEUE_LAND_STAMPED) printf '%s' 'queue-land-stamped' ;; QUEUE_LAND_PIN_MOVED) printf '%s' 'queue-land-pin-moved' ;; + QUEUE_GATE_DISPOSITION) printf '%s' 'queue-gate-disposition' ;; QUEUE_LAND_CHECKS) printf '%s' 'queue-land-checks' ;; QUEUE_LAND_MERGED_CHECKS) printf '%s' 'queue-land-merged-checks' ;; QUEUE_LAND_NOT_CLEAN) printf '%s' 'queue-land-not-clean' ;; diff --git a/bin/gstack-review-read b/bin/gstack-review-read index 9b1530c62..a030203cb 100755 --- a/bin/gstack-review-read +++ b/bin/gstack-review-read @@ -7,7 +7,9 @@ # bin/gstack-wtree --review, or "unknown"), ---TREE--- (HEAD tree, informational) and # ---DIRTY--- (tracked-file dirty flag). Consumers grade diff-scoped review # rows using review_freshness; legacy or uncaptured diff evidence cannot fall -# back to HEAD equality. Plan rows are unchanged. +# back to HEAD equality. Plan rows are unchanged. A gate finding the owner's gstack-approvals +# gate row closes reads `kept` with its `disposition`, and the row's `gate_dispositions` lists +# those approval ids (lib/gate-dispositions.ts); values the row itself carried are discarded. set -euo pipefail GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" @@ -21,14 +23,22 @@ gstack_state_root_select GSTACK_HOME="$_gstack_sr_root" WTREE=$("$SCRIPT_DIR/gstack-wtree" --review 2>/dev/null || echo "unknown") if [ -f "$GSTACK_HOME/projects/$SLUG/$BRANCH-reviews.jsonl" ]; then - GSTACK_REVIEW_LIB="$SCRIPT_DIR/../lib/review-evidence.ts" GSTACK_REVIEW_WTREE="$WTREE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' + GSTACK_REVIEW_LIB="$SCRIPT_DIR/../lib/review-evidence.ts" GSTACK_GATE_LIB="$SCRIPT_DIR/../lib/gate-dispositions.ts" GSTACK_REVIEW_WTREE="$WTREE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const { reviewFreshness } = await import(process.env.GSTACK_REVIEW_LIB); + const { dispositionedOn, gateKey, readGateDispositions, showDispositioned } = await import(process.env.GSTACK_GATE_LIB); + let dispositions = []; + try { dispositions = readGateDispositions(process.cwd()); } + catch (e) { console.error("gstack-review-read: gate dispositions unreadable, none applied: " + e.message); } for (const line of (await Bun.stdin.text()).split("\n").filter(Boolean)) { try { const rec = JSON.parse(line); delete rec.review_freshness; - const freshness = reviewFreshness(rec, process.env.GSTACK_REVIEW_WTREE); + delete rec.gate_dispositions; + for (const f of Array.isArray(rec.findings) ? rec.findings : []) if (f && typeof f === "object") delete f.disposition; + const closed = dispositionedOn(process.cwd(), rec, dispositions); + const freshness = reviewFreshness(rec, process.env.GSTACK_REVIEW_WTREE, new Set(), new Set(closed.map(d => gateKey(d.finding, d.gate_id)))); if (freshness) rec.review_freshness = freshness; + showDispositioned(rec, closed); console.log(JSON.stringify(rec)); } catch { console.error("gstack-review-read: skipping malformed review row"); } } diff --git a/bin/gstack-ship-queue b/bin/gstack-ship-queue index 0b38d53ea..3da9e6ce9 100755 --- a/bin/gstack-ship-queue +++ b/bin/gstack-ship-queue @@ -14,7 +14,8 @@ * gstack-ship-queue gate-ahead --list --pr * gstack-ship-queue verify --pr --screen (train policy: the head's screen verdict from its run) * gstack-ship-queue verify --post-merge [--timeout ] (after a merge: main's push workflows at that commit until they settle) - * gstack-ship-queue land --pr --pin [--repair] [--dry-run] [--allow-repo-commands] [--break-lock] (merge: direct; lib/ship-queue-land-direct.ts) + * gstack-ship-queue land --pr --pin [--accept-gate-disposition ]... [--repair] [--dry-run] [--allow-repo-commands] [--break-lock] (merge: direct; lib/ship-queue-land-direct.ts) + * gstack-ship-queue verify --pr --pin [--accept-gate-disposition ]... (land's receipt and gate-disposition acceptance, read-only, for a lander that merges another way) * gstack-ship-queue land --pr [--dry-run] [--no-wait] [--break-lock [--break-train-lock]] (the merge step; lib/ship-queue-land.ts) * gstack-ship-queue land --pr --repair (after QUEUE_LAND_DRIFT: open the outside merge's release PR) * gstack-ship-queue land --census @@ -59,15 +60,20 @@ const HELP = `gstack-ship-queue — the shared merge queue's protocol steps (doc gate-ahead --record --pr --attempt --exit --log bind the gate result to that attempt's gated tree; removes its worktree gate-ahead --list --pr attempts for a PR, newest last (the current one) + verify --pr --pin [--accept-gate-disposition ]... + land --pin's receipt and gate-disposition check, read-only, for a lander that merges + another way: prints each GATE_DISPOSITION line and exits 0 only when every one is accepted verify --pr --screen (train policy) the PR head's screen verdict read from its run (SCREEN_VERDICT) verify --post-merge [--timeout ] wait for every check run at a merged commit to settle: POST_MERGE: green|red|pending and one CHECK: line per workflow; resumable, default timeout 2700 s - land --pr --pin [--repair] [--dry-run] [--allow-repo-commands] [--break-lock] + land --pr --pin [--accept-gate-disposition ]... [--repair] [--dry-run] [--allow-repo-commands] [--break-lock] merge: direct: merge the PR at its own green head (receipt and READY at the pin, no stamp files, checks green, mergeable clean), verify the tree, write a land note; a stale base runs land.merged_checks, repo commands, only with --allow-repo-commands (never --dry-run); it holds the land lock from the re-fetch to the merge (--break-lock: a stale one); - --repair: while frozen, land the one repair PR the freeze admits (recorded under the lock) + --repair: while frozen, land the one repair PR the freeze admits (recorded under the lock); + an owner gate disposition the approval store applies at the pin merges only when the + lander passes --accept-gate-disposition (docs/ship-policy.md#gate-dispositions) land --pr [--dry-run] [--no-wait] [--timeout ] [--break-lock [--break-train-lock]] [--allow-repo-commands] the merge step: lock, merge the base, stamp (fragments folded), push the stamp commit, wait for its checks, squash with --match-head-commit, confirm the base tree; resumable @@ -89,6 +95,12 @@ function flag(args: string[], name: string): string | undefined { return i >= 0 ? args[i + 1] : undefined; } +/** Every --accept-gate-disposition value, or the first one that is not an approval id. */ +function acceptedIds(args: string[]): { ids: string[]; bad?: string } { + const ids = args.flatMap((a, i) => a === '--accept-gate-disposition' ? [args[i + 1] ?? ''] : []); + return { ids, bad: ids.find(id => !/^a-[0-9a-f]{12}$/.test(id)) }; +} + function finish(cmd: string, out: StepOutput): number { for (const line of out.lines) process.stdout.write(line + '\n'); if (out.failure) { @@ -187,7 +199,9 @@ async function main(argv: string[]): Promise { if (args.includes('--pin')) { const pin = flag(args, '--pin') ?? ''; if (!/^[0-9a-f]{40,64}$/.test(pin)) return usage('--pin needs the full head sha the READY names'); - return finish('land', (await import('../lib/ship-queue-land-direct')).landDirect(ctx, { pr, pin, dryRun: args.includes('--dry-run'), allowRepoCommands: args.includes('--allow-repo-commands'), breakLock: args.includes('--break-lock'), repair: args.includes('--repair') })); + const accept = acceptedIds(args); + if (accept.bad !== undefined) return usage(`--accept-gate-disposition needs an approval id (a-<12 hex>), not ${JSON.stringify(accept.bad)}`); + return finish('land', (await import('../lib/ship-queue-land-direct')).landDirect(ctx, { pr, pin, acceptGateDispositions: accept.ids, dryRun: args.includes('--dry-run'), allowRepoCommands: args.includes('--allow-repo-commands'), breakLock: args.includes('--break-lock'), repair: args.includes('--repair') })); } const timeout = flag(args, '--timeout'); return finish('land', l.land(ctx, { pr, repair: args.includes('--repair'), dryRun: args.includes('--dry-run'), noWait: args.includes('--no-wait'), breakLock: args.includes('--break-lock'), breakTrainLock: args.includes('--break-train-lock'), allowRepoCommands: args.includes('--allow-repo-commands'), waitTimeoutMin: timeout && /^\d+$/.test(timeout) ? Number(timeout) : undefined })); @@ -215,6 +229,13 @@ async function main(argv: string[]): Promise { failAfter: process.env.GSTACK_SHIP_QUEUE_FAIL_AFTER, })); } + if (cmd === 'verify' && args.includes('--pin')) { + const pin = flag(args, '--pin') ?? ''; + if (!/^[0-9a-f]{40,64}$/.test(pin)) return usage('--pin needs the full head sha the READY names'); + const accept = acceptedIds(args); + if (accept.bad !== undefined) return usage(`--accept-gate-disposition needs an approval id (a-<12 hex>), not ${JSON.stringify(accept.bad)}`); + return finish('verify', (await import('../lib/ship-queue-land-direct')).verifyPin(ctx, { pr, pin, acceptGateDispositions: accept.ids })); + } if (cmd === 'verify') { if (flag(args, '--train') || args.includes('--screen')) return (await import('../crew/lib/ship-queue-train-cli')).verifyMain(ctx, args, pr); const record = flag(args, '--record'); diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 17dc8bb0e..5709d718e 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -58,6 +58,7 @@ import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; import { readPause, receiptBaseRed } from '../lib/base-red'; import { readRuledNotes, ruledNoteText } from '../lib/ruled-notes'; +import { appliedGateDispositions, gateDispositionText } from '../lib/gate-dispositions'; import { laneExceptionBodyLines } from '../lib/test-lane-exception'; import { readMechanicalLines } from '../lib/ship-mechanical'; import { isRegistryUnavailable, loadRegistryFromBase } from '../lib/regen'; @@ -270,6 +271,12 @@ function cmdWrite(rest: string[], json: boolean): number { const ruledNotes = readRuledNotes(repoRoot); const unlisted = bodyFile ? ruledNotes.map(n => `Ruled note: ${ruledNoteText(n)}`).find(l => !bodyText.split(/\r?\n/).some(b => b.trim() === l)) : undefined; if (unlisted) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${bodyFile} does not list ruled note ${JSON.stringify(unlisted)} (gstack-approvals rulings prints each line the body carries); nothing written`) + '\n'); return EXIT.fail; } + let gateDispositions: ReturnType['applied']; + try { gateDispositions = appliedGateDispositions(repoRoot, { pin: 'HEAD' }).applied; } + catch (e: any) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `the owner gate dispositions at HEAD cannot be derived (${e.message}); the receipt always records them, so an unreadable approval store or a git error writes nothing`) + '\n'); return EXIT.fail; } + if (gateDispositions.length && !bodyFile) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `the approval store applies ${gateDispositions.length} owner gate disposition(s) at HEAD (${gateDispositions.map(d => d.approval).join(', ')}), so write needs --body carrying each "Gate disposition: …" line gstack-approvals dispositions prints; nothing written`) + '\n'); return EXIT.fail; } + const undisclosed = bodyFile ? gateDispositions.map(d => `Gate disposition: ${gateDispositionText(d)}`).find(l => !bodyText.split(/\r?\n/).some(b => b.trim() === l)) : undefined; + if (undisclosed) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${bodyFile} does not list gate disposition ${JSON.stringify(undisclosed)} (gstack-approvals dispositions prints each line the body carries); nothing written`) + '\n'); return EXIT.fail; } const carried = ciCarried(base); const unlistedException = bodyFile ? laneExceptionBodyLines(carried).find(l => !bodyText.split(/\r?\n/).some(b => b.trim() === l)) : undefined; if (unlistedException) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${bodyFile} does not list ${JSON.stringify(unlistedException)} (accept's TEST_LANE_EXCEPTION: line; the body carries it as one line); nothing written`) + '\n'); return EXIT.fail; } @@ -331,7 +338,7 @@ function cmdWrite(rest: string[], json: boolean): number { policy: flag(rest, '--policy') ?? tree?.policy, queue_mode: flag(rest, '--queue-mode'), preregistration: flag(rest, '--preregistration'), history: flag(rest, '--history'), ...pregate, issues: issuesFile ? receiptIssuesFrom(issuesFile) : undefined, approval: approval.kind === 'ok' ? approval.approval : undefined, review_loop: reviewLoop, ...(docs?.ok ? { docs: docs.docs } : {}), ...product, - gstack_version: installedGstackVersion(), review_depth: reviewDepth, review_loop_exception: loopException?.ok ? loopException.exception : undefined, ruled_notes: ruledNotes.length ? ruledNotes : undefined, mechanical: mechanical.length ? mechanical : undefined, review_targeted: targeted?.ok ? targeted.targeted : undefined, + gstack_version: installedGstackVersion(), review_depth: reviewDepth, review_loop_exception: loopException?.ok ? loopException.exception : undefined, ruled_notes: ruledNotes.length ? ruledNotes : undefined, gate_dispositions: gateDispositions, mechanical: mechanical.length ? mechanical : undefined, review_targeted: targeted?.ok ? targeted.targeted : undefined, design: review.frontend ? { mode: 'static', reason: 'no preview declared' } : undefined, base_red: pause ? receiptBaseRed(pause) : undefined, review_exclude: receiptExclude(review.exclude), diff --git a/changelog.d/owner-approved-gate-changes-finish-unattended-ship.md b/changelog.d/owner-approved-gate-changes-finish-unattended-ship.md new file mode 100644 index 000000000..18be68962 --- /dev/null +++ b/changelog.d/owner-approved-gate-changes-finish-unattended-ship.md @@ -0,0 +1,4 @@ +--- +important: blocker +--- +- **An unattended /ship can now finish a PR that carries an owner-approved gate change, and the lander has to accept each one before it merges.** When the review finds a loosened gate (a test threshold raised, a check skipped), an unattended run logs it as an open gate finding, because nobody is there to answer the gate question, and until now nothing could close it, so the run stopped for good. That blocked founderstar#906, whose gate change Garry had approved. The owner's answer can now be recorded from outside the run with `gstack-approvals gate --finding --gate-id --head --reason '' --words '' --by ''`. It closes only that finding, only on a review whose range holds that head, and the review reader then shows it as kept so the rest of /ship carries on. It is refused while a /ship run is open on that branch in any worktree, without the owner's words, for a literal `undefined` gate id, or when `--by` names gstack. A row that is missing a field the writer writes, or was edited after it was written, is ignored. The lane runs as the same user, so it could still write a row that looks right. That's why the lander decides: `gstack-ship-queue land --pin` works the dispositions out again from the approval store at the pin, prints each one, and merges only with `--accept-gate-disposition ` for every one. It also refuses when the receipt or the PR body leaves one out or invents one. `gstack-ship-queue verify --pr --pin ` runs the same check without merging, for a lander that merges another way, and `gstack-ship-receipt write` no longer skips the disclosure check when `--body` is left off. Gate findings are logged exactly as before, and a ruling or anything the lane writes on its own review row still closes nothing. A disposition recorded at a commit that a later rewrite removed from the clone just stops applying instead of blocking the branch, and a shallow landing clone is told to deepen itself. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index e84f35c8d..6797bcea6 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1718,6 +1718,103 @@ Pre-Landing Review and the PR body, `gstack-ship-receipt write` records them as `ruled_notes` (each with its `head`) and refuses a `--body` that does not carry each line word for word, and the READY block prints each as `RULED_NOTE:`. + +**Gate dispositions (GST-2491).** A ruling never closes a gate finding (a loosened +gate the review logged with a `gate` tag and the hunk identity `gate_id`). In an +interactive session the human answers `review-gate-disposition`; in an unattended +one `gstack-review-log` logs every gate finding `open`, and the log stays so. When +the owner approved the gate change, the owner's answer goes through the approval +record instead, recorded outside the lane's /ship run at the stop head, one finding +per row: + +```bash +gstack-approvals gate --finding '' --gate-id --head \ + --reason '' --words '' --by '' +``` + +The row is an approved `gstack-approvals` row with scope `gate_disposition + `, ask `gate_disposition at +: ` and the owner's words in `words` (`lib/gate-dispositions.ts`). +`gate` refuses a missing `--gate-id` (or a literal `undefined` or `null`), empty +`--words`, a reason, words or `--by` with a newline or control character (each is +trimmed first), a `--head` that is not a commit, a `--by` naming gstack, a row that +`dispositions` would not list (a head that is not 40 hex, for one), and any call +while a /ship run is open on the branch in any worktree of the repository: its +marker counts while its lease is open, the same test preflight uses, so a run parked +before finish stops counting once its 120-minute lease ends. It finds those worktrees +through the git common dir, and it checks and appends under a lock there that +/ship's preflight also takes to write its marker. `add` refuses the scope and +`--source gate`, so `gate` is the only writer. A row counts only when it has every +field `gate` writes: an approval-shaped id that its hash reproduces, `source: gate`, +`pending: none`, the store's own branch, `date`, `recorded_at`, `by`, one-line +`words` and a `digest` over those fields. The digest catches a row that was edited +or typed by hand. It isn't authentication: anyone who can read the code can compute +it. A gate finding with no `gate_id` is never closed, and a row gstack wrote is +ignored. A +disposition closes a gate finding only on a review row that carries that exact +fingerprint and `gate_id` and whose `range_base..commit` holds the head (a first +pass, with no `range_base`: its own commit), so a later, different loosening of the +same line (a new `gate_id`) stays open. Anything else closes nothing, and a +review-log field (`kept`, `reason`, `disposition`, `gate_dispositions` or any claim +the lane writes on its row) never makes one. `gstack-review-read` shows each closed +finding as `kept` with `reason: owner disposition : ` and +`disposition: `, lists the ids in the row's `gate_dispositions` and grades the +row as if the findings were kept, so /ship's gate rules (`--blocking-open`, +`--gate-open`, the draft state, the `Gate edits:` counts) read them as kept once it +reads the row again. As with rulings, a row derived from a findings file +(`status_source: derived`) is not cleared. Nothing is hidden: finish prints each as +`GATE_DISPOSITION: [gate ] at by at