Skip to content

Commit b5c7a04

Browse files
authored
fix(cloud-task): make question answered state durable across chat switches and restarts
Since questions relay over the durable event stream (#3199, #3215), users report that switching chats resets answered questions. Two gaps cause it: 1. The question card held its in-progress step answers in component state. Switching chats unmounts the card, so every answer selected so far was wiped and the card restarted at question 1 on return. The answers now live in a view store keyed by toolCallId, and the card restores them (including free-text input, via a new ActionSelector initialCustomInput seed) on remount. Submit and cancel clear the draft. 2. A question answered after its run terminalized (the resume-as-prompt path) never got a `_posthog/permission_resolved` record: the sandbox that would write one is gone, so the request stays pending in the persisted log forever and any log derivation that escapes the in-memory guards (app restart, another device, a session rebuild) re-surfaces the answered question as a fresh card. The client now appends the resolved marker to the owning run's log itself, tracks responded requestIds so stale snapshots (a marker not yet flushed to storage) cannot re-add them, and the watcher dedups replayed permission_request frames by stream id — they previously bypassed the seenEventIds guard entirely. Generated-By: PostHog Code Task-Id: 9c723d92-b8ef-4c9f-9317-2f53b4644f33
1 parent 7ece0f0 commit b5c7a04

10 files changed

Lines changed: 565 additions & 54 deletions

File tree

packages/core/src/cloud-task/cloud-task.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,62 @@ describe("CloudTaskService", () => {
135135
vi.unstubAllGlobals();
136136
});
137137

138+
it("emits a replayed permission_request frame only once", async () => {
139+
const updates: unknown[] = [];
140+
service.on(CloudTaskEvent.Update, (payload) => updates.push(payload));
141+
142+
mockNetFetch
143+
.mockResolvedValueOnce(
144+
createJsonResponse({
145+
id: "run-1",
146+
status: "in_progress",
147+
stage: "build",
148+
output: null,
149+
error_message: null,
150+
branch: "main",
151+
updated_at: "2026-01-01T00:00:00Z",
152+
}),
153+
)
154+
.mockResolvedValue(
155+
createJsonResponse([], 200, { "X-Has-More": "false" }),
156+
);
157+
158+
const frame =
159+
'id: 5\ndata: {"type":"permission_request","requestId":"req-1","toolCall":{"toolCallId":"tool-1"},"options":[]}\n\n';
160+
const trailingEntry =
161+
'id: 6\ndata: {"type":"notification","timestamp":"2026-01-01T00:00:02Z","notification":{"jsonrpc":"2.0","method":"_posthog/console","params":{"sessionId":"run-1","level":"info","message":"after replay"}}}\n\n';
162+
// The durable stream re-sends the tail on reconnect/replay — the same
163+
// frame arriving twice must not re-surface the question a second time.
164+
mockStreamFetch.mockResolvedValueOnce(
165+
createOpenSseResponse(frame + frame + trailingEntry),
166+
);
167+
168+
service.watch({
169+
taskId: "task-1",
170+
runId: "run-1",
171+
apiHost: "https://app.example.com",
172+
teamId: 2,
173+
});
174+
175+
await waitFor(() =>
176+
updates.some((u) => (u as { kind?: string }).kind === "logs"),
177+
);
178+
179+
const permissionUpdates = updates.filter(
180+
(u) => (u as { kind?: string }).kind === "permission_request",
181+
);
182+
expect(permissionUpdates).toEqual([
183+
{
184+
taskId: "task-1",
185+
runId: "run-1",
186+
kind: "permission_request",
187+
requestId: "req-1",
188+
toolCall: { toolCallId: "tool-1" },
189+
options: [],
190+
},
191+
]);
192+
});
193+
138194
it("bootstraps paged backlog for active runs and drains deduped live SSE entries", async () => {
139195
const updates: unknown[] = [];
140196
service.on(CloudTaskEvent.Update, (payload) => updates.push(payload));

packages/core/src/cloud-task/cloud-task.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,6 +1138,22 @@ export class CloudTaskService extends TypedEventEmitter<CloudTaskEvents> {
11381138
return null;
11391139
}
11401140

1141+
// Drop a re-delivered event by its stream id. The durable stream re-sends
1142+
// the tail on reconnect/replay: each re-sent log entry would otherwise be
1143+
// counted as a new entry (advancing totalEntryCount past the renderer's
1144+
// processedLineCount guard) and emitted again — the root cause of duplicate
1145+
// transcript entries and back-to-back completion notifications — and a
1146+
// re-sent permission_request frame would re-surface an already-answered
1147+
// question as a fresh pending card. Events without an id (legacy servers)
1148+
// fall through and are handled downstream.
1149+
const eventId = event.id;
1150+
if (eventId !== undefined) {
1151+
if (watcher.seenEventIds.has(eventId)) {
1152+
return null;
1153+
}
1154+
watcher.seenEventIds.add(eventId);
1155+
}
1156+
11411157
if (isPermissionRequestEvent(event.data)) {
11421158
this.emit(CloudTaskEvent.Update, {
11431159
taskId: watcher.taskId,
@@ -1150,20 +1166,6 @@ export class CloudTaskService extends TypedEventEmitter<CloudTaskEvents> {
11501166
return null;
11511167
}
11521168

1153-
// Drop a re-delivered log entry by its stream id. The durable stream
1154-
// re-sends the tail on reconnect/replay, and each resend would otherwise be
1155-
// counted as a new entry (advancing totalEntryCount past the renderer's
1156-
// processedLineCount guard) and emitted again — the root cause of duplicate
1157-
// transcript entries and back-to-back completion notifications. Entries
1158-
// without an id (legacy servers) fall through and are handled downstream.
1159-
const eventId = event.id;
1160-
if (eventId !== undefined) {
1161-
if (watcher.seenEventIds.has(eventId)) {
1162-
return null;
1163-
}
1164-
watcher.seenEventIds.add(eventId);
1165-
}
1166-
11671169
watcher.pendingLogEntries.push(event.data as StoredLogEntry);
11681170
if (watcher.pendingLogEntries.length >= EVENT_BATCH_MAX_SIZE) {
11691171
this.flushLogBatch(key);

packages/core/src/sessions/sessionService.ts

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ const AUTO_RETRY_MAX_ATTEMPTS = 2;
9898
const AUTO_RETRY_DELAY_MS = 10_000;
9999
const AUTH_RESTORE_MAX_RETRY_WAITS = 6;
100100
const MAX_SUPERSEDED_RUN_IDS = 100;
101+
const MAX_RESPONDED_PERMISSION_REQUEST_IDS = 500;
101102
/**
102103
* Streamed events are buffered and flushed on this cadence so a burst of tokens
103104
* coalesces into one processing pass (and roughly one render) instead of one
@@ -607,6 +608,13 @@ export class SessionService {
607608
private cloudLogGapReconciler: CloudLogGapReconciler;
608609
/** Maps toolCallId → cloud requestId for routing permission responses */
609610
private cloudPermissionRequestIds = new Map<string, string>();
611+
/**
612+
* Cloud permission requestIds the user has already responded to this app
613+
* session. A stale snapshot (a resolved marker not yet flushed to storage)
614+
* or a replayed stream frame can re-deliver an answered request; without
615+
* this guard it would re-surface as a fresh pending card.
616+
*/
617+
private respondedCloudPermissionRequestIds = new Set<string>();
610618
private liveTurnContent = new Map<
611619
string,
612620
{ startedAtTs: number; agentTextChunks: number; agentOutputEvents: number }
@@ -2167,6 +2175,15 @@ export class SessionService {
21672175
return;
21682176
}
21692177

2178+
if (this.respondedCloudPermissionRequestIds.has(update.requestId)) {
2179+
this.d.log.debug("Skipping already-answered cloud permission request", {
2180+
taskRunId,
2181+
requestId: update.requestId,
2182+
toolCallId: update.toolCall.toolCallId,
2183+
});
2184+
return;
2185+
}
2186+
21702187
if (
21712188
isPermissionRequestAlreadySurfaced(
21722189
session.pendingPermissions,
@@ -3179,6 +3196,7 @@ export class SessionService {
31793196
session: AgentSession,
31803197
permission: PermissionRequest | undefined,
31813198
toolCallId: string,
3199+
requestId: string | undefined,
31823200
optionId: string,
31833201
customInput?: string,
31843202
answers?: Record<string, string>,
@@ -3197,19 +3215,90 @@ export class SessionService {
31973215
toolCallId,
31983216
optionId,
31993217
});
3218+
await this.persistCloudPermissionResolution(
3219+
session,
3220+
permission,
3221+
toolCallId,
3222+
requestId,
3223+
optionId,
3224+
);
32003225
return;
32013226
}
32023227
await this.sendCloudPrompt(
32033228
{ ...session, cloudStatus: cloudStatus ?? session.cloudStatus },
32043229
answerPrompt,
32053230
);
3231+
await this.persistCloudPermissionResolution(
3232+
session,
3233+
permission,
3234+
toolCallId,
3235+
requestId,
3236+
optionId,
3237+
);
32063238
this.d.log.info("Permission answer resumed terminal cloud run", {
32073239
taskId: session.taskId,
32083240
toolCallId,
32093241
optionId,
32103242
});
32113243
}
32123244

3245+
/**
3246+
* Record a response to a permission request whose sandbox is gone. A live
3247+
* sandbox writes `_posthog/permission_resolved` to the run log when it
3248+
* resolves a request, but a request answered after its run terminalized has
3249+
* no sandbox left to do that — without this record the request stays
3250+
* pending in the persisted log forever, and every future derivation (app
3251+
* restart, another device, a session rebuild) re-surfaces the
3252+
* already-answered question as a fresh card.
3253+
*/
3254+
private async persistCloudPermissionResolution(
3255+
session: AgentSession,
3256+
permission: PermissionRequest | undefined,
3257+
toolCallId: string,
3258+
requestId: string | undefined,
3259+
optionId: string,
3260+
): Promise<void> {
3261+
if (!requestId) return;
3262+
this.markCloudPermissionResponded(requestId);
3263+
3264+
const client = await this.d.getAuthenticatedClient();
3265+
if (!client) return;
3266+
const taskRunId = permission?.taskRunId ?? session.taskRunId;
3267+
try {
3268+
await client.appendTaskRunLog(session.taskId, taskRunId, [
3269+
{
3270+
type: "notification",
3271+
timestamp: new Date().toISOString(),
3272+
notification: {
3273+
method: POSTHOG_NOTIFICATIONS.PERMISSION_RESOLVED,
3274+
params: { requestId, toolCallId, optionId },
3275+
},
3276+
},
3277+
]);
3278+
} catch (error) {
3279+
this.d.log.warn("Failed to persist permission resolution to run log", {
3280+
taskId: session.taskId,
3281+
taskRunId,
3282+
toolCallId,
3283+
error,
3284+
});
3285+
}
3286+
}
3287+
3288+
private markCloudPermissionResponded(requestId: string): void {
3289+
this.respondedCloudPermissionRequestIds.add(requestId);
3290+
while (
3291+
this.respondedCloudPermissionRequestIds.size >
3292+
MAX_RESPONDED_PERMISSION_REQUEST_IDS
3293+
) {
3294+
const oldest = this.respondedCloudPermissionRequestIds
3295+
.values()
3296+
.next().value;
3297+
if (oldest === undefined) break;
3298+
this.respondedCloudPermissionRequestIds.delete(oldest);
3299+
}
3300+
}
3301+
32133302
// --- Permissions ---
32143303

32153304
private resolvePermission(session: AgentSession, toolCallId: string): void {
@@ -3269,6 +3358,7 @@ export class SessionService {
32693358
session,
32703359
permission,
32713360
toolCallId,
3361+
cloudRequestId,
32723362
optionId,
32733363
customInput,
32743364
answers,
@@ -3292,6 +3382,7 @@ export class SessionService {
32923382
session,
32933383
permission,
32943384
toolCallId,
3385+
cloudRequestId,
32953386
optionId,
32963387
customInput,
32973388
answers,
@@ -3301,6 +3392,10 @@ export class SessionService {
33013392
}
33023393
throw error;
33033394
}
3395+
// The live sandbox persists its own resolved marker; remember the
3396+
// response locally so a snapshot fetched before that marker flushes
3397+
// to storage cannot re-surface the question.
3398+
this.markCloudPermissionResponded(cloudRequestId);
33043399
} else {
33053400
await this.d.trpc.agent.respondToPermission.mutate({
33063401
taskRunId: session.taskRunId,
@@ -3352,8 +3447,16 @@ export class SessionService {
33523447
try {
33533448
if (session.isCloud && isTerminalStatus(session.cloudStatus)) {
33543449
// The run is over — the card was resolved locally above and there is no
3355-
// live permission promise left to reject.
3450+
// live permission promise left to reject. Persist the dismissal so the
3451+
// request is not re-derived as pending from the run log later.
33563452
this.cloudPermissionRequestIds.delete(toolCallId);
3453+
await this.persistCloudPermissionResolution(
3454+
session,
3455+
permission,
3456+
toolCallId,
3457+
cloudRequestId,
3458+
"cancelled",
3459+
);
33573460
return;
33583461
}
33593462
if (session.isCloud && cloudRequestId) {
@@ -3363,6 +3466,7 @@ export class SessionService {
33633466
optionId: "reject_with_feedback",
33643467
customInput: "User cancelled the permission request.",
33653468
});
3469+
this.markCloudPermissionResponded(cloudRequestId);
33663470
} else {
33673471
await this.d.trpc.agent.cancelPermission.mutate({
33683472
taskRunId: session.taskRunId,
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { Theme } from "@radix-ui/themes";
2+
import { render, screen } from "@testing-library/react";
3+
import userEvent from "@testing-library/user-event";
4+
import { beforeEach, describe, expect, it, vi } from "vitest";
5+
import { QuestionPermission } from "./QuestionPermission";
6+
import { useQuestionDraftStore } from "./questionDraftStore";
7+
import type { PermissionToolCall } from "./types";
8+
9+
const toolCall = {
10+
toolCallId: "question-1",
11+
title: "Questions",
12+
_meta: {
13+
codeToolKind: "question",
14+
questions: [
15+
{
16+
question: "Which framework?",
17+
header: "Framework",
18+
options: [{ label: "React" }, { label: "Vue" }],
19+
},
20+
{
21+
question: "Which color?",
22+
header: "Color",
23+
options: [{ label: "Red" }, { label: "Blue" }],
24+
},
25+
],
26+
},
27+
} as unknown as PermissionToolCall;
28+
29+
function renderQuestion() {
30+
const onSelect = vi.fn();
31+
const onCancel = vi.fn();
32+
const view = render(
33+
<Theme>
34+
<QuestionPermission
35+
toolCall={toolCall}
36+
options={[]}
37+
onSelect={onSelect}
38+
onCancel={onCancel}
39+
/>
40+
</Theme>,
41+
);
42+
return { onSelect, onCancel, view };
43+
}
44+
45+
describe("QuestionPermission", () => {
46+
beforeEach(() => {
47+
useQuestionDraftStore.setState({ drafts: new Map() });
48+
});
49+
50+
it("restores in-progress answers when the card remounts", async () => {
51+
const user = userEvent.setup();
52+
const first = renderQuestion();
53+
54+
await user.click(screen.getByText("React"));
55+
await user.click(screen.getByText("Next"));
56+
expect(screen.getByText("Which color?")).toBeDefined();
57+
58+
// Switching chats unmounts the card; the store keeps the draft.
59+
first.view.unmount();
60+
61+
const second = renderQuestion();
62+
expect(screen.getByText("Which color?")).toBeDefined();
63+
64+
await user.click(screen.getByText("Blue"));
65+
await user.click(screen.getByText("Next"));
66+
67+
// The review step summarizes the answer given before the remount.
68+
expect(screen.getByText("Ready to submit your answers?")).toBeDefined();
69+
expect(screen.getByText("React")).toBeDefined();
70+
71+
const submitOptions = screen.getAllByText("Submit");
72+
await user.click(submitOptions[submitOptions.length - 1] as HTMLElement);
73+
74+
expect(second.onSelect).toHaveBeenCalledWith("_submit", undefined, {
75+
"Which framework?": "React",
76+
"Which color?": "Blue",
77+
});
78+
expect(useQuestionDraftStore.getState().drafts.size).toBe(0);
79+
});
80+
81+
it("clears the draft when the card is cancelled", async () => {
82+
const user = userEvent.setup();
83+
const { onCancel } = renderQuestion();
84+
85+
await user.click(screen.getByText("React"));
86+
await user.click(screen.getByText("Next"));
87+
await user.click(screen.getByText("Red"));
88+
await user.click(screen.getByText("Next"));
89+
90+
await user.click(screen.getByText("Cancel"));
91+
92+
expect(onCancel).toHaveBeenCalledTimes(1);
93+
expect(useQuestionDraftStore.getState().drafts.size).toBe(0);
94+
});
95+
});

0 commit comments

Comments
 (0)