fix(filesystem): resolve symlink/junction targets to directory type in readDirectoryEntries#28529
Closed
danielxxomg wants to merge 3 commits into
Closed
fix(filesystem): resolve symlink/junction targets to directory type in readDirectoryEntries#28529danielxxomg wants to merge 3 commits into
danielxxomg wants to merge 3 commits into
Conversation
When a user changes a provider in opencode.json, ALL MCPs restart and re-activate — including ones explicitly disabled via /mcps toggle. The disabled state was only in-memory (s.status[name]) and lost on any Instance reload. This persists disabled MCP IDs to Global.Path.state/mcp-state.json using the same AppFileSystem pattern as model.json. MCP.state() now reads this file during init and respects previously toggled disabled servers. Changes: - src/mcp/index.ts: readDisabledState/writeDisabledState helpers, wiring in state()/connect()/disconnect(), uses Effect.catchCause for defect safety - test/fixture/fixture.ts: add AppFileSystem.defaultLayer to test layer chain - test/mcp/lifecycle.test.ts: 7 persistence tests covering all spec scenarios Tests: 28/28 pass. Spec compliance: 8/8 scenarios. Closes anomalyco#28428
NotFound is expected on first run — silence it. Still warn on actual errors like corrupt JSON or permission denied.
…n readDirectoryEntries When isSymbolicLink() is true (Linux symlinks, Windows junction points like OneDrive Desktop), stat() the target to determine if it points to a directory. Previously these entries were classified as 'symlink' and silently filtered out by downstream code (file/index.ts), making symlinked directories invisible in the project picker, @file picker, and file listing on ALL platforms. This fixes the root cause instead of patching individual filter sites. Closes anomalyco#28526, also resolves anomalyco#10365 and anomalyco#16342.
Contributor
|
The following comment was made by an LLM, it may be inaccurate: I found one potentially related PR: Related PR:
No other open PRs with identical scope were found. The current PR (28529) appears to be the active fix for symlink/junction directory visibility issues. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds persistence for disabled MCP server IDs via a mcp-state.json file and validates the behavior through new lifecycle tests.
Changes:
- Persist disabled MCP server IDs on disconnect and restore them on init; remove them on connect.
- Add test coverage for persistence behaviors (missing/corrupt state file, precedence rules, stale IDs).
- Update filesystem directory listing to classify symlinks that point to directories as
directory.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/opencode/src/mcp/index.ts | Implements reading/writing mcp-state.json and integrates persisted disabled IDs into MCP init/connect/disconnect flows. |
| packages/opencode/test/mcp/lifecycle.test.ts | Adds tests covering persistence and precedence behavior around disabled MCP server IDs. |
| packages/opencode/test/fixture/fixture.ts | Provides AppFileSystem layer to tests so persistence tests can read/write state files. |
| packages/core/src/filesystem.ts | Adjusts readdir to resolve symlink targets and classify symlink-to-directory entries as directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
90
| return await Promise.all( | ||
| entries.map(async (e): Promise<DirEntry> => { | ||
| let type: DirEntry["type"] | ||
| if (e.isDirectory()) { | ||
| type = "directory" | ||
| } else if (e.isSymbolicLink()) { | ||
| try { | ||
| const target = await NFS.stat(join(dirPath, e.name)) | ||
| type = target.isDirectory() ? "directory" : "symlink" | ||
| } catch { | ||
| type = "symlink" | ||
| } | ||
| } else if (e.isFile()) { | ||
| type = "file" | ||
| } else { | ||
| type = "other" | ||
| } | ||
| return { name: e.name, type } | ||
| }), | ||
| ) |
Comment on lines
+286
to
+292
| Effect.catchCause((cause) => { | ||
| const err = Cause.squash(cause) | ||
| if (err && typeof err === "object" && "_tag" in err && err._tag === "NotFound") | ||
| return Effect.succeed(new Set<string>()) | ||
| log.warn("failed to read mcp-state.json", { cause: err }) | ||
| return Effect.succeed(new Set<string>()) | ||
| }), |
Comment on lines
+891
to
+916
| it.instance( | ||
| "disconnect persists disabled ID to mcp-state.json", | ||
| () => | ||
| MCP.Service.use((mcp: MCPNS.Interface) => | ||
| Effect.gen(function* () { | ||
| lastCreatedClientName = "persist-disc-server" | ||
| getOrCreateClientState("persist-disc-server") | ||
|
|
||
| yield* mcp.add("persist-disc-server", { | ||
| type: "local", | ||
| command: ["echo", "test"], | ||
| }) | ||
| yield* mcp.disconnect("persist-disc-server") | ||
|
|
||
| const fs = yield* AppFileSystem.Service | ||
| const data = yield* fs | ||
| .readJson(path.join(Global.Path.state, "mcp-state.json")) | ||
| .pipe(Effect.catch(() => Effect.succeed(null))) | ||
| expect(data).not.toBeNull() | ||
| const arr = (data as any).disabledMcpServerIds | ||
| expect(Array.isArray(arr)).toBe(true) | ||
| expect(arr).toContain("persist-disc-server") | ||
| }), | ||
| ), | ||
| { config: { mcp: {} } }, | ||
| ) |
danielxxomg
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #28526
Type of change
What does this PR do?
Symlinked directories (Linux ln -s) and Windows junction points (e.g., OneDrive Desktop) are invisible in the directory picker, @file picker, and file listing because readDirectoryEntries classifies them as "symlink" and downstream filters skip non-directory types.
This fixes the root cause in packages/core/src/filesystem.ts: when isSymbolicLink() is true, stat() the target and return "directory" if the target is a directory. This fixes ALL consumers at once — no downstream filter changes needed.
Also resolves:
How did you verify your code works?
bun test test/file/ — 87 pass, 1 skip, 0 fail. No regressions.
Screenshots / recordings
N/A
Checklist