From 9742513fc9c9f4f53b87609c45977273a81af4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alfonso=20Andr=C3=A9s=20L=C3=B3pez=20Molina?= Date: Thu, 12 Nov 2026 12:00:07 -0500 Subject: [PATCH 1/2] fix(server): retain consumed immutable check failures --- .../src/orchestration-v2/Orchestrator.ts | 3 ++ .../orchestration-v2/pullRequestWatch.test.ts | 41 +++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 25 ++++++++++- .../src/orchestration-v2/runtimeLayer.test.ts | 23 ++++++++++- packages/contracts/src/threadPullRequest.ts | 2 + 5 files changed, 92 insertions(+), 2 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index c186c40f9e73..deae6515c2b6 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4695,6 +4695,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...(existing.endedWatch && { headSha: existing.endedWatch.headSha, failedChecks: existing.endedWatch.failedChecks, + ...(existing.endedWatch.reportedFailures !== undefined && { + reportedFailures: existing.endedWatch.reportedFailures, + }), passed: existing.endedWatch.passed, passedChecks: existing.endedWatch.passedChecks, conflicting: existing.endedWatch.conflicting, diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index dece3e344983..8f3698a9dc3a 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -234,6 +234,47 @@ describe("evaluatePullRequestWatch", () => { { kind: "checks-failed", failed: [check("lint", "failure")] }, { kind: "checks-awaiting-approval", checks: [approval] }, ]); + + // A job URL identifies its run and immutable job attempt. A transient missing + // or pending row must not make the same consumed failure news again. + const job = (runId: number, jobId: number) => ({ + ...check("lint", "failure"), + url: `${run(runId)}/job/${jobId}`, + }); + const seen = evaluatePullRequestWatch( + watch(), + detail({ checks: [job(10, 20), bot] }), + noRemarks, + ); + const pending = evaluatePullRequestWatch( + seen.next, + detail({ checks: [check("lint", "pending"), bot] }), + noRemarks, + ); + assert.deepEqual( + evaluatePullRequestWatch(pending.next, detail({ checks: [job(10, 20), bot] }), noRemarks) + .changes, + [], + ); + assert.deepEqual( + evaluatePullRequestWatch( + seen.next, + detail({ + checks: [{ ...job(10, 20), url: `${run(10)}/job/20?check_suite_focus=true#step:1` }, bot], + }), + noRemarks, + ).changes, + [], + ); + for (const fresh of [job(10, 21), job(11, 22)]) { + const next = evaluatePullRequestWatch(seen.next, detail({ checks: [fresh, bot] }), noRemarks); + assert.deepEqual(next.changes, [{ kind: "checks-failed", failed: [fresh] }]); + assert.deepEqual( + evaluatePullRequestWatch(next.next, detail({ checks: [job(10, 20), bot] }), noRemarks) + .changes, + [], + ); + } }); it("reports passed once the required checks pass, whatever the others do", () => { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 05b08df73fb4..87a421de40d9 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -35,6 +35,16 @@ const isFailedCheck = (check: PullRequestCheck) => const workflowRunOf = (url: string | null) => url?.match(/^.*\/actions\/runs\/\d+/)?.[0] ?? null; const approvalKey = (check: PullRequestCheck) => workflowRunOf(check.url) ?? check.name; +function immutableFailureKey(head: string | null, check: PullRequestCheck): string | undefined { + // GitHub allocates a new job id for every run attempt. URLs without that + // immutable identity retain the existing transition-based reporting policy. + const job = check.url?.match( + /^(https:\/\/[^/]+\/[^/]+\/[^/]+\/actions\/runs\/\d+(?:\/attempts\/\d+)?\/job\/\d+)(?:$|[?#])/, + )?.[1]; + if (head === null || job === undefined) return; + return JSON.stringify([head, job.toLowerCase(), check.status]); +} + export function evaluatePullRequestWatch( watch: ThreadPullRequestWatch, detail: Pick< @@ -51,6 +61,7 @@ export function evaluatePullRequestWatch( // Hosts can return no checks on read failure; preserve prior state. let failedChecks = headMoved ? [] : watch.failedChecks; + const reportedFailures = new Set(watch.reportedFailures ?? []); let passed = headMoved ? false : watch.passed; let passedChecks = headMoved ? [] : watch.passedChecks; let owedChecks = headMoved ? [] : (watch.owedChecks ?? []); @@ -62,7 +73,16 @@ export function evaluatePullRequestWatch( const failed = checks.filter(isFailedCheck); const awaiting = checks.filter((check) => check.status === "action-required"); if (checks.length > 0) { - const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); + const newlyFailed = failed.filter((check) => { + const key = immutableFailureKey(headSha, check); + return key !== undefined && watch.reportedFailures !== undefined + ? !reportedFailures.has(key) + : !failedChecks.includes(check.name); + }); + for (const check of failed) { + const key = immutableFailureKey(headSha, check); + if (key !== undefined) reportedFailures.add(key); + } if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); if (!approvalAnnounced && awaiting.length > 0) { changes.push({ kind: "checks-awaiting-approval", checks: awaiting }); @@ -195,6 +215,9 @@ export function evaluatePullRequestWatch( startedAt: watch.startedAt, headSha, failedChecks, + ...(reportedFailures.size > 0 || watch.reportedFailures !== undefined + ? { reportedFailures: [...reportedFailures] } + : {}), passed, passedChecks, owedChecks, diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 1557e411af93..ffbb50143cd4 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -3571,7 +3571,13 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }); assert.deepEqual(yield* watchOf, started); - const recorded = { ...started, headSha: "abc123", failedChecks: ["lint"], wakes: 1 }; + const recorded = { + ...started, + headSha: "abc123", + failedChecks: ["lint"], + reportedFailures: ['["abc123","https://github.com/o/r/actions/runs/1/job/2","failure"]'], + wakes: 1, + }; yield* orchestrator.dispatch({ type: "thread.pull-request-watch.sync", commandId: CommandId.make("pr-watch-record"), @@ -3610,6 +3616,21 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { assert.isUndefined(yield* watchOf); const { messages } = yield* orchestrator.getThreadRecords(threadId, ["messages"]); assert.deepEqual(messages, []); + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-restart-history"), + threadId, + ...key, + watching: true, + }); + assert.deepEqual((yield* watchOf)?.reportedFailures, recorded.reportedFailures); + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-restart-stop"), + threadId, + ...key, + watching: false, + }); }), ); diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index bd0a65a3c0bc..66effe9c96f0 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -83,6 +83,8 @@ export const ThreadPullRequestWatch = Schema.Struct({ startedAt: IsoDateTime, headSha: Schema.NullOr(TrimmedNonEmptyString), failedChecks: Schema.Array(TrimmedNonEmptyString), + /** Consumed immutable job failures, retained across transient reads and rewatching. */ + reportedFailures: Schema.optional(Schema.Array(TrimmedNonEmptyString)), passed: Schema.Boolean, passedChecks: Schema.Array(TrimmedNonEmptyString).pipe( Schema.withDecodingDefault(Effect.succeed([])), From cf84ba22cff05dad06b0b698cf5d7689961f3ad4 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 15 Nov 2026 04:28:22 +0000 Subject: [PATCH 2/2] fix(server): persist migrated check failure history --- .../PullRequestWatchReactor.ts | 1 + .../src/orchestration-v2/runtimeLayer.test.ts | 35 ++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 5b69a7efa337..ffa62c3c0c4e 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -205,6 +205,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.startedAt === right.startedAt && left.headSha === right.headSha && left.failedChecks.join("\n") === right.failedChecks.join("\n") && + (left.reportedFailures ?? []).join("\n") === (right.reportedFailures ?? []).join("\n") && left.passed === right.passed && left.passedChecks.join("\n") === right.passedChecks.join("\n") && (left.owedChecks ?? []).join("\n") === (right.owedChecks ?? []).join("\n") && diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 0825ed4577ec..fba33bb4251f 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -4642,7 +4642,7 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }); yield* watch(true, "start"); - let remainingStatus: "pending" | "action-required" | "success" = initialStatus; + let remainingStatus: "pending" | "action-required" | "success" | "failure" = initialStatus; const approval = initialStatus === "action-required"; const runUrl = "https://github.com/pingdotgg/t3code/actions/runs/123"; const host = Layer.mergeAll( @@ -4731,6 +4731,39 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch?.owedChecks, [], ); + remainingStatus = "failure"; + yield* TestClock.adjust("10 minutes"); + yield* restarted.sweep; + yield* worker.drain(); + const failedWatch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; + assert.isDefined(failedWatch); + if (failedWatch === undefined) return; + yield* orchestrator.dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make(`pr-watch-owed-legacy-${initialStatus}`), + threadId, + host: "github.com", + repository: "pingdotgg/t3code", + number: 12, + startedAt: failedWatch.startedAt, + watch: { ...failedWatch, reportedFailures: undefined }, + }); + yield* TestClock.adjust("10 minutes"); + yield* restarted.sweep; + yield* worker.drain(); + assert.deepEqual( + (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch?.reportedFailures, + failedWatch.reportedFailures, + ); + remainingStatus = "pending"; + yield* TestClock.adjust("10 minutes"); + yield* restarted.sweep; + yield* worker.drain(); + remainingStatus = "failure"; + yield* TestClock.adjust("10 minutes"); + yield* restarted.sweep; + yield* worker.drain(); + assert.deepEqual(yield* summaries, ["#12: checks passed", "#12: checks failed"]); yield* watch(false, "end"); }), );