diff --git a/.qwen/e2e-tests/2026-08-04-secondary-artifact-workspace-ownership.md b/.qwen/e2e-tests/2026-08-04-secondary-artifact-workspace-ownership.md new file mode 100644 index 00000000000..fc33e5eb174 --- /dev/null +++ b/.qwen/e2e-tests/2026-08-04-secondary-artifact-workspace-ownership.md @@ -0,0 +1,117 @@ +# Secondary artifact workspace ownership + +Issue: https://github.com/QwenLM/qwen-code/issues/8494 + +## Baseline + +- Global CLI: `qwen 0.21.3` +- `qwen serve --help` confirms repeated `--workspace` registration and Web + Shell serving are available. +- Current component behavior is captured failure-first: an artifact tab with + no owner uses root workspace actions, and scheduled-task detail omits the + secondary `workspaceId`. + +## Local setup + +1. Create isolated primary and secondary temporary workspaces. +2. Put the same relative sentinel filename in both workspaces with different + contents. +3. Create distinct durable scheduled-task fixtures for both runtimes. +4. Start the built `dist/cli.js serve` on loopback with both `--workspace` + flags and the production Web Shell bundle. + +## Scenarios + +### Secondary file action + +Open a secondary-session artifact and download/preview its sentinel. + +Expected: + +- The request targets `/workspaces//file...`. +- The returned bytes are the secondary sentinel, never the primary sentinel. + +### Secondary scheduled task + +Open the secondary durable task, toggle it, edit it, and delete the test copy. + +Expected: + +- Every request targets `/workspaces//scheduled-tasks...`. +- The primary task and primary task file are unchanged. + +### Ownership loss + +Keep an artifact tab open, then remove or mark the secondary workspace +unavailable in the capability fixture before a delayed read resolves. + +Expected: + +- The panel displays the localized workspace-unavailable state. +- The delayed response is ignored. +- No request is retried against the primary route. + +## Evidence + +- Focused test output for resolver, turn outputs, artifact panel, nested + subagent, and app tab propagation. +- Production build/typecheck/lint output. +- Captured two-workspace HTTP route log and sentinel hashes. +- Browser screenshot of the secondary artifact panel and the fail-closed + ownership-loss state when the local browser harness can represent it. + +## Results (2026-08-04) + +Result: PASS + +### Automated verification + +- Failure-first run: 3 expected failures and 29 existing passes. The primary + sentinel leaked into an ownerless tab, the secondary qualified client was + unused, and scheduled-task list omitted `workspaceId`. +- Focused ownership suite: 413/413 tests passed across the six affected test + files. +- Full Web Shell suite: 167 files and 2,777 tests passed. +- Web Shell lint and TypeScript typecheck passed. +- Package Web Shell build and repository root production build passed. +- The changed files pass Prettier. The package-wide format check still reports + five unchanged baseline files: + `BranchPickerPopover.module.css`, `GitModePopover.module.css`, + `GitDialog.module.css`, `PlanExecutionView.module.css`, and `index.html`. + +### Production two-workspace verification + +The built CLI served the copied production bundle with two trusted workspaces. +`GET /capabilities` advertised distinct primary and secondary runtime IDs, and +the served JavaScript contained the new stale-owner guard. + +The same relative file was read through both routes: + +- `GET /file?path=artifact-owner.txt` returned + `PRIMARY_WORKSPACE_SENTINEL_8494`, SHA-256 + `818d5f4f1fb9c7e3f9bdfd9a3ad39361c93a23ca3f3d0ba5e4a7c889b33b9127`. +- `GET /workspaces//file?path=artifact-owner.txt` returned + `SECONDARY_WORKSPACE_SENTINEL_8494`, SHA-256 + `0e9fac7909b16ff8b17014ea103ed5018c8b4e9ad21d2f95e128fef3a3544a12`. +- The secondary bytes route returned only the secondary sentinel bytes. + +Durable task CRUD was exercised against isolated primary and secondary task +fixtures: + +- Secondary list, update, and delete requests all used + `/workspaces//scheduled-tasks...`. +- The secondary update changed only the secondary name/enabled state. +- The primary task remained present, enabled, and unchanged after that update. +- Deleting the secondary task emptied only the secondary list; the primary + task was still present. Both test tasks were then removed. + +The daemon request log independently recorded the qualified file, bytes, +scheduled-task POST/GET/PATCH/DELETE routes and their successful statuses. + +### Visual evidence + +The in-app browser runtime reported no available browser instance, so a UI +screenshot could not be captured in this environment. No synthetic screenshot +was substituted. The production server returned the current Web Shell bundle +with HTTP 200, and the DOM behavior is covered by the focused and full suites +above. diff --git a/docs/design/web-shell-artifact-workspace-ownership.md b/docs/design/web-shell-artifact-workspace-ownership.md new file mode 100644 index 00000000000..ea6f77ff5ac --- /dev/null +++ b/docs/design/web-shell-artifact-workspace-ownership.md @@ -0,0 +1,101 @@ +# Web Shell artifact workspace ownership + +Status: implemented and verified + +Issue: https://github.com/QwenLM/qwen-code/issues/8494 + +## Problem + +The Web Shell has one app-level `DaemonWorkspaceProvider`. Its default +workspace actions target the primary workspace. Session providers can attach to +secondary workspaces, but artifact surfaces currently retain or fall back to +the app-level actions. A file preview, download, review, nested subagent +artifact, or scheduled-task mutation can therefore reach the primary runtime +even though the producing session belongs to a secondary workspace. + +The unsafe fallback also survives ownership changes: action objects are stored +in right-panel tabs, so removing or distrusting a workspace does not invalidate +an already-open tab. + +## Ownership contract + +The producing session owns every turn output. Its `workspaceCwd`, resolved by +the session connection, is the source of the owner claim. The current daemon +capabilities are the authority that accepts or rejects that claim. + +A target is usable only when one of these cases holds: + +1. Exactly one advertised workspace has the same cwd and is trusted. +2. For a legacy single-workspace daemon with no workspace list, the cwd exactly + matches `capabilities.workspaceCwd`. + +Unknown, duplicate, untrusted, removed, or identity-mismatched targets fail +closed. They never fall back to the primary workspace. + +## Design + +### Resolve at use time + +Turn-output requests and right-panel tabs carry immutable owner identity +(`workspaceCwd` and the advertised `workspaceId`) instead of long-lived action +objects. `ArtifactPanel` resolves that identity against current capabilities on +every render. This makes workspace removal, trust loss, and runtime replacement +invalidate open tabs immediately. + +The resolver returns a small artifact action surface only: + +- `readWorkspaceFile` +- `readFileBytes` +- `stat` +- scheduled-task list/update/delete operations + +Primary targets reuse the provider's primary actions. Trusted secondary +targets use `client.workspaceByCwd(cwd)` for file operations. Scheduled-task +operations still use the Web UI REST actions, but always receive the resolved +workspace id explicitly. + +### Propagation + +- `TurnOutputs` derives the owner from its session `workspaceCwd`, uses the + scoped file actions for direct downloads, and stamps owner identity onto + review, artifact, and scheduled-task open requests. +- `ChatPane` and `SubagentDetail` preserve the request identity and add only the + producing session id. They no longer stamp root workspace actions onto the + request or artifact snapshot. +- `App` stores owner identity on each right-panel tab. Pane artifact snapshots + contain artifacts only; they no longer retain workspace clients. +- `ArtifactPanel` renders the existing workspace-unavailable state when the + owner cannot be resolved or no longer matches. It never substitutes + `useWorkspaceActions()` for a missing owner. +- Durable scheduled-task snapshots carry `workspaceId`, and every list, + update, toggle, and delete request passes that id. + +### Async invalidation + +Scoped action wrappers check that their captured owner is still current before +starting an operation and again after each response. Effects and scheduled-task +mutations also ignore results after unmount or owner replacement. A response +started for one runtime cannot populate a tab after that runtime is removed or +replaced. + +## Non-goals + +- No daemon route or workspace-registry semantics change. +- No primary fallback for compatibility on multi-workspace daemons. +- No redesign of artifact storage, session attachment, or scheduled-task data. +- No attempt to make arbitrary app-level workspace actions session-scoped; + only the artifact surfaces in issue #8494 are changed. + +## Verification + +1. Failure-first component tests pin the current primary fallback and missing + scheduled-task workspace id. +2. Resolver tests cover primary, trusted secondary, unknown, duplicate, + untrusted, removed, and runtime-replaced targets. +3. Turn-output tests verify secondary downloads and open requests use the + secondary owner. +4. Artifact-panel tests verify missing/stale ownership fails closed and all + durable scheduled-task operations include `workspaceId`. +5. A production bundle is run against two registered local workspaces with + distinct sentinel files/tasks. Captured requests must use the secondary + workspace-qualified routes and leave the primary sentinels unchanged. diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 3321f803f0b..852e33cd1d4 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -239,6 +239,9 @@ const { addScratchWorkspace: vi.fn(), suggestWorkspacePaths: vi.fn(), pickWorkspaceDirectory: vi.fn(), + listScheduledTasks: vi.fn(), + updateScheduledTask: vi.fn(), + deleteScheduledTask: vi.fn(), }, mockMcp: { initialize: vi.fn().mockResolvedValue({ accepted: true }), @@ -909,11 +912,7 @@ vi.doMock('./components/SplitView', async () => { onExit?: () => void; sessionIds?: string[]; onPanesChange?: (ids: string[]) => void; - onPaneArtifactsChange?: ( - sessionId: string, - artifacts: unknown[], - workspaceActions: unknown, - ) => void; + onPaneArtifactsChange?: (sessionId: string, artifacts: unknown[]) => void; onRightPanelOpen?: (request: unknown) => void; onOpenMonitor?: ( task: DaemonSessionMonitorTaskStatus, @@ -926,9 +925,6 @@ vi.doMock('./components/SplitView', async () => { }) => unknown; voiceWorkspaces?: readonly unknown[]; }) => { - const paneActions = { - readWorkspaceFile: vi.fn().mockResolvedValue('

