fix: resolve symlinks in Instance cache to prevent duplicate contexts#16650
fix: resolve symlinks in Instance cache to prevent duplicate contexts#16650jmylchreest wants to merge 4 commits intoanomalyco:devfrom
Conversation
Add configurable cleanup module that runs on startup to manage database size and remove orphaned storage files. Fixes log cleanup bug where non-deterministic glob ordering could delete newest files instead of oldest. Changes: - New cleanup config stanza with session retention, storage sweep, and vacuum options - Fix log cleanup: sort by filename before slicing, align guard with slice count - Sweep orphaned storage files (session, message, part, todo, session_diff, project, snapshot) - Prune empty directories after sweep - Run SQLite VACUUM and WAL checkpoint on startup - Fix pre-existing path.sep bug in Storage.list() for cross-platform correctness - Defer cleanup 500ms and yield every 100 deletions to avoid blocking TUI
The `@opencode-ai/script` import runs top-level shell commands that leave handles open, preventing the process from exiting naturally after Bun.build completes.
When opencode runs from a symlinked directory, Instance.provide() could create two separate contexts for the same physical directory because path.resolve() does not resolve symlinks. This caused Bus event isolation: events published on one Instance never reached subscribers on the other, resulting in a completely blank TUI after sending prompts. Add fs.realpathSync() to canonicalize paths before using them as Instance cache keys. Also harden the sync() merge logic so that fetched data is merged with any event-delivered data rather than overwriting it. Closes anomalyco#16648
|
The following comment was made by an LLM, it may be inaccurate: I found a potentially related PR: PR #15483: "fix: symlink path resolution causing duplicate instances (#15482)" Why it might be related:
You may want to check if #15483 was closed/rejected in favor of this new approach, or if they complement each other. |
|
Closing — this PR accidentally included unrelated commits from a local merge branch. Reopening as a clean PR from a branch based on dev. |
| async provide<R>(input: { directory: string; init?: () => Promise<any>; fn: () => R }): Promise<R> { | ||
| const directory = Filesystem.resolve(input.directory) | ||
| // Resolve symlinks so the same physical directory always maps to one Instance. | ||
| const directory = Filesystem.resolve(fs.realpathSync(input.directory)) |
There was a problem hiding this comment.
fs.realpathSync() throws ENOENT if the path doesn't exist. Filesystem.resolve() (via path.resolve()) previously handled non-existent paths gracefully.
This is a behavioral change that could surface as an unhandled exception in edge cases (e.g., a directory removed between CLI invocation and Instance.provide()).
Consider guarding:
| const directory = Filesystem.resolve(fs.realpathSync(input.directory)) | |
| const directory = Filesystem.resolve(fs.existsSync(input.directory) ? fs.realpathSync(input.directory) : input.directory) |
Or wrapping in try/catch to fall back to the unresolved path, matching the pattern already used in
normalizePath():
function realpath(p: string): string {
try {
return fs.realpathSync(p)
} catch {
return p
}
}| }, | ||
| async reload(input: { directory: string; init?: () => Promise<any>; project?: Project.Info; worktree?: string }) { | ||
| const directory = Filesystem.resolve(input.directory) | ||
| const directory = Filesystem.resolve(fs.realpathSync(input.directory)) |
There was a problem hiding this comment.
Same realpathSync throw risk as provide() above — should use the same guard pattern.
| const directory = Filesystem.resolve(fs.realpathSync(input.directory)) | |
| const directory = Filesystem.resolve(fs.existsSync(input.directory) ? fs.realpathSync(input.directory) : input.directory) |
| import { iife } from "@/util/iife" | ||
| import { GlobalBus } from "@/bus/global" | ||
| import { Filesystem } from "@/util/filesystem" | ||
| import fs from "fs" |
There was a problem hiding this comment.
Since symlink resolution is now an explicit concern of Instance, it might be cleaner to push this into Filesystem.resolve() itself — it already handles Windows path translation and normalizePath. That way all callers get symlink resolution, not just provide() and reload().
Something like:
// filesystem.ts
export function resolve(p: string): string {
const resolved = pathResolve(windowsPath(p))
try {
return normalizePath(realpathSync(resolved))
} catch {
return normalizePath(resolved)
}
}This would also cover server.ts middleware and thread.ts, which both call Filesystem.resolve() independently and could theoretically still produce non-canonical paths.
| export const Instance = { | ||
| async provide<R>(input: { directory: string; init?: () => Promise<any>; fn: () => R }): Promise<R> { | ||
| const directory = Filesystem.resolve(input.directory) | ||
| // Resolve symlinks so the same physical directory always maps to one Instance. |
There was a problem hiding this comment.
This fix has no test coverage. A symlink-specific test for Instance.provide() would prevent regressions — e.g.:
test("preserves single instance for symlinked directory", async () => {
if (process.platform === "win32") return
await using tmp = await tmpdir({ git: true, config: { model: "test/model" } })
const link = tmp.path + "-symlink"
await fs.symlink(tmp.path, link)
try {
await Instance.provide({
directory: link,
fn: async () => {
const config = await Config.get()
expect(config.model).toBe("test/model")
},
})
} finally {
await fs.unlink(link).catch(() => {})
}
})
2 participants
Issue for this PR
Closes #16648
Fixes #16647
Fixes #15482
Related: #16522
Type of change
What does this PR do?
When opencode runs from a symlinked directory,
Instance.provide()creates duplicate contexts for the same physical directory.Filesystem.resolve()usespath.resolve()which normalizes./..segments but does not resolve symlinks. Two different path strings for the same physical directory produce two separate Instance entries in the cache.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.Changes:
instance.ts: Addfs.realpathSync()beforeFilesystem.resolve()in bothprovide()andreload()so symlinks resolve to canonical paths before cache lookup.sync.tsx: Harden thesync()merge logic so fetched session data is merged with any event-delivered data rather than overwriting it. This prevents a race where real-time events arriving during the async fetch window get discarded.prompt/index.tsx: Updated comment on the 50ms navigation delay to reflect the current understanding.How did you verify your code works?
~/src-office(a symlink to~/dtkr4-cnjjf/...)-c) continues to work correctlybun tsc --noEmit— no type errorsChecklist