Repository navigation
🤖 fix: pinned files outside the checkout store real folder paths and read at the filesystem root - #7916
Conversation
…kout 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`_
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`_
… comments 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`_
…and drive-letter entries 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`_
…m root 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`_
_Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
# Conflicts: # src/node/services/pinnedArtifactFiles.test.ts # src/node/services/pinnedArtifactFiles.ts
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24b1532ee4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 47.1k tokens (23% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 23s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 7.9k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 20.2s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 5.8s · 2 targets (web, phone) · run artifacts |
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`_
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
… checkout 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`_
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 348fa24ab6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…entry 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`_
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…#7918) ## Summary Follow-up to coder#7916. When the user pins a new file, `findExistingPin` looks for a stored alias of it. It resolved the folder of every stored outside pin to do that. On an SSH or Docker checkout each resolution is one remote command, and they ran at once: about 2N+2 commands for N stored outside pins, which can hit an SSH server's session limit. Now it resolves only the stored entries with the same file name (usually none), and it logs a failed resolution at debug level instead of dropping the error. ## Background The independent review of coder#7916 found the cost and the silent `.catch(() => null)`. coder#7916 had used 5 Codex rounds, so the fix ships here instead of as a sixth push there. ## Implementation An outside pin stores its folder's real path plus the file name as given (coder#7916), so an alias of the same file always has the same file name. Filtering by name keeps every alias match and skips every other entry. A failed resolution still means "no match", and the pin is added as a new entry, as before. ## Validation The first commit adds a failing test. It pins a new file on an SSH checkout (a `LocalRuntime` stands in for the host) with three unrelated stored outside pins, and counts the folder resolutions. Before the fix the three stored folders were resolved. After it, none are. The alias test from coder#7916 still passes. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$15.82`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=15.82 -->
Summary
Follow-up to #7915 (pinned files outside the workspace checkout). It fixes the edge cases that the review of #7915 found: one file reached through a symlinked folder listed as two pins, SSH and Docker refused a file in a symlinked folder, a file directly under
/could not be pinned, and a hand-edited drive-letter entry on a POSIX host read below the backend's working folder.Implementation
pinnedArtifactFiles.ts: a pin outside the checkout stores the file name in its folder's real path. Both spellings of one file are one pin. A symlinked folder that leads into the checkout gives a checkout-relative pin. The runtime read script refuses a symlinked root folder, so storing the real folder also makes those files readable on SSH and Docker.artifactRuntimeStore.ts:resolveRealDirOnRuntimeresolves a folder withcd -Pandpwd -Pbehind a magic header (login banners can come first). The read script's containment checks use${xum_root%/}/, and its root check handles/as the folder, so a pinned file directly under/reads.artifactStore.ts: the host reader's two containment checks accept/as the folder (dirPrefix).isAbsoluteOn), so a hand-editedC:/x.mdon a POSIX host is invalid.Not changed, by design: a downgrade to a version before #7915 drops outside pins (the older reader filters invalid entries). Nothing breaks.
Validation
The first commit adds the repros. Before the fix they failed: the symlinked-folder pin test, the drive-letter test, and the filesystem-root read (runtime and host). With the fix,
pinnedArtifactFiles.test.tsandartifactRuntimeStore.test.tspass 46/46 locally.Risks
Low. The runtime read script change keeps every artifacts-folder check: the
artifactsname check still needs the real parent plusartifacts, and${xum_root%/}/equals$xum_root/for every root except/.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$4.48