Skip to content

feat: add assistant browser CLI with decoupled operations contract#26235

Merged
noanflaherty merged 8 commits into
mainfrom
noanflaherty/assistant-browser-cli-decoupling
Apr 17, 2026
Merged

feat: add assistant browser CLI with decoupled operations contract#26235
noanflaherty merged 8 commits into
mainfrom
noanflaherty/assistant-browser-cli-decoupling

Conversation

@noanflaherty
Copy link
Copy Markdown
Contributor

@noanflaherty noanflaherty commented Apr 17, 2026

Summary

Introduces a first-class assistant browser command namespace with 17 subcommands (navigate, click, type, screenshot, etc.) that are fully decoupled from the existing browser_* skill tools. The implementation adds a shared browser operations contract that both tool wrappers and the CLI consume, creating a clean removal path for browser_* tools later.

Self-review result

GAPS FOUND — 5 gaps fixed across 2 fix PRs

PRs merged into feature branch

Fix PRs

Part of plan: assistant-browser-cli-decoupling.md


Open with Devin

noanflaherty and others added 7 commits April 17, 2026 16:13
#26193)

* refactor: add browser operations contract decoupled from tool wrappers

* fix: remove TOOLS.json literal from doc comment to pass static analysis guard

* fix: remove bundled-skills reference from doc comment to pass static analysis guard
…mmands (#26207)

* feat: add assistant browser command namespace with 17 browser subcommands

* fix: address review feedback — daemon terminology, help text examples, boolean flags, write error handling

1. Replace "daemon" and "CLI IPC socket" with user-friendly terminology
   in all user-facing text per AGENTS.md guidelines.

2. Add optional `helpText` field to `BrowserOperationMeta` type and
   populate it for all 16 operations with behavioral notes and 2-3
   concrete examples per CLI AGENTS.md Help Text Standards.
   `buildSubcommand` now calls `subcmd.addHelpText("after", ...)`.

3. Fix boolean flags to support explicit `false` values via Commander's
   `--no-<flag>` negation pattern. `fieldToFlag` now emits
   `--flag, --no-flag` for booleans, and `parseFieldValue` passes
   through the actual boolean value instead of always coercing to `true`.

4. Wrap `writeFileSync` for screenshot `--output` in try/catch,
   producing structured error output (JSON or log) instead of an
   unhandled stack trace on invalid path or permission errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add enum choices validation and fix boolean option negation for Commander 13

- Use Commander's Option class with .choices() when field.enum is defined,
  enforcing valid values at the CLI level and showing allowed choices in --help.
  Required enum fields also call .makeOptionMandatory(true).

- Fix boolean option negation by declaring only --flag (not --flag, --no-flag).
  Commander 13 auto-generates --no-flag; the dual-declaration pattern caused
  --flag to parse as false.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@noanflaherty
Copy link
Copy Markdown
Contributor Author

@codex review this PR again — the previous issues have been fixed in commit 9a6c64f

@noanflaherty
Copy link
Copy Markdown
Contributor Author

@devin review this PR again — the previous issues have been fixed in commit 9a6c64f

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

registerDefaultAction(program);
registerBackupCommand(program);
registerBashCommand(program);
registerBrowserCommand(program);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 CLI browser command bypasses browser feature flag gate

The browser CLI command is registered unconditionally in assistant/src/cli/program.ts:68, and the IPC route at assistant/src/ipc/routes/browser.ts:71 dispatches directly through executeBrowserOperation — which calls into browser-execution.ts functions without checking the browser feature flag. In contrast, the LLM-facing path gates browser tool availability on the feature flag (as shown in browser-skill-endstate.test.ts with _setOverridesForTesting({ browser: true })). This means assistant browser navigate --url ... works even when the feature flag is disabled, while the LLM cannot invoke browser tools. This may be intentional (CLI as an independent entrypoint), but it's worth confirming the design intent since the feature flag is meant to control browser availability.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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".

@noanflaherty noanflaherty merged commit e14fe23 into main Apr 17, 2026
12 checks passed
@noanflaherty noanflaherty deleted the noanflaherty/assistant-browser-cli-decoupling branch April 17, 2026 22:42
@noanflaherty
Copy link
Copy Markdown
Contributor Author

Review Summary

Pulled this branch into a dedicated worktree and did a full diff review with targeted validation.

Validation run

  • bun test src/browser/__tests__/operations.test.ts src/ipc/__tests__/browser-ipc.test.ts src/cli/commands/__tests__/browser.test.ts src/config/bundled-skills/browser/tools/__tests__/wrapper-adapters.test.ts src/__tests__/browser-identifier-parity-guard.test.ts src/__tests__/browser-skill-endstate.test.ts
  • bunx tsc --noEmit

Both passed after installing package deps in the worktree.

Findings

1) [P2] assistant browser drops browser_mode parity with existing browser tools

Where:

  • assistant/src/browser/operations.ts (lines 222-224)
  • assistant/src/cli/commands/browser.ts (subcommands are generated from meta.fields only)

Issue:
The new CLI intentionally omits browser_mode from operation metadata and therefore exposes no --browser-mode flag. Existing browser_* tools all support browser_mode overrides today.

Impact:
This is a capability regression for workflows that need deterministic backend pinning (extension, cdp-inspect, local) for debugging/recovery. It also weakens the migration goal of eventually replacing tool usage with CLI usage.

Suggested fix:
Add a shared --browser-mode <mode> option (with enum validation) at the assistant browser command level or per subcommand, and pass it through in IPC input as browser_mode.


2) [P3] Policy modules now import a heavy browser runtime graph via BROWSER_TOOL_NAMES

Where:

  • assistant/src/permissions/defaults.ts (line 3)
  • assistant/src/permissions/workspace-policy.ts (line 4)
  • assistant/src/tools/side-effects.ts (line 7)
  • assistant/src/browser/operations.ts (lines 16-35)

Issue:
BROWSER_TOOL_NAMES is sourced from browser/operations.ts, which imports browser-execution, browser-manager, and browser-mode. That means policy/scoping modules now eagerly pull in the browser execution stack even when only identifier lists are needed.

Impact:
Unnecessary startup/module-load coupling in non-browser codepaths, and increased risk of future dependency cycles or environment-specific breakage.

Suggested fix:
Move identifier-only exports (BROWSER_OPERATIONS, BROWSER_TOOL_NAMES, name mapping helpers) into a lightweight module with no runtime browser imports (e.g. assistant/src/browser/identifiers.ts or extend browser/types.ts), and have both operations.ts and policy modules import from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant