From e424a9bf8af8627abb06805028d080fb10bfa2c5 Mon Sep 17 00:00:00 2001 From: Matthew Vaught <8752859+MTVaught@users.noreply.github.com> Date: Sun, 20 Sep 2026 16:59:05 -0500 Subject: [PATCH] fix(server): forward review flag events on the thread detail stream --- .../Layers/OrchestrationEngine.test.ts | 67 +++++++++++++++++++ apps/server/src/ws.threadDetailEvents.test.ts | 53 +++++++++++++++ apps/server/src/ws.ts | 10 ++- apps/web/src/components/DiffPanel.tsx | 11 +-- 4 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 apps/server/src/ws.threadDetailEvents.test.ts diff --git a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts index 4a2e99f3ebc0..aaaaae94cc23 100644 --- a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts +++ b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts @@ -759,6 +759,73 @@ describe("OrchestrationEngine", () => { await system.dispose(); }); + it("flags review files on a thread and reads them back from the detail snapshot", async () => { + const system = await createOrchestrationSystem(); + const { engine } = system; + const createdAt = now(); + const threadId = ThreadId.make("thread-review-flags"); + + await system.run( + engine.dispatch({ + type: "project.create", + commandId: CommandId.make("cmd-project-review-flags-create"), + projectId: asProjectId("project-review-flags"), + title: "Project Review Flags", + workspaceRoot: "/tmp/project-review-flags", + defaultModelSelection: { + instanceId: ProviderInstanceId.make("codex"), + model: "gpt-5-codex", + }, + createdAt, + }), + ); + await system.run( + engine.dispatch({ + type: "thread.create", + commandId: CommandId.make("cmd-thread-review-flags-create"), + threadId, + projectId: asProjectId("project-review-flags"), + title: "Flag me", + modelSelection: { + instanceId: ProviderInstanceId.make("codex"), + model: "gpt-5-codex", + }, + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + runtimeMode: "full-access", + branch: null, + worktreePath: null, + createdAt, + }), + ); + + await system.run( + engine.dispatch({ + type: "thread.review-file.flag", + commandId: CommandId.make("cmd-review-flag"), + threadId, + paths: ["src/app.ts"], + }), + ); + expect(Option.getOrNull(await system.readThread(threadId))?.reviewFollowUpPaths).toEqual([ + "src/app.ts", + ]); + expect( + (await system.readModel()).threads.find((thread) => thread.id === threadId) + ?.reviewFollowUpPaths, + ).toEqual(["src/app.ts"]); + + await system.run( + engine.dispatch({ + type: "thread.review-file.unflag", + commandId: CommandId.make("cmd-review-unflag"), + threadId, + }), + ); + expect(Option.getOrNull(await system.readThread(threadId))?.reviewFollowUpPaths).toEqual([]); + + await system.dispose(); + }); + it("archives and unarchives threads through orchestration commands", async () => { const system = await createOrchestrationSystem(); const { engine } = system; diff --git a/apps/server/src/ws.threadDetailEvents.test.ts b/apps/server/src/ws.threadDetailEvents.test.ts new file mode 100644 index 000000000000..7edf2ceaf0f7 --- /dev/null +++ b/apps/server/src/ws.threadDetailEvents.test.ts @@ -0,0 +1,53 @@ +import { EventId, ThreadId } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { isThreadDetailEvent } from "./ws.ts"; + +const base = { + eventId: EventId.make("event-1"), + sequence: 1, + occurredAt: "2026-01-01T00:00:00.000Z", + aggregateKind: "thread" as const, + aggregateId: ThreadId.make("thread-1"), + commandId: null, + causationEventId: null, + correlationId: null, + metadata: {}, +}; + +describe("isThreadDetailEvent", () => { + it("forwards review flag changes, which only the detail carries", () => { + expect( + isThreadDetailEvent({ + ...base, + type: "thread.review-file-flagged", + payload: { + threadId: ThreadId.make("thread-1"), + paths: ["src/app.ts"], + updatedAt: base.occurredAt, + }, + }), + ).toBe(true); + expect( + isThreadDetailEvent({ + ...base, + type: "thread.review-file-unflagged", + payload: { + threadId: ThreadId.make("thread-1"), + paths: ["src/app.ts"], + updatedAt: base.occurredAt, + }, + }), + ).toBe(true); + }); + + it("keeps shell-only changes off the detail stream", () => { + expect( + isThreadDetailEvent({ + ...base, + type: "thread.unpinned", + payload: { threadId: ThreadId.make("thread-1"), updatedAt: base.occurredAt }, + }), + ).toBe(false); + }); +}); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 64945adddcbc..e83957fe509c 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -344,16 +344,22 @@ export function isThreadDetailEvent(event: OrchestrationEvent): event is Extract | "thread.activity-appended" | "thread.turn-diff-completed" | "thread.reverted" - | "thread.session-set"; + | "thread.session-set" + | "thread.review-file-flagged" + | "thread.review-file-unflagged"; } > { + // Review flags live only on the detail (the shell does not carry them), so + // their events must ride this stream or the open diff never sees a change. return ( event.type === "thread.message-sent" || event.type === "thread.proposed-plan-upserted" || event.type === "thread.activity-appended" || event.type === "thread.turn-diff-completed" || event.type === "thread.reverted" || - event.type === "thread.session-set" + event.type === "thread.session-set" || + event.type === "thread.review-file-flagged" || + event.type === "thread.review-file-unflagged" ); } diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index b28eb2bb74c4..50e849050fba 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -800,7 +800,10 @@ export default function DiffPanel({ ], ); // Files the reviewer flagged as still needing follow-up. The flag lives on the thread, so - // it survives reloads and shows on every device; it blocks staging until cleared. + // it survives reloads and shows on every device; it blocks staging until cleared. Servers + // from before the flag reject the command, so the controls stay hidden there. + const supportsFollowUpFlags = + isWorkingTreeScope && serverConfig?.environment.capabilities.threadReviewFlags === true; const followUpPaths = useMemo( () => new Set(activeThread?.reviewFollowUpPaths ?? []), [activeThread?.reviewFollowUpPaths], @@ -1388,7 +1391,7 @@ export default function DiffPanel({ Next changed file (K) - {isWorkingTreeScope ? ( + {supportsFollowUpFlags ? ( {flaggedForFollowUp ? ( @@ -1583,7 +1586,7 @@ export default function DiffPanel({ {stagingBadge} ) : null} - {isWorkingTreeScope ? ( + {supportsFollowUpFlags ? (