Skip to content

Commit 2e05b82

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 2e05b82

10 files changed

Lines changed: 592 additions & 71 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: 108 additions & 10 deletions
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>,
@@ -3191,23 +3209,88 @@ export class SessionService {
31913209
customInput,
31923210
answers,
31933211
);
3194-
if (!answerPrompt) {
3212+
if (answerPrompt) {
3213+
await this.sendCloudPrompt(
3214+
{ ...session, cloudStatus: cloudStatus ?? session.cloudStatus },
3215+
answerPrompt,
3216+
);
3217+
this.d.log.info("Permission answer resumed terminal cloud run", {
3218+
taskId: session.taskId,
3219+
toolCallId,
3220+
optionId,
3221+
});
3222+
} else {
31953223
this.d.log.info("Dropped permission response for terminal cloud run", {
31963224
taskId: session.taskId,
31973225
toolCallId,
31983226
optionId,
31993227
});
3200-
return;
32013228
}
3202-
await this.sendCloudPrompt(
3203-
{ ...session, cloudStatus: cloudStatus ?? session.cloudStatus },
3204-
answerPrompt,
3205-
);
3206-
this.d.log.info("Permission answer resumed terminal cloud run", {
3207-
taskId: session.taskId,
3229+
await this.persistCloudPermissionResolution(
3230+
session.taskId,
3231+
permission?.taskRunId ?? session.taskRunId,
32083232
toolCallId,
3233+
requestId,
32093234
optionId,
3210-
});
3235+
);
3236+
}
3237+
3238+
/**
3239+
* Record a response to a permission request whose sandbox is gone. A live
3240+
* sandbox writes `_posthog/permission_resolved` to the run log when it
3241+
* resolves a request, but a request answered after its run terminalized has
3242+
* no sandbox left to do that — without this record the request stays
3243+
* pending in the persisted log forever, and every future derivation (app
3244+
* restart, another device, a session rebuild) re-surfaces the
3245+
* already-answered question as a fresh card.
3246+
*/
3247+
private async persistCloudPermissionResolution(
3248+
taskId: string,
3249+
taskRunId: string,
3250+
toolCallId: string,
3251+
requestId: string | undefined,
3252+
optionId: string,
3253+
): Promise<void> {
3254+
if (!requestId) return;
3255+
this.markCloudPermissionResponded(requestId);
3256+
3257+
const client = await this.d.getAuthenticatedClient();
3258+
if (!client) return;
3259+
try {
3260+
await client.appendTaskRunLog(taskId, taskRunId, [
3261+
{
3262+
type: "notification",
3263+
timestamp: new Date().toISOString(),
3264+
notification: {
3265+
method: POSTHOG_NOTIFICATIONS.PERMISSION_RESOLVED,
3266+
params: { requestId, toolCallId, optionId },
3267+
},
3268+
},
3269+
]);
3270+
} catch (error) {
3271+
this.d.log.warn("Failed to persist permission resolution to run log", {
3272+
taskId,
3273+
taskRunId,
3274+
toolCallId,
3275+
error,
3276+
});
3277+
}
3278+
}
3279+
3280+
private markCloudPermissionResponded(requestId: string): void {
3281+
this.respondedCloudPermissionRequestIds.add(requestId);
3282+
// add() grows the set by at most one, so one eviction restores the cap.
3283+
if (
3284+
this.respondedCloudPermissionRequestIds.size >
3285+
MAX_RESPONDED_PERMISSION_REQUEST_IDS
3286+
) {
3287+
const oldest = this.respondedCloudPermissionRequestIds
3288+
.values()
3289+
.next().value;
3290+
if (oldest !== undefined) {
3291+
this.respondedCloudPermissionRequestIds.delete(oldest);
3292+
}
3293+
}
32113294
}
32123295

32133296
// --- Permissions ---
@@ -3269,6 +3352,7 @@ export class SessionService {
32693352
session,
32703353
permission,
32713354
toolCallId,
3355+
cloudRequestId,
32723356
optionId,
32733357
customInput,
32743358
answers,
@@ -3292,6 +3376,7 @@ export class SessionService {
32923376
session,
32933377
permission,
32943378
toolCallId,
3379+
cloudRequestId,
32953380
optionId,
32963381
customInput,
32973382
answers,
@@ -3301,6 +3386,10 @@ export class SessionService {
33013386
}
33023387
throw error;
33033388
}
3389+
// The live sandbox persists its own resolved marker; remember the
3390+
// response locally so a snapshot fetched before that marker flushes
3391+
// to storage cannot re-surface the question.
3392+
this.markCloudPermissionResponded(cloudRequestId);
33043393
} else {
33053394
await this.d.trpc.agent.respondToPermission.mutate({
33063395
taskRunId: session.taskRunId,
@@ -3352,8 +3441,16 @@ export class SessionService {
33523441
try {
33533442
if (session.isCloud && isTerminalStatus(session.cloudStatus)) {
33543443
// The run is over — the card was resolved locally above and there is no
3355-
// live permission promise left to reject.
3444+
// live permission promise left to reject. Persist the dismissal so the
3445+
// request is not re-derived as pending from the run log later.
33563446
this.cloudPermissionRequestIds.delete(toolCallId);
3447+
await this.persistCloudPermissionResolution(
3448+
session.taskId,
3449+
permission?.taskRunId ?? session.taskRunId,
3450+
toolCallId,
3451+
cloudRequestId,
3452+
"cancelled",
3453+
);
33573454
return;
33583455
}
33593456
if (session.isCloud && cloudRequestId) {
@@ -3363,6 +3460,7 @@ export class SessionService {
33633460
optionId: "reject_with_feedback",
33643461
customInput: "User cancelled the permission request.",
33653462
});
3463+
this.markCloudPermissionResponded(cloudRequestId);
33663464
} else {
33673465
await this.d.trpc.agent.cancelPermission.mutate({
33683466
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)