fix(canvas): stop local generation stranding the canvas on "Generating"#2882
fix(canvas): stop local generation stranding the canvas on "Generating"#2882raquelmsmith wants to merge 2 commits into
Conversation
The freeform canvas derived its "is generating" state differently for cloud and local runs. Cloud falls back to the persisted run record and stops once the run status is terminal, but local relied solely on the live ACP session being connecting/connected. If that session went stale or the agent stalled without a clean disconnect, `running` stayed true and the canvas was pinned on "Generating" indefinitely — the "canvas keeps timing out" symptom. Extract the run-status derivation into a pure, tested helper and give the local path the same terminal-run guard cloud already has: a terminal run record (completed/failed/cancelled) always ends "running", so a stuck session can no longer strand the canvas. Generated-By: PostHog Code Task-Id: 8085fa09-5b8c-475d-930e-582a5bc920b3
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(canvas): stop local generation stran..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (genTaskLoading) return true; | ||
|
|
||
| if (latestRun?.environment === "cloud") { | ||
| const cloudStatus = session?.cloudStatus ?? latestRun?.status ?? null; |
There was a problem hiding this comment.
latestRun?.environment === "cloud" branch, TypeScript narrows latestRun to be non-undefined (the optional-chain comparison can only be true when latestRun is defined). The ?. and ?? null are therefore superfluous — latestRun.status is always a TaskRunStatus here, and isTerminalStatus already handles null/undefined the same way.
| const cloudStatus = session?.cloudStatus ?? latestRun?.status ?? null; | |
| const cloudStatus = session?.cloudStatus ?? latestRun.status; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| it.each<[string, Run, Session | undefined, boolean]>([ | ||
| ["in_progress, no session", run("cloud", "in_progress"), undefined, true], | ||
| [ | ||
| "session cloudStatus terminal overrides run record", | ||
| run("cloud", "in_progress"), | ||
| session("connected", "completed"), | ||
| false, | ||
| ], | ||
| ["run record terminal", run("cloud", "failed"), undefined, false], | ||
| ])("cloud: %s", (_label, latestRun, sess, expected) => { | ||
| expect( | ||
| isCanvasGenerationRunning({ | ||
| genTaskId: "t1", | ||
| genTaskLoading: false, | ||
| latestRun, | ||
| session: sess, | ||
| }), | ||
| ).toBe(expected); | ||
| }); | ||
|
|
||
| it.each<[string, Run, Session | undefined, boolean]>([ | ||
| [ | ||
| "session connected", | ||
| run("local", "in_progress"), | ||
| session("connected"), | ||
| true, | ||
| ], | ||
| [ | ||
| "session connecting", | ||
| run("local", "in_progress"), | ||
| session("connecting"), | ||
| true, | ||
| ], | ||
| ["no live session", run("local", "in_progress"), undefined, false], | ||
| [ | ||
| "session disconnected", | ||
| run("local", "in_progress"), | ||
| session("disconnected"), | ||
| false, | ||
| ], | ||
| // The regression: a terminal run record must stop "running" even if the | ||
| // live session is still (stale) reporting connected — otherwise the canvas | ||
| // is stranded on "Generating" forever. | ||
| [ | ||
| "terminal run wins over stale connected session", | ||
| run("local", "completed"), | ||
| session("connected"), | ||
| false, | ||
| ], | ||
| [ | ||
| "failed run wins over stale connected session", | ||
| run("local", "failed"), | ||
| session("connected"), | ||
| false, | ||
| ], | ||
| ])("local: %s", (_label, latestRun, sess, expected) => { | ||
| expect( | ||
| isCanvasGenerationRunning({ | ||
| genTaskId: "t1", | ||
| genTaskLoading: false, | ||
| latestRun, | ||
| session: sess, | ||
| }), | ||
| ).toBe(expected); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
latest_run yet (latestRun: undefined) combined with an active local session. After loading, if genTask.latest_run is absent (e.g., a task whose first run hasn't been created yet), the code falls through to the local path and the session check determines the result. The behaviour is correct — isTerminalStatus(undefined) returns false, so a connected session returns true — but it's an untested code path that could silently regress if the fallthrough logic changes.
…d local path Address PR review: - Drop superfluous optional-chain/`?? null` in the cloud branch where `latestRun` is already narrowed to defined. - Add a test for a loaded task with no run record yet plus a connected local session, exercising the previously untested fallthrough. Generated-By: PostHog Code Task-Id: 670cc6fb-860d-439a-a93b-97e9a82bd5aa
1 participant
Problem
A freeform canvas generated by a local task could get stuck showing "Generating" forever — the "canvas keeps timing out" experience.
FreeformCanvasViewderives itsrunningstate differently per environment:latest_run.status(or the sessioncloudStatus) is terminal.connecting/connected.So if a local session went stale or the agent stalled without cleanly disconnecting,
runningstayedtrue,isGeneratingnever cleared, and the canvas was pinned on the "Generating" empty state with no escape.Changes
canvasGenerationStatus.ts).completed/failed/cancelled) always endsrunning, so a stuck/stale session can no longer strand the canvas. Cloud behavior is unchanged.How did you test this?
I'm an agent (PostHog Slack app). I did not drive the desktop app manually. I added
canvasGenerationStatus.test.ts(11 cases, cloud + local, incl. the regression: a terminal run record must win over a staleconnectedsession) —vitest runpasses locally.@posthog/uitypecheck shows no errors in the changed files, and Biome check passes on them.Agent context
Investigated from a Slack thread: a user's self-driving canvas "errored out, then kept timing out." Traced his session — the canvas generation ran as a local repo-less task (
workspaceMode: "local"), and the only frontend defect that matches an indefinite "Generating" state is the cloud/local asymmetry in the run-status derivation. The canvas's own runtime error renders in theabout:srcdocsandbox and isn't captured in our product analytics, so this targets the most likely stuck-state mechanism rather than a specific captured stack trace — worth a sanity check from whoever owns the freeform canvas.Created with PostHog Code from a Slack thread