diff --git a/src/client/sidebar/use-host-feeds.ts b/src/client/sidebar/use-host-feeds.ts index 5b8505c..6a27505 100644 --- a/src/client/sidebar/use-host-feeds.ts +++ b/src/client/sidebar/use-host-feeds.ts @@ -8,6 +8,7 @@ import { useEffect, useRef } from 'react' import type { Context, SidebarSessionList } from '../../context-types.ts' import { reconcileAgentTerminals, type SidebarStore } from '../state.ts' +import { isNarrowWidth } from '../breakpoints.ts' import { detectNewDirectSubagent } from '../subagent-detect.ts' import { detectNewJob } from '../subagent-jobs.ts' import { t } from '../locales.ts' @@ -26,6 +27,55 @@ const FAILURE_LIMIT = 3 */ const AUTO_OPEN_DEBOUNCE_MS = 500 +/** + * The native column's public face, as this module reaches it. Both actions + * act on the session whose surface is MOUNTED (the controller reads its + * binding), which is why the park below is gated on the target session being + * the on-screen one. + */ +interface NativeColumnFace { + isExpanded?: () => boolean + toggleExpanded?: () => void +} + +/** + * Activate the Tasks page (the `subagent` tab type) in DSH's native right + * Sidebar — the landing the two auto-open switches promise: the right column + * IS the sidebar the user means, while the plugin's own bottom workbench only + * serves its own flows (its `+` menu and the first-expansion terminal). Every + * other open in this plugin already lands there, so a `target: 'bottom'` here + * puts the Tasks page in the bottom bar instead. + * + * A background activation must not take over a narrow viewport: below 768px + * the host draws that column FULLSCREEN, so the tab is placed and the column + * is put back to collapsed — parked, waiting behind the expand control, + * instead of covering the chat. The host expands on every open + * (`openContent` plans `setExpanded(true)`) and offers no option to suppress + * it, hence the read-then-restore pair: both commits land in one React batch, + * so the column never renders the intermediate state. The viewport is read + * when the activation FIRES (the debounced subagent trigger included), so a + * resize while arming is honoured. + * + * @param ctx - the client context (`ctx.sidebarRight` + `ctx.betterSidebar`). + * @param sessionId - the session the feed reports the activity for. + * @param options.background - `true` for background activity (parks on narrow + * viewports); `false` for the explicit topology jump-back, which is a user + * gesture and always leaves the column as the host expanded it. + */ +function activateTasksPage(ctx: Context, sessionId: string, options: { background: boolean }): void { + const column = ctx.get('sidebarRight') as unknown as NativeColumnFace | undefined + const park = options.background + // The face acts on the MOUNTED session: parking is only meaningful (and + // only safe) when the activation targets the one on screen. + && ctx.sessions.list.getSnapshot().current === sessionId + && isNarrowWidth(window.innerWidth) + // Only a column the user had COLLAPSED is put back: an expanded one is in + // use, and closing it under the user would be worse than the takeover. + && column?.isExpanded?.() === false + ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent') }) + if (park) column?.toggleExpanded?.() +} + export function useHostFeeds(feeds: { ctx: Context store: SidebarStore @@ -166,11 +216,11 @@ export function useHostFeeds(feeds: { * Subagent auto-activation: the moment the current conversation spawns its * FIRST direct subagent (a 0 → N transition on the list feed), the "auto * open" pref is on, and the Tasks tab type is enabled in settings, activate - * the Tasks page. Single-instance semantics focus an existing pane tab in - * place or raise an existing free window; a new tab lands in the right pane - * and is never duplicated. On wide viewports the right panel also expands; - * on narrow viewports background activity never forces the full-screen - * drawer open over the chat. + * the Tasks page in DSH's native right Sidebar. Single-instance semantics + * focus an existing tab in place; a new tab lands in that column and is + * never duplicated. Landing it EXPANDS the column on wide viewports, while + * a narrow viewport (where the host draws that column fullscreen) parks the + * tab instead of taking the screen over — see {@link activateTasksPage}. * Switching to a session that already has subagents never triggers — its * baseline starts at the current count — so a deliberate layout is never * fought. @@ -196,7 +246,7 @@ export function useHostFeeds(feeds: { if (!detectNewDirectSubagent(baseline, ctx.sessions.list.getSnapshot(), sessionId)) return if (!store.getPrefs().autoOpenSubagent) return if (ctx.get('betterSidebar')?.isTabEnabled('subagent') === false) return - ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent'), target: 'bottom' }) + activateTasksPage(ctx, sessionId, { background: true }) }, AUTO_OPEN_DEBOUNCE_MS) autoOpenPendingRef.current = { baseline, timer } }, [sessionList, sessionId, store, ctx]) @@ -212,11 +262,12 @@ export function useHostFeeds(feeds: { * Job auto-activation: the moment a NEW background job appears for the * current conversation (a job id the previous snapshot lacked), the * auto-open pref is on, and the Tasks tab type is enabled, activate the Tasks - * page that contains the background-jobs section. The right panel expands - * only on wide viewports. Unlike the subagent trigger (0 → N only), ANY - * new job id triggers: the agent may start several jobs in one session, and - * each should surface. A fresh page load never triggers — its baseline starts - * at the current snapshot. + * page that contains the background-jobs section — in DSH's native right + * Sidebar, expanded on wide viewports and parked on narrow ones exactly like + * the subagent trigger ({@link activateTasksPage}). Unlike that trigger + * (0 → N only), ANY new job id triggers: the agent may start several jobs in + * one session, and each should surface. A fresh page load never triggers — + * its baseline starts at the current snapshot. */ const jobBaselineRef = useRef(undefined) useEffect(() => { @@ -226,7 +277,7 @@ export function useHostFeeds(feeds: { if (!detectNewJob(prev, sessionList, sessionId)) return if (!store.getPrefs().autoOpenJobs) return if (ctx.get('betterSidebar')?.isTabEnabled('subagent') === false) return - ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent'), target: 'bottom' }) + activateTasksPage(ctx, sessionId, { background: true }) }, [sessionList, sessionId, store, ctx]) /** @@ -235,17 +286,18 @@ export function useHostFeeds(feeds: { * session's OWN layout (a fresh child session defaults to the explorer). * The README contract says the Subagent page must stay open with the jumped * node highlighted — so once the current session becomes the recorded jump - * target, re-open the Subagent page on top of the child's layout (expanding - * the panel first if it is collapsed). Only this explicit node click arms - * the flag, so switching to a subagent session by any other means keeps - * that session's own layout untouched. + * target, re-open the Tasks page on top of the child's layout, in DSH's + * native right Sidebar. This one is an explicit user gesture, so it always + * takes the host's expansion (no narrow-viewport parking). Only this node + * click arms the flag, so switching to a subagent session by any other means + * keeps that session's own layout untouched. */ const subagentJumpRef = useRef(undefined) useEffect(() => { const pending = subagentJumpRef.current if (pending === undefined || sessionId !== pending) return subagentJumpRef.current = undefined - ctx.get('betterSidebar')?.openTab({ type: 'subagent', title: t('subagent'), target: 'bottom' }) + activateTasksPage(ctx, sessionId, { background: false }) }, [sessionId, store, ctx]) return { subagentJumpRef } diff --git a/tests/sidebar-auto-activation.spec.tsx b/tests/sidebar-auto-activation.spec.tsx index c7b7249..6f2674f 100644 --- a/tests/sidebar-auto-activation.spec.tsx +++ b/tests/sidebar-auto-activation.spec.tsx @@ -1,13 +1,21 @@ /** - * Real-Sidebar regressions for issue #162: background activity (a new - * subagent, a new background job) activates the Tasks page in the bottom - * workbench. The right column belongs to DSH's native Sidebar, so there is no - * full-screen drawer to guard against any more — the activation lands in the - * docked workbench on every viewport width. + * Auto-activation regressions for issue #162: background activity (a new + * subagent, a new background job) activates the Tasks page in DSH's NATIVE + * right Sidebar — the column the two auto-open switches and the README promise + * ("wide viewports also expand the sidebar, while narrow full-screen drawers + * are not forced open"). The plugin's own bottom workbench is NOT that + * surface: it serves only its own flows (its + menu, the first-expansion + * auto-terminal), so a background activation must leave it exactly as it was. + * + * The narrow half of that promise is a PARK: the host draws the native column + * fullscreen below 768px and expands it on every open, so the activation places + * the tab and puts the column back to collapsed (src/client/sidebar/ + * use-host-feeds.ts `activateTasksPage`). The topology jump-back is an explicit + * user gesture and always takes the host's expansion. */ // @vitest-environment jsdom import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { createElement } from 'react' +import { createElement, useEffect, useRef, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { act } from 'react-dom/test-utils' @@ -15,8 +23,13 @@ import { setupReactAct } from './test-utils.ts' setupReactAct() import { Sidebar } from '../src/client/Sidebar.tsx' -import { allLeaves, createSidebarStore, firstLeaf, type SidebarStore } from '../src/client/state.ts' -import { createBetterSidebarService, type BetterSidebarService } from '../src/client/service.ts' +import { allLeaves, createSidebarStore, type SidebarStore } from '../src/client/state.ts' +import { + createBetterSidebarService, + type BetterSidebarService, + type SidebarSurface, + type TabComponentProps, +} from '../src/client/service.ts' import type { Context, SidebarSessionList } from '../src/context-types.ts' class FakeWebSocket { @@ -45,10 +58,79 @@ function makeSessionFeed(initial: SidebarSessionList) { type SessionFeed = ReturnType +/** One recorded native-surface open. */ +interface NativeOpen { + sessionId: string + kind: string + params: unknown + revealIfOpened: boolean +} + +/** The native right Sidebar, replaced by a recording stand-in: the shell's + * opens must reach THIS surface, not the plugin's own layout. */ +interface NativeSurfaceSpy { + surface: SidebarSurface + opens: NativeOpen[] +} + +function makeNativeSurfaceSpy(): NativeSurfaceSpy { + const opens: NativeOpen[] = [] + return { + opens, + surface: { + openTab: input => { opens.push({ ...input }) }, + openResource: () => {}, + fileAddress: (sessionId, cwd, path) => `addr://${sessionId}${cwd ?? ''}${path}`, + close: () => undefined, + update: () => false, + activate: () => false, + has: () => false, + }, + } +} + +/** `ctx.sidebarRight`, replaced by a stand-in column: `isExpanded` reports the + * current state, `toggleExpanded` flips it and counts the calls. */ +interface NativeColumnSpy { + face: { isExpanded: () => boolean; toggleExpanded: () => void } + toggles: number +} + +function makeNativeColumnSpy(expanded: boolean): NativeColumnSpy { + const spy = { + toggles: 0, + expanded, + face: {} as { isExpanded: () => boolean; toggleExpanded: () => void }, + } + spy.face = { + isExpanded: () => spy.expanded, + toggleExpanded: () => { + spy.toggles += 1 + spy.expanded = !spy.expanded + }, + } + return spy +} + +/** Stands in for the Tasks page: its node click is the jump gesture that arms + * the shell's jump-back (fired once, like a real click). */ +function JumpHarness({ onSubagentJump }: TabComponentProps): ReactNode { + const fired = useRef(false) + useEffect(() => { + if (fired.current) return + fired.current = true + onSubagentJump?.('child') + }, [onSubagentJump]) + return null +} + interface MountedSidebar { store: SidebarStore service: BetterSidebarService feed: SessionFeed + surface: NativeSurfaceSpy + column: NativeColumnSpy + sessionId: string unmount: () => void } @@ -59,7 +141,11 @@ function setViewport(width: number): void { Object.defineProperty(window, 'innerWidth', { configurable: true, value: width }) } -function mountSidebar(width: number, bottomOpen = false): MountedSidebar { +function mountSidebar( + width: number, + bottomOpen = false, + options: { columnExpanded?: boolean } = {}, +): MountedSidebar { setViewport(width) vi.stubGlobal('WebSocket', FakeWebSocket) const sessionId = `auto-activation-${++sessionSeq}` @@ -76,13 +162,18 @@ function mountSidebar(width: number, bottomOpen = false): MountedSidebar { store.setSession(sessionId) store.reduce(state => ({ ...state, bottomOpen })) const service = createBetterSidebarService(store) - service.registerTab({ id: 'subagent', title: 'Subagent', component: () => null }) + const surface = makeNativeSurfaceSpy() + service.setSurface(surface.surface) + service.registerTab({ id: 'subagent', title: 'Subagent', component: JumpHarness }) + const column = makeNativeColumnSpy(options.columnExpanded ?? false) const localeSnapshot = { active: 'en' } const ctx = { locale: { subscribe: () => () => {}, getSnapshot: () => localeSnapshot }, sessions: { list: feed }, betterSidebar: service, - get: (name: string) => name === 'betterSidebar' ? service : undefined, + get: (name: string) => name === 'betterSidebar' + ? service + : name === 'sidebarRight' ? column.face : undefined, } as unknown as Context const container = document.createElement('div') document.body.append(container) @@ -92,6 +183,9 @@ function mountSidebar(width: number, bottomOpen = false): MountedSidebar { store, service, feed, + surface, + column, + sessionId, unmount: () => { act(() => { root.unmount() }) container.remove() @@ -101,6 +195,18 @@ function mountSidebar(width: number, bottomOpen = false): MountedSidebar { return result } +type ActivitySource = 'subagent' | 'job' + +/** Deliver one piece of background activity through the host's own feeds. */ +function publishActivity(sidebar: MountedSidebar, source: ActivitySource): void { + if (source === 'job') { + publishJob(sidebar) + return + } + publishSubagent(sidebar) + flushSubagentDebounce() +} + function publishSubagent(sidebar: MountedSidebar): void { const before = sidebar.feed.getSnapshot() const sessionId = before.current! @@ -145,14 +251,44 @@ function publishJob(sidebar: MountedSidebar): void { }) } -function expectSubagentFocused(sidebar: MountedSidebar): void { +/** Switch the conversation to the child session the Tasks page jumped to. */ +function switchToChild(sidebar: MountedSidebar): void { + const before = sidebar.feed.getSnapshot() + const parent = before.current! + act(() => { + sidebar.feed.set({ + ...before, + current: 'child', + byId: { + ...before.byId, + child: { + id: 'child', + displayTitle: 'Worker', + origin: 'subagent', + parentId: parent, + running: true, + }, + }, + }) + }) +} + +/** The Tasks page landed in the native right Sidebar (the default open). */ +function expectNativeTasksOpen(sidebar: MountedSidebar, sessionId: string): void { + expect(sidebar.surface.opens).toEqual([{ + sessionId, + kind: 'subagent', + params: expect.objectContaining({ title: expect.any(String) }), + revealIfOpened: true, + }]) +} + +/** The plugin's own bottom workbench kept its own state and gained no tab. */ +function expectWorkbenchUntouched(sidebar: MountedSidebar, open = false): void { const state = sidebar.store.getSnapshot().state! - const pane = firstLeaf(state.bottomSplits) - expect(state.bottomOpen).toBe(true) - expect(state.activePane).toBe(pane.id) + expect(state.bottomOpen).toBe(open) expect(allLeaves(state.bottomSplits).flatMap(leaf => leaf.tabs) - .filter(tab => tab.type === 'subagent')).toHaveLength(1) - expect(pane.tabs.find(tab => tab.id === pane.active)?.type).toBe('subagent') + .filter(tab => tab.type === 'subagent')).toHaveLength(0) } beforeEach(() => { @@ -176,33 +312,57 @@ describe('Sidebar background-activity auto-activation (#162)', () => { { source: 'job', width: 390 }, { source: 'subagent', width: 1024 }, { source: 'job', width: 1024 }, - ] as const)('$source activation at $width px focuses the Tasks page in the workbench', ({ source, width }) => { + ] as const)('$source activation at $width px activates the Tasks page in the native Sidebar', ({ source, width }) => { const sidebar = mountSidebar(width) - if (source === 'subagent') { - publishSubagent(sidebar) - flushSubagentDebounce() - } else { - publishJob(sidebar) - } - expectSubagentFocused(sidebar) + publishActivity(sidebar, source) + expectNativeTasksOpen(sidebar, sidebar.sessionId) + expectWorkbenchUntouched(sidebar) + // Parking is the narrow-viewport half of the same promise. + expect(sidebar.column.toggles).toBe(width < 768 ? 1 : 0) }) - it.each(['subagent', 'job'] as const)('%s activation keeps an already-open workbench open', (source) => { - const sidebar = mountSidebar(390, true) - if (source === 'subagent') { - publishSubagent(sidebar) - flushSubagentDebounce() - } else { - publishJob(sidebar) - } - expectSubagentFocused(sidebar) + it.each(['subagent', 'job'] as const)('%s activation leaves a narrow fullscreen column to the park', (source) => { + const sidebar = mountSidebar(390) + publishActivity(sidebar, source) + expect(sidebar.column.toggles).toBe(1) }) - it('a resize between arming and firing the debounce does not change the landing', () => { + it('a narrow column the user already expanded is not closed under them', () => { + const sidebar = mountSidebar(390, false, { columnExpanded: true }) + publishActivity(sidebar, 'subagent') + expectNativeTasksOpen(sidebar, sidebar.sessionId) + expect(sidebar.column.toggles).toBe(0) + }) + + it('reads the viewport when the debounced activation fires, not when it arms', () => { const sidebar = mountSidebar(1024) publishSubagent(sidebar) setViewport(390) flushSubagentDebounce() - expectSubagentFocused(sidebar) + expectNativeTasksOpen(sidebar, sidebar.sessionId) + expect(sidebar.column.toggles).toBe(1) + }) + + it('leaves an already-open bottom workbench open and untouched', () => { + const sidebar = mountSidebar(1024, true) + publishActivity(sidebar, 'subagent') + expectNativeTasksOpen(sidebar, sidebar.sessionId) + expectWorkbenchUntouched(sidebar, true) + }) + + it('the topology jump-back opens the Tasks page for the child session and never parks it', () => { + const sidebar = mountSidebar(390) + // The Tasks page the user opened in the workbench (its own + menu) is what + // arms the jump: its node click records the child session. + act(() => { + sidebar.service.openTab( + { type: 'subagent', title: 'Tasks', target: 'bottom' }, + { sessionId: sidebar.sessionId }, + ) + }) + expect(sidebar.store.getSnapshot().state!.bottomOpen).toBe(true) + switchToChild(sidebar) + expectNativeTasksOpen(sidebar, 'child') + expect(sidebar.column.toggles).toBe(0) }) })