From 6f463e2c3e5fc6d9adbf0117f52c1f41e6194e1a Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 11:25:10 +0000 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=A4=96=20tests:=20repro=20for=20"Op?= =?UTF-8?q?en=20as=20artifact"=20refusing=20files=20outside=20the=20checko?= =?UTF-8?q?ut?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File tool cards show "Open as artifact" for every path the agent read, but pinFile refuses absolute paths outside the checkout. This test pins such a file and fails with "Only files inside the workspace checkout can be pinned". --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index 9508c42b881..4f72ce0f9ff 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -96,6 +96,55 @@ describe("pinned workspace files", () => { ]); }); + test("a file outside the checkout pins by its absolute path and reads live", async () => { + // File tool cards offer "Open as artifact" for any path the agent read, such as a plan or a + // skill file under the home folder. + const outside = path.join(tempDir, "notes", "plan.md"); + await fs.mkdir(path.dirname(outside), { recursive: true }); + await fs.writeFile(outside, "# Plan"); + expect(await pinFile(context, { workspaceId: "ws", path: outside })).toEqual({ + success: true, + data: { path: outside }, + }); + // A tool-cwd path that leaves the checkout names the same file. + expect( + await pinFile(context, { + workspaceId: "ws", + path: "../notes/plan.md", + relativeTo: "tool-cwd", + }) + ).toEqual({ success: true, data: { path: outside } }); + + const listed = await listPinnedFiles(context, { workspaceId: "ws" }); + if (!listed.success || !listed.data.available) throw new Error("expected pinned files"); + expect(listed.data.files.map(({ path, kind, size }) => ({ path, kind, size }))).toEqual([ + { path: outside, kind: "markdown", size: 6 }, + ]); + + await fs.writeFile(outside, "# Changed"); + expect(await readPinnedFile(context, { workspaceId: "ws", path: outside })).toMatchObject({ + success: true, + data: { status: "ok", content: "# Changed" }, + }); + + await unpinFile(context, { workspaceId: "ws", path: outside }); + expect(await readPinnedFile(context, { workspaceId: "ws", path: outside })).toEqual({ + success: false, + error: `File is not pinned: ${outside}`, + }); + }); + + test("refuses outside files that are missing, folders or symlinks", async () => { + const notes = path.join(tempDir, "notes"); + await fs.mkdir(notes, { recursive: true }); + await fs.symlink(path.join(checkout, "README.md"), path.join(notes, "link.md")); + for (const target of [path.join(notes, "missing.md"), notes, path.join(notes, "link.md")]) { + expect((await pinFile(context, { workspaceId: "ws", path: target })).success).toBe(false); + } + const listed = await listPinnedFiles(context, { workspaceId: "ws" }); + expect(listed.success && listed.data.available && listed.data.files).toEqual([]); + }); + test("a checkout root that is itself a symlink can pin and read files", async () => { const linked = path.join(tempDir, "linked-checkout"); await fs.symlink(checkout, linked); From 193b5fbef238d70f7a797d3155213a01375d9f8e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 11:32:12 +0000 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=A4=96=20fix:=20"Open=20as=20artifa?= =?UTF-8?q?ct"=20opens=20files=20outside=20the=20workspace=20checkout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File tool cards show "Open as artifact" for every path the agent read, including plans, skill files and scratch files outside the checkout. The backend refused them with "Only files inside the workspace checkout can be pinned", so the button was a dead end. A pin outside the checkout is now stored by its normalized absolute path on the checkout's host (local, worktree, SSH, Docker). It is read from its own folder by the same capped readers, which still refuse a symlinked file, folders and missing files. Checkout files keep their checkout-relative pins. Dev container workspaces still refuse outside files: tool paths there name container files, and pins are read on the host. Older versions drop absolute entries when they read the store. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- docs/workspaces/artifacts.mdx | 2 +- .../ArtifactsTab/OpenAsArtifactButton.tsx | 4 +- .../RightSidebar/ArtifactsTab/openArtifact.ts | 6 +- src/common/orpc/schemas/api.ts | 8 +- .../builtInSkillContent.generated.ts | 2 +- src/node/services/pinnedArtifactFiles.test.ts | 8 ++ src/node/services/pinnedArtifactFiles.ts | 114 +++++++++++++----- 7 files changed, 102 insertions(+), 42 deletions(-) diff --git a/docs/workspaces/artifacts.mdx b/docs/workspaces/artifacts.mdx index 056d3ba5544..e44aa8ec268 100644 --- a/docs/workspaces/artifacts.mdx +++ b/docs/workspaces/artifacts.mdx @@ -67,7 +67,7 @@ Pick a version from the version menu. **Latest (live)** follows the file as it c ## Open any file -Open any workspace file as an artifact from the review panel's file header, from a file path in a tool card, or with **Open File as Artifact…** in the command palette. Opened files appear under **Pinned files** in the picker and update as the file changes. Unpin them from the toolbar. +Open any file as an artifact from the review panel's file header, from a file path in a tool card, or with **Open File as Artifact…** in the command palette. A path outside the workspace checkout must be absolute, such as a plan or a skill file in your home folder. Dev container workspaces can open only files inside the checkout. Opened files appear under **Pinned files** in the picker and update as the file changes. Unpin them from the toolbar. ## Shelf diff --git a/src/browser/features/RightSidebar/ArtifactsTab/OpenAsArtifactButton.tsx b/src/browser/features/RightSidebar/ArtifactsTab/OpenAsArtifactButton.tsx index c6010aacbff..e98f9775a12 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/OpenAsArtifactButton.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/OpenAsArtifactButton.tsx @@ -9,12 +9,12 @@ import { cn } from "@/common/lib/utils"; import { pinAndOpenArtifact } from "./openArtifact"; /** - * "Open as artifact" for a checkout file (Review headers, file_read/file_edit cards): pins the + * "Open as artifact" for a file (Review headers, file_read/file_edit cards): pins the * file and shows it live in the Artifacts tab. Renders nothing while the experiment is off. */ export function OpenAsArtifactButton(props: { workspaceId: string | undefined; - /** Absolute inside the checkout, or relative to it (or to the tool cwd, see `relativeTo`). */ + /** Absolute, or relative to the checkout (or to the tool cwd, see `relativeTo`). */ path: string; /** * File tool cards pass "tool-cwd": their relative paths resolve from the tool's cwd (a diff --git a/src/browser/features/RightSidebar/ArtifactsTab/openArtifact.ts b/src/browser/features/RightSidebar/ArtifactsTab/openArtifact.ts index 33b4fcfc33f..15f17fb6678 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/openArtifact.ts +++ b/src/browser/features/RightSidebar/ArtifactsTab/openArtifact.ts @@ -6,7 +6,7 @@ import { readArtifactSelection, writeArtifactSelection } from "./artifactSelecti export interface OpenArtifactTarget { workspaceId: string; - /** Artifacts-relative path, or a checkout-relative path when `pinned`. */ + /** Artifacts-relative path, or the stored pin path (checkout-relative or absolute) when `pinned`. */ path: string; /** Version to show; null/absent means "Latest (live)". Ignored for pinned files. */ versionId?: number | null; @@ -67,8 +67,8 @@ const latestPinRequest = new Map(); let nextPinRequestId = 0; /** - * "Open as artifact" for any checkout file: pin it (absolute inside the checkout or relative), - * then open the stored checkout-relative path. Failures surface as a chat toast, the same + * "Open as artifact" for any file: pin it (absolute, or relative to the checkout), then open the + * stored pin path. Failures surface as a chat toast, the same * channel the command palette uses for feedback. `relativeTo: "tool-cwd"` resolves a relative * path like file tools do (a sub-project's cwd), instead of from the checkout root. */ diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 480b47ad325..7579bd3c92d 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -1521,9 +1521,9 @@ export const artifacts = { output: ResultSchema(PinnedArtifactFilesSchema, z.string()), }, /** - * Pin a checkout file; `path` may be absolute inside the checkout. A relative `path` resolves - * from the checkout root, or from the tool cwd (a sub-project dir) with `relativeTo: - * "tool-cwd"`, as file tools resolve it. Returns the stored checkout-relative path. + * Pin a file; `path` may be absolute. A relative `path` resolves from the checkout root, or + * from the tool cwd (a sub-project dir) with `relativeTo: "tool-cwd"`, as file tools resolve + * it. Returns the stored path: checkout-relative, or absolute for a file outside the checkout. */ pinFile: { input: z.object({ @@ -1537,7 +1537,7 @@ export const artifacts = { input: z.object({ workspaceId: z.string(), path: z.string() }), output: ResultSchema(z.void(), z.string()), }, - /** Read a pinned file (same shape and cap as `read`; path relative to the checkout). */ + /** Read a pinned file (same shape and cap as `read`; `path` as `pinFile` returned it). */ readPinned: { input: z.object({ workspaceId: z.string(), path: z.string() }), output: ArtifactReadOutcomeSchema, diff --git a/src/node/services/agentSkills/builtInSkillContent.generated.ts b/src/node/services/agentSkills/builtInSkillContent.generated.ts index 0cc5e8fb771..7d8ec1e154a 100644 --- a/src/node/services/agentSkills/builtInSkillContent.generated.ts +++ b/src/node/services/agentSkills/builtInSkillContent.generated.ts @@ -10685,7 +10685,7 @@ export const BUILTIN_SKILL_FILES: Record> = { "", "## Open any file", "", - "Open any workspace file as an artifact from the review panel's file header, from a file path in a tool card, or with **Open File as Artifact…** in the command palette. Opened files appear under **Pinned files** in the picker and update as the file changes. Unpin them from the toolbar.", + "Open any file as an artifact from the review panel's file header, from a file path in a tool card, or with **Open File as Artifact…** in the command palette. A path outside the workspace checkout must be absolute, such as a plan or a skill file in your home folder. Dev container workspaces can open only files inside the checkout. Opened files appear under **Pinned files** in the picker and update as the file changes. Unpin them from the toolbar.", "", "## Shelf", "", diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index 4f72ce0f9ff..275825743e9 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -6,6 +6,7 @@ import { acquireCrossProcessLock } from "@/node/utils/main/crossProcessLock"; import * as artifactStoreModule from "./artifactStore"; import type { ArtifactsContext } from "./artifactsOperations"; import { + PINNED_FILES_DEVCONTAINER_OUTSIDE_REASON, PINNED_FILES_MULTI_PROJECT_REASON, addPinnedPath, listPinnedFiles, @@ -255,6 +256,13 @@ describe("pinned workspace files", () => { for (const call of read.mock.calls) { expect(call[3]).toMatchObject({ requireDescriptorPaths: true }); } + // Tool paths outside the checkout name container files, not host files. + const outside = path.join(tempDir, "outside.md"); + await fs.writeFile(outside, "x"); + expect(await pinFile(context, { workspaceId: "ws", path: outside })).toEqual({ + success: false, + error: PINNED_FILES_DEVCONTAINER_OUTSIDE_REASON, + }); } finally { read.mockRestore(); } diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index f546314245b..74ffa29e443 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -33,11 +33,14 @@ import { } from "./artifactsOperations"; /** - * Pinned workspace files (Artifacts M4, Concept C): the user can open any checkout file in the - * Artifacts tab. Pins are live (re-read on change, never snapshotted), stored per workspace in - * `/pinned-files.json` as POSIX paths relative to the checkout, and read through the - * same capped, symlink-refusing readers as artifacts (hidden segments allowed: users pick files - * such as `.github/workflows/ci.yml` by hand). + * Pinned workspace files (Artifacts M4, Concept C): the user can open any file in the Artifacts + * tab. Pins are live (re-read on change, never snapshotted), stored per workspace in + * `/pinned-files.json`, and read through the same capped, symlink-refusing readers as + * artifacts (hidden segments allowed: users pick files such as `.github/workflows/ci.yml` by hand). + * A checkout file is stored as a POSIX path relative to the checkout, so the pin survives a + * checkout move. A file outside the checkout (a plan, a skill or a scratch file that a tool read) + * is stored by its absolute path on the checkout's host: file tool cards offer "Open as artifact" + * for every path, and refusing those was a dead button. */ export const PINNED_FILES_FILE_NAME = "pinned-files.json"; @@ -69,6 +72,22 @@ export const PINNED_FILES_MULTI_PROJECT_REASON = export const PINNED_FILES_DEVCONTAINER_REASON = "Pinned files are not available for dev container workspaces on this host."; +export const PINNED_FILES_DEVCONTAINER_OUTSIDE_REASON = + "Dev container workspaces can pin only files inside the workspace checkout."; + +const OUTSIDE_CHECKOUT_ERROR = "Only files inside the workspace checkout can be pinned"; + +/** + * A stored pin outside the checkout: an absolute path (POSIX on a runtime host, native here). + * Checkout-relative pins never match: their parser refuses a leading slash, backslashes and + * drive letters. + */ +function isOutsidePinPath(storedPath: string): boolean { + return ( + !storedPath.includes("\0") && (storedPath.startsWith("/") || path.win32.isAbsolute(storedPath)) + ); +} + export async function resolveCheckoutLocation( metadata: PinnedWorkspaceMetadata, createRuntime: (metadata: PinnedWorkspaceMetadata) => Runtime = createRuntimeForWorkspace @@ -110,7 +129,7 @@ export function toPinnedRelativePath(root: string, inputPath: string): string | if (path.isAbsolute(trimmed) || trimmed.startsWith("/")) { const rootPosix = normalizedRoot.replace(/\\/g, "/"); if (!relPath.startsWith(`${rootPosix}/`)) { - return { error: "Only files inside the workspace checkout can be pinned" }; + return { error: OUTSIDE_CHECKOUT_ERROR }; } relPath = relPath.slice(rootPosix.length + 1); } @@ -148,7 +167,8 @@ export async function readPinnedPaths(sessionDir: string): Promise { } // Drop entries a hand edit made invalid instead of failing every read. return parsed.data.paths.filter( - (p) => typeof parseArtifactRelativePath(p, { allowHidden: true }) !== "string" + (p) => + isOutsidePinPath(p) || typeof parseArtifactRelativePath(p, { allowHidden: true }) !== "string" ); } @@ -193,16 +213,53 @@ export async function removePinnedPath( await updatePinnedPaths(sessionDir, (paths) => paths.filter((p) => p !== relPath), locks); } +type AvailableLocation = Exclude; + +/** + * The folder a stored pin is read from and its path below it: the checkout for a checkout pin, + * the file's own folder for a pin outside the checkout (so the readers still refuse a symlinked + * file, as they do in the checkout). + */ +function pinTarget( + location: AvailableLocation, + storedPath: string +): { root: string; relPath: string } { + if (!isOutsidePinPath(storedPath)) return { root: location.root, relPath: storedPath }; + const paths = location.kind === "runtime" ? path.posix : path; + return { root: paths.dirname(storedPath), relPath: paths.basename(storedPath) }; +} + +/** + * Map a pin request to the stored path: checkout-relative inside the checkout, else the + * normalized absolute path. Dev containers refuse outside files: tool paths there name files in + * the container, while pins are read on this host. + */ +function toStoredPinPath( + location: AvailableLocation, + inputPath: string +): string | { error: string } { + if (!isOutsidePinPath(inputPath)) return toPinnedRelativePath(location.root, inputPath); + const absolute = + location.kind === "runtime" ? path.posix.normalize(inputPath) : path.resolve(inputPath); + const relPath = toPinnedRelativePath(location.root, absolute); + if (typeof relPath === "string" || relPath.error !== OUTSIDE_CHECKOUT_ERROR) return relPath; + if (location.kind === "host" && location.containerWritable === true) { + return { error: PINNED_FILES_DEVCONTAINER_OUTSIDE_REASON }; + } + return absolute; +} + async function statPinned( - location: Exclude, - relPath: string + location: AvailableLocation, + storedPath: string ): Promise<{ size: number; modifiedMs: number } | null> { + const { root, relPath } = pinTarget(location, storedPath); try { if (location.kind === "host") { - const stat = await fs.stat(path.join(location.root, ...relPath.split("/"))); + const stat = await fs.stat(path.join(root, ...relPath.split("/"))); return stat.isFile() ? { size: stat.size, modifiedMs: stat.mtimeMs } : null; } - const stat = await location.runtime.stat(`${location.root.replace(/\/+$/, "")}/${relPath}`); + const stat = await location.runtime.stat(`${root.replace(/\/+$/, "")}/${relPath}`); return stat.isDirectory ? null : { size: stat.size, modifiedMs: stat.modifiedTime.getTime() }; } catch { return null; @@ -254,7 +311,7 @@ export async function listPinnedFiles( * checkout root: resolve it against `execRoot` so the stored path is still checkout-relative. */ function resolvePinInput( - location: Exclude, + location: AvailableLocation, inputPath: string, relativeTo: "tool-cwd" | "checkout" | null | undefined ): string { @@ -285,27 +342,21 @@ async function realHostRoot(root: string): Promise { } /** - * Null when `relPath` is a regular file the pinned-file reader can open (same containment and + * Null when `storedPath` is a regular file the pinned-file reader can open (same containment and * symlink rules as readPinnedFile), else why it cannot be pinned. A tiny cap keeps this a probe: * a larger file reports `too_large`, which still proves it is readable. */ async function checkPinnable( - location: Exclude, - relPath: string + location: AvailableLocation, + storedPath: string ): Promise { const options = PINNED_READ_OPTIONS; + const { root, relPath } = pinTarget(location, storedPath); const outcome = location.kind === "runtime" - ? await readArtifactBytesOnRuntime( - location.runtime, - location.root, - relPath, - 1, - undefined, - options - ) + ? await readArtifactBytesOnRuntime(location.runtime, root, relPath, 1, undefined, options) : await readArtifactBytesFromDir( - await realHostRoot(location.root), + await realHostRoot(root), relPath, 1, hostReadOptions(location) @@ -315,7 +366,7 @@ async function checkPinnable( case "too_large": return null; case "missing": - return `Not a readable file in the checkout: ${relPath}`; + return `Not a readable file: ${storedPath}`; case "invalid": return outcome.error; } @@ -329,8 +380,8 @@ export async function pinFile( const location = await resolveForPinned(context, input.workspaceId); if (!location) return { success: false, error: `Workspace not found: ${input.workspaceId}` }; if (location.kind === "unavailable") return { success: false, error: location.reason }; - const relPath = toPinnedRelativePath( - location.root, + const relPath = toStoredPinPath( + location, resolvePinInput(location, input.path, input.relativeTo) ); if (typeof relPath !== "string") return { success: false, error: relPath.error }; @@ -376,12 +427,13 @@ export async function readPinnedFile( return { success: false, error: `File is not pinned: ${input.path}` }; } const options = PINNED_READ_OPTIONS; + const { root, relPath } = pinTarget(location, input.path); if (location.kind === "runtime") { try { const bytes = await readArtifactBytesOnRuntime( location.runtime, - location.root, - input.path, + root, + relPath, MAX_ARTIFACT_READ_BYTES, undefined, options @@ -395,8 +447,8 @@ export async function readPinnedFile( return toArtifactReadOutcome( input.path, await readArtifactBytesFromDir( - await realHostRoot(location.root), - input.path, + await realHostRoot(root), + relPath, MAX_ARTIFACT_READ_BYTES, hostReadOptions(location) ), From 89e9b14a2e55fc685c628a249db4f9317381afe9 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 11:43:23 +0000 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=A4=96=20fix:=20classify=20pin=20re?= =?UTF-8?q?quests=20by=20the=20host=20path=20rules,=20refresh=20pin=20path?= =?UTF-8?q?=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A drive-letter path is not absolute on a POSIX host, so it stays a checkout-relative request (refused, as before). Comments that said a pin path is always checkout-relative now say it can be absolute. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- .../features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx | 2 +- .../features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx | 2 +- .../RightSidebar/ArtifactsTab/SandboxedArtifactFrame.tsx | 2 +- .../RightSidebar/ArtifactsTab/artifactSelection.ts | 5 +++-- .../RightSidebar/ArtifactsTab/useArtifactAssetReader.ts | 2 +- src/common/constants/events.ts | 2 +- src/node/services/pinnedArtifactFiles.ts | 7 +++++-- 7 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx index dcdfdb0e1b6..8c84c26639f 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactViewer.tsx @@ -133,7 +133,7 @@ export function ArtifactViewer(props: { mainPaneInterruptKey?: ArtifactFrameInterruptKey; /** * Resolve relative images/assets through the artifacts folder (default). Off for pinned - * checkout files, whose neighbours live in the checkout, not the artifacts folder. + * files, whose neighbours live outside the artifacts folder. */ readRelativeAssets?: boolean; /** Host side of artifact interactions (M5b); absent where artifacts are read-only. */ diff --git a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx index e5613fe9d5d..392e6eda40e 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/ArtifactsPanel.tsx @@ -426,7 +426,7 @@ export function ArtifactsPanel(props: { }; }, [api, props.workspaceId, refreshTick]); - // Pinned checkout files are live: re-list them on the same refresh signal as the artifacts, + // Pinned 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; diff --git a/src/browser/features/RightSidebar/ArtifactsTab/SandboxedArtifactFrame.tsx b/src/browser/features/RightSidebar/ArtifactsTab/SandboxedArtifactFrame.tsx index 6c0df38f70d..9a3e6ee4435 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/SandboxedArtifactFrame.tsx +++ b/src/browser/features/RightSidebar/ArtifactsTab/SandboxedArtifactFrame.tsx @@ -474,7 +474,7 @@ export function SandboxedArtifactFrame(props: { interactions?.setState?.(message.state); break; case "openWorkspace": { - // Like send/setState, only for interactive artifacts: pinned checkout files and shelf + // Like send/setState, only for interactive artifacts: pinned files and shelf // copies stay read-only. Ignored without a gesture, with no notice, so a script cannot // spam the notice either. if (interactions == null || !hasLiveUserGesture(frameRef.current)) break; diff --git a/src/browser/features/RightSidebar/ArtifactsTab/artifactSelection.ts b/src/browser/features/RightSidebar/ArtifactsTab/artifactSelection.ts index 2002249ccac..a7820832614 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/artifactSelection.ts +++ b/src/browser/features/RightSidebar/ArtifactsTab/artifactSelection.ts @@ -14,8 +14,9 @@ import { export type ArtifactSelectionScope = "artifact" | "pinned" | "shelf"; /** - * The Artifacts tab selection of one workspace: an artifacts-relative path, a checkout-relative - * path when `pinned`, or a shelf entry; `version` null means "Latest (live)". + * The Artifacts tab selection of one workspace: an artifacts-relative path, the stored pin path + * (checkout-relative or absolute) when `pinned`, or a shelf entry; `version` null means "Latest + * (live)". */ export interface ArtifactSelection { scope: ArtifactSelectionScope; diff --git a/src/browser/features/RightSidebar/ArtifactsTab/useArtifactAssetReader.ts b/src/browser/features/RightSidebar/ArtifactsTab/useArtifactAssetReader.ts index c25ebd01c01..874473fdb25 100644 --- a/src/browser/features/RightSidebar/ArtifactsTab/useArtifactAssetReader.ts +++ b/src/browser/features/RightSidebar/ArtifactsTab/useArtifactAssetReader.ts @@ -54,7 +54,7 @@ function createConnectedReader( /** * Reader for files an artifact references, through the same size-capped artifacts.read route. - * A null workspaceId turns relative assets off (pinned checkout files: their neighbours are not + * A null workspaceId turns relative assets off (pinned files: their neighbours are not * in the artifacts folder and need not be pinned). * * The reader must keep its identity across renders: renderers list it in effect dependencies diff --git a/src/common/constants/events.ts b/src/common/constants/events.ts index 217aedb8611..7a563122160 100644 --- a/src/common/constants/events.ts +++ b/src/common/constants/events.ts @@ -309,7 +309,7 @@ export interface CustomEventPayloads { }; [CUSTOM_EVENTS.OPEN_ARTIFACT]: { workspaceId: string; - /** Artifacts-relative path, or a checkout-relative path when `pinned`. */ + /** Artifacts-relative path, or the stored pin path (checkout-relative or absolute) when `pinned`. */ path: string; /** Version number to show; absent means "Latest (live)". */ versionId?: number; diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index 74ffa29e443..7696f89fc88 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -238,7 +238,10 @@ function toStoredPinPath( location: AvailableLocation, inputPath: string ): string | { error: string } { - if (!isOutsidePinPath(inputPath)) return toPinnedRelativePath(location.root, inputPath); + // By its own rules: a drive-letter path is not absolute on a POSIX host. + const isAbsolute = + location.kind === "runtime" ? inputPath.startsWith("/") : path.isAbsolute(inputPath); + if (!isAbsolute) return toPinnedRelativePath(location.root, inputPath); const absolute = location.kind === "runtime" ? path.posix.normalize(inputPath) : path.resolve(inputPath); const relPath = toPinnedRelativePath(location.root, absolute); @@ -308,7 +311,7 @@ export async function listPinnedFiles( /** * A relative path from a file tool names a file under the tool cwd (a sub-project dir), not the - * checkout root: resolve it against `execRoot` so the stored path is still checkout-relative. + * checkout root: resolve it against `execRoot` before it is mapped to the stored path. */ function resolvePinInput( location: AvailableLocation, From edc0a26e0de7c72d3661e6fcc4e258268e46b3fc Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 12:29:36 +0000 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=A4=96=20tests:=20repros=20for=20pi?= =?UTF-8?q?nned=20files=20at=20the=20filesystem=20root,=20alias=20paths=20?= =?UTF-8?q?and=20drive-letter=20entries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three failing tests from the review of #7915: a pinned read whose folder is `/` is refused, one file reached through a symlinked folder is two pins, and a hand-edited `C:/x.md` entry on a POSIX host is read below the backend's working folder. Adds resolveRealDirOnRuntime, which the fix uses, with its own test. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- .../services/artifactRuntimeStore.test.ts | 26 +++++++++++++ src/node/services/artifactRuntimeStore.ts | 33 ++++++++++++++++ src/node/services/pinnedArtifactFiles.test.ts | 38 ++++++++++++++++++- 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/src/node/services/artifactRuntimeStore.test.ts b/src/node/services/artifactRuntimeStore.test.ts index 80c16effba2..c99841c1238 100644 --- a/src/node/services/artifactRuntimeStore.test.ts +++ b/src/node/services/artifactRuntimeStore.test.ts @@ -11,6 +11,7 @@ import { parseArtifactListOutput, readArtifactBytesOnRuntime, readArtifactOnRuntime, + resolveRealDirOnRuntime, writeArtifactOnRuntime, } from "./artifactRuntimeStore"; import { @@ -269,6 +270,31 @@ describe("artifactRuntimeStore", () => { }); }); + test("a pinned read can use the filesystem root as its folder", async () => { + // A pinned file directly under `/` is read from `/`: containment must not require `//`. + const checkout = path.join(tempDir, "checkout"); + await fs.mkdir(checkout); + await fs.writeFile(path.join(checkout, "a.txt"), "rooted"); + const options = { allowHidden: true, requireArtifactsBasename: false }; + const relPath = path.join(checkout, "a.txt").slice(1); + expect( + await readArtifactBytesOnRuntime(runtime, "/", relPath, CAP, undefined, options) + ).toMatchObject({ status: "ok", bytes: Buffer.from("rooted") }); + expect(await readArtifactFromDir("/", relPath, CAP, options)).toMatchObject({ + success: true, + data: { status: "ok", content: "rooted" }, + }); + }); + + test("resolves a folder's real path on the runtime", async () => { + const real = path.join(tempDir, "real dir"); + await fs.mkdir(real); + await fs.symlink(real, path.join(tempDir, "link")); + expect(await resolveRealDirOnRuntime(runtime, path.join(tempDir, "link"))).toBe(real); + expect(await resolveRealDirOnRuntime(runtime, real)).toBe(real); + expect(await resolveRealDirOnRuntime(runtime, path.join(tempDir, "missing"))).toBeNull(); + }); + test("reads a pinned checkout file only when the artifacts name check is off", async () => { const checkout = path.join(tempDir, "checkout"); await fs.mkdir(path.join(checkout, ".config"), { recursive: true }); diff --git a/src/node/services/artifactRuntimeStore.ts b/src/node/services/artifactRuntimeStore.ts index 3908827c667..85d0f7e7225 100644 --- a/src/node/services/artifactRuntimeStore.ts +++ b/src/node/services/artifactRuntimeStore.ts @@ -31,6 +31,7 @@ import { */ const LIST_MAGIC = "XUMARTIFACTS1"; +const REAL_DIR_MAGIC = "XUMREALDIR1"; const READ_MAGIC = "XUMREAD1"; const WRITE_MAGIC = "XUMWRITE1"; const EXEC_TIMEOUT_SECONDS = 30; @@ -152,6 +153,38 @@ export function parseArtifactListOutput(stdout: string): RuntimeArtifactListing } } +/** + * Real path of a folder on the runtime (symlinks resolved), or null when it is missing or not a + * folder. Pins outside the checkout store it: the read script refuses a symlinked root folder. + */ +export async function resolveRealDirOnRuntime( + runtime: Runtime, + dir: string, + abortSignal?: AbortSignal +): Promise { + const result = await execBuffered( + runtime, + `cd -P -- "$XUM_DIR" 2>/dev/null || exit 0\nprintf '${REAL_DIR_MAGIC}\\0'\npwd -P\n`, + { + cwd: "/", + pathEnv: { XUM_DIR: dir }, + timeout: EXEC_TIMEOUT_SECONDS, + abortSignal, + maxOutputBytes: READ_OUTPUT_SLACK_BYTES, + } + ); + if (result.exitCode !== 0) { + throw new Error( + `Resolving a folder failed (exit ${result.exitCode}): ${result.stderr.trim().slice(0, 500)}` + ); + } + // A login banner can come first; pwd ends its line with one newline. + const start = result.stdout.indexOf(`${REAL_DIR_MAGIC}\0`); + if (start < 0) return null; + const real = result.stdout.slice(start + REAL_DIR_MAGIC.length + 1).replace(/\n$/, ""); + return real.startsWith("/") ? real : null; +} + export async function listArtifactsOnRuntime( runtime: Runtime, artifactsDir: string, diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index 275825743e9..dd03c318b3c 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -100,7 +100,7 @@ describe("pinned workspace files", () => { test("a file outside the checkout pins by its absolute path and reads live", async () => { // File tool cards offer "Open as artifact" for any path the agent read, such as a plan or a // skill file under the home folder. - const outside = path.join(tempDir, "notes", "plan.md"); + const outside = path.join(await fs.realpath(tempDir), "notes", "plan.md"); await fs.mkdir(path.dirname(outside), { recursive: true }); await fs.writeFile(outside, "# Plan"); expect(await pinFile(context, { workspaceId: "ws", path: outside })).toEqual({ @@ -135,6 +135,42 @@ describe("pinned workspace files", () => { }); }); + test("one file reached through a symlinked folder is one pin", async () => { + const real = await fs.realpath(tempDir); + await fs.mkdir(path.join(real, "notes")); + await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan"); + await fs.symlink(path.join(real, "notes"), path.join(real, "notes-link")); + await fs.mkdir(path.join(checkout, "docs")); + await fs.writeFile(path.join(checkout, "docs", "x.md"), "x"); + await fs.symlink(path.join(checkout, "docs"), path.join(real, "docs-link")); + + // An alias of an outside folder stores the real path, so both spellings are one pin. + for (const alias of ["notes", "notes-link"]) { + expect( + await pinFile(context, { workspaceId: "ws", path: path.join(real, alias, "plan.md") }) + ).toEqual({ success: true, data: { path: path.join(real, "notes", "plan.md") } }); + } + // An alias that leads into the checkout is a checkout pin. + expect( + await pinFile(context, { workspaceId: "ws", path: path.join(real, "docs-link", "x.md") }) + ).toEqual({ success: true, data: { path: "docs/x.md" } }); + const listed = await listPinnedFiles(context, { workspaceId: "ws" }); + expect(listed.success && listed.data.available && listed.data.files.map((f) => f.path)).toEqual( + [path.join(real, "notes", "plan.md"), "docs/x.md"] + ); + }); + + test("a drive-letter entry on a POSIX host is invalid, not a path below the backend's folder", async () => { + if (process.platform === "win32") return; + // Only a hand edit of the store can add one. Read as a folder `C:` it would resolve against + // the backend's working folder. + const sessionDir = path.join(tempDir, "sessions", "ws"); + await addPinnedPath(sessionDir, "C:/x.md"); + const read = await readPinnedFile(context, { workspaceId: "ws", path: "C:/x.md" }); + expect(read.success).toBe(false); + expect(read).not.toHaveProperty("missing"); + }); + test("refuses outside files that are missing, folders or symlinks", async () => { const notes = path.join(tempDir, "notes"); await fs.mkdir(notes, { recursive: true }); From 58c7823ec7bc036b4249e67dde1effd1942fadcd Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 12:32:12 +0000 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=A4=96=20fix:=20pinned=20files=20st?= =?UTF-8?q?ore=20real=20folder=20paths=20and=20read=20at=20the=20filesyste?= =?UTF-8?q?m=20root?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review of #7915: - A pin outside the checkout stores the file name in its folder's real path. One file reached through a symlinked folder is now one pin, a symlinked folder into the checkout gives a checkout pin, and SSH and Docker read files in a symlinked folder (their read script refuses a symlinked root). - Containment checks accept `/` as a folder (host reader and the runtime read script), so a file directly under `/` can be pinned. - A stored entry is absolute only by the location's own rules, so a hand-edited `C:/x.md` on a POSIX host is invalid instead of a path below the backend's working folder. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/artifactRuntimeStore.ts | 16 +++++-- src/node/services/artifactStore.ts | 9 +++- src/node/services/pinnedArtifactFiles.ts | 56 ++++++++++++++++------- 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/node/services/artifactRuntimeStore.ts b/src/node/services/artifactRuntimeStore.ts index 85d0f7e7225..31d5fa165a6 100644 --- a/src/node/services/artifactRuntimeStore.ts +++ b/src/node/services/artifactRuntimeStore.ts @@ -231,21 +231,29 @@ d=$XUM_ARTIFACTS_DIR if [ -L "$d" ] || [ ! -d "$d" ]; then xum_missing; fi cd -- "$d" 2>/dev/null || xum_missing xum_root=$(pwd -P) || xum_missing -xum_parent=$(cd -P -- "$` + +xum_up=$` + "{d%/*}" + + String.raw` +xum_parent=$(cd -P -- "$` + + "{xum_up:-/}" + String.raw`" 2>/dev/null && pwd -P) || xum_missing -if [ "$xum_root" != "$xum_parent/${rootName}" ]; then xum_missing; fi +if [ "$xum_root" != "$` + + "{xum_parent%/}" + + String.raw`/${rootName}" ]; then xum_missing; fi +xum_pre=$` + + "{xum_root%/}" + + String.raw`/ for xum_seg in ${dirSegments.join(" ")}; do if [ -L "$xum_seg" ] || [ ! -d "$xum_seg" ]; then xum_missing; fi cd -- "$xum_seg" 2>/dev/null || xum_missing done -case $(pwd -P) in "$xum_root"/*|"$xum_root") ;; *) xum_missing ;; esac +case $(pwd -P) in "$xum_pre"*|"$xum_root") ;; *) xum_missing ;; esac xum_leaf=${leaf} if [ -L "$xum_leaf" ] || [ ! -f "$xum_leaf" ]; then xum_missing; fi ${STAT_PRELUDE} xum_send() { if xum_fd=$(readlink "/proc/$$/fd/3" 2>/dev/null); then - case $xum_fd in "$xum_root"/*) ;; *) xum_missing ;; esac + case $xum_fd in "$xum_pre"*) ;; *) xum_missing ;; esac fi if [ -L "$xum_leaf" ]; then xum_missing; fi xum_m=$(xum_meta "./$xum_leaf") || xum_missing diff --git a/src/node/services/artifactStore.ts b/src/node/services/artifactStore.ts index 2d93117b2c6..eff5549466d 100644 --- a/src/node/services/artifactStore.ts +++ b/src/node/services/artifactStore.ts @@ -78,6 +78,11 @@ export function getArtifactsDir(scratchDir: string): string { return path.join(scratchDir, ARTIFACTS_DIR_NAME); } +/** `dir` plus one separator: a pinned file's folder can be the filesystem root (`/`). */ +function dirPrefix(dir: string): string { + return dir.endsWith(path.sep) ? dir : dir + path.sep; +} + function isMissing(error: unknown): boolean { const code = (error as NodeJS.ErrnoException | undefined)?.code; return code === "ENOENT" || code === "ENOTDIR"; @@ -365,7 +370,7 @@ export async function readArtifactBytesFromDir( throw error; } // Symlinked parent folders could still point elsewhere: confine by real path. - if (!realTarget.startsWith(realDir + path.sep)) return notFound; + if (!realTarget.startsWith(dirPrefix(realDir))) return notFound; let handle: fs.FileHandle; try { @@ -610,7 +615,7 @@ async function verifyOpenedInsideDir( fdPath = undefined; } if (fdPath !== undefined) { - return fdPath === realTarget && fdPath.startsWith(realDir + path.sep); + return fdPath === realTarget && fdPath.startsWith(dirPrefix(realDir)); } if (requireDescriptorPath) return false; try { diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index 7696f89fc88..611cc0e82e8 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -17,7 +17,7 @@ import { MutexMap } from "@/node/utils/concurrency/mutexMap"; import { nodeSessionLocks } from "@/node/services/sessionLocks/nodeSessionLocks"; import type { SessionLocks } from "@/node/services/sessionLocks/sessionLocks"; import writeFileAtomic from "@/node/utils/writeFileAtomic"; -import { readArtifactBytesOnRuntime } from "./artifactRuntimeStore"; +import { readArtifactBytesOnRuntime, resolveRealDirOnRuntime } from "./artifactRuntimeStore"; import { hostSupportsDescriptorPaths, parseArtifactRelativePath, @@ -215,6 +215,17 @@ export async function removePinnedPath( type AvailableLocation = Exclude; +/** Absolute by the location's own rules: a drive-letter path is not absolute on a POSIX host. */ +function isAbsoluteOn(location: AvailableLocation, filePath: string): boolean { + return location.kind === "runtime" ? filePath.startsWith("/") : path.isAbsolute(filePath); +} + +/** Real path of a folder where the checkout lives, or null when it is missing. */ +async function realDirOn(location: AvailableLocation, dir: string): Promise { + if (location.kind === "runtime") return resolveRealDirOnRuntime(location.runtime, dir); + return fs.realpath(dir).catch(() => null); +} + /** * The folder a stored pin is read from and its path below it: the checkout for a checkout pin, * the file's own folder for a pin outside the checkout (so the readers still refuse a symlinked @@ -224,32 +235,38 @@ function pinTarget( location: AvailableLocation, storedPath: string ): { root: string; relPath: string } { - if (!isOutsidePinPath(storedPath)) return { root: location.root, relPath: storedPath }; + // A hand-edited entry that is not absolute here stays relative, and the readers refuse it. + if (!isAbsoluteOn(location, storedPath)) return { root: location.root, relPath: storedPath }; const paths = location.kind === "runtime" ? path.posix : path; return { root: paths.dirname(storedPath), relPath: paths.basename(storedPath) }; } /** - * Map a pin request to the stored path: checkout-relative inside the checkout, else the - * normalized absolute path. Dev containers refuse outside files: tool paths there name files in - * the container, while pins are read on this host. + * Map a pin request to the stored path: checkout-relative inside the checkout, else the file + * name in its folder's real path. The real path makes one file reached through a symlinked + * folder one pin (and a checkout pin when the folder is in the checkout), and the runtime + * readers refuse a symlinked folder. Dev containers refuse outside files: tool paths there name + * files in the container, while pins are read on this host. */ -function toStoredPinPath( +async function toStoredPinPath( location: AvailableLocation, inputPath: string -): string | { error: string } { - // By its own rules: a drive-letter path is not absolute on a POSIX host. - const isAbsolute = - location.kind === "runtime" ? inputPath.startsWith("/") : path.isAbsolute(inputPath); - if (!isAbsolute) return toPinnedRelativePath(location.root, inputPath); +): Promise { + if (!isAbsoluteOn(location, inputPath)) return toPinnedRelativePath(location.root, inputPath); + const paths = location.kind === "runtime" ? path.posix : path; const absolute = - location.kind === "runtime" ? path.posix.normalize(inputPath) : path.resolve(inputPath); + location.kind === "runtime" ? paths.normalize(inputPath) : paths.resolve(inputPath); const relPath = toPinnedRelativePath(location.root, absolute); if (typeof relPath === "string" || relPath.error !== OUTSIDE_CHECKOUT_ERROR) return relPath; if (location.kind === "host" && location.containerWritable === true) { return { error: PINNED_FILES_DEVCONTAINER_OUTSIDE_REASON }; } - return absolute; + const realDir = await realDirOn(location, paths.dirname(absolute)); + if (realDir == null) return { error: `Not a readable file: ${absolute}` }; + const real = paths.join(realDir, paths.basename(absolute)); + const realRoot = await realDirOn(location, location.root); + const inCheckout = realRoot == null ? null : toPinnedRelativePath(realRoot, real); + return typeof inCheckout === "string" ? inCheckout : real; } async function statPinned( @@ -383,10 +400,15 @@ export async function pinFile( const location = await resolveForPinned(context, input.workspaceId); if (!location) return { success: false, error: `Workspace not found: ${input.workspaceId}` }; if (location.kind === "unavailable") return { success: false, error: location.reason }; - const relPath = toStoredPinPath( - location, - resolvePinInput(location, input.path, input.relativeTo) - ); + let relPath: string | { error: string }; + try { + relPath = await toStoredPinPath( + location, + resolvePinInput(location, input.path, input.relativeTo) + ); + } catch (error) { + return unreachableError(error); + } if (typeof relPath !== "string") return { success: false, error: relPath.error }; const sessionDir = sessionDirFor(context, input.workspaceId); // Pins persist until unpinned: refuse missing files, folders and symlinks up front instead of From d254dc3f71165208fbf39492cda349b95cf7aef7 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 12:33:54 +0000 Subject: [PATCH 06/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20split=20the=20?= =?UTF-8?q?real-path=20step=20out=20of=20toStoredPinPath=20(CRAP)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index 611cc0e82e8..27c0916b629 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -261,6 +261,18 @@ async function toStoredPinPath( if (location.kind === "host" && location.containerWritable === true) { return { error: PINNED_FILES_DEVCONTAINER_OUTSIDE_REASON }; } + return await toRealOutsidePinPath(location, absolute); +} + +/** + * An outside file's stored path: its name in its folder's real path, or the checkout pin when + * that real folder is in the checkout. + */ +async function toRealOutsidePinPath( + location: AvailableLocation, + absolute: string +): Promise { + const paths = location.kind === "runtime" ? path.posix : path; const realDir = await realDirOn(location, paths.dirname(absolute)); if (realDir == null) return { error: `Not a readable file: ${absolute}` }; const real = paths.join(realDir, paths.basename(absolute)); From 9a67b4abc4313a1310f78f4b89b8049dcf3a17a2 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 12:59:24 +0000 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=A4=96=20fix:=20list=20only=20pins?= =?UTF-8?q?=20that=20are=20valid=20on=20the=20checkout's=20host?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hand-edited drive-letter entry on a POSIX host stayed in the pinned list as a pin that never reads. listPinnedFiles now drops entries that are neither absolute on the host nor a valid checkout-relative path. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.test.ts | 2 ++ src/node/services/pinnedArtifactFiles.ts | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index dd03c318b3c..8fb07e63d64 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -169,6 +169,8 @@ describe("pinned workspace files", () => { const read = await readPinnedFile(context, { workspaceId: "ws", path: "C:/x.md" }); expect(read.success).toBe(false); expect(read).not.toHaveProperty("missing"); + const listed = await listPinnedFiles(context, { workspaceId: "ws" }); + expect(listed.success && listed.data.available && listed.data.files).toEqual([]); }); test("refuses outside files that are missing, folders or symlinks", async () => { diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index 27c0916b629..366a2fbaa46 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -323,7 +323,13 @@ export async function listPinnedFiles( if (location.kind === "unavailable") { return { success: true, data: { available: false, reason: location.reason } }; } - const paths = await readPinnedPaths(sessionDirFor(context, input.workspaceId)); + // The store keeps any absolute entry; list only those absolute here (a hand-edited `C:/x.md` + // on a POSIX host would list as a pin that never reads). + const paths = (await readPinnedPaths(sessionDirFor(context, input.workspaceId))).filter( + (p) => + isAbsoluteOn(location, p) || + typeof parseArtifactRelativePath(p, { allowHidden: true }) !== "string" + ); const files = await Promise.all( paths.map(async (relPath) => { const stat = await statPinned(location, relPath); From 348fa24ab6bb4a92230e0a54df59e197f5d351da Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 13:44:21 +0000 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=A4=96=20tests:=20pin=20and=20read?= =?UTF-8?q?=20an=20outside=20file=20in=20a=20symlinked=20folder=20on=20an?= =?UTF-8?q?=20SSH=20checkout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-to-end check of the runtime path: pinFile stores the folder's real path and readPinnedFile reads it through the runtime read script. On main's code the pin is refused. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.test.ts | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index 8fb07e63d64..f03d570782c 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun: import * as fs from "fs/promises"; import * as os from "os"; import * as path from "path"; +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import * as runtimeHelpers from "@/node/runtime/runtimeHelpers"; import { acquireCrossProcessLock } from "@/node/utils/main/crossProcessLock"; import * as artifactStoreModule from "./artifactStore"; import type { ArtifactsContext } from "./artifactsOperations"; @@ -24,6 +26,8 @@ describe("pinned workspace files", () => { let projects: Array<{ projectPath: string; projectName: string }> | undefined; let subProjectPath: string | undefined; let runtimeConfig: Record; + let workspaceName: string; + let projectPath: string; beforeEach(async () => { tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pinned-files-")); @@ -34,6 +38,8 @@ describe("pinned workspace files", () => { projects = undefined; subProjectPath = undefined; runtimeConfig = { type: "worktree", srcBaseDir: tempDir }; + workspaceName = "checkout"; + projectPath = tempDir; context = { config: { sessionsDir: path.join(tempDir, "sessions") }, workspaceService: { @@ -42,8 +48,8 @@ describe("pinned workspace files", () => { workspaceId === "ws" ? { id: "ws", - name: "checkout", - projectPath: tempDir, + name: workspaceName, + projectPath, projectName: "project", namedWorkspacePath: checkout, runtimeConfig, @@ -160,6 +166,36 @@ describe("pinned workspace files", () => { ); }); + test("an SSH checkout pins and reads an outside file in a symlinked folder", async () => { + // A LocalRuntime stands in for the SSH host: pins and reads run the real runtime scripts. + const real = await fs.realpath(tempDir); + runtimeConfig = { type: "ssh", host: "example", srcBaseDir: real }; + // An in-place workspace (name equals project path) roots the checkout at the project path. + projectPath = workspaceName = path.join(real, "checkout"); + const create = spyOn(runtimeHelpers, "createRuntimeForWorkspace").mockReturnValue( + new LocalRuntime(real) + ); + try { + await fs.mkdir(path.join(real, "notes")); + await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan"); + await fs.symlink(path.join(real, "notes"), path.join(real, "notes-link")); + const stored = path.join(real, "notes", "plan.md"); + expect( + await pinFile(context, { + workspaceId: "ws", + path: path.join(real, "notes-link", "plan.md"), + }) + ).toEqual({ success: true, data: { path: stored } }); + expect(await readPinnedFile(context, { workspaceId: "ws", path: stored })).toMatchObject({ + success: true, + data: { status: "ok", content: "# Plan" }, + }); + expect(create).toHaveBeenCalled(); + } finally { + create.mockRestore(); + } + }); + test("a drive-letter entry on a POSIX host is invalid, not a path below the backend's folder", async () => { if (process.platform === "win32") return; // Only a hand edit of the store can add one. Read as a folder `C:` it would resolve against From 0c56d1ed561ea3e9a4daba6586a0716af96845c1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 13:53:43 +0000 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=A4=96=20fix:=20re-pinning=20an=20a?= =?UTF-8?q?lias=20stored=20before=20real=20folder=20paths=20keeps=20one=20?= =?UTF-8?q?entry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An outside pin that #7915 stored through a symlinked folder kept its alias path, and re-pinning it now produced the real path, so the file listed twice. pinFile compares absolute entries by their real form and opens the stored entry instead of adding a second one. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.test.ts | 20 ++++++++++ src/node/services/pinnedArtifactFiles.ts | 38 ++++++++++++++----- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index f03d570782c..069b9ab1479 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -196,6 +196,26 @@ describe("pinned workspace files", () => { } }); + test("re-pinning an alias stored before real folder paths keeps the one entry", async () => { + const real = await fs.realpath(tempDir); + await fs.mkdir(path.join(real, "notes")); + await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan"); + await fs.symlink(path.join(real, "notes"), path.join(real, "notes-link")); + const alias = path.join(real, "notes-link", "plan.md"); + // An older build stored the alias as given. + await addPinnedPath(path.join(tempDir, "sessions", "ws"), alias); + for (const request of [alias, path.join(real, "notes", "plan.md")]) { + expect(await pinFile(context, { workspaceId: "ws", path: request })).toEqual({ + success: true, + data: { path: alias }, + }); + } + const listed = await listPinnedFiles(context, { workspaceId: "ws" }); + expect(listed.success && listed.data.available && listed.data.files.map((f) => f.path)).toEqual( + [alias] + ); + }); + test("a drive-letter entry on a POSIX host is invalid, not a path below the backend's folder", async () => { if (process.platform === "win32") return; // Only a hand edit of the store can add one. Read as a folder `C:` it would resolve against diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index 366a2fbaa46..abefcb14945 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -410,6 +410,25 @@ async function checkPinnable( } } +/** + * The stored entry that already names the file of `storedPath`, or null. An outside pin stored + * before pins used real folder paths can hold an alias of the same file, so absolute entries + * are compared by their real form too: re-pinning the alias must not add a second entry. + */ +async function findExistingPin( + location: AvailableLocation, + pinned: readonly string[], + storedPath: string +): Promise { + if (pinned.includes(storedPath)) return storedPath; + const absolute = pinned.filter((p) => isAbsoluteOn(location, p)); + const real = await Promise.all( + absolute.map((p) => toStoredPinPath(location, p).catch(() => null)) + ); + const index = real.indexOf(storedPath); + return index < 0 ? null : absolute[index]; +} + export async function pinFile( context: ArtifactsContext, input: { workspaceId: string; path: string; relativeTo?: "tool-cwd" | "checkout" | null } @@ -429,17 +448,18 @@ export async function pinFile( } if (typeof relPath !== "string") return { success: false, error: relPath.error }; const sessionDir = sessionDirFor(context, input.workspaceId); + // Re-pinning an existing pin stays a no-op and opens the stored entry. + const existing = await findExistingPin(location, await readPinnedPaths(sessionDir), relPath); + if (existing != null) return { success: true, data: { path: existing } }; // Pins persist until unpinned: refuse missing files, folders and symlinks up front instead of - // storing a pin that can never be read. Re-pinning an existing pin stays a no-op. - if (!(await readPinnedPaths(sessionDir)).includes(relPath)) { - let refusal: string | null; - try { - refusal = await checkPinnable(location, relPath); - } catch (error) { - return unreachableError(error); - } - if (refusal != null) return { success: false, error: refusal }; + // storing a pin that can never be read. + let refusal: string | null; + try { + refusal = await checkPinnable(location, relPath); + } catch (error) { + return unreachableError(error); } + if (refusal != null) return { success: false, error: refusal }; await addPinnedPath(sessionDir, relPath); return { success: true, data: { path: relPath } }; } From febc8fdcb76c0549a305786cca51dc674778069f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 14:08:17 +0000 Subject: [PATCH 10/11] =?UTF-8?q?=F0=9F=A4=96=20tests:=20repro=20for=20pin?= =?UTF-8?q?ning=20on=20SSH=20resolving=20every=20stored=20outside=20pin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pinning a new file on an SSH checkout runs one folder resolution per stored outside pin (about 2N+2 execs at once). This test pins a new file with three unrelated stored pins and expects none of them resolved. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/node/services/pinnedArtifactFiles.test.ts b/src/node/services/pinnedArtifactFiles.test.ts index 069b9ab1479..7de46e1e8c2 100644 --- a/src/node/services/pinnedArtifactFiles.test.ts +++ b/src/node/services/pinnedArtifactFiles.test.ts @@ -196,6 +196,38 @@ describe("pinned workspace files", () => { } }); + test("pinning on an SSH checkout resolves only stored pins with the same file name", async () => { + // Each folder resolution is one exec on the SSH host; other outside pins cannot be aliases. + const real = await fs.realpath(tempDir); + runtimeConfig = { type: "ssh", host: "example", srcBaseDir: real }; + projectPath = workspaceName = path.join(real, "checkout"); + const resolved: string[] = []; + class CountingRuntime extends LocalRuntime { + override exec(command: string, options: Parameters[1]) { + if (command.includes("XUMREALDIR1")) resolved.push(options.pathEnv?.XUM_DIR ?? ""); + return super.exec(command, options); + } + } + const create = spyOn(runtimeHelpers, "createRuntimeForWorkspace").mockReturnValue( + new CountingRuntime(real) + ); + try { + const sessionDir = path.join(tempDir, "sessions", "ws"); + for (const name of ["a.md", "b.md", "c.md"]) { + await addPinnedPath(sessionDir, path.join(real, "old", name)); + } + await fs.mkdir(path.join(real, "notes")); + await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan"); + expect( + await pinFile(context, { workspaceId: "ws", path: path.join(real, "notes", "plan.md") }) + ).toEqual({ success: true, data: { path: path.join(real, "notes", "plan.md") } }); + // The new pin's folder and the checkout root, nothing for a.md, b.md or c.md. + expect(resolved.filter((dir) => dir.startsWith(path.join(real, "old")))).toEqual([]); + } finally { + create.mockRestore(); + } + }); + test("re-pinning an alias stored before real folder paths keeps the one entry", async () => { const real = await fs.realpath(tempDir); await fs.mkdir(path.join(real, "notes")); From 98feec8978662f4c000caabae106e85bbccfbc8a Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 14:09:39 +0000 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=A4=96=20perf:=20resolve=20only=20s?= =?UTF-8?q?ame-name=20stored=20pins=20when=20pinning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findExistingPin resolved every stored outside pin to find an alias of the new pin, about 2N+2 execs at once on SSH. Only an outside pin's folder is resolved, so an alias keeps its file name: resolve just those entries. Failed resolutions are now logged at debug level. _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --- src/node/services/pinnedArtifactFiles.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/node/services/pinnedArtifactFiles.ts b/src/node/services/pinnedArtifactFiles.ts index abefcb14945..5dd49fcca80 100644 --- a/src/node/services/pinnedArtifactFiles.ts +++ b/src/node/services/pinnedArtifactFiles.ts @@ -421,12 +421,22 @@ async function findExistingPin( storedPath: string ): Promise { if (pinned.includes(storedPath)) return storedPath; - const absolute = pinned.filter((p) => isAbsoluteOn(location, p)); + // Only an outside pin's folder is resolved, so an alias keeps the file name: resolve just the + // entries with that name (usually none). On SSH each resolution is one exec. + const paths = location.kind === "runtime" ? path.posix : path; + const name = paths.basename(storedPath); + const aliases = pinned.filter((p) => isAbsoluteOn(location, p) && paths.basename(p) === name); const real = await Promise.all( - absolute.map((p) => toStoredPinPath(location, p).catch(() => null)) + aliases.map((p) => + toStoredPinPath(location, p).catch((error: unknown) => { + // A failed resolution only means no match: the pin is added as a new entry. + log.debug("Could not resolve a stored pin", { error: getErrorMessage(error) }); + return null; + }) + ) ); const index = real.indexOf(storedPath); - return index < 0 ? null : absolute[index]; + return index < 0 ? null : aliases[index]; } export async function pinFile(