diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.test.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.test.ts index 0656b413e..f068efd10 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.test.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.test.ts @@ -1,4 +1,14 @@ import { describe, expect, it } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { type CliRuntime, main, parseArgs } from "./cli.ts"; import { fakeReader, passingCheck } from "./fakes.test-helper.ts"; import { renderJson, renderPretty } from "./render.ts"; @@ -153,6 +163,92 @@ describe("rendering", () => { }); }); +describe("source CLI with a controlled gh executable", () => { + it("waits on a queued workflow before checks appear and reports query failures", () => { + const directory = mkdtempSync(join(tmpdir(), "watch-pr-actions-")); + const script = `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +fs.appendFileSync(process.env.CALLS, JSON.stringify(args) + "\\n"); +const head = "a".repeat(40); +let value; +if (args[0] === "pr" && args[1] === "view") + value = { mergeable: "MERGEABLE", mergeStateStatus: "UNSTABLE", reviewDecision: "APPROVED", headRefOid: head, headRefName: "feature", baseRefName: "main", state: "OPEN", mergedAt: null, isDraft: false }; +else if (args[0] === "pr" && args[1] === "checks") + value = ["Secret scan", "WIP"].map(name => ({ name, state: "SUCCESS", bucket: "pass", description: "", link: "", workflow: "" })); +else if (args[0] === "api" && args.includes("graphql")) { + const query = args.find(arg => arg.startsWith("query=")); + const pullRequest = query.includes("ReviewThreads") + ? { reviewThreads: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } } } + : { commits: { nodes: [{ commit: { oid: head, statusCheckRollup: { state: "SUCCESS" } } }] } }; + value = { data: { repository: { pullRequest } } }; +} else if (args[0] === "api" && args.some(arg => arg.includes("/actions/runs"))) { + if (process.env.STATUS === "error") process.exit(1); + const status = process.env.STATUS || "queued"; + const page = args.find(arg => arg.startsWith("page=")); + const run = (id, attempt, conclusion) => ({ id, workflow_id: id, run_number: id, event: "push", head_repository: { id: 300 }, head_branch: "feature", pull_requests: [], run_attempt: attempt, name: "CI", head_sha: head, status: conclusion ? "completed" : status, conclusion, html_url: "" }); + value = status === "paged" + ? { total_count: 101, workflow_runs: page === "page=2" ? [run(101, 2, "failure")] : Array.from({ length: 100 }, (_, i) => run(i + 1, 1, "success")) } + : { total_count: status === "none" ? 0 : 1, workflow_runs: status === "none" ? [] : [run(1, 1, status === "success" ? "success" : null)] }; +} else process.exit(2); +console.log(JSON.stringify(value)); +`; + try { + const executable = join(directory, "gh"); + const calls = join(directory, "calls.ndjson"); + writeFileSync(executable, script); + chmodSync(executable, 0o755); + const invoke = (status: string) => + spawnSync( + process.execPath, + [ + join(import.meta.dir, "watch-pr"), + "--owner", + "example", + "--repo", + "fixture", + "--pr", + "1", + "--max-query-errors", + "1", + "--timeout", + ".01", + "--interval", + ".001", + ], + { + encoding: "utf8", + env: { + ...process.env, + PATH: `${directory}:${process.env.PATH}`, + STATUS: status, + CALLS: calls, + }, + } + ); + const queued = invoke("queued"); + expect(queued.status).toBe(5); + expect(queued.stdout).toContain('"kind":"TIMEOUT"'); + expect(queued.stdout).toContain('"reportedState":"QUEUED"'); + expect(readFileSync(calls, "utf8")).toContain( + `head_sha=${"a".repeat(40)}` + ); + const complete = invoke("success"); + expect(complete.status).toBe(0); + expect(complete.stdout).toContain('"kind":"READY"'); + const paged = invoke("paged"); + expect(paged.status).toBe(4); + expect(paged.stdout).toContain('"kind":"ci-failing"'); + expect(readFileSync(calls, "utf8")).toContain('"page=2"'); + const failed = invoke("error"); + expect(failed.status).toBe(7); + expect(failed.stdout).toContain('"kind":"BLOCKER"'); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); +}); + describe("main", () => { it("returns EX_USAGE 64 and writes usage errors only to stderr", async () => { const harness = testRuntime(fakeReader()); diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts index 84ccf596b..a9f8e1a81 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts @@ -13,6 +13,7 @@ import type { RollupPage, } from "./types.ts"; import { parsePrNumber } from "./types.ts"; +import { WatcherQueryError } from "./github.ts"; export interface FakeReaderOptions { readonly facts?: Partial>; @@ -24,6 +25,11 @@ export interface FakeReaderOptions { readonly openPullRequests?: readonly OpenPullRequest[]; readonly origin?: Repository | null; readonly current?: PrContext; + readonly workflowStatus?: + "queued" | "in_progress" | "success" | "failure" | "none"; + readonly workflowHead?: string; + readonly workflowError?: boolean; + readonly changedHead?: string; } function toRawThread(thread: ReviewThread): RawReviewThread { @@ -86,6 +92,7 @@ export function fakeReader( }; let page = 0; let threadPage = 0; + let factsReads = 0; return { calls, async originRepo() { @@ -100,7 +107,15 @@ export function fakeReader( }, async pullRequest(requested) { calls.push("pullRequest"); - return { ...defaults, ...options.facts, context: requested }; + factsReads++; + return { + ...defaults, + ...options.facts, + ...(factsReads > 1 && options.changedHead + ? { headRefOid: options.changedHead } + : {}), + context: requested, + }; }, async openPullRequests() { calls.push("openPullRequests"); @@ -114,6 +129,37 @@ export function fakeReader( calls.push(`checkRollupPage:${after ?? "null"}`); return options.rollupPages?.[page++] ?? { checks: [], endCursor: null }; }, + async workflowRuns(_requested, head) { + calls.push(`workflowRuns:${head}`); + if (options.workflowError) + throw new WatcherQueryError({ + kind: "command-exit", + retryable: true, + detail: "workflow query failed", + code: 1, + }); + if ( + options.workflowStatus === undefined || + options.workflowStatus === "none" || + (options.workflowHead && options.workflowHead !== head) + ) + return []; + return [ + { + name: "CI", + description: "", + workflow: "CI", + link: "", + kind: + options.workflowStatus === "failure" + ? "failed" + : options.workflowStatus === "success" + ? "passed" + : "pending", + reportedState: options.workflowStatus.toUpperCase(), + }, + ]; + }, async reviewThreadPage(_requested, after) { calls.push(`reviewThreadPage:${after ?? "null"}`); return options.threadPages?.[threadPage++] ?? { diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts index 4e5893838..b91f01d1f 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts @@ -15,6 +15,7 @@ import { resolveChecks, resolveContext, resolveReviewThreads, + parseWorkflowRunPage, } from "./github.ts"; import { fakeReader, @@ -30,6 +31,86 @@ const context = { number: parsePrNumber(42), }; +describe("Actions workflow runs", () => { + it("maps the matching head and does not borrow an old head's queued run", () => { + const page = parseWorkflowRunPage( + { + total_count: 2, + workflow_runs: [ + { + id: 1, + workflow_id: 200, + run_number: 1, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [], + run_attempt: 1, + name: "CI", + head_sha: "head", + status: "queued", + conclusion: null, + html_url: "", + }, + { + id: 2, + workflow_id: 200, + run_number: 2, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [], + run_attempt: 1, + name: "CI", + head_sha: "old", + status: "in_progress", + conclusion: null, + html_url: "", + }, + ], + }, + "head" + ); + expect(page.checks.map(({ kind }) => kind)).toEqual(["pending"]); + }); + + it("keeps Code Review Gate owner approval out of pending and failed workflows", () => { + for (const [status, conclusion] of [ + ["queued", null], + ["completed", "action_required"], + ] as const) { + const page = parseWorkflowRunPage( + { + total_count: 1, + workflow_runs: [{ + id: 1, + workflow_id: 200, + run_number: 1, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [], + run_attempt: 1, + name: "Code Review Gate", + head_sha: "head", + status, + conclusion, + html_url: "", + }], + }, + "head" + ); + expect(page.checks[0].kind).toBe("code-review-gate"); + } + }); + + it("rejects missing workflow-run fields rather than interpreting them as no Actions", () => { + expect(() => + parseWorkflowRunPage({ total_count: 1, workflow_runs: [{}] }, "head") + ).toThrow(WatcherQueryError); + }); +}); + describe("checks fallback chain", () => { it("uses a non-empty fast-path result without a rollup query", async () => { const reader = fakeReader({ diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts index 123a33b19..a011bf308 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts @@ -455,6 +455,122 @@ function graphqlArgs( ]; } +type WorkflowRunCheck = T.Check & { + readonly id: number; + readonly attempt: number; + readonly runNumber: number; + readonly execution: string; +}; + +function positiveInteger(value: unknown, path: string): number { + if (typeof value !== "number" || !Number.isSafeInteger(value) || value <= 0) + missing(path, value); + return value; +} + +export function parseWorkflowRunPage( + value: unknown, + head: string +): { + readonly checks: readonly WorkflowRunCheck[]; + readonly ids: readonly number[]; + readonly count: number; + readonly total: number; +} { + const page = record(value, "workflow runs"); + const total = page.total_count; + if (!Number.isSafeInteger(total) || Number(total) < 0) + missing("workflow runs.total_count", total); + const runs = list(page.workflow_runs, "workflow runs.workflow_runs"); + const ids: number[] = []; + const checks = runs + .map((value, index) => { + const run = record(value, `workflow runs[${index}]`); + const id = positiveInteger(run.id, "workflow run.id"); + const attempt = positiveInteger( + run.run_attempt, + "workflow run.run_attempt" + ); + const runNumber = positiveInteger( + run.run_number, + "workflow run.run_number" + ); + const workflowId = positiveInteger( + run.workflow_id, + "workflow run.workflow_id" + ); + const event = string(run.event, "workflow run.event"); + const repositoryId = positiveInteger( + at(run, ["head_repository", "id"]), + "workflow run.head_repository.id" + ); + const branch = optionalString(run.head_branch, "workflow run.head_branch"); + const prs = run.pull_requests === null + ? null + : list(run.pull_requests, "workflow run.pull_requests") + .map((pr) => JSON.stringify([ + positiveInteger( + at(pr, ["number"]), + "workflow run.pull_requests.number" + ), + positiveInteger( + at(pr, ["base", "repo", "id"]), + "workflow run.pull_requests.base.repo.id" + ), + string(at(pr, ["base", "ref"]), "workflow run.pull_requests.base.ref"), + ])) + .sort(); + const automatic = event === "push" || event === "pull_request" || + event === "pull_request_target"; + const unambiguous = automatic && branch !== null && prs !== null && + (event === "push" ? prs.length <= 1 : prs.length === 1); + const execution = JSON.stringify([ + workflowId, + event, + repositoryId, + branch, + prs, + ...(unambiguous ? [] : [id]), + ]); + ids.push(id); + const sha = string(run.head_sha, "workflow run.head_sha"); + const name = string(run.name, "workflow run.name"); + const link = string(run.html_url, "workflow run.html_url"); + const status = enumValue( + run.status, + [ + "queued", + "in_progress", + "waiting", + "pending", + "requested", + "completed", + ] as const, + "workflow run.status" + ); + const conclusion = optionalString( + run.conclusion, + "workflow run.conclusion" + ); + if (sha !== head) return null; + const details = { name, link, description: "", workflow: name }; + const reportedState = conclusion?.toUpperCase() ?? status.toUpperCase(); + const check: T.Check = + status !== "completed" || conclusion === null + ? gateOr("pending", details, reportedState) + : conclusion === "success" + ? { ...details, kind: "passed", reportedState } + : conclusion === "skipped" || conclusion === "neutral" + ? { ...details, kind: "skipped", reportedState } + : conclusion === "action_required" + ? gateOr("failed", details, reportedState) + : { ...details, kind: "failed", reportedState }; + return { ...check, id, attempt, runNumber, execution }; + }) + .filter((check): check is NonNullable => check !== null); + return { checks, ids, count: runs.length, total: Number(total) }; +} + export class GhGitHubReader implements T.GitHubReader { async originRepo(): Promise { const result = await run(["git", "remote", "get-url", "origin"]); @@ -576,6 +692,57 @@ export class GhGitHubReader implements T.GitHubReader { ); return { checks, endCursor: page.hasNextPage && cursor ? cursor : null }; } + async workflowRuns( + context: T.PrContext, + head: string + ): Promise { + const latest = new Map(); + const ids = new Set(); + let total: number | null = null; + for (let page = 1; page <= 10; page++) { + const result = parseWorkflowRunPage( + await runJson([ + "gh", + "api", + "-X", + "GET", + `repos/${context.owner}/${context.repo}/actions/runs`, + "-f", + `head_sha=${head}`, + "-F", + "per_page=100", + "-F", + `page=${page}`, + ]), + head + ); + if (result.total >= 1000) + missing("workflow runs exceeds complete-observation limit", result.total); + if (total !== null && result.total !== total) + missing("workflow runs total changed during pagination", result.total); + total = result.total; + if (result.count !== Math.min(100, total - ids.size)) + missing("workflow runs incomplete page", result.count); + for (const id of result.ids) { + if (ids.has(id)) missing("workflow runs duplicate id", id); + ids.add(id); + } + for (const check of result.checks) { + const previous = latest.get(check.execution); + if (previous !== undefined && + previous.runNumber === check.runNumber && previous.id !== check.id) + missing("workflow runs ambiguous run number", check.runNumber); + if (previous === undefined || previous.runNumber < check.runNumber || + (previous.runNumber === check.runNumber && previous.attempt < check.attempt)) + latest.set(check.execution, check); + } + if (ids.size === total) + return [...latest.values()].map( + ({ id, attempt, runNumber, execution, ...check }) => check + ); + } + return missing("workflow runs pagination limit"); + } async reviewThreadPage( context: T.PrContext, after: string | null diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.test.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.test.ts index 966b75367..5c8d20d87 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.test.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.test.ts @@ -91,6 +91,135 @@ describe("readiness truth table", () => { }); }); +describe("workflow startup", () => { + it("holds a clean partial check list while a workflow is queued or running", async () => { + for (const state of ["queued", "in_progress"] as const) { + const reader = fakeReader({ + workflowStatus: state, + facts: { mergeStateStatus: "UNSTABLE" }, + }); + const snapshot = await readSnapshot({ + reader, + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(snapshot.kind).toBe("open"); + if (snapshot.kind !== "open") throw new Error("expected open snapshot"); + expect(snapshot.ci.kind).toBe("ci-pending"); + expect(classifyPr(snapshot).kind).toBe("waiting"); + } + }); + + it("allows completed successful workflows, blocks failures, and preserves external CI", async () => { + for (const [status, expected] of [ + ["success", "ci-clean"], + ["failure", "ci-failing"], + ["none", "ci-clean"], + ] as const) { + const snapshot = await readSnapshot({ + reader: fakeReader({ workflowStatus: status }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(snapshot.kind).toBe("open"); + if (snapshot.kind !== "open") throw new Error("expected open snapshot"); + expect(snapshot.ci.kind).toBe(expected); + } + }); + + it("does not use an old head or a changed head as clean workflow evidence", async () => { + const old = await readSnapshot({ + reader: fakeReader({ workflowStatus: "queued", workflowHead: "old" }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(old.kind).toBe("open"); + if (old.kind !== "open") throw new Error("expected open snapshot"); + expect(old.ci.kind).toBe("ci-clean"); + await expect( + readSnapshot({ + reader: fakeReader({ facts: { headRefOid: null } }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }) + ).rejects.toBeInstanceOf(WatcherQueryError); + await expect( + readSnapshot({ + reader: fakeReader({ changedHead: "new" }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }) + ).rejects.toBeInstanceOf(WatcherQueryError); + }); + + it("does not wait on an owner-approval gate after workflows finish", async () => { + const gate = { + ...pendingCheck("Code Review Gate"), + kind: "code-review-gate" as const, + name: "Code Review Gate" as const, + }; + const snapshot = await readSnapshot({ + reader: fakeReader({ + fastPath: { kind: "checks", checks: [passingCheck(), gate] }, + workflowStatus: "success", + }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(snapshot.kind).toBe("open"); + if (snapshot.kind !== "open") throw new Error("expected open snapshot"); + expect(snapshot.ci.kind).toBe("ci-clean"); + expect(classifyPr(snapshot).kind).toBe("ready"); + }); + + it("fails closed on a workflow query error", async () => { + await expect( + readSnapshot({ + reader: fakeReader({ workflowError: true }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }) + ).rejects.toBeInstanceOf(WatcherQueryError); + }); +}); + +it("retries a workflow query error in a loop, then reports status-query", async () => { + let now = 0; + const events: ProgressVerdict[] = []; + const verdict = await runSimple({ + dependencies: { + reader: fakeReader({ workflowError: true }), + clock: { + now: () => now, + observedAt: () => "2026-07-26T00:00:00.000Z", + async sleep(seconds) { + now += seconds; + }, + }, + emit(event) { + events.push(event); + }, + }, + contexts: [context(1)], + mode: "single", + statusOnly: false, + options: { ...options, maxQueryErrors: 2 }, + }); + expect(events.map(({ kind }) => kind)).toEqual(["RETRY"]); + expect(verdict).toMatchObject({ + kind: "BLOCKER", + exitCode: 7, + blocker: { kind: "status-query", failures: 2 }, + }); +}); + describe("snapshot query planning", () => { it("does not query commit rollups while queued checks are pending", async () => { const reader = fakeReader({ @@ -109,6 +238,8 @@ describe("snapshot query planning", () => { "pullRequest", "reviewThreadPage:null", "checksFastPath", + "workflowRuns:head", + "pullRequest", ]); }); @@ -301,10 +432,13 @@ describe("queued-stack cadence", () => { expect(timeline).toEqual([ "emit:QUEUE", "read:20", + "read:20", "fail:21", "emit:RETRY", "sleep", "read:21", + "read:21", + "read:22", "read:22", "emit:STATUS", "emit:WAITING", @@ -348,7 +482,7 @@ describe("queued-stack cadence", () => { const facts = await base.pullRequest(pr); const count = (reads.get(pr.number) ?? 0) + 1; reads.set(pr.number, count); - return pr.number === one.number && count > 1 + return pr.number === one.number && count > 2 ? { ...facts, state: "MERGED" as const, @@ -384,11 +518,12 @@ describe("queued-stack cadence", () => { await expect(running).rejects.toThrow("stop after advance proof"); expect(emitted.some((event) => event.kind === "ADVANCE")).toBe(true); const firstSleep = timeline.indexOf("sleep"); - expect(timeline.slice(firstSleep, firstSleep + 5)).toEqual([ + expect(timeline.slice(firstSleep, firstSleep + 6)).toEqual([ "sleep", "read:40", "emit:ADVANCE", "read:41", + "read:41", "emit:WAITING", ]); }); diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts index 8179f2d8b..fe073448c 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts @@ -69,22 +69,29 @@ export async function readSnapshot(args: { return { kind: "closed", context: args.context, facts }; const threads = await resolveReviewThreads(args.reader, args.context); const checks = await resolveChecks(args.reader, args.context); + if (facts.headRefOid === null) + throw new WatcherQueryError({ + kind: "missing-key", + retryable: true, + detail: "PR head SHA unavailable", + }); + const workflows = await args.reader.workflowRuns( + args.context, + facts.headRefOid + ); + const all: T.NonEmpty = [...checks.checks, ...workflows]; const failed = nonEmpty( - checks.checks.filter( - (check): check is T.FailedCheck => check.kind === "failed" - ) + all.filter((check): check is T.FailedCheck => check.kind === "failed") ); const pending = nonEmpty( - checks.checks.filter( - (check): check is T.PendingCheck => check.kind === "pending" - ) + all.filter((check): check is T.PendingCheck => check.kind === "pending") ); let ci: T.CiState; if (failed === null && pending !== null && args.pendingHistory === "omit") ci = { kind: "ci-pending", source: checks.source, - all: checks.checks, + all, failed: [], pending, hadPreviousPassingCi: false, @@ -93,7 +100,7 @@ export async function readSnapshot(args: { const merge = await mergeAssessment(args.reader, facts); const base = { source: checks.source, - all: checks.checks, + all, hadPreviousPassingCi: merge.hadPreviousPassingCi, }; if (failed !== null) @@ -123,6 +130,13 @@ export async function readSnapshot(args: { github: merge.github, }; } + const current = await args.reader.pullRequest(args.context); + if (current.headRefOid !== facts.headRefOid || current.state !== facts.state) + throw new WatcherQueryError({ + kind: "missing-key", + retryable: true, + detail: "PR changed during snapshot", + }); return { kind: "open", context: args.context, diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts index 60b9abcac..3aebdb67e 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts @@ -399,6 +399,7 @@ export interface GitHubReader { context: PrContext, after: string | null ): Promise; + workflowRuns(context: PrContext, head: string): Promise; reviewThreadPage( context: PrContext, after: string | null diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/workflow-runs.test.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/workflow-runs.test.ts new file mode 100644 index 000000000..6ae91d751 --- /dev/null +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/workflow-runs.test.ts @@ -0,0 +1,401 @@ +import { describe, expect, it } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const head = "a".repeat(40); +const run = ( + id: number, + conclusion: string | null = "success", + overrides = {}, +) => ({ + id, + workflow_id: 200, + run_number: id, + run_attempt: 1, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [{ number: 1, base: { ref: "main", repo: { id: 400 } } }], + name: "CI", + head_sha: head, + status: conclusion === null ? "queued" : "completed", + conclusion, + html_url: `https://github.com/example/fixture/actions/runs/${id}`, + ...overrides, +}); +const page = (runs: readonly unknown[], total = runs.length) => ({ + total_count: total, + workflow_runs: runs, +}); +const first = Array.from({ length: 100 }, (_, i) => run(i + 1)); +const cases = [ + { + name: "obsolete cancellation replaced by success", + pages: [page([run(101), run(100, "cancelled")])], + code: 0, + }, + { + name: "obsolete failure replaced by success", + pages: [page([run(100, "failure"), run(101)])], + code: 0, + }, + { + name: "current failure", + pages: [page([run(100), run(101, "failure")])], + code: 4, + }, + { + name: "queued replacement of failed execution", + pages: [page([run(100, "failure"), run(101, null)])], + code: 5, + }, + { + name: "running replacement", + pages: [ + page([run(100, "cancelled"), run(101, null, { status: "in_progress" })]), + ], + code: 5, + }, + { + name: "new execution supersedes older rerun attempt", + pages: [page([run(100, "failure", { run_attempt: 5 }), run(101)])], + code: 0, + }, + { + name: "latest attempt fails", + pages: [page([run(100, "failure", { run_attempt: 2 })])], + code: 4, + }, + { + name: "latest attempt succeeds", + pages: [page([run(100, "success", { run_attempt: 2 })])], + code: 0, + }, + { + name: "latest attempt is queued", + pages: [page([run(100, null, { run_attempt: 2 })])], + code: 5, + }, + { + name: "workflow run number orders replacements", + pages: [ + page([ + run(101, "failure", { run_number: 1 }), + run(100, "success", { run_number: 2 }), + ]), + ], + code: 0, + }, + { + name: "push replacement without PR associations", + pages: [ + page([ + run(100, "failure", { event: "push", pull_requests: [] }), + run(101, "success", { event: "push", pull_requests: [] }), + ]), + ], + code: 0, + }, + { + name: "same name independent workflow", + pages: [ + page([run(100, "failure"), run(101, "success", { workflow_id: 201 })]), + ], + code: 4, + }, + { + name: "independent event", + pages: [ + page([run(100, "failure"), run(101, "success", { event: "push" })]), + ], + code: 4, + }, + { + name: "independent head repository", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { head_repository: { id: 301 } }), + ]), + ], + code: 4, + }, + { + name: "independent branch", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { head_branch: "other" }), + ]), + ], + code: 4, + }, + { + name: "independent PR association", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { + pull_requests: [ + { number: 2, base: { ref: "main", repo: { id: 400 } } }, + ], + }), + ]), + ], + code: 4, + }, + { + name: "independent PR base", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { + pull_requests: [ + { number: 1, base: { ref: "release", repo: { id: 400 } } }, + ], + }), + ]), + ], + code: 4, + }, + { + name: "manual dispatch inputs cannot be reconciled", + pages: [ + page([ + run(100, "failure", { event: "workflow_dispatch" }), + run(101, "success", { event: "workflow_dispatch" }), + ]), + ], + code: 4, + }, + { + name: "ambiguous multiple PR associations", + pages: [ + page([ + run(100, "failure", { + pull_requests: [ + { number: 1, base: { ref: "main", repo: { id: 400 } } }, + { number: 2, base: { ref: "main", repo: { id: 400 } } }, + ], + }), + run(101, "success", { + pull_requests: [ + { number: 1, base: { ref: "main", repo: { id: 400 } } }, + { number: 2, base: { ref: "main", repo: { id: 400 } } }, + ], + }), + ]), + ], + code: 4, + }, + { + name: "repository dispatch inputs cannot be reconciled", + pages: [ + page([ + run(100, "failure", { event: "repository_dispatch" }), + run(101, "success", { event: "repository_dispatch" }), + ]), + ], + code: 4, + }, + { + name: "unknown PR associations", + pages: [ + page([ + run(100, "failure", { pull_requests: null }), + run(101, "success", { pull_requests: null }), + ]), + ], + code: 4, + }, + { + name: "unknown head branch", + pages: [ + page([ + run(100, "failure", { head_branch: null }), + run(101, "success", { head_branch: null }), + ]), + ], + code: 4, + }, + { + name: "current registered failure remains authoritative", + pages: [page([run(101)])], + checkState: "FAILURE", + code: 4, + }, + { + name: "complete pagination", + pages: [page(first, 101), page([run(101)], 101)], + code: 0, + }, + { + name: "maximum complete observation below cap", + pages: Array.from({ length: 10 }, (_, p) => + page( + Array.from({ length: p === 9 ? 99 : 100 }, (_, i) => + run(p * 100 + i + 1), + ), + 999, + ), + ), + code: 0, + }, + { + name: "duplicate attempts cannot prove complete observation", + pages: [page([run(100), run(100, "failure", { run_attempt: 2 })])], + code: 7, + }, + { + name: "duplicate row cannot satisfy total", + pages: [page(first, 101), page([run(100)], 101)], + code: 7, + }, + { + name: "increasing total", + pages: [page(first, 101), page([run(101)], 102)], + code: 7, + }, + { + name: "shrinking total", + pages: [page(first, 101), page([], 100)], + code: 7, + }, + { + name: "empty page before completion", + pages: [page(first, 101), page([], 101)], + code: 7, + }, + { + name: "short page before completion", + pages: [page([run(100)], 2)], + code: 7, + }, + { + name: "rows exceed total", + pages: [page([run(100), run(101)], 1)], + code: 7, + }, + { name: "filtered result at API cap", pages: [page(first, 1000)], code: 7 }, + { + name: "filtered result above API cap", + pages: [page(first, 1001)], + code: 7, + }, + { + name: "hostile changing totals are bounded", + pages: [page(first, 1000000)], + code: 7, + }, + { + name: "missing execution identity", + pages: [page([run(100, "success", { workflow_id: null })])], + code: 7, + }, + { name: "no Actions with external checks", pages: [page([])], code: 0 }, +] as const; + +describe("effective Actions executions through the source CLI", () => { + for (const fixture of cases) { + it(fixture.name, () => { + const directory = mkdtempSync(join(tmpdir(), "watch-pr-executions-")); + try { + const calls = join(directory, "calls.ndjson"); + writeFileSync(join(directory, "fixture.json"), JSON.stringify(fixture)); + writeFileSync( + join(directory, "gh"), + `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +fs.appendFileSync(process.env.CALLS, JSON.stringify(args) + "\\n"); +const fixture = JSON.parse(fs.readFileSync(process.env.FIXTURE, "utf8")); +const head = "a".repeat(40); +let value; +if (args[0] === "pr" && args[1] === "view") + value = { mergeable: "MERGEABLE", mergeStateStatus: "UNSTABLE", reviewDecision: "APPROVED", headRefOid: head, headRefName: "feature", baseRefName: "main", state: "OPEN", mergedAt: null, isDraft: false }; +else if (args[0] === "pr" && args[1] === "checks") + value = [{ name: "CI", state: fixture.checkState || "SUCCESS", bucket: fixture.checkState ? "fail" : "pass", description: "", link: "", workflow: "CI" }]; +else if (args.includes("graphql")) { + const query = args.find(arg => arg.startsWith("query=")); + const pullRequest = query.includes("ReviewThreads") + ? { reviewThreads: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } } } + : { commits: { nodes: [{ commit: { oid: head, statusCheckRollup: { state: "SUCCESS" } } }] } }; + value = { data: { repository: { pullRequest } } }; +} else if (args.some(arg => arg.includes("/actions/runs"))) { + const page = Number(args.find(arg => arg.startsWith("page=")).slice(5)); + value = fixture.pages[page - 1] || fixture.pages[fixture.pages.length - 1]; +} else process.exit(2); +console.log(JSON.stringify(value)); +`, + ); + chmodSync(join(directory, "gh"), 0o755); + const result = spawnSync( + process.execPath, + [ + join(import.meta.dir, "watch-pr"), + "--owner", + "example", + "--repo", + "fixture", + "--pr", + "1", + "--timeout", + ".01", + "--interval", + ".001", + "--max-query-errors", + "1", + ], + { + encoding: "utf8", + timeout: 10000, + env: { + ...process.env, + PATH: `${directory}:${process.env.PATH}`, + CALLS: calls, + FIXTURE: join(directory, "fixture.json"), + }, + }, + ); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(fixture.code); + const events = result.stdout + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + expect(events.at(-1).kind).toBe( + fixture.code === 0 + ? "READY" + : fixture.code === 5 + ? "TIMEOUT" + : "BLOCKER", + ); + if (fixture.code !== 0) + expect(events.some((event) => event.kind === "READY")).toBe(false); + if (fixture.code === 5) expect(events[0].kind).toBe("WAITING"); + const queries: string[][] = readFileSync(calls, "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + const actions = queries.filter((args) => + args.some((arg) => arg.includes("/actions/runs")), + ); + expect(actions.every((args) => args.includes(`head_sha=${head}`))).toBe( + true, + ); + expect(actions.length).toBeGreaterThan(0); + expect(actions.length).toBeLessThanOrEqual(10); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + } +}); diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/cli.test.ts b/pstack/skills/poteto-mode/scripts/watch-pr/cli.test.ts index 0656b413e..f068efd10 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/cli.test.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/cli.test.ts @@ -1,4 +1,14 @@ import { describe, expect, it } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { type CliRuntime, main, parseArgs } from "./cli.ts"; import { fakeReader, passingCheck } from "./fakes.test-helper.ts"; import { renderJson, renderPretty } from "./render.ts"; @@ -153,6 +163,92 @@ describe("rendering", () => { }); }); +describe("source CLI with a controlled gh executable", () => { + it("waits on a queued workflow before checks appear and reports query failures", () => { + const directory = mkdtempSync(join(tmpdir(), "watch-pr-actions-")); + const script = `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +fs.appendFileSync(process.env.CALLS, JSON.stringify(args) + "\\n"); +const head = "a".repeat(40); +let value; +if (args[0] === "pr" && args[1] === "view") + value = { mergeable: "MERGEABLE", mergeStateStatus: "UNSTABLE", reviewDecision: "APPROVED", headRefOid: head, headRefName: "feature", baseRefName: "main", state: "OPEN", mergedAt: null, isDraft: false }; +else if (args[0] === "pr" && args[1] === "checks") + value = ["Secret scan", "WIP"].map(name => ({ name, state: "SUCCESS", bucket: "pass", description: "", link: "", workflow: "" })); +else if (args[0] === "api" && args.includes("graphql")) { + const query = args.find(arg => arg.startsWith("query=")); + const pullRequest = query.includes("ReviewThreads") + ? { reviewThreads: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } } } + : { commits: { nodes: [{ commit: { oid: head, statusCheckRollup: { state: "SUCCESS" } } }] } }; + value = { data: { repository: { pullRequest } } }; +} else if (args[0] === "api" && args.some(arg => arg.includes("/actions/runs"))) { + if (process.env.STATUS === "error") process.exit(1); + const status = process.env.STATUS || "queued"; + const page = args.find(arg => arg.startsWith("page=")); + const run = (id, attempt, conclusion) => ({ id, workflow_id: id, run_number: id, event: "push", head_repository: { id: 300 }, head_branch: "feature", pull_requests: [], run_attempt: attempt, name: "CI", head_sha: head, status: conclusion ? "completed" : status, conclusion, html_url: "" }); + value = status === "paged" + ? { total_count: 101, workflow_runs: page === "page=2" ? [run(101, 2, "failure")] : Array.from({ length: 100 }, (_, i) => run(i + 1, 1, "success")) } + : { total_count: status === "none" ? 0 : 1, workflow_runs: status === "none" ? [] : [run(1, 1, status === "success" ? "success" : null)] }; +} else process.exit(2); +console.log(JSON.stringify(value)); +`; + try { + const executable = join(directory, "gh"); + const calls = join(directory, "calls.ndjson"); + writeFileSync(executable, script); + chmodSync(executable, 0o755); + const invoke = (status: string) => + spawnSync( + process.execPath, + [ + join(import.meta.dir, "watch-pr"), + "--owner", + "example", + "--repo", + "fixture", + "--pr", + "1", + "--max-query-errors", + "1", + "--timeout", + ".01", + "--interval", + ".001", + ], + { + encoding: "utf8", + env: { + ...process.env, + PATH: `${directory}:${process.env.PATH}`, + STATUS: status, + CALLS: calls, + }, + } + ); + const queued = invoke("queued"); + expect(queued.status).toBe(5); + expect(queued.stdout).toContain('"kind":"TIMEOUT"'); + expect(queued.stdout).toContain('"reportedState":"QUEUED"'); + expect(readFileSync(calls, "utf8")).toContain( + `head_sha=${"a".repeat(40)}` + ); + const complete = invoke("success"); + expect(complete.status).toBe(0); + expect(complete.stdout).toContain('"kind":"READY"'); + const paged = invoke("paged"); + expect(paged.status).toBe(4); + expect(paged.stdout).toContain('"kind":"ci-failing"'); + expect(readFileSync(calls, "utf8")).toContain('"page=2"'); + const failed = invoke("error"); + expect(failed.status).toBe(7); + expect(failed.stdout).toContain('"kind":"BLOCKER"'); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); +}); + describe("main", () => { it("returns EX_USAGE 64 and writes usage errors only to stderr", async () => { const harness = testRuntime(fakeReader()); diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts b/pstack/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts index 84ccf596b..a9f8e1a81 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/fakes.test-helper.ts @@ -13,6 +13,7 @@ import type { RollupPage, } from "./types.ts"; import { parsePrNumber } from "./types.ts"; +import { WatcherQueryError } from "./github.ts"; export interface FakeReaderOptions { readonly facts?: Partial>; @@ -24,6 +25,11 @@ export interface FakeReaderOptions { readonly openPullRequests?: readonly OpenPullRequest[]; readonly origin?: Repository | null; readonly current?: PrContext; + readonly workflowStatus?: + "queued" | "in_progress" | "success" | "failure" | "none"; + readonly workflowHead?: string; + readonly workflowError?: boolean; + readonly changedHead?: string; } function toRawThread(thread: ReviewThread): RawReviewThread { @@ -86,6 +92,7 @@ export function fakeReader( }; let page = 0; let threadPage = 0; + let factsReads = 0; return { calls, async originRepo() { @@ -100,7 +107,15 @@ export function fakeReader( }, async pullRequest(requested) { calls.push("pullRequest"); - return { ...defaults, ...options.facts, context: requested }; + factsReads++; + return { + ...defaults, + ...options.facts, + ...(factsReads > 1 && options.changedHead + ? { headRefOid: options.changedHead } + : {}), + context: requested, + }; }, async openPullRequests() { calls.push("openPullRequests"); @@ -114,6 +129,37 @@ export function fakeReader( calls.push(`checkRollupPage:${after ?? "null"}`); return options.rollupPages?.[page++] ?? { checks: [], endCursor: null }; }, + async workflowRuns(_requested, head) { + calls.push(`workflowRuns:${head}`); + if (options.workflowError) + throw new WatcherQueryError({ + kind: "command-exit", + retryable: true, + detail: "workflow query failed", + code: 1, + }); + if ( + options.workflowStatus === undefined || + options.workflowStatus === "none" || + (options.workflowHead && options.workflowHead !== head) + ) + return []; + return [ + { + name: "CI", + description: "", + workflow: "CI", + link: "", + kind: + options.workflowStatus === "failure" + ? "failed" + : options.workflowStatus === "success" + ? "passed" + : "pending", + reportedState: options.workflowStatus.toUpperCase(), + }, + ]; + }, async reviewThreadPage(_requested, after) { calls.push(`reviewThreadPage:${after ?? "null"}`); return options.threadPages?.[threadPage++] ?? { diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts b/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts index 4e5893838..b91f01d1f 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts @@ -15,6 +15,7 @@ import { resolveChecks, resolveContext, resolveReviewThreads, + parseWorkflowRunPage, } from "./github.ts"; import { fakeReader, @@ -30,6 +31,86 @@ const context = { number: parsePrNumber(42), }; +describe("Actions workflow runs", () => { + it("maps the matching head and does not borrow an old head's queued run", () => { + const page = parseWorkflowRunPage( + { + total_count: 2, + workflow_runs: [ + { + id: 1, + workflow_id: 200, + run_number: 1, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [], + run_attempt: 1, + name: "CI", + head_sha: "head", + status: "queued", + conclusion: null, + html_url: "", + }, + { + id: 2, + workflow_id: 200, + run_number: 2, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [], + run_attempt: 1, + name: "CI", + head_sha: "old", + status: "in_progress", + conclusion: null, + html_url: "", + }, + ], + }, + "head" + ); + expect(page.checks.map(({ kind }) => kind)).toEqual(["pending"]); + }); + + it("keeps Code Review Gate owner approval out of pending and failed workflows", () => { + for (const [status, conclusion] of [ + ["queued", null], + ["completed", "action_required"], + ] as const) { + const page = parseWorkflowRunPage( + { + total_count: 1, + workflow_runs: [{ + id: 1, + workflow_id: 200, + run_number: 1, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [], + run_attempt: 1, + name: "Code Review Gate", + head_sha: "head", + status, + conclusion, + html_url: "", + }], + }, + "head" + ); + expect(page.checks[0].kind).toBe("code-review-gate"); + } + }); + + it("rejects missing workflow-run fields rather than interpreting them as no Actions", () => { + expect(() => + parseWorkflowRunPage({ total_count: 1, workflow_runs: [{}] }, "head") + ).toThrow(WatcherQueryError); + }); +}); + describe("checks fallback chain", () => { it("uses a non-empty fast-path result without a rollup query", async () => { const reader = fakeReader({ diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/github.ts b/pstack/skills/poteto-mode/scripts/watch-pr/github.ts index 123a33b19..a011bf308 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/github.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/github.ts @@ -455,6 +455,122 @@ function graphqlArgs( ]; } +type WorkflowRunCheck = T.Check & { + readonly id: number; + readonly attempt: number; + readonly runNumber: number; + readonly execution: string; +}; + +function positiveInteger(value: unknown, path: string): number { + if (typeof value !== "number" || !Number.isSafeInteger(value) || value <= 0) + missing(path, value); + return value; +} + +export function parseWorkflowRunPage( + value: unknown, + head: string +): { + readonly checks: readonly WorkflowRunCheck[]; + readonly ids: readonly number[]; + readonly count: number; + readonly total: number; +} { + const page = record(value, "workflow runs"); + const total = page.total_count; + if (!Number.isSafeInteger(total) || Number(total) < 0) + missing("workflow runs.total_count", total); + const runs = list(page.workflow_runs, "workflow runs.workflow_runs"); + const ids: number[] = []; + const checks = runs + .map((value, index) => { + const run = record(value, `workflow runs[${index}]`); + const id = positiveInteger(run.id, "workflow run.id"); + const attempt = positiveInteger( + run.run_attempt, + "workflow run.run_attempt" + ); + const runNumber = positiveInteger( + run.run_number, + "workflow run.run_number" + ); + const workflowId = positiveInteger( + run.workflow_id, + "workflow run.workflow_id" + ); + const event = string(run.event, "workflow run.event"); + const repositoryId = positiveInteger( + at(run, ["head_repository", "id"]), + "workflow run.head_repository.id" + ); + const branch = optionalString(run.head_branch, "workflow run.head_branch"); + const prs = run.pull_requests === null + ? null + : list(run.pull_requests, "workflow run.pull_requests") + .map((pr) => JSON.stringify([ + positiveInteger( + at(pr, ["number"]), + "workflow run.pull_requests.number" + ), + positiveInteger( + at(pr, ["base", "repo", "id"]), + "workflow run.pull_requests.base.repo.id" + ), + string(at(pr, ["base", "ref"]), "workflow run.pull_requests.base.ref"), + ])) + .sort(); + const automatic = event === "push" || event === "pull_request" || + event === "pull_request_target"; + const unambiguous = automatic && branch !== null && prs !== null && + (event === "push" ? prs.length <= 1 : prs.length === 1); + const execution = JSON.stringify([ + workflowId, + event, + repositoryId, + branch, + prs, + ...(unambiguous ? [] : [id]), + ]); + ids.push(id); + const sha = string(run.head_sha, "workflow run.head_sha"); + const name = string(run.name, "workflow run.name"); + const link = string(run.html_url, "workflow run.html_url"); + const status = enumValue( + run.status, + [ + "queued", + "in_progress", + "waiting", + "pending", + "requested", + "completed", + ] as const, + "workflow run.status" + ); + const conclusion = optionalString( + run.conclusion, + "workflow run.conclusion" + ); + if (sha !== head) return null; + const details = { name, link, description: "", workflow: name }; + const reportedState = conclusion?.toUpperCase() ?? status.toUpperCase(); + const check: T.Check = + status !== "completed" || conclusion === null + ? gateOr("pending", details, reportedState) + : conclusion === "success" + ? { ...details, kind: "passed", reportedState } + : conclusion === "skipped" || conclusion === "neutral" + ? { ...details, kind: "skipped", reportedState } + : conclusion === "action_required" + ? gateOr("failed", details, reportedState) + : { ...details, kind: "failed", reportedState }; + return { ...check, id, attempt, runNumber, execution }; + }) + .filter((check): check is NonNullable => check !== null); + return { checks, ids, count: runs.length, total: Number(total) }; +} + export class GhGitHubReader implements T.GitHubReader { async originRepo(): Promise { const result = await run(["git", "remote", "get-url", "origin"]); @@ -576,6 +692,57 @@ export class GhGitHubReader implements T.GitHubReader { ); return { checks, endCursor: page.hasNextPage && cursor ? cursor : null }; } + async workflowRuns( + context: T.PrContext, + head: string + ): Promise { + const latest = new Map(); + const ids = new Set(); + let total: number | null = null; + for (let page = 1; page <= 10; page++) { + const result = parseWorkflowRunPage( + await runJson([ + "gh", + "api", + "-X", + "GET", + `repos/${context.owner}/${context.repo}/actions/runs`, + "-f", + `head_sha=${head}`, + "-F", + "per_page=100", + "-F", + `page=${page}`, + ]), + head + ); + if (result.total >= 1000) + missing("workflow runs exceeds complete-observation limit", result.total); + if (total !== null && result.total !== total) + missing("workflow runs total changed during pagination", result.total); + total = result.total; + if (result.count !== Math.min(100, total - ids.size)) + missing("workflow runs incomplete page", result.count); + for (const id of result.ids) { + if (ids.has(id)) missing("workflow runs duplicate id", id); + ids.add(id); + } + for (const check of result.checks) { + const previous = latest.get(check.execution); + if (previous !== undefined && + previous.runNumber === check.runNumber && previous.id !== check.id) + missing("workflow runs ambiguous run number", check.runNumber); + if (previous === undefined || previous.runNumber < check.runNumber || + (previous.runNumber === check.runNumber && previous.attempt < check.attempt)) + latest.set(check.execution, check); + } + if (ids.size === total) + return [...latest.values()].map( + ({ id, attempt, runNumber, execution, ...check }) => check + ); + } + return missing("workflow runs pagination limit"); + } async reviewThreadPage( context: T.PrContext, after: string | null diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/policy.test.ts b/pstack/skills/poteto-mode/scripts/watch-pr/policy.test.ts index 966b75367..5c8d20d87 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/policy.test.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/policy.test.ts @@ -91,6 +91,135 @@ describe("readiness truth table", () => { }); }); +describe("workflow startup", () => { + it("holds a clean partial check list while a workflow is queued or running", async () => { + for (const state of ["queued", "in_progress"] as const) { + const reader = fakeReader({ + workflowStatus: state, + facts: { mergeStateStatus: "UNSTABLE" }, + }); + const snapshot = await readSnapshot({ + reader, + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(snapshot.kind).toBe("open"); + if (snapshot.kind !== "open") throw new Error("expected open snapshot"); + expect(snapshot.ci.kind).toBe("ci-pending"); + expect(classifyPr(snapshot).kind).toBe("waiting"); + } + }); + + it("allows completed successful workflows, blocks failures, and preserves external CI", async () => { + for (const [status, expected] of [ + ["success", "ci-clean"], + ["failure", "ci-failing"], + ["none", "ci-clean"], + ] as const) { + const snapshot = await readSnapshot({ + reader: fakeReader({ workflowStatus: status }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(snapshot.kind).toBe("open"); + if (snapshot.kind !== "open") throw new Error("expected open snapshot"); + expect(snapshot.ci.kind).toBe(expected); + } + }); + + it("does not use an old head or a changed head as clean workflow evidence", async () => { + const old = await readSnapshot({ + reader: fakeReader({ workflowStatus: "queued", workflowHead: "old" }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(old.kind).toBe("open"); + if (old.kind !== "open") throw new Error("expected open snapshot"); + expect(old.ci.kind).toBe("ci-clean"); + await expect( + readSnapshot({ + reader: fakeReader({ facts: { headRefOid: null } }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }) + ).rejects.toBeInstanceOf(WatcherQueryError); + await expect( + readSnapshot({ + reader: fakeReader({ changedHead: "new" }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }) + ).rejects.toBeInstanceOf(WatcherQueryError); + }); + + it("does not wait on an owner-approval gate after workflows finish", async () => { + const gate = { + ...pendingCheck("Code Review Gate"), + kind: "code-review-gate" as const, + name: "Code Review Gate" as const, + }; + const snapshot = await readSnapshot({ + reader: fakeReader({ + fastPath: { kind: "checks", checks: [passingCheck(), gate] }, + workflowStatus: "success", + }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }); + expect(snapshot.kind).toBe("open"); + if (snapshot.kind !== "open") throw new Error("expected open snapshot"); + expect(snapshot.ci.kind).toBe("ci-clean"); + expect(classifyPr(snapshot).kind).toBe("ready"); + }); + + it("fails closed on a workflow query error", async () => { + await expect( + readSnapshot({ + reader: fakeReader({ workflowError: true }), + context: context(1), + pendingHistory: "omit", + allowDraft: false, + }) + ).rejects.toBeInstanceOf(WatcherQueryError); + }); +}); + +it("retries a workflow query error in a loop, then reports status-query", async () => { + let now = 0; + const events: ProgressVerdict[] = []; + const verdict = await runSimple({ + dependencies: { + reader: fakeReader({ workflowError: true }), + clock: { + now: () => now, + observedAt: () => "2026-07-26T00:00:00.000Z", + async sleep(seconds) { + now += seconds; + }, + }, + emit(event) { + events.push(event); + }, + }, + contexts: [context(1)], + mode: "single", + statusOnly: false, + options: { ...options, maxQueryErrors: 2 }, + }); + expect(events.map(({ kind }) => kind)).toEqual(["RETRY"]); + expect(verdict).toMatchObject({ + kind: "BLOCKER", + exitCode: 7, + blocker: { kind: "status-query", failures: 2 }, + }); +}); + describe("snapshot query planning", () => { it("does not query commit rollups while queued checks are pending", async () => { const reader = fakeReader({ @@ -109,6 +238,8 @@ describe("snapshot query planning", () => { "pullRequest", "reviewThreadPage:null", "checksFastPath", + "workflowRuns:head", + "pullRequest", ]); }); @@ -301,10 +432,13 @@ describe("queued-stack cadence", () => { expect(timeline).toEqual([ "emit:QUEUE", "read:20", + "read:20", "fail:21", "emit:RETRY", "sleep", "read:21", + "read:21", + "read:22", "read:22", "emit:STATUS", "emit:WAITING", @@ -348,7 +482,7 @@ describe("queued-stack cadence", () => { const facts = await base.pullRequest(pr); const count = (reads.get(pr.number) ?? 0) + 1; reads.set(pr.number, count); - return pr.number === one.number && count > 1 + return pr.number === one.number && count > 2 ? { ...facts, state: "MERGED" as const, @@ -384,11 +518,12 @@ describe("queued-stack cadence", () => { await expect(running).rejects.toThrow("stop after advance proof"); expect(emitted.some((event) => event.kind === "ADVANCE")).toBe(true); const firstSleep = timeline.indexOf("sleep"); - expect(timeline.slice(firstSleep, firstSleep + 5)).toEqual([ + expect(timeline.slice(firstSleep, firstSleep + 6)).toEqual([ "sleep", "read:40", "emit:ADVANCE", "read:41", + "read:41", "emit:WAITING", ]); }); diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts b/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts index 8179f2d8b..fe073448c 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts @@ -69,22 +69,29 @@ export async function readSnapshot(args: { return { kind: "closed", context: args.context, facts }; const threads = await resolveReviewThreads(args.reader, args.context); const checks = await resolveChecks(args.reader, args.context); + if (facts.headRefOid === null) + throw new WatcherQueryError({ + kind: "missing-key", + retryable: true, + detail: "PR head SHA unavailable", + }); + const workflows = await args.reader.workflowRuns( + args.context, + facts.headRefOid + ); + const all: T.NonEmpty = [...checks.checks, ...workflows]; const failed = nonEmpty( - checks.checks.filter( - (check): check is T.FailedCheck => check.kind === "failed" - ) + all.filter((check): check is T.FailedCheck => check.kind === "failed") ); const pending = nonEmpty( - checks.checks.filter( - (check): check is T.PendingCheck => check.kind === "pending" - ) + all.filter((check): check is T.PendingCheck => check.kind === "pending") ); let ci: T.CiState; if (failed === null && pending !== null && args.pendingHistory === "omit") ci = { kind: "ci-pending", source: checks.source, - all: checks.checks, + all, failed: [], pending, hadPreviousPassingCi: false, @@ -93,7 +100,7 @@ export async function readSnapshot(args: { const merge = await mergeAssessment(args.reader, facts); const base = { source: checks.source, - all: checks.checks, + all, hadPreviousPassingCi: merge.hadPreviousPassingCi, }; if (failed !== null) @@ -123,6 +130,13 @@ export async function readSnapshot(args: { github: merge.github, }; } + const current = await args.reader.pullRequest(args.context); + if (current.headRefOid !== facts.headRefOid || current.state !== facts.state) + throw new WatcherQueryError({ + kind: "missing-key", + retryable: true, + detail: "PR changed during snapshot", + }); return { kind: "open", context: args.context, diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/types.ts b/pstack/skills/poteto-mode/scripts/watch-pr/types.ts index 60b9abcac..3aebdb67e 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/types.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/types.ts @@ -399,6 +399,7 @@ export interface GitHubReader { context: PrContext, after: string | null ): Promise; + workflowRuns(context: PrContext, head: string): Promise; reviewThreadPage( context: PrContext, after: string | null diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/workflow-runs.test.ts b/pstack/skills/poteto-mode/scripts/watch-pr/workflow-runs.test.ts new file mode 100644 index 000000000..6ae91d751 --- /dev/null +++ b/pstack/skills/poteto-mode/scripts/watch-pr/workflow-runs.test.ts @@ -0,0 +1,401 @@ +import { describe, expect, it } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const head = "a".repeat(40); +const run = ( + id: number, + conclusion: string | null = "success", + overrides = {}, +) => ({ + id, + workflow_id: 200, + run_number: id, + run_attempt: 1, + event: "pull_request", + head_repository: { id: 300 }, + head_branch: "feature", + pull_requests: [{ number: 1, base: { ref: "main", repo: { id: 400 } } }], + name: "CI", + head_sha: head, + status: conclusion === null ? "queued" : "completed", + conclusion, + html_url: `https://github.com/example/fixture/actions/runs/${id}`, + ...overrides, +}); +const page = (runs: readonly unknown[], total = runs.length) => ({ + total_count: total, + workflow_runs: runs, +}); +const first = Array.from({ length: 100 }, (_, i) => run(i + 1)); +const cases = [ + { + name: "obsolete cancellation replaced by success", + pages: [page([run(101), run(100, "cancelled")])], + code: 0, + }, + { + name: "obsolete failure replaced by success", + pages: [page([run(100, "failure"), run(101)])], + code: 0, + }, + { + name: "current failure", + pages: [page([run(100), run(101, "failure")])], + code: 4, + }, + { + name: "queued replacement of failed execution", + pages: [page([run(100, "failure"), run(101, null)])], + code: 5, + }, + { + name: "running replacement", + pages: [ + page([run(100, "cancelled"), run(101, null, { status: "in_progress" })]), + ], + code: 5, + }, + { + name: "new execution supersedes older rerun attempt", + pages: [page([run(100, "failure", { run_attempt: 5 }), run(101)])], + code: 0, + }, + { + name: "latest attempt fails", + pages: [page([run(100, "failure", { run_attempt: 2 })])], + code: 4, + }, + { + name: "latest attempt succeeds", + pages: [page([run(100, "success", { run_attempt: 2 })])], + code: 0, + }, + { + name: "latest attempt is queued", + pages: [page([run(100, null, { run_attempt: 2 })])], + code: 5, + }, + { + name: "workflow run number orders replacements", + pages: [ + page([ + run(101, "failure", { run_number: 1 }), + run(100, "success", { run_number: 2 }), + ]), + ], + code: 0, + }, + { + name: "push replacement without PR associations", + pages: [ + page([ + run(100, "failure", { event: "push", pull_requests: [] }), + run(101, "success", { event: "push", pull_requests: [] }), + ]), + ], + code: 0, + }, + { + name: "same name independent workflow", + pages: [ + page([run(100, "failure"), run(101, "success", { workflow_id: 201 })]), + ], + code: 4, + }, + { + name: "independent event", + pages: [ + page([run(100, "failure"), run(101, "success", { event: "push" })]), + ], + code: 4, + }, + { + name: "independent head repository", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { head_repository: { id: 301 } }), + ]), + ], + code: 4, + }, + { + name: "independent branch", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { head_branch: "other" }), + ]), + ], + code: 4, + }, + { + name: "independent PR association", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { + pull_requests: [ + { number: 2, base: { ref: "main", repo: { id: 400 } } }, + ], + }), + ]), + ], + code: 4, + }, + { + name: "independent PR base", + pages: [ + page([ + run(100, "failure"), + run(101, "success", { + pull_requests: [ + { number: 1, base: { ref: "release", repo: { id: 400 } } }, + ], + }), + ]), + ], + code: 4, + }, + { + name: "manual dispatch inputs cannot be reconciled", + pages: [ + page([ + run(100, "failure", { event: "workflow_dispatch" }), + run(101, "success", { event: "workflow_dispatch" }), + ]), + ], + code: 4, + }, + { + name: "ambiguous multiple PR associations", + pages: [ + page([ + run(100, "failure", { + pull_requests: [ + { number: 1, base: { ref: "main", repo: { id: 400 } } }, + { number: 2, base: { ref: "main", repo: { id: 400 } } }, + ], + }), + run(101, "success", { + pull_requests: [ + { number: 1, base: { ref: "main", repo: { id: 400 } } }, + { number: 2, base: { ref: "main", repo: { id: 400 } } }, + ], + }), + ]), + ], + code: 4, + }, + { + name: "repository dispatch inputs cannot be reconciled", + pages: [ + page([ + run(100, "failure", { event: "repository_dispatch" }), + run(101, "success", { event: "repository_dispatch" }), + ]), + ], + code: 4, + }, + { + name: "unknown PR associations", + pages: [ + page([ + run(100, "failure", { pull_requests: null }), + run(101, "success", { pull_requests: null }), + ]), + ], + code: 4, + }, + { + name: "unknown head branch", + pages: [ + page([ + run(100, "failure", { head_branch: null }), + run(101, "success", { head_branch: null }), + ]), + ], + code: 4, + }, + { + name: "current registered failure remains authoritative", + pages: [page([run(101)])], + checkState: "FAILURE", + code: 4, + }, + { + name: "complete pagination", + pages: [page(first, 101), page([run(101)], 101)], + code: 0, + }, + { + name: "maximum complete observation below cap", + pages: Array.from({ length: 10 }, (_, p) => + page( + Array.from({ length: p === 9 ? 99 : 100 }, (_, i) => + run(p * 100 + i + 1), + ), + 999, + ), + ), + code: 0, + }, + { + name: "duplicate attempts cannot prove complete observation", + pages: [page([run(100), run(100, "failure", { run_attempt: 2 })])], + code: 7, + }, + { + name: "duplicate row cannot satisfy total", + pages: [page(first, 101), page([run(100)], 101)], + code: 7, + }, + { + name: "increasing total", + pages: [page(first, 101), page([run(101)], 102)], + code: 7, + }, + { + name: "shrinking total", + pages: [page(first, 101), page([], 100)], + code: 7, + }, + { + name: "empty page before completion", + pages: [page(first, 101), page([], 101)], + code: 7, + }, + { + name: "short page before completion", + pages: [page([run(100)], 2)], + code: 7, + }, + { + name: "rows exceed total", + pages: [page([run(100), run(101)], 1)], + code: 7, + }, + { name: "filtered result at API cap", pages: [page(first, 1000)], code: 7 }, + { + name: "filtered result above API cap", + pages: [page(first, 1001)], + code: 7, + }, + { + name: "hostile changing totals are bounded", + pages: [page(first, 1000000)], + code: 7, + }, + { + name: "missing execution identity", + pages: [page([run(100, "success", { workflow_id: null })])], + code: 7, + }, + { name: "no Actions with external checks", pages: [page([])], code: 0 }, +] as const; + +describe("effective Actions executions through the source CLI", () => { + for (const fixture of cases) { + it(fixture.name, () => { + const directory = mkdtempSync(join(tmpdir(), "watch-pr-executions-")); + try { + const calls = join(directory, "calls.ndjson"); + writeFileSync(join(directory, "fixture.json"), JSON.stringify(fixture)); + writeFileSync( + join(directory, "gh"), + `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +fs.appendFileSync(process.env.CALLS, JSON.stringify(args) + "\\n"); +const fixture = JSON.parse(fs.readFileSync(process.env.FIXTURE, "utf8")); +const head = "a".repeat(40); +let value; +if (args[0] === "pr" && args[1] === "view") + value = { mergeable: "MERGEABLE", mergeStateStatus: "UNSTABLE", reviewDecision: "APPROVED", headRefOid: head, headRefName: "feature", baseRefName: "main", state: "OPEN", mergedAt: null, isDraft: false }; +else if (args[0] === "pr" && args[1] === "checks") + value = [{ name: "CI", state: fixture.checkState || "SUCCESS", bucket: fixture.checkState ? "fail" : "pass", description: "", link: "", workflow: "CI" }]; +else if (args.includes("graphql")) { + const query = args.find(arg => arg.startsWith("query=")); + const pullRequest = query.includes("ReviewThreads") + ? { reviewThreads: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } } } + : { commits: { nodes: [{ commit: { oid: head, statusCheckRollup: { state: "SUCCESS" } } }] } }; + value = { data: { repository: { pullRequest } } }; +} else if (args.some(arg => arg.includes("/actions/runs"))) { + const page = Number(args.find(arg => arg.startsWith("page=")).slice(5)); + value = fixture.pages[page - 1] || fixture.pages[fixture.pages.length - 1]; +} else process.exit(2); +console.log(JSON.stringify(value)); +`, + ); + chmodSync(join(directory, "gh"), 0o755); + const result = spawnSync( + process.execPath, + [ + join(import.meta.dir, "watch-pr"), + "--owner", + "example", + "--repo", + "fixture", + "--pr", + "1", + "--timeout", + ".01", + "--interval", + ".001", + "--max-query-errors", + "1", + ], + { + encoding: "utf8", + timeout: 10000, + env: { + ...process.env, + PATH: `${directory}:${process.env.PATH}`, + CALLS: calls, + FIXTURE: join(directory, "fixture.json"), + }, + }, + ); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(fixture.code); + const events = result.stdout + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + expect(events.at(-1).kind).toBe( + fixture.code === 0 + ? "READY" + : fixture.code === 5 + ? "TIMEOUT" + : "BLOCKER", + ); + if (fixture.code !== 0) + expect(events.some((event) => event.kind === "READY")).toBe(false); + if (fixture.code === 5) expect(events[0].kind).toBe("WAITING"); + const queries: string[][] = readFileSync(calls, "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + const actions = queries.filter((args) => + args.some((arg) => arg.includes("/actions/runs")), + ); + expect(actions.every((args) => args.includes(`head_sha=${head}`))).toBe( + true, + ); + expect(actions.length).toBeGreaterThan(0); + expect(actions.length).toBeLessThanOrEqual(10); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + } +});