Repository navigation
feat(core): showcase worktree tools and permission dependencies - #4
ivanmilevtues wants to merge 1 commit into
Conversation
Copy the four changed-file snapshots from anomalyco/opencode PR anomalyco#53981 for an architecture-diff demo against this fork's dev. This is not a runnable backport: the newer upstream tool plugin and worktree APIs are absent here. Upstream source: anomalyco#53981 Original author: James Long Amp-Thread-ID: https://ampcode.com/threads/T-01a13d0d-fbe1-76fa-8fad-21c3940a2399 Co-authored-by: Amp <amp@ampcode.com>
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
CodeBoarding reviewStatus: 2 changed components See the full change in CodeBoarding. graph LR
n_Core_Platform_Runtime["Core Platform #38; Runtime"]
n_Application_Client["Application Client"]
n_Management_Console["Management Console"]
n_Terminal_User_Interface["Terminal User Interface"]
n_Desktop_Shell["Desktop Shell"]
n_Infrastructure_Tooling["Infrastructure #38; Tooling"]
n_API_Protocol_Specifications["API Protocol Specifications"]
n_Effect_SQLite_Drivers["Effect SQLite Drivers"]
n_Web_Share_Viewer["Web Share Viewer"]
n_Enterprise_Portal["Enterprise Portal"]
n_Integrations_Automation["Integrations #38; Automation"]
n_Serverless_Functions["Serverless Functions"]
n_Core_Platform_Runtime -- "resolves locale utilities from" --> n_Management_Console
n_Desktop_Shell -- "detects native locale using" --> n_Application_Client
n_Infrastructure_Tooling -- "validates translation plurals against" --> n_Application_Client
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_Core_Platform_Runtime modified;
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbc7228936
ℹ️ 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".
| export const Plugin = { | ||
| id: "opencode.tools", | ||
| effect: Effect.fn("OpenCodeTools.Plugin")(function* (ctx: Context) { |
There was a problem hiding this comment.
Register the OpenCode tool bundle in production
The new Plugin is never imported or installed by any production bootstrap, so session_rename, session_move, the worktree operations, and models will not appear for users despite the new documentation. A repo-wide search at this commit finds only this declaration and tests, and those tests mask the omission by invoking OpenCodeTools.Plugin.effect(...) directly; register these built-ins through the production tool registration path.
AGENTS.md reference: packages/core/src/tool/AGENTS.md:L32-L32
Useful? React with 👍 / 👎.
|
|
||
| ### Shell | ||
|
|
||
| `shell` executes a command in the host user's shell. Set `workdir` instead of |
There was a problem hiding this comment.
Use the registered
bash tool and permission name
The command tool is registered as bash (packages/core/src/tool/bash.ts) and asserts the bash permission action, with no shell alias in the registry. Calling it shell here and in the permission guidance means users who copy the documented permission rule will not affect command execution and may receive unexpected prompts or denials.
Useful? React with 👍 / 👎.
| Set `background` to true for a dev server or another long-running process. | ||
| Background calls return immediately and notify the session when they finish; | ||
| they have no timeout unless one is supplied. Large output is shortened and the |
There was a problem hiding this comment.
Remove the unsupported background-command option
The documented command schema only accepts command, workdir, and timeout, and packages/core/src/tool/bash.ts explicitly leaves model-facing background launch as a future TODO. A user following this guidance will not get an immediately returning managed job; the command instead runs in the foreground and can block until the normal timeout.
Useful? React with 👍 / 👎.
| sections. | ||
|
|
||
| ```text | ||
| Use patch to rename src/old.ts to src/new.ts and update its exported name. |
There was a problem hiding this comment.
Stop recommending unsupported patch renames
This example asks the patch tool to rename a file, but packages/core/src/tool/apply-patch.ts explicitly rejects every update with a movePath as "moves are not supported yet." Following the documented workflow therefore deterministically fails instead of renaming the file; the example should use supported delete/add operations or avoid claiming move support.
Useful? React with 👍 / 👎.
| passes PNG, JPEG, GIF, WebP, and PDF files to the model, up to 20 MiB of source | ||
| data. Other binary files fail. See [Attachments](/attachments) for supported |
There was a problem hiding this comment.
Stop advertising PDF support in
read
The read implementation recognizes a PDF header or .pdf extension and immediately returns BinaryFileError in packages/core/src/tool/read-filesystem.ts; only PNG, JPEG, GIF, and WebP enter the media path. Users following this text will therefore be unable to pass any PDF to the model despite the explicit support claim.
Useful? React with 👍 / 👎.
| current directory unless `path` selects another relative directory; `hidden` | ||
| includes hidden entries and `limit` caps the results. |
There was a problem hiding this comment.
Remove the nonexistent
hidden glob option
The glob input schema contains only pattern, path, and limit, and its ripgrep invocation never reads a hidden value. Supplying the documented option is therefore rejected or ignored rather than enabling hidden entries, so callers cannot obtain the behavior promised here.
Useful? React with 👍 / 👎.
| Its `pattern` uses regular-expression syntax unless `literal` is true. Narrow a | ||
| search with `path`, `include`, `caseSensitive`, or `limit`. |
There was a problem hiding this comment.
Remove the nonexistent grep matching options
The grep schema and execution path accept only pattern, path, include, and limit; neither literal nor caseSensitive is implemented. In particular, a caller searching text containing regex metacharacters cannot make the pattern literal as documented, and supplying these fields is rejected or silently ineffective.
Useful? React with 👍 / 👎.
| ```text | ||
| Read package.json and README.md in parallel, then return their relevant setup details. |
There was a problem hiding this comment.
Limit the execute example to tools actually in its catalog
The execute implementation builds its sandbox catalog exclusively from MCP.Service.tools() in packages/opencode/src/tool/code-mode.ts; it does not expose built-ins such as read. Consequently, the documented example that reads two repository files in parallel cannot run in Code Mode unless an unrelated MCP server happens to provide such a tool.
Useful? React with 👍 / 👎.
Architecture diff showcase
Copies the four changed-file snapshots from anomalyco#53981 (original author: James Long). This branch starts directly from this fork's dev, independently of the provider demo.
What to highlight
Demo-only compatibility limits
This is a source snapshot demo, not a clean cherry-pick or runnable backport. The fork lacks the upstream tool-plugin module and newer service APIs. Consequently the diff includes the full upstream module and test fixture/documentation changes relative to this fork, not just the original upstream PR hunks. Missing dependencies and registration are not ported. Do not merge as a production feature.
Verification
All three copied TypeScript files pass Bun syntax transpilation. The four files are unchanged upstream snapshots. Runtime tests/typechecking are not claimed to pass: prerequisite APIs are missing. CodeBoarding's generated review is the showcase output to inspect.