From 197b10a3fea0576444ef06165dbdcaf372872276 Mon Sep 17 00:00:00 2001 From: LuckVd Date: Mon, 24 Aug 2026 00:45:50 +0800 Subject: [PATCH] fix(sidebar): read betterSidebar via ctx.get() in all internal consumers Under the DSH 0.1.1-rc.x web shell, the Cordis plugin loader registers the provided betterSidebar impl on a neighboring fiber that is not on the rendering fiber parent chain, so every direct ctx.betterSidebar read inside the plugin own tree throws cannot get property "betterSidebar" without inject and the panel dies inside RenderBoundary on page load (observed with 0.14.0 and 0.15.2 on npm-installed dsh 0.1.1-rc.2). This is the same failure class #135 fixed for the dsh-web-mobile path; declaring betterSidebar in the plugin own inject would deadlock the fiber (it waits for the service before running apply, but the service is only provided inside apply). ctx.get() resolves through the root reflect store and is unaffected by the fiber chain, so all 26 internal reads switch to it. External consumers keep using ctx.betterSidebar after injecting it, unchanged. Test mocks gain a matching get() shim. --- src/client/EditorHost.tsx | 8 +++---- src/client/SideChatView.tsx | 8 +++---- src/client/Sidebar.tsx | 34 ++++++++++++++--------------- src/client/index.tsx | 2 +- src/client/intercept.tsx | 2 +- tests/bottom-auto-terminal.spec.tsx | 1 + tests/editor-host.spec.tsx | 1 + tests/openpath-intercept.spec.ts | 3 +++ tests/sidebar-crash.spec.tsx | 1 + tests/turn-tail-intercept.spec.ts | 18 +++++++++------ 10 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/client/EditorHost.tsx b/src/client/EditorHost.tsx index a732a9f..37479c1 100644 --- a/src/client/EditorHost.tsx +++ b/src/client/EditorHost.tsx @@ -83,7 +83,7 @@ function treeWidthOf(tab: SidebarTab): number { /** Merge a patch into the tab's persisted meta (rides the layout). */ function patchMeta(ctx: Context, tab: SidebarTab, patch: Record): void { - ctx.betterSidebar?.updateTab(tab.id, { meta: { ...metaOf(tab), ...patch } }) + ctx.get('betterSidebar')?.updateTab(tab.id, { meta: { ...metaOf(tab), ...patch } }) } /** Clamp one dock width into the contract range. */ @@ -134,7 +134,7 @@ export function EditorHost(props: { */ const openFile = (absolute: string): void => { if (inPlace) { - ctx.betterSidebar?.updateTab(tab.id, { path: absolute, title: baseName(absolute) }) + ctx.get('betterSidebar')?.updateTab(tab.id, { path: absolute, title: baseName(absolute) }) } else { openSidebarFile(ctx, store, scope.sessionId, absolute) } @@ -300,7 +300,7 @@ export function EditorHost(props: { content: result.kind === 'text' ? result.content : '', truncated: result.truncated, head: result.kind === 'binary' ? result.head : undefined, - }, (head) => ctx.betterSidebar?.matchFileViewer(path, head), mediaUrlOf) + }, (head) => ctx.get('betterSidebar')?.matchFileViewer(path, head), mediaUrlOf) apply(outcome) }).catch((error: unknown) => { if (cancelled) return @@ -309,7 +309,7 @@ export function EditorHost(props: { return } } - apply(planFirstMatch(ctx.betterSidebar?.matchFileViewer(path), mediaUrlOf)) + apply(planFirstMatch(ctx.get('betterSidebar')?.matchFileViewer(path), mediaUrlOf)) return () => { cancelled = true; controller.abort() } }, [scope.sessionId, scope.cwd, path, ctx, showEmpty]) diff --git a/src/client/SideChatView.tsx b/src/client/SideChatView.tsx index 5fe9f0f..775d396 100644 --- a/src/client/SideChatView.tsx +++ b/src/client/SideChatView.tsx @@ -295,7 +295,7 @@ export function SideChatView(props: { setError(null) try { const { childId } = await api.sidechatStart(scope.sessionId) - ctx.betterSidebar?.updateTab(tab.id, { meta: { threadId: childId } }) + ctx.get('betterSidebar')?.updateTab(tab.id, { meta: { threadId: childId } }) } catch (cause) { setError(cause instanceof Error ? cause.message : String(cause)) } finally { @@ -319,7 +319,7 @@ export function SideChatView(props: { const title = threadDisplayTitle(display) if (title !== '' && title !== tab.title) { try { - ctx.betterSidebar?.updateTab(tab.id, { title }) + ctx.get('betterSidebar')?.updateTab(tab.id, { title }) } catch { // A stale title is cosmetic; the thread keeps working. } @@ -422,7 +422,7 @@ export function SideChatView(props: { * creates the thread on mount). */ const openNewThread = (): void => { setMenuOpen(false) - ctx.betterSidebar?.openTab({ type: 'sidechat' }, scope) + ctx.get('betterSidebar')?.openTab({ type: 'sidechat' }, scope) } /** Switch to an existing thread: parked for createTab, deduped to the @@ -431,7 +431,7 @@ export function SideChatView(props: { setMenuOpen(false) if (id === threadId) return parkSidechatReopen(id) - ctx.betterSidebar?.openTab({ type: 'sidechat' }, scope) + ctx.get('betterSidebar')?.openTab({ type: 'sidechat' }, scope) } const menuItems = useMemo(() => { diff --git a/src/client/Sidebar.tsx b/src/client/Sidebar.tsx index d7c8c07..0f12d3f 100644 --- a/src/client/Sidebar.tsx +++ b/src/client/Sidebar.tsx @@ -135,7 +135,7 @@ interface TabContentProps extends TabContentMemoKey { const TabContent = memo(function TabContent(props: TabContentProps) { const { tab, sessionId, cwd, expanded, onToggleDir, onReferenceFile, ctx, store, visible, onSubagentJump, onOpenDiff } = props const scope = { sessionId, cwd } - const descriptor = ctx.betterSidebar?.getTab(tab.type) + const descriptor = ctx.get('betterSidebar')?.getTab(tab.type) if (descriptor === undefined) { return } @@ -163,7 +163,7 @@ const TabContent = memo(function TabContent(props: TabContentProps) { * Tabs the user disabled in the side card settings are filtered out * entirely — re-enabling them is the settings page's job. */ function buildNewTabOptions(state: SidebarState, ctx: Context, scope: SessionScope): NewTabOption[] { - const service = ctx.betterSidebar + const service = ctx.get('betterSidebar') if (service === undefined) return [] return service.getTabs() .filter(d => !d.hidden && service.isTabEnabled(d.id)) @@ -229,7 +229,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // from going stale, mirroring the localeRevision mechanism above. const [tabsVersion, setTabsVersion] = useState(0) useEffect(() => { - const service = ctx.betterSidebar + const service = ctx.get('betterSidebar') if (service === undefined) return return service.subscribe(() => setTabsVersion(version => version + 1)) }, [ctx]) @@ -419,7 +419,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { try { const list = JSON.parse(event.data) as Array<{ uuid: string; title: string; command: string; exited: boolean }> if (!Array.isArray(list)) return - store.reduce(s => ctx.betterSidebar?.isTabEnabled('terminal') === false + store.reduce(s => ctx.get('betterSidebar')?.isTabEnabled('terminal') === false ? s : reconcileAgentTerminals(s, list)) } catch { @@ -475,13 +475,13 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { autoOpenPendingRef.current = null if (!detectNewDirectSubagent(baseline, ctx.sessions.list.getSnapshot(), sessionId)) return if (!store.getPrefs().autoOpenSubagent) return - if (ctx.betterSidebar?.isTabEnabled('subagent') === false) return + if (ctx.get('betterSidebar')?.isTabEnabled('subagent') === false) return store.reduce(s => s.panelOpen ? s : togglePanel(s)) // Pin the landing to the right panel: the auto-opened Subagent page must // appear where the panel just expanded, not in a bottom-panel pane the // user last touched. store.reduce(s => ({ ...s, activePane: firstLeaf(s.splits).id })) - ctx.betterSidebar?.openTab({ type: 'subagent', title: t('subagent') }) + ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent') }) }, AUTO_OPEN_DEBOUNCE_MS) autoOpenPendingRef.current = { baseline, timer } }, [sessionList, sessionId, store, ctx]) @@ -509,10 +509,10 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { if (sessionId === undefined || prev === undefined) return if (!detectNewJob(prev, sessionList, sessionId)) return if (!store.getPrefs().autoOpenJobs) return - if (ctx.betterSidebar?.isTabEnabled('subagent') === false) return + if (ctx.get('betterSidebar')?.isTabEnabled('subagent') === false) return store.reduce(s => s.panelOpen ? s : togglePanel(s)) store.reduce(s => ({ ...s, activePane: firstLeaf(s.splits).id })) - ctx.betterSidebar?.openTab({ type: 'subagent', title: t('subagent') }) + ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent') }) }, [sessionList, sessionId, store, ctx]) /** @@ -533,7 +533,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { subagentJumpRef.current = undefined store.reduce(s => s.panelOpen ? s : togglePanel(s)) store.reduce(s => ({ ...s, activePane: firstLeaf(s.splits).id })) - ctx.betterSidebar?.openTab({ type: 'subagent', title: t('subagent') }) + ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent') }) }, [sessionId, store, ctx]) // The app shell's center column: the bottom panel spans ONLY that column @@ -705,11 +705,11 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { if (wasOpen === undefined || wasOpen || !state.bottomOpen) return if (state.bottomOpenedOnce) return if (store.getPrefs().bottomPanelAutoTerminal === false) return - if (ctx.betterSidebar?.isTabEnabled('terminal') === false) return + if (ctx.get('betterSidebar')?.isTabEnabled('terminal') === false) return // Land the tab in the bottom panel's first pane; the once-flag is set // atomically so later expansions never repeat the auto-open. store.reduce(s => ({ ...s, activePane: firstLeaf(s.bottomSplits).id, bottomOpenedOnce: true })) - ctx.betterSidebar?.openTab({ type: 'terminal' }) + ctx.get('betterSidebar')?.openTab({ type: 'terminal' }) }, [state, store, ctx, narrow]) // Panel drags: the right panel's width (left edge strip), the bottom @@ -980,7 +980,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // Route through the service: the tab-bar close is the canonical close // path (finds the pane itself, fires descriptor.onClose); the session // scope (with its cwd) rides to the callback. - ctx.betterSidebar?.closeTab(tabId, sessionId === undefined ? undefined : { sessionId, cwd }) + ctx.get('betterSidebar')?.closeTab(tabId, sessionId === undefined ? undefined : { sessionId, cwd }) if (tab?.type === 'terminal') { if (isAgentTabId(tabId)) { const uuid = agentUuidOf(tabId) @@ -994,7 +994,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // Route through the service: same reducer (finds the pane in EITHER // tree, sets the active pane) and fires descriptor.onActivate; the // session scope (with its cwd) rides to the callback. - ctx.betterSidebar?.activateTab(tabId, sessionId === undefined ? undefined : { sessionId, cwd }) + ctx.get('betterSidebar')?.activateTab(tabId, sessionId === undefined ? undefined : { sessionId, cwd }) }, focusPane: (paneId) => { store.reduce(s => ({ ...s, activePane: paneId })) }, moveTabToEdge: (payload: TabDragPayload, toPane: string, zone: DropZone) => { @@ -1051,9 +1051,9 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { } const onNewTab = (optionId: string): void => { - const service = ctx.betterSidebar + const service = ctx.get('betterSidebar') const descriptor = service?.getTab(optionId) - if (descriptor === undefined) return + if (service === undefined || descriptor === undefined) return const title = typeof descriptor.title === 'function' ? descriptor.title() : descriptor.title // The session scope rides along: lifecycle callbacks receive it (and // the open stays in the current session, as before). @@ -1068,7 +1068,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { */ /** The tab icon from the tab-type registry (shared by every workbench). */ const tabIconOf = (tab: SidebarTab): ReactNode => { - const descriptor = ctx.betterSidebar?.getTab(tab.type) + const descriptor = ctx.get('betterSidebar')?.getTab(tab.type) if (descriptor === undefined) return null return typeof descriptor.icon === 'function' ? descriptor.icon(14) : descriptor.icon } @@ -1079,7 +1079,7 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { * strip must never break because a plugin's badge computation failed. */ const tabBadgeOf = (tab: SidebarTab): ReactNode => { - const descriptor = ctx.betterSidebar?.getTab(tab.type) + const descriptor = ctx.get('betterSidebar')?.getTab(tab.type) if (descriptor?.badge === undefined) return null let value: string | number | null | undefined try { diff --git a/src/client/index.tsx b/src/client/index.tsx index 95b9530..6d973d6 100644 --- a/src/client/index.tsx +++ b/src/client/index.tsx @@ -363,7 +363,7 @@ export function apply(ctx: Context): void { let title: string | undefined try { title = new URL(url).hostname } catch { /* keep the default title */ } const type = urlTargetOf(new URL(url)) ?? 'browser' - ctx.betterSidebar?.openTab({ type, url, title }) + ctx.get('betterSidebar')?.openTab({ type, url, title }) }, selfOrigin: window.location.origin, }) diff --git a/src/client/intercept.tsx b/src/client/intercept.tsx index 7095c7a..eb4830f 100644 --- a/src/client/intercept.tsx +++ b/src/client/intercept.tsx @@ -22,7 +22,7 @@ export function openSidebarFile(ctx: Context, store: SidebarStore, sessionId: st const title = at === -1 ? absolute : absolute.slice(at + 1) // Route through the sidebar service so the editor descriptor's dedupeKey // (per-path) applies; the id is path-derived so multiple editors coexist. - ctx.betterSidebar?.openTab({ type: 'editor', title, path: absolute, id: `editor:${absolute}` }) + ctx.get('betterSidebar')?.openTab({ type: 'editor', title, path: absolute, id: `editor:${absolute}` }) } /** The intercepted produced-files row (visual twin of the deliverables chips). */ diff --git a/tests/bottom-auto-terminal.spec.tsx b/tests/bottom-auto-terminal.spec.tsx index dfba81d..5cb23c4 100644 --- a/tests/bottom-auto-terminal.spec.tsx +++ b/tests/bottom-auto-terminal.spec.tsx @@ -65,6 +65,7 @@ function mountSidebar(): MountedSidebar { locale: { subscribe: () => () => {}, getSnapshot: () => localeSnapshot }, sessions: { list: { subscribe: () => () => {}, getSnapshot: () => sessionsSnapshot } }, betterSidebar: service, + get: (name: string) => name === 'betterSidebar' ? service : undefined, } const root: Root = createRoot(container) act(() => { root.render(createElement(Sidebar, { ctx: ctx as never, store })) }) diff --git a/tests/editor-host.spec.tsx b/tests/editor-host.spec.tsx index 34bdf7f..b82257e 100644 --- a/tests/editor-host.spec.tsx +++ b/tests/editor-host.spec.tsx @@ -40,6 +40,7 @@ function setup(): { const sessionsSnapshot = { byId: { 'editor-home-session': { cwd: '/tmp' } }, current: 'editor-home-session' } const ctx = { betterSidebar: service, + get: (name: string) => name === 'betterSidebar' ? service : undefined, sessions: { list: { subscribe: () => () => {}, getSnapshot: () => sessionsSnapshot } }, } as unknown as Context return { store, ctx, homeTab } diff --git a/tests/openpath-intercept.spec.ts b/tests/openpath-intercept.spec.ts index 763003f..8ad7bdc 100644 --- a/tests/openpath-intercept.spec.ts +++ b/tests/openpath-intercept.spec.ts @@ -108,6 +108,9 @@ describe('open-path interception wiring', () => { }, workspaces: funnel, betterSidebar: { openTab: (seed: unknown) => { opened.push(seed as Record) } }, + get: (name: string) => name === 'betterSidebar' + ? { openTab: (seed: unknown) => { opened.push(seed as Record) } } + : undefined, } as unknown as Context const store = createSidebarStore() const original = ctx.workspaces.openPath diff --git a/tests/sidebar-crash.spec.tsx b/tests/sidebar-crash.spec.tsx index 095ef86..e99fce9 100644 --- a/tests/sidebar-crash.spec.tsx +++ b/tests/sidebar-crash.spec.tsx @@ -68,6 +68,7 @@ function mountSidebar(): MountedSidebar { locale: { subscribe: () => () => {}, getSnapshot: () => localeSnapshot }, sessions: { list: { subscribe: () => () => {}, getSnapshot: () => sessionsSnapshot } }, betterSidebar: service, + get: (name: string) => name === 'betterSidebar' ? service : undefined, } const root: Root = createRoot(container) act(() => { root.render(createElement(Sidebar, { ctx: ctx as never, store })) }) diff --git a/tests/turn-tail-intercept.spec.ts b/tests/turn-tail-intercept.spec.ts index 8504fa1..d709454 100644 --- a/tests/turn-tail-intercept.spec.ts +++ b/tests/turn-tail-intercept.spec.ts @@ -75,13 +75,17 @@ const producedOwner = (paths: string[]): unknown => ({ const emptyOwner = (): unknown => ({ nodes: [{ kind: 'assistant', seq: 1, turn: 1 }], seq: 1 }) /** The minimal client-context fake the registration (and its seats) touch. */ -const clientCtx = (slots: unknown): Context => ({ - slots, - sessions: { - list: { getSnapshot: () => ({ current: 's1', byId: { s1: { id: 's1', cwd: '/w', displayTitle: 's1' } } }) }, - }, - betterSidebar: { openTab: vi.fn() }, -} as unknown as Context) +const clientCtx = (slots: unknown): Context => { + const betterSidebar = { openTab: vi.fn() } + return { + slots, + sessions: { + list: { getSnapshot: () => ({ current: 's1', byId: { s1: { id: 's1', cwd: '/w', displayTitle: 's1' } } }) }, + }, + betterSidebar, + get: (name: string) => name === 'betterSidebar' ? betterSidebar : undefined, + } as unknown as Context +} describe('turn-tail interception registration (issue #15)', () => { it('registers through slots.inject and lands once the slot is already declared', () => {