From d07d9ae0da9ff5178d6456c8c5f5552dc327dc2f Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 31 Jul 2026 10:46:43 -0400 Subject: [PATCH] fix(tui): remove shells from their location (#39885) --- packages/tui/src/context/data.tsx | 21 ++++++-- .../src/routes/session/composer/shell-tab.tsx | 9 ++-- packages/tui/test/cli/tui/data.test.tsx | 52 ++++++++++++++++--- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/packages/tui/src/context/data.tsx b/packages/tui/src/context/data.tsx index af023c2c9f..383310b3f7 100644 --- a/packages/tui/src/context/data.tsx +++ b/packages/tui/src/context/data.tsx @@ -45,6 +45,7 @@ const messageIDFromEvent = (eventID: string) => eventID.replace(/^evt_/, "msg_") // server cannot recover their Location when settling them. Preserve the event Location // until MCP elicitations carry session ownership. export type FormWithLocation = FormInfo & { readonly location?: LocationRef } +type ShellWithLocation = ShellInfo & { readonly location: LocationRef } type LocationData = { info?: LocationGetOutput @@ -61,7 +62,7 @@ type LocationData = { websearch?: WebSearchProvider[] // Currently running shell commands for this location, keyed by shell id. Entries are removed // once the command exits or is deleted, so this only ever holds in-flight shells. - shell?: Record + shell?: Record skill?: SkillInfo[] } @@ -851,7 +852,10 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ case "shell.created": setStore("location", locationKey(event.location ?? defaultLocation()), (data) => ({ ...data, - shell: { ...data?.shell, [event.data.info.id]: event.data.info }, + shell: { + ...data?.shell, + [event.data.info.id]: { ...event.data.info, location: event.location ?? defaultLocation() }, + }, })) break case "shell.exited": @@ -1106,7 +1110,18 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ const key = locationKey(response.location) setStore("location", key, { ...store.location[key], - shell: Object.fromEntries(response.data.map((info) => [info.id, info])), + shell: Object.fromEntries( + response.data.map((info) => [ + info.id, + { + ...info, + location: { + directory: response.location.directory, + workspaceID: response.location.workspaceID, + }, + }, + ]), + ), }) }) }, diff --git a/packages/tui/src/routes/session/composer/shell-tab.tsx b/packages/tui/src/routes/session/composer/shell-tab.tsx index 3eed9deae5..6c0f02ce2a 100644 --- a/packages/tui/src/routes/session/composer/shell-tab.tsx +++ b/packages/tui/src/routes/session/composer/shell-tab.tsx @@ -2,7 +2,6 @@ import { createMemo, For, Show, createEffect, onMount, onCleanup } from "solid-j import { createStore } from "solid-js/store" import { TextAttributes, ScrollBoxRenderable } from "@opentui/core" import { useData } from "../../../context/data" -import { useLocation } from "../../../context/location" import { useClient } from "../../../context/client" import { useTheme } from "../../../context/theme" import { Keymap } from "../../../context/keymap" @@ -10,13 +9,14 @@ import { useComposerTab } from "./index" export function ShellTab(props: { sessionID: string }) { const data = useData() - const location = useLocation() const client = useClient() const theme = useTheme() const composer = useComposerTab() const shortcuts = Keymap.useShortcuts() - const entries = createMemo(() => data.shell.listBySession(props.sessionID).filter((shell) => shell.status === "running")) + const entries = createMemo(() => + data.shell.listBySession(props.sessionID).filter((shell) => shell.status === "running"), + ) const [store, setStore] = createStore({ selected: 0 }) let scroll: ScrollBoxRenderable | undefined @@ -83,10 +83,9 @@ export function ShellTab(props: { sessionID: string }) { run() { const entry = selectedEntry() if (!entry) return - const ref = location.current void client.api.shell.remove({ id: entry.id, - location: ref ? { directory: ref.directory, workspace: ref.workspaceID } : undefined, + location: { directory: entry.location.directory, workspace: entry.location.workspaceID }, }) }, }, diff --git a/packages/tui/test/cli/tui/data.test.tsx b/packages/tui/test/cli/tui/data.test.tsx index c814f57679..61b7070551 100644 --- a/packages/tui/test/cli/tui/data.test.tsx +++ b/packages/tui/test/cli/tui/data.test.tsx @@ -9,7 +9,11 @@ import { createEffect, onMount, type ParentProps } from "solid-js" import { ConfigProvider } from "../../../src/config" import { ClientProvider, useClient } from "../../../src/context/client" import { DataProvider as DataProviderBase, useData } from "../../../src/context/data" +import { Keymap } from "../../../src/context/keymap" import { LocationProvider, useLocation } from "../../../src/context/location" +import { RouteProvider } from "../../../src/context/route" +import { ThemeProvider } from "../../../src/context/theme" +import { Composer } from "../../../src/routes/session/composer" import { createSessionRows, type SessionRow } from "../../../src/routes/session/rows" import { createApi, createEventStream, createFetch, directory, json, worktree } from "../../fixture/tui-client" import { TestTuiContexts } from "../../fixture/tui-environment" @@ -1782,12 +1786,19 @@ test("refreshes references after updates", async () => { test("keeps shell state scoped to location", async () => { const events = createEventStream() const other = "/tmp/opencode/other" - const calls = createFetch((url) => { + const workspace = "ws_other" + let removed: URL | undefined + const calls = createFetch((url, request) => { + if (url.pathname === "/api/shell/sh_other" && request.method === "DELETE") { + removed = url + return new Response(null, { status: 204 }) + } if (url.pathname !== "/api/shell") return const requestDirectory = url.searchParams.get("location[directory]") return json({ location: { directory: requestDirectory ?? directory, + workspaceID: url.searchParams.get("location[workspace]") ?? undefined, project: { id: "proj_test", directory: requestDirectory ?? directory }, }, data: [ @@ -1798,7 +1809,7 @@ test("keeps shell state scoped to location", async () => { cwd: requestDirectory ?? directory, shell: "/bin/sh", file: "/tmp/opencode-shell", - metadata: { sessionID: requestDirectory === other ? "ses_other" : "ses_default" }, + metadata: { sessionID: "ses_shared" }, time: { started: 1 }, }, ], @@ -1808,7 +1819,15 @@ test("keeps shell state scoped to location", async () => { function Probe() { data = useData() - return + return ( + + + Promise.resolve({}) }}> + + + + + ) } const app = await testRender(() => ( @@ -1822,19 +1841,31 @@ test("keeps shell state scoped to location", async () => { )) + app.renderer.start() try { await wait(() => data.shell.list().some((shell) => shell.id === "sh_default")) - await data.shell.sync({ directory: other }) + await data.shell.sync({ directory: other, workspaceID: workspace }) expect(data.shell.list().map((shell) => shell.id)).toEqual(["sh_default"]) - expect(data.shell.list({ directory: other }).map((shell) => shell.id)).toEqual(["sh_other"]) + expect(data.shell.list({ directory: other, workspaceID: workspace }).map((shell) => shell.id)).toEqual(["sh_other"]) + expect(data.shell.listBySession("ses_shared").map((shell) => [shell.id, shell.location.directory])).toEqual([ + ["sh_default", directory], + ["sh_other", other], + ]) + + await app.waitForFrame((frame) => frame.includes("pnpm dev")) + app.mockInput.pressArrow("down") + app.mockInput.pressKey("d", { ctrl: true }) + await wait(() => removed !== undefined) + expect(removed?.searchParams.get("location[directory]")).toBe(other) + expect(removed?.searchParams.get("location[workspace]")).toBe(workspace) events.emit({ id: "evt_shell_created", created: 0, type: "shell.created", - location: { directory: other }, + location: { directory: other, workspaceID: workspace }, data: { info: { id: "sh_live_other", @@ -1843,13 +1874,18 @@ test("keeps shell state scoped to location", async () => { cwd: other, shell: "/bin/sh", file: "/tmp/opencode-shell-live", - metadata: { sessionID: "ses_other" }, + metadata: { sessionID: "ses_shared" }, time: { started: 2 }, }, }, }) - await wait(() => data.shell.list({ directory: other }).some((shell) => shell.id === "sh_live_other")) + await wait(() => + data.shell.list({ directory: other, workspaceID: workspace }).some((shell) => shell.id === "sh_live_other"), + ) expect(data.shell.list().map((shell) => shell.id)).toEqual(["sh_default"]) + expect(data.shell.listBySession("ses_shared").find((shell) => shell.id === "sh_live_other")?.location.directory).toBe( + other, + ) } finally { app.renderer.destroy() }