Repository navigation
fix(storage): resolve a Home behind a symlink the same way before it exists - #210
Conversation
The first create in a Home that does not exist yet and sits behind a symlink exits 6 with HOST_UNREACHABLE, while its session keeps running. These tests fail on main: the CLI and the host compute different paths for the same Home, and home forget misses a removed Home by the symlinked path it was created through. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…exists The CLI resolved a new Home before create made it, so realpath failed and it kept the symlinked path. The host resolved the same Home after create made it and got the real path. The two hashed different paths for /tmp/agent-tty/<hash>, so create dialed a socket that did not exist and exited 6 with HOST_UNREACHABLE while the session ran. canonicalHomePath resolves the deepest existing ancestor and appends the parts that do not exist yet. resolveHome, validateHomePath and normalizeHomePath all use it, so every process derives one Home, one socket path and one registry key whether or not the Home exists. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
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. |
|
Codex Review: Didn't find any major issues. Swish! 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 ReviewSecurity 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. |
…me a symlink A Home registered under a path whose parent later became a symlink can no longer be forgotten: forget and home forget resolve the path through the new symlink and miss the stored entry, and gc reports the Home deregistered on every run while homes.json keeps it. These tests pass on main and fail on 84f56e1. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
normalizeHomePath now follows symlinks in the existing ancestors of a removed Home. When an ancestor of a stored path became a symlink, the canonical key no longer equaled the stored entry, so forget, home forget and gc's deregistration left the entry in homes.json. forget now also matches the path as written, and home forget passes it the path as written. The canonicalHomePath doc now says "deepest ancestor that resolves", because it also walks past an ancestor that exists but fails realpath. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Pick up #208 so quality-gates-macos runs the tests on this branch. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08950300db
ℹ️ 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".
…e wrong path When a registry holds a stale entry and the entry it now resolves to, forget of the stale path removes both. home forget then reports the canonical path instead of the entry it removed. Both tests fail on 0895030. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
forget matched both the path as written and its canonical form, so with a stale entry and the entry it now resolves to, it removed both. It now removes the entry stored under the path as written when there is one, and else the canonical entry. forget returns the removed path, and home forget reports that path instead of the canonical one. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Hooray! 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 ReviewSecurity 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. |
Why
The first
createin a new Home reached through a symlink must succeed, andcreatemust not report a failure for a session that runs. Today that firstcreateexits 6 withHOST_UNREACHABLE, and the session it reports as failed keeps running. The CLI resolves the Home beforecreatemakes it and keeps the symlinked path. The host resolves the Home aftercreatemade it and gets the real path. So the two hash different/tmp/agent-tty/<hash>folders, and the CLI dials a socket that does not exist.What changed
canonicalHomePathtakes the real path of the deepest ancestor of a Home that resolves, then appends the parts below it. It gives the same path before and after the Home exists.resolveHome, the CLI check of--homeandAGENT_TTY_HOME, and the registry'snormalizeHomePathall use it. Sohome forgetnow also finds a removed Home that you name through a symlink.forgetremoves the entry stored under the path as written, or else the canonical one, and returns the path it removed. Sohome forgetandgcstill remove an entry after an ancestor of its stored path became a symlink, andhome forgetreports that entry.Scope
--home,AGENT_TTY_HOMEand the default~/.agent-ttyunder a symlinked$HOME.createfor another reason, such as a slow start.createstill reports failure and leaves that session running. Issue create: a host that misses the readiness window keeps running after create reports failure #209 tracks it.Blast Radius
Every command resolves its Home through this function. A Home that exists resolves to the same real path as before. Only a Home that does not exist yet changes. It now follows symlinks in its existing ancestors. CLI JSON and error codes do not change.
Verification
createexits 6 and both sessions run. On this branch bothcreatecalls exit 0, both sessions run, anddestroyleaves no host. A symlinked$HOMEwith the default Home gives the same before and after.createin a new Home behind a symlink,resolveHomebefore and after the Home exists, andforgetthrough a symlink. The second coversforget,home forgetandgcafter a stored path's parent became a symlink, and those pass on main too. The third covers a stale entry next to the entry it now resolves to.npm run verifystep exits 0 locally when the tests run in CI's layout. Under the load of a fully parallelnpm run verify, the export-fixtures color-grid screenshot test and the backend-selection semantic snapshot test can time out. Both also fail that way on main. Theverify-session-lifecyclecheck exits 0, because no covered code changed.Fixes #206
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh