fix: close review-cycle gaps (a stamp must change a stamp file, result requires a full --head and fails closed without headRefOid, digits-only --pr) with tests for every cycle-3 branch

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-26 02:56:14 +00:00
1 parent 4afaa98f54
commit 95cf833708
7 files changed
+54 -12

No files matched your search

+1 -1
View File
@@ -15,7 +15,7 @@
- **Gate edit (size budgets):** `test/helpers/carve-guards.ts` raises ship's `maxSkeletonBytes` 81,000 → 81,600 and `maxSizeRatio` 1.6690 → 1.6740 for Step 22 and the Step 18 routing, offset in part by tightening Steps 14, 16, 18 and 20 prose (measured skeleton 81,535, union 314,027; the 2026-10-14 tightening had left 2 bytes of headroom). No threshold, trial count or case kind changed.
### Tests
- `test/ship-queue-train.test.ts`: READY, train build (dry run and live) and verify --train refuse a missing, invalid, other-head and other-PR receipt and admit the current one; the pinned-head receipt admits a stamp commit; verify refuses `QUEUE_PROOF_MISSING` when gh cannot read the PR; `--screen` refuses a lagging or missing merge ref and never follows a 0-case screen. The train fixture's `refs/pull/<n>/merge` is now a real merge commit (`mergeRefCommit`), as GitHub keeps it. `test/gstack-ship-queue.test.ts`: `ready` at a stamp commit (slot 1, and slot 2 with main merged in) accepts the pin's receipt; a bare merge of main with no stamp on top, or an edit on the pin side under the slot-2 stamp, is `QUEUE_RECEIPT_STALE`. `test/auq-error-fallback-hook.test.ts`: the unattended directive names the same `GSTACK_RESULT` writers as docs/unattended.md and `gstack-skill-start` (GSTA-91). `test/gstack-ship-receipt.test.ts`: the `bindReceipt` head-matching table (12-character floor, other PR, one-line `SHIP_RECEIPT:`), `result --body` refusals, `--head`, a non-git cwd and gh failures. The train fixture's `openPr` now writes a receipt for the head it opens (`shipped: false` leaves the body empty). `test/install-verdict.test.ts` pins the GSTA-85 fix in both twins; `test/gstack-skill-start.test.ts` pins the new unattended /ship block.
- `test/ship-queue-train.test.ts`: READY, train build (dry run and live) and verify --train refuse a missing, invalid, other-head and other-PR receipt and admit the current one; the pinned-head receipt admits a stamp commit; verify refuses `QUEUE_PROOF_MISSING` when gh cannot read the PR; `--screen` refuses a lagging or missing merge ref and never follows a 0-case screen. The train fixture's `refs/pull/<n>/merge` is now a real merge commit (`mergeRefCommit`), as GitHub keeps it. `test/gstack-ship-queue.test.ts`: `ready` at a stamp commit (slot 1, and slot 2 with main merged in) accepts the pin's receipt; a bare merge of main with no stamp on top, an empty commit over the merge, or an edit on the pin side under the slot-2 stamp, is `QUEUE_RECEIPT_STALE`. `gstack-ship-receipt result --pr <n>` refuses `QUEUE_RECEIPT_STALE` when HEAD is not the PR's pushed head and prints `status=incomplete` when gh returns no head or the run record cannot be written; `ready --screen` names a failed fetch of the merge ref instead of blaming a conflict. `test/auq-error-fallback-hook.test.ts`: the unattended directive names the same `GSTACK_RESULT` writers as docs/unattended.md and `gstack-skill-start` (GSTA-91). `test/gstack-ship-receipt.test.ts`: the `bindReceipt` head-matching table (12-character floor, other PR, one-line `SHIP_RECEIPT:`), `result --body` refusals, `--head`, a non-git cwd and gh failures. The train fixture's `openPr` now writes a receipt for the head it opens (`shipped: false` leaves the body empty). `test/install-verdict.test.ts` pins the GSTA-85 fix in both twins; `test/gstack-skill-start.test.ts` pins the new unattended /ship block.
## [1.91.224.0] - 2026-10-25
+4 -2
View File
@@ -203,6 +203,7 @@ function cmdRead(rest: string[], json: boolean): number {
function cmdResult(rest: string[], json: boolean): number {
const bodyFile = flag(rest, '--body');
const pr = flag(rest, '--pr');
if (pr !== undefined && !/^\d+$/.test(pr)) { process.stderr.write(`gstack-ship-receipt result: --pr must be a PR number, got ${JSON.stringify(pr.slice(0, 40))}\n`); return EXIT.usage; }
const repoRoot = git(['rev-parse', '--show-toplevel']);
const head = flag(rest, '--head') ?? (repoRoot ? git(['rev-parse', 'HEAD'], repoRoot) : undefined);
const refused = (code: 'QUEUE_RECEIPT_MISSING' | 'QUEUE_RECEIPT_STALE', message: string) => {
@@ -211,7 +212,7 @@ function cmdResult(rest: string[], json: boolean): number {
return EXIT.refused;
};
if (!head) { process.stderr.write('gstack-ship-receipt result: --head <sha> is required outside a git repository\n'); return EXIT.usage; }
if (!/^[0-9a-f]{12,64}$/i.test(head)) { process.stderr.write(`gstack-ship-receipt result: --head must be a hex commit id of at least 12 characters, got ${JSON.stringify(head.slice(0, 40))}\n`); return EXIT.usage; }
if (!/^[0-9a-f]{40,64}$/i.test(head)) { process.stderr.write(`gstack-ship-receipt result: --head must be a full hex commit id, got ${JSON.stringify(head.slice(0, 40))}\n`); return EXIT.usage; }
const incomplete = (why: string) => {
process.stderr.write(`gstack-ship-receipt result: ${why}\n`);
process.stdout.write(json ? JSON.stringify({ skill: 'ship', status: 'incomplete', run: '-', message: why }) + '\n' : 'GSTACK_RESULT: skill=ship status=incomplete run=-\n');
@@ -229,7 +230,8 @@ function cmdResult(rest: string[], json: boolean): number {
if (!r.ok) return incomplete(`gh pr view failed: ${r.err.trim()}`);
let pushed: unknown;
try { const v = JSON.parse(r.out); body = v.body ?? ''; pushed = v.headRefOid; } catch { return incomplete('gh returned no JSON body'); }
if (typeof pushed === 'string' && /^[0-9a-f]{12,64}$/i.test(pushed) && !pushed.toLowerCase().startsWith(head.toLowerCase())) {
if (typeof pushed !== 'string' || !/^[0-9a-f]{12,64}$/i.test(pushed)) return incomplete(`gh pr view returned no headRefOid for PR #${pr}`);
if (!pushed.toLowerCase().startsWith(head.toLowerCase())) {
return refused('QUEUE_RECEIPT_STALE', `HEAD ${head.slice(0, 12)} is not PR #${pr}'s pushed head ${pushed.slice(0, 12)}; push it, then rerun gstack-ship-receipt result`);
}
source = `PR #${pr}${repo ? ` (${repo})` : ''}`;
+1 -1
View File
@@ -118,7 +118,7 @@ export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise<Tr
const again = `resolve the conflict with ${ctx.base} (or wait for GitHub to recompute refs/pull/${o.pr}/merge), push, then ${Q} ready --pr ${o.pr} --screen; a screen dispatched on a stale merge ref plans no cases`;
if (!/^[0-9a-f]{40,64}$/.test(mergeSha)) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `${env.remote} has no refs/pull/${o.pr}/merge (the PR conflicts with ${ctx.base}, or GitHub has not computed it yet)` }, lines, again, 'none');
const fetched = git(ctx.repoRoot, ['fetch', '-q', env.remote, `refs/pull/${o.pr}/merge`]);
if (!fetched.ok) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `could not fetch refs/pull/${o.pr}/merge from ${env.remote}: ${(fetched.err || fetched.out).trim().slice(0, 200) || 'git fetch failed'}` }, lines, `check the remote, then ${Q} ready --pr ${o.pr} --screen`, 'none');
if (!fetched.ok) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `could not fetch refs/pull/${o.pr}/merge from ${env.remote}: ${(fetched.err || fetched.out).trim().replace(/\s+/g, ' ').slice(0, 200) || 'git fetch failed'}` }, lines, `check the remote, then ${Q} ready --pr ${o.pr} --screen`, 'none');
const mergeParent = git(ctx.repoRoot, ['rev-parse', '--verify', '--quiet', `${mergeSha}^2`]).out;
if (mergeParent !== head) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `${env.remote}'s refs/pull/${o.pr}/merge (${mergeSha.slice(0, 12)}) merges ${mergeParent ? mergeParent.slice(0, 12) : 'no PR head'}, not the PR head ${head.slice(0, 12)}: GitHub has not recomputed it, which is what a conflicting PR looks like (CHANGELOG-only conflicts included)` }, lines, again, 'none');
const d = await dispatchScreen(env, o, mergeSha, baseSha);
+2 -1
View File
@@ -82,7 +82,8 @@ function acceptedReceiptHeads(ctx: QueueContext, policy: LoadedPolicy, head: str
const baseSha = git(root, ['rev-parse', `origin/${ctx.base}`]).out;
const onBase = (sha: string) => !!sha && git(root, ['merge-base', '--is-ancestor', sha, baseSha]).ok;
const merge = git(root, ['rev-parse', '--verify', '--quiet', `${head}^1`]).out;
if (!merge || !stampOnly(merge, head)) return [head];
const stamp = merge && merge !== head ? treeReceipt(ctx, policy, merge, head) : null;
if (!stamp || 'message' in stamp || stamp.tree !== 'same-modulo-stamps' || stamp.files.length === 0) return [head];
const parents = git(root, ['rev-list', '--parents', '-n', '1', merge]).out.split(' ').slice(1);
if (parents.length !== 2) return [head];
const pinSide = onBase(parents[0]!) ? parents[1]! : onBase(parents[1]!) ? parents[0]! : '';
+5
View File
@@ -330,6 +330,11 @@ describe('ready', () => {
const bare = queue(repo, ['ready', '--pr', '1', '--pregate', pregateFor(repo, bareMerge)], shim(bareMerge, shippedBody('1', pin)));
expect(bare.status).toBe(EXIT.refused);
expect(bare.err).toMatch(staleRe(bareMerge));
repo.git('commit', '-q', '--allow-empty', '-m', 'empty commit is not a stamp');
const emptyOnMerge = repo.git('rev-parse', 'HEAD');
const empty = queue(repo, ['ready', '--pr', '1', '--pregate', pregateFor(repo, emptyOnMerge)], shim(emptyOnMerge, shippedBody('1', pin)));
expect(empty.status).toBe(EXIT.refused);
expect(empty.err).toMatch(staleRe(emptyOnMerge));
repo.git('reset', '-q', '--hard', pin);
fs.appendFileSync(path.join(repo.work, 'pr-s.txt'), 'real change before the merge\n');
repo.git('commit', '-qam', 'edit on the pin side');
+25 -7
View File
@@ -154,9 +154,11 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => {
const head = repo.git('rev-parse', 'HEAD');
const state = path.join(repo.root, 'state');
const env = { ...ghShim(repo, {
5: { body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 5, head, base: 'main', queue_mode: 'stamp-at-merge' } as any) },
6: { body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 6, head: 'b'.repeat(40), base: 'main' } as any) },
7: { body: 'No receipt here.\n' },
5: { headRefOid: head, body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 5, head, base: 'main', queue_mode: 'stamp-at-merge' } as any) },
6: { headRefOid: head, body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 6, head: 'b'.repeat(40), base: 'main' } as any) },
7: { headRefOid: head, body: 'No receipt here.\n' },
9: { headRefOid: 'd'.repeat(40), body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 9, head, base: 'main' } as any) },
10: { body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 10, head, base: 'main' } as any) },
}), GSTACK_STATE_ROOT: state, GSTACK_SESSION_KIND: 'unattended' };
const ok = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '5'], env);
expect(ok.status).toBe(EXIT.ok);
@@ -179,6 +181,13 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => {
const missing = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '7', '--json'], env);
expect(missing.status).toBe(EXIT.refused);
expect(JSON.parse(missing.out)).toMatchObject({ skill: 'ship', status: 'refused', run: '-', code: 'QUEUE_RECEIPT_MISSING' });
const unpushed = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '9'], env);
expect([unpushed.status, unpushed.out.trim()]).toEqual([EXIT.refused, 'GSTACK_RESULT: skill=ship status=refused run=-']);
expect(unpushed.err).toContain(`HEAD ${head.slice(0, 12)} is not PR #9's pushed head dddddddddddd; push it, then rerun gstack-ship-receipt result`);
const noOid = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '10'], env);
expect([noOid.status, noOid.out.trim()]).toEqual([EXIT.fail, 'GSTACK_RESULT: skill=ship status=incomplete run=-']);
expect(noOid.err).toContain('gh pr view returned no headRefOid for PR #10');
expect(runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '--repo'], env).status).toBe(EXIT.usage);
const bodyFile = path.join(repo.work, '.gstack/tmp/ship-receipt.md');
fs.mkdirSync(path.dirname(bodyFile), { recursive: true });
@@ -189,6 +198,15 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => {
expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'run-out', 'ship-run.json'), 'utf8'))).toMatchObject({ head, pr: null, receipt_source: bodyFile, session_kind: 'unattended' });
expect(runBin(repo, 'gstack-ship-receipt', ['result', '--body', path.join(repo.root, 'absent.md')], env).status).toBe(EXIT.refused);
expect(runBin(repo, 'gstack-ship-receipt', ['result'], env).status).toBe(EXIT.usage);
const relative = runBin(repo, 'gstack-ship-receipt', ['result', '--body', bodyFile, '--out', 'rel-out'], env);
expect(relative.status).toBe(EXIT.ok);
expect(relative.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.work, 'rel-out')}`);
expect(fs.existsSync(path.join(repo.work, 'rel-out', 'ship-run.json'))).toBe(true);
const blocker = path.join(repo.root, 'not-a-dir');
fs.writeFileSync(blocker, 'file\n');
const unwritable = runBin(repo, 'gstack-ship-receipt', ['result', '--body', bodyFile, '--out', path.join(blocker, 'sub')], env);
expect([unwritable.status, unwritable.out.trim()]).toEqual([EXIT.fail, 'GSTACK_RESULT: skill=ship status=incomplete run=-']);
expect(unwritable.err).toContain(`cannot write the run record under ${path.join(blocker, 'sub')}`);
expect(fs.readdirSync(path.join(state, 'ship', 'runs')).length).toBe(1);
});
@@ -218,7 +236,7 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => {
const invalid = runBin(repo, 'gstack-ship-receipt', ['result', '--body', invalidFile], env);
expect([invalid.status, invalid.out.trim()]).toEqual([EXIT.refused, 'GSTACK_RESULT: skill=ship status=refused run=-']);
expect(invalid.err).toMatch(/the receipt's ship receipt is invalid \(.*\).*\(QUEUE_RECEIPT_MISSING/);
const otherPrEnv = { ...env, ...ghShim(repo, { 8: { body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 99, head, base: 'main' } as any) } }) };
const otherPrEnv = { ...env, ...ghShim(repo, { 8: { headRefOid: head, body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 99, head, base: 'main' } as any) } }) };
const otherPr = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '8'], otherPrEnv);
expect([otherPr.status, otherPr.out.trim()]).toEqual([EXIT.refused, 'GSTACK_RESULT: skill=ship status=refused run=-']);
expect(otherPr.err).toContain("PR #8's ship receipt names pr=99, not this PR");
@@ -229,10 +247,10 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => {
expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'pinned-out', 'ship-run.json'), 'utf8')).head).toBe('c'.repeat(40));
const badHead = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', '../../etc'], env);
expect([badHead.status, badHead.out]).toEqual([EXIT.usage, '']);
expect(badHead.err).toContain('--head must be a hex commit id of at least 12 characters');
const shortHead = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', 'abcdef1'], env);
expect(badHead.err).toContain('--head must be a full hex commit id');
const shortHead = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', head.slice(0, 12)], env);
expect([shortHead.status, shortHead.out]).toEqual([EXIT.usage, '']);
expect(shortHead.err).toContain('--head must be a hex commit id of at least 12 characters');
expect(shortHead.err).toContain('--head must be a full hex commit id');
const noGit = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-receipt-nogit-'));
const outside = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned], env, noGit);
expect([outside.status, outside.err.trim()]).toEqual([EXIT.usage, 'gstack-ship-receipt result: --head <sha> is required outside a git repository']);
+16
View File
@@ -12,6 +12,7 @@
* prune, ready's screen choice, and the policy gate.
*/
import { afterEach, describe, expect, test } from 'bun:test';
import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { EXIT } from '../lib/headless-artifacts';
@@ -215,6 +216,16 @@ describe('ready under a train policy: the screen choice', () => {
const missing = queue(fx, ['ready', '--pr', '1', '--pregate', pregate2, '--screen']);
expect(missing.status).toBe(EXIT.refused);
expect(missing.err).toMatch(/has no refs\/pull\/1\/merge.*\(QUEUE_SCREEN_STALE_REF\)/);
const tree = repo.git('rev-parse', `${moved}^{tree}`);
const main = repo.git('rev-parse', 'origin/main');
const badMerge = spawnSync('git', ['-C', repo.origin, 'hash-object', '-t', 'commit', '-w', '--stdin', '--literally'], { input: `tree ${tree}\nparent ${main}\nparent ${moved}\nauthor A <a@example.com> 1700000000 +0000\ncommitter B 1700000000 +0000\n\nmerge without a committer email\n`, encoding: 'utf8' }).stdout.trim();
spawnSync('git', ['-C', repo.origin, 'update-ref', 'refs/pull/1/merge', badMerge]);
repo.git('config', 'fetch.fsckObjects', 'true');
const unfetchable = queue(fx, ['ready', '--pr', '1', '--pregate', pregate2, '--screen']);
expect(unfetchable.status).toBe(EXIT.refused);
expect(unfetchable.err).toMatch(/could not fetch refs\/pull\/1\/merge from origin: .*\(QUEUE_SCREEN_STALE_REF\)/);
expect(line(unfetchable.out, 'NEXT:')).toBe('NEXT: check the remote, then gstack-ship-queue ready --pr 1 --screen');
repo.git('config', '--unset', 'fetch.fsckObjects');
expect(dispatches(fx)).toEqual([]);
expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'ready'))).toBe(false);
repo.git('push', '-q', '-f', 'origin', `${mergeRefCommit(fx, moved)}:refs/pull/1/merge`);
@@ -247,6 +258,11 @@ describe('ready under a train policy: the screen choice', () => {
expect(followed.status).toBe(EXIT.ok);
expect(line(followed.out, 'SCREEN_DISPATCH:')).toBe('SCREEN_DISPATCH: following run 102 (same merge SHA, base and policy; no duplicate)');
expect(dispatches(fx, 'evals.yml').length).toBe(3);
const liveBuild = queue(fx, ['train', 'build', '--prs', `1@${head}`, '--dry-run']);
expect([liveBuild.status, line(liveBuild.out, 'ORDER:'), line(liveBuild.out, 'SCREEN:')]).toEqual([EXIT.ok, 'ORDER: 1:#1(screened)', '']);
fx.write(d => { d.rules['evals.yml'] = { status: 'in_progress', conclusion: null }; });
const runningBuild = queue(fx, ['train', 'build', '--prs', `1@${head}`, '--dry-run']);
expect([runningBuild.status, line(runningBuild.out, 'ORDER:'), line(runningBuild.out, 'SCREEN:')]).toEqual([EXIT.ok, 'ORDER: 1:#1(screened)', '']);
}, 120_000);
});