feat(opencli): expose bundled site adapters to browser tools - #1272
Conversation
|
Warning Review limit reached
More reviews will be available in 3 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughAdds OpenCLI runtime discovery/packaging, adapter registry and search, adapter runner with CDP-backed page fallbacks, two desktop-only tools (opencli_search, opencli_run) wired into deferred tool_info, permission mapping for opencli tools, and extensive tests including built-artifact validation. ChangesOpenCLI Adapter Integration
Sequence Diagram(s)sequenceDiagram
participant Client
participant Registry
participant AdapterRunner
participant Page
participant Adapter
Client->>Registry: openCliCommand(name) / searchOpenCliCommands(query)
Registry-->>Client: CliCommand / search results
Client->>AdapterRunner: runOpenCliAdapterCommand(cmd, page?, args)
AdapterRunner->>AdapterRunner: prepareOpenCliCommandArgs(args)
alt navigateBefore
AdapterRunner->>Page: navigate(url)
end
alt browser mode
AdapterRunner->>AdapterRunner: createOpenCliAdapterPage (CDP shims if needed)
AdapterRunner->>Adapter: cmd.func(page, args) or pipeline
else non-browser mode
AdapterRunner->>Adapter: cmd.func(null, args)
end
Adapter-->>AdapterRunner: result
AdapterRunner-->>Client: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request integrates OpenCLI site adapters into the PawWork desktop-electron and opencode packages, introducing the opencli_search and opencli_run tools along with their registry, runner, and testing infrastructure. The review feedback highlights several important improvements: refining commandPermissionPatterns to prevent overly restrictive browser permissions when navigateBefore is set, resolving a defaulting discrepancy for the browser property in search scoring, preventing empty strings from silently coercing to 0 for numeric arguments, and enriching the search tool's output with detailed argument metadata (types, choices, defaults, and help text) to ensure the model can successfully execute commands.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/opencode/src/opencli/adapter-registry.ts (1)
44-47: ⚡ Quick winVerify package structure assumption in path navigation.
The function assumes
@jackwener/openclipackage structure wherebrowser/cdpis exactly 3 directories deep from the package root. If the OpenCLI package structure changes, this will break.Consider using a more robust resolution:
function openCliPackageRoot() { - const cdp = fileURLToPath(import.meta.resolve("`@jackwener/opencli/browser/cdp`")) - return path.resolve(path.dirname(cdp), "../../..") + // Resolve package.json directly if exported, or use a well-known entrypoint + try { + return path.dirname(fileURLToPath(import.meta.resolve("`@jackwener/opencli/package.json`"))) + } catch { + // Fallback to current approach + const cdp = fileURLToPath(import.meta.resolve("`@jackwener/opencli/browser/cdp`")) + return path.resolve(path.dirname(cdp), "../../..") + } }Or verify with the package maintainer that the structure is stable.
🤖 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 `@packages/opencode/src/opencli/adapter-registry.ts` around lines 44 - 47, The current openCliPackageRoot() assumes the package file path ends with "browser/cdp" and walks up three directories which is fragile; change it to resolve a stable entry such as the package manifest instead (use import.meta.resolve("`@jackwener/opencli/package.json`") or another canonical package entry) and compute the package root from the dirname of that resolved file (update references to cdp/import.meta.resolve and the return logic in openCliPackageRoot to use package.json resolution), so the root discovery does not depend on the internal "browser/cdp" layout.packages/opencode/test/opencli/adapter-capabilities.test.ts (1)
59-72: <!-- [scratchpad_start]
[task review_file_4/7 review_segment_2/4]
[observations]
packages/opencode/src/opencli/adapter-registry.ts:
BLOCKED_OPENCLI_COMMANDS = new Set(["instagram/reel"]); command gating only applies to whole commands.packages/opencode/test/opencli/adapter-capabilities.test.ts:
- ACCEPTED_CAPABILITY_GAPS includes
instagram/postwith kind"page-method"value"closeWindow".- SUPPORTED_PAGE_METHODS set in the test does not include
"closeWindow".- EXPECTED gaps are pinned via
expect(gaps).toEqual(ACCEPTED_CAPABILITY_GAPS)and browser-page-import commands matchBLOCKED_OPENCLI_COMMANDS.packages/opencode/src/opencli/adapter-runner.ts:
- createOpenCliAdapterPage proxy only special-cases
setFileInput,insertText,nativeType,nativeClick, andwaitForTimeout; it does not shim/handlecloseWindow.- No
closeWindowreferences found in adapter-runner.ts.
[analysis]
instagram/poststays exposed because onlyinstagram/reelis blocked at the registry level.The repo treats
closeWindowas an unsupported"page-method"in the capability-guard baseline; without a shim/runtime guard, invoking it depends on whether the underlyingpagebackend providescloseWindow(otherwise it will fail).
[pending]None; enough evidence to rewrite the comment.
[actions]Rewrite to reflect: avoid recommending adding
instagram/postto BLOCKED_OPENCLI_COMMANDS without design change; suggest shim or documentation consistent with existing test baseline.
[done]
scratchpad_end] -->
Handleinstagram/post’s unsupportedcloseWindowcapability more explicitly.
BLOCKED_OPENCLI_COMMANDSonly blocksinstagram/reel, soinstagram/poststays exposed.The capability-guard test pins
instagram/post’s unsupported"page-method"closeWindow(not in SUPPORTED_PAGE_METHODS), andadapter-runner.tsdoesn’t shim/guardcloseWindow—it will only work if the underlyingpageprovides it.Either add a
closeWindowimplementation/runtime capability check, or document that"page-method"gaps are intentionally tolerated while only daemon Page imports are blocked.🤖 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 `@packages/opencode/test/opencli/adapter-capabilities.test.ts` around lines 59 - 72, The test reveals instagram/post advertises an unsupported page-method closeWindow while only instagram/reel is blocked by BLOCKED_OPENCLI_COMMANDS; fix by either (A) adding a shim/guard for closeWindow in createOpenCliAdapterPage (same pattern as setFileInput/insertText/nativeType/nativeClick) so the adapter safely handles or no-ops that method at runtime, or (B) keep behavior explicit by updating documentation/tests: ensure ACCEPTED_CAPABILITY_GAPS and SUPPORTED_PAGE_METHODS reflect that closeWindow is intentionally unsupported and mention that only commands in BLOCKED_OPENCLI_COMMANDS are gated (do not add instagram/post to BLOCKED_OPENCLI_COMMANDS without a design change).packages/opencode/test/tool/opencli-tools.test.ts (1)
25-35: ⚡ Quick winUse the Effect test harness helpers instead of ad hoc runtime wiring.
This helper manually composes runtime/execution (
Instance.provide+ customexec) for an Effect workflow. Please migrate this test totestEffect(...)(and fixture instance helpers likeprovideTmpdirInstance(...)/tmpdir) to stay aligned with the repo’s test contract.As per coding guidelines:
packages/opencode/test/**/*.test.{ts,tsx}should usetestEffect(...)for Effect-based workflows and fixture helpers for instance-scoped setup.🤖 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 `@packages/opencode/test/tool/opencli-tools.test.ts` around lines 25 - 35, Replace the ad-hoc runtime wiring in the exec helper with the repo test harness: remove the manual Instance.provide-based exec function and rework the test to use testEffect(...) and the provided fixture helpers (e.g., provideTmpdirInstance(...) / tmpdir) to supply the instance and tmpdir; instantiate and initialize the tool by calling the AnyToolEffect init via the testEffect environment, then call the tool.execute within testEffect so you can rely on the harness to provide Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer) and proper lifecycle management instead of calling Instance.provide, Effect.runPromise, or using ctx directly.Source: Coding guidelines
🤖 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 `@packages/opencode/src/opencli/adapter-registry.ts`:
- Around line 128-131: Add the module-as-namespace self-reexport at the end of
the file so consumers can import the namespace; specifically, after the existing
resetOpenCliAdaptersForTest function (and other exports like
loadOpenCliAdapters), add the line that reexports the module as AdapterRegistry:
export * as AdapterRegistry from "./adapter-registry" so callers can use
AdapterRegistry.loadOpenCliAdapters,
AdapterRegistry.resetOpenCliAdaptersForTest, etc.
In `@packages/opencode/src/opencli/adapter-runner.ts`:
- Line 210: Add a module-as-namespace self-reexport at the end of the
adapter-runner module: append an export line that re-exports the module
namespace so consumers can import the namespace and call members like
AdapterRunner.runOpenCliAdapterCommand; specifically add the statement exporting
the module as AdapterRunner (export * as AdapterRunner from "./adapter-runner")
to the bottom of the file that defines the AdapterRunner symbols.
In `@packages/opencode/src/tool/opencli-run.ts`:
- Around line 50-55: The code unconditionally requests "browser" permission via
ctx.ask even for commands with browser explicitly disabled; update the call that
yields ctx.ask in opencli-run.ts so it only includes permission: "browser" and
browserAlwaysPatterns(...) (and metadata.browser true) when command.browser !==
false (or adapter supports browser), otherwise call ctx.ask without the browser
permission/patterns or set permission to undefined; locate the ctx.ask
invocation, the variables patterns and browserAlwaysPatterns, and the
metadata.browser/command.browser usage to implement the conditional branch.
---
Nitpick comments:
In `@packages/opencode/src/opencli/adapter-registry.ts`:
- Around line 44-47: The current openCliPackageRoot() assumes the package file
path ends with "browser/cdp" and walks up three directories which is fragile;
change it to resolve a stable entry such as the package manifest instead (use
import.meta.resolve("`@jackwener/opencli/package.json`") or another canonical
package entry) and compute the package root from the dirname of that resolved
file (update references to cdp/import.meta.resolve and the return logic in
openCliPackageRoot to use package.json resolution), so the root discovery does
not depend on the internal "browser/cdp" layout.
In `@packages/opencode/test/opencli/adapter-capabilities.test.ts`:
- Around line 59-72: The test reveals instagram/post advertises an unsupported
page-method closeWindow while only instagram/reel is blocked by
BLOCKED_OPENCLI_COMMANDS; fix by either (A) adding a shim/guard for closeWindow
in createOpenCliAdapterPage (same pattern as
setFileInput/insertText/nativeType/nativeClick) so the adapter safely handles or
no-ops that method at runtime, or (B) keep behavior explicit by updating
documentation/tests: ensure ACCEPTED_CAPABILITY_GAPS and SUPPORTED_PAGE_METHODS
reflect that closeWindow is intentionally unsupported and mention that only
commands in BLOCKED_OPENCLI_COMMANDS are gated (do not add instagram/post to
BLOCKED_OPENCLI_COMMANDS without a design change).
In `@packages/opencode/test/tool/opencli-tools.test.ts`:
- Around line 25-35: Replace the ad-hoc runtime wiring in the exec helper with
the repo test harness: remove the manual Instance.provide-based exec function
and rework the test to use testEffect(...) and the provided fixture helpers
(e.g., provideTmpdirInstance(...) / tmpdir) to supply the instance and tmpdir;
instantiate and initialize the tool by calling the AnyToolEffect init via the
testEffect environment, then call the tool.execute within testEffect so you can
rely on the harness to provide Layer.mergeAll(Truncate.defaultLayer,
Agent.defaultLayer) and proper lifecycle management instead of calling
Instance.provide, Effect.runPromise, or using ctx directly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a96db0eb-ad43-4694-b323-66e08cf35167
📒 Files selected for processing (21)
packages/desktop-electron/electron-builder-app-update.test.tspackages/desktop-electron/electron-builder.config.tspackages/desktop-electron/electron-vite.config.test.tspackages/desktop-electron/electron.vite.config.tspackages/opencode/script/build-node.tspackages/opencode/src/opencli/adapter-registry.tspackages/opencode/src/opencli/adapter-runner.tspackages/opencode/src/tool/opencli-run.tspackages/opencode/src/tool/opencli-run.txtpackages/opencode/src/tool/opencli-search.tspackages/opencode/src/tool/opencli-search.txtpackages/opencode/src/tool/registry.tspackages/opencode/src/tool/tool-info.tspackages/opencode/test/opencli/adapter-capabilities.test.tspackages/opencode/test/opencli/adapter-registry.test.tspackages/opencode/test/opencli/adapter-runner.test.tspackages/opencode/test/script/build-node.test.tspackages/opencode/test/server/built-node-opencli-adapters.test.tspackages/opencode/test/tool/opencli-tools.test.tspackages/opencode/test/tool/registry.test.tspackages/opencode/test/tool/tool-info.test.ts
Summary
opencli_searchandopencli_runtools.Why
The browser series needs a practical bridge from PawWork's embedded browser surface to site-specific OpenCLI adapters. This PR keeps that bridge opt-in through
tool_info, exposes searchable command metadata to the model, and verifies that the built embedded server can still discover packaged adapters after bundling.Related Issue
Refs #1186
Human Review Status
Pending
Review Focus
Risk Notes
How To Verify
Screenshots or Recordings
Not applicable, no visible UI or copy changes.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
New Features
Packaging
Permissions
Tests