From e7ce3b8e563d4604e5cebb89a0fea0a65dde2e22 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 18 Oct 2026 14:38:13 +0200 Subject: [PATCH] fix(story-mode): live helper steps behind an unchanged page, a clean reopen, no wedged view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review round 7 (PR review): - A helper's drawer froze while the helper worked: a background helper writes only to its own transcript, so the parent page answered 304 and the refresh returned before anything refreshed the drawer. An unchanged page now still refreshes an open drawer's working helpers (ETag'd). - A helper messaged twice read as superseded ("The trail stops here…"): its one-page run took its ending from the first turn. It takes the last. - More than 60 turns in a small file listed the newest 60 with no hint; `older` is now set whenever the list leaves turns out. - One /story request that never settled wedged the view, even across a reopen. Requests carry a token and a 30 s timeout, and opening resets the request state, with what was announced and shown. - The background canvases (megabytes each at 2x) are released when Story Mode closes. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/story-mode.ts | 3 ++ src/web/public/story-mode.js | 49 +++++++++++++++++++++---- src/web/routes/story-routes.ts | 5 +-- test/routes/story-routes.test.ts | 10 ++++++ test/story-mode-ui.test.ts | 62 ++++++++++++++++++++++++++++++++ test/story-mode.test.ts | 12 +++++++ 6 files changed, 132 insertions(+), 9 deletions(-) diff --git a/src/story-mode.ts b/src/story-mode.ts index c04c2458..7082a720 100644 --- a/src/story-mode.ts +++ b/src/story-mode.ts @@ -2512,10 +2512,13 @@ export function buildHelperPage(turns: StoryTurn[], finished?: boolean): StoryPa if (!turns.length) return null; const first = turns[0]; const last = turns[turns.length - 1]; + // One helper's whole run as one page: how it ended is the LAST turn's (the first one "ended" only + // because the helper was messaged again, which would read as superseded). return buildStoryPage({ ...first, events: turns.flatMap((t) => t.events), ended: finished ?? last.ended, + endedBy: last.endedBy === 'next prompt' ? undefined : last.endedBy, interrupted: false, endedAt: last.endedAt, durationMs: undefined, diff --git a/src/web/public/story-mode.js b/src/web/public/story-mode.js index 3db4c310..3faf1209 100644 --- a/src/web/public/story-mode.js +++ b/src/web/public/story-mode.js @@ -33,6 +33,8 @@ const STORY_REFRESH_DEBOUNCE_MS = 800; const STORY_POLL_MS = 4000; /** How often a finished newest page is checked for a turn no hook announced. */ const STORY_IDLE_POLL_MS = 15000; +/** A /story request that has not settled by then counts as failed, so the view never wedges. */ +const STORY_REQUEST_TIMEOUT_MS = 30000; /** * Widest stage a scene is drawn on, in virtual units. A panel stretched across a whole row keeps its * art together in a centered stage instead of spreading it to both ends; the background still fills it. @@ -1579,6 +1581,15 @@ Object.assign(CodemanApp.prototype, { st.epoch++; st.first = true; st.drawer = null; + st.drawerHolds = false; + // What was said and shown belongs to the last session drawn, not this one. + st.announced = undefined; + st.knownTurns = undefined; + st.shownPageId = undefined; + st.picked = false; + // A request still out from before is ignored (its epoch is old); it must not block this one. + st.inFlight = 0; + st.again = false; st.root.hidden = false; document.body.classList.add('story-open'); this._storyApplyTheme(); @@ -1604,6 +1615,8 @@ Object.assign(CodemanApp.prototype, { this._storyStopRuntime(); this._storyCloseDrawer({ refocus: false }); for (const rt of st.panels) this._storyDropPanel(rt); + // Full-size backing canvases (megabytes each at 2x): not kept once Story Mode is closed. + storyBgCache.clear(); st.panels = []; st.pageEl.replaceChildren(); st.wireList.replaceChildren(); @@ -1874,16 +1887,28 @@ Object.assign(CodemanApp.prototype, { st.again = true; return; } - st.inFlight = true; + const mine = (st.requestSeq = (st.requestSeq || 0) + 1); + st.inFlight = mine; const epoch = st.epoch; try { const qs = st.turnId ? `?turn=${encodeURIComponent(st.turnId)}` : ''; - const res = await this._api(`/api/sessions/${encodeURIComponent(st.sessionId)}/story${qs}`); + // A request that never settles must not wedge the view: after the timeout it counts as failed. + let timer; + const timeout = new Promise((resolve) => (timer = setTimeout(() => resolve(null), STORY_REQUEST_TIMEOUT_MS))); + const res = await Promise.race([ + this._api(`/api/sessions/${encodeURIComponent(st.sessionId)}/story${qs}`), + timeout, + ]).finally(() => clearTimeout(timer)); if (epoch !== st.epoch || !st.open) return; // The route sends an ETag and no-cache, so the browser revalidates and an unchanged page comes // back from its cache with the same tag: nothing to parse, nothing to redraw. const etag = res?.ok ? res.headers?.get?.('etag') || null : null; - if (etag && etag === st.etag && !st.error) return; + if (etag && etag === st.etag && !st.error) { + // The page holds still, but a helper still working writes only to its own transcript (a + // foreground helper adds nothing to this one until it returns): its open drawer refreshes. + this._storyRefreshWorkingHelpers(); + return; + } let body = null; try { body = res ? await res.json() : null; @@ -1916,14 +1941,24 @@ Object.assign(CodemanApp.prototype, { } this._storyRender(); } finally { - st.inFlight = false; - if (st.again) { - st.again = false; - this._storyScheduleRefresh(250); + if (st.inFlight === mine) { + st.inFlight = 0; + if (st.again) { + st.again = false; + this._storyScheduleRefresh(250); + } } } }, + /** An open drawer's helpers that are still working fetch their newest steps (each ETag'd). */ + _storyRefreshWorkingHelpers() { + const st = this._story; + if (!st?.drawer) return; + for (const box of st.drawerBody.querySelectorAll('.story-helper-steps')) + if (box.dataset.working === '1') this._storyFillHelperSteps(box, box.dataset.agent, true); + }, + // ── Runtime: frame loop, observers, poll, Escape ── _storyStartRuntime() { diff --git a/src/web/routes/story-routes.ts b/src/web/routes/story-routes.ts index 45635466..74917afd 100644 --- a/src/web/routes/story-routes.ts +++ b/src/web/routes/story-routes.ts @@ -30,7 +30,7 @@ import { type StoryResponse, } from '../../types.js'; import { getCli } from '../../config/cli-registry/registry.js'; -import { buildHelperPage, buildStoryPage, buildStoryTurns, summarizeTurns } from '../../story-mode.js'; +import { STORY_LIMITS, buildHelperPage, buildStoryPage, buildStoryTurns, summarizeTurns } from '../../story-mode.js'; import { readSmallRegularFile, readStoryEntries, storyFileVersion } from '../../story-reader.js'; import { claudeProjectsDir, findClaudeTranscriptPath } from '../../utils/claude-transcript.js'; import { findSessionOrFail } from '../route-helpers.js'; @@ -169,7 +169,8 @@ export function registerStoryRoutes(app: FastifyInstance, ctx: SessionPort & Con supported: true, turns: summarizeTurns(turns), page: wanted ? buildStoryPage(wanted) : null, - ...(read.partial ? { older: true } : {}), + // Older turns exist that the list leaves out: before the part read, or past the listed ones. + ...(read.partial || turns.length > STORY_LIMITS.turnsListed ? { older: true } : {}), }, }; }); diff --git a/test/routes/story-routes.test.ts b/test/routes/story-routes.test.ts index 5b694d05..4828054b 100644 --- a/test/routes/story-routes.test.ts +++ b/test/routes/story-routes.test.ts @@ -202,6 +202,16 @@ describe('GET /api/sessions/:id/story', () => { expect(res.json().data).toMatchObject({ supported: true, turns: [], page: null }); }); + it('says when the list leaves turns out past the 60 it shows, even in a small file', async () => { + const lines: unknown[] = []; + for (let i = 0; i < 80; i++) lines.push(prompt(`job ${i}`), said('ok'), end(1000)); + writeFileSync(transcript, jsonl(lines)); + const { app } = await setup(); + const body = (await app.inject({ method: 'GET', url: URL })).json(); + expect(body.data.turns).toHaveLength(60); + expect(body.data.older).toBe(true); + }); + it('404s an unknown session', async () => { const { app } = await setup(); const res = await app.inject({ method: 'GET', url: '/api/sessions/nope/story' }); diff --git a/test/story-mode-ui.test.ts b/test/story-mode-ui.test.ts index feec7bab..cdc1a8e6 100644 --- a/test/story-mode-ui.test.ts +++ b/test/story-mode-ui.test.ts @@ -604,6 +604,68 @@ describe('story-mode.js overlay', () => { expect(app.openFilePreview).toHaveBeenLastCalledWith('/work/demo-shop/a.md', SID); }); + it("a page that holds still (304) still refreshes a working helper's steps in its open drawer", async () => { + const data = storyOf([ + prompt('docs'), + tool('a1', 'Agent', { description: 'Write README', subagent_type: 'general-purpose', prompt: 'Write it' }), + result('a1', 'Async agent launched successfully. agentId: h9', { + toolUseResult: { isAsync: true, status: 'async_launched', agentId: 'h9' }, + }), + end(1000), + ]); + boot(data); + const helperPage = storyOf([prompt('Write it'), ...read('r1', '/work/demo-shop/a.md')]).page; + let helperCalls = 0; + app._api = async (path: string) => { + if (path.includes('/story/helpers/h9')) { + helperCalls++; + return { + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => ({ success: true, data: { supported: true, helper: { agentId: 'h9' }, page: helperPage } }), + }; + } + // The parent page never changes while the helper works: the same tag every time. + return { + ok: true, + status: 200, + headers: { get: (h: string) => (h.toLowerCase() === 'etag' ? 'W/"same"' : null) }, + json: async () => ({ success: true, data }), + }; + }; + app.openStoryMode(SID); + await settle(); + sizeAndDraw(); + $('.story-panel[data-kind="backup"] .story-hit')!.click(); + await settle(); + const before = helperCalls; + expect(before).toBeGreaterThan(0); + await app._storyRefresh(); + await settle(); + expect(helperCalls).toBe(before + 1); + }); + + it('a request that never settled does not block the next open: the page draws', async () => { + const data = storyOf(demoTurn()); + boot(data); + app._api = () => new Promise(() => {}); // hangs forever + app.openStoryMode(SID); + await settle(); + app.closeStoryMode(); + app._api = async () => ({ + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => ({ success: true, data }), + }); + app.openStoryMode(SID); + await settle(); + expect($$('.story-panel').length).toBeGreaterThan(0); + // Reopened, nothing from before is announced as news. + expect($('.story-sr')!.textContent).toBe(''); + }); + it('Tab stays inside the overlay', async () => { boot(storyOf(demoTurn())); app.openStoryMode(SID); diff --git a/test/story-mode.test.ts b/test/story-mode.test.ts index 5b93c7fa..773a069d 100644 --- a/test/story-mode.test.ts +++ b/test/story-mode.test.ts @@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { STORY_LIMITS, + buildHelperPage, buildStoryPage, buildStoryTurns, classifyBashCommand, @@ -996,6 +997,17 @@ describe('buildStoryPage', () => { for (const [name, fn] of shapes) expect([name, ms(fn) < 250]).toEqual([name, true]); }); + it('a helper messaged twice reads as one finished run, not as superseded', () => { + const r = createStoryReducer({ sidechain: true }); + const lines = [prompt('write it'), said('Done.'), prompt('also the tests'), said('Done too.')].map((l) => ({ + ...(l as object), + isSidechain: true, + })); + const page = buildHelperPage(buildStoryTurns(lines.flatMap((l) => r.reduce(l))), true)!; + expect(page.superseded).toBeUndefined(); + expect(page.panels.at(-1)!.caption.story).not.toMatch(/trail stops here/); + }); + it('a python or node script fed by a here-document: the files it opens for writing, never guessed', () => { const files = (command: string) => heredocWrites(command, '/w').map((d) => d.file); expect(