From a45152437255d232d602d056d9fd942fae97e567 Mon Sep 17 00:00:00 2001 From: Lance Woodson <65194841+woodsonl@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:29:03 -0500 Subject: [PATCH] fix(tui): stop fetching placeholder session id on --continue --continue seeded the session route with sessionID "dummy", which the route eagerly fetched; the server rejects it because ids must start with ses_, so cold start showed "Unexpected server error". Seed an empty id and skip the fetch instead, and let the continue effect resolve the real session. Also make an explicit --session win over -c, and navigate home when -c finds no previous session instead of leaving a blank screen. Adds a regression test that fails on dev (FAST_BOOT + delayed /session response) and asserts no request is made for the placeholder id. --- packages/tui/src/app.tsx | 44 +++++++++++------- packages/tui/src/routes/session/index.tsx | 3 ++ packages/tui/test/app-lifecycle.test.tsx | 55 +++++++++++++++++------ 3 files changed, 74 insertions(+), 28 deletions(-) diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index 6852311f9e84..0ff0c992dcbc 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -288,7 +288,10 @@ export const run = Effect.fn("Tui.run")(function* (input: TuiInput) { input.args.continue ? { type: "session", - sessionID: "dummy", + // Placeholder until the continue effect resolves the + // real id from the session list. Empty, not "dummy", + // so no session API call fires for an invalid id. + sessionID: "", } : undefined } @@ -503,23 +506,34 @@ function App(props: { onSnapshot?: () => Promise; pluginHost: TuiPlugi let continued = false createEffect(() => { // When using -c, session list is loaded in blocking phase, so we can navigate at "partial" - if (continued || sync.status === "loading" || !args.continue) return + // An explicit --session wins over -c; onMount already navigated to it. + if (continued || sync.status === "loading" || !args.continue || args.sessionID) return const match = sync.data.session .toSorted((a, b) => b.time.updated - a.time.updated) .find((x) => x.parentID === undefined)?.id - if (match) { - continued = true - if (args.fork) { - void sdk.client.session.fork({ sessionID: match }).then((result) => { - if (result.data?.id) { - route.navigate({ type: "session", sessionID: result.data.id }) - } else { - toast.show({ message: "Failed to fork session", variant: "error" }) - } - }) - } else { - route.navigate({ type: "session", sessionID: match }) - } + if (!match) return + continued = true + if (args.fork) { + void sdk.client.session.fork({ sessionID: match }).then((result) => { + if (result.data?.id) { + route.navigate({ type: "session", sessionID: result.data.id }) + } else { + toast.show({ message: "Failed to fork session", variant: "error" }) + } + }) + } else { + route.navigate({ type: "session", sessionID: match }) + } + }) + + // No resumable session exists: leave the placeholder route so the home + // screen renders instead of a blank session view. + let drained = false + createEffect(() => { + if (drained || sync.status !== "complete" || !args.continue) return + drained = true + if (route.data.type === "session" && !route.data.sessionID && !args.sessionID) { + route.navigate({ type: "home" }) } }) diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index 2b54f21671b3..fbc48d96c7ad 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -285,6 +285,9 @@ export function Session() { createEffect(() => { const sessionID = route.sessionID + // Empty id is the --continue placeholder before the real id resolves from + // the session list; skip fetch and let the navigation re-trigger this. + if (!sessionID) return void (async () => { const previousWorkspace = untrack(() => project.workspace.current()) const result = await sdk.client.session.get({ sessionID }, { throwOnError: true }) diff --git a/packages/tui/test/app-lifecycle.test.tsx b/packages/tui/test/app-lifecycle.test.tsx index 8cd40315ccc5..2512d9c1fc98 100644 --- a/packages/tui/test/app-lifecycle.test.tsx +++ b/packages/tui/test/app-lifecycle.test.tsx @@ -61,23 +61,34 @@ test("SIGHUP clears title and disposes scoped resources once", async () => { }) test("app.exit prints the session epilogue after scoped cleanup", async () => { + // FAST_BOOT mounts the session route before the continue effect resolves + // the real id, mirroring how the app runs in practice. Saved/restored so + // the env never leaks across test files. + const ORIGINAL_FAST_BOOT = process.env.OPENCODE_FAST_BOOT + process.env.OPENCODE_FAST_BOOT = "1" const setup = await createTestRenderer({ width: 80, height: 24, useThread: false }) const core = await import("@opentui/core") mock.module("@opentui/core", () => ({ ...core, createCliRenderer: async () => setup.renderer })) const events = createEventSource() + const requested: string[] = [] + const demoSession = { + id: "ses_demo", + title: "Demo session", + slug: "ses_demo", + projectID: "project", + directory, + version: "0.0.0-test", + time: { created: 0, updated: 0 }, + } const calls = createFetch((url) => { - if (url.pathname === "/session") - return json([ - { - id: "dummy", - title: "Demo session", - slug: "dummy", - projectID: "project", - directory, - version: "0.0.0-test", - time: { created: 0, updated: 0 }, - }, - ]) + requested.push(url.pathname) + // Delay the session list so the mounted session route fires its eager + // fetch before the continue effect navigates. With the placeholder bug + // this produces /session/dummy* requests; with the fix it cannot. + if (url.pathname === "/session") { + return new Promise((resolve) => setTimeout(() => resolve(json([demoSession])), 50)) + } + if (url.pathname === "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/session/ses_demo") return json(demoSession) }) const originalWrite = process.stdout.write.bind(process.stdout) let stdout = "" @@ -115,15 +126,33 @@ test("app.exit prints the session epilogue after scoped cleanup", async () => { await ready await setup.renderOnce() await setup.renderOnce() + // Poll until the session route fetched the real id, so the test never + // races the 50ms mock delay. + for (let waited = 0; !requested.includes("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/session/ses_demo") && waited < 2000; waited += 20) { + await new Promise((resolve) => setTimeout(resolve, 20)) + } api?.keymap.dispatchCommand("app.exit") await task expect(stdout).toContain("Demo session") - expect(stdout).toContain("opencode -s dummy") + expect(stdout).toContain("opencode -s ses_demo") + // Regression: --continue must never fetch a placeholder session id. + // The server rejects ids without the "ses" prefix (400), which the + // session route surfaces as an error toast. Every /session/:id request + // after the list itself must carry the resolved, valid id. + expect(requested).not.toContain("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/session/dummy") + expect(requested).toContain("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/session/ses_demo") + const fetchedIds = requested + .filter((p) => p.startsWith("/session/") && p !== "/session" && p !== "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/session/status") + .map((p) => p.split("/")[2]) + expect(fetchedIds.length).toBeGreaterThan(0) + expect(fetchedIds.every((id) => id === "ses_demo")).toBe(true) } finally { process.stdout.write = originalWrite if (!setup.renderer.isDestroyed) setup.renderer.destroy() mock.restore() + if (ORIGINAL_FAST_BOOT === undefined) delete process.env.OPENCODE_FAST_BOOT + else process.env.OPENCODE_FAST_BOOT = ORIGINAL_FAST_BOOT } })