diff --git a/bin/gstack-ship-queue b/bin/gstack-ship-queue index 3575a29da..30c1c051c 100755 --- a/bin/gstack-ship-queue +++ b/bin/gstack-ship-queue @@ -166,8 +166,8 @@ async function main(argv: string[]): Promise { if (cmd === 'train') { const train = await import('../lib/ship-queue-train-cli'); if (!args[0] || args[0].startsWith('-')) { process.stdout.write(train.TRAIN_HELP + '\n'); return args.includes('--help') || args.includes('-h') ? EXIT.ok : EXIT.usage; } - const loaded = args[0] === 'build' ? loadBasePolicy(ctx) : null; - if (loaded && !('message' in loaded) && loaded.policy.merge === 'direct') return finish('train build', { ok: false, lines: [], run: 'none', failure: { code: 'QUEUE_NOT_READY', message: 'the base policy sets merge: direct, so trains are off', fix: 'land each READY PR with gstack-ship-queue land --pr --pin (docs/ship-policy.md#land-direct)' } }); + const loaded = args[0] === 'build' || args[0] === 'land' ? loadBasePolicy(ctx) : null; + if (loaded && !('message' in loaded) && loaded.policy.merge === 'direct') return finish(`train ${args[0]}`, { ok: false, lines: [], run: 'none', failure: { code: 'QUEUE_NOT_READY', message: 'the base policy sets merge: direct, so trains are off', fix: 'land each READY PR with gstack-ship-queue land --pr --pin (docs/ship-policy.md#land-direct)' } }); return train.trainMain(ctx, args); } if (cmd === 'wave') return (await import('../lib/ship-queue-wave')).waveMain(ctx, args); diff --git a/lib/ship-queue-land-direct.ts b/lib/ship-queue-land-direct.ts index 4c1b663b1..dcc08dc98 100644 --- a/lib/ship-queue-land-direct.ts +++ b/lib/ship-queue-land-direct.ts @@ -35,18 +35,31 @@ export function stampedFiles(root: string, policy: ShipPolicy, mergeBase: string const npm = (rev: string) => { const r = git(root, ['show', `${rev}:package.json`]); try { return r.ok ? JSON.parse(r.out).version ?? null : null; } catch { return null; } }; if (!('rel' in now && now.rel === 'package.json') && npm(mergeBase) !== npm(pin)) out.push('package.json version'); if (policy.changelog && git(root, ['diff', '--quiet', mergeBase, pin, '--', policy.changelog]).ok === false) out.push(policy.changelog); + if (!git(root, ['diff', '--quiet', mergeBase, pin, '--', '.gstack/version-path']).ok) out.push('.gstack/version-path'); return out; } -/** The merge commit's tree is the pin merged onto the merge commit's first parent: GitHub merged exactly the pin. */ +/** The pin's commit is local, fetched from its branch or GitHub's refs/pull//head (which outlives a deleted branch). */ +function havePin(root: string, pr: string, pin: string, branch?: string): boolean { + const have = () => git(root, ['cat-file', '-e', `${pin}^{commit}`]).ok; + if (have()) return true; + if (branch) git(root, ['fetch', '-q', 'origin', `refs/heads/${branch}`]); + if (!have()) git(root, ['fetch', '-q', 'origin', `refs/pull/${pr}/head`]); + return have(); +} + +/** The merge commit's tree is the pin merged onto the merge commit's first parent: GitHub merged exactly the pin. A read that fails is "cannot verify", never a mismatch. */ function verifyMerge(ctx: QueueContext, root: string, pr: string, pin: string, merge: string, lines: string[], run: string, now: Date): StepOutput { - git(root, ['fetch', '-q', 'origin', ctx.base]); + const cannot = (why: string): StepOutput => ({ ok: false, run, lines, failure: { code: 'QUEUE_NOT_READY', message: `#${pr} merged as ${merge.slice(0, 12)}, but land cannot verify it: ${why}`, fix: `fix the read named, then rerun gstack-ship-queue land --pr ${pr} --pin ${pin}; it goes straight to the tree check` } }); + if (!git(root, ['fetch', '-q', 'origin', ctx.base]).ok) return cannot(`git fetch origin ${ctx.base} failed`); + if (!havePin(root, pr, pin)) return cannot(`the pin ${pin.slice(0, 12)} is not in this clone and refs/pull/${pr}/head did not fetch it`); if (!git(root, ['merge-base', '--is-ancestor', merge, `origin/${ctx.base}`]).ok) return { ok: false, run, lines, failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `the merge commit ${merge.slice(0, 12)} is not on origin/${ctx.base}` } }; const parent = git(root, ['rev-parse', `${merge}^1`]).out; const tree = git(root, ['rev-parse', `${merge}^{tree}`]).out; const mt = git(root, ['merge-tree', '--write-tree', parent, pin]); - const expected = mt.ok ? mt.out.split('\n')[0] : ''; - if (!isSha(expected) || tree !== expected) return { ok: false, run, lines: [...lines, `MAIN: ${merge} tree=${tree} parent=${parent} expected=${expected || 'conflict'}`], failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `origin/${ctx.base} ${merge.slice(0, 12)} is not the pin ${pin.slice(0, 12)} merged onto ${parent.slice(0, 12)}`, fix: `compare git diff ${expected || parent} ${merge} and revert what does not belong to #${pr}; report it to the Captain` } }; + const expected = mt.out.split('\n')[0] ?? ''; + if (!isSha(expected)) return cannot(`git merge-tree ${parent.slice(0, 12)} ${pin.slice(0, 12)} failed: ${mt.err.split('\n')[0]}`); + if (!mt.ok || tree !== expected) return { ok: false, run, lines: [...lines, `MAIN: ${merge} tree=${tree} parent=${parent} expected=${expected || 'conflict'}`], failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `origin/${ctx.base} ${merge.slice(0, 12)} is not the pin ${pin.slice(0, 12)} merged onto ${parent.slice(0, 12)}`, fix: `compare git diff ${expected || parent} ${merge} and revert what does not belong to #${pr}; report it to the Captain` } }; const keys = repoIdentity(root); const at = now.toISOString(); const note = persist(root, [{ schema: 1, kind: 'land', repo_key: keys.key, pr, pin, base_sha: parent, merge_sha: merge, tree, landed_at: at, updated_at: at }], { remote: ctx.remote }); @@ -59,6 +72,7 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { const run = 'none'; const now = o.now ?? (() => new Date()); const fail = (lines: string[], failure: QueueFailure): StepOutput => ({ ok: false, lines, failure, run }); + if (!git(root, ['fetch', '-q', 'origin', ctx.base]).ok) return fail([], { code: 'QUEUE_NOT_READY', message: `cannot fetch origin/${ctx.base}; land reads the policy and the required checks from the current base`, fix: 'fix the fetch, then rerun land' }); const loaded = loadBasePolicy(ctx); if ('message' in loaded) return fail([], loaded); const policy = loaded.policy; @@ -76,8 +90,7 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { if (info.state !== 'OPEN') return fail(lines, { code: 'QUEUE_NOT_READY', message: `#${o.pr} is ${info.state}` }); if (info.headRefOid !== o.pin) return fail(lines, { code: 'QUEUE_LAND_PIN_MOVED', message: `#${o.pr}'s head is ${info.headRefOid.slice(0, 12)}, not the pin ${o.pin.slice(0, 12)}`, fix: `the lane runs /ship and gstack-ship-queue ready --pr ${o.pr} at the new head and reports READY with it` }); - git(root, ['fetch', '-q', 'origin', ctx.base]); - if (!git(root, ['cat-file', '-e', `${o.pin}^{commit}`]).ok && !git(root, ['fetch', '-q', 'origin', `refs/heads/${info.headRefName}`]).ok) git(root, ['fetch', '-q', 'origin', `refs/pull/${o.pr}/head`]); + if (!havePin(root, o.pr, o.pin, info.headRefName)) return fail(lines, { code: 'QUEUE_NOT_READY', message: `cannot fetch the pin ${o.pin.slice(0, 12)} from ${info.headRefName} or refs/pull/${o.pr}/head`, fix: 'fix the fetch, then rerun land' }); const receipt = bindShippedReceipt(root, `origin/${ctx.base}`, info.body ?? '', o.pr, [o.pin]); lines.push(receipt.line, ...(receipt.depthLine ? [receipt.depthLine] : []), ...(receipt.targetedLine ? [receipt.targetedLine] : [])); if (!receipt.ok) return fail(lines, { code: receipt.code, message: receipt.message, fix: `run /ship at the pin, then gstack-ship-queue ready --pr ${o.pr}` }); @@ -111,8 +124,13 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { const method = policy.history === 'merge-only' ? '--merge' : '--squash'; const m = ctx.gh(['pr', 'merge', o.pr, method, '--match-head-commit', o.pin]); - const after = view(ctx, o.pr); - const merge = 'code' in after ? undefined : after.state === 'MERGED' ? after.mergeCommit?.oid : undefined; + let merge: string | undefined; + for (let i = 0; i < (m.ok ? 4 : 1) && !isSha(merge); i++) { + if (i) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 3_000); + const after = view(ctx, o.pr); + merge = 'code' in after || after.state !== 'MERGED' ? undefined : after.mergeCommit?.oid; + } + if (!isSha(merge) && m.ok) return fail(lines, { code: 'QUEUE_NOT_READY', message: `gh pr merge accepted #${o.pr}, but GitHub does not show it merged yet; nothing is verified`, fix: `rerun gstack-ship-queue land --pr ${o.pr} --pin ${o.pin} in a minute; once GitHub shows the merge it goes straight to the tree check` }); if (!isSha(merge)) return fail(lines, { code: 'QUEUE_NOT_READY', message: `gh pr merge refused: ${(m.err ?? m.out).trim().slice(0, 200) || 'the PR is not merged'}`, fix: `read the refusal; rerun land --pr ${o.pr} --pin ${o.pin} once it is fixed` }); return verifyMerge(ctx, root, o.pr, o.pin, merge!, lines, run, now()); } diff --git a/test/ship-queue-land-direct.test.ts b/test/ship-queue-land-direct.test.ts index bae403c9c..787a75dbd 100644 --- a/test/ship-queue-land-direct.test.ts +++ b/test/ship-queue-land-direct.test.ts @@ -6,6 +6,7 @@ * check, the land note, train build under merge: direct, and the policy key's validation. */ 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'; @@ -95,6 +96,8 @@ const landNotes = (fx: Fx) => { try { list = notes('list'); } catch { return []; } return list.split('\n').filter(Boolean).map(l => JSON.parse(notes('show', l.split(' ')[1]!))).filter(r => r.kind === 'land'); }; +/** Whether a clone holds a commit. */ +const spawnCheck = (dir: string, sha: string) => spawnSync('git', ['-C', dir, 'cat-file', '-e', `${sha}^{commit}`], { timeout: 30_000 }).status === 0; /** Commit a file straight onto origin/main (another PR that landed first). */ function moveMain(fx: Fx, file: string, text: string): string { const { repo } = fx; @@ -316,6 +319,45 @@ describe('land --pin: fail closed and merge methods', () => { expect(fx.read().mergeArgs).toEqual(['25', '--merge', '--match-head-commit', pin]); }, 90_000); + test('land reads the policy after fetching the base: a required check main added since the last fetch is enforced', () => { + const fx = setup(); + const pin = openPr(fx, '27'); + const other = fx.repo.root + '/other'; + fx.repo.git('clone', '-q', fx.repo.origin, other); + fs.writeFileSync(path.join(other, '.gstack', 'ship-policy.json'), JSON.stringify({ ...POLICY, changelog_fragments: { dir: 'changelog.d', required_checks: [...REQUIRED, 'security-scan'] } }, null, 2) + '\n'); + fx.repo.git('-C', other, 'commit', '-qam', 'require security-scan'); + fx.repo.git('-C', other, 'push', '-q', 'origin', 'HEAD:refs/heads/main'); + const r = land(fx, '27', pin, ['--dry-run']); + expect(r.err).toContain('security-scan'); + expect(r.err).toContain('(QUEUE_LAND_CHECKS)'); + }, 90_000); + + test('a rerun from a fresh clone after the branch was deleted fetches the pin from refs/pull//head and lands', () => { + const fx = setup(); + const pin = openPr(fx, '28'); + expect(land(fx, '28', pin).status).toBe(EXIT.ok); + const g = (...a: string[]) => fx.repo.git('--git-dir', fx.repo.origin, ...a); + g('update-ref', 'refs/pull/28/head', pin); + fx.write(d => { d.prs['28'].headRefOid = pin; }); + g('update-ref', '-d', 'refs/heads/pr-28'); + const fresh = fx.repo.root + '/fresh'; + fx.repo.git('clone', '-q', '--no-local', '--single-branch', '--branch', 'main', fx.repo.origin, fresh); + expect(spawnCheck(fresh, pin)).toBe(false); + const r = runBin(fx.repo, 'gstack-ship-queue', ['land', '--pr', '28', '--pin', pin, '--base', 'main'], fx.env, fresh); + expect(r.err).toBe(''); + expect(r.status).toBe(EXIT.ok); + expect(r.out).toMatch(/^LANDED: /m); + }, 90_000); + + test('QUEUE_LAND_STAMPED: a head that repoints .gstack/version-path', () => { + const fx = setup(); + const pin = openPr(fx, '29', { edit: w => { fs.mkdirSync(path.join(w, '.gstack'), { recursive: true }); fs.writeFileSync(path.join(w, 'OTHER_VERSION'), fs.readFileSync(path.join(w, 'VERSION'))); fs.writeFileSync(path.join(w, '.gstack', 'version-path'), 'OTHER_VERSION\n'); fs.writeFileSync(path.join(w, 'VERSION'), '9.9.9.9\n'); } }); + const r = land(fx, '29', pin); + expect(r.err).toContain('(QUEUE_LAND_STAMPED)'); + expect(r.err).toContain('.gstack/version-path'); + expect(fx.read().mergeArgs).toBeUndefined(); + }, 90_000); + test('merge: direct without changelog_fragments refuses with the key to add', () => { const fx = setup({ ...POLICY, changelog_fragments: null }); const pin = openPr(fx, '26');