Skip to content

Commit e134fe2

Browse files
committed
fix(web): add turn-diff-completed fallback for turn-end notifications
The prior widened session-set check still missed turn-end notifications in practice. The session-set signal is apparently unreliable in the user's environment — possibly lost to subscription races, snapshot overwrites, or missed batches. Add thread.turn-diff-completed as a fallback completion signal. This event is dispatched by CheckpointReactor strictly in response to an actual turn.completed runtime event, so its presence is a strong guarantee that a turn ended. It is already in the thread-detail subscription stream and flows through the same applyRecoveredEventBatch path. Deduplication: a completion fired via the primary session-set path in the same batch marks the thread in completionFiredThreadIds, preventing the turn-diff-completed branch from firing a second notification for the same turn. Both paths honor the 5-second user-stop suppression window and the in-batch userInitiated guard. Tests cover: fallback firing when session-set is absent, dedup when both signals arrive together, and suppression respect on the fallback path. Also renames the in-batch running-transition tracker to make its intent clearer in the guarded-completion branch.
1 parent 9c96b86 commit e134fe2

2 files changed

Lines changed: 115 additions & 13 deletions

File tree

apps/web/src/turnNotification.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ function makeActivityAppendedEvent(threadId: string, kind: string): Orchestratio
5555
} as unknown as OrchestrationEvent;
5656
}
5757

