diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index a8a1c87db..bfdb76169 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -206,7 +206,7 @@ function cmdResult(rest: string[], json: boolean): number { if (prFlag !== undefined && !/^[1-9]\d*$/.test(prFlag)) { process.stderr.write(`gstack-ship-receipt result: --pr must be a PR number, got ${JSON.stringify(prFlag.slice(0, 40))}\n`); return EXIT.usage; } const pr = prFlag; const repoRoot = git(['rev-parse', '--show-toplevel']); - const head = flag(rest, '--head') ?? (repoRoot ? git(['rev-parse', 'HEAD'], repoRoot) : undefined); + const head = (flag(rest, '--head') ?? (repoRoot ? git(['rev-parse', 'HEAD'], repoRoot) : undefined))?.toLowerCase(); const refused = (code: 'QUEUE_RECEIPT_MISSING' | 'QUEUE_RECEIPT_STALE', message: string) => { process.stderr.write(resultLineWithDocs('gstack-ship-receipt result', code, message) + '\n'); process.stdout.write(json ? JSON.stringify({ skill: 'ship', status: 'refused', run: '-', code, message }) + '\n' : 'GSTACK_RESULT: skill=ship status=refused run=-\n'); diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 36b812eb9..326554489 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4075,7 +4075,10 @@ run uploads no train record, so the evidence is its plan: a superseded run skips `plan-slices` and uploads no `paid-plan` artifact, and a manifest with nothing planned counts the same. `ready --screen` dispatches a fresh one and `train build` prints `SCREEN: # run planned no cases (no paid-plan -artifact; …); counted as not-run` and seats the slot unscreened. +artifact; …)` or `… planned no cases (an empty plan manifest; …)`, each +ending `; counted as not-run`, and seats the slot unscreened. A plan it cannot +read (an expired artifact, an unreadable manifest) keeps the slot screened and +prints `SCREEN: # run … unreadable; counted as screened, unverified`. **Fix.** Resolve the conflict with the base branch (or wait for GitHub to recompute the merge ref), push, and run `gstack-ship-queue ready --pr diff --git a/lib/ship-queue-train-github.ts b/lib/ship-queue-train-github.ts index f437161fa..952f0a101 100644 --- a/lib/ship-queue-train-github.ts +++ b/lib/ship-queue-train-github.ts @@ -57,15 +57,15 @@ export function asRun(summary: RunSummary): { id: number; url: string } { return { id: summary.databaseId, url: summary.url }; } -/** One JSON artifact of a run (the first .json file it holds); null when it is missing or unreadable. */ -export function downloadJson(ctx: QueueContext, slug: string, run: number, name: string, scratch: string): unknown { +/** One JSON artifact of a run (the named file, else the first .json file it holds); null when it is missing or unreadable. */ +export function downloadJson(ctx: QueueContext, slug: string, run: number, name: string, scratch: string, file?: string): unknown { const dir = path.join(scratch, `${run}-${name}`); fs.rmSync(dir, { recursive: true, force: true }); fs.mkdirSync(dir, { recursive: true }); try { if (!ctx.gh(['run', 'download', String(run), '--repo', slug, '-n', name, '-D', dir]).ok) return null; - const file = fs.readdirSync(dir).find(f => f.endsWith('.json')); - return file ? JSON.parse(fs.readFileSync(path.join(dir, file), 'utf8')) : null; + const pick = file ?? fs.readdirSync(dir).find(f => f.endsWith('.json')); + return pick && fs.existsSync(path.join(dir, pick)) ? JSON.parse(fs.readFileSync(path.join(dir, pick), 'utf8')) : null; } catch { return null; } finally { fs.rmSync(dir, { recursive: true, force: true }); } } @@ -74,23 +74,27 @@ export function downloadJson(ctx: QueueContext, slug: string, run: number, name: * ref it was dispatched for was superseded, evals.yml skips plan-slices and * concludes success (GSTA-95 on #3431). A screen run uploads no train record, * so the evidence is the plan itself: no `paid-plan` artifact, or a manifest - * with nothing planned. Returns the reason when the run is hollow, null when - * it is live, still running, or its plan cannot be read. + * with nothing planned. `hollow` carries the reason; `live` is a screen that + * planned cases or is still running; `unknown` names what could not be read + * (the caller decides, and says, how it counts). */ -export function hollowScreen(ctx: QueueContext, slug: string, run: number): string | null { +export type ScreenPlan = { kind: 'hollow'; reason: string } | { kind: 'live' } | { kind: 'unknown'; reason: string }; +export function hollowScreen(ctx: QueueContext, slug: string, run: number): ScreenPlan { + if (!Number.isSafeInteger(run) || run <= 0) return { kind: 'unknown', reason: `screen run id ${String(run).slice(0, 40)} is not a run number` }; const view = runView(ctx, slug, run); - if (!view || view.status !== 'completed') return null; - const list = json<{ artifacts?: Array<{ name?: string; expired?: boolean }> }>(ctx.gh(['api', `repos/${slug}/actions/runs/${run}/artifacts?per_page=100`])); - if (!Array.isArray(list?.artifacts)) return null; + if (!view) return { kind: 'unknown', reason: `run ${run} is unreadable` }; + if (view.status !== 'completed') return { kind: 'live' }; + const list = json<{ artifacts?: Array<{ name?: string; expired?: boolean }> }>(ctx.gh(['api', `repos/${slug}/actions/runs/${run}/artifacts?name=${PLAN_ARTIFACT}&per_page=10`])); + if (!Array.isArray(list?.artifacts)) return { kind: 'unknown', reason: `run ${run}'s artifact list is unreadable` }; const plan = list.artifacts.find(a => a.name === PLAN_ARTIFACT); - if (!plan) return `run ${run} planned no cases (no ${PLAN_ARTIFACT} artifact; concluded ${view.conclusion ?? 'unknown'})`; - if (plan.expired) return null; + if (!plan) return { kind: 'hollow', reason: `run ${run} planned no cases (no ${PLAN_ARTIFACT} artifact; concluded ${view.conclusion ?? 'unknown'})` }; + if (plan.expired) return { kind: 'unknown', reason: `run ${run}'s ${PLAN_ARTIFACT} artifact has expired` }; const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-screen-')); let manifest: { entries?: Array<{ status?: unknown }> } | null; - try { manifest = downloadJson(ctx, slug, run, PLAN_ARTIFACT, scratch) as typeof manifest; } finally { fs.rmSync(scratch, { recursive: true, force: true }); } - if (!Array.isArray(manifest?.entries)) return null; + try { manifest = downloadJson(ctx, slug, run, PLAN_ARTIFACT, scratch, 'manifest.json') as typeof manifest; } finally { fs.rmSync(scratch, { recursive: true, force: true }); } + if (!Array.isArray(manifest?.entries)) return { kind: 'unknown', reason: `run ${run}'s ${PLAN_ARTIFACT} manifest.json is unreadable` }; const planned = manifest.entries.filter(e => e?.status === 'planned').length; - return planned === 0 ? `run ${run} planned 0 cases (concluded ${view.conclusion ?? 'unknown'})` : null; + return planned === 0 ? { kind: 'hollow', reason: `run ${run} planned no cases (an empty plan manifest; concluded ${view.conclusion ?? 'unknown'})` } : { kind: 'live' }; } /** The plan manifest every evals.yml run that planned anything uploads (plan-slices); a superseded screen never reaches that job. */ diff --git a/lib/ship-queue-train-ready.ts b/lib/ship-queue-train-ready.ts index 1ed8c46a3..624b45d0f 100644 --- a/lib/ship-queue-train-ready.ts +++ b/lib/ship-queue-train-ready.ts @@ -85,7 +85,8 @@ async function dispatchScreen(env: TrainEnv, o: ReadyOptions, mergeSha: string, if (!slug) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: 'gh repo view could not name the repository' }, [], `${Q} ready --pr ${o.pr} --screen`, 'none'); const exec = execFor(env.ctx); const existing = findDuplicateRun(listDispatchRuns(dispatch.workflow, exec, slug) ?? [], dedupeKey('pr', mergeSha, baseSha, scope)); - const hollow = existing ? hollowScreen(env.ctx, slug, existing.databaseId) : null; + const plan = existing ? hollowScreen(env.ctx, slug, existing.databaseId) : null; + const hollow = plan?.kind === 'hollow' ? plan.reason : null; if (existing && !hollow) return { choice: { choice: 'screen', policy, nonce: /eval-bg (\S+?)\]/.exec(existing.displayTitle)?.[1] ?? nonce, run: existing.databaseId, merge_sha: mergeSha }, line: `SCREEN_DISPATCH: following run ${existing.databaseId} (same merge SHA, base and policy; no duplicate)` }; const found = dispatchWorkflow(dispatch, env.ctx.base, exec, slug) ? await awaitNonceRun(dispatch.workflow, nonce, exec, slug) : null; if (!found) return outPending({ code: 'QUEUE_TRAIN_DISPATCH_UNKNOWN', message: `no ${dispatch.workflow} run named ${nonce} appeared after the screen dispatch` }, [], `${Q} ready --pr ${o.pr} --screen${o.broad ? ' --broad' : ''} (adopts the run by its key once it appears)`, 'none'); diff --git a/lib/ship-queue-train.ts b/lib/ship-queue-train.ts index 8066bf75d..f8f26fcf7 100644 --- a/lib/ship-queue-train.ts +++ b/lib/ship-queue-train.ts @@ -302,8 +302,9 @@ function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>): { if (!ready) return outRefused({ code: 'QUEUE_NOT_READY', message: `no READY receipt for #${p.pr} at ${info.headRefOid.slice(0, 12)} (local state or ${'refs/notes/gstack-trains'})` }, [], `the lane runs ${Q} ready --pr ${p.pr} --screen|--no-screen`, 'none'); let screen = ready.screen; if (screen.choice === 'screen' && screen.run && slug === undefined) slug = repoSlug(ctx); - const hollow = screen.choice === 'screen' && screen.run && slug ? hollowScreen(ctx, slug, screen.run) : null; - if (hollow) { lines.push(`SCREEN: #${p.pr} ${hollow}; counted as not-run, the slot rides unscreened`); screen = { ...screen, choice: 'no-screen' }; } + const plan = screen.choice === 'screen' && screen.run && slug ? hollowScreen(ctx, slug, screen.run) : null; + if (plan?.kind === 'hollow') { lines.push(`SCREEN: #${p.pr} ${plan.reason}; counted as not-run, the slot rides unscreened`); screen = { ...screen, choice: 'no-screen' }; } + else if (plan?.kind === 'unknown') lines.push(`SCREEN: #${p.pr} ${plan.reason}; counted as screened, unverified`); slots.push({ pr: p.pr, head: info.headRefOid, ready_at: ready.ready_at, screen }); } return { slots, lines }; diff --git a/test/helpers/ship-queue-train-fixture.ts b/test/helpers/ship-queue-train-fixture.ts index 92c20d25d..f5d0ec199 100644 --- a/test/helpers/ship-queue-train-fixture.ts +++ b/test/helpers/ship-queue-train-fixture.ts @@ -27,8 +27,8 @@ export const TRAIN_POLICY = { ...BASE_POLICY, train: TRAIN_BLOCK, min_gstack_ver export interface Rule { fail?: boolean; silent?: boolean; failButCreate?: boolean; status?: string; conclusion?: string | null; event?: string; headSha?: string; record?: { panels: unknown[]; census?: unknown; verdict?: 'GREEN' | 'RED'; nonce?: string }; - /** The run's paid-plan manifest: planned entries (default 1); null uploads no paid-plan artifact, as a superseded screen does. */ - plan?: { planned: number } | null; + /** The run's paid-plan manifest: planned entries (default 1), or a raw manifest object; null uploads no paid-plan artifact, as a superseded screen does. */ + plan?: { planned: number } | { manifest: unknown } | null; check?: string | null; bisect?: { prefixes: string[]; patterns: Record; costUsd?: number }; } @@ -157,7 +157,7 @@ function shimSource(): string { " const run = { databaseId: id, workflow: w, headBranch: ref, headSha: head, status: rule.status || 'completed', conclusion: rule.conclusion === undefined ? 'success' : rule.conclusion,", " url: 'https://example.invalid/runs/' + id, event: rule.event || 'workflow_dispatch', displayTitle: inputs.nonce ? 'E2E Evals [eval-bg ' + inputs.nonce + ']' : w, inputs };", " d.runs.push(run); d.artifacts[id] = {};", - " if (rule.plan !== null) d.artifacts[id]['paid-plan'] = { tier: 'gate', sliceCount: 1, entries: Array.from({ length: rule.plan ? rule.plan.planned : 1 }, (_, i) => ({ file: 'test/case-' + i + '.test.ts', status: 'planned' })) };", + " if (rule.plan !== null) d.artifacts[id]['paid-plan'] = rule.plan && rule.plan.manifest ? rule.plan.manifest : { tier: 'gate', sliceCount: 1, entries: Array.from({ length: rule.plan ? rule.plan.planned : 1 }, (_, i) => ({ file: 'test/case-' + i + '.test.ts', status: 'planned' })) };", " if (rule.record && !inputs.prefix_shas) d.artifacts[id][d.artifact] = { schema: 1, kind: 'train-gate', nonce: rule.record.nonce || inputs.nonce, head_sha: head, tree: rev(head + '^{tree}'),", " verdict: rule.record.verdict || 'GREEN', census: rule.record.census, panels: rule.record.panels };", " if (rule.check && !inputs.prefix_shas) (d.checks[head] = d.checks[head] || []).push({ name: 'gstack/full-gate', status: 'completed', conclusion: rule.check });", @@ -176,11 +176,12 @@ function shimSource(): string { "if (a[0] === 'run' && a[1] === 'view') { const r = d.runs.find(x => String(x.databaseId) === a[2]); if (!r) fail('run not found'); out(r); }", "if (a[0] === 'run' && a[1] === 'download') {", " const art = (d.artifacts[a[2]] || {})[opt('-n')]; if (!art) fail('no artifact matches');", - " fs.mkdirSync(opt('-D'), { recursive: true }); fs.writeFileSync(path.join(opt('-D'), 'record.json'), JSON.stringify(art)); save(); process.exit(0);", + " fs.mkdirSync(opt('-D'), { recursive: true }); fs.writeFileSync(path.join(opt('-D'), opt('-n') === 'paid-plan' ? 'manifest.json' : 'record.json'), JSON.stringify(art));", + " if (opt('-n') === 'paid-plan') fs.writeFileSync(path.join(opt('-D'), 'gate-reuse.json'), '{\"reuse\":\"confirmed\"}'); save(); process.exit(0);", "}", "if (a[0] === 'api') {", - " const arts = /actions\\/runs\\/(\\d+)\\/artifacts/.exec(a[1] || '');", - " if (arts) { const names = Object.keys(d.artifacts[arts[1]] || {}); out({ total_count: names.length, artifacts: names.map(name => ({ name, expired: false })) }); }", + " const arts = /actions\\/runs\\/(\\d+)\\/artifacts(?:\\?name=([^&]+))?/.exec(a[1] || '');", + " if (arts) { const names = Object.keys(d.artifacts[arts[1]] || {}).filter(n => !arts[2] || n === decodeURIComponent(arts[2])); out({ total_count: names.length, artifacts: names.map(name => ({ name, expired: false })) }); }", " const named = /commits\\/([0-9a-f]+)\\/check-runs\\?check_name=([^&]+)/.exec(a[1] || '');", " if (named) { const list = (d.checks[named[1]] || []).filter(c => c.name === decodeURIComponent(named[2])); out({ total_count: list.length, check_runs: list }); }", " const paged = /commits\\/([0-9a-f]+)\\/(check-runs|status)\\?.*page=(\\d+)/.exec(a[1] || '');", diff --git a/test/ship-queue-train.test.ts b/test/ship-queue-train.test.ts index 9f2df7289..0818d4b29 100644 --- a/test/ship-queue-train.test.ts +++ b/test/ship-queue-train.test.ts @@ -256,7 +256,7 @@ describe('ready under a train policy: the screen choice', () => { fx.write(d => { d.rules['evals.yml'] = { status: 'completed', conclusion: 'success', plan: { planned: 3 } }; }); const live = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); expect(live.status).toBe(EXIT.ok); - expect(line(live.out, 'SCREEN_DISPATCH:')).toMatch(/^SCREEN_DISPATCH: evals\.yml run 103 .*not following run 102 planned 0 cases \(concluded success\)/); + expect(line(live.out, 'SCREEN_DISPATCH:')).toMatch(/^SCREEN_DISPATCH: evals\.yml run 103 .*not following run 102 planned no cases \(an empty plan manifest; concluded success\)/); const followed = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); expect(followed.status).toBe(EXIT.ok); expect(line(followed.out, 'SCREEN_DISPATCH:')).toBe('SCREEN_DISPATCH: following run 103 (same merge SHA, base and policy; no duplicate)'); @@ -266,6 +266,11 @@ describe('ready under a train policy: the screen choice', () => { 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)', '']); + fx.write(d => { d.rules['evals.yml'] = { status: 'completed', conclusion: 'success' }; d.artifacts['103']['paid-plan'] = { reuse: 'confirmed' }; }); + const unreadable = queue(fx, ['train', 'build', '--prs', `1@${head}`, '--dry-run']); + expect([unreadable.status, line(unreadable.out, 'ORDER:'), line(unreadable.out, 'SCREEN:')]).toEqual([EXIT.ok, 'ORDER: 1:#1(screened)', "SCREEN: #1 run 103's paid-plan manifest.json is unreadable; counted as screened, unverified"]); + const stillFollowed = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); + expect(line(stillFollowed.out, 'SCREEN_DISPATCH:')).toBe('SCREEN_DISPATCH: following run 103 (same merge SHA, base and policy; no duplicate)'); }, 120_000); });