diff --git a/apps/web/e2e/tests/chat/unread-divider.spec.ts b/apps/web/e2e/tests/chat/unread-divider.spec.ts index 04cd5a7097..c72d07109c 100644 --- a/apps/web/e2e/tests/chat/unread-divider.spec.ts +++ b/apps/web/e2e/tests/chat/unread-divider.spec.ts @@ -185,6 +185,7 @@ test.describe("Unread divider", () => { expect(await isScrolledIntoView(scrollContainer, newestRow)).toBe(false); }); + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.15 test("completed task switch keeps each read cursor and returns to the bottom", async ({ testPage, apiClient, @@ -214,6 +215,7 @@ test.describe("Unread divider", () => { element.scrollTop = element.scrollHeight; element.dispatchEvent(new Event("scroll")); }); + await testPage.getByTestId("dockview-tab-changes").click(); const taskBMarkRead = testPage.waitForResponse( (response) => @@ -237,6 +239,19 @@ test.describe("Unread divider", () => { const activeChat = session.activeChat(); const scrollContainer = activeChat.locator(".chat-message-list"); + await expect + .poll(() => + testPage.evaluate( + () => + ( + window as unknown as { + __dockviewApi__?: { activePanel?: { id: string } }; + } + ).__dockviewApi__?.activePanel?.id ?? null, + ), + ) + .toBe("changes"); + await expect(scrollContainer).toBeVisible(); await expect(activeChat.getByTestId("unread-divider")).toHaveCount(0); await expect .poll(() => diff --git a/apps/web/hooks/use-panel-active.test.ts b/apps/web/hooks/use-panel-active.test.ts index fa0fd2eb82..9da9b6ede4 100644 --- a/apps/web/hooks/use-panel-active.test.ts +++ b/apps/web/hooks/use-panel-active.test.ts @@ -10,34 +10,53 @@ afterEach(() => { type FakePanelApi = { isActive: boolean; + isVisible: boolean; onDidActiveChange: (cb: (event: { isActive: boolean }) => void) => { dispose: () => void }; + onDidVisibilityChange: (cb: (event: { isVisible: boolean }) => void) => { + dispose: () => void; + }; }; type FakeApiHandle = { fakeApi: FakePanelApi; fireActiveChange: (isActive: boolean) => void; + fireVisibilityChange: (isVisible: boolean) => void; }; -function makeFakeApi(initialIsActive: boolean): FakeApiHandle { - let listener: ((event: { isActive: boolean }) => void) | null = null; +function makeFakeApi(initialIsActive: boolean, initialIsVisible = initialIsActive): FakeApiHandle { + let activeListener: ((event: { isActive: boolean }) => void) | null = null; + let visibilityListener: ((event: { isVisible: boolean }) => void) | null = null; const fakeApi: FakePanelApi = { isActive: initialIsActive, + isVisible: initialIsVisible, onDidActiveChange: (cb) => { - listener = cb; - return { dispose: () => (listener = null) }; + activeListener = cb; + return { dispose: () => (activeListener = null) }; + }, + onDidVisibilityChange: (cb) => { + visibilityListener = cb; + return { dispose: () => (visibilityListener = null) }; }, }; return { fakeApi, fireActiveChange: (isActive: boolean) => { fakeApi.isActive = isActive; - listener?.({ isActive }); + activeListener?.({ isActive }); + }, + fireVisibilityChange: (isVisible: boolean) => { + fakeApi.isVisible = isVisible; + visibilityListener?.({ isVisible }); }, }; } -function acquirePanel(panelId: string, initialIsActive: boolean): FakeApiHandle { - const handle = makeFakeApi(initialIsActive); +function acquirePanel( + panelId: string, + initialIsActive: boolean, + initialIsVisible = initialIsActive, +): FakeApiHandle { + const handle = makeFakeApi(initialIsActive, initialIsVisible); // Test double: only the subset of DockviewPanelApi this hook actually // reads/calls is implemented. Unchecked cast is intentional here — the // full interface has 30+ unrelated members no test in this file exercises. @@ -57,7 +76,7 @@ describe("usePanelActive", () => { expect(result.current).toBe(false); }); - it("reflects the panel's initial isActive state once registered", () => { + it("reflects the panel's initial isVisible state once registered", () => { acquirePanel("panel-active", true); const { result } = renderHook(() => usePanelActive("panel-active")); expect(result.current).toBe(true); @@ -67,15 +86,31 @@ describe("usePanelActive", () => { expect(result2.current).toBe(false); }); - it("updates when the panel's active tab changes via onDidActiveChange", () => { + it("treats a selected panel as visible when another Dockview group owns focus", () => { + const handle = acquirePanel("panel-visible-unfocused", false, true); + const { result } = renderHook(() => usePanelActive("panel-visible-unfocused")); + + expect(result.current).toBe(true); + + // The hook subscribes to onDidVisibilityChange, not onDidActiveChange. + // Firing active-change events is a no-op; the result must not move. + act(() => handle.fireActiveChange(true)); + act(() => handle.fireActiveChange(false)); + expect(result.current).toBe(true); + + act(() => handle.fireVisibilityChange(false)); + expect(result.current).toBe(false); + }); + + it("updates when the panel's visible tab changes via onDidVisibilityChange", () => { const handle = acquirePanel("panel-toggle", false); const { result } = renderHook(() => usePanelActive("panel-toggle")); expect(result.current).toBe(false); - act(() => handle.fireActiveChange(true)); + act(() => handle.fireVisibilityChange(true)); expect(result.current).toBe(true); - act(() => handle.fireActiveChange(false)); + act(() => handle.fireVisibilityChange(false)); expect(result.current).toBe(false); }); @@ -103,13 +138,13 @@ describe("usePanelActive", () => { // without the old one ever being released. const fresh = acquirePanel(PANEL_ID, false); - act(() => fresh.fireActiveChange(true)); + act(() => fresh.fireVisibilityChange(true)); expect(result.current).toBe(true); // The stale api's listener must have been torn down — firing it no // longer moves the hook (it would incorrectly flip back to false // without the resubscribe fix). - act(() => stale.fireActiveChange(false)); + act(() => stale.fireVisibilityChange(false)); expect(result.current).toBe(true); }); }); diff --git a/apps/web/hooks/use-panel-active.ts b/apps/web/hooks/use-panel-active.ts index 514e155ca5..f70faf698a 100644 --- a/apps/web/hooks/use-panel-active.ts +++ b/apps/web/hooks/use-panel-active.ts @@ -5,7 +5,7 @@ import type { DockviewPanelApi } from "dockview-react"; import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; /** - * Tracks whether the dockview panel identified by panelId is the active tab + * Tracks whether the dockview panel identified by panelId is the visible tab * in its dockview group. Portal-hosted panel content (see * `lib/layout/panel-portal-manager.ts`) stays mounted while its tab is * inactive — e.g. the Chat tab keeps running behind an active Files/Changes @@ -19,8 +19,10 @@ import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; * renders, so the entry is virtually always present by the time this runs. * Subscribes to both the manager's own add/remove notifications (a panel * acquired after this hook's first mount, or released while it stays - * mounted) and the panel's own `onDidActiveChange` (tab switches within an - * existing group). + * mounted) and the panel's own `onDidVisibilityChange` (tab switches within + * an existing group). Dockview's `isActive` also requires the panel's group + * to own global focus, so it is not a valid proxy for whether the panel is + * rendered on screen beside another focused group. * * Only for dockview-hosted panels — pass the panel's real id. Defaults to * `false` until a portal entry/api is actually registered for it, rather @@ -34,7 +36,7 @@ import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; export function usePanelActive(panelId: string): boolean { const subscribe = useCallback( (onStoreChange: () => void) => { - // Tracks which api our onDidActiveChange listener is currently bound + // Tracks which api our onDidVisibilityChange listener is currently bound // to, so a manager notification (panel acquired/released/remounted // anywhere) can detect when *this* panelId's api was replaced — // Dockview does this during a layout restore/switch — and rebind @@ -47,7 +49,7 @@ export function usePanelActive(panelId: string): boolean { if (api === boundApi) return; disposable?.dispose(); boundApi = api; - disposable = api?.onDidActiveChange(onStoreChange) ?? null; + disposable = api?.onDidVisibilityChange(onStoreChange) ?? null; }; rebindIfApiChanged(); @@ -63,7 +65,7 @@ export function usePanelActive(panelId: string): boolean { [panelId], ); const getSnapshot = useCallback( - () => panelPortalManager.get(panelId)?.api?.isActive ?? false, + () => panelPortalManager.get(panelId)?.api?.isVisible ?? false, [panelId], ); return useSyncExternalStore(subscribe, getSnapshot, () => false); diff --git a/docs/plans/task-switch-read-cursor-continuity/plan.md b/docs/plans/task-switch-read-cursor-continuity/plan.md index 1f6a36e686..9742c8a0a3 100644 --- a/docs/plans/task-switch-read-cursor-continuity/plan.md +++ b/docs/plans/task-switch-read-cursor-continuity/plan.md @@ -41,6 +41,14 @@ A focused throwaway hook test reproduced the race: session A's delayed response produced no `updateSessionReadCursor("session-1", "m2")` call after session B dispatched. The temporary test was removed after recording the red evidence. +The follow-up trace exposed a separate visibility defect. Dockview restored the +incoming Chat tab as selected in the center group while the side-by-side +Changes group retained global focus. `usePanelActive` read Dockview's +`isActive`, which combines group-local tab visibility with global group focus, +so the visibly rendered transcript reported `isVisible=false` and skipped +initial placement. Completed agents produced no later message or work-state +write to mask the skipped placement. + ## Scope ### In scope @@ -119,6 +127,7 @@ changes. ## Work orders - [x] [Task 01: Preserve read cursor across task switches](task-01-preserve-read-cursor-across-task-switches.md) +- [x] [Task 02: Recognize visible Dockview chat panels](task-02-recognize-visible-dockview-chat-panels.md) ## Verification results @@ -132,6 +141,15 @@ changes. - Mobile Chrome completed-task switch E2E passed (1 test). - Specification linter tests passed (30 tests); all specification files passed. - `git diff --check` passed. +- Follow-up TDD regression: the visibility hook returned false when Dockview + reported `isActive=false` and `isVisible=true`; it passed after switching to + the group-local visibility contract. +- Follow-up focused Vitest passed: 1 file, 6 tests. +- Follow-up TypeScript typecheck and scoped ESLint passed. +- Follow-up Chromium task-switch E2E passed with Changes retaining global + Dockview focus while the visible Chat transcript returned to the bottom. +- Follow-up Mobile Chrome completed-task switch E2E passed. +- Follow-up specification linter tests and full specification lint passed. ## Risks diff --git a/docs/plans/task-switch-read-cursor-continuity/task-02-recognize-visible-dockview-chat-panels.md b/docs/plans/task-switch-read-cursor-continuity/task-02-recognize-visible-dockview-chat-panels.md new file mode 100644 index 0000000000..d15f1aa22c --- /dev/null +++ b/docs/plans/task-switch-read-cursor-continuity/task-02-recognize-visible-dockview-chat-panels.md @@ -0,0 +1,97 @@ +--- +id: "02-recognize-visible-dockview-chat-panels" +title: "Recognize visible Dockview chat panels" +status: done +wave: 2 +depends_on: + - "01-preserve-read-cursor-across-task-switches" +plan: "plan.md" +requirements: + - REQ-UI-TRANSCRIPT-AUTO-SCROLL-001 +acceptance_criteria: + - AC-UI-TRANSCRIPT-AUTO-SCROLL-001.11 + - AC-UI-TRANSCRIPT-AUTO-SCROLL-001.14 + - AC-UI-TRANSCRIPT-AUTO-SCROLL-001.15 +system_design: + - ../../specs/ui/system-design/transcript-auto-scroll.md +--- + +# Task 02: Recognize Visible Dockview Chat Panels + +## Summary + +Treat the selected Chat tab as visible independently of which side-by-side +Dockview group owns global focus. Prove that a completed task restores its +transcript placement without relying on a later agent update. + +## In scope + +- Subscribe to Dockview panel visibility rather than global active state. +- Cover the focused-sibling-group contract in the visibility hook. +- Extend the desktop completed-task switch regression with Changes-group focus. + +## Out of scope + +- Dockview layout persistence or active-group restoration. +- Transcript auto-scroll preference semantics or stored offsets. +- Mobile composition, which does not mount Dockview. + +## Acceptance + +- A selected Chat tab reports visible when another Dockview group owns focus. +- Hiding Chat behind another tab in its own group reports not visible. +- Returning to an idle completed task places the transcript at the bottom while + Changes remains the globally active panel. + +## Verification + +```bash +(cd apps && pnpm --filter @kandev/web test -- --run hooks/use-panel-active.test.ts) +(cd apps/web && pnpm exec eslint hooks/use-panel-active.ts hooks/use-panel-active.test.ts e2e/tests/chat/unread-divider.spec.ts) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm e2e:run --host --project chromium tests/chat/unread-divider.spec.ts -- --grep "completed task switch" --retries=0) +(cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/chat/mobile-unread-divider.spec.ts -- --grep "completed task switch" --retries=0) +python3 scripts/lint-spec-files.test.py +python3 scripts/lint-spec-files.py --all +git diff --check +``` + +## Files likely touched + +- `apps/web/hooks/use-panel-active.ts` +- `apps/web/hooks/use-panel-active.test.ts` +- `apps/web/e2e/tests/chat/unread-divider.spec.ts` +- `docs/specs/ui/requirements/transcript-auto-scroll.md` +- `docs/specs/ui/system-design/transcript-auto-scroll.md` + +## Dependencies + +Task 01 established the environment-switch placement lifecycle and its +completed-task regression. + +## Risks + +- Using global Dockview focus recreates the defect when another visible group + remains active after layout restoration. +- Treating every mounted portal as visible could mark hidden session tabs read. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-UI-TRANSCRIPT-AUTO-SCROLL-001`, especially acceptance criteria `.11`, + `.14`, and `.15`. +- UI transcript auto-scroll system design. +- Dockview panel visibility and active-state contracts. + +## Results + +- Replaced Dockview's global active-state subscription with the group-local + visibility property and event. +- Added unit coverage for a visible Chat panel whose sibling group owns focus. +- Extended the desktop completed-task switch regression to retain Changes as + the active panel and prove the visible transcript returns to the bottom. +- Focused Vitest, scoped ESLint, TypeScript typecheck, desktop Chromium E2E, + Mobile Chrome parity E2E, specification lint, and diff checks passed. diff --git a/docs/specs/ui/requirements/transcript-auto-scroll.md b/docs/specs/ui/requirements/transcript-auto-scroll.md index 7dbbb24734..5a63df47a8 100644 --- a/docs/specs/ui/requirements/transcript-auto-scroll.md +++ b/docs/specs/ui/requirements/transcript-auto-scroll.md @@ -2,7 +2,7 @@ status: active system: ui created: 2026-07-30 -updated: 2026-09-04 +updated: 2026-09-07 owners: - cfl --- @@ -72,6 +72,11 @@ hand-off it is designed to protect. newest cached message and a disabled transcript shows its saved reader position as soon as the panel is measurable, without first exposing the browser's default top position. +- **AC-UI-TRANSCRIPT-AUTO-SCROLL-001.15:** **GIVEN** a desktop transcript is the + selected tab in its Dockview group while another visible group owns global + focus, **WHEN** a task switch restores that layout, **THEN** the transcript + is treated as visible and completes its enabled or disabled initial + placement. ## Migrated source detail @@ -127,6 +132,9 @@ the asynchronous hand-off it is designed to protect. **WHEN** the user switches between them and returns, **THEN** the incoming enabled transcript shows the newest cached message immediately and remains at the newest message after its history refresh. +- **GIVEN** the Changes panel owns Dockview focus while Chat remains the + selected center tab, **WHEN** the user returns to that task, **THEN** Chat + completes initial transcript placement without requiring an agent update. - **GIVEN** the incoming transcript has auto-scroll disabled, **WHEN** that environment-switch placement runs, **THEN** it restores the incoming session's saved position rather than the outgoing transcript's position. diff --git a/docs/specs/ui/system-design/transcript-auto-scroll.md b/docs/specs/ui/system-design/transcript-auto-scroll.md index 75755ca71e..31661bd2fa 100644 --- a/docs/specs/ui/system-design/transcript-auto-scroll.md +++ b/docs/specs/ui/system-design/transcript-auto-scroll.md @@ -4,7 +4,7 @@ system: ui requirements: - REQ-UI-TRANSCRIPT-AUTO-SCROLL-001 created: 2026-08-27 -updated: 2026-09-04 +updated: 2026-09-07 owners: - kandev --- @@ -50,8 +50,15 @@ if that path reads it. The scroll setter records the WebKit-safe offset. Desktop Dockview renders panel content through persistent portals. An inactive session transcript can therefore be mounted and receive messages while its scroll container has zero or detached geometry. `usePanelActive` supplies the -authoritative active-tab signal to `TaskChatPanel`, which passes it to the -native transcript scroll coordinator as `isVisible`. +authoritative group-local visibility signal to `TaskChatPanel`, which passes it +to the native transcript scroll coordinator as `isVisible`. + +Dockview's panel `isVisible` property means the panel is the selected tab in +its own group. Its `isActive` property additionally requires that group to own +global Dockview focus. The transcript uses `isVisible` and +`onDidVisibilityChange`; clicking a side-by-side Files or Changes group cannot +make a still-rendered Chat panel ineligible for initial placement or read +tracking. Geometry-based initialization and divider placement do not consume their one-time completion state while `isVisible` is false. When a transcript first @@ -194,10 +201,11 @@ pinning, and disabled position restoration. ## Failure modes - If the container is not mounted, the helper performs no work. -- If a transcript is mounted but inactive, one-time placement remains pending - until the panel becomes active and measurable. -- If a panel becomes inactive again before the post-restore frames complete, - the scheduled placement is cancelled. +- If a transcript is mounted but hidden behind another tab in its group, + one-time placement remains pending until the panel becomes visible and + measurable. +- If a panel becomes hidden again before the post-restore frames complete, the + scheduled placement is cancelled. - Chromium, Gecko, and WebKit clamp the signed 32-bit maximum to the native maximum scroll position. - An offset outside WebKit's safe range can resolve to zero and move the