fix: resolve symlinks in Instance cache to prevent duplicate contexts#16651
fix: resolve symlinks in Instance cache to prevent duplicate contexts#16651jmylchreest wants to merge 2 commits intoanomalyco:devfrom
Conversation
|
The following comment was made by an LLM, it may be inaccurate: I found one potentially related PR: PR #15483: "fix: symlink path resolution causing duplicate instances (#15482)"
Note: The current PR (#16651) explicitly mentions this is complementary to #16641 and references issues #16647 and #15482. PR #15483 appears to be a prior attempt at fixing the same symlink/duplicate instance problem. You may want to review if #15483 was merged or closed, and whether the current PR represents an improved or different solution. |
3684976 to
f4f02cc
Compare
|
Thank you for jumping on this @jmylchreest |
f4f02cc to
790f4c6
Compare
|
Updated to move this logic into resolve(), it's working from what I can see in all my local tests, plus the test condition I think covers it. I've not tested under WSL, I can't, nor MacOS directly - but I trust they're OK. |
…stance contexts Filesystem.resolve() used path.resolve() which normalizes path segments but does not resolve symlinks. When opencode runs from a symlinked directory, Instance.provide() could create duplicate contexts for the same physical directory, causing Bus event isolation and a blank TUI. Move symlink resolution (realpathSync) into Filesystem.resolve() so all callers benefit. Falls back to the unresolved path if the target does not exist, matching the guard pattern in normalizePath(). Fixes anomalyco#16647 Fixes anomalyco#15482
790f4c6 to
3903347
Compare
|
I'm validating on MacOS right now and I'll post here in a moment with results. |
|
Confirming this resolves the issue on MacOS and the full test suite passes 🎉 |
|
#16659 I think makes sense to add, I'll push an update to the branch to test now. I personally think 16660 is a separate issue. |
The bare catch swallowed all realpathSync errors (EACCES, ELOOP, ENOTDIR), silently falling back to the unresolved path. This could re-introduce duplicate Instance contexts for broken symlinks. Now only ENOENT is caught (path doesn't exist yet); all other errors propagate to the caller. Fixes anomalyco#16659
Instance.containsPath() compared Instance.directory (always canonical after anomalyco#16651) against an uncanonicalized filepath argument. When callers passed a symlinked path, the lexical comparison failed even though the path resolved to a location inside the project. This caused false negatives in bash.ts, external-directory.ts, and file/index.ts — triggering unnecessary external_directory permission prompts or rejecting valid file reads via symlinked paths. Fix: resolve the filepath through Filesystem.resolve() before comparing, so both sides use canonical paths. Adds tests for: symlinks inside project, external symlinks to project, symlinks escaping project, dangling symlinks, and symlink cycles. Fixes anomalyco#16660
Instance.containsPath() compared Instance.directory (always canonical after anomalyco#16651) against an uncanonicalized filepath argument. When callers passed a symlinked path, the lexical comparison failed even though the path resolved to a location inside the project. This caused false negatives in bash.ts, external-directory.ts, and file/index.ts — triggering unnecessary external_directory permission prompts or rejecting valid file reads via symlinked paths. Fix: resolve the filepath through Filesystem.resolve() before comparing, so both sides use canonical paths. Adds tests for: symlinks inside project, external symlinks to project, symlinks escaping project, dangling symlinks, and symlink cycles. Fixes anomalyco#16660
…omalyco#16651 Tests for external symlinks, symlinks escaping project, and ELOOP propagation probe at runtime whether Filesystem.resolve() follows symlinks. They skip gracefully on dev (before anomalyco#16651) and activate once realpathSync lands.
Issue for this PR
Fixes #16647
Fixes #15482
Fixes #16659
Related: #16522, #16641
Type of change
What does this PR do?
Filesystem.resolve()usedpath.resolve()which normalizes./..segments but does not resolve symlinks. When opencode runs from a symlinked directory,Instance.provide()creates duplicate contexts for the same physical directory because two different path strings produce two separate cache entries.Because the
Buspub/sub system is Instance-scoped (viaInstance.state()), events published on one Instance (where the LLM session runs) are never received by subscribers on the other Instance (where the SSE endpoint listens). This causes a completely blank TUI after sending prompts.Fix: Move symlink resolution (
realpathSync) intoFilesystem.resolve()so all callers benefit — not justInstance.provide()andInstance.reload(). Falls back to the unresolved path only forENOENT(path doesn't exist yet); all other errors (EACCES,ELOOP,ENOTDIR) are rethrown to avoid silently masking broken paths.This is complementary to #16641 which canonicalizes at the caller (thread.ts) level.
How did you verify your code works?
~/src-office(a symlink to~/dtkr4-cnjjf/...)bun test test/util/filesystem.test.ts— 55 tests pass (including 5 new)bun tsc --noEmit— no type errorsChecklist