From c08e1af358fd22e9d25324b09ce3b2c9f3440a85 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 2 Oct 2026 06:59:21 +0000 Subject: [PATCH 01/14] =?UTF-8?q?=F0=9F=A4=96=20feat:=20artifact=20version?= =?UTF-8?q?s,=20the=20artifact=20tool=20and=20pinned=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Artifact version store per workspace (`artifact-versions//vN`, content-hash dedupe) with list/read routes and a version menu in the Artifacts tab - `artifact` tool to publish a version (title, focus, pin request); attach_file registers files inside the artifacts dir via `ui_only.artifact` - Turn-end snapshots: one version per changed file per logical turn when nothing was published (goal board excluded) - Pinned workspace files (`pinFile` / `readPinned`) and a Pinned files picker group - Artifact chat card, "Open in Artifacts" on attach_file and file cards, "Open as artifact" in Review and the command palette - Post-compaction `artifacts_index` note; chat cards stay inert in hosts without an Artifacts surface (VS Code) - Stories for the chat cards and version menu (Pixel budget +6 snapshots, +1 file) - Codex review round 1 fixes: selection map gains scope (artifact or pinned) and version, the turn-end hook gets an abort signal at its timeout, the post-compaction artifacts index only with the experiment on, size+mtime shortcut only 1 s after mtime, deleted artifacts stay selectable at their latest stored version, pinFile relativeTo the tool cwd and refuses missing files, folders and symlinks, host path rules for the artifact tool, listVersions and listing errors shown inline, only the latest pin-and-open request opens the tab, Artifacts buttons hidden on hosts without the surface - Codex review round 2 fixes: the post-compaction index states only a count, pinned checkout files read on SSH and Docker again (requireArtifactsBasename), unchanged republishes rewrite the kind, the turn-end fast path needs an equal stored sourceModifiedMs, U unpins the selected pinned file, panel keys ignore the version menu, a publish cancelled during its read records nothing, a failed preview read retries on the next refresh - Codex review round 3 fixes: a new version re-checks the abort signal before the index commit, attach_file passes its abort signal to recordArtifactVersion and registers nothing once cancelled, artifact_list also lists deleted artifacts with stored versions, a finished unpin clears the selection only if it is still selected, the artifact card opens the tab only while the experiment is on, host pinned-file reads use the checkout root's real path, the live preview read key includes the file size --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$123.74`_ --- docs/hooks/tools.mdx | 13 + scripts/check-storybook-snapshot-budget.mjs | 6 +- .../check-test-seam-comments.allowlist.json | 1 + src/browser/App.tsx | 2 + .../WorkspaceMenuBar/WorkspaceMenuBar.tsx | 11 +- .../ArtifactsTab/ArtifactVersionMenu.tsx | 144 +++++ .../ArtifactsTab/ArtifactViewer.test.tsx | 8 + .../ArtifactsTab/ArtifactViewer.tsx | 21 +- .../ArtifactsTab/ArtifactsPanel.stories.tsx | 64 ++- .../ArtifactsTab/ArtifactsPanel.test.tsx | 425 ++++++++++++++- .../ArtifactsTab/ArtifactsPanel.tsx | 502 ++++++++++++++--- .../ArtifactsTab/MarkdownArtifact.tsx | 7 +- .../ArtifactsTab/OpenAsArtifactButton.tsx | 53 ++ .../ArtifactsTab/SandboxedArtifactFrame.tsx | 3 +- .../ArtifactsTab/artifactSelection.ts | 21 +- .../ArtifactsTab/mcpAppViewsStore.ts | 6 +- .../ArtifactsTab/openArtifact.test.ts | 123 +++++ .../RightSidebar/ArtifactsTab/openArtifact.ts | 108 ++++ .../ArtifactsTab/useArtifactAssetReader.ts | 10 +- .../RightSidebar/CodeReview/HunkViewer.tsx | 2 + .../features/RightSidebar/RightSidebar.tsx | 19 +- .../Settings/Sections/KeybindsSection.tsx | 2 + .../Tools/ArtifactToolCall.stories.tsx | 125 +++++ .../features/Tools/ArtifactToolCall.test.tsx | 204 +++++++ .../features/Tools/ArtifactToolCall.tsx | 136 +++++ .../features/Tools/AttachFileToolCall.tsx | 28 + .../features/Tools/FileEditToolCall.tsx | 10 + .../features/Tools/FileReadToolCall.tsx | 5 + .../features/Tools/GenericToolCall.tsx | 5 +- .../features/Tools/Shared/ToolPrimitives.tsx | 3 + .../features/Tools/Shared/getToolComponent.ts | 2 + src/browser/stories/mocks/orpc.ts | 51 +- src/browser/utils/commandIds.ts | 2 + src/browser/utils/commands/sources.test.ts | 39 ++ src/browser/utils/commands/sources.ts | 41 ++ .../utils/messages/attachmentRenderer.test.ts | 26 + .../utils/messages/attachmentRenderer.ts | 29 +- src/browser/utils/ui/keybinds.ts | 2 + src/common/constants/chatUiFeatures.ts | 2 + src/common/constants/events.ts | 15 + src/common/constants/storage.ts | 4 +- src/common/orpc/schemas/api.ts | 43 ++ src/common/orpc/schemas/artifacts.ts | 62 +++ .../orpc/schemas/continuousCompaction.ts | 6 + src/common/orpc/schemas/tools.ts | 7 + src/common/types/attachment.ts | 12 +- src/common/types/tools.ts | 8 + src/common/utils/tools/toolDefinitions.ts | 67 ++- src/common/utils/tools/toolOutputUiOnly.ts | 13 + src/common/utils/tools/tools.ts | 7 + src/node/orpc/router.ts | 34 ++ .../services/agentSession.logicalTurn.test.ts | 188 +++++++ ...tSession.postCompactionAttachments.test.ts | 36 ++ src/node/services/agentSession.testHarness.ts | 6 + src/node/services/agentSession.ts | 93 ++++ .../builtInSkillContent.generated.ts | 13 + .../services/artifactRuntimeStore.test.ts | 41 ++ src/node/services/artifactRuntimeStore.ts | 67 ++- src/node/services/artifactStore.ts | 109 +++- src/node/services/artifactVersionStore.ts | 282 ++++++++++ .../artifactVersionsOperations.test.ts | 505 ++++++++++++++++++ .../services/artifactVersionsOperations.ts | 398 ++++++++++++++ src/node/services/artifactsOperations.test.ts | 23 + src/node/services/artifactsOperations.ts | 89 ++- src/node/services/pinnedArtifactFiles.test.ts | 169 ++++++ src/node/services/pinnedArtifactFiles.ts | 355 ++++++++++++ src/node/services/tools/artifact.test.ts | 124 +++++ src/node/services/tools/artifact.ts | 59 ++ src/node/services/tools/artifact_list.test.ts | 32 ++ src/node/services/tools/artifact_list.ts | 44 +- src/node/services/tools/attach_file.test.ts | 65 ++- src/node/services/tools/attach_file.ts | 52 +- src/node/services/workspaceService.ts | 15 + ...orkflowRunRecordsFromModelMessages.test.ts | 27 + vscode/src/webview/chatUiCapabilities.ts | 3 + 75 files changed, 5153 insertions(+), 181 deletions(-) create mode 100644 src/browser/features/RightSidebar/ArtifactsTab/ArtifactVersionMenu.tsx create mode 100644 src/browser/features/RightSidebar/ArtifactsTab/OpenAsArtifactButton.tsx create mode 100644 src/browser/features/RightSidebar/ArtifactsTab/openArtifact.test.ts create mode 100644 src/browser/features/RightSidebar/ArtifactsTab/openArtifact.ts create mode 100644 src/browser/features/Tools/ArtifactToolCall.stories.tsx create mode 100644 src/browser/features/Tools/ArtifactToolCall.test.tsx create mode 100644 src/browser/features/Tools/ArtifactToolCall.tsx create mode 100644 src/node/services/agentSession.logicalTurn.test.ts create mode 100644 src/node/services/artifactVersionStore.ts create mode 100644 src/node/services/artifactVersionsOperations.test.ts create mode 100644 src/node/services/artifactVersionsOperations.ts create mode 100644 src/node/services/pinnedArtifactFiles.test.ts create mode 100644 src/node/services/pinnedArtifactFiles.ts create mode 100644 src/node/services/tools/artifact.test.ts create mode 100644 src/node/services/tools/artifact.ts diff --git a/docs/hooks/tools.mdx b/docs/hooks/tools.mdx index 17d5e8bc340..e41ba81a758 100644 --- a/docs/hooks/tools.mdx +++ b/docs/hooks/tools.mdx @@ -303,6 +303,19 @@ If a value is too large for the environment, it may be omitted (not set). Xum al +
+artifact (5) + +| Env var | JSON path | Type | Description | +| ---------------------- | --------- | ------- | ------------------------------------------------------------------- | +| `XUM_TOOL_INPUT_FOCUS` | `focus` | boolean | Open the Artifacts tab on this version for the user. | +| `XUM_TOOL_INPUT_KIND` | `kind` | enum | Override the viewer; by default it follows the file extension. | +| `XUM_TOOL_INPUT_PATH` | `path` | string | Path relative to $XUM_SCRATCH_DIR/artifacts, or absolute inside it. | +| `XUM_TOOL_INPUT_PIN` | `pin` | enum | Request that this artifact be kept on the project or global shelf. | +| `XUM_TOOL_INPUT_TITLE` | `title` | string | Short label for this version (defaults to the file name). | + +
+
ask_user_question (8) diff --git a/scripts/check-storybook-snapshot-budget.mjs b/scripts/check-storybook-snapshot-budget.mjs index c6280c8f5af..91b7c355ae2 100644 --- a/scripts/check-storybook-snapshot-budget.mjs +++ b/scripts/check-storybook-snapshot-budget.mjs @@ -31,8 +31,10 @@ import * as pixelUtils from "../node_modules/@coder/pixel-storybook/build/utils. // or intentionally rebalance snapshots rather than silently increasing Pixel load. // +4 snapshots / +1 file: the Artifacts tab gallery (ArtifactsPanel.stories.tsx), consolidated // from 24 per-renderer variants to one phone and one laptop story in dark + light. -const MAX_SNAPSHOTS = 659; -const MAX_SNAPSHOT_ENABLED_FILES = 116; +// +6 snapshots / +1 file: Artifacts M4 chat cards (ArtifactToolCall.stories.tsx, phone + laptop +// in dark + light) and the open version menu (ArtifactsPanel.stories.tsx, laptop dark + light). +const MAX_SNAPSHOTS = 665; +const MAX_SNAPSHOT_ENABLED_FILES = 117; const { values } = parseArgs({ options: { diff --git a/scripts/check-test-seam-comments.allowlist.json b/scripts/check-test-seam-comments.allowlist.json index 2e906829ead..5041f3fe39a 100644 --- a/scripts/check-test-seam-comments.allowlist.json +++ b/scripts/check-test-seam-comments.allowlist.json @@ -20,6 +20,7 @@ { "file": "src/node/services/agentPlugins/hookService.ts", "symbol": "readHookSourceCapped", "reason": "production caller: HookService.discoverHookPlugins in the same file" }, { "file": "src/node/services/agentPlugins/installService.ts", "symbol": "withDiskQuotaWatchdog", "reason": "production caller: runGit in the same file" }, { "file": "src/node/services/agentSkills/skillDynamicContext.ts", "symbol": "injectSkillDynamicContext", "reason": "injection point: the timeoutMs argument; skillDynamicContext.test.ts uses it to witness that a hung directive becomes a timeout note without failing the send; production (AgentSession) uses the default" }, + { "file": "src/node/services/agentSession.ts", "symbol": "AgentSessionOptions.logicalTurnCompletedTimeoutMs", "reason": "injection point: the logical-turn completion hook deadline (LOGICAL_TURN_COMPLETED_TIMEOUT_MS, 10 s); src/node/services/agentSession.logicalTurn.test.ts uses it to witness that a stalled artifact snapshot cannot hold the session busy; production uses the default" }, { "file": "src/node/services/agentSession.ts", "symbol": "AgentSessionOptions.planSnapshotCaptureTimeoutMs", "reason": "injection point: the plan-review snapshot capture deadline (PLAN_REVIEW_SNAPSHOT_CAPTURE_TIMEOUT_MS, 15 s); src/node/services/agentSession.planReviewSnapshot.test.ts uses it to witness that a stalled capture is abandoned at the deadline, cannot append afterwards, and stops delaying later turns; production uses the default" }, { "file": "src/node/services/branchSummary.ts", "symbol": "deriveSideChannelModelCandidates", "reason": "production caller: WorkspaceService.getWorkspaceNamingCandidates (workspaceService.ts)" }, { "file": "src/node/services/historyScanner.ts", "symbol": "classifyHistoryScanRow", "reason": "plain-row gate property test (#4655): historyScanner.plainRow.test.ts compares readPlainHistoryRow with the full classifier on generated rows; production callers: findProviderHistoryStart, hasUnreadableHistoryResetEvidence and the history row readers in the same file" }, diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 029031f43a5..e9c9f7d1c00 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -239,6 +239,7 @@ function AppInner() { const [isMultiProjectWorkspaceModalOpen, setMultiProjectWorkspaceModalOpen] = useState(false); const multiProjectWorkspacesEnabled = useExperimentValue(EXPERIMENT_IDS.MULTI_PROJECT_WORKSPACES); + const artifactsEnabled = useExperimentValue(EXPERIMENT_IDS.ARTIFACTS); const autoModelRoutingEnabled = useExperimentValue(EXPERIMENT_IDS.AUTO_MODEL_ROUTING); // Left sidebar is drag-resizable (mirrors RightSidebar). Width is persisted globally; @@ -1013,6 +1014,7 @@ function AppInner() { onStartWorkspaceCreation: openNewWorkspaceFromPalette, onStartMultiProjectWorkspaceCreation: openNewMultiProjectWorkspaceFromPalette, multiProjectWorkspacesEnabled, + artifactsEnabled, onArchiveMergedWorkspacesInProject: archiveMergedWorkspacesInProjectFromPalette, getBranchesForProject, onSelectWorkspace: selectWorkspaceFromPalette, diff --git a/src/browser/components/WorkspaceMenuBar/WorkspaceMenuBar.tsx b/src/browser/components/WorkspaceMenuBar/WorkspaceMenuBar.tsx index 81fd8f887da..915af7890f6 100644 --- a/src/browser/components/WorkspaceMenuBar/WorkspaceMenuBar.tsx +++ b/src/browser/components/WorkspaceMenuBar/WorkspaceMenuBar.tsx @@ -305,8 +305,9 @@ export const WorkspaceMenuBar: React.FC = ({ return () => window.removeEventListener("keydown", handler); }, [artifactsExperimentEnabled, isTimelineSidebarHidden, workspaceId]); - // "Open in Artifacts" on an MCP Apps tool card: while the sidebar is hidden, the dialog is - // the only place the view can show. + // "Open in Artifacts" on an MCP Apps tool card and openArtifact() (chat cards, file cards, + // Review, palette): while the sidebar is hidden, the dialog is the only place to show them + // (RightSidebar handles the same events on wider layouts). useEffect(() => { if (!artifactsExperimentEnabled) { return; @@ -319,7 +320,11 @@ export const WorkspaceMenuBar: React.FC = ({ setArtifactsDialogWorkspaceId(workspaceId); }; window.addEventListener(CUSTOM_EVENTS.OPEN_MCP_APP_VIEW, handler); - return () => window.removeEventListener(CUSTOM_EVENTS.OPEN_MCP_APP_VIEW, handler); + window.addEventListener(CUSTOM_EVENTS.OPEN_ARTIFACT, handler); + return () => { + window.removeEventListener(CUSTOM_EVENTS.OPEN_MCP_APP_VIEW, handler); + window.removeEventListener(CUSTOM_EVENTS.OPEN_ARTIFACT, handler); + }; }, [artifactsExperimentEnabled, isTimelineSidebarHidden, workspaceId]); const isDevcontainerWorkspace = isDevcontainerRuntime(runtimeConfig); diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactVersionMenu.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactVersionMenu.tsx new file mode 100644 index 00000000000..588a2697c80 --- /dev/null +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactVersionMenu.tsx @@ -0,0 +1,144 @@ +import React, { useEffect, useRef, useState } from "react"; +import { Check, ChevronDown } from "lucide-react"; +import { TooltipIfPresent } from "@/browser/components/Tooltip/Tooltip"; +import { stopKeyboardPropagation } from "@/browser/utils/events"; +import { formatRelativeTime } from "@/browser/utils/ui/dateTime"; +import { cn } from "@/common/lib/utils"; +import type { ArtifactVersion } from "@/common/orpc/schemas/artifacts"; + +/** Label for a stored version: its publish title, or a plain name for turn-end snapshots. */ +function versionLabel(version: ArtifactVersion): string { + if (version.label != null && version.label.length > 0) return version.label; + return version.source === "turn-end" ? "Turn snapshot" : "Untitled"; +} + +/** + * Toolbar version menu ("v"): "Latest (live)" follows the working file, every other entry + * shows a stored version. Rendered inline (not through a Radix portal) so it opens in + * happy-dom tests and inside the panel's fullscreen overlay alike. + */ +export function ArtifactVersionMenu(props: { + /** Newest first; the caller hides the menu when there are none. */ + versions: readonly ArtifactVersion[]; + /** null = Latest (live). */ + selectedVersion: number | null; + onSelect: (version: number | null) => void; +}) { + const [open, setOpen] = useState(false); + const containerRef = useRef(null); + const triggerRef = useRef(null); + const listRef = useRef(null); + + // Close when clicking anywhere outside the trigger and its list. + useEffect(() => { + if (!open) return; + const handleMouseDown = (event: MouseEvent) => { + if (containerRef.current?.contains(event.target as Node)) return; + setOpen(false); + }; + document.addEventListener("mousedown", handleMouseDown); + return () => document.removeEventListener("mousedown", handleMouseDown); + }, [open]); + + // Keyboard users land on the checked entry when the list opens. + useEffect(() => { + if (!open) return; + listRef.current?.querySelector('[aria-checked="true"]')?.focus(); + }, [open]); + + const choose = (version: number | null) => { + setOpen(false); + triggerRef.current?.focus(); + props.onSelect(version); + }; + + const handleListKeyDown = (event: React.KeyboardEvent) => { + if (event.key === "Escape") { + // Closing the menu must not also exit fullscreen or interrupt the stream. + event.preventDefault(); + stopKeyboardPropagation(event); + setOpen(false); + triggerRef.current?.focus(); + return; + } + if (event.key !== "ArrowDown" && event.key !== "ArrowUp") return; + event.preventDefault(); + const items = Array.from( + listRef.current?.querySelectorAll('[role="menuitemradio"]') ?? [] + ); + const index = items.findIndex((item) => item === document.activeElement); + const next = event.key === "ArrowDown" ? index + 1 : index - 1; + items[Math.min(Math.max(next, 0), items.length - 1)]?.focus(); + }; + + const triggerText = props.selectedVersion == null ? "Live" : `v${props.selectedVersion}`; + const itemClassName = + "hover:bg-hover focus:bg-hover flex w-full items-center gap-2 rounded-sm px-2 py-1.5 text-left text-xs outline-none"; + + return ( +
+ + + + {open && ( +
+ + {props.versions.map((version) => { + const checked = props.selectedVersion === version.version; + return ( + + ); + })} +
+ )} +
+ ); +} + +function MenuCheck(props: { checked: boolean }) { + return ( + + + + ); +} diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.test.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.test.tsx index 0fd7ab6e22e..43b69e6bb9c 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.test.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.test.tsx @@ -46,6 +46,14 @@ let capabilityRequests = 0; function Wrapper(props: { children: ReactNode }) { const api: TestApiOverrides = { artifacts: { + // No versions or pinned files: these tests cover the live renderers. + listVersions: (input: { workspaceId: string; path: string }) => + Promise.resolve({ + success: true as const, + data: { artifactId: input.path, path: input.path, pin: null, versions: [] }, + }), + listPinned: () => + Promise.resolve({ success: true as const, data: { available: true as const, files: [] } }), list: () => Promise.resolve({ success: true as const, diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx index 82e827d6d14..63892f7028b 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx @@ -91,15 +91,19 @@ export const AGENT_BROWSER_MISSING_WARNING = * none, say so above the frame (null means unknown, which shows nothing). */ function SandboxedArtifactWithCheckNotice( - props: React.ComponentProps + props: React.ComponentProps & { + /** The workspace whose runtime is probed; `workspaceId` may be null (no relative assets). */ + checkWorkspaceId: string; + } ) { - const agentBrowserAvailable = useAgentBrowserAvailable(props.workspaceId); + const { checkWorkspaceId, ...frameProps } = props; + const agentBrowserAvailable = useAgentBrowserAvailable(checkWorkspaceId); // One stable tree, so the answer arriving never remounts (reloads) the frame. return (
{agentBrowserAvailable === false && {AGENT_BROWSER_MISSING_WARNING}}
- +
); @@ -112,8 +116,14 @@ export function ArtifactViewer(props: { artifactsDir?: string | null; /** Escape / Shift+F pressed inside a sandboxed HTML/SVG frame. */ onFrameKey?: (key: ArtifactFrameKey) => void; + /** + * Resolve relative images/assets through the artifacts folder (default). Off for pinned + * checkout files, whose neighbours live in the checkout, not the artifacts folder. + */ + readRelativeAssets?: boolean; }) { const result = props.result; + const assetWorkspaceId = props.readRelativeAssets === false ? null : props.workspaceId; if (result.status === "too_large") { return ( ); case "json": @@ -159,7 +169,8 @@ export function ArtifactViewer(props: { } return ( = FILES, options: { allowCdn?: boolean } = {} ) { - writeArtifactSelection(WORKSPACE_ID, { path: selectedPath }); + writeArtifactSelection(WORKSPACE_ID, { scope: "artifact", path: selectedPath, version: null }); updatePersistedState(ARTIFACTS_ALLOW_CDN_SCRIPTS_KEY, options.allowCdn ?? true); return ( waitForGallery(canvasElement), }; +/** Toolbar version menu, open: "Latest (live)" plus stored versions, newest first. */ +function renderVersionMenu() { + const workspaceId = `${WORKSPACE_ID}-versions`; + writeArtifactSelection(workspaceId, { scope: "artifact", path: "report.md", version: null }); + const now = Date.now(); + const at = (minutesAgo: number) => now - minutesAgo * 60_000; + const base = { sha256: "sha", size: MARKDOWN.length, path: "report.md" } as const; + return ( + +
+
+ +
+
+ + ); +} + +export const VersionMenuOpen: Story = { + parameters: { pixel: { matrix: { themes: ["dark", "light"], viewports: ["laptop"] } } }, + render: renderVersionMenu, + play: async ({ canvasElement }) => { + await waitForMarkdown(canvasElement); + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByRole("button", { name: "Version: Latest (live)" })); + const menu = await canvas.findByRole("menu", { name: "Artifact versions" }); + await within(menu).findByText("Turn snapshot"); + }, +}; + // --------------------------------------------------------------------------------------------- // Escape attempts (executed in a real browser by the Storybook test runner). // diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.test.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.test.tsx index 9796dae6f2a..ce3d3ff60fe 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.test.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.test.tsx @@ -1,9 +1,9 @@ // Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). import "../../../../../tests/ui/dom"; -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { createTestApiClient, type TestApiOverrides } from "@/browser/testUtils"; -import { cleanup, fireEvent, render, waitFor } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react"; import type { ReactNode } from "react"; import { installDom } from "../../../../../tests/ui/dom"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; @@ -12,6 +12,8 @@ import type { ArtifactEntry, ArtifactListing, ArtifactReadResult, + ArtifactVersion, + PinnedArtifactFile, } from "@/common/orpc/schemas/artifacts"; import { ARTIFACTS_SELECTION_KEY, @@ -20,21 +22,101 @@ import { import { ArtifactsPanel } from "./ArtifactsPanel"; import { readArtifactSelection, writeArtifactSelection } from "./artifactSelection"; import { closeMcpAppView, openMcpAppView } from "./mcpAppViewsStore"; +import { openArtifact } from "./openArtifact"; function entry(path: string, modifiedMs: number, kind: ArtifactEntry["kind"]): ArtifactEntry { return { path, kind, size: 10, modifiedMs }; } +function version(n: number, label: string | null, path: string): ArtifactVersion { + return { + version: n, + label, + source: label == null ? "turn-end" : "publish", + createdAtMs: Date.now() - n * 60_000, + sha256: `sha-${n}`, + size: 10, + path, + }; +} + +/** Test stand-in for the backend's path-derived artifact id. */ +const idFor = (path: string) => `id-${path}`; + function createFakeArtifactsApi( listing: ArtifactListing, - files: Record + files: Record, + extra: { + /** Stored versions per artifact path, newest first. */ + versions?: Record; + /** Version contents keyed by `${artifactId}@${version}`. */ + versionFiles?: Record; + pinned?: PinnedArtifactFile[]; + pinnedFiles?: Record; + /** Makes `list` fail with this error. */ + listError?: string; + /** Makes `listVersions` fail with this error. */ + listVersionsError?: string; + } = {} ) { - const state = { listing, files, listCalls: 0, readCalls: [] as string[] }; + const state = { + listing, + files, + pinned: extra.pinned ?? [], + listCalls: 0, + readCalls: [] as string[], + readVersionCalls: [] as string[], + readPinnedCalls: [] as string[], + unpinCalls: [] as string[], + }; + const found = (file: ArtifactReadResult | undefined, label: string) => + Promise.resolve( + file + ? { success: true as const, data: file } + : { success: false as const, error: `Not found: ${label}` } + ); const api: TestApiOverrides = { artifacts: { + listVersions: (input: { workspaceId: string; path: string }) => + Promise.resolve( + extra.listVersionsError != null + ? { success: false as const, error: extra.listVersionsError } + : { + success: true as const, + data: { + artifactId: idFor(input.path), + path: input.path, + pin: null, + versions: extra.versions?.[input.path] ?? [], + }, + } + ), + readVersion: (input: { workspaceId: string; artifactId: string; version: number }) => { + const key = `${input.artifactId}@${input.version}`; + state.readVersionCalls.push(key); + return found(extra.versionFiles?.[key], key); + }, + listPinned: () => + Promise.resolve({ + success: true as const, + data: { available: true as const, files: state.pinned }, + }), + readPinned: (input: { workspaceId: string; path: string }) => { + state.readPinnedCalls.push(input.path); + return found(extra.pinnedFiles?.[input.path], input.path); + }, + unpinFile: (input: { workspaceId: string; path: string }) => { + state.unpinCalls.push(input.path); + state.pinned = state.pinned.filter((file) => file.path !== input.path); + return Promise.resolve({ success: true as const, data: undefined }); + }, list: () => { state.listCalls += 1; - return Promise.resolve({ success: true as const, data: state.listing }); + return Promise.resolve( + extra.listError != null + ? { success: false as const, error: extra.listError } + : { success: true as const, data: state.listing } + ); }, read: (input: { workspaceId: string; path: string }) => { state.readCalls.push(input.path); @@ -145,7 +227,7 @@ describe("ArtifactsPanel", () => { expect(view.getByTestId("artifacts-panel").textContent).toContain(content); }); - test("J/K/R do nothing while the picker list is open", async () => { + test("J/K/R do nothing while the picker list or the version menu is open", async () => { fake = createFakeArtifactsApi( { available: true, @@ -158,18 +240,21 @@ describe("ArtifactsPanel", () => { const view = renderPanel(); const panel = view.getByTestId("artifacts-panel"); expect(await view.findByText("alpha")).toBeTruthy(); - // Stand-in for the open Radix listbox, whose key events bubble to the panel. - const listbox = document.createElement("div"); - listbox.setAttribute("role", "listbox"); - const option = document.createElement("div"); - listbox.appendChild(option); - panel.appendChild(listbox); - const listsBefore = fake.state.listCalls; - fireEvent.keyDown(option, { key: "j" }); - fireEvent.keyDown(option, { key: "r" }); - expect(view.getByRole("combobox", { name: "Artifact" }).textContent).toContain("a.txt"); - expect(fake.state.listCalls).toBe(listsBefore); - listbox.remove(); + // Stand-ins for the open Radix listbox and the version menu, whose key events bubble to the + // panel. + for (const role of ["listbox", "menu"]) { + const popup = document.createElement("div"); + popup.setAttribute("role", role); + const item = document.createElement("div"); + popup.appendChild(item); + panel.appendChild(popup); + const listsBefore = fake.state.listCalls; + fireEvent.keyDown(item, { key: "j" }); + fireEvent.keyDown(item, { key: "r" }); + expect(view.getByRole("combobox", { name: "Artifact" }).textContent).toContain("a.txt"); + expect(fake.state.listCalls).toBe(listsBefore); + popup.remove(); + } }); test("keeps the selection of only the most recently used workspaces", () => { @@ -227,6 +312,71 @@ describe("ArtifactsPanel", () => { await waitFor(() => expect(document.activeElement).toBe(panel)); }); + test("retries a failed preview on the next successful poll", async () => { + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("a.txt", 1, "text")], + truncated: false, + }, + {} + ); + const setIntervalSpy = spyOn(window, "setInterval"); + try { + const view = renderPanel(); + // The first read fails (e.g. the runtime was briefly unreachable). + expect(await view.findByText("Artifact not found: a.txt")).toBeTruthy(); + const poll = setIntervalSpy.mock.calls.find((call) => call[1] === 3000)?.[0]; + if (typeof poll !== "function") throw new Error("Test bug: no 3 s poll registered"); + + fake.state.files = { "a.txt": textFile("a.txt", "text", "alpha") }; + act(() => poll()); + expect(await view.findByText("alpha")).toBeTruthy(); + expect(fake.state.readCalls).toEqual(["a.txt", "a.txt"]); + + // A healthy preview is not re-read by later polls. + act(() => poll()); + await waitFor(() => expect(fake?.state.listCalls).toBeGreaterThanOrEqual(3)); + expect(fake.state.readCalls).toEqual(["a.txt", "a.txt"]); + } finally { + setIntervalSpy.mockRestore(); + } + }); + + test("re-reads a file rewritten with the same mtime but a new size", async () => { + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("a.txt", 1, "text")], + truncated: false, + }, + { "a.txt": textFile("a.txt", "text", "alpha") } + ); + const setIntervalSpy = spyOn(window, "setInterval"); + try { + const view = renderPanel(); + expect(await view.findByText("alpha")).toBeTruthy(); + const poll = setIntervalSpy.mock.calls.find((call) => call[1] === 3000)?.[0]; + if (typeof poll !== "function") throw new Error("Test bug: no 3 s poll registered"); + + // `cp -p`-style rewrite: same mtime, different size. + fake.state.listing = { + available: true, + dir: "/scratch/artifacts", + entries: [{ ...entry("a.txt", 1, "text"), size: 42 }], + truncated: false, + }; + fake.state.files = { "a.txt": textFile("a.txt", "text", "alpha, longer now") }; + act(() => poll()); + expect(await view.findByText("alpha, longer now")).toBeTruthy(); + expect(fake.state.readCalls).toEqual(["a.txt", "a.txt"]); + } finally { + setIntervalSpy.mockRestore(); + } + }); + test("marks artifacts that changed while the tab was open", async () => { fake = createFakeArtifactsApi( { @@ -465,4 +615,243 @@ describe("ArtifactsPanel", () => { closeMcpAppView("ws-app-close", "call-b"); } }); + + test("version menu switches between stored versions and the live file", async () => { + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("report.md", 3, "markdown")], + truncated: false, + }, + { "report.md": textFile("report.md", "markdown", "live draft", 3) }, + { + versions: { + "report.md": [version(2, "Final numbers", "report.md"), version(1, null, "report.md")], + }, + versionFiles: { + [`${idFor("report.md")}@1`]: textFile("report.md", "markdown", "first snapshot"), + }, + } + ); + const view = renderPanel(); + expect(await view.findByText("live draft")).toBeTruthy(); + + fireEvent.click(await view.findByRole("button", { name: "Version: Latest (live)" })); + const menu = view.getByRole("menu", { name: "Artifact versions" }); + // Newest first after "Latest (live)"; unlabeled snapshots get a plain fallback name. + const items = Array.from(menu.querySelectorAll('[role="menuitemradio"]')).map( + (item) => item.textContent + ); + expect(items[0]).toBe("Latest (live)"); + expect(items[1]).toContain("v2Final numbers"); + expect(items[2]).toContain("v1Turn snapshot"); + + fireEvent.click(view.getByRole("menuitemradio", { name: /v1/ })); + expect(await view.findByText("first snapshot")).toBeTruthy(); + expect(fake.state.readVersionCalls).toEqual([`${idFor("report.md")}@1`]); + expect(view.queryByRole("menu")).toBeNull(); + + const liveReads = fake.state.readCalls.length; + fireEvent.click(view.getByRole("button", { name: "Version: v1" })); + fireEvent.click(view.getByRole("menuitemradio", { name: "Latest (live)" })); + expect(await view.findByText("live draft")).toBeTruthy(); + expect(fake.state.readCalls.length).toBe(liveReads + 1); + }); + + test("hides the version menu for artifacts without versions", async () => { + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("notes.txt", 1, "text")], + truncated: false, + }, + { "notes.txt": textFile("notes.txt", "text", "plain notes") } + ); + const view = renderPanel(); + expect(await view.findByText("plain notes")).toBeTruthy(); + expect(view.queryByRole("button", { name: /^Version:/ })).toBeNull(); + }); + + test("keeps a stored version viewable after its working file is deleted", async () => { + fake = createFakeArtifactsApi( + { available: true, dir: "/scratch/artifacts", entries: [], truncated: false }, + {}, + { + versions: { "gone.md": [version(1, "Draft", "gone.md")] }, + versionFiles: { [`${idFor("gone.md")}@1`]: textFile("gone.md", "markdown", "kept bytes") }, + } + ); + writeArtifactSelection("ws-artifacts", { path: "gone.md", version: 1 }); + const view = renderPanel(); + expect(await view.findByText("kept bytes")).toBeTruthy(); + expect(view.getByRole("combobox", { name: "Artifact" }).textContent).toContain("gone.md"); + }); + + test("keeps a deleted artifact selected at its latest stored version", async () => { + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [], + truncated: false, + versionedPaths: ["gone.md"], + }, + {}, + { + versions: { "gone.md": [version(2, "Final", "gone.md"), version(1, "Draft", "gone.md")] }, + versionFiles: { + [`${idFor("gone.md")}@2`]: textFile("gone.md", "markdown", "latest kept bytes"), + }, + } + ); + writeArtifactSelection("ws-artifacts", { path: "gone.md", version: null }); + const view = renderPanel(); + expect(await view.findByText("latest kept bytes")).toBeTruthy(); + expect(fake.state.readVersionCalls).toEqual([`${idFor("gone.md")}@2`]); + expect(view.getByRole("combobox", { name: "Artifact" }).textContent).toContain("gone.md"); + }); + + test("shows why a stored version cannot load when listing versions fails", async () => { + fake = createFakeArtifactsApi( + { available: true, dir: "/scratch/artifacts", entries: [], truncated: false }, + {}, + { listVersionsError: "Version index unreadable" } + ); + writeArtifactSelection("ws-artifacts", { path: "gone.md", version: 1 }); + const view = renderPanel(); + expect(await view.findByText("Version index unreadable")).toBeTruthy(); + expect(view.queryByText("Loading…")).toBeNull(); + }); + + test("shows a stored version while listing the live folder fails", async () => { + fake = createFakeArtifactsApi( + { available: true, dir: "/scratch/artifacts", entries: [], truncated: false }, + {}, + { + listError: "Could not reach this workspace's runtime: offline", + versions: { "report.md": [version(1, "Draft", "report.md")] }, + versionFiles: { + [`${idFor("report.md")}@1`]: textFile("report.md", "markdown", "offline bytes"), + }, + } + ); + writeArtifactSelection("ws-artifacts", { path: "report.md", version: 1 }); + const view = renderPanel(); + expect(await view.findByText("offline bytes")).toBeTruthy(); + expect(view.getByText("Could not reach this workspace's runtime: offline")).toBeTruthy(); + }); + + test("follows openArtifact while mounted and reads pinned files live", async () => { + const pinnedFile: PinnedArtifactFile = { + path: "README.md", + kind: "markdown", + size: 5, + modifiedMs: 1, + }; + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("a.txt", 1, "text")], + truncated: false, + }, + { "a.txt": textFile("a.txt", "text", "alpha") }, + { + pinned: [pinnedFile], + pinnedFiles: { "README.md": textFile("README.md", "markdown", "# Readme") }, + } + ); + const view = renderPanel(); + expect(await view.findByText("alpha")).toBeTruthy(); + + act(() => openArtifact({ workspaceId: "ws-artifacts", path: "README.md", pinned: true })); + expect(await view.findByRole("heading", { name: "Readme" })).toBeTruthy(); + expect(fake.state.readPinnedCalls).toEqual(["README.md"]); + expect(view.getByRole("combobox", { name: "Artifact" }).textContent).toContain("README.md"); + + // A new mtime from the pinned listing re-reads the live file. + fake.state.pinned = [{ ...pinnedFile, modifiedMs: 2 }]; + fireEvent.click(view.getByRole("button", { name: "Reload artifact" })); + await waitFor(() => expect(fake?.state.readPinnedCalls.length).toBeGreaterThanOrEqual(2)); + + fireEvent.click(view.getByRole("button", { name: "Unpin file" })); + expect(await view.findByText("alpha")).toBeTruthy(); + expect(fake.state.unpinCalls).toEqual(["README.md"]); + expect(view.queryByRole("button", { name: "Unpin file" })).toBeNull(); + }); + + test("a slow unpin does not take away a selection made meanwhile", async () => { + const pinnedFile: PinnedArtifactFile = { + path: "README.md", + kind: "markdown", + size: 5, + modifiedMs: 1, + }; + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("a.txt", 2, "text"), entry("b.txt", 1, "text")], + truncated: false, + }, + { "a.txt": textFile("a.txt", "text", "alpha"), "b.txt": textFile("b.txt", "text", "beta") }, + { + pinned: [pinnedFile], + pinnedFiles: { "README.md": textFile("README.md", "markdown", "# Readme") }, + } + ); + const unpinned = Promise.withResolvers<{ success: true; data: undefined }>(); + fake.api.artifacts!.unpinFile = () => unpinned.promise; + const view = renderPanel(); + expect(await view.findByText("alpha")).toBeTruthy(); + act(() => openArtifact({ workspaceId: "ws-artifacts", path: "README.md", pinned: true })); + expect(await view.findByRole("heading", { name: "Readme" })).toBeTruthy(); + + fireEvent.click(view.getByRole("button", { name: "Unpin file" })); + // The user picks another artifact before the unpin completes. + act(() => openArtifact({ workspaceId: "ws-artifacts", path: "b.txt" })); + expect(await view.findByText("beta")).toBeTruthy(); + await act(async () => { + unpinned.resolve({ success: true, data: undefined }); + await unpinned.promise; + }); + expect(view.getByRole("combobox", { name: "Artifact" }).textContent).toContain("b.txt"); + expect(view.queryByText("alpha")).toBeNull(); + }); + + test("U unpins the selected pinned file and nothing else", async () => { + const pinnedFile: PinnedArtifactFile = { + path: "README.md", + kind: "markdown", + size: 5, + modifiedMs: 1, + }; + fake = createFakeArtifactsApi( + { + available: true, + dir: "/scratch/artifacts", + entries: [entry("a.txt", 1, "text")], + truncated: false, + }, + { "a.txt": textFile("a.txt", "text", "alpha") }, + { + pinned: [pinnedFile], + pinnedFiles: { "README.md": textFile("README.md", "markdown", "# Readme") }, + } + ); + const view = renderPanel(); + const panel = view.getByTestId("artifacts-panel"); + expect(await view.findByText("alpha")).toBeTruthy(); + // An artifact is selected: U has nothing to unpin. + fireEvent.keyDown(panel, { key: "u" }); + expect(fake.state.unpinCalls).toEqual([]); + + act(() => openArtifact({ workspaceId: "ws-artifacts", path: "README.md", pinned: true })); + expect(await view.findByRole("heading", { name: "Readme" })).toBeTruthy(); + fireEvent.keyDown(panel, { key: "u" }); + await waitFor(() => expect(fake?.state.unpinCalls).toEqual(["README.md"])); + expect(await view.findByText("alpha")).toBeTruthy(); + }); }); diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx index 22b082b5b94..ff4afc99367 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useRef, useState } from "react"; -import { Download, Maximize2, Minimize2, RefreshCw } from "lucide-react"; +import { Download, Maximize2, Minimize2, PinOff, RefreshCw } from "lucide-react"; import { Dialog, DialogContent, @@ -19,30 +19,104 @@ import { TooltipIfPresent } from "@/browser/components/Tooltip/Tooltip"; import { useAPI } from "@/browser/contexts/API"; import { workspaceStore } from "@/browser/stores/WorkspaceStore"; import { isAbortError } from "@/browser/utils/isAbortError"; -import { isEditableElement, KEYBINDS, matchesKeybind } from "@/browser/utils/ui/keybinds"; +import { + formatKeybind, + isEditableElement, + KEYBINDS, + matchesKeybind, +} from "@/browser/utils/ui/keybinds"; import type { ArtifactEntry, ArtifactListing, ArtifactReadResult, + ArtifactVersionList, + PinnedArtifactFile, + PinnedArtifactFiles, } from "@/common/orpc/schemas/artifacts"; import { getErrorMessage } from "@/common/utils/errors"; -import { useArtifactSelection, writeArtifactSelection } from "./artifactSelection"; import { downloadArtifact } from "./artifactDownload"; +import { ArtifactVersionMenu } from "./ArtifactVersionMenu"; import { ArtifactViewer } from "./ArtifactViewer"; import { McpAppFrame } from "./McpAppFrame"; import { mcpAppSelectionKey, useMcpAppViews } from "./mcpAppViewsStore"; +import { + type ArtifactSelection, + type ArtifactSelectionScope, + readArtifactSelection, + useArtifactSelection, + writeArtifactSelection, +} from "./artifactSelection"; import type { ArtifactFrameKey } from "./SandboxedArtifactFrame"; /** While the tab is visible, re-list this often to catch writes no tool event reports. */ const ARTIFACTS_POLL_MS = 3000; interface ReadState { - path: string; - modifiedMs: number; + /** Identifies what was read (see Selection/readKey), so stale reads are never shown. */ + key: string; result: ArtifactReadResult | null; error: string | null; } +/** + * What the toolbar points at. An artifact with a stored version selected stays selectable after + * its working file is gone (`entry` null), so old versions remain viewable. + */ +type Selection = + | { scope: "pinned"; path: string; file: PinnedArtifactFile } + | { scope: "artifact"; path: string; entry: ArtifactEntry | null; version: number | null }; + +/** Picker values carry the scope, since a pinned file and an artifact may share a path. */ +function pickerValue(scope: ArtifactSelectionScope, path: string): string { + return `${scope}:${path}`; +} + +function parsePickerValue(value: string): { scope: ArtifactSelectionScope; path: string } | null { + const colon = value.indexOf(":"); + const scope = value.slice(0, colon); + if (scope !== "artifact" && scope !== "pinned") return null; + return { scope, path: value.slice(colon + 1) }; +} + +/** + * The persisted selection when it still points at something, else the first artifact, else + * the first pinned file, else the first deleted artifact that still has stored versions. + * `versionOnlyPaths` are artifacts whose working file is gone but whose versions are kept; with + * no version selected they show their latest stored version. + */ +export function resolveSelection(input: { + scope: ArtifactSelectionScope; + path: string | null; + version: number | null; + entries: readonly ArtifactEntry[]; + pinnedFiles: readonly PinnedArtifactFile[]; + versionOnlyPaths: readonly string[]; +}): Selection | null { + if (input.scope === "pinned") { + const file = input.pinnedFiles.find((f) => f.path === input.path); + if (file) return { scope: "pinned", path: file.path, file }; + } else { + const entry = input.entries.find((e) => e.path === input.path); + if (entry) return { scope: "artifact", path: entry.path, entry, version: input.version }; + if ( + input.path != null && + (input.version != null || input.versionOnlyPaths.includes(input.path)) + ) { + return { scope: "artifact", path: input.path, entry: null, version: input.version }; + } + } + const firstEntry = input.entries[0]; + if (firstEntry) + return { scope: "artifact", path: firstEntry.path, entry: firstEntry, version: null }; + const firstPinned = input.pinnedFiles[0]; + if (firstPinned) return { scope: "pinned", path: firstPinned.path, file: firstPinned }; + const firstVersionOnly = input.versionOnlyPaths[0]; + if (firstVersionOnly != null) { + return { scope: "artifact", path: firstVersionOnly, entry: null, version: null }; + } + return null; +} + /** * Artifacts tab (experiment: "artifacts"): files the agent writes to * $XUM_SCRATCH_DIR/artifacts, listed newest first with a preview of the selected one. @@ -67,13 +141,33 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean const panelRef = useRef(null); // Set while a list request runs, so a slow walk is not aborted by the next poll tick. const listInFlightRef = useRef(false); - // MCP Apps: "Open in Artifacts" on a tool card selects its view through the selection map. - const { path: selectedPath } = useArtifactSelection(props.workspaceId); - const setSelectedPath = (path: string) => writeArtifactSelection(props.workspaceId, { path }); + const [pinned, setPinned] = useState(null); + const [versionsState, setVersionsState] = useState<{ + path: string; + list: ArtifactVersionList | null; + /** Why listVersions failed; shown instead of a stored version that cannot be resolved. */ + error: string | null; + } | null>(null); + const [actionError, setActionError] = useState(null); + // openArtifact() (chat cards, file cards, palette) writes the selection before asking for the + // tab; the listener keeps a mounted panel in sync with those writes. + // MCP Apps: "Open in Artifacts" on a tool card selects its view through the path. + const { + path: selectedPath, + version: selectedVersion, + scope: selectedScope, + } = useArtifactSelection(props.workspaceId); + const setSelection = (next: Partial) => + writeArtifactSelection(props.workspaceId, next); const appViews = useMcpAppViews(props.workspaceId); const selectedApp = appViews.find((view) => mcpAppSelectionKey(view.toolCallId) === selectedPath) ?? null; + const select = (next: { scope: ArtifactSelectionScope; path: string | null }) => { + setActionError(null); + setSelection({ scope: next.scope, path: next.path, version: null }); + }; + useEffect(() => { if (!api) return; const controller = new AbortController(); @@ -91,15 +185,12 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean } setListError(null); setListing(result.data); - // One retry per failure: cleared here so later polls do not abort a slow retry. if (readFailedRef.current) { readFailedRef.current = false; setReloadTick((tick) => tick + 1); } const entries = result.data.available ? result.data.entries : []; setSeen((prev) => prev ?? new Map(entries.map((e) => [e.path, e.modifiedMs]))); - // Nothing left to show: close fullscreen so it cannot pop back when a file reappears. - if (entries.length === 0) setFullscreen(false); }) .catch((error: unknown) => { if (isAbortError(error) || controller.signal.aborted) return; @@ -111,6 +202,24 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean }; }, [api, props.workspaceId, refreshTick]); + // Pinned checkout files are live: re-list them on the same refresh signal as the artifacts, + // and their mtimes drive re-reads like an artifact's. + useEffect(() => { + if (!api) return; + const controller = new AbortController(); + api.artifacts + .listPinned({ workspaceId: props.workspaceId }, { signal: controller.signal }) + .then((result) => { + if (controller.signal.aborted) return; + setPinned(result.success ? result.data : { available: false, reason: result.error }); + }) + .catch((error: unknown) => { + if (isAbortError(error) || controller.signal.aborted) return; + setPinned({ available: false, reason: getErrorMessage(error) }); + }); + return () => controller.abort(); + }, [api, props.workspaceId, refreshTick]); + // Re-list after the agent's file edits and bash commands, the usual ways it writes files. useEffect(() => { return workspaceStore.subscribeFileModifyingTool((wsId) => { @@ -129,50 +238,193 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean }, []); const entries: ArtifactEntry[] = listing?.available === true ? listing.entries : []; + const pinnedFiles: PinnedArtifactFile[] = pinned?.available === true ? pinned.files : []; + // Deleted working files whose versions are kept stay listed and selectable. + const versionOnlyPaths: string[] = + listing?.available === true + ? (listing.versionedPaths ?? []).filter((path) => !entries.some((e) => e.path === path)) + : []; + // A pinned selection waits for the pinned list instead of flashing the first artifact. + const waitingForPinned = selectedScope === "pinned" && selectedPath != null && pinned == null; + const selected = - selectedApp != null + selectedApp != null || waitingForPinned ? null - : (entries.find((entry) => entry.path === selectedPath) ?? entries[0] ?? null); - // Size too: a same-mtime rewrite (cp -p, 1 s filesystems) must still re-read. - const selectedKey = selected - ? `${selected.path}\u0000${selected.modifiedMs}\u0000${selected.size}` - : null; + : resolveSelection({ + scope: selectedScope, + path: selectedPath, + version: selectedVersion, + entries, + pinnedFiles, + versionOnlyPaths, + }); + const selectedArtifactPath = selected?.scope === "artifact" ? selected.path : null; + const selectedVersionForFetch = selected?.scope === "artifact" ? selected.version : null; + // Versions of the selected artifact, refetched on every refresh: a publish or turn-end + // snapshot adds a version without changing the working file. useEffect(() => { - if (!api || selected == null) return; - const { path, modifiedMs } = selected; - readFailedRef.current = false; + if (!api || selectedArtifactPath == null) return; + const path = selectedArtifactPath; const controller = new AbortController(); api.artifacts - .read({ workspaceId: props.workspaceId, path }, { signal: controller.signal }) + .listVersions({ workspaceId: props.workspaceId, path }, { signal: controller.signal }) + .then((result) => { + if (controller.signal.aborted) return; + setVersionsState( + result.success + ? { path, list: result.data, error: null } + : { path, list: null, error: result.error } + ); + }) + .catch((error: unknown) => { + if (isAbortError(error) || controller.signal.aborted) return; + setVersionsState({ path, list: null, error: getErrorMessage(error) }); + }); + return () => controller.abort(); + }, [api, props.workspaceId, selectedArtifactPath, selectedVersionForFetch, refreshTick]); + + const versionList = + selected?.scope === "artifact" && versionsState?.path === selected.path + ? versionsState.list + : null; + const versionListError = + selected?.scope === "artifact" && versionsState?.path === selected.path + ? versionsState.error + : null; + // A deleted working file with no version chosen shows its latest stored version. + const versionToRead = + selected?.scope === "artifact" + ? (selected.version ?? + (selected.entry == null ? (versionList?.versions[0]?.version ?? null) : null)) + : null; + + // What to read and how. Stored versions need the artifact id, which listVersions reports. + let readRequest: + | { key: string; kind: "live"; path: string; modifiedMs: number } + | { key: string; kind: "pinned"; path: string } + | { key: string; kind: "version"; artifactId: string; version: number } + | null = null; + if (selected?.scope === "pinned") { + readRequest = { + key: `pinned\u0000${selected.path}\u0000${selected.file.modifiedMs ?? "missing"}`, + kind: "pinned", + path: selected.path, + }; + } else if ( + selected?.scope === "artifact" && + (selected.version != null || selected.entry == null) + ) { + if (versionList != null && versionToRead != null) { + readRequest = { + key: `version\u0000${versionList.artifactId}\u0000${versionToRead}`, + kind: "version", + artifactId: versionList.artifactId, + version: versionToRead, + }; + } + } else if (selected?.scope === "artifact" && selected.entry != null) { + readRequest = { + // Size too: a same-mtime rewrite (`cp -p`, 1 s mtime resolution) still changes the key. + key: `live\u0000${selected.path}\u0000${selected.entry.modifiedMs}\u0000${selected.entry.size}`, + kind: "live", + path: selected.path, + modifiedMs: selected.entry.modifiedMs, + }; + } + const readKey = readRequest?.key ?? null; + + useEffect(() => { + readFailedRef.current = false; + if (!api || readRequest == null) return; + const request = readRequest; + const controller = new AbortController(); + const options = { signal: controller.signal }; + const workspaceId = props.workspaceId; + const read = + request.kind === "live" + ? api.artifacts.read({ workspaceId, path: request.path }, options) + : request.kind === "pinned" + ? api.artifacts.readPinned({ workspaceId, path: request.path }, options) + : api.artifacts.readVersion( + { workspaceId, artifactId: request.artifactId, version: request.version }, + options + ); + read .then((result) => { if (controller.signal.aborted) return; - readFailedRef.current = !result.success; if (result.success) { - setReadState({ path, modifiedMs, result: result.data, error: null }); - setSeen((prev) => new Map(prev ?? []).set(path, modifiedMs)); + setReadState({ key: request.key, result: result.data, error: null }); + if (request.kind === "live") { + setSeen((prev) => new Map(prev ?? []).set(request.path, request.modifiedMs)); + } } else { - setReadState({ path, modifiedMs, result: null, error: result.error }); + readFailedRef.current = true; + setReadState({ key: request.key, result: null, error: result.error }); } }) .catch((error: unknown) => { if (isAbortError(error) || controller.signal.aborted) return; readFailedRef.current = true; - setReadState({ path, modifiedMs, result: null, error: getErrorMessage(error) }); + setReadState({ key: request.key, result: null, error: getErrorMessage(error) }); }); return () => controller.abort(); - // selectedKey covers path + modifiedMs + size: re-read when the selected file changes on disk. + // readKey identifies the request (scope, path, mtime or version): re-read when it changes. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [api, props.workspaceId, selectedKey, reloadTick]); + }, [api, props.workspaceId, readKey, reloadTick]); - // Fullscreen only makes sense with something selected (the list callback also clears it). + // Nothing left to show: close fullscreen so it cannot pop back when a file reappears. + if (fullscreen && selected == null && listing != null && !waitingForPinned) { + setFullscreen(false); + } + // Fullscreen only makes sense with something selected. const showFullscreen = allowFullscreen && fullscreen && selected != null; + // Picker order: pinned files first, then artifacts, then deleted artifacts with stored + // versions. A selected version whose working file is gone keeps its own entry so the picker + // can still name it. + const deletedPaths = [...versionOnlyPaths]; + if ( + selected?.scope === "artifact" && + selected.entry == null && + !deletedPaths.includes(selected.path) + ) { + deletedPaths.push(selected.path); + } + const options: Array<{ scope: ArtifactSelectionScope; path: string }> = [ + ...pinnedFiles.map((file) => ({ scope: "pinned" as const, path: file.path })), + ...entries.map((entry) => ({ scope: "artifact" as const, path: entry.path })), + ...deletedPaths.map((path) => ({ scope: "artifact" as const, path })), + ]; + const selectRelative = (offset: number) => { - if (entries.length === 0) return; - const index = selected ? entries.indexOf(selected) : -1; - const next = entries[Math.min(Math.max(index + offset, 0), entries.length - 1)]; - if (next) setSelectedPath(next.path); + if (options.length === 0) return; + const index = selected + ? options.findIndex((o) => o.scope === selected.scope && o.path === selected.path) + : -1; + const next = options[Math.min(Math.max(index + offset, 0), options.length - 1)]; + if (next) select(next); + }; + + const unpinSelected = () => { + if (!api || selected?.scope !== "pinned") return; + const path = selected.path; + api.artifacts + .unpinFile({ workspaceId: props.workspaceId, path }) + .then((result) => { + if (!result.success) { + setActionError(result.error); + return; + } + // Leave the unpinned file only if it is still selected: the user may have moved on + // while the request ran. + const current = readArtifactSelection(props.workspaceId); + if (current.scope === "pinned" && current.path === path) { + select({ scope: "artifact", path: null }); + } + setRefreshTick((tick) => tick + 1); + }) + .catch((error: unknown) => setActionError(getErrorMessage(error))); }; const reload = () => { @@ -184,12 +436,11 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean // overlay, whose events bubble here through the portal). const handleKeyDown = (e: React.KeyboardEvent) => { if (isEditableElement(e.target)) return; - // The open picker owns its keys (type-ahead, arrows); J/K/R must not change the selection - // behind it. - // The closed trigger too: Radix Select type-ahead acts on printable keys there. + // The picker (open list, or its focused closed trigger, where Radix runs type-ahead) and the + // version menu own their keys; J/K/R must not change the selection behind them. if ( e.target instanceof Element && - e.target.closest('[role="listbox"], [role="combobox"]') != null + e.target.closest('[role="listbox"],[role="menu"],[role="combobox"]') != null ) { return; } @@ -205,6 +456,9 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean } else if (matchesKeybind(e, KEYBINDS.RELOAD_ARTIFACT)) { e.preventDefault(); reload(); + } else if (matchesKeybind(e, KEYBINDS.UNPIN_ARTIFACT_FILE) && selected?.scope === "pinned") { + e.preventDefault(); + unpinSelected(); } }; @@ -218,16 +472,16 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean if (showFullscreen) setFullscreen(false); }; - const selectedResult = - selected != null && readState?.path === selected.path ? readState.result : null; + const currentRead = readKey != null && readState?.key === readKey ? readState : null; + const currentResult = currentRead?.result ?? null; // Text arrives decoded as UTF-8 with U+FFFD for invalid bytes (e.g. Windows-1252 files). // Downloading would re-encode that string and save different bytes than the file on disk, // so such files are not downloadable here; the original stays in the artifacts folder. const lossyText = - selectedResult?.status === "ok" && - selectedResult.encoding === "utf8" && - selectedResult.content.includes("\uFFFD"); - const downloadableResult = selectedResult?.status === "ok" && !lossyText ? selectedResult : null; + currentResult?.status === "ok" && + currentResult.encoding === "utf8" && + currentResult.content.includes("\uFFFD"); + const downloadableResult = currentResult?.status === "ok" && !lossyText ? currentResult : null; const viewerBody = selectedApp != null ? ( @@ -237,17 +491,23 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean workspaceId={props.workspaceId} view={selectedApp} /> - ) : selected == null ? null : readState?.path === selected.path && readState.result ? ( + ) : selected == null && !waitingForPinned ? null : currentRead?.result ? ( - ) : readState?.path === selected.path && readState.error ? ( -
{readState.error}
+ ) : currentRead?.error ? ( +
{currentRead.error}
+ ) : readRequest == null && versionListError != null ? ( + // A stored version is resolved through listVersions: without it there is nothing to read. +
{versionListError}
) : (
Loading…
); @@ -256,7 +516,7 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean entries .filter( (entry) => - entry.path !== selected?.path && + !(selected?.scope === "artifact" && entry.path === selected.path) && seen != null && (seen.get(entry.path) ?? -1) < entry.modifiedMs ) @@ -266,13 +526,54 @@ export function ArtifactsPanel(props: { workspaceId: string; inDialog?: boolean const toolbarButtonClassName = "border-border-light text-muted hover:text-foreground bg-background flex h-6 w-6 items-center justify-center rounded border disabled:opacity-40"; - // Toolbar layout follows the brainstorm demo: picker on the left, actions on the right. - // The version menu joins the actions once artifact versions exist. + const versions = versionList?.versions ?? []; + const artifactItems = ( + <> + {entries.map((entry) => ( + + + {entry.path} + {changedPaths.has(entry.path) && ( + + )} + + + ))} + {deletedPaths.map((path) => ( + + + {path} + deleted + + + ))} + + ); + + // Toolbar layout follows the brainstorm demo: picker | version | fullscreen | reload. Pinned + // files have no versions; their slot holds the unpin action instead. const artbar = (