58+
function makeTurnDiffCompletedEvent(threadId: string): OrchestrationEvent {
59+
return {
60+
type: "thread.turn-diff-completed",
61+
payload: {
62+
threadId: threadId as ThreadId,
63+
turnId: `turn-${threadId}`,
64+
},
65+
} as unknown as OrchestrationEvent;
66+
}
67+
5868
describe("BUILT_IN_SOUNDS", () => {
5969
it("has 4 entries with valid id, label, and src", () => {
6070
expect(BUILT_IN_SOUNDS).toHaveLength(4);
@@ -255,6 +265,72 @@ describe("deriveTurnNotificationTriggers", () => {
255265

256266
expect(triggers).toHaveLength(0);
257267
});
268+
269+
it("falls back to turn-diff-completed when session-set signal is missing", () => {
270+
// The session-set path can be lost (subscription race, snapshot overwrite)
271+
// but the CheckpointReactor reliably dispatches thread.turn-diff-completed
272+
// after an actual turn.completed runtime event, so the user still gets
273+
// notified.
274+
const thread = makeThread({
275+
session: { orchestrationStatus: "ready" },
276+
} as Partial<Thread>);
277+
const project = makeProject();
278+
const events = [makeTurnDiffCompletedEvent("thread-1")];
279+
280+
const triggers = deriveTurnNotificationTriggers(
281+
events,
282+
() => thread,
283+
() => project,
284+
);
285+
286+
expect(triggers).toHaveLength(1);
287+
expect(triggers[0]!.reason).toBe("turn-completed");
288+
});
289+
290+
it("dedupes session-set + turn-diff-completed for the same thread", () => {
291+
// Both the primary session-set signal and the turn-diff-completed fallback
292+
// arrive together in the happy path. Only one notification should fire.
293+
const thread = makeThread({
294+
session: { orchestrationStatus: "running" },
295+
} as Partial<Thread>);
296+
const project = makeProject();
297+
const events = [
298+
makeSessionSetEvent("thread-1", "ready"),
299+
makeTurnDiffCompletedEvent("thread-1"),
300+
];
301+
302+
const triggers = deriveTurnNotificationTriggers(
303+
events,
304+
() => thread,
305+
() => project,
306+
);
307+
308+
expect(triggers).toHaveLength(1);
309+
expect(triggers[0]!.reason).toBe("turn-completed");
310+
});
311+
312+
it("honors user-initiated stops on the turn-diff-completed fallback", () => {
313+
// If the user stopped the turn themselves, the fallback must respect the
314+
// 5-second suppression window so we don't notify for a stop they just did.
315+
const thread = makeThread({
316+
session: { orchestrationStatus: "running" },
317+
} as Partial<Thread>);
318+
const project = makeProject();
319+
320+
vi.spyOn(Date, "now").mockReturnValue(1000);
321+
markThreadUserStopped("thread-1" as ThreadId);
322+
323+
vi.spyOn(Date, "now").mockReturnValue(2000);
324+
const events = [makeTurnDiffCompletedEvent("thread-1")];
325+
326+
const triggers = deriveTurnNotificationTriggers(
327+
events,
328+
() => thread,
329+
() => project,
330+
);
331+
332+
expect(triggers).toHaveLength(0);
333+
});
258334
});
259335

260336
describe("locally interrupted turn tracker", () => {

apps/web/src/turnNotification.ts

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,13 @@ export function deriveTurnNotificationTriggers(
116116
}
117117
}
118118

119-
// Track threads observed transitioning to "running" earlier in this batch so
120-
// that a subsequent completion event still fires a notification even if the
121-
// stored session hasn't been updated yet (e.g. on reconnect/replay when
122-
// turn.started and turn.completed arrive in the same batch).
119+
// Threads that already produced a completion trigger in this batch. Used to
120+
// dedupe between the primary session-set signal and the turn-diff-completed
121+
// fallback so we never fire two notifications for the same turn end.
122+
const completionFiredThreadIds = new Set<ThreadId>();
123+
// Threads observed transitioning to "running" earlier in this batch, used
124+
// when reconnect/replay delivers turn.started + turn.completed together and
125+
// the stored session still reflects the pre-batch state.
123126
const threadTransitionedToRunning = new Set<ThreadId>();
124127

125128
for (const event of events) {
@@ -140,19 +143,18 @@ export function deriveTurnNotificationTriggers(
140143

141144
const thread = getThread(threadId);
142145
if (!thread) continue;
143-
// The thread is considered to have been actively turning if the stored
144-
// session is still "running", the current batch already transitioned it
145-
// into "running", or the latest turn is still marked as running. The
146-
// activeTurnId check catches the common case where completion notifications
147-
// would otherwise be dropped because the session orchestrationStatus has
148-
// already been written ahead of this derivation (e.g. when the server
149-
// replays both start + completion events as a single batch).
150-
const wasRunning =
146+
147+
// Skip session-bootstrap completions (e.g. provider "session.started"
148+
// maps to status:ready when no turn is active). A real turn completion
149+
// always has one of: a session previously in running state, an
150+
// activeTurnId on the prior session, a latestTurn still flagged
151+
// running, or an in-batch running transition.
152+
const turnWasActive =
151153
thread.session?.orchestrationStatus === "running" ||
152154
thread.session?.activeTurnId !== undefined ||
153155
thread.latestTurn?.state === "running" ||
154156
threadTransitionedToRunning.has(threadId);
155-
if (!wasRunning) continue;
157+
if (!turnWasActive) continue;
156158

157159
const project = getProject(thread.projectId);
158160
triggers.push({
@@ -161,6 +163,30 @@ export function deriveTurnNotificationTriggers(
161163
threadTitle: thread.title || "Untitled",
162164
projectName: project?.name || "Unknown project",
163165
});
166+
completionFiredThreadIds.add(threadId);
167+
continue;
168+
}
169+
170+
// Fallback signal: a turn-diff capture completing is a strong guarantee
171+
// that a turn ended — CheckpointReactor only fires this after an actual
172+
// turn.completed runtime event. If the session-set path was dropped
173+
// (subscription race, snapshot overwrite, missed batch), this still
174+
// notifies the user and is deduped against the primary path above.
175+
if (event.type === "thread.turn-diff-completed") {
176+
const { threadId } = event.payload;
177+
if (completionFiredThreadIds.has(threadId)) continue;
178+
if (isThreadSuppressed(threadId)) continue;
179+
if (userInitiatedThreadIds.has(threadId)) continue;
180+
const thread = getThread(threadId);
181+
if (!thread) continue;
182+
const project = getProject(thread.projectId);
183+
triggers.push({
184+
threadId,
185+
reason: "turn-completed",
186+
threadTitle: thread.title || "Untitled",
187+
projectName: project?.name || "Unknown project",
188+
});
189+
completionFiredThreadIds.add(threadId);
164190
continue;
165191
}
166192

0 commit comments

Comments
 (0)