diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts index e329ade90d9e..641941146f69 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts @@ -248,6 +248,7 @@ describe("pull request toolkit handlers", () => { headSha: null, failedChecks: [], passed: false, + passedChecks: [], remarksThrough: "2026-08-20T00:00:00.000Z", remarkIds: [], conflicting: false, diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index abdfc29579cd..699a45d303c0 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -2998,6 +2998,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio headSha: null, failedChecks: [], passed: false, + passedChecks: [], remarksThrough: startedAt, remarkIds: [], conflicting: false, diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 6e0046c5e59a..739bec5721ae 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -54,6 +54,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.headSha === right.headSha && left.failedChecks.join("\n") === right.failedChecks.join("\n") && left.passed === right.passed && + left.passedChecks.join("\n") === right.passedChecks.join("\n") && left.remarksThrough === right.remarksThrough && left.remarkIds.join("\n") === right.remarkIds.join("\n") && left.conflicting === right.conflicting && diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index c247d78fd332..75480a29a532 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -19,6 +19,7 @@ const watch = (overrides: Partial = {}): ThreadPullReque headSha: null, failedChecks: [], passed: false, + passedChecks: [], remarksThrough: STARTED, remarkIds: [], conflicting: false, @@ -100,6 +101,61 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(watch(), plain, noRemarks).changes, []); }); + it("reports passed again when a required check shows up already passed", () => { + const required = (name: string, status: PullRequestCheck["status"]) => ({ + ...check(name, status), + required: true, + }); + const tests = detail({ + checks: [required("Tests", "success"), check("Smoke Tests", "pending")], + }); + const first = evaluatePullRequestWatch(watch(), tests, noRemarks); + assert.deepEqual(first.changes, [{ kind: "checks-passed", count: 1, required: true }]); + + // The gate job was created and finished between two passes, so it was never seen pending. + const gated = detail({ + checks: [ + required("Tests", "success"), + check("Smoke Tests", "success"), + required("Smoke Tests Gate", "success"), + ], + }); + const second = evaluatePullRequestWatch(first.next, gated, noRemarks); + assert.deepEqual(second.changes, [{ kind: "checks-passed", count: 2, required: true }]); + assert.deepEqual(evaluatePullRequestWatch(second.next, gated, noRemarks).changes, []); + + // Seen pending first, the gate is reported once it passes, as before. + const pending = detail({ + checks: [required("Tests", "success"), required("Smoke Tests Gate", "pending")], + }); + const waiting = evaluatePullRequestWatch(first.next, pending, noRemarks); + assert.deepEqual(waiting.changes, []); + assert.deepEqual(evaluatePullRequestWatch(waiting.next, gated, noRemarks).changes, [ + { kind: "checks-passed", count: 2, required: true }, + ]); + }); + + it("reports passed again when a check shows up already passed where none is required", () => { + const first = evaluatePullRequestWatch( + watch(), + detail({ checks: [check("test", "success")] }), + noRemarks, + ); + assert.deepEqual(first.changes, [{ kind: "checks-passed", count: 1, required: false }]); + const both = detail({ checks: [check("test", "success"), check("lint", "success")] }); + assert.deepEqual(evaluatePullRequestWatch(first.next, both, noRemarks).changes, [ + { kind: "checks-passed", count: 2, required: false }, + ]); + }); + + it("does not wake a watch saved before passed checks were recorded", () => { + const green = detail({ checks: [{ ...check("test", "success"), required: true }] }); + const told = watch({ headSha: "aaaaaaaaaa", passed: true }); + const saved = evaluatePullRequestWatch(told, green, noRemarks); + assert.deepEqual(saved.changes, []); + assert.deepEqual(saved.next.passedChecks, ["test"]); + }); + it("keeps remarks for a later pass when the conversation was not read whole", () => { const comments = [remark("reviewer", "2026-10-02T12:06:00Z")]; const partial = evaluatePullRequestWatch(watch(), detail(), null); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index af9ce27636ee..780557db0b57 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -53,6 +53,7 @@ export function evaluatePullRequestWatch( // An empty list keeps the last state: a host can answer with one when its check read fails. let failedChecks = headMoved ? [] : watch.failedChecks; let passed = headMoved ? false : watch.passed; + let passedChecks = headMoved ? [] : watch.passedChecks; if (detail.checks.length > 0) { const failed = detail.checks.filter(isFailedCheck); const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); @@ -63,10 +64,16 @@ export function evaluatePullRequestWatch( const required = detail.checks.filter((check) => check.required === true); const gate = required.length > 0 ? required : detail.checks; const passedNow = gate.every((check) => check.status !== "pending" && !isFailedCheck(check)); - if (passedNow && !passed) { + const gateNames = gate.map((check) => check.name); + // A watch saved before passedChecks existed takes the current names, so it does not wake. + const told = passed && passedChecks.length === 0 ? gateNames : passedChecks; + // A required job created and finished between two passes is never seen pending. + const gateGrew = gateNames.some((name) => !told.includes(name)); + if (passedNow && (!passed || gateGrew)) { changes.push({ kind: "checks-passed", count: gate.length, required: required.length > 0 }); } passed = passedNow; + passedChecks = passedNow ? gateNames : []; } const own = (detail.viewer ?? detail.author?.login)?.toLowerCase(); @@ -105,6 +112,7 @@ export function evaluatePullRequestWatch( headSha, failedChecks, passed, + passedChecks, remarksThrough, remarkIds, conflicting, diff --git a/packages/contracts/src/threadPullRequest.test.ts b/packages/contracts/src/threadPullRequest.test.ts index 456761c4ebc8..108902bc59a9 100644 --- a/packages/contracts/src/threadPullRequest.test.ts +++ b/packages/contracts/src/threadPullRequest.test.ts @@ -54,3 +54,32 @@ it.effect("decodes thread pull request links with snapshot and stack", () => assert.strictEqual(links[1]?.snapshot?.state, "open"); }), ); + +it.effect("decodes a watch saved before passed checks were recorded", () => + Effect.gen(function* () { + const [link] = yield* decodeLinks([ + { + host: "github.com", + repository: "pingdotgg/t3code", + number: 42, + url: "https://github.com/pingdotgg/t3code/pull/42", + source: "agent", + linkedAt: "2026-01-01T00:00:00.000Z", + snapshot: null, + stack: null, + watch: { + startedAt: "2026-01-01T00:00:00.000Z", + headSha: "abc123", + failedChecks: [], + passed: true, + remarksThrough: "2026-01-01T00:00:00.000Z", + remarkIds: [], + conflicting: false, + wakes: 0, + }, + }, + ]); + + assert.deepStrictEqual(link?.watch?.passedChecks, []); + }), +); diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 9df5cbe9670a..2dfb532c7f2f 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -1,3 +1,4 @@ +import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import { @@ -106,6 +107,13 @@ export const ThreadPullRequestWatch = Schema.Struct({ failedChecks: Schema.Array(TrimmedNonEmptyString), /** The agent was told the required checks on that commit passed. */ passed: Schema.Boolean, + /** + * Names in the passing gate on that commit, so a required check that first shows up already + * passed is news. Empty on watches saved before this field existed. + */ + passedChecks: Schema.Array(TrimmedNonEmptyString).pipe( + Schema.withDecodingDefault(Effect.succeed([])), + ), /** Remarks from others created up to this host time were reported. */ remarksThrough: IsoDateTime, /** Remarks created exactly at `remarksThrough` that were reported, so a late one still counts. */