pane

'), - }; const artifact = { id: 'pane-artifact', kind: 'report', @@ -949,6 +945,35 @@ vi.doMock('./components/SplitView', async () => { ...updatedArtifact, status: 'changed', }; + const mainArtifact = { + id: 'main-artifact', + kind: 'report', + storage: 'memory', + source: 'tool', + status: 'available', + title: 'Main artifact', + updatedAt: '2026-07-10T00:00:00Z', + sizeBytes: 10, + }; + const paneScheduledTask = { + id: 'pane-cron', + toolCallId: 'pane-cron-call', + title: 'Pane task', + cron: '0 9 * * *', + prompt: 'pane task prompt', + recurring: true, + durable: true, + workspaceId: 'pane-ws', + }; + const paneReviewChanges = [ + { + path: 'notes.md', + status: 'modified', + toolCallId: 'tool-notes', + isArtifact: false, + diffs: [], + }, + ]; return React.createElement( 'div', { 'data-testid': 'split-view-mock' }, @@ -981,11 +1006,7 @@ vi.doMock('./components/SplitView', async () => { 'data-testid': 'split-report-artifact', type: 'button', onClick: () => - props.onPaneArtifactsChange?.( - 'pane-session', - [artifact], - paneActions, - ), + props.onPaneArtifactsChange?.('pane-session', [artifact]), }, 'artifact', ), @@ -995,11 +1016,7 @@ vi.doMock('./components/SplitView', async () => { 'data-testid': 'split-report-updated-artifact', type: 'button', onClick: () => - props.onPaneArtifactsChange?.( - 'pane-session', - [updatedArtifact], - paneActions, - ), + props.onPaneArtifactsChange?.('pane-session', [updatedArtifact]), }, 'updated artifact', ), @@ -1009,11 +1026,7 @@ vi.doMock('./components/SplitView', async () => { 'data-testid': 'split-report-changed-artifact', type: 'button', onClick: () => - props.onPaneArtifactsChange?.( - 'pane-session', - [changedArtifact], - paneActions, - ), + props.onPaneArtifactsChange?.('pane-session', [changedArtifact]), }, 'changed artifact', ), @@ -1022,8 +1035,7 @@ vi.doMock('./components/SplitView', async () => { { 'data-testid': 'split-clear-artifacts', type: 'button', - onClick: () => - props.onPaneArtifactsChange?.('pane-session', [], paneActions), + onClick: () => props.onPaneArtifactsChange?.('pane-session', []), }, 'clear artifacts', ), @@ -1040,12 +1052,71 @@ vi.doMock('./components/SplitView', async () => { turnId: 'turn-1', artifactId: artifact.id, artifact, - workspaceActions: paneActions, + workspaceCwd: '/tmp/project', + workspaceId: 'primary', + sourceSessionId: 'pane-session', previewContent: '

