Repository navigation
Conversation
📝 WalkthroughWalkthroughOpenCode installation now deploys skills, MCP configuration, and a TypeScript augmentation plugin. The plugin adds tool-result context and a session reminder. Uninstallation removes the skills and plugin. Tests cover creation, updates, escaping, removal, and skills content. ChangesOpenCode lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/cli.c`:
- Around line 1836-1840: Update the plugin removal logic to call
cbm_unlink(plugin_path) directly instead of checking stat(), so dangling
symlinks are removed. Suppress only ENOENT as a successful absence; propagate or
return errors for all other unlink failures.
- Around line 551-560: The hook-augment subprocess flow must not wait
indefinitely or leak asynchronous stdin errors. Update the generated code around
spawn and the child close/error promise to attach a child.stdin error listener,
add a bounded timeout that kills the child if it hangs, and clear the timeout
when the child exits through either close or error.
- Around line 1812-1822: Update the plugin creation helper around the shown
file-write flow to check fchmod() and fclose(), returning CLI_ERR when either
fails; ensure cleanup is preserved. In src/cli/cli.c lines 3332-3336, check
cbm_upsert_opencode_plugin() and only print the installed-plugin success message
on success, otherwise propagate or report the failure. In src/cli/cli.c lines
3839-3842, apply the same handling to cbm_remove_opencode_plugin() before
printing the removal success message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cea285a7-efec-41e8-a872-2e80f6539383
📒 Files selected for processing (3)
src/cli/cli.csrc/cli/cli.htests/test_cli.c
| " const child = spawn(BIN, [\"hook-augment\"], " | ||
| "{ stdio: [\"pipe\", \"pipe\", \"ignore\"] })\n" | ||
| " child.stdin.write(payload)\n" | ||
| " child.stdin.end()\n" | ||
| " let stdout = \"\"\n" | ||
| " child.stdout.on(\"data\", (d: Buffer) => { stdout += d.toString() })\n" | ||
| " await new Promise<void>((resolve) => {\n" | ||
| " child.on(\"close\", () => resolve())\n" | ||
| " child.on(\"error\", () => resolve())\n" | ||
| " })\n" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound and safely handle the augmentation subprocess.
A hung hook-augment child makes the OpenCode grep/glob hook await forever. Also, child.stdin can emit an asynchronous error that the surrounding try/catch will not catch. Add a timeout that kills the child and attach an stdin error listener.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/cli.c` around lines 551 - 560, The hook-augment subprocess flow must
not wait indefinitely or leak asynchronous stdin errors. Update the generated
code around spawn and the child close/error promise to attach a child.stdin
error listener, add a bounded timeout that kills the child if it hangs, and
clear the timeout when the child exits through either close or error.
| if (fprintf(f, opencode_plugin_content, escaped_binary_path) < 0) { | ||
| (void)fclose(f); | ||
| return CLI_ERR; | ||
| } | ||
| #ifndef _WIN32 | ||
| fchmod(fileno(f), CLI_OCTAL_PERM); | ||
| #endif | ||
| (void)fclose(f); | ||
| #ifdef _WIN32 | ||
| chmod(plugin_path, CLI_OCTAL_PERM); | ||
| #endif |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not report plugin lifecycle operations as successful when they fail.
fclose() can surface a buffered write failure, while fchmod() can fail; both are ignored. The install and uninstall dispatch then discard helper return codes and print success regardless.
src/cli/cli.c#L1812-L1822: checkfchmod()andfclose()and returnCLI_ERRon failure.src/cli/cli.c#L3332-L3336: checkcbm_upsert_opencode_plugin()before printing the installed-plugin message and propagate/report failure.src/cli/cli.c#L3839-L3842: checkcbm_remove_opencode_plugin()before printing the removal message and propagate/report failure.
📍 Affects 1 file
src/cli/cli.c#L1812-L1822(this comment)src/cli/cli.c#L3332-L3336src/cli/cli.c#L3839-L3842
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/cli.c` around lines 1812 - 1822, Update the plugin creation helper
around the shown file-write flow to check fchmod() and fclose(), returning
CLI_ERR when either fails; ensure cleanup is preserved. In src/cli/cli.c lines
3332-3336, check cbm_upsert_opencode_plugin() and only print the
installed-plugin success message on success, otherwise propagate or report the
failure. In src/cli/cli.c lines 3839-3842, apply the same handling to
cbm_remove_opencode_plugin() before printing the removal success message.
| struct stat st; | ||
| if (stat(plugin_path, &st) == 0) { | ||
| return cbm_unlink(plugin_path); | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the plugin path even when it is a dangling symlink.
stat() follows links, so a dangling cbm-augment.ts returns ENOENT and is left behind while uninstall reports success. Call cbm_unlink() directly and suppress only ENOENT; return an error for other failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/cli.c` around lines 1836 - 1840, Update the plugin removal logic to
call cbm_unlink(plugin_path) directly instead of checking stat(), so dangling
symlinks are removed. Suppress only ENOENT as a successful absence; propagate or
return errors for all other unlink failures.
There was a problem hiding this comment.
Pull request overview
This PR extends the CLI’s OpenCode integration by adding an auto-discovered TypeScript plugin (for grep/glob augmentation + session reminders) and wiring OpenCode skills install/remove into the install/uninstall flows, with accompanying unit tests.
Changes:
- Add OpenCode TypeScript plugin template and install/remove helpers (
cbm_upsert_opencode_plugin,cbm_remove_opencode_plugin) with JSON escaping for embedded binary paths. - Integrate OpenCode plugin + skills into install/uninstall and install-plan recording.
- Add unit tests covering plugin upsert/remove and OpenCode skills install/remove.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_cli.c | Adds tests for OpenCode plugin creation/idempotence/path-escaping and skills install/remove. |
| src/cli/cli.h | Exposes new OpenCode plugin install/remove function prototypes. |
| src/cli/cli.c | Implements OpenCode plugin content + install/remove logic and integrates plugin/skills into install/uninstall flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!g_install_plan) { | ||
| int skill_count = cbm_install_skills(skills_dir, true, dry_run); | ||
| printf(" skills: %d installed\n", skill_count); | ||
|
|
| #ifndef _WIN32 | ||
| fchmod(fileno(f), CLI_OCTAL_PERM); | ||
| #endif | ||
| (void)fclose(f); | ||
| #ifdef _WIN32 | ||
| chmod(plugin_path, CLI_OCTAL_PERM); | ||
| #endif |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14f3168029
ℹ️ 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".
| " const payload = JSON.stringify({ tool_name: toolName, " | ||
| "tool_input: { pattern } })\n" |
There was a problem hiding this comment.
On Windows, cbm_cmd_hook_augment returns before querying unless the hook JSON contains a usable cwd (src/cli/hook_augment.c:315-334), but this OpenCode payload only sends tool_name and tool_input. As a result, every OpenCode grep/glob augmentation on Windows silently produces no context even though the installer reports the plugin as active; include the plugin context directory/cwd in the payload or spawn the child with that cwd.
Useful? React with 👍 / 👎.
| cbm_upsert_opencode_mcp); | ||
|
|
||
| if (!g_install_plan) { | ||
| int skill_count = cbm_install_skills(skills_dir, true, dry_run); |
There was a problem hiding this comment.
This always passes true for force, so a normal OpenCode install/update rewrites an existing ~/.config/opencode/skills/codebase-memory/SKILL.md even when the user did not request --force. The Claude path preserves existing skills unless force is set, and OpenCode should follow the same behavior to avoid clobbering local edits on routine installs.
Useful? React with 👍 / 👎.
Motivation
grep/globtool output and injects a session reminder, and ship bundled Agent SKILL files for OpenCode installs.Description
opencode_plugin_contentthat wirestool.execute.after(forgrep/globaugmentation) andexperimental.chat.system.transform(session reminder) and embeds the CLI binary path.json_escape_stringand new functionscbm_upsert_opencode_pluginandcbm_remove_opencode_pluginto write/remove~/.config/opencode/plugins/cbm-augment.ts, honoringdry_runand setting file permissions (fchmod/chmod) similarly to other install helpers.g_install_plan, createskillsdirectory and callcbm_install_skills, and remove skills + plugin during uninstall; addCMM_OPENCODE_PLUGIN_NAMEconstant.cli.hforcbm_upsert_opencode_pluginandcbm_remove_opencode_pluginand update related install/uninstall logic to print summary messages.Testing
tests/test_cli.ccoveringcbm_upsert_opencode_plugin(fresh, idempotent, path escaping),cbm_remove_opencode_plugin, andcbm_install_skills/cbm_remove_skillsbehavior, and all new tests passed locally.Codex Task
Summary by CodeRabbit