From 8aadba342dc55cacc5cfed83a047c861eb11c9e3 Mon Sep 17 00:00:00 2001 From: Nothing Chan Date: Tue, 4 Aug 2026 13:03:20 +0800 Subject: [PATCH 1/5] fix(web-shell): scope artifact actions to owning workspace --- ...-secondary-artifact-workspace-ownership.md | 117 +++++++ .../web-shell-artifact-workspace-ownership.md | 101 ++++++ packages/web-shell/client/App.test.tsx | 56 ++-- packages/web-shell/client/App.tsx | 106 +++--- .../client/components/ChatPane.test.tsx | 8 +- .../web-shell/client/components/ChatPane.tsx | 25 +- .../web-shell/client/components/SplitView.tsx | 2 - .../artifacts/ArtifactPanel.test.tsx | 304 +++++++++++++++++- .../components/artifacts/ArtifactPanel.tsx | 227 ++++++++++--- .../artifacts/CodeReviewArtifactDetail.tsx | 6 +- .../components/artifacts/SideTaskPanel.tsx | 2 - .../SubagentDetail.integration.test.tsx | 7 +- .../components/artifacts/SubagentDetail.tsx | 19 +- .../artifacts/TurnOutputs.dom.test.tsx | 206 +++++++++++- .../components/artifacts/TurnOutputs.tsx | 32 +- .../useArtifactWorkspaceTarget.test.ts | 100 ++++++ .../artifacts/useArtifactWorkspaceTarget.ts | 168 ++++++++++ 17 files changed, 1294 insertions(+), 192 deletions(-) create mode 100644 .qwen/e2e-tests/2026-08-04-secondary-artifact-workspace-ownership.md create mode 100644 docs/design/web-shell-artifact-workspace-ownership.md create mode 100644 packages/web-shell/client/components/artifacts/useArtifactWorkspaceTarget.test.ts create mode 100644 packages/web-shell/client/components/artifacts/useArtifactWorkspaceTarget.ts 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..07ad106baef 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -909,11 +909,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 +922,6 @@ vi.doMock('./components/SplitView', async () => { }) => unknown; voiceWorkspaces?: readonly unknown[]; }) => { - const paneActions = { - readWorkspaceFile: vi.fn().mockResolvedValue('

pane

'), - }; const artifact = { id: 'pane-artifact', kind: 'report', @@ -981,11 +974,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 +984,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 +994,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 +1003,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,7 +1020,9 @@ vi.doMock('./components/SplitView', async () => { turnId: 'turn-1', artifactId: artifact.id, artifact, - workspaceActions: paneActions, + workspaceCwd: '/tmp/project', + workspaceId: 'primary', + sourceSessionId: 'pane-session', previewContent: '

stale

', }), }, @@ -4554,6 +4536,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', @@ -9898,6 +9891,17 @@ describe('App session callbacks', () => { }); it('reconciles split pane artifact snapshots in the right panel', 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(); diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 0ec17b82cc1..a4c3540607f 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', @@ -2242,7 +2244,9 @@ export function App({ const artifactIds = new Set(artifacts.map((artifact) => artifact.id)); const paneArtifactIds = new Set( artifactPanelTabs - .filter((tab) => tab.kind === 'artifact' && tab.workspaceActions) + .filter( + (tab) => tab.kind === 'artifact' && tab.sourceSessionId !== undefined, + ) .map((tab) => (tab.kind === 'artifact' ? tab.artifactId : '')), ); setArtifactPanelExtraArtifacts((previous) => { @@ -2283,13 +2287,22 @@ export function App({ ( paneSessionId: string, paneArtifacts: readonly DaemonSessionArtifact[], - paneWorkspaceActions: DaemonWorkspaceActions, ) => { + if (paneArtifacts.length > 0) { + const paneArtifactIds = new Set( + paneArtifacts.map((artifact) => artifact.id), + ); + setArtifactPanelExtraArtifacts((current) => { + const next = current.filter( + (artifact) => !paneArtifactIds.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 +2324,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 +2741,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 +2762,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 +2783,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 +2803,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 +2828,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 +3005,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 +3016,8 @@ export function App({ if (request.kind === 'scheduled_task') { openScheduledTaskPanel( request.task, - request.workspaceActions, + request.workspaceCwd, + request.workspaceId, request.sourceSessionId, ); return; @@ -3017,7 +3030,7 @@ export function App({ ); return; } - if (!request.workspaceActions || request.sourceSessionId) { + if (request.sourceSessionId) { setArtifactPanelExtraArtifacts((current) => { const index = current.findIndex( (artifact) => artifact.id === request.artifact.id, @@ -3035,8 +3048,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 +3081,8 @@ export function App({ const openFilePreview = useCallback( ( change: TurnOutputFileChange, - workspaceActions: DaemonWorkspaceActions, previewWorkspaceCwd?: string, + previewWorkspaceId?: string, ) => { const previewContent = getFileChangePreviewContent(change); const tab: ArtifactPanelTab = { @@ -3075,7 +3090,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..55728390d57 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -706,11 +706,9 @@ describe('ChatPane', () => { await Promise.resolve(); }); - expect(onPaneArtifactsChange).toHaveBeenLastCalledWith( - 'sess-1', - [artifact], - expect.any(Object), - ); + expect(onPaneArtifactsChange).toHaveBeenLastCalledWith('sess-1', [ + artifact, + ]); }); it('suppresses the rotating loading phrase in its compact status', () => { diff --git a/packages/web-shell/client/components/ChatPane.tsx b/packages/web-shell/client/components/ChatPane.tsx index fbd425fdef5..fd3a22e0410 100644 --- a/packages/web-shell/client/components/ChatPane.tsx +++ b/packages/web-shell/client/components/ChatPane.tsx @@ -14,9 +14,7 @@ import { useTranscriptHistory, useTranscriptStore, useWorkspace, - useWorkspaceActions, type DaemonSessionActions, - type DaemonWorkspaceActions, } from '@qwen-code/webui/daemon-react-sdk'; import type { DaemonSessionArtifact, @@ -171,7 +169,6 @@ export interface ChatPaneProps { onPaneArtifactsChange?: ( sessionId: string, artifacts: readonly DaemonSessionArtifact[], - workspaceActions: DaemonWorkspaceActions, ) => void; messageTurnOutputs?: readonly TurnOutputKind[]; /** Allow prompt admission to recover a disconnected SSE stream. */ @@ -221,7 +218,6 @@ export function ChatPane({ useWebShellCustomization(); const connection = useConnection(); const actions = useActions(); - const workspaceActions = useWorkspaceActions(); const workspace = useWorkspace(); const blocks = useAnimationFrameTranscriptBlocks(); const messages = useMessagesFromBlocks(t, blocks); @@ -292,16 +288,11 @@ export function ChatPane({ useEffect(() => { const sessionId = connection.sessionId; if (!sessionId) return; - onPaneArtifactsChange?.(sessionId, artifacts, workspaceActions); + onPaneArtifactsChange?.(sessionId, artifacts); return () => { - onPaneArtifactsChange?.(sessionId, [], workspaceActions); + onPaneArtifactsChange?.(sessionId, []); }; - }, [ - artifacts, - connection.sessionId, - onPaneArtifactsChange, - workspaceActions, - ]); + }, [artifacts, connection.sessionId, onPaneArtifactsChange]); const streamingStateRef = useRef(streamingState); streamingStateRef.current = streamingState; const firstPromptAdmittedRef = useRef(false); @@ -539,20 +530,12 @@ export function ChatPane({ const handleRightPanelOpen = useCallback( (request: TurnOutputOpenRequest) => { if (!onRightPanelOpen) return; - if (request.kind === 'subagent') { - onRightPanelOpen({ - ...request, - sourceSessionId: connection.sessionId, - }); - return; - } onRightPanelOpen({ ...request, - workspaceActions, sourceSessionId: connection.sessionId, }); }, - [connection.sessionId, onRightPanelOpen, workspaceActions], + [connection.sessionId, onRightPanelOpen], ); // Composer wiring, all scoped to THIS pane's own DaemonSession context. The diff --git a/packages/web-shell/client/components/SplitView.tsx b/packages/web-shell/client/components/SplitView.tsx index 53c2252d89c..592b96e808a 100644 --- a/packages/web-shell/client/components/SplitView.tsx +++ b/packages/web-shell/client/components/SplitView.tsx @@ -9,7 +9,6 @@ import { DaemonSessionProvider, useConnection, type DaemonSessionActions, - type DaemonWorkspaceActions, } from '@qwen-code/webui/daemon-react-sdk'; import type { DaemonSessionArtifact, @@ -67,7 +66,6 @@ export interface SplitViewProps { onPaneArtifactsChange?: ( sessionId: string, artifacts: readonly DaemonSessionArtifact[], - workspaceActions: DaemonWorkspaceActions, ) => void; messageTurnOutputs?: readonly TurnOutputKind[]; /** diff --git a/packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx b/packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx index ed1613e1738..355a7fa5d74 100644 --- a/packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx +++ b/packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx @@ -7,26 +7,68 @@ import type { DaemonSessionMonitorTaskStatus, DaemonSessionShellTaskStatus, } from '@qwen-code/sdk/daemon'; -import type { DaemonSessionActions } from '@qwen-code/webui/daemon-react-sdk'; +import type { + DaemonScheduledTask, + DaemonSessionActions, +} from '@qwen-code/webui/daemon-react-sdk'; import { I18nProvider } from '../../i18n'; -const { mockActions, mockWorkspaceActions } = vi.hoisted(() => ({ - mockActions: { - cancelTask: vi.fn(), - getTasks: vi.fn(), - }, - mockWorkspaceActions: { - readFileBytes: vi.fn(), +const { + mockActions, + mockWorkspace, + mockWorkspaceActions, + mockSecondaryWorkspaceActions, +} = vi.hoisted(() => { + const mockSecondaryWorkspaceActions = { readWorkspaceFile: vi.fn(), - stat: vi.fn(), - }, -})); + readWorkspaceFileBytes: vi.fn(), + fileStat: vi.fn(), + }; + return { + mockActions: { + cancelTask: vi.fn(), + getTasks: vi.fn(), + }, + mockWorkspaceActions: { + readFileBytes: vi.fn(), + readWorkspaceFile: vi.fn(), + stat: vi.fn(), + listScheduledTasks: vi.fn(), + updateScheduledTask: vi.fn(), + deleteScheduledTask: vi.fn(), + }, + mockSecondaryWorkspaceActions, + mockWorkspace: { + capabilities: { + workspaceCwd: '/primary', + workspaces: [ + { + id: 'primary-id', + cwd: '/primary', + primary: true, + trusted: true, + }, + { + id: 'secondary-id', + cwd: '/secondary', + primary: false, + trusted: true, + }, + ], + }, + client: { + workspaceByCwd: vi.fn(() => mockSecondaryWorkspaceActions), + }, + }, + }; +}); vi.mock( '@qwen-code/webui/daemon-react-sdk', async (importOriginal: () => Promise>) => ({ ...(await importOriginal()), useActions: () => mockActions, + useWorkspace: () => mockWorkspace, useWorkspaceActions: () => mockWorkspaceActions, }), ); @@ -113,7 +155,13 @@ function codeReviewArtifact( }; } -function artifactPanel(artifact: DaemonSessionArtifact) { +function artifactPanel( + artifact: DaemonSessionArtifact, + owner: { workspaceCwd: string; workspaceId: string } | null = { + workspaceCwd: '/primary', + workspaceId: 'primary-id', + }, +) { return ( + {}} + onCloseTab={() => {}} + onOpenFilePreview={() => {}} + onClose={() => {}} + /> + + ); +} + afterEach(() => { for (const { root, container } of mounted) { act(() => root.unmount()); @@ -149,6 +248,30 @@ afterEach(() => { mockWorkspaceActions.readFileBytes.mockReset(); mockWorkspaceActions.readWorkspaceFile.mockReset(); mockWorkspaceActions.stat.mockReset(); + mockWorkspaceActions.listScheduledTasks.mockReset(); + mockWorkspaceActions.updateScheduledTask.mockReset(); + mockWorkspaceActions.deleteScheduledTask.mockReset(); + mockSecondaryWorkspaceActions.readWorkspaceFile.mockReset(); + mockSecondaryWorkspaceActions.readWorkspaceFileBytes.mockReset(); + mockSecondaryWorkspaceActions.fileStat.mockReset(); + mockWorkspace.client.workspaceByCwd.mockClear(); + mockWorkspace.capabilities = { + workspaceCwd: '/primary', + workspaces: [ + { + id: 'primary-id', + cwd: '/primary', + primary: true, + trusted: true, + }, + { + id: 'secondary-id', + cwd: '/secondary', + primary: false, + trusted: true, + }, + ], + }; }); function openAddMenu(container: HTMLElement) { @@ -171,6 +294,27 @@ async function flush() { } describe('ArtifactPanel code review artifacts', () => { + it('fails closed when an artifact tab has no workspace owner', async () => { + mockWorkspaceActions.readWorkspaceFile.mockResolvedValue({ + content: 'PRIMARY_WORKSPACE_SECRET', + truncated: false, + }); + const artifact = codeReviewArtifact({ metadata: {} }); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + mounted.push({ root, container }); + + act(() => root.render(artifactPanel(artifact, null))); + await flush(); + + expect(container.textContent).toContain( + 'This workspace may have been removed or the link is no longer valid.', + ); + expect(mockWorkspaceActions.readWorkspaceFile).not.toHaveBeenCalled(); + expect(container.textContent).not.toContain('PRIMARY_WORKSPACE_SECRET'); + }); + it('dispatches an available workspace artifact to the dedicated renderer', async () => { mockWorkspaceActions.readWorkspaceFile.mockResolvedValue({ content: JSON.stringify({ @@ -282,6 +426,124 @@ describe('ArtifactPanel code review artifacts', () => { '.qwen/reviews/review.json', ); }); + + it('discards a pending read when its workspace owner is replaced', async () => { + let resolveRead: + | ((file: { content: string; truncated: boolean }) => void) + | undefined; + mockSecondaryWorkspaceActions.readWorkspaceFile.mockReturnValue( + new Promise((resolve) => { + resolveRead = resolve; + }), + ); + const artifact = codeReviewArtifact({ metadata: {} }); + const owner = { + workspaceCwd: '/secondary', + workspaceId: 'secondary-id', + }; + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + mounted.push({ root, container }); + + act(() => root.render(artifactPanel(artifact, owner))); + await flush(); + expect(mockWorkspace.client.workspaceByCwd).toHaveBeenCalledWith( + '/secondary', + ); + + mockWorkspace.capabilities = { + ...mockWorkspace.capabilities, + workspaces: [ + mockWorkspace.capabilities.workspaces[0]!, + { + id: 'secondary-replacement-id', + cwd: '/secondary', + primary: false, + trusted: true, + }, + ], + }; + act(() => root.render(artifactPanel(artifact, owner))); + await flush(); + expect(container.querySelector('[role="alert"]')?.textContent).toContain( + 'removed', + ); + + await act(async () => { + resolveRead?.({ content: 'REMOVED_WORKSPACE_SECRET', truncated: false }); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(container.textContent).not.toContain('REMOVED_WORKSPACE_SECRET'); + expect(mockWorkspaceActions.readWorkspaceFile).not.toHaveBeenCalled(); + }); +}); + +describe('ArtifactPanel scheduled-task ownership', () => { + it('loads a durable task through its secondary workspace route', async () => { + mockWorkspaceActions.listScheduledTasks.mockResolvedValue([]); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + mounted.push({ root, container }); + + act(() => root.render(scheduledTaskPanel())); + await flush(); + + expect(mockWorkspaceActions.listScheduledTasks).toHaveBeenCalledWith( + 'secondary-id', + ); + }); + + it('updates and deletes only through the task workspace id', async () => { + mockWorkspaceActions.listScheduledTasks.mockResolvedValue([ + secondaryScheduledTask, + ]); + mockWorkspaceActions.updateScheduledTask.mockResolvedValue({ + ...secondaryScheduledTask, + enabled: false, + }); + mockWorkspaceActions.deleteScheduledTask.mockResolvedValue(undefined); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + mounted.push({ root, container }); + + act(() => root.render(scheduledTaskPanel())); + await flush(); + + const disable = Array.from(container.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Disable', + ); + await act(async () => { + disable?.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(mockWorkspaceActions.updateScheduledTask).toHaveBeenCalledWith( + 'cron-secondary', + { enabled: false }, + 'secondary-id', + ); + + const openDelete = Array.from(container.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Delete', + ); + act(() => openDelete?.click()); + const confirmDelete = Array.from(document.body.querySelectorAll('button')) + .filter((button) => button.textContent?.trim() === 'Delete') + .at(-1); + await act(async () => { + confirmDelete?.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(mockWorkspaceActions.deleteScheduledTask).toHaveBeenCalledWith( + 'cron-secondary', + 'secondary-id', + ); + }); }); describe('ArtifactPanel add menu', () => { @@ -564,7 +826,15 @@ describe('ArtifactPanel add menu', () => { { kind: 'artifact', title: 'Report', artifactId: 'report', + workspaceCwd: '/primary', + workspaceId: 'primary-id', }, ]} activeTabId="artifact" @@ -669,6 +941,8 @@ describe('ArtifactPanel review downloads', () => { kind: 'review', title: 'Review', changes, + workspaceCwd: '/primary', + workspaceId: 'primary-id', }, ]} activeTabId="review" @@ -737,6 +1011,8 @@ describe('ArtifactPanel review downloads', () => { kind: 'review', title: 'Review', changes, + workspaceCwd: '/primary', + workspaceId: 'primary-id', }, ]} activeTabId="review" @@ -796,6 +1072,8 @@ describe('ArtifactPanel review downloads', () => { kind: 'review', title: 'Review', changes, + workspaceCwd: '/primary', + workspaceId: 'primary-id', }, ]} activeTabId="review" diff --git a/packages/web-shell/client/components/artifacts/ArtifactPanel.tsx b/packages/web-shell/client/components/artifacts/ArtifactPanel.tsx index f2cb564ab3b..0386b8c97aa 100644 --- a/packages/web-shell/client/components/artifacts/ArtifactPanel.tsx +++ b/packages/web-shell/client/components/artifacts/ArtifactPanel.tsx @@ -6,9 +6,7 @@ import type { import type { ACPToolCall } from '../../adapters/types'; import type { WebShellRightPanelItem } from '../../customization'; import { - useWorkspaceActions, type DaemonSessionActions, - type DaemonWorkspaceActions, type DaemonScheduledTask, } from '@qwen-code/webui/daemon-react-sdk'; import { EditorState } from '@codemirror/state'; @@ -79,6 +77,10 @@ import styles from './ArtifactPanel.module.css'; import { CodeReviewArtifactDetail } from './CodeReviewArtifactDetail'; import { SubagentDetail } from './SubagentDetail'; import { SideTaskPanel } from './SideTaskPanel'; +import { + useArtifactWorkspaceTarget, + type ArtifactWorkspaceActions, +} from './useArtifactWorkspaceTarget'; import { MonitorTaskDetail, ShellTaskDetail, @@ -108,8 +110,8 @@ export type ArtifactPanelTab = id: string; kind: 'review'; title: string; - workspaceActions?: DaemonWorkspaceActions; workspaceCwd?: string; + workspaceId?: string; changes?: readonly TurnOutputFileChange[]; selectedPath?: string; } @@ -118,7 +120,8 @@ export type ArtifactPanelTab = kind: 'file'; title: string; workspacePath: string; - workspaceActions?: DaemonWorkspaceActions; + workspaceCwd?: string; + workspaceId?: string; previewContent?: string; } | { @@ -126,7 +129,9 @@ export type ArtifactPanelTab = kind: 'artifact'; title: string; artifactId: string; - workspaceActions?: DaemonWorkspaceActions; + workspaceCwd?: string; + workspaceId?: string; + sourceSessionId?: string; previewContent?: string; } | { @@ -134,7 +139,8 @@ export type ArtifactPanelTab = kind: 'scheduled_task'; title: string; task: TurnOutputScheduledTask; - workspaceActions?: DaemonWorkspaceActions; + workspaceCwd?: string; + workspaceId?: string; } | { id: string; @@ -172,6 +178,22 @@ export type ArtifactPanelTab = initialPrompt?: string; }; +type WorkspaceScopedArtifactPanelTab = Extract< + ArtifactPanelTab, + { kind: 'review' | 'file' | 'artifact' | 'scheduled_task' } +>; + +function isWorkspaceScopedTab( + tab: ArtifactPanelTab, +): tab is WorkspaceScopedArtifactPanelTab { + return ( + tab.kind === 'review' || + tab.kind === 'file' || + tab.kind === 'artifact' || + tab.kind === 'scheduled_task' + ); +} + export interface SideTaskListItem { sessionId: string; title: string; @@ -198,8 +220,8 @@ interface ArtifactPanelProps { onCloseTab: (tabId: string) => void; onOpenFilePreview: ( change: TurnOutputFileChange, - workspaceActions: DaemonWorkspaceActions, workspaceCwd?: string, + workspaceId?: string, ) => void; latestReviewAvailable?: boolean; onOpenLatestReview?: () => void; @@ -224,7 +246,6 @@ interface ArtifactPanelProps { onNestedArtifactsChange?: ( sessionId: string, artifacts: readonly DaemonSessionArtifact[], - workspaceActions: DaemonWorkspaceActions, ) => void; onError?: (error: unknown, fallback: string) => void; sessionWorkflowEnabled?: boolean; @@ -301,11 +322,20 @@ export function ArtifactPanel({ Boolean(onCreateSideTask); const showAddMenu = Boolean(activeTab) && (showReviewMenuItem || showSideTaskMenuItems); - const defaultWorkspaceActions = useWorkspaceActions(); + const activeWorkspaceIdentity = + activeTab && isWorkspaceScopedTab(activeTab) + ? { + workspaceCwd: activeTab.workspaceCwd, + workspaceId: activeTab.workspaceId, + } + : undefined; + const activeWorkspaceTarget = useArtifactWorkspaceTarget( + activeWorkspaceIdentity?.workspaceCwd, + ); const activeWorkspaceActions = - activeTab && 'workspaceActions' in activeTab - ? (activeTab.workspaceActions ?? defaultWorkspaceActions) - : defaultWorkspaceActions; + activeWorkspaceTarget?.workspaceId === activeWorkspaceIdentity?.workspaceId + ? activeWorkspaceTarget?.actions + : undefined; return (