From 9b690bb633cceb16c565c6bad75fdc2cca50b372 Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Thu, 15 Oct 2026 22:50:01 +1000 Subject: [PATCH 1/6] test(factory): port softfac's pull request, compare and check-run cases Red: github.ts has no pullRequest, markPrReady, markPrDraft, commentOnPr, compare or checks yet, so the 38 new cases fail and the file does not typecheck. Issue: #14 --- .../server/src/factory/tracker/github.test.ts | 586 ++++++++++++++++++ 1 file changed, 586 insertions(+) diff --git a/apps/server/src/factory/tracker/github.test.ts b/apps/server/src/factory/tracker/github.test.ts index 3e1f7ba61dd0..91b2a3326000 100644 --- a/apps/server/src/factory/tracker/github.test.ts +++ b/apps/server/src/factory/tracker/github.test.ts @@ -1342,3 +1342,589 @@ it("error text escapes newlines in other arguments", () => { "cannot start `gh --body`: nope", ); }); + +// ---- pullRequest ---- + +const HEAD_SHA = "4f1c2d3e4a5b60718293a4b5c6d7e8f901234567"; +const PR_VIEW = + '{"baseRefName":"main","headRefName":"t3code/2bb65018","headRefOid":"4f1c2d3e4a5b60718293a4b5c6d7e8f901234567","isCrossRepository":false,"isDraft":true,"number":2,"state":"OPEN"}'; +const prView = (state: string, draft: boolean) => + `{"baseRefName":"main","headRefName":"x","headRefOid":"${HEAD_SHA}","isCrossRepository":false,"isDraft":${draft},"number":9,"state":"${state}"}`; +const prViewCall = (number: string) => [ + "pr", + "view", + number, + "--repo", + REPO, + "--json", + "number,isDraft,state,headRefName,isCrossRepository,baseRefName,headRefOid", +]; + +it.effect("pullRequest reads the real shape with the exact call", () => + Effect.gen(function* () { + const t = setup(reply(PR_VIEW)); + const pr = yield* t.ok(t.github.pullRequest(2)); + assert.deepStrictEqual(t.calls(), [prViewCall("2")]); + assert.deepStrictEqual(pr, { + number: 2, + isDraft: true, + open: true, + merged: false, + headRef: "t3code/2bb65018", + crossRepository: false, + baseRef: "main", + headSha: HEAD_SHA, + }); + }), +); + +it.effect("pullRequest says when it is merged and is then not open", () => + Effect.gen(function* () { + const merged = setup(reply(PR_VIEW.replace('"state":"OPEN"', '"state":"MERGED"'))); + const pr = yield* merged.ok(merged.github.pullRequest(2)); + assert.isTrue(pr?.merged); + assert.isFalse(pr?.open); + const closed = setup(reply(PR_VIEW.replace('"state":"OPEN"', '"state":"CLOSED"'))); + const other = yield* closed.ok(closed.github.pullRequest(2)); + assert.isFalse(other?.merged); + assert.isFalse(other?.open); + }), +); + +it.effect("pullRequest from a fork and into another base says so", () => + Effect.gen(function* () { + const t = setup( + reply( + `{"baseRefName":"release/1.0","headRefName":"t3code/x","headRefOid":"${HEAD_SHA}","isCrossRepository":true,"isDraft":true,"number":4,"state":"OPEN"}`, + ), + ); + const pr = yield* t.ok(t.github.pullRequest(4)); + assert.isTrue(pr?.crossRepository); + assert.strictEqual(pr?.baseRef, "release/1.0"); + assert.strictEqual(pr?.headRef, "t3code/x"); + }), +); + +it.effect("a pullRequest view without the fork or base fields is a parse error", () => + Effect.gen(function* () { + const t = setup( + reply( + `{"headRefName":"x","headRefOid":"${HEAD_SHA}","isDraft":true,"number":3,"state":"OPEN"}`, + ), + ); + const error = yield* t.err(t.github.pullRequest(3)); + assert.strictEqual(error._tag, "GitHubParse"); + }), +); + +it.effect("a pullRequest view without the head commit is a parse error", () => + Effect.gen(function* () { + const t = setup( + reply( + '{"baseRefName":"main","headRefName":"x","isCrossRepository":false,"isDraft":true,"number":3,"state":"OPEN"}', + ), + ); + const error = yield* t.err(t.github.pullRequest(3)); + assert.strictEqual(error._tag, "GitHubParse"); + }), +); + +it.effect("pullRequest reads the head commit as given", () => + Effect.gen(function* () { + const sha = "0123456789abcdef0123456789abcdef01234567"; + const t = setup( + reply( + `{"baseRefName":"main","headRefName":"x","headRefOid":"${sha}","isCrossRepository":false,"isDraft":true,"number":3,"state":"OPEN"}`, + ), + ); + assert.strictEqual((yield* t.ok(t.github.pullRequest(3)))?.headSha, sha); + }), +); + +it.effect("pullRequest merged and closed are not open", () => + Effect.gen(function* () { + for (const state of ["MERGED", "CLOSED"]) { + const t = setup(reply(prView(state, false))); + const pr = yield* t.ok(t.github.pullRequest(9)); + assert.isFalse(pr?.open, state); + assert.isFalse(pr?.isDraft, state); + } + }), +); + +it.effect("a pullRequest that does not exist is null", () => + Effect.gen(function* () { + for (const stderr of [ + "GraphQL: Could not resolve to a PullRequest with the number of 999. (repository.pullRequest)\n", + "graphql: could not resolve to a pullrequest with the number of 999.", + "GRAPHQL: COULD NOT RESOLVE TO A PULLREQUEST", + ]) { + const t = setup(fail(1, stderr)); + assert.isNull(yield* t.ok(t.github.pullRequest(999)), stderr); + } + }), +); + +it.effect("any other pullRequest failure stays an error", () => + Effect.gen(function* () { + for (const stderr of [ + "HTTP 502: Bad Gateway", + "Could not resolve host: api.github.com", + "Could not resolve to an Issue with the number of 5.", + "", + ]) { + const t = setup(fail(1, stderr)); + const error = yield* t.err(t.github.pullRequest(999)); + assert.strictEqual(error._tag, "GitHubFailed", stderr); + if (error._tag === "GitHubFailed") assert.strictEqual(error.status, 1, stderr); + } + const t = setup(spawnError()); + const error = yield* t.err(t.github.pullRequest(999)); + assert.strictEqual(error._tag, "GitHubSpawnError"); + }), +); + +it.effect("pullRequest errors are typed", () => + Effect.gen(function* () { + const failed = setup(fail(1, "no pull requests found")); + assert.strictEqual((yield* failed.err(failed.github.pullRequest(2)))._tag, "GitHubFailed"); + const parse = setup(reply("{}")); + assert.strictEqual((yield* parse.err(parse.github.pullRequest(2)))._tag, "GitHubParse"); + }), +); + +it.effect("issue and pullRequest calls use the number they are given", () => + Effect.gen(function* () { + const t = setup(reply(issueJson(["factory:ready"], [], '"b"')), reply(prView("OPEN", true))); + yield* t.ok(t.github.issue(7)); + yield* t.ok(t.github.pullRequest(3)); + assert.deepStrictEqual(t.calls(), [ + [ + "issue", + "view", + "7", + "--repo", + REPO, + "--json", + "number,title,body,labels,assignees,state,author", + ], + prViewCall("3"), + ]); + }), +); + +// ---- markPrReady, markPrDraft, commentOnPr ---- + +const readyCall = ["pr", "ready", "9", "--repo", REPO]; +const undoCall = [...readyCall, "--undo"]; + +it.effect("markPrReady runs gh pr ready", () => + Effect.gen(function* () { + const t = setup(reply("")); + yield* t.ok(t.github.markPrReady(9)); + assert.deepStrictEqual(t.calls(), [readyCall]); + + const failed = setup(fail(1, "not a draft")); + const error = yield* failed.err(failed.github.markPrReady(9)); + assert.strictEqual(error._tag, "GitHubFailed"); + }), +); + +it.effect("markPrReady on a PR that is already ready is success", () => + Effect.gen(function* () { + // What `gh` 2.102 does: it warns on stderr and exits 0. + const stderr = '! Pull request rawphp/softfac#9 is already "ready for review"\n'; + const warned = setup(reply("", stderr)); + yield* warned.ok(warned.github.markPrReady(9)); + + // Should a version exit non-zero for it, the PR is still ready: that is success too. + const failed = setup(fail(1, stderr)); + yield* failed.ok(failed.github.markPrReady(9)); + assert.deepStrictEqual(failed.calls(), [readyCall]); + }), +); + +it.effect("markPrReady still fails for a closed PR or any other error", () => + Effect.gen(function* () { + for (const stderr of [ + '! Pull request rawphp/softfac#9 is closed. Only draft pull requests can be marked as "ready for review"\n', + "GraphQL: Resource not accessible by integration\n", + "", + ]) { + const t = setup(fail(1, stderr)); + const error = yield* t.err(t.github.markPrReady(9)); + assert.strictEqual(error._tag, "GitHubFailed", stderr); + } + }), +); + +it.effect("markPrDraft runs gh pr ready --undo", () => + Effect.gen(function* () { + const t = setup(reply("")); + yield* t.ok(t.github.markPrDraft(9)); + assert.deepStrictEqual(t.calls(), [undoCall]); + + const failed = setup(fail(1, "GraphQL: Resource not accessible by integration\n")); + const error = yield* failed.err(failed.github.markPrDraft(9)); + assert.strictEqual(error._tag, "GitHubFailed"); + }), +); + +it.effect("markPrDraft on a PR that is already a draft is success", () => + Effect.gen(function* () { + // What `gh` does for `--undo` on a draft: it warns on stderr and exits 0. Should a version + // exit non-zero for it, the PR is still a draft: that is success too. + const stderr = '! Pull request rawphp/softfac#9 is already "in draft"\n'; + const warned = setup(reply("", stderr)); + yield* warned.ok(warned.github.markPrDraft(9)); + + const failed = setup(fail(1, stderr)); + yield* failed.ok(failed.github.markPrDraft(9)); + assert.deepStrictEqual(failed.calls(), [undoCall]); + }), +); + +it.effect("markPrDraft on a PR that is closed or merged is done whatever gh says", () => + Effect.gen(function* () { + // The words of the refusal are not relied on: the pull request's state decides. + for (const state of ["CLOSED", "MERGED"]) { + for (const stderr of [ + "! Pull request rawphp/softfac#9 is closed. Only open pull requests can be converted to draft\n", + "GraphQL: Something unforeseen\n", + ]) { + const t = setup(fail(1, stderr), reply(prView(state, false))); + yield* t.ok(t.github.markPrDraft(9)); + assert.deepStrictEqual(t.calls(), [undoCall, prViewCall("9")], `${state}: ${stderr}`); + } + } + }), +); + +it.effect("markPrDraft that fails on an open PR or cannot read it is the original error", () => + Effect.gen(function* () { + // Open: the failure is real. + const open = setup( + fail(1, "GraphQL: Resource not accessible by integration\n"), + reply(prView("OPEN", false)), + ); + const onOpen = yield* open.err(open.github.markPrDraft(9)); + assert.strictEqual(onOpen._tag, "GitHubFailed"); + if (onOpen._tag === "GitHubFailed") assert.include(onOpen.stderr, "not accessible"); + + // The read fails too: still the original error. + const unread = setup( + fail(1, "GraphQL: Resource not accessible by integration\n"), + fail(1, "HTTP 502: Bad Gateway"), + ); + const onUnread = yield* unread.err(unread.github.markPrDraft(9)); + assert.strictEqual(onUnread._tag, "GitHubFailed"); + if (onUnread._tag === "GitHubFailed") assert.include(onUnread.stderr, "not accessible"); + + // One that is gone: nothing to draft either. + const gone = setup( + fail(1, "boom"), + fail(1, "GraphQL: Could not resolve to a PullRequest with the number of 9."), + ); + const onGone = yield* gone.err(gone.github.markPrDraft(9)); + assert.strictEqual(onGone._tag, "GitHubFailed"); + if (onGone._tag === "GitHubFailed") assert.include(onGone.stderr, "boom"); + }), +); + +it.effect("commentOnPr appends the marker", () => + Effect.gen(function* () { + const t = setup(reply("")); + yield* t.ok(t.github.commentOnPr(9, "gate passed")); + assert.deepStrictEqual(t.calls(), [ + ["pr", "comment", "9", "--repo", REPO, "--body", "gate passed\n\n"], + ]); + + const failed = setup(spawnError()); + const error = yield* failed.err(failed.github.commentOnPr(9, "x")); + assert.strictEqual(error._tag, "GitHubSpawnError"); + }), +); + +// ---- compare ---- + +const compareCall = (range: string) => [ + "api", + `repos/rawphp/softfac/compare/${range}?per_page=1`, + "--jq", + ".status", +]; + +it.effect("compare asks for the status of the two refs with the exact call", () => + Effect.gen(function* () { + const t = setup(reply("ahead\n")); + assert.strictEqual(yield* t.ok(t.github.compare("main", BASE_SHA)), "ahead"); + assert.deepStrictEqual(t.calls(), [compareCall(`main...${BASE_SHA}`)]); + }), +); + +it.effect("compare reads each status GitHub gives", () => + Effect.gen(function* () { + for (const [text, relation] of [ + ["ahead", "ahead"], + ["behind\n", "behind"], + ["identical\n", "identical"], + ["diverged", "diverged"], + ] as const) { + const t = setup(reply(text)); + assert.strictEqual(yield* t.ok(t.github.compare("main", BASE_SHA)), relation, text); + } + }), +); + +it.effect("compare of a commit GitHub does not know is null", () => + Effect.gen(function* () { + const t = setup(fail(1, "gh: No commit found for SHA: 9f8e7d6 (HTTP 404)")); + assert.isNull(yield* t.ok(t.github.compare("main", BASE_SHA))); + }), +); + +it.effect("compare keeps every other failure an error", () => + Effect.gen(function* () { + for (const answer of [ + fail(1, "gh: API rate limit exceeded (HTTP 403)"), + fail(1, "gh: Server Error (HTTP 500)"), + fail(4, "gh: To get started with GitHub CLI, please run: gh auth login"), + spawnError(), + ]) { + const t = setup(answer); + yield* t.err(t.github.compare("main", BASE_SHA)); + } + }), +); + +it.effect("compare refuses a status it does not know", () => + Effect.gen(function* () { + for (const answer of ["", "sideways", '"ahead"', "null", "{}"]) { + const t = setup(reply(answer)); + const error = yield* t.err(t.github.compare("main", BASE_SHA)); + assert.strictEqual(error._tag, "GitHubParse", answer); + } + }), +); + +it.effect("compare encodes a branch and refuses a ref that could leave the compare path", () => + Effect.gen(function* () { + const t = setup(reply("behind")); + yield* t.ok(t.github.compare("release/1.0", BASE_SHA)); + assert.strictEqual( + t.calls()[0]![1], + `repos/rawphp/softfac/compare/release/1.0...${BASE_SHA}?per_page=1`, + ); + for (const [base, head] of [ + ["", BASE_SHA], + ["main", ""], + ["..", BASE_SHA], + ["a..b", BASE_SHA], + ["main", "../x"], + ["main", "a...b"], + ["a/../b", BASE_SHA], + ]) { + const refused = setup(reply("ahead")); + const error = yield* refused.err(refused.github.compare(base!, head!)); + assert.strictEqual(error._tag, "GitHubInvalidArgument", `${base} ${head}`); + assert.deepStrictEqual(refused.calls(), [], `${base} ${head}`); + } + }), +); + +// ---- checks ---- + +const run = (name: string, status: string, conclusion: string | null) => ({ + id: 1, + name, + status, + conclusion, + html_url: `https://github.com/rawphp/softfac/actions/runs/1/job/${name}`, + app: { slug: "github-actions" }, +}); +const page = (runs: ReadonlyArray) => JSON.stringify(runs); +const checksCall = [ + "api", + `repos/rawphp/softfac/commits/${BASE_SHA}/check-runs?per_page=100&filter=latest`, + "--paginate", + "--jq", + "[.check_runs[] | {name, status, conclusion, html_url}]", +]; + +it.effect("checks reads the check runs of the commit with the exact call", () => + Effect.gen(function* () { + const t = setup(reply(page([run("gate", "completed", "success")]))); + const checks = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.deepStrictEqual(t.calls(), [checksCall]); + assert.deepStrictEqual(checks, [ + { + name: "gate", + state: "passed", + conclusion: "success", + url: "https://github.com/rawphp/softfac/actions/runs/1/job/gate", + }, + ]); + }), +); + +it.effect("checks answers for each name asked in the order asked and ignores other runs", () => + Effect.gen(function* () { + const t = setup( + reply( + page([ + run("lint", "completed", "success"), + run("other", "completed", "failure"), + run("gate", "completed", "success"), + ]), + ), + ); + const checks = yield* t.ok(t.github.checks(BASE_SHA, ["gate", "lint"])); + assert.deepStrictEqual( + checks.map((c) => [c.name, c.state]), + [ + ["gate", "passed"], + ["lint", "passed"], + ], + ); + }), +); + +it.effect("a completed run passes on success and neutral and fails on anything else", () => + Effect.gen(function* () { + for (const conclusion of ["success", "neutral"]) { + const t = setup(reply(page([run("gate", "completed", conclusion)]))); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(got[0]!.state, "passed", conclusion); + } + for (const conclusion of [ + "failure", + "cancelled", + "timed_out", + "action_required", + "stale", + "startup_failure", + "something_new", + ]) { + const t = setup(reply(page([run("gate", "completed", conclusion)]))); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(got[0]!.state, "failed", conclusion); + assert.strictEqual(got[0]!.conclusion, conclusion); + } + // Completed with no conclusion is not a pass. + const t = setup(reply(page([run("gate", "completed", null)]))); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(got[0]!.state, "failed"); + assert.strictEqual(got[0]!.conclusion, "completed"); + }), +); + +it.effect("a run that has not completed is pending", () => + Effect.gen(function* () { + for (const status of ["queued", "in_progress", "waiting", "pending", "requested"]) { + const t = setup(reply(page([run("gate", status, null)]))); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(got[0]!.state, "pending", status); + assert.strictEqual(got[0]!.conclusion, status); + } + }), +); + +it.effect("a check with no run is missing and not pending", () => + Effect.gen(function* () { + const t = setup(reply("[]")); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.deepStrictEqual(got, [{ name: "gate", state: "missing", conclusion: "", url: null }]); + }), +); + +it.effect("a name that is not exactly the run's is missing", () => + Effect.gen(function* () { + const t = setup(reply(page([run("Gate", "completed", "success")]))); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(got[0]!.state, "missing"); + }), +); + +it.effect("a skipped run is not a result so the check stays missing", () => + Effect.gen(function* () { + // The factory's pull request is a draft until the checks pass, and a job guarded by + // "not a draft" is skipped on it: nothing ran, so nothing passed. + const t = setup(reply(page([run("gate", "completed", "skipped")]))); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(got[0]!.state, "missing"); + assert.strictEqual(got[0]!.conclusion, "skipped"); + }), +); + +it.effect("several runs of one name fail before they pend and pend before they pass", () => + Effect.gen(function* () { + type Run = readonly [string, string | null]; + const state = (a: Run, b: Run) => + Effect.gen(function* () { + const t = setup(reply(page([run("gate", a[0], a[1]), run("gate", b[0], b[1])]))); + return (yield* t.ok(t.github.checks(BASE_SHA, ["gate"])))[0]!.state; + }); + assert.strictEqual(yield* state(["completed", "success"], ["completed", "failure"]), "failed"); + assert.strictEqual(yield* state(["in_progress", null], ["completed", "success"]), "pending"); + assert.strictEqual(yield* state(["completed", "success"], ["completed", "neutral"]), "passed"); + // A failure is not hidden by a run that is going again, in either order. + assert.strictEqual(yield* state(["completed", "failure"], ["in_progress", null]), "failed"); + assert.strictEqual(yield* state(["in_progress", null], ["completed", "failure"]), "failed"); + // One skipped run among passing ones is not every run passing. + assert.strictEqual(yield* state(["completed", "success"], ["completed", "skipped"]), "missing"); + assert.strictEqual(yield* state(["completed", "skipped"], ["completed", "success"]), "missing"); + }), +); + +it.effect("checks reads every page", () => + Effect.gen(function* () { + const t = setup( + reply( + `${page([run("gate", "completed", "success")])}\n${page([run("lint", "in_progress", null)])}`, + ), + ); + const got = yield* t.ok(t.github.checks(BASE_SHA, ["gate", "lint"])); + assert.strictEqual(got[0]!.state, "passed"); + assert.strictEqual(got[1]!.state, "pending"); + }), +); + +it.effect("checks keeps a details link only when it is a GitHub one", () => + Effect.gen(function* () { + const odd = setup( + reply(page([{ ...run("gate", "completed", "failure"), html_url: "https://evil.example/x" }])), + ); + assert.isNull((yield* odd.ok(odd.github.checks(BASE_SHA, ["gate"])))[0]!.url); + const none = setup(reply(page([{ ...run("gate", "completed", "failure"), html_url: null }]))); + assert.isNull((yield* none.ok(none.github.checks(BASE_SHA, ["gate"])))[0]!.url); + }), +); + +it.effect("checks refuses a commit that is not a full id before any call", () => + Effect.gen(function* () { + for (const sha of ["", "main", "9f8e7d6", "../x", BASE_SHA.replaceAll("9", "g")]) { + const t = setup(reply("[]")); + const error = yield* t.err(t.github.checks(sha, ["gate"])); + assert.strictEqual(error._tag, "GitHubInvalidArgument", sha); + assert.deepStrictEqual(t.calls(), [], sha); + } + }), +); + +it.effect("checks refuses an answer that is not a list of runs", () => + Effect.gen(function* () { + for (const answer of ["{}", "not json", '[{"name":"gate"}]']) { + const t = setup(reply(answer)); + const error = yield* t.err(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(error._tag, "GitHubParse", answer); + } + }), +); + +it.effect("checks that gh cannot read is the gh failure", () => + Effect.gen(function* () { + const t = setup(fail(1, "gh: Not Found (HTTP 404)")); + const error = yield* t.err(t.github.checks(BASE_SHA, ["gate"])); + assert.strictEqual(error._tag, "GitHubFailed"); + }), +); From 9a1137638087162e11bd67a55a55ceb958ed83a3 Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Thu, 15 Oct 2026 22:52:31 +1000 Subject: [PATCH 2/6] feat(factory): pull request reads and writes through gh tracker/github.ts gains pullRequest, markPrReady, markPrDraft, commentOnPr, compare and checks, each with the argv softfac's github.rs uses. branchTip landed with #12. A ref refused before the call (a `..` in compare, a sha that is not a full id in checks) is a GitHubInvalidArgument, as branchTip and fileAt already do, where softfac reports it as a parse error. Issue: #14 --- apps/server/src/factory/tracker/github.ts | 242 ++++++++++++++++++++++ 1 file changed, 242 insertions(+) diff --git a/apps/server/src/factory/tracker/github.ts b/apps/server/src/factory/tracker/github.ts index 1193ef602066..407ecaf6a4e4 100644 --- a/apps/server/src/factory/tracker/github.ts +++ b/apps/server/src/factory/tracker/github.ts @@ -191,6 +191,25 @@ const RefView = Schema.Struct({ object: Schema.Struct({ sha: Schema.String, type: Schema.String }), }); +const PullRequestView = Schema.Struct({ + number: Schema.Number, + isDraft: Schema.Boolean, + state: Schema.String, + headRefName: Schema.String, + isCrossRepository: Schema.Boolean, + baseRefName: Schema.String, + headRefOid: Schema.String, +}); + +const CheckRunRows = Schema.Array( + Schema.Struct({ + name: Schema.String, + status: Schema.String, + conclusion: OptionalText, + html_url: OptionalText, + }), +); + const FileView = Schema.Struct({ type: Schema.String, encoding: OptionalText, @@ -231,6 +250,54 @@ export interface BaseConfig { readonly text: string; } +/** One pull request, as read by `gh pr view`. */ +export interface PullRequest { + readonly number: number; + readonly isDraft: boolean; + /** Neither merged nor closed. */ + readonly open: boolean; + /** Merged into its base. */ + readonly merged: boolean; + /** The branch the pull request comes from. */ + readonly headRef: string; + /** The head is in a fork, not in this repo. */ + readonly crossRepository: boolean; + /** The branch the pull request merges into. */ + readonly baseRef: string; + /** The commit the pull request's head branch points at, as GitHub reports it. */ + readonly headSha: string; +} + +/** + * How one ref stands against another, in GitHub's words: `ahead` when the second + * holds everything of the first and more, `behind` when the second is inside the + * first one's history, `identical`, or `diverged` when each has commits the other lacks. + */ +export type Relation = "ahead" | "behind" | "identical" | "diverged"; + +/** + * How one required check stands on a commit, in the words the ledger records: + * `passed` when every run completed as `success` or `neutral`, `failed` when a run + * completed as anything else, `pending` when a run exists and has not completed, and + * `missing` when no run of that name has a result (none exists, or every one was + * skipped, which means nothing ran). + */ +export type CheckState = "passed" | "failed" | "pending" | "missing"; + +/** One required check as read for a commit, see `checks`. */ +export interface CheckStatus { + /** The name that was asked for. */ + readonly name: string; + readonly state: CheckState; + /** + * What GitHub says about the run that decided `state`: its conclusion when it + * completed, its status when it did not, and empty for a missing check. + */ + readonly conclusion: string; + /** The run's page on GitHub, when it has one. */ + readonly url: string | null; +} + // ---- parsing helpers ---- const isJsonSpace = (char: string): boolean => @@ -339,6 +406,22 @@ const isPlainSegment = (part: string): boolean => const isCommitId = (text: string): boolean => /^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/.test(text); +/** gh's stderr for a pull request number the repo does not have. */ +const PULL_REQUEST_GONE = /could not resolve to a pullrequest/i; + +/** Failed beats pending beats missing beats passed: a check passes only when every run passed. */ +const CHECK_RANK: Record = { failed: 3, pending: 2, missing: 1, passed: 0 }; + +/** The state one check run decides, and the word GitHub gave for it. */ +const checkOutcome = (status: string, conclusion: string | null): readonly [CheckState, string] => { + if (status !== "completed") return ["pending", status]; + if (conclusion === "success" || conclusion === "neutral") return ["passed", conclusion]; + // Nothing ran. The factory's pull request is a draft until the checks pass, and a job + // guarded by "not a draft" is skipped on it. + if (conclusion === "skipped") return ["missing", conclusion]; + return ["failed", conclusion ?? "completed"]; +}; + const withMarker = (body: string, marker: string): string => `${body}\n\n${marker}`; // ---- the client ---- @@ -659,6 +742,159 @@ export const makeGitHub = (options: GitHubOptions) => { return { branch, tip, text }; }); + /** The pull request as `gh pr view` reads it; null for a number the repo does not have. */ + const pullRequest = (number: number): GitHubEffect => + Effect.gen(function* () { + const args = [ + ...repoArgs(["pr", "view", String(number)]), + "--json", + "number,isDraft,state,headRefName,isCrossRepository,baseRefName,headRefOid", + ]; + const stdout = yield* gh(args).pipe( + Effect.catchTags({ + GitHubFailed: (error) => + PULL_REQUEST_GONE.test(error.stderr) ? Effect.succeed(null) : Effect.fail(error), + }), + ); + if (stdout === null) return null; + const raw = yield* decode( + PullRequestView, + args, + "an object describing a pull request", + stdout, + ); + return { + number: raw.number, + isDraft: raw.isDraft, + open: raw.state === "OPEN", + merged: raw.state === "MERGED", + headRef: raw.headRefName, + crossRepository: raw.isCrossRepository, + baseRef: raw.baseRefName, + headSha: raw.headRefOid, + }; + }); + + /** + * Takes the pull request out of draft. One that is already ready counts as done: `gh` + * warns `Pull request # is already "ready for review"` on stderr and exits 0, + * and an answer that was lost on the way back leaves a caller asking again. + */ + const markPrReady = (pr: number): GitHubEffect => + gh(repoArgs(["pr", "ready", String(pr)])).pipe( + Effect.asVoid, + Effect.catchTags({ + GitHubFailed: (error) => + error.stderr.includes('is already "ready for review"') ? Effect.void : Effect.fail(error), + }), + ); + + /** + * Puts the pull request back to draft (`gh pr ready --undo`). One that is a draft already + * counts as done: `gh` warns `Pull request # is already "in draft"` on stderr and + * exits 0, and an answer that was lost on the way back leaves a caller asking again. So + * does one that is no longer open, merged or closed in the meantime: there is nothing to + * draft, and a caller that resumes a decision would otherwise fail on it every time. When + * the call fails for another reason the pull request is read, and its state decides, not + * the words of the error; a read that fails or finds it open leaves the original error. + */ + const markPrDraft = (pr: number): GitHubEffect => + gh([...repoArgs(["pr", "ready", String(pr)]), "--undo"]).pipe( + Effect.asVoid, + Effect.catchTags({ + GitHubFailed: (original) => + original.stderr.includes('is already "in draft"') + ? Effect.void + : pullRequest(pr).pipe( + Effect.orElseSucceed(() => null), + Effect.flatMap((pull) => + pull !== null && !pull.open ? Effect.void : Effect.fail(original), + ), + ), + }), + ); + + /** Comments on the pull request, ending with the factory marker. */ + const commentOnPr = (pr: number, body: string): GitHubEffect => + gh([...repoArgs(["pr", "comment", String(pr)]), "--body", withMarker(body, MARKER)]).pipe( + Effect.asVoid, + ); + + /** + * How `head` stands against `base`, by GitHub's compare API (`status` of `base...head`). + * A ref that GitHub does not know (a 404) is null. Only the status is read, so a long + * history costs one small answer. Each ref goes into the API path the way `branchTip` + * places a branch, and a ref with an empty or dot segment, or `..` anywhere, is refused + * before the call so that it cannot leave the compare path or name a second range. + */ + const compare = (base: string, head: string): GitHubEffect => + Effect.gen(function* () { + const encoded: Array = []; + for (const name of [base, head]) { + const part = name.includes("..") ? null : encodeBranch(name); + if (part === null) { + return yield* new GitHubInvalidArgument({ + argument: name, + expected: "a ref without `..`, empty or dot segments", + }); + } + encoded.push(part); + } + const args = [ + "api", + `repos/${repo}/compare/${encoded[0]}...${encoded[1]}?per_page=1`, + "--jq", + ".status", + ]; + const stdout = yield* gh(args).pipe(Effect.catchIf(isNotFound, () => Effect.succeed(null))); + if (stdout === null) return null; + switch (stdout.trim()) { + case "ahead": + case "behind": + case "identical": + case "diverged": + return stdout.trim() as Relation; + default: + return yield* parseError(args, "ahead, behind, identical or diverged"); + } + }); + + /** + * How each of the check `names` stands on the commit `sha` (a full 40 or 64 digit id, + * refused before the call otherwise): one answer for every name, in the order asked, + * whatever else ran. Only check runs count (what GitHub Actions and other GitHub apps + * report), and only the latest run of each. A name is matched exactly. A skipped run is + * no result. When one name has several runs, a failed one decides, then a pending one, + * then a skipped one, and it passes only when all passed. + */ + const checks = (sha: string, names: ReadonlyArray): GitHubEffect> => + Effect.gen(function* () { + if (!isCommitId(sha)) { + return yield* new GitHubInvalidArgument({ argument: sha, expected: "a full commit id" }); + } + const args = [ + "api", + `repos/${repo}/commits/${sha}/check-runs?per_page=100&filter=latest`, + "--paginate", + "--jq", + "[.check_runs[] | {name, status, conclusion, html_url}]", + ]; + const runs = yield* ghPages(args, CheckRunRows, "JSON arrays of check runs"); + return names.map((name) => { + let found: CheckStatus | null = null; + for (const run of runs) { + if (run.name !== name) continue; + const [state, conclusion] = checkOutcome(run.status, run.conclusion ?? null); + if (found === null || CHECK_RANK[state] > CHECK_RANK[found.state]) { + // A link that does not point at GitHub is not passed on. + const url = run.html_url?.startsWith("https://github.com/") ? run.html_url : null; + found = { name, state, conclusion, url }; + } + } + return found ?? { name, state: "missing", conclusion: "", url: null }; + }); + }); + return { listIssues, issue, @@ -674,5 +910,11 @@ export const makeGitHub = (options: GitHubOptions) => { branchTip, fileAt, baseConfig, + pullRequest, + markPrReady, + markPrDraft, + commentOnPr, + compare, + checks, }; }; From 32b4a18517d5237a7f121dbfc97c929799b2e639 Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Thu, 15 Oct 2026 22:54:35 +1000 Subject: [PATCH 3/6] test(factory): markPrDraft reads the PR state after any failure Red: markPrDraft only reads the pull request after a non-zero exit, so a timed out, rate limited or oversized undo on a closed PR is still an error. softfac's mark_pr_draft falls back on every error. Issue: #14 --- .../server/src/factory/tracker/github.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/apps/server/src/factory/tracker/github.test.ts b/apps/server/src/factory/tracker/github.test.ts index 91b2a3326000..bed431361e10 100644 --- a/apps/server/src/factory/tracker/github.test.ts +++ b/apps/server/src/factory/tracker/github.test.ts @@ -1600,6 +1600,25 @@ it.effect("markPrDraft on a PR that is closed or merged is done whatever gh says }), ); +it.effect("markPrDraft reads the state after any failure, not only a non-zero exit", () => + Effect.gen(function* () { + // A timed out, rate limited or oversized undo on a PR that is closed by now is done too. + for (const first of [ + timedOut(), + fail(1, "gh: API rate limit exceeded (HTTP 403)"), + truncated(), + ]) { + const t = setup(first, reply(prView("CLOSED", false))); + yield* t.ok(t.github.markPrDraft(9)); + assert.deepStrictEqual(t.calls(), [undoCall, prViewCall("9")]); + } + // On one that is still open the original error comes back, whatever kind it was. + const open = setup(timedOut(), reply(prView("OPEN", false))); + const error = yield* open.err(open.github.markPrDraft(9)); + assert.strictEqual(error._tag, "GitHubTimedOut"); + }), +); + it.effect("markPrDraft that fails on an open PR or cannot read it is the original error", () => Effect.gen(function* () { // Open: the failure is real. From 249fae93e20ba69f0fec2bf959d7ebcb6b2c1abe Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Thu, 15 Oct 2026 22:54:50 +1000 Subject: [PATCH 4/6] fix(factory): markPrDraft reads the PR state after any failure A timed out, rate limited or oversized `gh pr ready --undo` on a pull request that is closed or merged by now is done, as in softfac; one that is still open keeps the original error. Issue: #14 --- apps/server/src/factory/tracker/github.ts | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/server/src/factory/tracker/github.ts b/apps/server/src/factory/tracker/github.ts index 407ecaf6a4e4..1d69a7232de8 100644 --- a/apps/server/src/factory/tracker/github.ts +++ b/apps/server/src/factory/tracker/github.ts @@ -795,23 +795,23 @@ export const makeGitHub = (options: GitHubOptions) => { * exits 0, and an answer that was lost on the way back leaves a caller asking again. So * does one that is no longer open, merged or closed in the meantime: there is nothing to * draft, and a caller that resumes a decision would otherwise fail on it every time. When - * the call fails for another reason the pull request is read, and its state decides, not - * the words of the error; a read that fails or finds it open leaves the original error. + * the call fails for any other reason, a non-zero exit, a timeout, a rate limit or too much + * output, the pull request is read, and its state decides, not the words of the error; a + * read that fails or finds it open leaves the original error. */ const markPrDraft = (pr: number): GitHubEffect => gh([...repoArgs(["pr", "ready", String(pr)]), "--undo"]).pipe( Effect.asVoid, - Effect.catchTags({ - GitHubFailed: (original) => - original.stderr.includes('is already "in draft"') - ? Effect.void - : pullRequest(pr).pipe( - Effect.orElseSucceed(() => null), - Effect.flatMap((pull) => - pull !== null && !pull.open ? Effect.void : Effect.fail(original), - ), + Effect.catch((original) => + original._tag === "GitHubFailed" && original.stderr.includes('is already "in draft"') + ? Effect.void + : pullRequest(pr).pipe( + Effect.orElseSucceed(() => null), + Effect.flatMap((pull) => + pull !== null && !pull.open ? Effect.void : Effect.fail(original), ), - }), + ), + ), ); /** Comments on the pull request, ending with the factory marker. */ From 44cd981e314586d49e96b3bc63d3a1987330b42a Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Thu, 15 Oct 2026 22:58:06 +1000 Subject: [PATCH 5/6] test(factory): a check's conclusion and link come from the deciding run Already green: covers what the several-runs case only asserted by state. Issue: #14 --- apps/server/src/factory/tracker/github.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/apps/server/src/factory/tracker/github.test.ts b/apps/server/src/factory/tracker/github.test.ts index bed431361e10..8ac83b9f4389 100644 --- a/apps/server/src/factory/tracker/github.test.ts +++ b/apps/server/src/factory/tracker/github.test.ts @@ -1884,6 +1884,23 @@ it.effect("several runs of one name fail before they pend and pend before they p return (yield* t.ok(t.github.checks(BASE_SHA, ["gate"])))[0]!.state; }); assert.strictEqual(yield* state(["completed", "success"], ["completed", "failure"]), "failed"); + // The conclusion and the link are the deciding run's, whichever came first. + const mixed = setup( + reply( + page([ + run("gate", "completed", "success"), + { ...run("gate", "completed", "failure"), html_url: "https://github.com/x/y/runs/2" }, + ]), + ), + ); + assert.deepStrictEqual(yield* mixed.ok(mixed.github.checks(BASE_SHA, ["gate"])), [ + { + name: "gate", + state: "failed", + conclusion: "failure", + url: "https://github.com/x/y/runs/2", + }, + ]); assert.strictEqual(yield* state(["in_progress", null], ["completed", "success"]), "pending"); assert.strictEqual(yield* state(["completed", "success"], ["completed", "neutral"]), "passed"); // A failure is not hidden by a run that is going again, in either order. From a75706af3f3c01100f60f063e7e43cfcdad5583d Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Fri, 16 Oct 2026 06:54:45 +1000 Subject: [PATCH 6/6] fix(factory): pullRequest refuses a head that is not a full commit id Tests pin the check ranking against a skipped run, the tie between two failed runs, and lookalike GitHub links; the CheckStatus doc says a skipped run keeps its conclusion. --- .../server/src/factory/tracker/github.test.ts | 48 +++++++++++++++++-- apps/server/src/factory/tracker/github.ts | 6 ++- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/apps/server/src/factory/tracker/github.test.ts b/apps/server/src/factory/tracker/github.test.ts index 8ac83b9f4389..cfc845dcf6c5 100644 --- a/apps/server/src/factory/tracker/github.test.ts +++ b/apps/server/src/factory/tracker/github.test.ts @@ -1441,6 +1441,16 @@ it.effect("pullRequest reads the head commit as given", () => }), ); +it.effect("a pullRequest whose head commit is not a full id is a parse error", () => + Effect.gen(function* () { + for (const sha of ["", "4f1c2d3", `${HEAD_SHA}0`, HEAD_SHA.replaceAll("4", "g")]) { + const t = setup(reply(PR_VIEW.replace(HEAD_SHA, sha))); + const error = yield* t.err(t.github.pullRequest(2)); + assert.strictEqual(error._tag, "GitHubParse", sha); + } + }), +); + it.effect("pullRequest merged and closed are not open", () => Effect.gen(function* () { for (const state of ["MERGED", "CLOSED"]) { @@ -1909,6 +1919,32 @@ it.effect("several runs of one name fail before they pend and pend before they p // One skipped run among passing ones is not every run passing. assert.strictEqual(yield* state(["completed", "success"], ["completed", "skipped"]), "missing"); assert.strictEqual(yield* state(["completed", "skipped"], ["completed", "success"]), "missing"); + // A skipped run does not hide one that is still going, or one that failed. + assert.strictEqual(yield* state(["completed", "skipped"], ["in_progress", null]), "pending"); + assert.strictEqual(yield* state(["in_progress", null], ["completed", "skipped"]), "pending"); + assert.strictEqual(yield* state(["completed", "skipped"], ["completed", "failure"]), "failed"); + assert.strictEqual(yield* state(["completed", "failure"], ["completed", "skipped"]), "failed"); + }), +); + +it.effect("of several runs that decide alike, the first one listed is reported", () => + Effect.gen(function* () { + const t = setup( + reply( + page([ + { ...run("gate", "completed", "failure"), html_url: "https://github.com/x/y/runs/1" }, + { ...run("gate", "completed", "cancelled"), html_url: "https://github.com/x/y/runs/2" }, + ]), + ), + ); + assert.deepStrictEqual(yield* t.ok(t.github.checks(BASE_SHA, ["gate"])), [ + { + name: "gate", + state: "failed", + conclusion: "failure", + url: "https://github.com/x/y/runs/1", + }, + ]); }), ); @@ -1927,10 +1963,14 @@ it.effect("checks reads every page", () => it.effect("checks keeps a details link only when it is a GitHub one", () => Effect.gen(function* () { - const odd = setup( - reply(page([{ ...run("gate", "completed", "failure"), html_url: "https://evil.example/x" }])), - ); - assert.isNull((yield* odd.ok(odd.github.checks(BASE_SHA, ["gate"])))[0]!.url); + for (const url of [ + "https://evil.example/x", + "https://github.zerozr99.workers.dev.evil.example/x", + "http://github.com/x", + ]) { + const odd = setup(reply(page([{ ...run("gate", "completed", "failure"), html_url: url }]))); + assert.isNull((yield* odd.ok(odd.github.checks(BASE_SHA, ["gate"])))[0]!.url, url); + } const none = setup(reply(page([{ ...run("gate", "completed", "failure"), html_url: null }]))); assert.isNull((yield* none.ok(none.github.checks(BASE_SHA, ["gate"])))[0]!.url); }), diff --git a/apps/server/src/factory/tracker/github.ts b/apps/server/src/factory/tracker/github.ts index 1d69a7232de8..5d2371701f8a 100644 --- a/apps/server/src/factory/tracker/github.ts +++ b/apps/server/src/factory/tracker/github.ts @@ -291,7 +291,8 @@ export interface CheckStatus { readonly state: CheckState; /** * What GitHub says about the run that decided `state`: its conclusion when it - * completed, its status when it did not, and empty for a missing check. + * completed (`skipped` for a check that is missing because its run was skipped), its + * status when it did not, and empty for a check with no run at all. */ readonly conclusion: string; /** The run's page on GitHub, when it has one. */ @@ -763,6 +764,9 @@ export const makeGitHub = (options: GitHubOptions) => { "an object describing a pull request", stdout, ); + if (!isCommitId(raw.headRefOid)) { + return yield* parseError(args, "a pull request and its full head commit id"); + } return { number: raw.number, isDraft: raw.isDraft,