From 2b7c51aaaac4ad0afea75e7fcbe0ebe4a173b86e Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 22:34:40 -0700 Subject: [PATCH 01/13] feat(server): agents can watch a pull request and get woken on changes Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/git/GitOverviewSheet.tsx | 45 +++- .../threads/git/gitSheetComponents.tsx | 2 + .../src/features/threads/thread-work-log.tsx | 2 + .../src/environment/ServerEnvironment.ts | 1 + .../toolkits/pullRequests/handlers.test.ts | 49 +++++ .../src/mcp/toolkits/pullRequests/handlers.ts | 75 ++++++- .../src/mcp/toolkits/pullRequests/tools.ts | 62 ++++++ .../src/orchestration-v2/Orchestrator.ts | 61 +++++- .../ProjectionSettlement.test.ts | 30 +++ .../src/orchestration-v2/ProjectionStore.ts | 60 ++++- .../ProviderTurnControlService.test.ts | 1 + .../PullRequestWatchReactor.ts | 185 ++++++++++++++++ .../orchestration-v2/pullRequestWatch.test.ts | 154 +++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 189 ++++++++++++++++ .../src/orchestration-v2/runtimeLayer.test.ts | 207 ++++++++++++++++++ .../testkit/OrchestratorScenario.ts | 1 + .../src/provider/RuntimeInstructions.ts | 2 +- .../src/pullRequest/PullRequestProvider.ts | 2 + .../src/pullRequest/PullRequestService.ts | 1 + apps/server/src/server.ts | 11 + .../src/components/chat/MessagesTimeline.tsx | 2 + .../pullRequest/ThreadPullRequestsPanel.tsx | 51 ++++- docs/user/source-control.md | 7 + .../client-runtime/src/operations/commands.ts | 14 ++ .../src/state/threadCommands.ts | 9 + packages/client-runtime/src/t3ToolSummary.ts | 10 + .../src/work-log/presentation.ts | 10 +- packages/contracts/src/environment.ts | 2 + packages/contracts/src/orchestrationV2.ts | 17 ++ packages/contracts/src/pullRequest.ts | 2 + packages/contracts/src/threadPullRequest.ts | 20 ++ packages/shared/src/t3McpToolPresentation.ts | 12 + 32 files changed, 1274 insertions(+), 22 deletions(-) create mode 100644 apps/server/src/orchestration-v2/PullRequestWatchReactor.ts create mode 100644 apps/server/src/orchestration-v2/pullRequestWatch.test.ts create mode 100644 apps/server/src/orchestration-v2/pullRequestWatch.ts diff --git a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx index 17b1b3ea377c..78e69e57531c 100644 --- a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx +++ b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx @@ -8,7 +8,7 @@ import { resolveThreadPullRequestChains, threadPullRequestKeyOf, } from "@t3tools/shared/threadPullRequests"; -import { EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { EnvironmentId, ThreadId, type ThreadPullRequestLink } from "@t3tools/contracts"; import { CommonActions, StackActions, @@ -40,6 +40,8 @@ import { useThreadSelection } from "../../../state/use-thread-selection"; import { useSelectedThreadGitActions } from "../../../state/use-selected-thread-git-actions"; import { useSelectedThreadGitState } from "../../../state/use-selected-thread-git-state"; import { useSelectedThreadWorktree } from "../../../state/use-selected-thread-worktree"; +import { threadEnvironment } from "../../../state/threads"; +import { useAtomCommand } from "../../../state/use-atom-command"; import { vcsEnvironment } from "../../../state/vcs"; import { resolveGitOverviewReviewNavigationAction } from "./git-overview-navigation"; import { MetaCard, SheetListRow, menuItemIconName, statusSummary } from "./gitSheetComponents"; @@ -73,6 +75,40 @@ export function GitOverviewSheet(props: GitOverviewSheetProps) { ), [selectedThread?.pullRequests, supportsLinkedPrSnapshots], ); + const supportsWatch = + selectedEnvironmentRuntime?.serverConfig?.environment.capabilities.threadPullRequestWatch === + true; + const watchPullRequest = useAtomCommand(threadEnvironment.watchPullRequest); + // Long-press on a linked pull request starts or stops waking the agent on its changes. + const confirmWatchToggle = useCallback( + (link: ThreadPullRequestLink) => { + const watching = link.watch !== undefined; + Alert.alert( + watching ? `Stop watching #${link.number}?` : `Watch #${link.number}?`, + watching + ? "The agent will no longer wake for this pull request." + : "The agent wakes when checks finish, someone comments, or the branch conflicts.", + [ + { text: "Cancel", style: "cancel" }, + { + text: watching ? "Stop watching" : "Watch", + onPress: () => + void watchPullRequest({ + environmentId, + input: { + threadId, + host: link.host, + repository: link.repository, + number: link.number, + watching: !watching, + }, + }), + }, + ], + ); + }, + [environmentId, threadId, watchPullRequest], + ); const gitState = useSelectedThreadGitState(); const gitActions = useSelectedThreadGitActions(); const theme = useUniwindTheme(); @@ -339,7 +375,12 @@ export function GitOverviewSheet(props: GitOverviewSheetProps) { confirmWatchToggle(link) + : undefined + } onPress={() => { void tryOpenExternalUrl(link.url, "pull-request").then((opened) => { if (!opened) diff --git a/apps/mobile/src/features/threads/git/gitSheetComponents.tsx b/apps/mobile/src/features/threads/git/gitSheetComponents.tsx index c358466e22d2..3dd43e1d07d5 100644 --- a/apps/mobile/src/features/threads/git/gitSheetComponents.tsx +++ b/apps/mobile/src/features/threads/git/gitSheetComponents.tsx @@ -88,12 +88,14 @@ export function SheetListRow(props: { readonly subtitle?: string | null; readonly disabled?: boolean; readonly onPress: () => void; + readonly onLongPress?: (() => void) | undefined; }) { return ( { }), ); + it.effect("watching an unlinked pull request links it first", () => + Effect.gen(function* () { + const harness = yield* makeHarness(); + const result = yield* harness.call("watch_pull_request", { + url: "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/t3tools/t3code/pull/9", + }); + expect(result).toMatchObject({ number: 9, watching: true, wasWatching: false }); + expect(yield* Ref.get(harness.commands)).toMatchObject([ + { type: "thread.pull-request.link", number: 9, source: "agent" }, + { type: "thread.pull-request.watch", number: 9, watching: true }, + ]); + }), + ); + + it.effect("refuses to watch a merged pull request and stops only an existing watch", () => + Effect.gen(function* () { + const watch = { + startedAt: "2026-08-20T00:00:00.000Z", + headSha: null, + checks: null, + remarksThrough: "2026-08-20T00:00:00.000Z", + conflicting: false, + wakes: 0, + }; + const merged = makeLink(1, { headBranch: "done" }); + const harness = yield* makeHarness({ + thread: makeThread([ + { ...merged, snapshot: merged.snapshot && { ...merged.snapshot, state: "merged" } }, + makeLink(2, { headBranch: "idle" }), + makeLink(3, { headBranch: "watched", watch }), + ]), + }); + const error = yield* harness + .call("watch_pull_request", { repository: "t3tools/t3code", number: 1 }) + .pipe(Effect.flip); + expect(error).toMatchObject({ _tag: "PullRequestNotOpenError", state: "merged" }); + expect( + yield* harness.call("unwatch_pull_request", { repository: "t3tools/t3code", number: 2 }), + ).toMatchObject({ watching: false, wasWatching: false }); + expect( + yield* harness.call("unwatch_pull_request", { repository: "t3tools/t3code", number: 3 }), + ).toMatchObject({ watching: false, wasWatching: true }); + expect(yield* Ref.get(harness.commands)).toMatchObject([ + { type: "thread.pull-request.watch", number: 3, watching: false }, + ]); + }), + ); + it.effect("links by repository and number, defaulting the host to the project's", () => Effect.gen(function* () { const harness = yield* makeHarness(); @@ -396,6 +444,7 @@ describe("pull request toolkit handlers", () => { number: 3, url: "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/t3tools/t3code/pull/3", source: "agent", + watching: false, state: "open", title: "PR 3", headBranch: "feat-c", diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.ts index 19677e1bd212..aac480dd6450 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.ts @@ -34,7 +34,9 @@ import { PullRequestHostRequiredError, PullRequestUnlinkFailedError, PullRequestListFailedError, + PullRequestNotOpenError, type PullRequestTargetInput, + PullRequestWatchFailedError, PullRequestThreadNotFoundError, PullRequestsToolkit, type ThreadPullRequestEntry, @@ -122,6 +124,7 @@ function entryOf( number: link.number, url: link.url, source: link.source, + watching: link.watch !== undefined, state: link.snapshot?.state ?? null, title: link.snapshot?.title ?? null, headBranch: link.snapshot?.headBranch ?? null, @@ -163,7 +166,8 @@ const make = Effect.gen(function* () { Failure: | typeof PullRequestLinkFailedError | typeof PullRequestUnlinkFailedError - | typeof PullRequestListFailedError, + | typeof PullRequestListFailedError + | typeof PullRequestWatchFailedError, ) { const scope = yield* McpInvocationContext.requireMcpCapability("pull-requests"); const thread = yield* engine @@ -178,7 +182,10 @@ const make = Effect.gen(function* () { const projectOf = ( thread: OrchestrationV2ThreadShell, - Failure: typeof PullRequestLinkFailedError | typeof PullRequestUnlinkFailedError, + Failure: + | typeof PullRequestLinkFailedError + | typeof PullRequestUnlinkFailedError + | typeof PullRequestWatchFailedError, ) => projects.getShell(thread.projectId).pipe( Effect.map(Option.getOrUndefined), @@ -186,14 +193,72 @@ const make = Effect.gen(function* () { ); const dispatchFailure = - (Failure: typeof PullRequestLinkFailedError | typeof PullRequestUnlinkFailedError) => + ( + Failure: + | typeof PullRequestLinkFailedError + | typeof PullRequestUnlinkFailedError + | typeof PullRequestWatchFailedError, + ) => ( cause: Cause.Cause, - ): Effect.Effect => + ): Effect.Effect< + never, + PullRequestLinkFailedError | PullRequestUnlinkFailedError | PullRequestWatchFailedError + > => Cause.hasInterruptsOnly(cause) ? Effect.failCause(cause as Cause.Cause) : Effect.fail(new Failure({ cause })); + /** Starts or stops a watch on a linked pull request; `watch_pull_request` links it first. */ + const setWatching = Effect.fn("PullRequestsToolkit.setWatching")(function* ( + input: PullRequestTargetInput, + watching: boolean, + ) { + const thread = yield* requireThread(PullRequestWatchFailedError); + const project = yield* projectOf(thread, PullRequestWatchFailedError); + const target = yield* resolveTarget(input, project); + const identity = { + host: target.host, + repository: target.repository, + number: target.number, + url: target.url, + }; + const existing = threadPullRequestsOf(thread).find( + (link) => link.source !== "stack-dismissed" && threadPullRequestKeysEqual(link, target), + ); + const wasWatching = existing?.watch !== undefined; + if (wasWatching === watching || (!watching && existing === undefined)) { + return { ...identity, watching: wasWatching, wasWatching }; + } + const state = existing?.snapshot?.state; + if (watching && state !== undefined && state !== "open") { + return yield* new PullRequestNotOpenError({ state }); + } + if (existing === undefined) { + yield* engine + .dispatch({ + type: "thread.pull-request.link", + commandId: yield* commandId("mcp-pr-link", thread.id), + threadId: thread.id, + ...identity, + source: "agent", + }) + .pipe(Effect.catchCause(dispatchFailure(PullRequestWatchFailedError))); + } + yield* engine + .dispatch({ + type: "thread.pull-request.watch", + commandId: yield* commandId("mcp-pr-watch", thread.id), + threadId: thread.id, + host: target.host, + repository: target.repository, + number: target.number, + watching, + }) + .pipe(Effect.catchCause(dispatchFailure(PullRequestWatchFailedError))); + return { ...identity, watching, wasWatching }; + }); + return PullRequestsToolkit.of({ link_pull_request: (input) => Effect.gen(function* () { @@ -260,6 +325,8 @@ const make = Effect.gen(function* () { }), list_thread_pull_requests: () => requireThread(PullRequestListFailedError).pipe(Effect.map(listThreadPullRequests)), + watch_pull_request: (input) => setWatching(input, true), + unwatch_pull_request: (input) => setWatching(input, false), }); }); diff --git a/apps/server/src/mcp/toolkits/pullRequests/tools.ts b/apps/server/src/mcp/toolkits/pullRequests/tools.ts index 90f22cea2615..0c66f917f318 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/tools.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/tools.ts @@ -108,6 +108,24 @@ export class PullRequestUnlinkFailedError extends Schema.TaggedError()( + "PullRequestWatchFailedError", + { cause: Schema.Defect() }, +) { + override get message(): string { + return "Could not change whether the pull request is watched."; + } +} + +export class PullRequestNotOpenError extends Schema.TaggedError()( + "PullRequestNotOpenError", + { state: Schema.String }, +) { + override get message(): string { + return `The pull request is ${this.state}, so there is nothing to watch.`; + } +} + export class PullRequestListFailedError extends Schema.TaggedError()( "PullRequestListFailedError", { cause: Schema.Defect() }, @@ -126,6 +144,8 @@ export const PullRequestToolError = Schema.Union([ PullRequestLinkFailedError, PullRequestUnlinkFailedError, PullRequestListFailedError, + PullRequestWatchFailedError, + PullRequestNotOpenError, ]); export type PullRequestToolError = typeof PullRequestToolError.Type; @@ -154,9 +174,21 @@ export const UnlinkPullRequestResult = Schema.Struct({ }); export type UnlinkPullRequestResult = typeof UnlinkPullRequestResult.Type; +export const WatchPullRequestResult = Schema.Struct({ + ...PullRequestIdentity, + watching: Schema.Boolean.annotate({ + description: "Whether T3 Code now watches the pull request for this thread.", + }), + wasWatching: Schema.Boolean.annotate({ + description: "Whether it was already watched before the call.", + }), +}); +export type WatchPullRequestResult = typeof WatchPullRequestResult.Type; + export const ThreadPullRequestEntry = Schema.Struct({ ...PullRequestIdentity, source: ThreadPullRequestLinkSource, + watching: Schema.Boolean, state: Schema.NullOr(PullRequestState), title: Schema.NullOr(Schema.String), headBranch: Schema.NullOr(Schema.String), @@ -224,8 +256,38 @@ const ListThreadPullRequestsTool = Tool.make("list_thread_pull_requests", { .annotate(Tool.Idempotent, true) .annotate(Tool.OpenWorld, false); +const WatchPullRequestTool = Tool.make("watch_pull_request", { + description: + "Have T3 Code watch an open pull request for this thread, linking it first if needed. T3 Code checks it every minute and wakes you with a message when checks finish on the latest commit, someone else comments or reviews, or the branch starts to conflict with its base. Use this to monitor or babysit a pull request instead of polling, sleeping, or running a watcher; end your turn after calling it. Watching ends when the pull request merges or closes, or when you call unwatch_pull_request.", + parameters: PullRequestTargetInput, + success: WatchPullRequestResult, + failure: PullRequestToolError, + dependencies, +}) + .annotate(Tool.Title, "Watch pull request") + .annotate(Tool.Readonly, false) + .annotate(Tool.Destructive, false) + .annotate(Tool.Idempotent, true) + .annotate(Tool.OpenWorld, false); + +const UnwatchPullRequestTool = Tool.make("unwatch_pull_request", { + description: + "Stop T3 Code from watching a pull request for this thread. The pull request stays linked. Pass the URL, or repository plus number.", + parameters: PullRequestTargetInput, + success: WatchPullRequestResult, + failure: PullRequestToolError, + dependencies, +}) + .annotate(Tool.Title, "Stop watching pull request") + .annotate(Tool.Readonly, false) + .annotate(Tool.Destructive, false) + .annotate(Tool.Idempotent, true) + .annotate(Tool.OpenWorld, false); + export const PullRequestsToolkit = Toolkit.make( LinkPullRequestTool, UnlinkPullRequestTool, ListThreadPullRequestsTool, + WatchPullRequestTool, + UnwatchPullRequestTool, ); diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 28296f9d7b78..8b76303d8f79 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -19,6 +19,8 @@ import { OrchestrationV2Command, type OrchestrationV2InternalCommand, type OrchestrationV2ServerCommand, + type ThreadPullRequestLink, + type ThreadPullRequestWatch, type OrchestrationV2AppThread, type OrchestrationV2ContextHandoff, type OrchestrationV2ContextSourcePoint, @@ -371,6 +373,8 @@ function commandThreadId(command: OrchestrationV2ServerCommand): ThreadId { case "thread.pull-request.link": case "thread.pull-request.unlink": case "thread.pull-request-link.sync": + case "thread.pull-request.watch": + case "thread.pull-request-watch.sync": case "thread.pull-request.sync": case "thread.title.regeneration.complete": case "thread.runtime-mode.set": @@ -444,6 +448,15 @@ function hasLiveRun(projection: Pick): ); } +/** The link with its watch replaced, or removed when `watch` is undefined. */ +function withPullRequestWatch( + link: ThreadPullRequestLink, + watch: ThreadPullRequestWatch | undefined, +): ThreadPullRequestLink { + const { watch: _previous, ...rest } = link; + return watch === undefined ? rest : { ...rest, watch }; +} + function delegatedCompletionWakeDetail(taskIds: ReadonlyArray): string { const taskList = taskIds.join(", "); return taskIds.length === 1 @@ -2190,7 +2203,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio const dispatchThreadMutation = Effect.fn("orchestrationV2.dispatch.threadMutation")(function* ( command: Extract< - OrchestrationV2Command, + OrchestrationV2ServerCommand, { readonly type: | "thread.archive" @@ -2209,6 +2222,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio | "thread.pull-request.link" | "thread.pull-request.unlink" | "thread.pull-request-link.sync" + | "thread.pull-request.watch" + | "thread.pull-request-watch.sync" | "thread.pull-request.sync" | "thread.title.regeneration.complete" | "thread.runtime-mode.set" @@ -2801,7 +2816,12 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ); pullRequests = belongsToStack ? links.map((link) => - link === existing ? { ...link, source: "stack-dismissed" as const } : link, + link === existing + ? { + ...withPullRequestWatch(link, undefined), + source: "stack-dismissed" as const, + } + : link, ) : links.filter((link) => link !== existing); } else { @@ -2830,6 +2850,39 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio updatedAt: command.type === "thread.pull-request-link.sync" ? thread.updatedAt : now, }; } + case "thread.pull-request.watch": + case "thread.pull-request-watch.sync": { + const key = normalizeThreadPullRequestKey(command); + const links = threadPullRequestsOf(thread); + const existing = links.find( + (link) => link.source !== "stack-dismissed" && threadPullRequestKeysEqual(link, key), + ); + if (existing === undefined) return thread; + const startedAt = DateTime.formatIso(now); + const watch = + command.type === "thread.pull-request-watch.sync" + ? // Progress read before a stop or restart must not bring the old watch back. + existing.watch?.startedAt === command.watch.startedAt + ? command.watch + : existing.watch + : !command.watching + ? undefined + : (existing.watch ?? { + startedAt, + headSha: null, + checks: null, + remarksThrough: startedAt, + conflicting: false, + wakes: 0, + }); + if (watch === existing.watch) return thread; + return { + ...thread, + pullRequests: links.map((link) => + link === existing ? withPullRequestWatch(link, watch) : link, + ), + }; + } case "thread.pull-request.sync": return { ...thread, @@ -2927,6 +2980,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio case "thread.pull-request.link": case "thread.pull-request.unlink": case "thread.pull-request-link.sync": + case "thread.pull-request.watch": + case "thread.pull-request-watch.sync": case "thread.pull-request.sync": return "thread.pull-request-synced" as const; case "thread.runtime-mode.set": @@ -9130,6 +9185,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio case "thread.pull-request.link": case "thread.pull-request.unlink": case "thread.pull-request-link.sync": + case "thread.pull-request.watch": + case "thread.pull-request-watch.sync": case "thread.pull-request.sync": case "thread.title.regeneration.complete": case "thread.runtime-mode.set": diff --git a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts index 60149fe8a292..b2c320f08f69 100644 --- a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts @@ -342,6 +342,36 @@ it.effect.each([ }).pipe(Effect.provide(testLayer)), ); +it.effect.each([ + ["sql", SqlLayer], + ["memory", ProjectionStore.layerMemory], +] as const)("%s: lists only active threads that watch a pull request", ([, testLayer]) => + Effect.gen(function* () { + const store = yield* ProjectionStore.ProjectionStoreV2; + const watch = { + startedAt: DateTime.formatIso(old), + headSha: null, + checks: null, + remarksThrough: DateTime.formatIso(old), + conflicting: false, + wakes: 0, + }; + yield* createThread("unwatched-link", { pullRequests: [pullRequestLink(1)] }); + yield* createThread("archived-watch", { + archivedAt: old, + pullRequests: [{ ...pullRequestLink(2), watch }], + }); + const watching = yield* createThread("watched-link", { + pullRequests: [pullRequestLink(3), { ...pullRequestLink(4), watch }], + }); + + assert.deepEqual( + (yield* store.getThreadsWatchingPullRequests()).map((thread) => thread.id), + [watching], + ); + }).pipe(Effect.provide(testLayer)), +); + it.effect.each([ ["sql", SqlLayer], ["memory", ProjectionStore.layerMemory], diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index ac0624b05806..906b8931fa82 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -355,6 +355,11 @@ export interface ProjectionStoreV2Shape { readonly getThreadsWithPullRequests: ( threadId?: ThreadId, ) => Effect.Effect, ProjectionStoreV2Error>; + /** Active threads with at least one watched pull request link. */ + readonly getThreadsWatchingPullRequests: () => Effect.Effect< + ReadonlyArray, + ProjectionStoreV2Error + >; readonly getTurnStartContext: ( threadId: ThreadId, runId: RunId, @@ -5133,6 +5138,19 @@ export const layer: Layer.Layer = ) .pipe(Effect.mapError((cause) => new ProjectionStoreSetupError({ cause }))); + const decodeThreadPullRequests = (rows: ReadonlyArray) => + Effect.forEach(rows, (row) => + decodeThreadPayload(row.payload_json).pipe( + Effect.map((thread): ProjectionThreadPullRequests => ({ + id: thread.id, + projectId: thread.projectId, + settledOverride: thread.settledOverride, + settledAt: thread.settledAt, + pullRequests: thread.pullRequests ?? [], + })), + ), + ); + const getThreadsWithPullRequests: ProjectionStoreV2Shape["getThreadsWithPullRequests"] = ( threadId, ) => @@ -5145,19 +5163,28 @@ export const layer: Layer.Layer = AND json_array_length(payload_json, '$.pullRequests') > 0 ORDER BY updated_at ASC, thread_id ASC `; - return yield* Effect.forEach(rows, (row) => - decodeThreadPayload(row.payload_json).pipe( - Effect.map((thread): ProjectionThreadPullRequests => ({ - id: thread.id, - projectId: thread.projectId, - settledOverride: thread.settledOverride, - settledAt: thread.settledAt, - pullRequests: thread.pullRequests ?? [], - })), - ), - ); + return yield* decodeThreadPullRequests(rows); }).pipe(Effect.mapError((cause) => new ProjectionStoreSetupError({ cause }))); + // The text match skips the JSON walk for the many threads that watch nothing. + const getThreadsWatchingPullRequests: ProjectionStoreV2Shape["getThreadsWatchingPullRequests"] = + () => + Effect.gen(function* () { + const rows = yield* sql` + SELECT payload_json + FROM orchestration_v2_projection_threads + WHERE deleted_at IS NULL + AND instr(payload_json, '"watch":') > 0 + AND json_extract(payload_json, '$.archivedAt') IS NULL + AND EXISTS ( + SELECT 1 FROM json_each(payload_json, '$.pullRequests') + WHERE json_extract(value, '$.watch') IS NOT NULL + ) + ORDER BY updated_at ASC, thread_id ASC + `; + return yield* decodeThreadPullRequests(rows); + }).pipe(Effect.mapError((cause) => new ProjectionStoreSetupError({ cause }))); + const shellThreadStateFromRow = (input: { readonly row: ShellThreadRow; readonly runOrdinalsByThreadId: ReadonlyMap>; @@ -5456,6 +5483,7 @@ export const layer: Layer.Layer = getThread, getSettlementCandidates, getThreadsWithPullRequests, + getThreadsWatchingPullRequests, getThreadProjection, getTurnStartContext, getTurnStartHistory, @@ -5585,6 +5613,16 @@ export const layerMemory: Layer.Layer = Layer.effect( left.id.localeCompare(right.id), ); }), + getThreadsWatchingPullRequests: () => + service + .getThreadsWithPullRequests() + .pipe( + Effect.map((threads) => + threads.filter((thread) => + (thread.pullRequests ?? []).some((link) => link.watch !== undefined), + ), + ), + ), getThreadsWithPullRequests: (threadId) => Ref.get(replayState).pipe( Effect.map((state) => diff --git a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts index eba830319aaf..5663772ebe02 100644 --- a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts +++ b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts @@ -217,6 +217,7 @@ it.effect( getThread: () => Ref.get(projection).pipe(Effect.map((state) => state.thread)), getSettlementCandidates: () => Effect.die("unused getSettlementCandidates"), getThreadsWithPullRequests: () => Effect.die("unused getThreadsWithPullRequests"), + getThreadsWatchingPullRequests: () => Effect.die("unused getThreadsWatchingPullRequests"), getThreadProjection: () => Effect.die("control effects must not load transcript"), getTurnStartContext: () => Effect.die("unused"), getTurnStartHistory: () => Effect.die("unused"), diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts new file mode 100644 index 000000000000..464b9b551db6 --- /dev/null +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -0,0 +1,185 @@ +import { + CommandId, + MessageId, + type ThreadPullRequestLink, + type ThreadPullRequestWatch, +} from "@t3tools/contracts"; +import { + normalizeThreadPullRequestKey, + threadPullRequestKeyOf, + visibleThreadPullRequests, +} from "@t3tools/shared/threadPullRequests"; +import * as Cause from "effect/Cause"; +import * as Context from "effect/Context"; +import * as Crypto from "effect/Crypto"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Schedule from "effect/Schedule"; +import type * as Scope from "effect/Scope"; + +import * as PullRequestService from "../pullRequest/PullRequestService.ts"; +import { forkParked } from "../serverActivation.ts"; +import * as Orchestrator from "./Orchestrator.ts"; +import * as ProjectionStore from "./ProjectionStore.ts"; +import { evaluatePullRequestWatch, pullRequestWatchMessage } from "./pullRequestWatch.ts"; + +const logFailure = + (message: string, fields: Record) => + (cause: Cause.Cause): Effect.Effect => + Cause.hasInterruptsOnly(cause) + ? Effect.interrupt + : Effect.logWarning(message, { ...fields, cause: Cause.pretty(cause) }); + +function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatch): boolean { + return ( + left.startedAt === right.startedAt && + left.headSha === right.headSha && + left.checks === right.checks && + left.remarksThrough === right.remarksThrough && + left.conflicting === right.conflicting && + left.wakes === right.wakes + ); +} + +/** + * Wakes a thread's agent when a pull request it watches (`watch_pull_request`) needs a look: + * checks finished on the head commit, someone else commented, or the branch started to + * conflict. One pass a minute reads each watched pull request; settled threads wait until + * they are active again, and a merged or closed pull request ends its watch. + */ +export class PullRequestWatchReactor extends Context.Service< + PullRequestWatchReactor, + { + readonly start: () => Effect.Effect; + /** One pass over every watched pull request. */ + readonly sweep: Effect.Effect; + } +>()("t3/orchestration-v2/PullRequestWatchReactor") {} + +/** @public Service construction is part of the canonical Effect module API. */ +export const make = Effect.gen(function* () { + const engine = yield* Orchestrator.OrchestratorV2; + const projections = yield* ProjectionStore.ProjectionStoreV2; + const pullRequests = yield* PullRequestService.PullRequestService; + const crypto = yield* Crypto.Crypto; + + const check = Effect.fn("PullRequestWatchReactor.check")(function* ( + thread: ProjectionStore.ProjectionThreadPullRequests, + link: ThreadPullRequestLink, + watch: ThreadPullRequestWatch, + ) { + // Host-level identity, with the repository as linked, the way pull request sync reads it. + const pullRequest = { + host: normalizeThreadPullRequestKey(link).host, + repository: link.repository, + number: link.number, + }; + const stop = crypto.randomUUIDv4.pipe( + Effect.flatMap((uuid) => + engine.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make(`server:pr-watch-stop:${thread.id}:${uuid}`), + threadId: thread.id, + ...pullRequest, + watching: false, + }), + ), + ); + if (link.snapshot !== null && link.snapshot.state !== "open") return yield* stop; + if (thread.settledOverride === "settled" || thread.settledAt !== null) return; + + const reference = { projectId: thread.projectId, ...pullRequest }; + const [detail, activity] = yield* Effect.all( + [pullRequests.detail({ ...reference, allowStale: false }), pullRequests.activity(reference)], + { concurrency: 2 }, + ); + if (detail.state !== "open") return yield* stop; + + const report = evaluatePullRequestWatch(watch, detail, activity); + if (report.changes.length > 0) { + const { text, notification } = pullRequestWatchMessage({ + number: link.number, + url: link.url, + baseBranch: detail.baseBranch, + headSha: report.next.headSha, + report, + }); + // Derived from the watch it records, so a pass that repeats after a failed record below + // replays this command's receipt instead of waking the agent twice. A wake the + // orchestrator refuses is logged and still recorded, so the watch cannot stall on it. + const next = report.next; + const wakeId = [ + thread.id, + threadPullRequestKeyOf(link), + Date.parse(next.startedAt), + next.wakes, + next.headSha ?? "-", + next.checks ?? "-", + Date.parse(next.remarksThrough), + next.conflicting ? 1 : 0, + ].join(":"); + yield* engine + .dispatch({ + type: "message.dispatch", + commandId: CommandId.make(`server:pr-watch:${wakeId}`), + threadId: thread.id, + messageId: MessageId.make(`message:pr-watch:${wakeId}`), + text, + notification, + attachments: [], + dispatchMode: { type: "queue_after_active" }, + createdBy: "agent", + creationSource: "server", + }) + .pipe( + Effect.catchCause( + logFailure("pull request watch wake failed", { + threadId: thread.id, + pullRequest: threadPullRequestKeyOf(link), + }), + ), + ); + } + if (report.exhausted) return yield* stop; + if (watchesEqual(report.next, watch)) return; + const uuid = yield* crypto.randomUUIDv4; + yield* engine.dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make(`server:pr-watch-sync:${thread.id}:${uuid}`), + threadId: thread.id, + ...pullRequest, + watch: report.next, + }); + }); + + const sweep = Effect.gen(function* () { + const threads = yield* projections.getThreadsWatchingPullRequests(); + yield* Effect.forEach( + threads.flatMap((thread) => + visibleThreadPullRequests(thread.pullRequests ?? []).flatMap((link) => + link.watch === undefined ? [] : [{ thread, link, watch: link.watch }], + ), + ), + ({ thread, link, watch }) => + check(thread, link, watch).pipe( + Effect.catchCause( + logFailure("pull request watch check failed", { + threadId: thread.id, + pullRequest: threadPullRequestKeyOf(link), + }), + ), + ), + { concurrency: 4, discard: true }, + ); + }).pipe( + Effect.catchCause(logFailure("pull request watch sweep failed", {})), + Effect.withSpan("PullRequestWatchReactor.sweep"), + ); + + const start: PullRequestWatchReactor["Service"]["start"] = () => + forkParked(sweep.pipe(Effect.repeat(Schedule.spaced("1 minute")), Effect.asVoid)); + + return { start, sweep } satisfies PullRequestWatchReactor["Service"]; +}); + +export const layer = Layer.effect(PullRequestWatchReactor, make); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts new file mode 100644 index 000000000000..91338114a1b3 --- /dev/null +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -0,0 +1,154 @@ +import type { + PullRequestCheck, + PullRequestComment, + PullRequestDetail, + ThreadPullRequestWatch, +} from "@t3tools/contracts"; +import { assert, describe, it } from "@effect/vitest"; + +import { + PULL_REQUEST_WATCH_WAKE_LIMIT, + evaluatePullRequestWatch, + pullRequestWatchMessage, +} from "./pullRequestWatch.ts"; + +const STARTED = "2026-10-02T12:00:00.000Z"; + +const watch = (overrides: Partial = {}): ThreadPullRequestWatch => ({ + startedAt: STARTED, + headSha: null, + checks: null, + remarksThrough: STARTED, + conflicting: false, + wakes: 0, + ...overrides, +}); + +const check = (name: string, status: PullRequestCheck["status"]): PullRequestCheck => ({ + name, + status, + description: null, + url: `https://ci.example/${name}`, +}); + +type Detail = Parameters[1]; + +const detail = (overrides: Partial = {}): Detail => ({ + headSha: "aaaaaaaaaa", + checks: [check("lint", "success"), check("test", "pending")], + mergeability: "mergeable", + viewer: "agent-user", + author: { login: "agent-user", name: null, avatarUrl: null }, + ...overrides, +}); + +const remark = ( + login: string, + createdAt: string, + body = "Please rename this.", +): PullRequestComment => ({ + id: `${login}-${createdAt}`, + kind: "review-comment", + author: { login, name: null, avatarUrl: null }, + body, + createdAt, + url: `https://github.com/o/r/pull/1#${login}`, + path: "src/index.ts", + reviewState: null, +}); + +const noRemarks = { comments: [] }; + +describe("evaluatePullRequestWatch", () => { + it("reports a check result once per head commit", () => { + const running = evaluatePullRequestWatch(watch(), detail(), noRemarks); + assert.deepEqual(running.changes, []); + assert.equal(running.next.headSha, "aaaaaaaaaa"); + + const failing = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); + const failed = evaluatePullRequestWatch(running.next, failing, noRemarks); + assert.deepEqual(failed.changes, [ + { kind: "checks-failed", failed: [check("lint", "failure")] }, + ]); + assert.deepEqual(evaluatePullRequestWatch(failed.next, failing, noRemarks).changes, []); + + // A push that fails the same way before a pass ever sees it pending is still news. + const pushed = detail({ ...failing, headSha: "bbbbbbbbbb" }); + assert.equal( + evaluatePullRequestWatch(failed.next, pushed, noRemarks).changes[0]?.kind, + "checks-failed", + ); + + // A rerun that passes on the same head is news too. + const rerun = detail({ checks: [check("lint", "success"), check("test", "success")] }); + assert.deepEqual(evaluatePullRequestWatch(failed.next, rerun, noRemarks).changes, [ + { kind: "checks-passed", count: 2 }, + ]); + }); + + it("reports remarks from others once and never the agent's own", () => { + const comments = [ + remark("agent-user", "2026-10-02T12:05:00Z", "Fixed in the latest push."), + remark("macroscope-app[bot]", "2026-10-02T12:06:00Z"), + remark("reviewer", "2026-10-02T11:00:00Z", "Older than the watch."), + ]; + const report = evaluatePullRequestWatch(watch(), detail(), { comments }); + assert.deepEqual(report.changes, [{ kind: "remarks", remarks: [comments[1]!] }]); + assert.equal(report.next.remarksThrough, "2026-10-02T12:06:00Z"); + assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), { comments }).changes, []); + }); + + it("reports a conflict once, until the branch is clean again", () => { + const conflicting = detail({ mergeability: "conflicting" }); + const first = evaluatePullRequestWatch(watch(), conflicting, noRemarks); + assert.deepEqual(first.changes, [{ kind: "conflicting" }]); + // GitHub answers "unknown" while it recomputes after a push; that is not a resolution. + const recomputing = evaluatePullRequestWatch( + first.next, + detail({ mergeability: "unknown" }), + noRemarks, + ); + assert.deepEqual( + evaluatePullRequestWatch(recomputing.next, conflicting, noRemarks).changes, + [], + ); + const clean = evaluatePullRequestWatch(first.next, detail(), noRemarks); + assert.deepEqual(evaluatePullRequestWatch(clean.next, conflicting, noRemarks).changes, [ + { kind: "conflicting" }, + ]); + }); + + it("stops after the wake limit unless the head moves", () => { + const comments = [remark("reviewer", "2026-10-02T12:10:00Z")]; + const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); + assert.isTrue(evaluatePullRequestWatch(tired, detail(), { comments }).exhausted); + const pushed = evaluatePullRequestWatch(tired, detail({ headSha: "cccccccccc" }), { comments }); + assert.isFalse(pushed.exhausted); + assert.equal(pushed.next.wakes, 1); + }); +}); + +describe("pullRequestWatchMessage", () => { + it("tells the agent what changed and marks failures for the timeline", () => { + const report = evaluatePullRequestWatch( + watch(), + detail({ checks: [check("lint", "failure")] }), + { comments: [remark("reviewer", "2026-10-02T12:10:00Z", "Needs a test.")] }, + ); + const message = pullRequestWatchMessage({ + number: 12, + url: "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/o/r/pull/12", + baseBranch: "main", + headSha: report.next.headSha, + report, + }); + assert.include(message.text, "- Checks failed on aaaaaaa:\n - lint https://ci.example/lint"); + assert.include(message.text, ' - reviewer on src/index.ts: "Needs a test."'); + assert.include(message.text, "unwatch_pull_request"); + assert.deepEqual(message.notification, { + source: { kind: "monitor" }, + outcome: "failed", + summary: "#12: checks failed, new comments", + }); + }); +}); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts new file mode 100644 index 000000000000..0086fc65d105 --- /dev/null +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -0,0 +1,189 @@ +import type { + OrchestrationV2Notification, + PullRequestActivity, + PullRequestCheck, + PullRequestComment, + PullRequestDetail, + ThreadPullRequestWatch, +} from "@t3tools/contracts"; + +/** Wakes allowed without a new push before watching stops, so a chatty bot cannot loop an agent. */ +export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10; +const LISTED_ITEMS = 10; +const SNIPPET_LENGTH = 200; + +export type PullRequestWatchChange = + | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } + | { readonly kind: "checks-passed"; readonly count: number } + | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } + | { readonly kind: "conflicting" }; + +export interface PullRequestWatchReport { + /** What the agent has not been told yet. Empty means no wake. */ + readonly changes: ReadonlyArray; + /** The watch to record, whether or not anything is reported. */ + readonly next: ThreadPullRequestWatch; + /** This report spends the last wake before the limit, so watching stops after it. */ + readonly exhausted: boolean; +} + +const isFailedCheck = (check: PullRequestCheck) => + check.status === "failure" || check.status === "cancelled"; + +function checksOutcome( + checks: ReadonlyArray, +): "passing" | "failing" | "pending" | null { + if (checks.length === 0) return null; + if (checks.some(isFailedCheck)) return "failing"; + if (checks.some((check) => check.status === "pending" || check.status === "action-required")) { + return "pending"; + } + return "passing"; +} + +/** + * Compares a watched pull request with what its agent was last told. A check result is + * reported once per head commit, as soon as any check fails or once every check passed. + * Remarks count when someone other than the viewer or the pull request's author wrote them: + * the agent posts as the viewer, so its own replies never wake it. + */ +export function evaluatePullRequestWatch( + watch: ThreadPullRequestWatch, + detail: Pick, + activity: Pick, +): PullRequestWatchReport { + const changes: Array = []; + const headSha = detail.headSha ?? null; + const headMoved = headSha !== watch.headSha; + + let checks = headMoved ? null : watch.checks; + const outcome = checksOutcome(detail.checks); + if ((outcome === "failing" || outcome === "passing") && outcome !== checks) { + changes.push( + outcome === "failing" + ? { kind: "checks-failed", failed: detail.checks.filter(isFailedCheck) } + : { kind: "checks-passed", count: detail.checks.length }, + ); + checks = outcome; + } + + const own = new Set( + [detail.viewer, detail.author?.login].flatMap((login) => (login ? [login.toLowerCase()] : [])), + ); + const reportedThrough = Date.parse(watch.remarksThrough); + const remarks = activity.comments.filter( + (remark) => + Date.parse(remark.createdAt) > reportedThrough && + !own.has(remark.author?.login.toLowerCase() ?? ""), + ); + if (remarks.length > 0) changes.push({ kind: "remarks", remarks }); + const remarksThrough = remarks.reduce( + (latest, remark) => + Date.parse(remark.createdAt) > Date.parse(latest) ? remark.createdAt : latest, + watch.remarksThrough, + ); + + if (detail.mergeability === "conflicting" && !watch.conflicting) { + changes.push({ kind: "conflicting" }); + } + // "unknown" is GitHub still computing after a push; only a clean answer clears a conflict. + const conflicting = + detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; + + const wakes = (headMoved ? 0 : watch.wakes) + (changes.length > 0 ? 1 : 0); + return { + changes, + next: { startedAt: watch.startedAt, headSha, checks, remarksThrough, conflicting, wakes }, + exhausted: changes.length > 0 && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT, + }; +} + +function snippet(body: string): string { + const text = body + .replaceAll(//g, " ") + .replaceAll(/\s+/g, " ") + .trim(); + return text.length <= SNIPPET_LENGTH ? text : `${text.slice(0, SNIPPET_LENGTH - 3)}...`; +} + +function listed(items: ReadonlyArray, line: (item: T) => string): Array { + const lines = items.slice(0, LISTED_ITEMS).map(line); + if (items.length > LISTED_ITEMS) lines.push(` - and ${items.length - LISTED_ITEMS} more`); + return lines; +} + +function changeLines( + change: PullRequestWatchChange, + context: { readonly baseBranch: string; readonly commit: string }, +): Array { + switch (change.kind) { + case "checks-failed": + return [ + `- Checks failed${context.commit}:`, + ...listed(change.failed, (check) => ` - ${check.name}${check.url ? ` ${check.url}` : ""}`), + ]; + case "checks-passed": + return [ + `- All ${change.count} ${change.count === 1 ? "check" : "checks"} passed${context.commit}.`, + ]; + case "remarks": + return [ + `- ${change.remarks.length} new ${change.remarks.length === 1 ? "comment" : "comments"}:`, + ...listed(change.remarks, (remark) => { + const where = remark.path === null ? "" : ` on ${remark.path}`; + const body = snippet(remark.body); + const said = body.length === 0 ? (remark.reviewState ?? "reviewed") : `"${body}"`; + return ` - ${remark.author?.login ?? "someone"}${where}: ${said}${remark.url ? ` ${remark.url}` : ""}`; + }), + ]; + case "conflicting": + return [`- The branch now conflicts with ${context.baseBranch}.`]; + } +} + +const SUMMARY: Record = { + "checks-failed": "checks failed", + "checks-passed": "checks passed", + remarks: "new comments", + conflicting: "merge conflict", +}; + +/** The wake the agent reads and the timeline notification the user sees. */ +export function pullRequestWatchMessage(input: { + readonly number: number; + readonly url: string; + readonly baseBranch: string; + readonly headSha: string | null; + readonly report: PullRequestWatchReport; +}): { readonly text: string; readonly notification: OrchestrationV2Notification } { + const { changes, exhausted } = input.report; + const context = { + baseBranch: input.baseBranch, + commit: input.headSha === null ? "" : ` on ${input.headSha.slice(0, 7)}`, + }; + const text = [ + `Update on pull request #${input.number} (${input.url}), which T3 Code is watching for you:`, + ...changes.flatMap((change) => changeLines(change, context)), + "", + exhausted + ? `T3 Code stopped watching after ${PULL_REQUEST_WATCH_WAKE_LIMIT} updates without a new push. Call watch_pull_request to watch it again.` + : "Look into each item and act on it as your task requires. T3 Code keeps watching and wakes you on the next change, so end your turn when you are done. Call unwatch_pull_request when you no longer need updates.", + ].join("\n"); + const failed = changes.some( + (change) => change.kind === "checks-failed" || change.kind === "conflicting", + ); + const summary = changes.map((change) => SUMMARY[change.kind]); + if (exhausted) summary.push("stopped watching"); + return { + text, + notification: { + source: { kind: "monitor" }, + outcome: failed + ? "failed" + : changes.every((change) => change.kind === "checks-passed") + ? "completed" + : "updated", + summary: `#${input.number}: ${summary.join(", ")}`, + }, + }; +} diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index d077f7e20910..a7a4ebda59c7 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -17,6 +17,7 @@ import { type ModelSelection, type OrchestrationV2Run, ProjectId, + type PullRequestDetail, ProviderDriverKind, ProviderInstanceId, ProviderThreadId, @@ -58,6 +59,9 @@ import * as EffectOutbox from "./EffectOutbox.ts"; import * as EventSink from "./EventSink.ts"; import * as ProviderRuntimeRecoveryService from "./ProviderRuntimeRecoveryService.ts"; import * as ProjectionMaintenance from "./ProjectionMaintenance.ts"; +import * as ProjectionStore from "./ProjectionStore.ts"; +import * as PullRequestWatchReactor from "./PullRequestWatchReactor.ts"; +import * as PullRequestService from "../pullRequest/PullRequestService.ts"; import * as ProjectStore from "./ProjectStore.ts"; import type { ProviderAdapterV2SessionRuntime, ProviderAdapterV2Shape } from "./ProviderAdapter.ts"; import * as ProviderSessionManager from "./ProviderSessionManager.ts"; @@ -212,6 +216,7 @@ const TestLayer = Layer.mergeAll( OrchestrationV2LayerLive, OrchestrationV2EventSinkLayerLive, ProjectStore.layer, + ProjectionStore.layer, EffectOutbox.layer, ThreadCommandExecutor.layer, ).pipe( @@ -2148,6 +2153,208 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect("starts, records, and stops a pull request watch", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const maintenance = yield* ProjectionMaintenance.ProjectionMaintenanceV2; + const threadId = ThreadId.make("runtime-pull-request-watch"); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make("pr-watch-create"), + threadId, + projectId: ProjectId.make("pr-watch-project"), + title: "Watch", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + const key = { host: "github.com", repository: "pingdotgg/t3code", number: 7 }; + yield* orchestrator.dispatch({ + type: "thread.pull-request.link", + commandId: CommandId.make("pr-watch-link"), + threadId, + ...key, + url: "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/pingdotgg/t3code/pull/7", + source: "agent", + }); + const watchOf = Effect.map( + orchestrator.getThreadShell(threadId), + (thread) => thread?.pullRequests?.[0]?.watch, + ); + + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-start"), + threadId, + ...key, + watching: true, + }); + const started = yield* watchOf; + assert.isDefined(started); + if (started === undefined) return; + + const recorded = { ...started, headSha: "abc123", checks: "failing" as const, wakes: 1 }; + yield* orchestrator.dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make("pr-watch-record"), + threadId, + ...key, + watch: recorded, + }); + assert.deepEqual(yield* watchOf, recorded); + assert.isTrue((yield* maintenance.rebuild).valid); + assert.deepEqual(yield* watchOf, recorded); + + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-stop"), + threadId, + ...key, + watching: false, + }); + // Progress read before the stop must not bring the watch back. + yield* orchestrator.dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make("pr-watch-late-record"), + threadId, + ...key, + watch: { ...recorded, wakes: 2 }, + }); + assert.isUndefined(yield* watchOf); + }), + ); + + it.effect("wakes a watched thread once when its pull request's checks fail", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const threadId = ThreadId.make("runtime-pull-request-watch-wake"); + const projectId = ProjectId.make("pr-watch-wake-project"); + yield* seedProject({ + projectId, + title: "Watch wake", + workspaceRoot: "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/workspace/watch", + defaultModelSelection: null, + createdAt: "2026-10-01T00:00:00.000Z", + }); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make("pr-watch-wake-create"), + threadId, + projectId, + title: "Watch wake", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + const key = { host: "github.com", repository: "pingdotgg/t3code", number: 7 }; + const url = "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/pingdotgg/t3code/pull/7"; + yield* orchestrator.dispatch({ + type: "thread.pull-request.link", + commandId: CommandId.make("pr-watch-wake-link"), + threadId, + ...key, + url, + source: "agent", + }); + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-wake-start"), + threadId, + ...key, + watching: true, + }); + + const at = "2026-10-02T12:00:00.000Z"; + const detail: PullRequestDetail = { + provider: "github", + capabilities: { + diff: true, + comment: true, + actions: [], + mergeMethods: [], + search: false, + review: { inlineComment: false, reply: false, resolve: false, verdicts: [] }, + reviewers: { request: false, listCandidates: false }, + }, + viewerPermissions: { + actions: [], + comment: true, + resolve: true, + verdicts: [], + requestReviewers: false, + }, + projectId, + projectTitle: "Watch wake", + workspaceRoot: "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/workspace/watch", + repository: key.repository, + number: key.number, + title: "Watched pull request", + body: "", + url, + author: { login: "agent-user", name: null, avatarUrl: null }, + state: "open", + isDraft: false, + mergeability: "mergeable", + additions: 1, + deletions: 0, + changedFiles: 1, + headBranch: "feature", + headSha: "abc1234def", + baseBranch: "main", + createdAt: at, + updatedAt: at, + mergedAt: null, + closedAt: null, + reviewers: [], + labels: [], + checks: [{ name: "lint", status: "failure", description: null, url: null }], + mergeCapabilities: { merge: true, squash: true, rebase: true }, + viewer: "agent-user", + }; + const reactor = yield* PullRequestWatchReactor.make.pipe( + Effect.provide( + Layer.mergeAll( + NodeServices.layer, + Layer.mock(PullRequestService.PullRequestService)({ + detail: () => Effect.succeed(detail), + activity: () => + Effect.succeed({ + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreads: [], + commits: [], + }), + }), + ), + ), + ); + yield* reactor.sweep; + yield* reactor.sweep; + + const { messages } = yield* orchestrator.getThreadRecords(threadId, ["messages"]); + assert.deepEqual( + messages.flatMap((message) => + message.notification === undefined ? [] : [message.notification.summary], + ), + ["#7: checks failed"], + ); + const watch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; + assert.deepEqual( + { headSha: watch?.headSha, checks: watch?.checks, wakes: watch?.wakes }, + { headSha: "abc1234def", checks: "failing", wakes: 1 }, + ); + }), + ); + it.effect("persists rejected command receipts across retries", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; diff --git a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts index 4cc834dddd62..b9cad107409a 100644 --- a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts +++ b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts @@ -153,6 +153,7 @@ function commandThreadIds(command: OrchestrationV2Command): ReadonlyArray -When the t3-code MCP server exposes link_pull_request, you must use it to register every pull request you create or work on for this thread. Call link_pull_request with the full PR URL immediately after creating a PR or starting work on an existing PR. For a stack, call it for every layer, not just the current branch or the top PR. This applies when creating or updating PRs through gh, gh stack, another CLI, or the host API: those operations do not register the PRs with this thread. Linking an already-linked PR is safe. Before finishing PR work, call list_thread_pull_requests and link any PR from your work that is missing. Do not link unrelated PRs mentioned only as background. If a linking call fails, report that failure instead of claiming the PR is linked. +When the t3-code MCP server exposes link_pull_request, you must use it to register every pull request you create or work on for this thread. Call link_pull_request with the full PR URL immediately after creating a PR or starting work on an existing PR. For a stack, call it for every layer, not just the current branch or the top PR. This applies when creating or updating PRs through gh, gh stack, another CLI, or the host API: those operations do not register the PRs with this thread. Linking an already-linked PR is safe. Before finishing PR work, call list_thread_pull_requests and link any PR from your work that is missing. Do not link unrelated PRs mentioned only as background. If a linking call fails, report that failure instead of claiming the PR is linked. When asked to monitor, watch, or babysit a PR and watch_pull_request is available, call it and end your turn: T3 Code wakes you when checks finish, someone else comments, or the branch conflicts, so do not poll or run your own watcher. `; /** diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index ed4312c125c7..cc370637c601 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -213,6 +213,8 @@ export interface ProviderChangeRequestStat { } export interface ProviderChangeRequestDetail extends ProviderChangeRequest { + /** The head commit, where the host's detail read reports it. */ + readonly headSha?: string | null; readonly body: string; readonly changedFiles: number; readonly mergedAt: string | null; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 44c175ab652f..293318f91c4b 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1697,6 +1697,7 @@ export const make = Effect.gen(function* () { ...(changeRequest.headRepositoryNameWithOwner === undefined ? {} : { headRepositoryNameWithOwner: changeRequest.headRepositoryNameWithOwner }), + ...(changeRequest.headSha ? { headSha: changeRequest.headSha } : {}), baseBranch: changeRequest.baseBranch, createdAt: changeRequest.createdAt, updatedAt: changeRequest.updatedAt, diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 98039e56642a..9e9d5c5e3064 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -4,6 +4,7 @@ import * as Random from "effect/Random"; import * as Semaphore from "effect/Semaphore"; import * as StorageCleanup from "./storageCleanup.ts"; import * as PullRequestSyncReactor from "./orchestration-v2/PullRequestSyncReactor.ts"; +import * as PullRequestWatchReactor from "./orchestration-v2/PullRequestWatchReactor.ts"; // @effect-diagnostics nodeBuiltinImport:off import * as NodeHttp from "node:http"; @@ -521,6 +522,16 @@ const RuntimeCoreDependenciesBaseLive = Layer.mergeAll( Layer.provide(PullRequestServiceLive), Layer.provide(ProjectionStoreV2.layer), ), + Layer.effectDiscard( + Effect.gen(function* () { + const service = yield* PullRequestWatchReactor.PullRequestWatchReactor; + yield* service.start(); + }), + ).pipe( + Layer.provide(PullRequestWatchReactor.layer), + Layer.provide(PullRequestServiceLive), + Layer.provide(ProjectionStoreV2.layer), + ), // Subscribes to `account.rate-limits.updated` so usage bars track live // telemetry instead of waiting for the next status probe. ProviderUsageLimitsIngestionLive, diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 5dec1b6d411d..b5113b5f41a3 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -3630,6 +3630,8 @@ function toolGroupSummaryIconName( case "link-pr": case "unlink-pr": case "list-prs": + case "watch-pr": + case "unwatch-pr": return "pull-request"; case "read": return "eye"; diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx index 3dd1bcbefaff..cfd42e2c532a 100644 --- a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx +++ b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx @@ -3,7 +3,14 @@ import { resolveThreadPullRequestChains, visibleThreadPullRequests, } from "@t3tools/shared/threadPullRequests"; -import { ArrowUpRightIcon, LinkIcon, MoreHorizontalIcon, PlusIcon } from "lucide-react"; +import { + ArrowUpRightIcon, + EyeIcon, + EyeOffIcon, + LinkIcon, + MoreHorizontalIcon, + PlusIcon, +} from "lucide-react"; import { useCallback, useMemo } from "react"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; @@ -68,14 +75,19 @@ function LinkRow({ line, threadRef, onUnlink, + onSetWatching, }: { line: PullRequestListLine; threadRef: ScopedThreadRef; onUnlink: (link: ThreadPullRequestLink) => void; + /** Null when the environment cannot watch pull requests. */ + onSetWatching: ((link: ThreadPullRequestLink, watching: boolean) => void) | null; }) { const openPrLink = useOpenPrLink(threadRef); const { link, depth, stack } = line; const snapshot = link.snapshot; + const open = snapshot === null || snapshot.state === "open"; + const watching = link.watch !== undefined; return (
+ {watching ? ( + + }> + + + + Watching: the agent wakes when checks finish, someone comments, or the branch + conflicts + + + ) : null} {snapshot.checksState ? : null} {snapshot.reviewDecision ? ( @@ -220,6 +243,12 @@ function LinkRow({ Open + {onSetWatching !== null && open ? ( + onSetWatching(link, !watching)}> + {watching ? : } + {watching ? "Stop watching" : "Watch for changes"} + + ) : null} onUnlink(link)}> {link.source === "stack" ? "Dismiss from thread" : "Unlink from thread"} @@ -248,6 +277,10 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread const thread = useThreadShell(threadRef); const openLinkDialog = useCallback(() => openLinkPullRequestDialog(threadRef), [threadRef]); const unlink = useAtomCommand(threadEnvironment.unlinkPullRequest, { reportFailure: true }); + const watch = useAtomCommand(threadEnvironment.watchPullRequest, { reportFailure: true }); + const supportsWatch = + useServerConfigs().get(threadRef.environmentId)?.environment.capabilities + .threadPullRequestWatch === true; const links = useMemo(() => visibleThreadPullRequests(thread?.pullRequests ?? []), [thread]); const lines = useMemo(() => pullRequestListLines(resolveThreadPullRequestChains(links)), [links]); const handleUnlink = useCallback( @@ -264,6 +297,21 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread }, [threadRef, unlink], ); + const handleSetWatching = useCallback( + (link: ThreadPullRequestLink, watching: boolean) => { + void watch({ + environmentId: threadRef.environmentId, + input: { + threadId: threadRef.threadId, + host: link.host, + repository: link.repository, + number: link.number, + watching, + }, + }); + }, + [threadRef, watch], + ); const openCount = useMemo( () => links.filter((link) => link.snapshot === null || link.snapshot.state === "open").length, [links], @@ -304,6 +352,7 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread line={line} threadRef={threadRef} onUnlink={handleUnlink} + onSetWatching={supportsWatch ? handleSetWatching : null} /> ))}
diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 3d068a232be7..9f9fb5c3f14c 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -181,6 +181,13 @@ closed reviews refresh periodically so reopening one on the host is detected. Me when requested. With **Auto-settle merged threads** enabled, a thread can settle after every linked review is terminal. An open or unsynced link keeps it active. +Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While +the thread is active, the server checks the pull request every minute and wakes the agent when checks +finish on the latest commit, someone else comments or reviews, or the branch starts to conflict. +Comments from your own account do not wake it. Watching ends when the pull request merges or closes, +or after 10 wakes without a new push. To start or stop it yourself, use the row menu in the **Linked +pull requests** panel, or long-press the review in the mobile Git overview. + Cross-repository links use a project on the same host. Azure DevOps reviews require a project checked out from the matching organization and repository. diff --git a/packages/client-runtime/src/operations/commands.ts b/packages/client-runtime/src/operations/commands.ts index ded1ff733e22..c0cb8e67504c 100644 --- a/packages/client-runtime/src/operations/commands.ts +++ b/packages/client-runtime/src/operations/commands.ts @@ -1027,6 +1027,20 @@ export const linkThreadPullRequest = Effect.fn("EnvironmentCommands.linkThreadPu }); }, ); +export type WatchThreadPullRequestInput = Omit< + Extract, + "type" | "commandId" +> & + CommandMetadata; +export const watchThreadPullRequest = Effect.fn("EnvironmentCommands.watchThreadPullRequest")( + function* (input: WatchThreadPullRequestInput) { + return yield* dispatch({ + ...input, + type: "thread.pull-request.watch", + commandId: yield* allocateCommandId(input), + }); + }, +); export const unlinkThreadPullRequest = Effect.fn("EnvironmentCommands.unlinkThreadPullRequest")( function* (input: UnlinkThreadPullRequestInput) { return yield* dispatch({ diff --git a/packages/client-runtime/src/state/threadCommands.ts b/packages/client-runtime/src/state/threadCommands.ts index 685ccc454a08..e3ad5c07f583 100644 --- a/packages/client-runtime/src/state/threadCommands.ts +++ b/packages/client-runtime/src/state/threadCommands.ts @@ -48,6 +48,7 @@ import { type UnarchiveThreadInput, type UnlinkThreadPullRequestInput, type UnpinThreadInput, + type WatchThreadPullRequestInput, type UnsettleThreadInput, type UnsnoozeThreadInput, type UpdateThreadMetadataInput, @@ -86,6 +87,7 @@ import { unsnoozeThread, updateThreadMetadata, visitThread, + watchThreadPullRequest, } from "../operations/commands.ts"; import type { EnvironmentRegistry } from "../connection/registry.ts"; import * as EnvironmentSupervisor from "../connection/supervisor.ts"; @@ -130,6 +132,7 @@ export type { UnsnoozeThreadInput, UpdateThreadMetadataInput, VisitThreadInput, + WatchThreadPullRequestInput, } from "../operations/commands.ts"; export function createThreadEnvironmentAtoms( @@ -251,6 +254,12 @@ export function createThreadEnvironmentAtoms( scheduler, concurrency, }), + watchPullRequest: createEnvironmentCommand(runtime, { + label: "environment-data:commands:thread:watch-pull-request", + execute: (input: WatchThreadPullRequestInput) => watchThreadPullRequest(input), + scheduler, + concurrency, + }), setRuntimeMode: createEnvironmentCommand(runtime, { label: "environment-data:commands:thread:set-runtime-mode", execute: (input: SetThreadRuntimeModeInput) => setThreadRuntimeMode(input), diff --git a/packages/client-runtime/src/t3ToolSummary.ts b/packages/client-runtime/src/t3ToolSummary.ts index 79577892a697..c6b40e5a62ff 100644 --- a/packages/client-runtime/src/t3ToolSummary.ts +++ b/packages/client-runtime/src/t3ToolSummary.ts @@ -372,6 +372,16 @@ export function summarizeT3ToolCalls( case "unlink-pr": label = phrase("Unlinked", "unlink", quantity(selected.length, "pull request")); break; + case "watch-pr": + label = phrase("Watching", "watch", quantity(selected.length, "pull request")); + break; + case "unwatch-pr": + label = phrase( + "Stopped watching", + "stop watching", + quantity(selected.length, "pull request"), + ); + break; case "list-prs": label = phrase( "Checked", diff --git a/packages/client-runtime/src/work-log/presentation.ts b/packages/client-runtime/src/work-log/presentation.ts index fdaf28dcf3b7..84229d3bac7f 100644 --- a/packages/client-runtime/src/work-log/presentation.ts +++ b/packages/client-runtime/src/work-log/presentation.ts @@ -85,6 +85,8 @@ export type ToolGroupAction = | "link-pr" | "unlink-pr" | "list-prs" + | "watch-pr" + | "unwatch-pr" | "read" | "edit" | "command" @@ -156,7 +158,9 @@ function resolveT3McpToolPresentation( const actionKind = definition.summaryAction === "link-pr" || definition.summaryAction === "unlink-pr" || - definition.summaryAction === "list-prs" + definition.summaryAction === "list-prs" || + definition.summaryAction === "watch-pr" || + definition.summaryAction === "unwatch-pr" ? definition.summaryAction : undefined; const payload = asRecord(data); @@ -539,6 +543,10 @@ function toolGroupActionLabel(action: ToolGroupAction, count: number): string { return `Linked ${count} ${count === 1 ? "pull request" : "pull requests"}`; case "unlink-pr": return `Unlinked ${count} ${count === 1 ? "pull request" : "pull requests"}`; + case "watch-pr": + return `Watching ${count} ${count === 1 ? "pull request" : "pull requests"}`; + case "unwatch-pr": + return `Stopped watching ${count} ${count === 1 ? "pull request" : "pull requests"}`; case "list-prs": return count === 1 ? "Checked linked pull requests" diff --git a/packages/contracts/src/environment.ts b/packages/contracts/src/environment.ts index a649cfe38de0..6752fa2a62ee 100644 --- a/packages/contracts/src/environment.ts +++ b/packages/contracts/src/environment.ts @@ -162,6 +162,8 @@ export const ExecutionEnvironmentCapabilities = Schema.Struct({ shaping and validation when this is absent. */ serverResolvedCommandContext: Schema.optionalKey(Schema.Boolean), threadPullRequests: Schema.optionalKey(Schema.Boolean), + /** Server understands thread.pull-request.watch and wakes agents on pull request changes. */ + threadPullRequestWatch: Schema.optionalKey(Schema.Boolean), pullRequestStackActions: Schema.optionalKey(Schema.Boolean), /** The update path clients should offer for this server. Absent on servers that must be relaunched manually (dev checkouts, Windows diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index 1820008b77dd..c74f64ed09ad 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -46,6 +46,7 @@ import { ThreadPullRequestLinkSource, ThreadPullRequestSnapshot, ThreadPullRequestStack, + ThreadPullRequestWatch, } from "./threadPullRequest.ts"; import { ProviderApprovalDecision, @@ -2600,6 +2601,14 @@ export const OrchestrationV2Command = Schema.Union([ snapshot: ThreadPullRequestSnapshot, stack: Schema.NullOr(ThreadPullRequestStack), }), + /** Start or stop the server watching a linked pull request for this thread's agent. */ + Schema.Struct({ + type: Schema.Literal("thread.pull-request.watch"), + commandId: CommandId, + threadId: ThreadId, + ...ThreadPullRequestKey.fields, + watching: Schema.Boolean, + }), Schema.Struct({ type: Schema.Literal("thread.pull-request.sync"), commandId: CommandId, @@ -2859,6 +2868,14 @@ export type OrchestrationV2Command = typeof OrchestrationV2Command.Type; * send them. */ const OrchestrationV2InternalCommand = Schema.Union([ + /** Records what a pull request watch reported; ignored unless still watched since `startedAt`. */ + Schema.Struct({ + type: Schema.Literal("thread.pull-request-watch.sync"), + commandId: CommandId, + threadId: ThreadId, + ...ThreadPullRequestKey.fields, + watch: ThreadPullRequestWatch, + }), /** Records that the provider rollback `requestId` failed for good. */ Schema.Struct({ type: Schema.Literal("checkpoint.rollback.fail"), diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index cf8c00db0a88..98255b0a546e 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -844,6 +844,8 @@ export const PullRequestDetail = Schema.Struct({ changedFiles: NonNegativeInt, headBranch: TrimmedNonEmptyString, headRepositoryNameWithOwner: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), + /** The head commit, where the host reports it with the detail. */ + headSha: Schema.optional(TrimmedNonEmptyString), baseBranch: TrimmedNonEmptyString, createdAt: IsoDateTime, updatedAt: IsoDateTime, diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 119503c2a85d..6fc860d455df 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -92,6 +92,25 @@ export const ThreadPullRequestKey = Schema.Struct({ }); export type ThreadPullRequestKey = typeof ThreadPullRequestKey.Type; +/** + * Present while the server watches the pull request for its thread. The server wakes the + * thread's agent when checks finish on the head commit, someone else comments, or the branch + * starts to conflict. The other fields record what the agent was last told, so each change is + * reported once. + */ +export const ThreadPullRequestWatch = Schema.Struct({ + startedAt: IsoDateTime, + /** Head commit whose check result was last reported; null until one is. */ + headSha: Schema.NullOr(TrimmedNonEmptyString), + checks: Schema.NullOr(Schema.Literals(["passing", "failing"])), + /** Remarks from others created up to this host time were reported. */ + remarksThrough: IsoDateTime, + conflicting: Schema.Boolean, + /** Wakes since the head commit last moved. Watching stops at a limit, so bots cannot loop it. */ + wakes: NonNegativeInt, +}); +export type ThreadPullRequestWatch = typeof ThreadPullRequestWatch.Type; + export const ThreadPullRequestLink = Schema.Struct({ ...ThreadPullRequestKey.fields, url: TrimmedNonEmptyString, @@ -99,5 +118,6 @@ export const ThreadPullRequestLink = Schema.Struct({ linkedAt: IsoDateTime, snapshot: Schema.NullOr(ThreadPullRequestSnapshot), stack: Schema.NullOr(ThreadPullRequestStack), + watch: Schema.optional(ThreadPullRequestWatch), }); export type ThreadPullRequestLink = typeof ThreadPullRequestLink.Type; diff --git a/packages/shared/src/t3McpToolPresentation.ts b/packages/shared/src/t3McpToolPresentation.ts index dd38709bd0e6..d85104cf4bc1 100644 --- a/packages/shared/src/t3McpToolPresentation.ts +++ b/packages/shared/src/t3McpToolPresentation.ts @@ -55,6 +55,8 @@ export type T3McpToolSummaryAction = | "link-pr" | "unlink-pr" | "list-prs" + | "watch-pr" + | "unwatch-pr" | "browser" | "device"; @@ -93,6 +95,16 @@ const T3_MCP_TOOLS: Readonly> = { "list-prs", "pull-request", ), + watch_pull_request: tool( + ["Watch", "Watching", "Watching", "a pull request"], + "watch-pr", + "pull-request", + ), + unwatch_pull_request: tool( + ["Stop watching", "Stopping watching", "Stopped watching", "a pull request"], + "unwatch-pr", + "pull-request", + ), orchestrator_capabilities: tool( ["Get", "Getting", "Got", "orchestration capabilities"], "capabilities", From 22c56fcc13245a38940f62c7bd4572ca98180689 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 22:46:20 -0700 Subject: [PATCH 02/13] fix(server): record pull request watch progress and wakes atomically Co-Authored-By: Claude Opus 5.5 (1M context) --- .../toolkits/pullRequests/handlers.test.ts | 1 + .../src/orchestration-v2/Orchestrator.ts | 64 +++++++- .../ProjectionSettlement.test.ts | 1 + .../PullRequestWatchReactor.ts | 139 ++++++++++-------- .../orchestration-v2/pullRequestWatch.test.ts | 49 +++++- .../src/orchestration-v2/pullRequestWatch.ts | 67 ++++++--- .../src/orchestration-v2/runtimeLayer.test.ts | 63 ++++++-- docs/user/source-control.md | 2 +- packages/contracts/src/orchestrationV2.ts | 17 ++- packages/contracts/src/threadPullRequest.ts | 4 +- 10 files changed, 293 insertions(+), 114 deletions(-) diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts index 9c0b58d7b6da..88d709fd35f9 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts @@ -243,6 +243,7 @@ describe("pull request toolkit handlers", () => { headSha: null, checks: null, remarksThrough: "2026-08-20T00:00:00.000Z", + remarkIds: [], conflicting: false, wakes: 0, }; diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 8b76303d8f79..a1322437195c 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -2201,6 +2201,61 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }); }); + // Checked under the thread lock: the watch or the thread can change while the host is read. + // The watch is recorded first so the wake's own thread events carry it. + const dispatchPullRequestWatchSync = Effect.fn("orchestrationV2.dispatch.pullRequestWatchSync")( + function* ( + command: Extract< + OrchestrationV2ServerCommand, + { readonly type: "thread.pull-request-watch.sync" } + >, + events: Ref.Ref>, + effects: Ref.Ref>, + ) { + const thread = yield* projectionStore + .getThread(command.threadId) + .pipe( + Effect.mapError( + (cause) => new OrchestratorProjectionError({ threadId: command.threadId, cause }), + ), + ); + const key = normalizeThreadPullRequestKey(command); + const link = threadPullRequestsOf(thread).find( + (candidate) => + candidate.source !== "stack-dismissed" && threadPullRequestKeysEqual(candidate, key), + ); + const inactive = + thread.archivedAt !== null || + thread.settledOverride === "settled" || + thread.settledAt !== null; + if (link?.watch?.startedAt !== command.startedAt || (command.wake && inactive)) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "The pull request watch ended or its thread settled while it was read.", + }); + } + yield* dispatchThreadMutation(command, events, effects); + if (command.wake === undefined) return; + yield* dispatchMessage( + { + type: "message.dispatch", + commandId: command.commandId, + threadId: command.threadId, + messageId: command.wake.messageId, + text: command.wake.text, + notification: command.wake.notification, + attachments: [], + dispatchMode: { type: "queue_after_active" }, + createdBy: "agent", + creationSource: "server", + }, + events, + effects, + ); + }, + ); + const dispatchThreadMutation = Effect.fn("orchestrationV2.dispatch.threadMutation")(function* ( command: Extract< OrchestrationV2ServerCommand, @@ -2862,8 +2917,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio const watch = command.type === "thread.pull-request-watch.sync" ? // Progress read before a stop or restart must not bring the old watch back. - existing.watch?.startedAt === command.watch.startedAt - ? command.watch + existing.watch?.startedAt === command.startedAt + ? (command.watch ?? undefined) : existing.watch : !command.watching ? undefined @@ -2872,6 +2927,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio headSha: null, checks: null, remarksThrough: startedAt, + remarkIds: [], conflicting: false, wakes: 0, }); @@ -9186,7 +9242,6 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio case "thread.pull-request.unlink": case "thread.pull-request-link.sync": case "thread.pull-request.watch": - case "thread.pull-request-watch.sync": case "thread.pull-request.sync": case "thread.title.regeneration.complete": case "thread.runtime-mode.set": @@ -9195,6 +9250,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio case "provider.switch": yield* dispatchThreadMutation(command, events, effects); break; + case "thread.pull-request-watch.sync": + yield* dispatchPullRequestWatchSync(command, events, effects); + break; case "provider-session.detach": yield* dispatchProviderSessionDetach(command, events, effects); break; diff --git a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts index b2c320f08f69..60db5e59485e 100644 --- a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts @@ -353,6 +353,7 @@ it.effect.each([ headSha: null, checks: null, remarksThrough: DateTime.formatIso(old), + remarkIds: [], conflicting: false, wakes: 0, }; diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 464b9b551db6..626a643e96da 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -1,6 +1,9 @@ import { CommandId, MessageId, + type OrchestrationV2Notification, + type PullRequestComment, + type PullRequestThreadCommentsResult, type ThreadPullRequestLink, type ThreadPullRequestWatch, } from "@t3tools/contracts"; @@ -23,6 +26,9 @@ import * as Orchestrator from "./Orchestrator.ts"; import * as ProjectionStore from "./ProjectionStore.ts"; import { evaluatePullRequestWatch, pullRequestWatchMessage } from "./pullRequestWatch.ts"; +/** Extra comment pages read per review thread; a longer thread leaves the read incomplete. */ +const THREAD_PAGES = 5; + const logFailure = (message: string, fields: Record) => (cause: Cause.Cause): Effect.Effect => @@ -36,6 +42,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.headSha === right.headSha && left.checks === right.checks && left.remarksThrough === right.remarksThrough && + left.remarkIds.join("\n") === right.remarkIds.join("\n") && left.conflicting === right.conflicting && left.wakes === right.wakes ); @@ -74,18 +81,27 @@ export const make = Effect.gen(function* () { repository: link.repository, number: link.number, }; - const stop = crypto.randomUUIDv4.pipe( - Effect.flatMap((uuid) => - engine.dispatch({ - type: "thread.pull-request.watch", - commandId: CommandId.make(`server:pr-watch-stop:${thread.id}:${uuid}`), + // The orchestrator applies this only while the same watch is on, so a stop or restart that + // lands during the host read wins. + const record = ( + next: ThreadPullRequestWatch | null, + wake?: { readonly text: string; readonly notification: OrchestrationV2Notification }, + ) => + Effect.gen(function* () { + const uuid = yield* crypto.randomUUIDv4; + yield* engine.dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make(`server:pr-watch:${thread.id}:${uuid}`), threadId: thread.id, ...pullRequest, - watching: false, - }), - ), - ); - if (link.snapshot !== null && link.snapshot.state !== "open") return yield* stop; + startedAt: watch.startedAt, + watch: next, + ...(wake === undefined + ? {} + : { wake: { ...wake, messageId: MessageId.make(`message:pr-watch:${uuid}`) } }), + }); + }); + if (link.snapshot !== null && link.snapshot.state !== "open") return yield* record(null); if (thread.settledOverride === "settled" || thread.settledAt !== null) return; const reference = { projectId: thread.projectId, ...pullRequest }; @@ -93,63 +109,58 @@ export const make = Effect.gen(function* () { [pullRequests.detail({ ...reference, allowStale: false }), pullRequests.activity(reference)], { concurrency: 2 }, ); - if (detail.state !== "open") return yield* stop; + if (detail.state !== "open") return yield* record(null); + + // GitHub sends the first comments of each review thread; the rest are read here so a late + // reply in a long thread still counts. + const longThreads = activity.reviewThreads.filter( + (reviewThread) => reviewThread.nextCommentsCursor !== undefined, + ); + const rest = yield* Effect.forEach( + longThreads, + (reviewThread) => + Effect.gen(function* () { + const comments: Array = []; + let cursor: string | null | undefined = reviewThread.nextCommentsCursor; + for (let page = 0; cursor != null && page < THREAD_PAGES; page += 1) { + const result: PullRequestThreadCommentsResult = yield* pullRequests.threadComments({ + ...reference, + threadId: reviewThread.id, + cursor, + }); + for (const comment of result.comments) { + comments.push({ + ...comment, + kind: "review-comment", + path: reviewThread.path, + reviewState: null, + }); + } + cursor = result.nextCursor; + } + return { comments, whole: cursor == null }; + }), + { concurrency: 2 }, + ); + // A truncated conversation with no long thread to explain it is a degraded read. + const whole = + (!activity.commentsTruncated || longThreads.length > 0) && rest.every((read) => read.whole); + const remarks = whole ? [...activity.comments, ...rest.flatMap((read) => read.comments)] : null; - const report = evaluatePullRequestWatch(watch, detail, activity); + const report = evaluatePullRequestWatch(watch, detail, remarks); if (report.changes.length > 0) { - const { text, notification } = pullRequestWatchMessage({ - number: link.number, - url: link.url, - baseBranch: detail.baseBranch, - headSha: report.next.headSha, - report, - }); - // Derived from the watch it records, so a pass that repeats after a failed record below - // replays this command's receipt instead of waking the agent twice. A wake the - // orchestrator refuses is logged and still recorded, so the watch cannot stall on it. - const next = report.next; - const wakeId = [ - thread.id, - threadPullRequestKeyOf(link), - Date.parse(next.startedAt), - next.wakes, - next.headSha ?? "-", - next.checks ?? "-", - Date.parse(next.remarksThrough), - next.conflicting ? 1 : 0, - ].join(":"); - yield* engine - .dispatch({ - type: "message.dispatch", - commandId: CommandId.make(`server:pr-watch:${wakeId}`), - threadId: thread.id, - messageId: MessageId.make(`message:pr-watch:${wakeId}`), - text, - notification, - attachments: [], - dispatchMode: { type: "queue_after_active" }, - createdBy: "agent", - creationSource: "server", - }) - .pipe( - Effect.catchCause( - logFailure("pull request watch wake failed", { - threadId: thread.id, - pullRequest: threadPullRequestKeyOf(link), - }), - ), - ); + return yield* record( + report.exhausted ? null : report.next, + pullRequestWatchMessage({ + number: link.number, + url: link.url, + baseBranch: detail.baseBranch, + headSha: report.next.headSha, + report, + }), + ); } - if (report.exhausted) return yield* stop; - if (watchesEqual(report.next, watch)) return; - const uuid = yield* crypto.randomUUIDv4; - yield* engine.dispatch({ - type: "thread.pull-request-watch.sync", - commandId: CommandId.make(`server:pr-watch-sync:${thread.id}:${uuid}`), - threadId: thread.id, - ...pullRequest, - watch: report.next, - }); + if (!watchesEqual(report.next, watch)) yield* record(report.next); }); const sweep = Effect.gen(function* () { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 91338114a1b3..ea537ad3b150 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, checks: null, remarksThrough: STARTED, + remarkIds: [], conflicting: false, wakes: 0, ...overrides, @@ -57,7 +58,7 @@ const remark = ( reviewState: null, }); -const noRemarks = { comments: [] }; +const noRemarks: ReadonlyArray = []; describe("evaluatePullRequestWatch", () => { it("reports a check result once per head commit", () => { @@ -86,16 +87,52 @@ describe("evaluatePullRequestWatch", () => { ]); }); + it("reports a new run as news on hosts that send no head commit", () => { + const passing = detail({ headSha: undefined, checks: [check("ci", "success")] }); + const passed = evaluatePullRequestWatch(watch({ wakes: 3 }), passing, noRemarks); + assert.equal(passed.changes[0]?.kind, "checks-passed"); + const rerun = evaluatePullRequestWatch( + passed.next, + detail({ headSha: undefined, checks: [check("ci", "pending")] }), + noRemarks, + ); + assert.deepEqual(rerun.changes, []); + assert.equal(rerun.next.wakes, 0); + assert.equal( + evaluatePullRequestWatch(rerun.next, passing, noRemarks).changes[0]?.kind, + "checks-passed", + ); + }); + + 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); + assert.deepEqual(partial.changes, []); + assert.equal( + evaluatePullRequestWatch(partial.next, detail(), comments).changes[0]?.kind, + "remarks", + ); + }); + + it("reports a remark that shows up late with the same time as a reported one", () => { + const first = remark("reviewer", "2026-10-02T12:06:00Z"); + const late = { ...remark("bot", "2026-10-02T12:06:00Z"), id: "late" }; + const reported = evaluatePullRequestWatch(watch(), detail(), [first]); + const again = evaluatePullRequestWatch(reported.next, detail(), [first, late]); + assert.deepEqual(again.changes, [{ kind: "remarks", remarks: [late] }]); + assert.deepEqual(again.next.remarkIds, [first.id, "late"]); + }); + it("reports remarks from others once and never the agent's own", () => { const comments = [ remark("agent-user", "2026-10-02T12:05:00Z", "Fixed in the latest push."), remark("macroscope-app[bot]", "2026-10-02T12:06:00Z"), remark("reviewer", "2026-10-02T11:00:00Z", "Older than the watch."), ]; - const report = evaluatePullRequestWatch(watch(), detail(), { comments }); + const report = evaluatePullRequestWatch(watch(), detail(), comments); assert.deepEqual(report.changes, [{ kind: "remarks", remarks: [comments[1]!] }]); assert.equal(report.next.remarksThrough, "2026-10-02T12:06:00Z"); - assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), { comments }).changes, []); + assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), comments).changes, []); }); it("reports a conflict once, until the branch is clean again", () => { @@ -121,8 +158,8 @@ describe("evaluatePullRequestWatch", () => { it("stops after the wake limit unless the head moves", () => { const comments = [remark("reviewer", "2026-10-02T12:10:00Z")]; const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); - assert.isTrue(evaluatePullRequestWatch(tired, detail(), { comments }).exhausted); - const pushed = evaluatePullRequestWatch(tired, detail({ headSha: "cccccccccc" }), { comments }); + assert.isTrue(evaluatePullRequestWatch(tired, detail(), comments).exhausted); + const pushed = evaluatePullRequestWatch(tired, detail({ headSha: "cccccccccc" }), comments); assert.isFalse(pushed.exhausted); assert.equal(pushed.next.wakes, 1); }); @@ -133,7 +170,7 @@ describe("pullRequestWatchMessage", () => { const report = evaluatePullRequestWatch( watch(), detail({ checks: [check("lint", "failure")] }), - { comments: [remark("reviewer", "2026-10-02T12:10:00Z", "Needs a test.")] }, + [remark("reviewer", "2026-10-02T12:10:00Z", "Needs a test.")], ); const message = pullRequestWatchMessage({ number: 12, diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 0086fc65d105..080e534484c9 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -1,13 +1,15 @@ import type { OrchestrationV2Notification, - PullRequestActivity, PullRequestCheck, PullRequestComment, PullRequestDetail, ThreadPullRequestWatch, } from "@t3tools/contracts"; -/** Wakes allowed without a new push before watching stops, so a chatty bot cannot loop an agent. */ +/** + * Wakes allowed before checks start over (a push or a rerun), so a chatty bot cannot loop an + * agent that is only replying to it. + */ export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10; const LISTED_ITEMS = 10; const SNIPPET_LENGTH = 200; @@ -43,21 +45,26 @@ function checksOutcome( /** * Compares a watched pull request with what its agent was last told. A check result is - * reported once per head commit, as soon as any check fails or once every check passed. - * Remarks count when someone other than the viewer or the pull request's author wrote them: - * the agent posts as the viewer, so its own replies never wake it. + * reported as soon as any check fails or once every check passed, and again only after the + * head commit moves or the checks start over. Remarks count when someone other than the + * viewer or the pull request's author wrote them: the agent posts as the viewer, so its own + * replies never wake it. `remarks` is null when the conversation could not be read whole; + * remarks are then left for a later pass rather than skipped. */ export function evaluatePullRequestWatch( watch: ThreadPullRequestWatch, detail: Pick, - activity: Pick, + remarks: ReadonlyArray | null, ): PullRequestWatchReport { const changes: Array = []; const headSha = detail.headSha ?? null; - const headMoved = headSha !== watch.headSha; - - let checks = headMoved ? null : watch.checks; const outcome = checksOutcome(detail.checks); + // Hosts that report no head commit still show a push or a rerun as checks starting over. + const restarted = + headSha !== watch.headSha || + (watch.checks !== null && (outcome === "pending" || outcome === null)); + + let checks = restarted ? null : watch.checks; if ((outcome === "failing" || outcome === "passing") && outcome !== checks) { changes.push( outcome === "failing" @@ -70,18 +77,22 @@ export function evaluatePullRequestWatch( const own = new Set( [detail.viewer, detail.author?.login].flatMap((login) => (login ? [login.toLowerCase()] : [])), ); - const reportedThrough = Date.parse(watch.remarksThrough); - const remarks = activity.comments.filter( - (remark) => - Date.parse(remark.createdAt) > reportedThrough && - !own.has(remark.author?.login.toLowerCase() ?? ""), - ); - if (remarks.length > 0) changes.push({ kind: "remarks", remarks }); - const remarksThrough = remarks.reduce( - (latest, remark) => - Date.parse(remark.createdAt) > Date.parse(latest) ? remark.createdAt : latest, - watch.remarksThrough, - ); + const through = Date.parse(watch.remarksThrough); + const fresh = (remarks ?? []).filter((remark) => { + const at = Date.parse(remark.createdAt); + return ( + (at > through || (at === through && !watch.remarkIds.includes(remark.id))) && + !own.has(remark.author?.login.toLowerCase() ?? "") + ); + }); + if (fresh.length > 0) changes.push({ kind: "remarks", remarks: fresh }); + const latest = Math.max(through, ...fresh.map((remark) => Date.parse(remark.createdAt))); + const atLatest = fresh.filter((remark) => Date.parse(remark.createdAt) === latest); + const remarksThrough = latest === through ? watch.remarksThrough : atLatest[0]!.createdAt; + const remarkIds = [ + ...(latest === through ? watch.remarkIds : []), + ...atLatest.map((remark) => remark.id), + ]; if (detail.mergeability === "conflicting" && !watch.conflicting) { changes.push({ kind: "conflicting" }); @@ -90,10 +101,18 @@ export function evaluatePullRequestWatch( const conflicting = detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; - const wakes = (headMoved ? 0 : watch.wakes) + (changes.length > 0 ? 1 : 0); + const wakes = (restarted ? 0 : watch.wakes) + (changes.length > 0 ? 1 : 0); return { changes, - next: { startedAt: watch.startedAt, headSha, checks, remarksThrough, conflicting, wakes }, + next: { + startedAt: watch.startedAt, + headSha, + checks, + remarksThrough, + remarkIds, + conflicting, + wakes, + }, exhausted: changes.length > 0 && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT, }; } @@ -166,7 +185,7 @@ export function pullRequestWatchMessage(input: { ...changes.flatMap((change) => changeLines(change, context)), "", exhausted - ? `T3 Code stopped watching after ${PULL_REQUEST_WATCH_WAKE_LIMIT} updates without a new push. Call watch_pull_request to watch it again.` + ? `T3 Code stopped watching after ${PULL_REQUEST_WATCH_WAKE_LIMIT} updates without a new push or check run. Call watch_pull_request to watch it again.` : "Look into each item and act on it as your task requires. T3 Code keeps watching and wakes you on the next change, so end your turn when you are done. Call unwatch_pull_request when you no longer need updates.", ].join("\n"); const failed = changes.some( diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index a7a4ebda59c7..fdf4464f8977 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2203,6 +2203,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { commandId: CommandId.make("pr-watch-record"), threadId, ...key, + startedAt: started.startedAt, watch: recorded, }); assert.deepEqual(yield* watchOf, recorded); @@ -2216,19 +2217,30 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { ...key, watching: false, }); - // Progress read before the stop must not bring the watch back. - yield* orchestrator.dispatch({ - type: "thread.pull-request-watch.sync", - commandId: CommandId.make("pr-watch-late-record"), - threadId, - ...key, - watch: { ...recorded, wakes: 2 }, - }); + // A wake read before the stop must neither wake the agent nor bring the watch back. + const late = yield* orchestrator + .dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make("pr-watch-late-record"), + threadId, + ...key, + startedAt: started.startedAt, + watch: { ...recorded, wakes: 2 }, + wake: { + messageId: MessageId.make("pr-watch-late-wake"), + text: "Update", + notification: { source: { kind: "monitor" }, outcome: "updated", summary: "#7" }, + }, + }) + .pipe(Effect.flip); + assert.equal(late._tag, "OrchestratorDispatchError"); assert.isUndefined(yield* watchOf); + const { messages } = yield* orchestrator.getThreadRecords(threadId, ["messages"]); + assert.deepEqual(messages, []); }), ); - it.effect("wakes a watched thread once when its pull request's checks fail", () => + it.effect("wakes a watched thread once for failed checks and a late thread reply", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; const threadId = ThreadId.make("runtime-pull-request-watch-wake"); @@ -2325,14 +2337,39 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { NodeServices.layer, Layer.mock(PullRequestService.PullRequestService)({ detail: () => Effect.succeed(detail), + // A long review thread: its later replies only arrive through threadComments. activity: () => Effect.succeed({ comments: [], - commentCount: 0, - commentsTruncated: false, - reviewThreads: [], + commentCount: 11, + commentsTruncated: true, + reviewThreads: [ + { + id: "thread-1", + path: "src/index.ts", + line: 1, + side: "right", + isResolved: false, + isOutdated: false, + comments: [], + nextCommentsCursor: "page-2", + }, + ], commits: [], }), + threadComments: () => + Effect.succeed({ + comments: [ + { + id: "late-reply", + author: { login: "reviewer", name: null, avatarUrl: null }, + body: "One more thing.", + createdAt: "2999-01-01T00:00:00.000Z", + url: null, + }, + ], + nextCursor: null, + }), }), ), ), @@ -2345,7 +2382,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { messages.flatMap((message) => message.notification === undefined ? [] : [message.notification.summary], ), - ["#7: checks failed"], + ["#7: checks failed, new comments"], ); const watch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; assert.deepEqual( diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 9f9fb5c3f14c..b6ea1d95c3f1 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -185,7 +185,7 @@ Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_p the thread is active, the server checks the pull request every minute and wakes the agent when checks finish on the latest commit, someone else comments or reviews, or the branch starts to conflict. Comments from your own account do not wake it. Watching ends when the pull request merges or closes, -or after 10 wakes without a new push. To start or stop it yourself, use the row menu in the **Linked +or after 10 wakes without a new push or check run. To start or stop it yourself, use the row menu in the **Linked pull requests** panel, or long-press the review in the mobile Git overview. Cross-repository links use a project on the same host. Azure DevOps reviews require a project checked diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index c74f64ed09ad..38e13087aa13 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -2868,13 +2868,26 @@ export type OrchestrationV2Command = typeof OrchestrationV2Command.Type; * send them. */ const OrchestrationV2InternalCommand = Schema.Union([ - /** Records what a pull request watch reported; ignored unless still watched since `startedAt`. */ + /** + * Records what a pull request watch saw, and wakes the agent in the same transaction when + * `wake` is set. Rejected once the watch started at `startedAt` has ended, and a wake is + * rejected on a settled or archived thread, so a read that raced either changes nothing. + */ Schema.Struct({ type: Schema.Literal("thread.pull-request-watch.sync"), commandId: CommandId, threadId: ThreadId, ...ThreadPullRequestKey.fields, - watch: ThreadPullRequestWatch, + startedAt: IsoDateTime, + /** The watch to record, or null to end it. */ + watch: Schema.NullOr(ThreadPullRequestWatch), + wake: Schema.optional( + Schema.Struct({ + messageId: MessageId, + text: Schema.String, + notification: OrchestrationV2Notification, + }), + ), }), /** Records that the provider rollback `requestId` failed for good. */ Schema.Struct({ diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 6fc860d455df..cf309a5c7fe8 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -105,8 +105,10 @@ export const ThreadPullRequestWatch = Schema.Struct({ checks: Schema.NullOr(Schema.Literals(["passing", "failing"])), /** 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. */ + remarkIds: Schema.Array(TrimmedNonEmptyString), conflicting: Schema.Boolean, - /** Wakes since the head commit last moved. Watching stops at a limit, so bots cannot loop it. */ + /** Wakes since checks last started over. Watching stops at a limit, so bots cannot loop it. */ wakes: NonNegativeInt, }); export type ThreadPullRequestWatch = typeof ThreadPullRequestWatch.Type; From f169f2e9fd52bee51c2eec179ecdf3c4426f785b Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 23:19:48 -0700 Subject: [PATCH 03/13] fix(server): watch links and starts in one command, keeps watches on re-link Co-Authored-By: Claude Opus 5.5 (1M context) --- .../toolkits/pullRequests/handlers.test.ts | 18 ++--- .../src/mcp/toolkits/pullRequests/handlers.ts | 47 ++++++------- .../src/orchestration-v2/Orchestrator.ts | 69 +++++++++++++++---- .../PullRequestWatchReactor.ts | 26 ++++++- .../src/orchestration-v2/runtimeLayer.test.ts | 24 ++++--- .../pullRequest/ThreadPullRequestsPanel.tsx | 6 +- packages/contracts/src/orchestrationV2.ts | 4 ++ 7 files changed, 130 insertions(+), 64 deletions(-) diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts index 88d709fd35f9..e7c85dfb3a30 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts @@ -228,15 +228,20 @@ describe("pull request toolkit handlers", () => { const result = yield* harness.call("watch_pull_request", { url: "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/t3tools/t3code/pull/9", }); - expect(result).toMatchObject({ number: 9, watching: true, wasWatching: false }); + // The harness thread never changes, so the result reports what it still holds. + expect(result).toMatchObject({ number: 9, watching: false, wasWatching: false }); expect(yield* Ref.get(harness.commands)).toMatchObject([ - { type: "thread.pull-request.link", number: 9, source: "agent" }, - { type: "thread.pull-request.watch", number: 9, watching: true }, + { + type: "thread.pull-request.watch", + number: 9, + watching: true, + link: { url: "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/t3tools/t3code/pull/9", source: "agent" }, + }, ]); }), ); - it.effect("refuses to watch a merged pull request and stops only an existing watch", () => + it.effect("refuses to watch a merged pull request and stops an existing watch", () => Effect.gen(function* () { const watch = { startedAt: "2026-08-20T00:00:00.000Z", @@ -259,12 +264,9 @@ describe("pull request toolkit handlers", () => { .call("watch_pull_request", { repository: "t3tools/t3code", number: 1 }) .pipe(Effect.flip); expect(error).toMatchObject({ _tag: "PullRequestNotOpenError", state: "merged" }); - expect( - yield* harness.call("unwatch_pull_request", { repository: "t3tools/t3code", number: 2 }), - ).toMatchObject({ watching: false, wasWatching: false }); expect( yield* harness.call("unwatch_pull_request", { repository: "t3tools/t3code", number: 3 }), - ).toMatchObject({ watching: false, wasWatching: true }); + ).toMatchObject({ wasWatching: true }); expect(yield* Ref.get(harness.commands)).toMatchObject([ { type: "thread.pull-request.watch", number: 3, watching: false }, ]); diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.ts index aac480dd6450..97af65e1a7bc 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.ts @@ -209,7 +209,10 @@ const make = Effect.gen(function* () { ? Effect.failCause(cause as Cause.Cause) : Effect.fail(new Failure({ cause })); - /** Starts or stops a watch on a linked pull request; `watch_pull_request` links it first. */ + /** + * Starts or stops a watch. One command links an unlinked pull request and watches it, and the + * result reports the state the thread holds afterwards. + */ const setWatching = Effect.fn("PullRequestsToolkit.setWatching")(function* ( input: PullRequestTargetInput, watching: boolean, @@ -217,34 +220,15 @@ const make = Effect.gen(function* () { const thread = yield* requireThread(PullRequestWatchFailedError); const project = yield* projectOf(thread, PullRequestWatchFailedError); const target = yield* resolveTarget(input, project); - const identity = { - host: target.host, - repository: target.repository, - number: target.number, - url: target.url, - }; - const existing = threadPullRequestsOf(thread).find( - (link) => link.source !== "stack-dismissed" && threadPullRequestKeysEqual(link, target), - ); - const wasWatching = existing?.watch !== undefined; - if (wasWatching === watching || (!watching && existing === undefined)) { - return { ...identity, watching: wasWatching, wasWatching }; - } - const state = existing?.snapshot?.state; + const watchedLink = (shell: OrchestrationV2ThreadShell) => + threadPullRequestsOf(shell).find( + (link) => link.source !== "stack-dismissed" && threadPullRequestKeysEqual(link, target), + ); + const before = watchedLink(thread); + const state = before?.snapshot?.state; if (watching && state !== undefined && state !== "open") { return yield* new PullRequestNotOpenError({ state }); } - if (existing === undefined) { - yield* engine - .dispatch({ - type: "thread.pull-request.link", - commandId: yield* commandId("mcp-pr-link", thread.id), - threadId: thread.id, - ...identity, - source: "agent", - }) - .pipe(Effect.catchCause(dispatchFailure(PullRequestWatchFailedError))); - } yield* engine .dispatch({ type: "thread.pull-request.watch", @@ -254,9 +238,18 @@ const make = Effect.gen(function* () { repository: target.repository, number: target.number, watching, + ...(watching ? { link: { url: target.url, source: "agent" as const } } : {}), }) .pipe(Effect.catchCause(dispatchFailure(PullRequestWatchFailedError))); - return { ...identity, watching, wasWatching }; + const after = yield* requireThread(PullRequestWatchFailedError); + return { + host: target.host, + repository: target.repository, + number: target.number, + url: target.url, + watching: watchedLink(after)?.watch !== undefined, + wasWatching: before?.watch !== undefined, + }; }); return PullRequestsToolkit.of({ diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index a1322437195c..83bb463cc24b 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -2224,10 +2224,12 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio (candidate) => candidate.source !== "stack-dismissed" && threadPullRequestKeysEqual(candidate, key), ); + // Same rule as a direct message.dispatch: a provider-native subagent takes no messages. const inactive = thread.archivedAt !== null || thread.settledOverride === "settled" || - thread.settledAt !== null; + thread.settledAt !== null || + isProviderNativeSubagentThread(thread); if (link?.watch?.startedAt !== command.startedAt || (command.wake && inactive)) { return yield* new OrchestratorDispatchError({ commandId: command.commandId, @@ -2306,6 +2308,16 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio cause: `Thread ${command.threadId} is deleted.`, }); } + if ( + command.type === "thread.pull-request.watch" && + command.watching && + isProviderNativeSubagentThread(thread) + ) { + return yield* new OrchestratorSubagentThreadReadOnlyError({ + commandId: command.commandId, + threadId: command.threadId, + }); + } if ( command.type === "thread.metadata.update" && command.expectedWorktreePath !== undefined && @@ -2790,14 +2802,23 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ), ...(command.linkedPullRequest ? [ - { - ...legacyThreadPullRequestKey(command.linkedPullRequest), - url: command.linkedPullRequest.url, - source: "manual" as const, - linkedAt: DateTime.formatIso(now), - snapshot: null, - stack: null, - }, + withPullRequestWatch( + { + ...legacyThreadPullRequestKey(command.linkedPullRequest), + url: command.linkedPullRequest.url, + source: "manual" as const, + linkedAt: DateTime.formatIso(now), + snapshot: null, + stack: null, + }, + // Re-linking the same pull request keeps its watch. + threadPullRequestsOf(thread).find((link) => + threadPullRequestKeysEqual( + link, + legacyThreadPullRequestKey(command.linkedPullRequest!), + ), + )?.watch, + ), ] : []), ], @@ -2908,12 +2929,30 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio case "thread.pull-request.watch": case "thread.pull-request-watch.sync": { const key = normalizeThreadPullRequestKey(command); - const links = threadPullRequestsOf(thread); - const existing = links.find( - (link) => link.source !== "stack-dismissed" && threadPullRequestKeysEqual(link, key), - ); - if (existing === undefined) return thread; const startedAt = DateTime.formatIso(now); + const linked = threadPullRequestsOf(thread); + const visible = (link: ThreadPullRequestLink) => + link.source !== "stack-dismissed" && threadPullRequestKeysEqual(link, key); + // A watch started on an unlinked (or dismissed) pull request links it in the same step. + const links = + command.type === "thread.pull-request.watch" && + command.watching && + command.link !== undefined && + !linked.some(visible) + ? [ + ...linked.filter((link) => !threadPullRequestKeysEqual(link, key)), + { + ...key, + url: command.link.url, + source: command.link.source, + linkedAt: startedAt, + snapshot: null, + stack: null, + }, + ] + : linked; + const existing = links.find(visible); + if (existing === undefined) return thread; const watch = command.type === "thread.pull-request-watch.sync" ? // Progress read before a stop or restart must not bring the old watch back. @@ -2931,7 +2970,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio conflicting: false, wakes: 0, }); - if (watch === existing.watch) return thread; + if (watch === existing.watch && links === linked) return thread; return { ...thread, pullRequests: links.map((link) => diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 626a643e96da..d6175b9b5813 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -34,7 +34,7 @@ const logFailure = (cause: Cause.Cause): Effect.Effect => Cause.hasInterruptsOnly(cause) ? Effect.interrupt - : Effect.logWarning(message, { ...fields, cause: Cause.pretty(cause) }); + : Effect.logWarning(message, { ...fields, cause }); function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatch): boolean { return ( @@ -139,7 +139,13 @@ export const make = Effect.gen(function* () { cursor = result.nextCursor; } return { comments, whole: cursor == null }; - }), + }).pipe( + // A failed page only leaves the remarks for a later pass; checks still count now. + Effect.catchCauseIf( + (cause) => !Cause.hasInterruptsOnly(cause), + () => Effect.succeed({ comments: [], whole: false }), + ), + ), { concurrency: 2 }, ); // A truncated conversation with no long thread to explain it is a degraded read. @@ -147,7 +153,21 @@ export const make = Effect.gen(function* () { (!activity.commentsTruncated || longThreads.length > 0) && rest.every((read) => read.whole); const remarks = whole ? [...activity.comments, ...rest.flatMap((read) => read.comments)] : null; - const report = evaluatePullRequestWatch(watch, detail, remarks); + // GitHub names the head commit; elsewhere the newest commit stands in, so a push still reads + // as one. Azure DevOps reports neither, and relies on checks starting over. + const newest = activity.commits.reduce<(typeof activity.commits)[number] | undefined>( + (latest, commit) => + latest === undefined || Date.parse(commit.committedDate) > Date.parse(latest.committedDate) + ? commit + : latest, + undefined, + ); + const headSha = detail.headSha ?? newest?.oid; + const report = evaluatePullRequestWatch( + watch, + { ...detail, ...(headSha === undefined ? {} : { headSha }) }, + remarks, + ); if (report.changes.length > 0) { return yield* record( report.exhausted ? null : report.next, diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index fdf4464f8977..23200f111c49 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2173,30 +2173,38 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { worktreePath: null, }); const key = { host: "github.com", repository: "pingdotgg/t3code", number: 7 }; - yield* orchestrator.dispatch({ - type: "thread.pull-request.link", - commandId: CommandId.make("pr-watch-link"), - threadId, - ...key, - url: "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/pingdotgg/t3code/pull/7", - source: "agent", - }); + const url = "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/pingdotgg/t3code/pull/7"; const watchOf = Effect.map( orchestrator.getThreadShell(threadId), (thread) => thread?.pullRequests?.[0]?.watch, ); + // Watching an unlinked pull request links it in the same command. yield* orchestrator.dispatch({ type: "thread.pull-request.watch", commandId: CommandId.make("pr-watch-start"), threadId, ...key, watching: true, + link: { url, source: "agent" }, }); + assert.equal( + (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.source, + "agent", + ); const started = yield* watchOf; assert.isDefined(started); if (started === undefined) return; + // A legacy client re-linking the same pull request keeps its watch. + yield* orchestrator.dispatch({ + type: "thread.metadata.update", + commandId: CommandId.make("pr-watch-legacy-relink"), + threadId, + linkedPullRequest: { projectId: ProjectId.make("pr-watch-project"), ...key, url }, + }); + assert.deepEqual(yield* watchOf, started); + const recorded = { ...started, headSha: "abc123", checks: "failing" as const, wakes: 1 }; yield* orchestrator.dispatch({ type: "thread.pull-request-watch.sync", diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx index cfd42e2c532a..643c17d4b775 100644 --- a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx +++ b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx @@ -128,7 +128,7 @@ function LinkRow({ } title={snapshot?.title ?? link.repository} signals={ - snapshot?.state === "open" ? ( + open ? ( <> {watching ? ( @@ -141,8 +141,8 @@ function LinkRow({ ) : null} - {snapshot.checksState ? : null} - {snapshot.reviewDecision ? ( + {snapshot?.checksState ? : null} + {snapshot?.reviewDecision ? ( ) : null} diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index 38e13087aa13..6004ab119917 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -2608,6 +2608,10 @@ export const OrchestrationV2Command = Schema.Union([ threadId: ThreadId, ...ThreadPullRequestKey.fields, watching: Schema.Boolean, + /** Links the pull request first when starting a watch on one the thread has not linked. */ + link: Schema.optional( + Schema.Struct({ url: TrimmedNonEmptyString, source: ThreadPullRequestLinkSource }), + ), }), Schema.Struct({ type: Schema.Literal("thread.pull-request.sync"), From 2f43c809e311ec303b2e996fee85fa43a9f4ea1d Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 23:23:42 -0700 Subject: [PATCH 04/13] fix(server): a closed pull request's watch waits for the host before ending Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/orchestration-v2/PullRequestWatchReactor.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index d6175b9b5813..c37084087b11 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -101,7 +101,9 @@ export const make = Effect.gen(function* () { : { wake: { ...wake, messageId: MessageId.make(`message:pr-watch:${uuid}`) } }), }); }); - if (link.snapshot !== null && link.snapshot.state !== "open") return yield* record(null); + // A merged pull request cannot reopen, so its watch ends without a host read, even on a + // settled thread. A closed one can, so the host decides below. + if (link.snapshot?.state === "merged") return yield* record(null); if (thread.settledOverride === "settled" || thread.settledAt !== null) return; const reference = { projectId: thread.projectId, ...pullRequest }; From 7e076261d4d7a242203589c2ae54dae459d14752 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 23:26:00 -0700 Subject: [PATCH 05/13] fix(server): watched PRs report the final check result and action-required checks Co-Authored-By: Claude Opus 5.5 (1M context) --- .../orchestration-v2/pullRequestWatch.test.ts | 25 +++++++++- .../src/orchestration-v2/pullRequestWatch.ts | 48 +++++++++++++------ .../src/orchestration-v2/runtimeLayer.test.ts | 2 +- packages/contracts/src/threadPullRequest.ts | 3 +- 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index ea537ad3b150..b7393977a4d5 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -69,10 +69,24 @@ describe("evaluatePullRequestWatch", () => { const failing = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); const failed = evaluatePullRequestWatch(running.next, failing, noRemarks); assert.deepEqual(failed.changes, [ - { kind: "checks-failed", failed: [check("lint", "failure")] }, + { kind: "checks-failed", failed: [check("lint", "failure")], running: 1 }, ]); assert.deepEqual(evaluatePullRequestWatch(failed.next, failing, noRemarks).changes, []); + // The final result follows once the rest finish, so a second failure is not lost. + const finished = detail({ checks: [check("lint", "failure"), check("test", "failure")] }); + const final = evaluatePullRequestWatch(failed.next, finished, noRemarks); + assert.deepEqual(final.changes, [ + { + kind: "checks-failed", + failed: [check("lint", "failure"), check("test", "failure")], + running: 0, + }, + ]); + // Rerunning one of them is not news until the rerun finishes. + const rerunning = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); + assert.deepEqual(evaluatePullRequestWatch(final.next, rerunning, noRemarks).changes, []); + // A push that fails the same way before a pass ever sees it pending is still news. const pushed = detail({ ...failing, headSha: "bbbbbbbbbb" }); assert.equal( @@ -87,6 +101,15 @@ describe("evaluatePullRequestWatch", () => { ]); }); + it("reports a check that needs someone's action", () => { + const blocked = detail({ + checks: [check("lint", "success"), check("deploy", "action-required")], + }); + assert.deepEqual(evaluatePullRequestWatch(watch(), blocked, noRemarks).changes, [ + { kind: "checks-failed", failed: [check("deploy", "action-required")], running: 0 }, + ]); + }); + it("reports a new run as news on hosts that send no head commit", () => { const passing = detail({ headSha: undefined, checks: [check("ci", "success")] }); const passed = evaluatePullRequestWatch(watch({ wakes: 3 }), passing, noRemarks); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 080e534484c9..5c11eff6d9f1 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -15,7 +15,12 @@ const LISTED_ITEMS = 10; const SNIPPET_LENGTH = 200; export type PullRequestWatchChange = - | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } + | { + readonly kind: "checks-failed"; + readonly failed: ReadonlyArray; + /** Checks still running; a later report follows once they finish. */ + readonly running: number; + } | { readonly kind: "checks-passed"; readonly count: number } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "conflicting" }; @@ -29,18 +34,18 @@ export interface PullRequestWatchReport { readonly exhausted: boolean; } +// "action-required" is a finished check that needs someone, so the agent hears about it. const isFailedCheck = (check: PullRequestCheck) => - check.status === "failure" || check.status === "cancelled"; + check.status === "failure" || check.status === "cancelled" || check.status === "action-required"; +const isRunningCheck = (check: PullRequestCheck) => check.status === "pending"; function checksOutcome( checks: ReadonlyArray, -): "passing" | "failing" | "pending" | null { +): "passing" | "failing" | "failed" | "pending" | null { if (checks.length === 0) return null; - if (checks.some(isFailedCheck)) return "failing"; - if (checks.some((check) => check.status === "pending" || check.status === "action-required")) { - return "pending"; - } - return "passing"; + const running = checks.some(isRunningCheck); + if (checks.some(isFailedCheck)) return running ? "failing" : "failed"; + return running ? "pending" : "passing"; } /** @@ -65,11 +70,22 @@ export function evaluatePullRequestWatch( (watch.checks !== null && (outcome === "pending" || outcome === null)); let checks = restarted ? null : watch.checks; - if ((outcome === "failing" || outcome === "passing") && outcome !== checks) { + // An early failure is reported while other checks run, and the final result once they finish. + // A rerun that leaves another failure in place is not news until it finishes. + if ( + outcome !== null && + outcome !== "pending" && + outcome !== checks && + !(checks === "failed" && outcome === "failing") + ) { changes.push( - outcome === "failing" - ? { kind: "checks-failed", failed: detail.checks.filter(isFailedCheck) } - : { kind: "checks-passed", count: detail.checks.length }, + outcome === "passing" + ? { kind: "checks-passed", count: detail.checks.length } + : { + kind: "checks-failed", + failed: detail.checks.filter(isFailedCheck), + running: detail.checks.filter(isRunningCheck).length, + }, ); checks = outcome; } @@ -138,8 +154,12 @@ function changeLines( switch (change.kind) { case "checks-failed": return [ - `- Checks failed${context.commit}:`, - ...listed(change.failed, (check) => ` - ${check.name}${check.url ? ` ${check.url}` : ""}`), + `- Checks failed${context.commit}${change.running > 0 ? ` (${change.running} still running)` : ""}:`, + ...listed( + change.failed, + (check) => + ` - ${check.name}${check.status === "failure" ? "" : ` (${check.status})`}${check.url ? ` ${check.url}` : ""}`, + ), ]; case "checks-passed": return [ diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 23200f111c49..16b0ea75203d 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2395,7 +2395,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { const watch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; assert.deepEqual( { headSha: watch?.headSha, checks: watch?.checks, wakes: watch?.wakes }, - { headSha: "abc1234def", checks: "failing", wakes: 1 }, + { headSha: "abc1234def", checks: "failed", wakes: 1 }, ); }), ); diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index cf309a5c7fe8..f8e9f72a3448 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -102,7 +102,8 @@ export const ThreadPullRequestWatch = Schema.Struct({ startedAt: IsoDateTime, /** Head commit whose check result was last reported; null until one is. */ headSha: Schema.NullOr(TrimmedNonEmptyString), - checks: Schema.NullOr(Schema.Literals(["passing", "failing"])), + /** Last reported result: `failing` while other checks still run, `failed` once all finished. */ + checks: Schema.NullOr(Schema.Literals(["passing", "failing", "failed"])), /** 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. */ From b130195cbe7f1e0be6b3b94b8aa0911ddd983cba Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 23:27:52 -0700 Subject: [PATCH 06/13] fix(server): a partial check rerun reports its final result Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/orchestration-v2/pullRequestWatch.test.ts | 9 +++++++-- apps/server/src/orchestration-v2/pullRequestWatch.ts | 11 ++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index b7393977a4d5..bf084326e15d 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -83,9 +83,14 @@ describe("evaluatePullRequestWatch", () => { running: 0, }, ]); - // Rerunning one of them is not news until the rerun finishes. + // Rerunning one of them is not news until the rerun finishes, and then it is. const rerunning = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); - assert.deepEqual(evaluatePullRequestWatch(final.next, rerunning, noRemarks).changes, []); + const quiet = evaluatePullRequestWatch(final.next, rerunning, noRemarks); + assert.deepEqual(quiet.changes, []); + assert.equal( + evaluatePullRequestWatch(quiet.next, finished, noRemarks).changes[0]?.kind, + "checks-failed", + ); // A push that fails the same way before a pass ever sees it pending is still news. const pushed = detail({ ...failing, headSha: "bbbbbbbbbb" }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 5c11eff6d9f1..752dae7591bf 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -71,13 +71,10 @@ export function evaluatePullRequestWatch( let checks = restarted ? null : watch.checks; // An early failure is reported while other checks run, and the final result once they finish. - // A rerun that leaves another failure in place is not news until it finishes. - if ( - outcome !== null && - outcome !== "pending" && - outcome !== checks && - !(checks === "failed" && outcome === "failing") - ) { + // A rerun that leaves another failure in place is recorded quietly and reported when it ends. + if (checks === "failed" && outcome === "failing") { + checks = "failing"; + } else if (outcome !== null && outcome !== "pending" && outcome !== checks) { changes.push( outcome === "passing" ? { kind: "checks-passed", count: detail.checks.length } From d953b87e05d1eedc512e4818d2d8e2dd39493a5f Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 23:30:33 -0700 Subject: [PATCH 07/13] fix(server): watch wake limit only counts comment-only wakes, names neutral checks Co-Authored-By: Claude Opus 5.5 (1M context) --- .../orchestration-v2/pullRequestWatch.test.ts | 13 ++++++- .../src/orchestration-v2/pullRequestWatch.ts | 37 +++++++++++++++---- .../src/orchestration-v2/runtimeLayer.test.ts | 2 +- docs/user/source-control.md | 2 +- packages/contracts/src/threadPullRequest.ts | 2 +- 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index bf084326e15d..1acbd733e9bd 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -102,7 +102,7 @@ describe("evaluatePullRequestWatch", () => { // A rerun that passes on the same head is news too. const rerun = detail({ checks: [check("lint", "success"), check("test", "success")] }); assert.deepEqual(evaluatePullRequestWatch(failed.next, rerun, noRemarks).changes, [ - { kind: "checks-passed", count: 2 }, + { kind: "checks-passed", count: 2, neutral: [] }, ]); }); @@ -183,6 +183,17 @@ describe("evaluatePullRequestWatch", () => { ]); }); + it("does not spend the comment wake limit on check results", () => { + const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); + const result = evaluatePullRequestWatch( + tired, + detail({ checks: [check("lint", "failure")] }), + noRemarks, + ); + assert.isFalse(result.exhausted); + assert.equal(result.next.wakes, 0); + }); + it("stops after the wake limit unless the head moves", () => { const comments = [remark("reviewer", "2026-10-02T12:10:00Z")]; const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 752dae7591bf..3d4533ed0931 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -7,8 +7,8 @@ import type { } from "@t3tools/contracts"; /** - * Wakes allowed before checks start over (a push or a rerun), so a chatty bot cannot loop an - * agent that is only replying to it. + * Wakes in a row that bring only new comments. A check result, a conflict, or a push resets the + * count, so this only stops a chatty bot looping an agent that is replying to it. */ export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10; const LISTED_ITEMS = 10; @@ -21,7 +21,12 @@ export type PullRequestWatchChange = /** Checks still running; a later report follows once they finish. */ readonly running: number; } - | { readonly kind: "checks-passed"; readonly count: number } + | { + readonly kind: "checks-passed"; + readonly count: number; + /** Finished without a verdict, which on GitLab can mean a manual job waits on someone. */ + readonly neutral: ReadonlyArray; + } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "conflicting" }; @@ -77,7 +82,11 @@ export function evaluatePullRequestWatch( } else if (outcome !== null && outcome !== "pending" && outcome !== checks) { changes.push( outcome === "passing" - ? { kind: "checks-passed", count: detail.checks.length } + ? { + kind: "checks-passed", + count: detail.checks.length, + neutral: detail.checks.filter((check) => check.status === "neutral"), + } : { kind: "checks-failed", failed: detail.checks.filter(isFailedCheck), @@ -114,7 +123,14 @@ export function evaluatePullRequestWatch( const conflicting = detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; - const wakes = (restarted ? 0 : watch.wakes) + (changes.length > 0 ? 1 : 0); + const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks"); + const wakes = commentsOnly + ? (restarted ? 0 : watch.wakes) + 1 + : changes.length > 0 + ? 0 + : restarted + ? 0 + : watch.wakes; return { changes, next: { @@ -126,7 +142,7 @@ export function evaluatePullRequestWatch( conflicting, wakes, }, - exhausted: changes.length > 0 && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT, + exhausted: commentsOnly && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT, }; } @@ -160,7 +176,12 @@ function changeLines( ]; case "checks-passed": return [ - `- All ${change.count} ${change.count === 1 ? "check" : "checks"} passed${context.commit}.`, + `- All ${change.count} ${change.count === 1 ? "check" : "checks"} finished without a failure${context.commit}.`, + ...listed( + change.neutral, + (check) => + ` - ${check.name} (neutral, may be waiting on someone)${check.url ? ` ${check.url}` : ""}`, + ), ]; case "remarks": return [ @@ -202,7 +223,7 @@ export function pullRequestWatchMessage(input: { ...changes.flatMap((change) => changeLines(change, context)), "", exhausted - ? `T3 Code stopped watching after ${PULL_REQUEST_WATCH_WAKE_LIMIT} updates without a new push or check run. Call watch_pull_request to watch it again.` + ? `T3 Code stopped watching after ${PULL_REQUEST_WATCH_WAKE_LIMIT} comment-only updates in a row. Call watch_pull_request to watch it again.` : "Look into each item and act on it as your task requires. T3 Code keeps watching and wakes you on the next change, so end your turn when you are done. Call unwatch_pull_request when you no longer need updates.", ].join("\n"); const failed = changes.some( diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 16b0ea75203d..31db11254659 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2395,7 +2395,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { const watch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; assert.deepEqual( { headSha: watch?.headSha, checks: watch?.checks, wakes: watch?.wakes }, - { headSha: "abc1234def", checks: "failed", wakes: 1 }, + { headSha: "abc1234def", checks: "failed", wakes: 0 }, ); }), ); diff --git a/docs/user/source-control.md b/docs/user/source-control.md index b6ea1d95c3f1..0ad130722e4c 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -185,7 +185,7 @@ Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_p the thread is active, the server checks the pull request every minute and wakes the agent when checks finish on the latest commit, someone else comments or reviews, or the branch starts to conflict. Comments from your own account do not wake it. Watching ends when the pull request merges or closes, -or after 10 wakes without a new push or check run. To start or stop it yourself, use the row menu in the **Linked +or after 10 wakes in a row that bring only comments. To start or stop it yourself, use the row menu in the **Linked pull requests** panel, or long-press the review in the mobile Git overview. Cross-repository links use a project on the same host. Azure DevOps reviews require a project checked diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index f8e9f72a3448..7b63e4f9c66d 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -109,7 +109,7 @@ export const ThreadPullRequestWatch = Schema.Struct({ /** Remarks created exactly at `remarksThrough` that were reported, so a late one still counts. */ remarkIds: Schema.Array(TrimmedNonEmptyString), conflicting: Schema.Boolean, - /** Wakes since checks last started over. Watching stops at a limit, so bots cannot loop it. */ + /** Comment-only wakes in a row. Watching stops at a limit, so bots cannot loop it. */ wakes: NonNegativeInt, }); export type ThreadPullRequestWatch = typeof ThreadPullRequestWatch.Type; From c1b8f4ae6028b6a2e1fe47bd61cfa99adfc7c356 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 2 Oct 2026 23:33:02 -0700 Subject: [PATCH 08/13] fix(server): a partial check rerun restarts the comment wake count Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/orchestration-v2/pullRequestWatch.test.ts | 15 +++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 13 +++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 1acbd733e9bd..d7bf47edae59 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -194,6 +194,21 @@ describe("evaluatePullRequestWatch", () => { assert.equal(result.next.wakes, 0); }); + it("restarts the comment wake count when a partial rerun starts", () => { + const tired = watch({ + headSha: "aaaaaaaaaa", + checks: "failed", + wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1, + }); + const rerun = evaluatePullRequestWatch( + tired, + detail({ checks: [check("lint", "failure"), check("test", "pending")] }), + [remark("reviewer", "2026-10-02T12:10:00Z")], + ); + assert.isFalse(rerun.exhausted); + assert.equal(rerun.next.wakes, 1); + }); + it("stops after the wake limit unless the head moves", () => { const comments = [remark("reviewer", "2026-10-02T12:10:00Z")]; const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 3d4533ed0931..4a89278dc78f 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -77,7 +77,8 @@ export function evaluatePullRequestWatch( let checks = restarted ? null : watch.checks; // An early failure is reported while other checks run, and the final result once they finish. // A rerun that leaves another failure in place is recorded quietly and reported when it ends. - if (checks === "failed" && outcome === "failing") { + const rerun = checks === "failed" && outcome === "failing"; + if (rerun) { checks = "failing"; } else if (outcome !== null && outcome !== "pending" && outcome !== checks) { changes.push( @@ -123,14 +124,10 @@ export function evaluatePullRequestWatch( const conflicting = detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; + // A new check run, a push, or any check or conflict news is progress, so the count restarts. const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks"); - const wakes = commentsOnly - ? (restarted ? 0 : watch.wakes) + 1 - : changes.length > 0 - ? 0 - : restarted - ? 0 - : watch.wakes; + const progress = restarted || rerun || (changes.length > 0 && !commentsOnly); + const wakes = (progress ? 0 : watch.wakes) + (commentsOnly ? 1 : 0); return { changes, next: { From 9cd0c93b735f8046188cce7f0fbda0e46ae20bb8 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 3 Oct 2026 02:03:57 -0700 Subject: [PATCH 09/13] refactor(server): trim pull request watching to the smallest useful version Co-Authored-By: Claude Opus 5.5 (1M context) --- .../features/threads/git/GitOverviewSheet.tsx | 43 +-------- .../threads/git/gitSheetComponents.tsx | 2 - .../ProjectionSettlement.test.ts | 31 ------ .../src/orchestration-v2/ProjectionStore.ts | 60 +++--------- .../ProviderTurnControlService.test.ts | 1 - .../PullRequestWatchReactor.ts | 71 ++------------ .../orchestration-v2/pullRequestWatch.test.ts | 94 ++++--------------- .../src/orchestration-v2/pullRequestWatch.ts | 83 +++++----------- .../src/orchestration-v2/runtimeLayer.test.ts | 34 ++----- docs/user/source-control.md | 10 +- packages/contracts/src/threadPullRequest.ts | 6 +- 11 files changed, 79 insertions(+), 356 deletions(-) diff --git a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx index 78e69e57531c..828c8838e31a 100644 --- a/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx +++ b/apps/mobile/src/features/threads/git/GitOverviewSheet.tsx @@ -8,7 +8,7 @@ import { resolveThreadPullRequestChains, threadPullRequestKeyOf, } from "@t3tools/shared/threadPullRequests"; -import { EnvironmentId, ThreadId, type ThreadPullRequestLink } from "@t3tools/contracts"; +import { EnvironmentId, ThreadId } from "@t3tools/contracts"; import { CommonActions, StackActions, @@ -40,8 +40,6 @@ import { useThreadSelection } from "../../../state/use-thread-selection"; import { useSelectedThreadGitActions } from "../../../state/use-selected-thread-git-actions"; import { useSelectedThreadGitState } from "../../../state/use-selected-thread-git-state"; import { useSelectedThreadWorktree } from "../../../state/use-selected-thread-worktree"; -import { threadEnvironment } from "../../../state/threads"; -import { useAtomCommand } from "../../../state/use-atom-command"; import { vcsEnvironment } from "../../../state/vcs"; import { resolveGitOverviewReviewNavigationAction } from "./git-overview-navigation"; import { MetaCard, SheetListRow, menuItemIconName, statusSummary } from "./gitSheetComponents"; @@ -75,40 +73,6 @@ export function GitOverviewSheet(props: GitOverviewSheetProps) { ), [selectedThread?.pullRequests, supportsLinkedPrSnapshots], ); - const supportsWatch = - selectedEnvironmentRuntime?.serverConfig?.environment.capabilities.threadPullRequestWatch === - true; - const watchPullRequest = useAtomCommand(threadEnvironment.watchPullRequest); - // Long-press on a linked pull request starts or stops waking the agent on its changes. - const confirmWatchToggle = useCallback( - (link: ThreadPullRequestLink) => { - const watching = link.watch !== undefined; - Alert.alert( - watching ? `Stop watching #${link.number}?` : `Watch #${link.number}?`, - watching - ? "The agent will no longer wake for this pull request." - : "The agent wakes when checks finish, someone comments, or the branch conflicts.", - [ - { text: "Cancel", style: "cancel" }, - { - text: watching ? "Stop watching" : "Watch", - onPress: () => - void watchPullRequest({ - environmentId, - input: { - threadId, - host: link.host, - repository: link.repository, - number: link.number, - watching: !watching, - }, - }), - }, - ], - ); - }, - [environmentId, threadId, watchPullRequest], - ); const gitState = useSelectedThreadGitState(); const gitActions = useSelectedThreadGitActions(); const theme = useUniwindTheme(); @@ -376,11 +340,6 @@ export function GitOverviewSheet(props: GitOverviewSheetProps) { icon="arrow.triangle.pull" title={`#${link.number} ${link.snapshot?.title ?? "Pull request"}`} subtitle={`${link.repository} · ${link.snapshot === null ? "Status pending" : link.snapshot.isDraft && link.snapshot.state === "open" ? "Draft" : link.snapshot.state}${link.watch === undefined ? "" : " · Watching"}`} - onLongPress={ - supportsWatch && (link.snapshot === null || link.snapshot.state === "open") - ? () => confirmWatchToggle(link) - : undefined - } onPress={() => { void tryOpenExternalUrl(link.url, "pull-request").then((opened) => { if (!opened) diff --git a/apps/mobile/src/features/threads/git/gitSheetComponents.tsx b/apps/mobile/src/features/threads/git/gitSheetComponents.tsx index 3dd43e1d07d5..c358466e22d2 100644 --- a/apps/mobile/src/features/threads/git/gitSheetComponents.tsx +++ b/apps/mobile/src/features/threads/git/gitSheetComponents.tsx @@ -88,14 +88,12 @@ export function SheetListRow(props: { readonly subtitle?: string | null; readonly disabled?: boolean; readonly onPress: () => void; - readonly onLongPress?: (() => void) | undefined; }) { return ( - Effect.gen(function* () { - const store = yield* ProjectionStore.ProjectionStoreV2; - const watch = { - startedAt: DateTime.formatIso(old), - headSha: null, - checks: null, - remarksThrough: DateTime.formatIso(old), - remarkIds: [], - conflicting: false, - wakes: 0, - }; - yield* createThread("unwatched-link", { pullRequests: [pullRequestLink(1)] }); - yield* createThread("archived-watch", { - archivedAt: old, - pullRequests: [{ ...pullRequestLink(2), watch }], - }); - const watching = yield* createThread("watched-link", { - pullRequests: [pullRequestLink(3), { ...pullRequestLink(4), watch }], - }); - - assert.deepEqual( - (yield* store.getThreadsWatchingPullRequests()).map((thread) => thread.id), - [watching], - ); - }).pipe(Effect.provide(testLayer)), -); - it.effect.each([ ["sql", SqlLayer], ["memory", ProjectionStore.layerMemory], diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 906b8931fa82..ac0624b05806 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -355,11 +355,6 @@ export interface ProjectionStoreV2Shape { readonly getThreadsWithPullRequests: ( threadId?: ThreadId, ) => Effect.Effect, ProjectionStoreV2Error>; - /** Active threads with at least one watched pull request link. */ - readonly getThreadsWatchingPullRequests: () => Effect.Effect< - ReadonlyArray, - ProjectionStoreV2Error - >; readonly getTurnStartContext: ( threadId: ThreadId, runId: RunId, @@ -5138,19 +5133,6 @@ export const layer: Layer.Layer = ) .pipe(Effect.mapError((cause) => new ProjectionStoreSetupError({ cause }))); - const decodeThreadPullRequests = (rows: ReadonlyArray) => - Effect.forEach(rows, (row) => - decodeThreadPayload(row.payload_json).pipe( - Effect.map((thread): ProjectionThreadPullRequests => ({ - id: thread.id, - projectId: thread.projectId, - settledOverride: thread.settledOverride, - settledAt: thread.settledAt, - pullRequests: thread.pullRequests ?? [], - })), - ), - ); - const getThreadsWithPullRequests: ProjectionStoreV2Shape["getThreadsWithPullRequests"] = ( threadId, ) => @@ -5163,28 +5145,19 @@ export const layer: Layer.Layer = AND json_array_length(payload_json, '$.pullRequests') > 0 ORDER BY updated_at ASC, thread_id ASC `; - return yield* decodeThreadPullRequests(rows); + return yield* Effect.forEach(rows, (row) => + decodeThreadPayload(row.payload_json).pipe( + Effect.map((thread): ProjectionThreadPullRequests => ({ + id: thread.id, + projectId: thread.projectId, + settledOverride: thread.settledOverride, + settledAt: thread.settledAt, + pullRequests: thread.pullRequests ?? [], + })), + ), + ); }).pipe(Effect.mapError((cause) => new ProjectionStoreSetupError({ cause }))); - // The text match skips the JSON walk for the many threads that watch nothing. - const getThreadsWatchingPullRequests: ProjectionStoreV2Shape["getThreadsWatchingPullRequests"] = - () => - Effect.gen(function* () { - const rows = yield* sql` - SELECT payload_json - FROM orchestration_v2_projection_threads - WHERE deleted_at IS NULL - AND instr(payload_json, '"watch":') > 0 - AND json_extract(payload_json, '$.archivedAt') IS NULL - AND EXISTS ( - SELECT 1 FROM json_each(payload_json, '$.pullRequests') - WHERE json_extract(value, '$.watch') IS NOT NULL - ) - ORDER BY updated_at ASC, thread_id ASC - `; - return yield* decodeThreadPullRequests(rows); - }).pipe(Effect.mapError((cause) => new ProjectionStoreSetupError({ cause }))); - const shellThreadStateFromRow = (input: { readonly row: ShellThreadRow; readonly runOrdinalsByThreadId: ReadonlyMap>; @@ -5483,7 +5456,6 @@ export const layer: Layer.Layer = getThread, getSettlementCandidates, getThreadsWithPullRequests, - getThreadsWatchingPullRequests, getThreadProjection, getTurnStartContext, getTurnStartHistory, @@ -5613,16 +5585,6 @@ export const layerMemory: Layer.Layer = Layer.effect( left.id.localeCompare(right.id), ); }), - getThreadsWatchingPullRequests: () => - service - .getThreadsWithPullRequests() - .pipe( - Effect.map((threads) => - threads.filter((thread) => - (thread.pullRequests ?? []).some((link) => link.watch !== undefined), - ), - ), - ), getThreadsWithPullRequests: (threadId) => Ref.get(replayState).pipe( Effect.map((state) => diff --git a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts index 5663772ebe02..eba830319aaf 100644 --- a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts +++ b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts @@ -217,7 +217,6 @@ it.effect( getThread: () => Ref.get(projection).pipe(Effect.map((state) => state.thread)), getSettlementCandidates: () => Effect.die("unused getSettlementCandidates"), getThreadsWithPullRequests: () => Effect.die("unused getThreadsWithPullRequests"), - getThreadsWatchingPullRequests: () => Effect.die("unused getThreadsWatchingPullRequests"), getThreadProjection: () => Effect.die("control effects must not load transcript"), getTurnStartContext: () => Effect.die("unused"), getTurnStartHistory: () => Effect.die("unused"), diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index c37084087b11..c5f104db6065 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -2,8 +2,6 @@ import { CommandId, MessageId, type OrchestrationV2Notification, - type PullRequestComment, - type PullRequestThreadCommentsResult, type ThreadPullRequestLink, type ThreadPullRequestWatch, } from "@t3tools/contracts"; @@ -26,9 +24,6 @@ import * as Orchestrator from "./Orchestrator.ts"; import * as ProjectionStore from "./ProjectionStore.ts"; import { evaluatePullRequestWatch, pullRequestWatchMessage } from "./pullRequestWatch.ts"; -/** Extra comment pages read per review thread; a longer thread leaves the read incomplete. */ -const THREAD_PAGES = 5; - const logFailure = (message: string, fields: Record) => (cause: Cause.Cause): Effect.Effect => @@ -113,63 +108,13 @@ export const make = Effect.gen(function* () { ); if (detail.state !== "open") return yield* record(null); - // GitHub sends the first comments of each review thread; the rest are read here so a late - // reply in a long thread still counts. - const longThreads = activity.reviewThreads.filter( - (reviewThread) => reviewThread.nextCommentsCursor !== undefined, - ); - const rest = yield* Effect.forEach( - longThreads, - (reviewThread) => - Effect.gen(function* () { - const comments: Array = []; - let cursor: string | null | undefined = reviewThread.nextCommentsCursor; - for (let page = 0; cursor != null && page < THREAD_PAGES; page += 1) { - const result: PullRequestThreadCommentsResult = yield* pullRequests.threadComments({ - ...reference, - threadId: reviewThread.id, - cursor, - }); - for (const comment of result.comments) { - comments.push({ - ...comment, - kind: "review-comment", - path: reviewThread.path, - reviewState: null, - }); - } - cursor = result.nextCursor; - } - return { comments, whole: cursor == null }; - }).pipe( - // A failed page only leaves the remarks for a later pass; checks still count now. - Effect.catchCauseIf( - (cause) => !Cause.hasInterruptsOnly(cause), - () => Effect.succeed({ comments: [], whole: false }), - ), - ), - { concurrency: 2 }, - ); - // A truncated conversation with no long thread to explain it is a degraded read. - const whole = - (!activity.commentsTruncated || longThreads.length > 0) && rest.every((read) => read.whole); - const remarks = whole ? [...activity.comments, ...rest.flatMap((read) => read.comments)] : null; - - // GitHub names the head commit; elsewhere the newest commit stands in, so a push still reads - // as one. Azure DevOps reports neither, and relies on checks starting over. - const newest = activity.commits.reduce<(typeof activity.commits)[number] | undefined>( - (latest, commit) => - latest === undefined || Date.parse(commit.committedDate) > Date.parse(latest.committedDate) - ? commit - : latest, - undefined, - ); - const headSha = detail.headSha ?? newest?.oid; - const report = evaluatePullRequestWatch( - watch, - { ...detail, ...(headSha === undefined ? {} : { headSha }) }, - remarks, - ); + // A degraded read (GitHub's review thread query failed) is truncated with no long thread to + // explain it, and would skip review comments, so remarks wait for a later pass. Replies past + // the first ten of a long review thread are not read. + const degraded = + activity.commentsTruncated && + !activity.reviewThreads.some((reviewThread) => reviewThread.nextCommentsCursor !== undefined); + const report = evaluatePullRequestWatch(watch, detail, degraded ? null : activity.comments); if (report.changes.length > 0) { return yield* record( report.exhausted ? null : report.next, @@ -186,7 +131,7 @@ export const make = Effect.gen(function* () { }); const sweep = Effect.gen(function* () { - const threads = yield* projections.getThreadsWatchingPullRequests(); + const threads = yield* projections.getThreadsWithPullRequests(); yield* Effect.forEach( threads.flatMap((thread) => visibleThreadPullRequests(thread.pullRequests ?? []).flatMap((link) => diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index d7bf47edae59..dc08502f9b93 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -61,77 +61,32 @@ const remark = ( const noRemarks: ReadonlyArray = []; describe("evaluatePullRequestWatch", () => { - it("reports a check result once per head commit", () => { - const running = evaluatePullRequestWatch(watch(), detail(), noRemarks); - assert.deepEqual(running.changes, []); - assert.equal(running.next.headSha, "aaaaaaaaaa"); + it("reports the check result once every check finished, once per run", () => { + const running = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); + const waiting = evaluatePullRequestWatch(watch(), running, noRemarks); + assert.deepEqual(waiting.changes, []); - const failing = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); - const failed = evaluatePullRequestWatch(running.next, failing, noRemarks); + const finished = detail({ checks: [check("lint", "failure"), check("test", "cancelled")] }); + const failed = evaluatePullRequestWatch(waiting.next, finished, noRemarks); assert.deepEqual(failed.changes, [ - { kind: "checks-failed", failed: [check("lint", "failure")], running: 1 }, + { kind: "checks-failed", failed: [check("lint", "failure"), check("test", "cancelled")] }, ]); - assert.deepEqual(evaluatePullRequestWatch(failed.next, failing, noRemarks).changes, []); - - // The final result follows once the rest finish, so a second failure is not lost. - const finished = detail({ checks: [check("lint", "failure"), check("test", "failure")] }); - const final = evaluatePullRequestWatch(failed.next, finished, noRemarks); - assert.deepEqual(final.changes, [ - { - kind: "checks-failed", - failed: [check("lint", "failure"), check("test", "failure")], - running: 0, - }, - ]); - // Rerunning one of them is not news until the rerun finishes, and then it is. - const rerunning = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); - const quiet = evaluatePullRequestWatch(final.next, rerunning, noRemarks); - assert.deepEqual(quiet.changes, []); - assert.equal( - evaluatePullRequestWatch(quiet.next, finished, noRemarks).changes[0]?.kind, - "checks-failed", - ); + assert.deepEqual(evaluatePullRequestWatch(failed.next, finished, noRemarks).changes, []); - // A push that fails the same way before a pass ever sees it pending is still news. - const pushed = detail({ ...failing, headSha: "bbbbbbbbbb" }); - assert.equal( - evaluatePullRequestWatch(failed.next, pushed, noRemarks).changes[0]?.kind, - "checks-failed", - ); + // A rerun of one job shows as running again, so its result is reported even if it matches. + const rerun = evaluatePullRequestWatch(failed.next, running, noRemarks); + assert.equal(evaluatePullRequestWatch(rerun.next, finished, noRemarks).changes.length, 1); - // A rerun that passes on the same head is news too. - const rerun = detail({ checks: [check("lint", "success"), check("test", "success")] }); - assert.deepEqual(evaluatePullRequestWatch(failed.next, rerun, noRemarks).changes, [ - { kind: "checks-passed", count: 2, neutral: [] }, - ]); - }); + // A push reports its own result, even when it finished between two passes. + const pushed = detail({ ...finished, headSha: "bbbbbbbbbb" }); + assert.equal(evaluatePullRequestWatch(failed.next, pushed, noRemarks).changes.length, 1); - it("reports a check that needs someone's action", () => { - const blocked = detail({ - checks: [check("lint", "success"), check("deploy", "action-required")], - }); - assert.deepEqual(evaluatePullRequestWatch(watch(), blocked, noRemarks).changes, [ - { kind: "checks-failed", failed: [check("deploy", "action-required")], running: 0 }, + const passing = detail({ checks: [check("lint", "success"), check("test", "success")] }); + assert.deepEqual(evaluatePullRequestWatch(failed.next, passing, noRemarks).changes, [ + { kind: "checks-passed", count: 2 }, ]); }); - it("reports a new run as news on hosts that send no head commit", () => { - const passing = detail({ headSha: undefined, checks: [check("ci", "success")] }); - const passed = evaluatePullRequestWatch(watch({ wakes: 3 }), passing, noRemarks); - assert.equal(passed.changes[0]?.kind, "checks-passed"); - const rerun = evaluatePullRequestWatch( - passed.next, - detail({ headSha: undefined, checks: [check("ci", "pending")] }), - noRemarks, - ); - assert.deepEqual(rerun.changes, []); - assert.equal(rerun.next.wakes, 0); - assert.equal( - evaluatePullRequestWatch(rerun.next, passing, noRemarks).changes[0]?.kind, - "checks-passed", - ); - }); - 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); @@ -194,21 +149,6 @@ describe("evaluatePullRequestWatch", () => { assert.equal(result.next.wakes, 0); }); - it("restarts the comment wake count when a partial rerun starts", () => { - const tired = watch({ - headSha: "aaaaaaaaaa", - checks: "failed", - wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1, - }); - const rerun = evaluatePullRequestWatch( - tired, - detail({ checks: [check("lint", "failure"), check("test", "pending")] }), - [remark("reviewer", "2026-10-02T12:10:00Z")], - ); - assert.isFalse(rerun.exhausted); - assert.equal(rerun.next.wakes, 1); - }); - it("stops after the wake limit unless the head moves", () => { const comments = [remark("reviewer", "2026-10-02T12:10:00Z")]; const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 4a89278dc78f..3bd7a01c3555 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -7,26 +7,16 @@ import type { } from "@t3tools/contracts"; /** - * Wakes in a row that bring only new comments. A check result, a conflict, or a push resets the - * count, so this only stops a chatty bot looping an agent that is replying to it. + * Wakes in a row that bring only comments. Check, conflict, or push news resets the count, so + * this only stops a chatty bot looping an agent that is replying to it. */ export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10; const LISTED_ITEMS = 10; const SNIPPET_LENGTH = 200; export type PullRequestWatchChange = - | { - readonly kind: "checks-failed"; - readonly failed: ReadonlyArray; - /** Checks still running; a later report follows once they finish. */ - readonly running: number; - } - | { - readonly kind: "checks-passed"; - readonly count: number; - /** Finished without a verdict, which on GitLab can mean a manual job waits on someone. */ - readonly neutral: ReadonlyArray; - } + | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } + | { readonly kind: "checks-passed"; readonly count: number } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "conflicting" }; @@ -42,24 +32,19 @@ export interface PullRequestWatchReport { // "action-required" is a finished check that needs someone, so the agent hears about it. const isFailedCheck = (check: PullRequestCheck) => check.status === "failure" || check.status === "cancelled" || check.status === "action-required"; -const isRunningCheck = (check: PullRequestCheck) => check.status === "pending"; -function checksOutcome( - checks: ReadonlyArray, -): "passing" | "failing" | "failed" | "pending" | null { - if (checks.length === 0) return null; - const running = checks.some(isRunningCheck); - if (checks.some(isFailedCheck)) return running ? "failing" : "failed"; - return running ? "pending" : "passing"; +/** The result once every check finished; null while one still runs or there are none. */ +function checksResult(checks: ReadonlyArray): "passing" | "failed" | null { + if (checks.length === 0 || checks.some((check) => check.status === "pending")) return null; + return checks.some(isFailedCheck) ? "failed" : "passing"; } /** - * Compares a watched pull request with what its agent was last told. A check result is - * reported as soon as any check fails or once every check passed, and again only after the - * head commit moves or the checks start over. Remarks count when someone other than the - * viewer or the pull request's author wrote them: the agent posts as the viewer, so its own - * replies never wake it. `remarks` is null when the conversation could not be read whole; - * remarks are then left for a later pass rather than skipped. + * Compares a watched pull request with what its agent was last told. The check result is + * reported once every check finished, and again after the head commit moves or checks run + * again. Remarks count when someone other than the viewer or the pull request's author wrote + * them: the agent posts as the viewer, so its own replies never wake it. `remarks` is null when + * the conversation could not be read; remarks then wait for a later pass. */ export function evaluatePullRequestWatch( watch: ThreadPullRequestWatch, @@ -68,39 +53,25 @@ export function evaluatePullRequestWatch( ): PullRequestWatchReport { const changes: Array = []; const headSha = detail.headSha ?? null; - const outcome = checksOutcome(detail.checks); - // Hosts that report no head commit still show a push or a rerun as checks starting over. - const restarted = - headSha !== watch.headSha || - (watch.checks !== null && (outcome === "pending" || outcome === null)); + const result = checksResult(detail.checks); + // Hosts that report no head commit still show a push or a rerun as checks running again. + const restarted = headSha !== watch.headSha || (watch.checks !== null && result === null); let checks = restarted ? null : watch.checks; - // An early failure is reported while other checks run, and the final result once they finish. - // A rerun that leaves another failure in place is recorded quietly and reported when it ends. - const rerun = checks === "failed" && outcome === "failing"; - if (rerun) { - checks = "failing"; - } else if (outcome !== null && outcome !== "pending" && outcome !== checks) { + if (result !== null && result !== checks) { changes.push( - outcome === "passing" - ? { - kind: "checks-passed", - count: detail.checks.length, - neutral: detail.checks.filter((check) => check.status === "neutral"), - } - : { - kind: "checks-failed", - failed: detail.checks.filter(isFailedCheck), - running: detail.checks.filter(isRunningCheck).length, - }, + result === "passing" + ? { kind: "checks-passed", count: detail.checks.length } + : { kind: "checks-failed", failed: detail.checks.filter(isFailedCheck) }, ); - checks = outcome; + checks = result; } const own = new Set( [detail.viewer, detail.author?.login].flatMap((login) => (login ? [login.toLowerCase()] : [])), ); const through = Date.parse(watch.remarksThrough); + // GitHub times are per second, so remarks at the boundary time are told apart by ID. const fresh = (remarks ?? []).filter((remark) => { const at = Date.parse(remark.createdAt); return ( @@ -124,9 +95,8 @@ export function evaluatePullRequestWatch( const conflicting = detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; - // A new check run, a push, or any check or conflict news is progress, so the count restarts. const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks"); - const progress = restarted || rerun || (changes.length > 0 && !commentsOnly); + const progress = restarted || (changes.length > 0 && !commentsOnly); const wakes = (progress ? 0 : watch.wakes) + (commentsOnly ? 1 : 0); return { changes, @@ -164,7 +134,7 @@ function changeLines( switch (change.kind) { case "checks-failed": return [ - `- Checks failed${context.commit}${change.running > 0 ? ` (${change.running} still running)` : ""}:`, + `- Checks failed${context.commit}:`, ...listed( change.failed, (check) => @@ -174,11 +144,6 @@ function changeLines( case "checks-passed": return [ `- All ${change.count} ${change.count === 1 ? "check" : "checks"} finished without a failure${context.commit}.`, - ...listed( - change.neutral, - (check) => - ` - ${check.name} (neutral, may be waiting on someone)${check.url ? ` ${check.url}` : ""}`, - ), ]; case "remarks": return [ diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 31db11254659..0b0dcdbb7680 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2205,7 +2205,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { }); assert.deepEqual(yield* watchOf, started); - const recorded = { ...started, headSha: "abc123", checks: "failing" as const, wakes: 1 }; + const recorded = { ...started, headSha: "abc123", checks: "failed" as const, wakes: 1 }; yield* orchestrator.dispatch({ type: "thread.pull-request-watch.sync", commandId: CommandId.make("pr-watch-record"), @@ -2248,7 +2248,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); - it.effect("wakes a watched thread once for failed checks and a late thread reply", () => + it.effect("wakes a watched thread once for failed checks and a review comment", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; const threadId = ThreadId.make("runtime-pull-request-watch-wake"); @@ -2345,38 +2345,24 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { NodeServices.layer, Layer.mock(PullRequestService.PullRequestService)({ detail: () => Effect.succeed(detail), - // A long review thread: its later replies only arrive through threadComments. activity: () => - Effect.succeed({ - comments: [], - commentCount: 11, - commentsTruncated: true, - reviewThreads: [ - { - id: "thread-1", - path: "src/index.ts", - line: 1, - side: "right", - isResolved: false, - isOutdated: false, - comments: [], - nextCommentsCursor: "page-2", - }, - ], - commits: [], - }), - threadComments: () => Effect.succeed({ comments: [ { - id: "late-reply", + id: "review-1", + kind: "review-comment", author: { login: "reviewer", name: null, avatarUrl: null }, body: "One more thing.", createdAt: "2999-01-01T00:00:00.000Z", url: null, + path: "src/index.ts", + reviewState: null, }, ], - nextCursor: null, + commentCount: 1, + commentsTruncated: false, + reviewThreads: [], + commits: [], }), }), ), diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 0ad130722e4c..d930c4a8ba9d 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -182,11 +182,11 @@ when requested. With **Auto-settle merged threads** enabled, a thread can settle review is terminal. An open or unsynced link keeps it active. Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While -the thread is active, the server checks the pull request every minute and wakes the agent when checks -finish on the latest commit, someone else comments or reviews, or the branch starts to conflict. -Comments from your own account do not wake it. Watching ends when the pull request merges or closes, -or after 10 wakes in a row that bring only comments. To start or stop it yourself, use the row menu in the **Linked -pull requests** panel, or long-press the review in the mobile Git overview. +the thread is active, the server checks the pull request every minute and wakes the agent when every +check on the latest commit finished, someone else comments or reviews, or the branch starts to +conflict. Comments from your own account do not wake it. Watching ends when the pull request merges or +closes, or after 10 wakes in a row that bring only comments. To start or stop it yourself, use the row +menu in the **Linked pull requests** panel. Cross-repository links use a project on the same host. Azure DevOps reviews require a project checked out from the matching organization and repository. diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 7b63e4f9c66d..34eb2f59d9ac 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -100,10 +100,10 @@ export type ThreadPullRequestKey = typeof ThreadPullRequestKey.Type; */ export const ThreadPullRequestWatch = Schema.Struct({ startedAt: IsoDateTime, - /** Head commit whose check result was last reported; null until one is. */ + /** Head commit at the last pass; null where the host does not report one. */ headSha: Schema.NullOr(TrimmedNonEmptyString), - /** Last reported result: `failing` while other checks still run, `failed` once all finished. */ - checks: Schema.NullOr(Schema.Literals(["passing", "failing", "failed"])), + /** The check result last reported for that commit. */ + checks: Schema.NullOr(Schema.Literals(["passing", "failed"])), /** 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. */ From c92e031d8a38140fca351064b65b1fd96f29e386 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 3 Oct 2026 02:08:59 -0700 Subject: [PATCH 10/13] fix(server): watch ignores only the agent's own comments and empty check reads Co-Authored-By: Claude Opus 5.5 (1M context) --- .../orchestration-v2/pullRequestWatch.test.ts | 18 ++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 19 +++++++++++-------- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index dc08502f9b93..8e6a265853a3 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -106,6 +106,24 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(again.next.remarkIds, [first.id, "late"]); }); + it("does not treat a failed check read as a rerun", () => { + const failed = detail({ checks: [check("lint", "failure")] }); + const reported = evaluatePullRequestWatch(watch(), failed, noRemarks); + const unreadable = evaluatePullRequestWatch(reported.next, detail({ checks: [] }), noRemarks); + assert.deepEqual(evaluatePullRequestWatch(unreadable.next, failed, noRemarks).changes, []); + }); + + it("wakes for the pull request's author when the agent is someone else", () => { + const contributor = detail({ author: { login: "contributor", name: null, avatarUrl: null } }); + const reply = remark("contributor", "2026-10-02T12:06:00Z"); + assert.deepEqual(evaluatePullRequestWatch(watch(), contributor, [reply]).changes, [ + { kind: "remarks", remarks: [reply] }, + ]); + // Without a viewer, the author is taken to be the agent. + const noViewer = detail({ viewer: undefined, author: contributor.author }); + assert.deepEqual(evaluatePullRequestWatch(watch(), noViewer, [reply]).changes, []); + }); + it("reports remarks from others once and never the agent's own", () => { const comments = [ remark("agent-user", "2026-10-02T12:05:00Z", "Fixed in the latest push."), diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 3bd7a01c3555..2171114305c2 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -42,8 +42,8 @@ function checksResult(checks: ReadonlyArray): "passing" | "fai /** * Compares a watched pull request with what its agent was last told. The check result is * reported once every check finished, and again after the head commit moves or checks run - * again. Remarks count when someone other than the viewer or the pull request's author wrote - * them: the agent posts as the viewer, so its own replies never wake it. `remarks` is null when + * again. Remarks count when someone other than the agent's own account wrote them, so its own + * replies never wake it. `remarks` is null when * the conversation could not be read; remarks then wait for a later pass. */ export function evaluatePullRequestWatch( @@ -54,8 +54,11 @@ export function evaluatePullRequestWatch( const changes: Array = []; const headSha = detail.headSha ?? null; const result = checksResult(detail.checks); - // Hosts that report no head commit still show a push or a rerun as checks running again. - const restarted = headSha !== watch.headSha || (watch.checks !== null && result === null); + // Hosts that report no head commit still show a push or a rerun as checks running again. An + // empty list is not a rerun: a host can answer with one when its check read fails. + const restarted = + headSha !== watch.headSha || + (watch.checks !== null && detail.checks.some((check) => check.status === "pending")); let checks = restarted ? null : watch.checks; if (result !== null && result !== checks) { @@ -67,16 +70,16 @@ export function evaluatePullRequestWatch( checks = result; } - const own = new Set( - [detail.viewer, detail.author?.login].flatMap((login) => (login ? [login.toLowerCase()] : [])), - ); + // The agent posts as the viewer. Where the host does not name one, the agent most likely + // opened the pull request, so its author stands in. + const own = (detail.viewer ?? detail.author?.login)?.toLowerCase(); const through = Date.parse(watch.remarksThrough); // GitHub times are per second, so remarks at the boundary time are told apart by ID. const fresh = (remarks ?? []).filter((remark) => { const at = Date.parse(remark.createdAt); return ( (at > through || (at === through && !watch.remarkIds.includes(remark.id))) && - !own.has(remark.author?.login.toLowerCase() ?? "") + remark.author?.login.toLowerCase() !== own ); }); if (fresh.length > 0) changes.push({ kind: "remarks", remarks: fresh }); From d3acf29c27ce617af60463fe1f46fb0b31d55f5f Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 3 Oct 2026 02:32:12 -0700 Subject: [PATCH 11/13] fix(server): watch reports each failed check at once and stops visibly when it cannot read Co-Authored-By: Claude Opus 5.5 (1M context) --- .../toolkits/pullRequests/handlers.test.ts | 3 +- .../src/mcp/toolkits/pullRequests/tools.ts | 2 +- .../src/orchestration-v2/Orchestrator.ts | 3 +- .../PullRequestWatchReactor.ts | 143 ++++++++++++------ .../orchestration-v2/pullRequestWatch.test.ts | 60 +++++--- .../src/orchestration-v2/pullRequestWatch.ts | 60 ++++---- .../src/orchestration-v2/runtimeLayer.test.ts | 65 +++++++- .../src/pullRequest/GitHubPullRequestCli.ts | 4 +- .../pullRequest/gitHubPullRequestJson.test.ts | 20 +++ .../src/pullRequest/gitHubPullRequestJson.ts | 19 ++- docs/user/source-control.md | 10 +- packages/contracts/src/pullRequest.ts | 2 + packages/contracts/src/threadPullRequest.ts | 6 +- 13 files changed, 277 insertions(+), 120 deletions(-) diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts index e7c85dfb3a30..e329ade90d9e 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts @@ -246,7 +246,8 @@ describe("pull request toolkit handlers", () => { const watch = { startedAt: "2026-08-20T00:00:00.000Z", headSha: null, - checks: null, + failedChecks: [], + passed: false, remarksThrough: "2026-08-20T00:00:00.000Z", remarkIds: [], conflicting: false, diff --git a/apps/server/src/mcp/toolkits/pullRequests/tools.ts b/apps/server/src/mcp/toolkits/pullRequests/tools.ts index 0c66f917f318..52ac473a3d50 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/tools.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/tools.ts @@ -258,7 +258,7 @@ const ListThreadPullRequestsTool = Tool.make("list_thread_pull_requests", { const WatchPullRequestTool = Tool.make("watch_pull_request", { description: - "Have T3 Code watch an open pull request for this thread, linking it first if needed. T3 Code checks it every minute and wakes you with a message when checks finish on the latest commit, someone else comments or reviews, or the branch starts to conflict with its base. Use this to monitor or babysit a pull request instead of polling, sleeping, or running a watcher; end your turn after calling it. Watching ends when the pull request merges or closes, or when you call unwatch_pull_request.", + "Have T3 Code watch an open pull request for this thread, linking it first if needed. T3 Code checks it every minute and wakes you with a message when a check fails, the required checks pass, someone else comments or reviews, or the branch starts to conflict with its base. Use this to monitor or babysit a pull request instead of polling, sleeping, or running a watcher. Only comments posted after this call wake you, so handle the existing ones first, then end your turn. A wake is news, not a merge decision: check readiness yourself before merging. Watching ends when the pull request merges or closes, when T3 Code cannot read it for 15 minutes, or when you call unwatch_pull_request.", parameters: PullRequestTargetInput, success: WatchPullRequestResult, failure: PullRequestToolError, diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 83bb463cc24b..aadbc850b9da 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -2964,7 +2964,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio : (existing.watch ?? { startedAt, headSha: null, - checks: null, + failedChecks: [], + passed: false, remarksThrough: startedAt, remarkIds: [], conflicting: false, diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index c5f104db6065..305ea280a27f 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -24,6 +24,9 @@ import * as Orchestrator from "./Orchestrator.ts"; import * as ProjectionStore from "./ProjectionStore.ts"; import { evaluatePullRequestWatch, pullRequestWatchMessage } from "./pullRequestWatch.ts"; +/** Passes in a row that could not read a pull request before its watch ends (one a minute). */ +const READ_FAILURE_LIMIT = 15; + const logFailure = (message: string, fields: Record) => (cause: Cause.Cause): Effect.Effect => @@ -31,11 +34,21 @@ const logFailure = ? Effect.interrupt : Effect.logWarning(message, { ...fields, cause }); +interface WatchTarget { + readonly thread: ProjectionStore.ProjectionThreadPullRequests; + readonly link: ThreadPullRequestLink; + readonly watch: ThreadPullRequestWatch; +} + +const failureKey = ({ thread, link, watch }: WatchTarget) => + `${thread.id} ${threadPullRequestKeyOf(link)} ${watch.startedAt}`; + function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatch): boolean { return ( left.startedAt === right.startedAt && left.headSha === right.headSha && - left.checks === right.checks && + left.failedChecks.join("\n") === right.failedChecks.join("\n") && + left.passed === right.passed && left.remarksThrough === right.remarksThrough && left.remarkIds.join("\n") === right.remarkIds.join("\n") && left.conflicting === right.conflicting && @@ -65,40 +78,46 @@ export const make = Effect.gen(function* () { const pullRequests = yield* PullRequestService.PullRequestService; const crypto = yield* Crypto.Crypto; - const check = Effect.fn("PullRequestWatchReactor.check")(function* ( - thread: ProjectionStore.ProjectionThreadPullRequests, - link: ThreadPullRequestLink, - watch: ThreadPullRequestWatch, - ) { - // Host-level identity, with the repository as linked, the way pull request sync reads it. - const pullRequest = { - host: normalizeThreadPullRequestKey(link).host, - repository: link.repository, - number: link.number, - }; - // The orchestrator applies this only while the same watch is on, so a stop or restart that - // lands during the host read wins. - const record = ( - next: ThreadPullRequestWatch | null, - wake?: { readonly text: string; readonly notification: OrchestrationV2Notification }, - ) => - Effect.gen(function* () { - const uuid = yield* crypto.randomUUIDv4; - yield* engine.dispatch({ - type: "thread.pull-request-watch.sync", - commandId: CommandId.make(`server:pr-watch:${thread.id}:${uuid}`), - threadId: thread.id, - ...pullRequest, - startedAt: watch.startedAt, - watch: next, - ...(wake === undefined - ? {} - : { wake: { ...wake, messageId: MessageId.make(`message:pr-watch:${uuid}`) } }), - }); + // Passes in a row that failed, per watch. Kept in memory: a restart only delays the stop. + const readFailures = new Map(); + + // Host-level identity, with the repository as linked, the way pull request sync reads it. + const identityOf = (link: ThreadPullRequestLink) => ({ + host: normalizeThreadPullRequestKey(link).host, + repository: link.repository, + number: link.number, + }); + + /** + * Records what a pass saw, and wakes the agent with it. The orchestrator applies this only + * while the same watch is on, so a stop or restart that lands during the host read wins. + */ + const record = ( + target: WatchTarget, + next: ThreadPullRequestWatch | null, + wake?: { readonly text: string; readonly notification: OrchestrationV2Notification }, + ) => + Effect.gen(function* () { + const uuid = yield* crypto.randomUUIDv4; + yield* engine.dispatch({ + type: "thread.pull-request-watch.sync", + commandId: CommandId.make(`server:pr-watch:${target.thread.id}:${uuid}`), + threadId: target.thread.id, + ...identityOf(target.link), + startedAt: target.watch.startedAt, + watch: next, + ...(wake === undefined + ? {} + : { wake: { ...wake, messageId: MessageId.make(`message:pr-watch:${uuid}`) } }), }); + }); + + const check = Effect.fn("PullRequestWatchReactor.check")(function* (target: WatchTarget) { + const { thread, link, watch } = target; + const pullRequest = identityOf(link); // A merged pull request cannot reopen, so its watch ends without a host read, even on a // settled thread. A closed one can, so the host decides below. - if (link.snapshot?.state === "merged") return yield* record(null); + if (link.snapshot?.state === "merged") return yield* record(target, null); if (thread.settledOverride === "settled" || thread.settledAt !== null) return; const reference = { projectId: thread.projectId, ...pullRequest }; @@ -106,7 +125,7 @@ export const make = Effect.gen(function* () { [pullRequests.detail({ ...reference, allowStale: false }), pullRequests.activity(reference)], { concurrency: 2 }, ); - if (detail.state !== "open") return yield* record(null); + if (detail.state !== "open") return yield* record(target, null); // A degraded read (GitHub's review thread query failed) is truncated with no long thread to // explain it, and would skip review comments, so remarks wait for a later pass. Replies past @@ -117,6 +136,7 @@ export const make = Effect.gen(function* () { const report = evaluatePullRequestWatch(watch, detail, degraded ? null : activity.comments); if (report.changes.length > 0) { return yield* record( + target, report.exhausted ? null : report.next, pullRequestWatchMessage({ number: link.number, @@ -127,25 +147,54 @@ export const make = Effect.gen(function* () { }), ); } - if (!watchesEqual(report.next, watch)) yield* record(report.next); + if (!watchesEqual(report.next, watch)) yield* record(target, report.next); }); + // A watch that cannot read its pull request ends with a wake saying so, rather than showing + // "Watching" while it learns nothing. + const giveUp = (target: WatchTarget) => + record(target, null, { + text: `T3 Code stopped watching pull request #${target.link.number} (${target.link.url}) because it could not read it from the host for ${READ_FAILURE_LIMIT} minutes. Check it yourself, and call watch_pull_request to watch it again.`, + notification: { + source: { kind: "monitor" }, + outcome: "failed", + summary: `#${target.link.number}: stopped watching, could not read it`, + }, + }).pipe(Effect.catch(() => record(target, null))); + const sweep = Effect.gen(function* () { const threads = yield* projections.getThreadsWithPullRequests(); - yield* Effect.forEach( - threads.flatMap((thread) => - visibleThreadPullRequests(thread.pullRequests ?? []).flatMap((link) => - link.watch === undefined ? [] : [{ thread, link, watch: link.watch }], - ), + const targets = threads.flatMap((thread) => + visibleThreadPullRequests(thread.pullRequests ?? []).flatMap((link) => + link.watch === undefined ? [] : [{ thread, link, watch: link.watch }], ), - ({ thread, link, watch }) => - check(thread, link, watch).pipe( - Effect.catchCause( - logFailure("pull request watch check failed", { - threadId: thread.id, - pullRequest: threadPullRequestKeyOf(link), - }), - ), + ); + const keys = new Set(targets.map(failureKey)); + for (const key of readFailures.keys()) if (!keys.has(key)) readFailures.delete(key); + yield* Effect.forEach( + targets, + (target) => + check(target).pipe( + Effect.tap(() => Effect.sync(() => readFailures.delete(failureKey(target)))), + Effect.catchCause((cause) => { + const key = failureKey(target); + const failures = (readFailures.get(key) ?? 0) + 1; + readFailures.set(key, failures); + return logFailure("pull request watch check failed", { + threadId: target.thread.id, + pullRequest: threadPullRequestKeyOf(target.link), + failures, + })(cause).pipe( + Effect.andThen( + failures >= READ_FAILURE_LIMIT + ? giveUp(target).pipe( + Effect.tap(() => Effect.sync(() => readFailures.delete(key))), + Effect.catchCause(logFailure("pull request watch stop failed", {})), + ) + : Effect.void, + ), + ); + }), ), { concurrency: 4, discard: true }, ); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 8e6a265853a3..c247d78fd332 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -17,7 +17,8 @@ const STARTED = "2026-10-02T12:00:00.000Z"; const watch = (overrides: Partial = {}): ThreadPullRequestWatch => ({ startedAt: STARTED, headSha: null, - checks: null, + failedChecks: [], + passed: false, remarksThrough: STARTED, remarkIds: [], conflicting: false, @@ -61,30 +62,42 @@ const remark = ( const noRemarks: ReadonlyArray = []; describe("evaluatePullRequestWatch", () => { - it("reports the check result once every check finished, once per run", () => { - const running = detail({ checks: [check("lint", "failure"), check("test", "pending")] }); - const waiting = evaluatePullRequestWatch(watch(), running, noRemarks); - assert.deepEqual(waiting.changes, []); - - const finished = detail({ checks: [check("lint", "failure"), check("test", "cancelled")] }); - const failed = evaluatePullRequestWatch(waiting.next, finished, noRemarks); - assert.deepEqual(failed.changes, [ - { kind: "checks-failed", failed: [check("lint", "failure"), check("test", "cancelled")] }, - ]); - assert.deepEqual(evaluatePullRequestWatch(failed.next, finished, noRemarks).changes, []); - - // A rerun of one job shows as running again, so its result is reported even if it matches. - const rerun = evaluatePullRequestWatch(failed.next, running, noRemarks); - assert.equal(evaluatePullRequestWatch(rerun.next, finished, noRemarks).changes.length, 1); + it("reports each failure at once, even while another check never finishes", () => { + const bot = check("CodeRabbit", "pending"); + const first = detail({ checks: [check("lint", "failure"), check("test", "pending"), bot] }); + const lint = evaluatePullRequestWatch(watch(), first, noRemarks); + assert.deepEqual(lint.changes, [{ kind: "checks-failed", failed: [check("lint", "failure")] }]); + assert.deepEqual(evaluatePullRequestWatch(lint.next, first, noRemarks).changes, []); + + // A different job failing later is news of its own. + const second = detail({ checks: [check("lint", "failure"), check("test", "failure"), bot] }); + const test = evaluatePullRequestWatch(lint.next, second, noRemarks); + assert.deepEqual(test.changes, [{ kind: "checks-failed", failed: [check("test", "failure")] }]); + + // A rerun leaves the list while it runs, so failing again is reported again. + const rerun = evaluatePullRequestWatch(test.next, first, noRemarks); + assert.equal(evaluatePullRequestWatch(rerun.next, second, noRemarks).changes.length, 1); + + // A push reports its failures, even ones that failed between two passes. + const pushed = detail({ ...second, headSha: "bbbbbbbbbb" }); + assert.equal(evaluatePullRequestWatch(test.next, pushed, noRemarks).changes.length, 1); + }); - // A push reports its own result, even when it finished between two passes. - const pushed = detail({ ...finished, headSha: "bbbbbbbbbb" }); - assert.equal(evaluatePullRequestWatch(failed.next, pushed, noRemarks).changes.length, 1); + it("reports passed once the required checks pass, whatever the others do", () => { + const required = (name: string, status: PullRequestCheck["status"]) => ({ + ...check(name, status), + required: true, + }); + const green = detail({ + checks: [required("test", "success"), required("lint", "success"), check("bot", "pending")], + }); + const passed = evaluatePullRequestWatch(watch(), green, noRemarks); + assert.deepEqual(passed.changes, [{ kind: "checks-passed", count: 2, required: true }]); + assert.deepEqual(evaluatePullRequestWatch(passed.next, green, noRemarks).changes, []); - const passing = detail({ checks: [check("lint", "success"), check("test", "success")] }); - assert.deepEqual(evaluatePullRequestWatch(failed.next, passing, noRemarks).changes, [ - { kind: "checks-passed", count: 2 }, - ]); + // Where nothing is marked required, every check has to pass. + const plain = detail({ checks: [check("test", "success"), check("bot", "pending")] }); + assert.deepEqual(evaluatePullRequestWatch(watch(), plain, noRemarks).changes, []); }); it("keeps remarks for a later pass when the conversation was not read whole", () => { @@ -109,6 +122,7 @@ describe("evaluatePullRequestWatch", () => { it("does not treat a failed check read as a rerun", () => { const failed = detail({ checks: [check("lint", "failure")] }); const reported = evaluatePullRequestWatch(watch(), failed, noRemarks); + assert.equal(reported.changes.length, 1); const unreadable = evaluatePullRequestWatch(reported.next, detail({ checks: [] }), noRemarks); assert.deepEqual(evaluatePullRequestWatch(unreadable.next, failed, noRemarks).changes, []); }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 2171114305c2..af9ce27636ee 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -16,7 +16,7 @@ const SNIPPET_LENGTH = 200; export type PullRequestWatchChange = | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } - | { readonly kind: "checks-passed"; readonly count: number } + | { readonly kind: "checks-passed"; readonly count: number; readonly required: boolean } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "conflicting" }; @@ -33,18 +33,13 @@ export interface PullRequestWatchReport { const isFailedCheck = (check: PullRequestCheck) => check.status === "failure" || check.status === "cancelled" || check.status === "action-required"; -/** The result once every check finished; null while one still runs or there are none. */ -function checksResult(checks: ReadonlyArray): "passing" | "failed" | null { - if (checks.length === 0 || checks.some((check) => check.status === "pending")) return null; - return checks.some(isFailedCheck) ? "failed" : "passing"; -} - /** - * Compares a watched pull request with what its agent was last told. The check result is - * reported once every check finished, and again after the head commit moves or checks run - * again. Remarks count when someone other than the agent's own account wrote them, so its own - * replies never wake it. `remarks` is null when - * the conversation could not be read; remarks then wait for a later pass. + * Compares a watched pull request with what its agent was last told. Each check is reported as + * soon as it fails, so a check that never finishes (an advisory review bot) cannot hold the + * news back. "Passed" is reported once the checks the base branch requires all passed, or all + * checks where the host marks none required. Remarks count when someone other than the agent's + * own account wrote them, so its own replies never wake it. `remarks` is null when the + * conversation could not be read; remarks then wait for a later pass. */ export function evaluatePullRequestWatch( watch: ThreadPullRequestWatch, @@ -53,25 +48,27 @@ export function evaluatePullRequestWatch( ): PullRequestWatchReport { const changes: Array = []; const headSha = detail.headSha ?? null; - const result = checksResult(detail.checks); - // Hosts that report no head commit still show a push or a rerun as checks running again. An - // empty list is not a rerun: a host can answer with one when its check read fails. - const restarted = - headSha !== watch.headSha || - (watch.checks !== null && detail.checks.some((check) => check.status === "pending")); + const headMoved = headSha !== watch.headSha; - let checks = restarted ? null : watch.checks; - if (result !== null && result !== checks) { - changes.push( - result === "passing" - ? { kind: "checks-passed", count: detail.checks.length } - : { kind: "checks-failed", failed: detail.checks.filter(isFailedCheck) }, - ); - checks = result; + // 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; + if (detail.checks.length > 0) { + const failed = detail.checks.filter(isFailedCheck); + const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); + if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); + // A check that runs again leaves the list, so a rerun that fails again is reported. + failedChecks = failed.map((check) => check.name); + + 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) { + changes.push({ kind: "checks-passed", count: gate.length, required: required.length > 0 }); + } + passed = passedNow; } - // The agent posts as the viewer. Where the host does not name one, the agent most likely - // opened the pull request, so its author stands in. const own = (detail.viewer ?? detail.author?.login)?.toLowerCase(); const through = Date.parse(watch.remarksThrough); // GitHub times are per second, so remarks at the boundary time are told apart by ID. @@ -99,14 +96,15 @@ export function evaluatePullRequestWatch( detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks"); - const progress = restarted || (changes.length > 0 && !commentsOnly); + const progress = headMoved || (changes.length > 0 && !commentsOnly); const wakes = (progress ? 0 : watch.wakes) + (commentsOnly ? 1 : 0); return { changes, next: { startedAt: watch.startedAt, headSha, - checks, + failedChecks, + passed, remarksThrough, remarkIds, conflicting, @@ -146,7 +144,7 @@ function changeLines( ]; case "checks-passed": return [ - `- All ${change.count} ${change.count === 1 ? "check" : "checks"} finished without a failure${context.commit}.`, + `- All ${change.count} ${change.required ? "required " : ""}${change.count === 1 ? "check" : "checks"} passed${context.commit}.`, ]; case "remarks": return [ diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 0b0dcdbb7680..2c5235c767d7 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2205,7 +2205,7 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { }); assert.deepEqual(yield* watchOf, started); - const recorded = { ...started, headSha: "abc123", checks: "failed" as const, wakes: 1 }; + const recorded = { ...started, headSha: "abc123", failedChecks: ["lint"], wakes: 1 }; yield* orchestrator.dispatch({ type: "thread.pull-request-watch.sync", commandId: CommandId.make("pr-watch-record"), @@ -2248,6 +2248,65 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect("ends a watch it cannot read, and tells the agent", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const threadId = ThreadId.make("runtime-pull-request-watch-unreadable"); + const projectId = ProjectId.make("pr-watch-unreadable-project"); + yield* seedProject({ + projectId, + title: "Watch unreadable", + workspaceRoot: "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/workspace/watch-unreadable", + defaultModelSelection: null, + createdAt: "2026-10-01T00:00:00.000Z", + }); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make("pr-watch-unreadable-create"), + threadId, + projectId, + title: "Watch unreadable", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-unreadable-start"), + threadId, + host: "github.com", + repository: "pingdotgg/t3code", + number: 8, + watching: true, + link: { url: "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/pingdotgg/t3code/pull/8", source: "agent" }, + }); + const reactor = yield* PullRequestWatchReactor.make.pipe( + Effect.provide( + Layer.mergeAll( + NodeServices.layer, + Layer.mock(PullRequestService.PullRequestService)({ + detail: () => Effect.die("host unreachable"), + activity: () => Effect.die("host unreachable"), + }), + ), + ), + ); + for (let pass = 0; pass < 15; pass += 1) yield* reactor.sweep; + + const thread = yield* orchestrator.getThreadShell(threadId); + assert.isUndefined(thread?.pullRequests?.[0]?.watch); + const { messages } = yield* orchestrator.getThreadRecords(threadId, ["messages"]); + assert.deepEqual( + messages.flatMap((message) => message.notification?.summary ?? []), + ["#8: stopped watching, could not read it"], + ); + }), + ); + it.effect("wakes a watched thread once for failed checks and a review comment", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; @@ -2380,8 +2439,8 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { ); const watch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; assert.deepEqual( - { headSha: watch?.headSha, checks: watch?.checks, wakes: watch?.wakes }, - { headSha: "abc1234def", checks: "failed", wakes: 0 }, + { headSha: watch?.headSha, failedChecks: watch?.failedChecks, wakes: watch?.wakes }, + { headSha: "abc1234def", failedChecks: ["lint"], wakes: 0 }, ); }), ); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 020b571fe248..f40cff8f50bf 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -52,7 +52,7 @@ import { decodePullRequestActivityJson, decodePullRequestDetailJson, decodePullRequestCoreJson, - PULL_REQUEST_CORE_GRAPHQL_QUERY, + pullRequestCoreGraphQlQuery, type GitHubPullRequestCore, type GitHubPullRequestSummary, decodePullRequestPreviewJson, @@ -1595,7 +1595,7 @@ export const make = Effect.gen(function* () { ["-F", `number=${input.number}`], ["-f", `headRef=refs/pull/${input.number}/head`], ], - query: PULL_REQUEST_CORE_GRAPHQL_QUERY, + query: pullRequestCoreGraphQlQuery(input.host), decode: decodePullRequestCoreJson, }), ), diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 12239a2af0ee..1393ab6e6b6a 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -26,6 +26,7 @@ import { decodeWorkflowRunApprovalsJson, reviewThreadConversation, REVIEW_THREADS_GRAPHQL_QUERY, + pullRequestCoreGraphQlQuery, pullRequestSearchGraphQlQuery, } from "./gitHubPullRequestJson.ts"; @@ -285,6 +286,25 @@ describe("pull request detail decoding", () => { ]); }); + it("keeps what branch protection requires, and asks for it on github.com only", () => { + const raw = JSON.parse(detailJson) as Record; + const detail = expectSuccess( + decodePullRequestDetailJson( + JSON.stringify({ + ...raw, + statusCheckRollup: [ + { __typename: "CheckRun", name: "test", status: "IN_PROGRESS", isRequired: true }, + { __typename: "StatusContext", context: "bot", state: "PENDING", isRequired: false }, + { __typename: "StatusContext", context: "legacy", state: "SUCCESS" }, + ], + }), + ), + ); + expect(detail.checks.map((check) => check.required)).toEqual([true, false, undefined]); + expect(pullRequestCoreGraphQlQuery("github.com")).toContain("isRequired"); + expect(pullRequestCoreGraphQlQuery("github.example.com")).not.toContain("isRequired"); + }); + it("keeps a workflow waiting for approval out of the passing state", () => { const raw = JSON.parse(detailJson) as Record; const detail = expectSuccess( diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index 8e3a9ee330b8..d1600754b50b 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -93,6 +93,8 @@ const RawCheckSchema = Schema.Struct({ workflowName: Schema.optional(Schema.NullOr(Schema.String)), startedAt: Schema.optional(Schema.NullOr(Schema.String)), completedAt: Schema.optional(Schema.NullOr(Schema.String)), + /** Branch protection requires this check; read by the detail query on github.com only. */ + isRequired: Schema.optional(Schema.NullOr(Schema.Boolean)), }); const RawListItemSchema = Schema.Struct({ @@ -706,8 +708,15 @@ export const PULL_REQUEST_LIST_JSON_FIELDS = export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; -/** Pull refs let the comparison share the detail read without first resolving a fork branch. */ -export const PULL_REQUEST_CORE_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!, $headRef: String!) { +/** + * Pull refs let the comparison share the detail read without first resolving a fork branch. + * `isRequired` is asked for on github.com only: an older Enterprise server may not know it, and + * an unknown field fails the whole read. + */ +export const pullRequestCoreGraphQlQuery = (host: string) => { + const required = + host.toLowerCase() === "github.com" ? " isRequired(pullRequestNumber: $number)" : ""; + return `query($owner: String!, $name: String!, $number: Int!, $headRef: String!) { repository(owner: $owner, name: $name) { mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission pullRequest(number: $number) { @@ -727,9 +736,9 @@ export const PULL_REQUEST_CORE_GRAPHQL_QUERY = `query($owner: String!, $name: St nodes { commit { statusCheckRollup { contexts(first: 100) { nodes { __typename - ... on StatusContext { context state targetUrl createdAt description } + ... on StatusContext { context state targetUrl createdAt description${required} } ... on CheckRun { - name status conclusion startedAt completedAt detailsUrl + name status conclusion startedAt completedAt detailsUrl${required} checkSuite { workflowRun { workflow { name } } } } } @@ -739,6 +748,7 @@ export const PULL_REQUEST_CORE_GRAPHQL_QUERY = `query($owner: String!, $name: St } } }`; +}; export const PULL_REQUEST_PREVIEW_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { @@ -1468,6 +1478,7 @@ function toCheckEntries( status: toCheckStatus(check), description: trimmed(check.description), url: trimmed(check.detailsUrl) ?? trimmed(check.targetUrl), + ...(typeof check.isRequired === "boolean" ? { required: check.isRequired } : {}), }, workflowName: trimmed(check.workflowName), at: realTimestamp(check.completedAt) ?? realTimestamp(check.startedAt), diff --git a/docs/user/source-control.md b/docs/user/source-control.md index d930c4a8ba9d..64beb946c408 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -182,11 +182,11 @@ when requested. With **Auto-settle merged threads** enabled, a thread can settle review is terminal. An open or unsynced link keeps it active. Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While -the thread is active, the server checks the pull request every minute and wakes the agent when every -check on the latest commit finished, someone else comments or reviews, or the branch starts to -conflict. Comments from your own account do not wake it. Watching ends when the pull request merges or -closes, or after 10 wakes in a row that bring only comments. To start or stop it yourself, use the row -menu in the **Linked pull requests** panel. +the thread is active, the server checks the pull request every minute and wakes the agent when a check +fails, the required checks pass, someone else comments or reviews, or the branch starts to conflict. +Comments from your own account do not wake it. Watching ends when the pull request merges or closes, +after 10 wakes in a row that bring only comments, or when the server cannot read the pull request for +15 minutes. To start or stop it yourself, use the row menu in the **Linked pull requests** panel. Cross-repository links use a project on the same host. Azure DevOps reviews require a project checked out from the matching organization and repository. diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 98255b0a546e..c8feaad9a41f 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -152,6 +152,8 @@ export const PullRequestCheck = Schema.Struct({ status: PullRequestCheckStatus, description: Schema.NullOr(Schema.String), url: Schema.NullOr(Schema.String), + /** The base branch requires this check to merge. Absent where the host does not say. */ + required: Schema.optional(Schema.Boolean), }); export type PullRequestCheck = typeof PullRequestCheck.Type; diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 34eb2f59d9ac..9df5cbe9670a 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -102,8 +102,10 @@ export const ThreadPullRequestWatch = Schema.Struct({ startedAt: IsoDateTime, /** Head commit at the last pass; null where the host does not report one. */ headSha: Schema.NullOr(TrimmedNonEmptyString), - /** The check result last reported for that commit. */ - checks: Schema.NullOr(Schema.Literals(["passing", "failed"])), + /** Failed checks on that commit the agent was told about; a rerun that fails again is news. */ + failedChecks: Schema.Array(TrimmedNonEmptyString), + /** The agent was told the required checks on that commit passed. */ + passed: Schema.Boolean, /** 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. */ From 2403198f91f9345c7de6b33f2049de8e54a6210b Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 3 Oct 2026 02:46:07 -0700 Subject: [PATCH 12/13] fix(server): watches survive legacy re-links and stop only on host read failures Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/orchestration-v2/Orchestrator.ts | 53 +++++++------- .../PullRequestWatchReactor.ts | 72 ++++++++++--------- 2 files changed, 63 insertions(+), 62 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index aadbc850b9da..2707fe5b8344 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -457,6 +457,26 @@ function withPullRequestWatch( return watch === undefined ? rest : { ...rest, watch }; } +/** A legacy single-PR link as a link entry. Re-linking a pull request keeps its watch. */ +function legacyPullRequestLink( + thread: OrchestrationV2AppThread, + linked: ThreadLinkedPullRequest, + now: DateTime.Utc, +): ThreadPullRequestLink { + const key = legacyThreadPullRequestKey(linked); + return withPullRequestWatch( + { + ...key, + url: linked.url, + source: "manual", + linkedAt: DateTime.formatIso(now), + snapshot: null, + stack: null, + }, + threadPullRequestsOf(thread).find((link) => threadPullRequestKeysEqual(link, key))?.watch, + ); +} + function delegatedCompletionWakeDetail(taskIds: ReadonlyArray): string { const taskList = taskIds.join(", "); return taskIds.length === 1 @@ -2801,25 +2821,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ), ), ...(command.linkedPullRequest - ? [ - withPullRequestWatch( - { - ...legacyThreadPullRequestKey(command.linkedPullRequest), - url: command.linkedPullRequest.url, - source: "manual" as const, - linkedAt: DateTime.formatIso(now), - snapshot: null, - stack: null, - }, - // Re-linking the same pull request keeps its watch. - threadPullRequestsOf(thread).find((link) => - threadPullRequestKeysEqual( - link, - legacyThreadPullRequestKey(command.linkedPullRequest!), - ), - )?.watch, - ), - ] + ? [legacyPullRequestLink(thread, command.linkedPullRequest, now)] : []), ], }), @@ -2977,6 +2979,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio pullRequests: links.map((link) => link === existing ? withPullRequestWatch(link, watch) : link, ), + // A user or agent starting or stopping a watch is activity; recorded progress is not. + updatedAt: command.type === "thread.pull-request.watch" ? now : thread.updatedAt, }; } case "thread.pull-request.sync": @@ -3006,16 +3010,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ), ), ...(command.linkedPullRequest - ? [ - { - ...legacyThreadPullRequestKey(command.linkedPullRequest), - url: command.linkedPullRequest.url, - source: "manual" as const, - linkedAt: DateTime.formatIso(now), - snapshot: null, - stack: null, - }, - ] + ? [legacyPullRequestLink(thread, command.linkedPullRequest, now)] : []), ], }), diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 305ea280a27f..c34fb1f2c4b4 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -14,6 +14,7 @@ import * as Cause from "effect/Cause"; import * as Context from "effect/Context"; import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as Layer from "effect/Layer"; import * as Schedule from "effect/Schedule"; import type * as Scope from "effect/Scope"; @@ -112,6 +113,18 @@ export const make = Effect.gen(function* () { }); }); + // A watch that cannot read its pull request ends with a wake saying so, rather than showing + // "Watching" while it learns nothing. + const giveUp = (target: WatchTarget) => + record(target, null, { + text: `T3 Code stopped watching pull request #${target.link.number} (${target.link.url}) because it could not read it from the host for ${READ_FAILURE_LIMIT} minutes. Check it yourself, and call watch_pull_request to watch it again.`, + notification: { + source: { kind: "monitor" }, + outcome: "failed", + summary: `#${target.link.number}: stopped watching, could not read it`, + }, + }).pipe(Effect.catch(() => record(target, null))); + const check = Effect.fn("PullRequestWatchReactor.check")(function* (target: WatchTarget) { const { thread, link, watch } = target; const pullRequest = identityOf(link); @@ -121,10 +134,29 @@ export const make = Effect.gen(function* () { if (thread.settledOverride === "settled" || thread.settledAt !== null) return; const reference = { projectId: thread.projectId, ...pullRequest }; - const [detail, activity] = yield* Effect.all( - [pullRequests.detail({ ...reference, allowStale: false }), pullRequests.activity(reference)], - { concurrency: 2 }, + const read = yield* Effect.exit( + Effect.all( + [ + pullRequests.detail({ ...reference, allowStale: false }), + pullRequests.activity(reference), + ], + { concurrency: 2 }, + ), ); + // Only host reads count towards giving up; a refused wake is not the host's fault. + const key = failureKey(target); + if (Exit.isFailure(read)) { + if (Cause.hasInterruptsOnly(read.cause)) return yield* Effect.failCause(read.cause); + const failures = (readFailures.get(key) ?? 0) + 1; + readFailures.set(key, failures); + if (failures >= READ_FAILURE_LIMIT) { + readFailures.delete(key); + yield* giveUp(target); + } + return yield* Effect.failCause(read.cause); + } + readFailures.delete(key); + const [detail, activity] = read.value; if (detail.state !== "open") return yield* record(target, null); // A degraded read (GitHub's review thread query failed) is truncated with no long thread to @@ -150,18 +182,6 @@ export const make = Effect.gen(function* () { if (!watchesEqual(report.next, watch)) yield* record(target, report.next); }); - // A watch that cannot read its pull request ends with a wake saying so, rather than showing - // "Watching" while it learns nothing. - const giveUp = (target: WatchTarget) => - record(target, null, { - text: `T3 Code stopped watching pull request #${target.link.number} (${target.link.url}) because it could not read it from the host for ${READ_FAILURE_LIMIT} minutes. Check it yourself, and call watch_pull_request to watch it again.`, - notification: { - source: { kind: "monitor" }, - outcome: "failed", - summary: `#${target.link.number}: stopped watching, could not read it`, - }, - }).pipe(Effect.catch(() => record(target, null))); - const sweep = Effect.gen(function* () { const threads = yield* projections.getThreadsWithPullRequests(); const targets = threads.flatMap((thread) => @@ -175,26 +195,12 @@ export const make = Effect.gen(function* () { targets, (target) => check(target).pipe( - Effect.tap(() => Effect.sync(() => readFailures.delete(failureKey(target)))), - Effect.catchCause((cause) => { - const key = failureKey(target); - const failures = (readFailures.get(key) ?? 0) + 1; - readFailures.set(key, failures); - return logFailure("pull request watch check failed", { + Effect.catchCause( + logFailure("pull request watch check failed", { threadId: target.thread.id, pullRequest: threadPullRequestKeyOf(target.link), - failures, - })(cause).pipe( - Effect.andThen( - failures >= READ_FAILURE_LIMIT - ? giveUp(target).pipe( - Effect.tap(() => Effect.sync(() => readFailures.delete(key))), - Effect.catchCause(logFailure("pull request watch stop failed", {})), - ) - : Effect.void, - ), - ); - }), + }), + ), ), { concurrency: 4, discard: true }, ); From 8d608f9f5b6d9e013268d626f4e0c2427b8453dd Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 3 Oct 2026 02:50:39 -0700 Subject: [PATCH 13/13] fix(server): a failed watch stop is retried on the next pass Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/orchestration-v2/PullRequestWatchReactor.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index c34fb1f2c4b4..4eb6ec80ce17 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -149,9 +149,10 @@ export const make = Effect.gen(function* () { if (Cause.hasInterruptsOnly(read.cause)) return yield* Effect.failCause(read.cause); const failures = (readFailures.get(key) ?? 0) + 1; readFailures.set(key, failures); + // The count stays until the stop lands, so a failed stop is tried again next pass. if (failures >= READ_FAILURE_LIMIT) { - readFailures.delete(key); yield* giveUp(target); + readFailures.delete(key); } return yield* Effect.failCause(read.cause); }