fix(config): make resource loading deterministic#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves OpenCode’s configuration/resource loading behavior (including deterministic glob ordering and project-root command discovery) and migrates the server stack from Bun-specific APIs to a Node-compatible Hono + WebSocket setup, along with supporting refactors/tests/docs.
Changes:
- Make glob scanning deterministic (sorted results) and add/adjust tests to assert stable ordering.
- Extend config loading to support project-root
commands//command/directories alongsideopencode.json, with plural-over-singular precedence and new collision tests. - Migrate server/OAuth/WebSocket/PTy plumbing away from
Bun.serveandhono/bun, add Node PTY abstraction, and update docs/dependencies accordingly.
Reviewed changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/content/docs/skills.mdx | Document skill duplicate precedence and logging fields. |
| packages/web/src/content/docs/providers.mdx | Update Cloudflare connect flow and add env-var alternative. |
| packages/web/src/content/docs/plugins.mdx | Document plugin directory locations and precedence. |
| packages/web/src/content/docs/modes.mdx | Document mode directory locations and precedence. |
| packages/web/src/content/docs/ecosystem.mdx | Add ecosystem entries. |
| packages/web/src/content/docs/config.mdx | Clarify plural vs legacy singular dir precedence. |
| packages/web/src/content/docs/commands.mdx | Document project-root commands directories near opencode.json. |
| packages/web/src/content/docs/agents.mdx | Document agent directory locations and precedence. |
| packages/plugin/src/index.ts | Extend plugin hook typing for chat.params to include maxOutputTokens. |
| packages/plugin/package.json | Bump OpenTUI peer/dev dependency versions. |
| packages/opencode/test/util/glob.test.ts | Assert deterministic glob ordering without local .sort(). |
| packages/opencode/test/tool/write.test.ts | Make permission assertion deterministic via temporary umask control. |
| packages/opencode/test/skill/skill.test.ts | Add skill duplicate precedence + warning log tests. |
| packages/opencode/test/session/llm.test.ts | Update fixture provider/model used in streaming test. |
| packages/opencode/test/lsp/index.test.ts | Add tests for TypeScript LSP spawn args and ignore-node-modules behavior. |
| packages/opencode/test/config/config.test.ts | Add tests for project-root commands loading and precedence collisions. |
| packages/opencode/src/util/glob.ts | Sort glob results in async and sync scans. |
| packages/opencode/src/skill/index.ts | Rename duplicate-skill log fields (shadowed/active). |
| packages/opencode/src/session/llm.ts | Thread maxOutputTokens through plugin hook output and into provider call. |
| packages/opencode/src/server/server.ts | Replace Bun.serve + bun WS with @hono/node-server + @hono/node-ws; adjust routing. |
| packages/opencode/src/server/routes/pty.ts | Switch WS upgrade to injected UpgradeWebSocket (non-bun). |
| packages/opencode/src/server/router.ts | Make workspace router middleware accept an injected WS upgrader. |
| packages/opencode/src/server/instance.ts | Update InstanceRoutes to accept WS upgrader and wire Pty routes accordingly. |
| packages/opencode/src/pty/pty.ts | Introduce runtime-agnostic PTY types. |
| packages/opencode/src/pty/pty.node.ts | Add Node PTY implementation via @lydell/node-pty. |
| packages/opencode/src/pty/pty.bun.ts | Add Bun PTY implementation via bun-pty. |
| packages/opencode/src/pty/index.ts | Swap direct bun-pty usage for #pty conditional import + subscriber key handling. |
| packages/opencode/src/provider/transform.ts | Adjust provider/model reasoning variants exclusions and Copilot Claude efforts mapping. |
| packages/opencode/src/plugin/index.ts | Make $ potentially unavailable outside Bun (guarded access). |
| packages/opencode/src/plugin/codex.ts | Migrate OAuth callback server from Bun.serve to Node http.createServer. |
| packages/opencode/src/mcp/oauth-callback.ts | Migrate MCP OAuth callback server from Bun.serve to Node http.createServer. |
| packages/opencode/src/lsp/server.ts | Add explicit TS server args and --ignore-node-modules when no ts/js config exists. |
| packages/opencode/src/config/config.ts | Add project-root command directory loading; enforce plural-over-singular scanning order; refactor loaders. |
| packages/opencode/src/cli/cmd/web.ts | Await async Server.listen. |
| packages/opencode/src/cli/cmd/serve.ts | Await async Server.listen. |
| packages/opencode/src/cli/cmd/acp.ts | Await async Server.listen. |
| packages/opencode/script/fix-node-pty.ts | Postinstall helper to fix node-pty spawn-helper exec bits. |
| packages/opencode/script/build-node.ts | Install Node PTY dep for node build; enable linked sourcemaps; handle bun linker layouts. |
| packages/opencode/package.json | Add #pty conditional export mapping; add fix-node-pty script; bump deps. |
| packages/app/script/e2e-local.ts | Await async server startup and tolerate optional stop signature. |
| package.json | Add postinstall hook for fix-node-pty; trust node-pty. |
| nix/hashes.json | Update Nix nodeModules hashes. |
| bun.lock | Lockfile updates for new/updated dependencies. |
| .gitignore | Ignore .agents compatibility directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const parsed = Agent.safeParse(config) | ||
| if (parsed.success) { | ||
| result[config.name] = { | ||
| ...parsed.data, | ||
| mode: "primary" as const, |
There was a problem hiding this comment.
If Agent.safeParse(config) fails for a mode markdown file, the code currently falls through silently (no error surfaced to the user / Session error bus, and no exception like commands/agents). This will make invalid mode files hard to debug; consider throwing InvalidError (or publishing a Session error) on parse failure to match the other loaders.
| $: Bun.$, | ||
| // @ts-expect-error | ||
| $: typeof Bun === "undefined" ? undefined : Bun.$, | ||
| } |
There was a problem hiding this comment.
PluginInput.$ is being set to undefined in non-Bun runtimes with a // @ts-expect-error. This is a runtime breaking change for plugins that assume $ is always available, and it also means the public plugin type definition is now inaccurate. Consider making $ optional in @opencode-ai/plugin types (or providing a Node-compatible shell implementation) so plugin authors can handle this safely without relying on ts-expect-error.
| const file = Log.file() | ||
| const prev = await fs.readFile(file, "utf8").catch(() => "") | ||
|
|
||
| await Instance.provide({ | ||
| directory: tmp.path, | ||
| fn: async () => { | ||
| const skill = await Skill.get("dup") | ||
| expect(skill?.description).toBe("from-opencode") | ||
| }, | ||
| }) | ||
|
|
||
| await Bun.sleep(20) | ||
| const next = await fs.readFile(file, "utf8").catch(() => "") | ||
| const tail = next.slice(prev.length) | ||
| expect(tail).toContain("duplicate skill name") | ||
| expect(tail).toContain("shadowed=") | ||
| expect(tail).toContain("active=") | ||
| expect(tail).toContain(path.join(".claude", "skills", "dup", "SKILL.md")) |
There was a problem hiding this comment.
This log assertion relies on Bun.sleep(20) to wait for async log file writes to flush. Log.write is asynchronous when print: false, so a fixed 20ms delay can be flaky under load/slow filesystems. Consider polling the file until the expected substring appears (with a timeout) or exposing/using a log flush hook for deterministic tests.
There was a problem hiding this comment.
Fixed — Bun.sleep(20) replaced with bounded poll loop in commit dc74ca3.
a0ef5e1 to
b1c6ec3
Compare
|
Fixed — loadMode() now throws InvalidError on Agent.safeParse failure (commit ebb9bd7), matching loadCommand and loadAgent. |
|
Stale — test/lsp/index.test.ts additions removed from this PR when narrowed to config-determinism changes only. |
|
Stale — script/fix-node-pty.ts removed from this PR when narrowed to config-determinism changes only. |
1 similar comment
|
Stale — script/fix-node-pty.ts removed from this PR when narrowed to config-determinism changes only. |
|
Fixed — unsupported project-root plugin loading claim removed from docs (commit 8ba9515). Plugins only loaded from config directories. |
|
Fixed — unsupported project-root agent loading claim removed from docs (commit 8ba9515). Agents only loaded from .opencode/, ~/.config/opencode/, and OPENCODE_CONFIG_DIR. |
|
Acknowledged. modes.mdx project-root loading claim already removed in commit 8ba9515. |
|
Fixed — unsupported project-root mode loading claim removed from docs (commit 8ba9515). Modes only loaded from config directories. |
|
Stale — src/plugin/index.ts removed from this PR when narrowed to config-determinism changes only. |
|
Fixed — Bun.sleep(20) replaced with bounded poll loop (commit dc74ca3). |
|
Fixed — Bun.sleep(20) replaced with bounded poll loop in commit dc74ca3. |
… sequential subdir scans
Sort Glob.scan/scanSync results for deterministic ordering. Split
brace expansion patterns ({command,commands}) into sequential subdir
scans to encode singular→plural precedence explicitly in loadCommand,
loadAgent, loadMode, and loadPlugin. Remove redundant .sort() calls in
glob tests since results are now sorted by default.
…al subdir scans
Split {tool,tools} and {skill,skills} brace expansion patterns in
tool/registry.ts and skill/index.ts into sequential subdir scans for
deterministic singular→plural precedence, consistent with command,
agent, and plugin loading.
Add unit tests for Shell.args (bash, zsh, nu, fish, cmd), Shell.ps, and glob result sort order to cover gaps found in production readiness review.
8ba9515 to
daf72e4
Compare
Issue for this PR
Closes anomalyco#18987
Type of change
What does this PR do?
This PR focuses strictly on config/resource loading determinism and precedence:
command/andcommands/directories next toopencode.jsonGlob.scan/scanSyncresultscommandbeforecommands, etc.)loadMode()now throwsInvalidErroron parse failure (matching other loaders)Bun.sleep(20)in skill warning test with bounded poll loopshadowed/activeCommits
ebb9bd7fix(config): surface invalid mode markdowndc74ca3test(skill): avoid fixed sleep for warning log8ba9515docs(config): remove unsupported project loader claimsHow did you verify your code works?
bun typecheckinpackages/opencodebun test test/config/config.test.ts test/skill/skill.test.ts test/util/glob.test.ts --timeout 30000Security notes
Checklist