From ab7e0235783939eee1bfc8130b86a3a07bb2e915 Mon Sep 17 00:00:00 2001 From: Mnigos Date: Wed, 7 Oct 2026 03:20:43 +0200 Subject: [PATCH 1/3] fix(server): Forgejo skipped jobs no longer keep a pull request watch pending Forgejo reports a job that did not run (for example a deploy job gated to the default branch) as a `skipped` commit status. Every status other than success, failure and error was read as pending, so a skipped job stayed pending forever, showed as running, and the watch never reported that checks passed. Forgejo statuses now map like the other hosts: skipped is skipped, warning is neutral, and only states we do not recognise stay pending. --- .../orchestration-v2/pullRequestWatch.test.ts | 7 ++++ .../forgejoPullRequestJson.test.ts | 37 +++++++++++++++++++ .../src/pullRequest/forgejoPullRequestJson.ts | 26 ++++++++++--- 3 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 apps/server/src/pullRequest/forgejoPullRequestJson.test.ts diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index ddd329d41173..7f9cf85e8ea3 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -100,6 +100,13 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(watch(), plain, noRemarks).changes, []); }); + it("reports passed when a check was skipped and the rest succeeded", () => { + const green = detail({ checks: [check("checks", "success"), check("deploy", "skipped")] }); + assert.deepEqual(evaluatePullRequestWatch(watch(), green, noRemarks).changes, [ + { kind: "checks-passed", count: 2, required: false }, + ]); + }); + it("reports passed again when a required check shows up already passed", () => { const required = (name: string, status: PullRequestCheck["status"]) => ({ ...check(name, status), diff --git a/apps/server/src/pullRequest/forgejoPullRequestJson.test.ts b/apps/server/src/pullRequest/forgejoPullRequestJson.test.ts new file mode 100644 index 000000000000..e845afe196e4 --- /dev/null +++ b/apps/server/src/pullRequest/forgejoPullRequestJson.test.ts @@ -0,0 +1,37 @@ +import { assert, describe, it } from "@effect/vitest"; + +import { forgejoChecks } from "./forgejoPullRequestJson.ts"; + +const status = (context: string, state: string) => ({ + context, + status: state, + description: null, + target_url: null, + updated_at: "2026-10-05T12:00:00Z", +}); + +describe("forgejoChecks", () => { + it("maps every Forgejo status state onto a check status, and unknown ones to pending", () => { + const checks = forgejoChecks([ + status("CI / checks (pull_request)", "success"), + status("CI / e2e (pull_request)", "failure"), + status("CI / lint (pull_request)", "error"), + status("CI / deploy (pull_request)", "skipped"), + status("CI / audit (pull_request)", "warning"), + status("CI / build (pull_request)", "pending"), + status("CI / docs (pull_request)", "future-state"), + ]); + assert.deepEqual( + checks.map((check) => [check.name, check.status]), + [ + ["CI / checks (pull_request)", "success"], + ["CI / e2e (pull_request)", "failure"], + ["CI / lint (pull_request)", "failure"], + ["CI / deploy (pull_request)", "skipped"], + ["CI / audit (pull_request)", "neutral"], + ["CI / build (pull_request)", "pending"], + ["CI / docs (pull_request)", "pending"], + ], + ); + }); +}); diff --git a/apps/server/src/pullRequest/forgejoPullRequestJson.ts b/apps/server/src/pullRequest/forgejoPullRequestJson.ts index cade3b65d9c8..2be944709e84 100644 --- a/apps/server/src/pullRequest/forgejoPullRequestJson.ts +++ b/apps/server/src/pullRequest/forgejoPullRequestJson.ts @@ -4,6 +4,7 @@ import * as Option from "effect/Option"; import type { PullRequestActor, PullRequestCheck, + PullRequestCheckStatus, PullRequestComment, PullRequestCommit, PullRequestReaction, @@ -208,6 +209,24 @@ export function forgejoCommit(commit: typeof ForgejoCommit.Type): PullRequestCom }; } +// Forgejo's commit status states; Actions reports a job that did not run as `skipped` and a +// cancelled job as `failure`. A skipped job is done, so it must not hold back "checks passed". +function forgejoCheckStatus(state: string): PullRequestCheckStatus { + switch (state) { + case "success": + return "success"; + case "failure": + case "error": + return "failure"; + case "skipped": + return "skipped"; + case "warning": + return "neutral"; + default: + return "pending"; + } +} + export function forgejoChecks( statuses: ReadonlyArray, ): ReadonlyArray { @@ -219,12 +238,7 @@ export function forgejoChecks( name: status.context || "check", description: status.description || null, url: status.target_url || null, - status: - status.status === "success" - ? "success" - : status.status === "failure" || status.status === "error" - ? "failure" - : "pending", + status: forgejoCheckStatus(status.status), }, })), ); From 291c6accecf5efefc66834fa94aaa1464d646782 Mon Sep 17 00:00:00 2001 From: Mnigos Date: Sun, 18 Oct 2026 21:38:25 +0200 Subject: [PATCH 2/3] test(forgejo): pass the pull request URL that forgejoChecks now requires --- .../src/server/forgejoPullRequestJson.test.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts b/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts index e845afe196e4..4c41dda71a13 100644 --- a/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts +++ b/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts @@ -12,15 +12,18 @@ const status = (context: string, state: string) => ({ describe("forgejoChecks", () => { it("maps every Forgejo status state onto a check status, and unknown ones to pending", () => { - const checks = forgejoChecks([ - status("CI / checks (pull_request)", "success"), - status("CI / e2e (pull_request)", "failure"), - status("CI / lint (pull_request)", "error"), - status("CI / deploy (pull_request)", "skipped"), - status("CI / audit (pull_request)", "warning"), - status("CI / build (pull_request)", "pending"), - status("CI / docs (pull_request)", "future-state"), - ]); + const checks = forgejoChecks( + [ + status("CI / checks (pull_request)", "success"), + status("CI / e2e (pull_request)", "failure"), + status("CI / lint (pull_request)", "error"), + status("CI / deploy (pull_request)", "skipped"), + status("CI / audit (pull_request)", "warning"), + status("CI / build (pull_request)", "pending"), + status("CI / docs (pull_request)", "future-state"), + ], + "https://forgejo.test/acme/web/pulls/1", + ); assert.deepEqual( checks.map((check) => [check.name, check.status]), [ From 36485573cd74dbd4abec3c57a9eb6531eda63c70 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:11:35 -0700 Subject: [PATCH 3/3] fix(forgejo): finished statuses no longer keep a pull request watch pending Forgejo reports a job that did not run as `skipped` and a non-passing finished job as `warning`. The adapter mapped both to pending, so a gated deploy job kept a pull request watch open forever. Map skipped to skipped, warning to neutral, and leave only pending and unknown states pending, as the GitLab adapter already does. Replaces the per-state resolver test and the already-passing watch assertion with one provider-level test through getChangeRequestChecks. Co-Authored-By: Claude Fable 5.1 --- .../orchestration-v2/pullRequestWatch.test.ts | 7 --- .../ForgejoSourceControlProvider.test.ts | 61 +++++++++++++++++++ .../src/server/forgejoPullRequestJson.test.ts | 40 ------------ .../src/server/forgejoPullRequestJson.ts | 5 +- 4 files changed, 64 insertions(+), 49 deletions(-) delete mode 100644 packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 57af5fdfd295..faf8a05b9ef2 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -96,13 +96,6 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(watch(), plain, noRemarks).changes, []); }); - it("reports passed when a check was skipped and the rest succeeded", () => { - const green = detail({ checks: [check("checks", "success"), check("deploy", "skipped")] }); - assert.deepEqual(evaluatePullRequestWatch(watch(), green, noRemarks).changes, [ - { kind: "checks-passed", count: 2, required: false }, - ]); - }); - it("reports passed again when a required check shows up already passed", () => { const required = (name: string, status: PullRequestCheck["status"]) => ({ ...check(name, status), diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index 354edbb981e4..056292d66409 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -164,6 +164,67 @@ it.effect("reads the newest Forgejo checks without repository or viewer requests ); }); +it.effect("reads Forgejo jobs that did not run as finished checks", () => + Effect.gen(function* () { + const provider = yield* ForgejoPullRequestProvider.make; + const read = provider.getChangeRequestChecks; + if (read === undefined) return yield* Effect.die("checks read missing"); + const result = yield* read({ + cwd: "/repo", + repository: "acme/web", + host: "forgejo.test", + number: 1, + }); + assert.deepStrictEqual( + result.checks.map((check) => [check.name, check.status]), + [ + ["deploy", "skipped"], + ["audit", "neutral"], + ["docs", "pending"], + ], + ); + }).pipe( + Effect.provide( + Layer.mock(ForgejoCli.ForgejoCli)({ + api: (input) => { + const status = (context: string, state: string) => ({ + context, + status: state, + description: null, + target_url: null, + updated_at: "2026-10-11T00:00:00Z", + }); + const body = input.path.endsWith("pulls/1") + ? { + number: 1, + title: "Checks", + body: "", + html_url: "https://forgejo.test/acme/web/pulls/1", + user: null, + state: "open", + merged: false, + head: { ref: "feature", sha: "head", repo: null }, + base: { ref: "main", sha: "base", repo: null }, + created_at: "2026-10-11T00:00:00Z", + updated_at: "2026-10-11T00:00:00Z", + closed_at: null, + merged_at: null, + labels: [], + } + : input.path.endsWith("page=1") + ? [ + status("deploy", "skipped"), + status("audit", "warning"), + status("docs", "future-state"), + ] + : []; + return encodeJsonEffect(body).pipe(Effect.orDie, Effect.map(processOutput)); + }, + }), + ), + ), +); + it.effect("opens Forgejo Actions check links on the pull request's server", () => Effect.gen(function* () { const provider = yield* ForgejoPullRequestProvider.make; diff --git a/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts b/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts deleted file mode 100644 index 4c41dda71a13..000000000000 --- a/packages/source-control-forgejo/src/server/forgejoPullRequestJson.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { assert, describe, it } from "@effect/vitest"; - -import { forgejoChecks } from "./forgejoPullRequestJson.ts"; - -const status = (context: string, state: string) => ({ - context, - status: state, - description: null, - target_url: null, - updated_at: "2026-10-05T12:00:00Z", -}); - -describe("forgejoChecks", () => { - it("maps every Forgejo status state onto a check status, and unknown ones to pending", () => { - const checks = forgejoChecks( - [ - status("CI / checks (pull_request)", "success"), - status("CI / e2e (pull_request)", "failure"), - status("CI / lint (pull_request)", "error"), - status("CI / deploy (pull_request)", "skipped"), - status("CI / audit (pull_request)", "warning"), - status("CI / build (pull_request)", "pending"), - status("CI / docs (pull_request)", "future-state"), - ], - "https://forgejo.test/acme/web/pulls/1", - ); - assert.deepEqual( - checks.map((check) => [check.name, check.status]), - [ - ["CI / checks (pull_request)", "success"], - ["CI / e2e (pull_request)", "failure"], - ["CI / lint (pull_request)", "failure"], - ["CI / deploy (pull_request)", "skipped"], - ["CI / audit (pull_request)", "neutral"], - ["CI / build (pull_request)", "pending"], - ["CI / docs (pull_request)", "pending"], - ], - ); - }); -}); diff --git a/packages/source-control-forgejo/src/server/forgejoPullRequestJson.ts b/packages/source-control-forgejo/src/server/forgejoPullRequestJson.ts index 7f5952e5c7de..70fc652704e7 100644 --- a/packages/source-control-forgejo/src/server/forgejoPullRequestJson.ts +++ b/packages/source-control-forgejo/src/server/forgejoPullRequestJson.ts @@ -213,8 +213,9 @@ export function forgejoCommit(commit: typeof ForgejoCommit.Type): PullRequestCom }; } -// Forgejo's commit status states; Actions reports a job that did not run as `skipped` and a -// cancelled job as `failure`. A skipped job is done, so it must not hold back "checks passed". +// Forgejo Actions reports a job that did not run as `skipped` and a cancelled one as `failure`, +// and `warning` is finished without being a pass. Only `pending` and states we do not know may +// hold a watch open. function forgejoCheckStatus(state: string): PullRequestCheckStatus { switch (state) { case "success":