stale

', }), }, 'open artifact', ), + React.createElement( + 'button', + { + 'data-testid': 'split-open-main-artifact', + type: 'button', + onClick: () => + props.onRightPanelOpen?.({ + id: 'artifact:main-artifact', + kind: 'artifact', + title: mainArtifact.title, + turnId: 'turn-1', + artifactId: mainArtifact.id, + artifact: mainArtifact, + workspaceCwd: '/tmp/project', + workspaceId: 'primary', + }), + }, + 'open main artifact', + ), + React.createElement( + 'button', + { + 'data-testid': 'split-open-scheduled-task', + type: 'button', + onClick: () => + props.onRightPanelOpen?.({ + id: 'scheduled-task:pane-cron-call', + kind: 'scheduled_task', + title: 'Scheduled Tasks', + turnId: 'turn-1', + task: paneScheduledTask, + workspaceCwd: '/tmp/pane', + workspaceId: 'pane-ws', + sourceSessionId: 'pane-session', + }), + }, + 'open scheduled task', + ), + React.createElement( + 'button', + { + 'data-testid': 'split-open-review', + type: 'button', + onClick: () => + props.onRightPanelOpen?.({ + id: 'review', + kind: 'review', + title: 'Review', + turnId: 'turn-1', + changes: paneReviewChanges, + workspaceCwd: '/tmp/pane', + workspaceId: 'pane-ws', + sourceSessionId: 'pane-session', + }), + }, + 'open review', + ), React.createElement( 'button', { @@ -2352,6 +2423,9 @@ beforeEach(() => { mockWorkspaceActions.addScratchWorkspace.mockReset(); mockWorkspaceActions.suggestWorkspacePaths.mockReset(); mockWorkspaceActions.pickWorkspaceDirectory.mockReset(); + mockWorkspaceActions.listScheduledTasks.mockReset(); + mockWorkspaceActions.updateScheduledTask.mockReset(); + mockWorkspaceActions.deleteScheduledTask.mockReset(); mockMcp.initialize.mockClear(); mockMcp.initialize.mockResolvedValue({ accepted: true }); mockMcp.reloadConfig.mockClear(); @@ -4554,6 +4628,17 @@ describe('App session callbacks', () => { }); it('opens the latest reviewable turn from the empty right panel', () => { + mockWorkspace.capabilities = { + workspaceCwd: '/tmp/project', + workspaces: [ + { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + }, + ], + } as typeof mockWorkspace.capabilities; testState.messages = [ { id: 'user-1', @@ -9897,7 +9982,18 @@ describe('App session callbacks', () => { ).toBe('s1,s2,s3'); }); - it('reconciles split pane artifact snapshots in the right panel', async () => { + it('updates an open artifact tab from pane snapshots and keeps it after the pane clears', async () => { + mockWorkspace.capabilities = { + workspaceCwd: '/tmp/project', + workspaces: [ + { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + }, + ], + } as typeof mockWorkspace.capabilities; const { container } = renderApp(); await flush(); @@ -9909,15 +10005,15 @@ describe('App session callbacks', () => { }); await act(async () => { container - .querySelector( - '[data-testid="split-report-artifact"]', - ) + .querySelector('[data-testid="split-open-artifact"]') ?.click(); await Promise.resolve(); }); await act(async () => { container - .querySelector('[data-testid="split-open-artifact"]') + .querySelector( + '[data-testid="split-report-artifact"]', + ) ?.click(); await Promise.resolve(); }); @@ -9956,7 +10052,232 @@ describe('App session callbacks', () => { await Promise.resolve(); }); - expect(document.body.textContent).toContain('Artifact not found.'); + // The pane snapshot is gone, but the extra pushed on open keeps the + // still-open tab renderable instead of orphaning it. + expect(document.body.textContent).toContain('Pane artifact'); + expect(document.body.textContent).toContain('10 B'); + expect(document.body.textContent).not.toContain('Artifact not found.'); + }); + + it('routes a split pane scheduled task through its stamped workspace identity', async () => { + mockWorkspace.capabilities = { + workspaceCwd: '/tmp/project', + workspaces: [ + { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + }, + { + id: 'pane-ws', + cwd: '/tmp/pane', + primary: false, + trusted: true, + }, + ], + } as typeof mockWorkspace.capabilities; + mockWorkspaceActions.listScheduledTasks.mockResolvedValue([ + { + id: 'pane-cron', + name: 'Pane task', + cron: '0 9 * * *', + prompt: 'pane task prompt', + recurring: true, + enabled: true, + createdAt: 1_700_000_000_000, + lastFiredAt: null, + nextRunAt: null, + sessionId: null, + runs: [], + }, + ]); + const { container } = renderApp(); + await flush(); + + await act(async () => { + container + .querySelector('[data-testid="open-split-view"]') + ?.click(); + await Promise.resolve(); + }); + await act(async () => { + container + .querySelector( + '[data-testid="split-open-scheduled-task"]', + ) + ?.click(); + await Promise.resolve(); + }); + + expect(mockWorkspaceActions.listScheduledTasks).toHaveBeenCalledWith( + 'pane-ws', + ); + expect(document.body.textContent).toContain('Pane task'); + expect(document.body.textContent).not.toContain( + 'This workspace may have been removed', + ); + }); + + it('routes a split pane review download through its stamped workspace identity', async () => { + mockWorkspace.capabilities = { + workspaceCwd: '/tmp/project', + workspaces: [ + { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + }, + { + id: 'pane-ws', + cwd: '/tmp/pane', + primary: false, + trusted: true, + }, + ], + } as typeof mockWorkspace.capabilities; + const paneFileStat = vi.fn().mockResolvedValue({ + sizeBytes: 5, + modifiedMs: 1, + }); + const paneReadBytes = vi.fn().mockResolvedValue({ + contentBase64: btoa('notes'), + offset: 0, + returnedBytes: 5, + sizeBytes: 5, + }); + const paneWorkspaceClient = { + workspaceGit: vi.fn().mockResolvedValue({ branch: 'main' }), + workspaceSkills: mockWorkspaceActions.loadSkillsStatus, + workspaceGitHubPullRequests: vi.fn().mockResolvedValue({ + v: 1, + workspaceCwd: '/tmp/pane', + available: true, + pullRequests: [], + }), + fileStat: paneFileStat, + readWorkspaceFileBytes: paneReadBytes, + }; + mockWorkspace.client.workspaceByCwd.mockImplementation( + () => paneWorkspaceClient, + ); + Object.defineProperty(URL, 'createObjectURL', { + configurable: true, + value: vi.fn(() => 'blob:pane-review'), + }); + Object.defineProperty(URL, 'revokeObjectURL', { + configurable: true, + value: vi.fn(), + }); + const { container } = renderApp(); + await flush(); + + await act(async () => { + container + .querySelector('[data-testid="open-split-view"]') + ?.click(); + await Promise.resolve(); + }); + await act(async () => { + container + .querySelector('[data-testid="split-open-review"]') + ?.click(); + await Promise.resolve(); + }); + + const download = Array.from(document.body.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Download', + ); + expect(download).toBeDefined(); + await act(async () => { + download?.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mockWorkspace.client.workspaceByCwd).toHaveBeenCalledWith( + '/tmp/pane', + ); + expect(paneFileStat).toHaveBeenCalledWith('notes.md'); + expect(paneReadBytes).toHaveBeenCalledWith( + 'notes.md', + expect.objectContaining({ offset: 0 }), + ); + }); + + it('keeps a main-session artifact tab renderable across a live-list gap', async () => { + mockWorkspace.capabilities = { + workspaceCwd: '/tmp/project', + workspaces: [ + { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + }, + ], + } as typeof mockWorkspace.capabilities; + mockConnection.capabilities = { + ...mockConnection.capabilities, + features: ['session_artifacts'], + }; + const mainArtifactRow = { + id: 'main-artifact', + kind: 'report', + storage: 'memory', + source: 'tool', + status: 'available', + title: 'Main artifact', + updatedAt: '2026-07-10T00:00:00Z', + sizeBytes: 10, + }; + mockSessionActions.loadArtifacts.mockResolvedValue({ + artifacts: [mainArtifactRow], + }); + const { container, rerender } = renderApp(); + await flush(); + + await act(async () => { + container + .querySelector('[data-testid="open-split-view"]') + ?.click(); + await Promise.resolve(); + }); + await act(async () => { + container + .querySelector( + '[data-testid="split-open-main-artifact"]', + ) + ?.click(); + await Promise.resolve(); + }); + + expect(document.body.textContent).toContain('Main artifact'); + expect(document.body.textContent).toContain('10 B'); + + // A transient disconnect empties the live artifact list; the cached + // open-time row keeps the tab renderable through the gap. + mockConnection.status = 'disconnected'; + await act(async () => { + rerender(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(document.body.textContent).toContain('Main artifact'); + expect(document.body.textContent).not.toContain('Artifact not found.'); + + // Reconnecting restores the live list and reconciles the cached copy. + mockConnection.status = 'connected'; + await act(async () => { + rerender(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(document.body.textContent).toContain('Main artifact'); + mockSessionActions.loadArtifacts.mockResolvedValue({ artifacts: [] }); }); it('opens a split pane monitor in the right panel', async () => { @@ -9985,6 +10306,17 @@ describe('App session callbacks', () => { }); it('clears split pane artifact snapshots when switching sessions', async () => { + mockWorkspace.capabilities = { + workspaceCwd: '/tmp/project', + workspaces: [ + { + id: 'primary', + cwd: '/tmp/project', + primary: true, + trusted: true, + }, + ], + } as typeof mockWorkspace.capabilities; const { container, rerender } = renderApp(); await flush(); @@ -10010,6 +10342,10 @@ describe('App session callbacks', () => { }); expect(document.body.textContent).toContain('Pane artifact'); + expect(document.body.textContent).toContain('10 B'); + expect(document.body.textContent).not.toContain( + 'This workspace may have been removed', + ); await act(async () => { mockConnection.sessionId = 'session-2'; diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 0ec17b82cc1..eb5bd408781 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -26,7 +26,6 @@ import { useWorkspaceActions, useWorkspaceEventSignals, type DaemonSessionActions, - type DaemonWorkspaceActions, type DaemonSessionNotice, type DaemonStreamingState, } from '@qwen-code/webui/daemon-react-sdk'; @@ -124,6 +123,7 @@ import { getFileChangePreviewContent, TURN_OUTPUT_KINDS, } from './components/artifacts/TurnOutputs'; +import { useArtifactWorkspaceTarget } from './components/artifacts/useArtifactWorkspaceTarget'; import { getArtifactsByTurn, getFileChangesByTurn, @@ -386,7 +386,6 @@ interface ArtifactPanelSessionState { } interface PaneArtifactSnapshot { artifacts: readonly DaemonSessionArtifact[]; - workspaceActions: DaemonWorkspaceActions; } // Cap on how long a manual "run now" waits for its bound session to become // active before giving up, so the scheduled-tasks UI can't stay stuck disabled @@ -1883,6 +1882,9 @@ export function App({ ) === true; const { notices, dismissNotice } = useSessionNotices(); const workspaceActions = useWorkspaceActions(); + const artifactWorkspaceTarget = useArtifactWorkspaceTarget( + connection.workspaceCwd, + ); const dynamicWorkspaceRegistrationSupported = workspace.capabilities?.features?.includes( 'dynamic_workspace_registration', @@ -2240,15 +2242,18 @@ export function App({ return; } const artifactIds = new Set(artifacts.map((artifact) => artifact.id)); - const paneArtifactIds = new Set( + // Extras referenced by open artifact tabs must survive the reconcile: + // they keep those tabs renderable through transient gaps in the live + // list, and stay inert while the live list (merged first) covers them. + const openArtifactIds = new Set( artifactPanelTabs - .filter((tab) => tab.kind === 'artifact' && tab.workspaceActions) + .filter((tab) => tab.kind === 'artifact') .map((tab) => (tab.kind === 'artifact' ? tab.artifactId : '')), ); setArtifactPanelExtraArtifacts((previous) => { const next = previous.filter( (artifact) => - !artifactIds.has(artifact.id) || paneArtifactIds.has(artifact.id), + !artifactIds.has(artifact.id) || openArtifactIds.has(artifact.id), ); return next.length === previous.length ? previous : next; }); @@ -2268,9 +2273,11 @@ export function App({ return artifacts; } const merged = [...artifacts]; + // Pane snapshots outrank open-time extras so a retained extra never + // shadows a fresher pane report for the same artifact id. for (const artifact of [ - ...artifactPanelExtraArtifacts, ...paneArtifactExtras, + ...artifactPanelExtraArtifacts, ]) { const index = merged.findIndex((item) => item.id === artifact.id); if (index < 0) { @@ -2283,13 +2290,31 @@ export function App({ ( paneSessionId: string, paneArtifacts: readonly DaemonSessionArtifact[], - paneWorkspaceActions: DaemonWorkspaceActions, ) => { + if (paneArtifacts.length > 0) { + const paneArtifactIds = new Set( + paneArtifacts.map((artifact) => artifact.id), + ); + // Keep extras whose artifact tab is still open: once the pane closes + // and its snapshot is dropped, the extra is the tab's only copy. + const openArtifactIds = new Set( + artifactPanelTabsRef.current + .filter((tab) => tab.kind === 'artifact') + .map((tab) => (tab.kind === 'artifact' ? tab.artifactId : '')), + ); + setArtifactPanelExtraArtifacts((current) => { + const next = current.filter( + (artifact) => + !paneArtifactIds.has(artifact.id) || + openArtifactIds.has(artifact.id), + ); + return next.length === current.length ? current : next; + }); + } setPaneArtifactSnapshots((current) => { const previous = current.get(paneSessionId); const unchanged = - previous?.workspaceActions === paneWorkspaceActions && - previous.artifacts.length === paneArtifacts.length && + previous?.artifacts.length === paneArtifacts.length && previous.artifacts.every((artifact, index) => { const nextArtifact = paneArtifacts[index]; // `metadata` is deliberately not compared: artifact events carry @@ -2311,32 +2336,10 @@ export function App({ } else { next.set(paneSessionId, { artifacts: [...paneArtifacts], - workspaceActions: paneWorkspaceActions, }); } return next; }); - const artifactIds = new Set(paneArtifacts.map((artifact) => artifact.id)); - setArtifactPanelTabs((tabs) => { - let changed = false; - const next = tabs.map((tab) => { - if (tab.kind !== 'artifact' || !artifactIds.has(tab.artifactId)) { - return tab; - } - const updated = { - id: tab.id, - kind: 'artifact' as const, - title: tab.title, - artifactId: tab.artifactId, - workspaceActions: tab.workspaceActions ?? paneWorkspaceActions, - }; - if (tab.previewContent !== undefined) changed = true; - if (tab.workspaceActions) return updated; - changed = true; - return updated; - }); - return changed ? next : tabs; - }); }, [], ); @@ -2750,6 +2753,12 @@ export function App({ kind: 'artifact', artifactId, title: artifact?.title ?? 'Artifact', + ...(connection.workspaceCwd + ? { workspaceCwd: connection.workspaceCwd } + : {}), + ...(artifactWorkspaceTarget?.workspaceId + ? { workspaceId: artifactWorkspaceTarget.workspaceId } + : {}), ...(previewContent !== undefined ? { previewContent } : {}), }; setArtifactPanelTabs((tabs) => @@ -2765,14 +2774,19 @@ export function App({ ); setArtifactPanelOpen(true); }, - [artifactPanelArtifacts, getDefaultReviewPanelWidth], + [ + artifactPanelArtifacts, + artifactWorkspaceTarget?.workspaceId, + connection.workspaceCwd, + getDefaultReviewPanelWidth, + ], ); const openReviewPanel = useCallback( ( changes: readonly TurnOutputFileChange[], selectedPath?: string, - workspaceActions?: DaemonWorkspaceActions, reviewWorkspaceCwd?: string, + reviewWorkspaceId?: string, tabId = 'review', ) => { const reviewTab: ArtifactPanelTab = { @@ -2781,8 +2795,8 @@ export function App({ title: t('turnOutputs.review'), changes, ...(selectedPath ? { selectedPath } : {}), - ...(workspaceActions ? { workspaceActions } : {}), ...(reviewWorkspaceCwd ? { workspaceCwd: reviewWorkspaceCwd } : {}), + ...(reviewWorkspaceId ? { workspaceId: reviewWorkspaceId } : {}), }; setArtifactPanelTabs((tabs) => tabs.some((item) => item.id === reviewTab.id) @@ -2801,12 +2815,23 @@ export function App({ ); const openLatestReviewPanel = useCallback(() => { if (latestReviewChanges.length === 0) return; - openReviewPanel(latestReviewChanges); - }, [latestReviewChanges, openReviewPanel]); + openReviewPanel( + latestReviewChanges, + undefined, + connection.workspaceCwd, + artifactWorkspaceTarget?.workspaceId, + ); + }, [ + artifactWorkspaceTarget?.workspaceId, + connection.workspaceCwd, + latestReviewChanges, + openReviewPanel, + ]); const openScheduledTaskPanel = useCallback( ( task: TurnOutputScheduledTask, - tabWorkspaceActions?: ReturnType, + tabWorkspaceCwd?: string, + tabWorkspaceId?: string, sourceSessionId?: string, ) => { const tab: ArtifactPanelTab = { @@ -2815,10 +2840,9 @@ export function App({ : `scheduled-task:${task.toolCallId}`, kind: 'scheduled_task', title: t('scheduledTasks.title'), - task, - ...(tabWorkspaceActions - ? { workspaceActions: tabWorkspaceActions } - : {}), + task: tabWorkspaceId ? { ...task, workspaceId: tabWorkspaceId } : task, + ...(tabWorkspaceCwd ? { workspaceCwd: tabWorkspaceCwd } : {}), + ...(tabWorkspaceId ? { workspaceId: tabWorkspaceId } : {}), }; setArtifactPanelTabs((tabs) => tabs.some((item) => item.id === tab.id) @@ -2993,8 +3017,8 @@ export function App({ openReviewPanel( request.changes, request.selectedPath, - request.workspaceActions, request.workspaceCwd, + request.workspaceId, request.sourceSessionId ? `review:${request.sourceSessionId}:${request.turnId}` : undefined, @@ -3004,7 +3028,8 @@ export function App({ if (request.kind === 'scheduled_task') { openScheduledTaskPanel( request.task, - request.workspaceActions, + request.workspaceCwd, + request.workspaceId, request.sourceSessionId, ); return; @@ -3017,17 +3042,19 @@ export function App({ ); return; } - if (!request.workspaceActions || request.sourceSessionId) { - setArtifactPanelExtraArtifacts((current) => { - const index = current.findIndex( - (artifact) => artifact.id === request.artifact.id, - ); - if (index < 0) return [...current, request.artifact]; - const next = [...current]; - next[index] = request.artifact; - return next; - }); - } + // Cache the opened row so the tab keeps rendering through transient + // gaps in the live artifact lists (an SSE reconnect, or the source + // pane closing); the snapshot/live-list reconciles drop the copy once + // a fresher source covers the artifact again. + setArtifactPanelExtraArtifacts((current) => { + const index = current.findIndex( + (artifact) => artifact.id === request.artifact.id, + ); + if (index < 0) return [...current, request.artifact]; + const next = [...current]; + next[index] = request.artifact; + return next; + }); const tab: ArtifactPanelTab = { id: request.sourceSessionId ? `${request.sourceSessionId}:${request.id}` @@ -3035,8 +3062,10 @@ export function App({ kind: 'artifact', title: request.title, artifactId: request.artifactId, - ...(request.workspaceActions - ? { workspaceActions: request.workspaceActions } + ...(request.workspaceCwd ? { workspaceCwd: request.workspaceCwd } : {}), + ...(request.workspaceId ? { workspaceId: request.workspaceId } : {}), + ...(request.sourceSessionId + ? { sourceSessionId: request.sourceSessionId } : {}), ...(request.previewContent !== undefined ? { previewContent: request.previewContent } @@ -3066,8 +3095,8 @@ export function App({ const openFilePreview = useCallback( ( change: TurnOutputFileChange, - workspaceActions: DaemonWorkspaceActions, previewWorkspaceCwd?: string, + previewWorkspaceId?: string, ) => { const previewContent = getFileChangePreviewContent(change); const tab: ArtifactPanelTab = { @@ -3075,7 +3104,8 @@ export function App({ kind: 'file', title: displayPath(change.path, previewWorkspaceCwd), workspacePath: change.path, - workspaceActions, + ...(previewWorkspaceCwd ? { workspaceCwd: previewWorkspaceCwd } : {}), + ...(previewWorkspaceId ? { workspaceId: previewWorkspaceId } : {}), ...(previewContent !== undefined ? { previewContent } : {}), }; setArtifactPanelTabs((tabs) => diff --git a/packages/web-shell/client/components/ChatPane.test.tsx b/packages/web-shell/client/components/ChatPane.test.tsx index 818577da41e..efb630751b2 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -144,6 +144,21 @@ vi.mock('./MessageList', () => ({ data-approval={props.pendingApproval ? 'yes' : 'no'} > {props.messages.length} +