Skip to content

fix: IPC timeout for browser operations, JSDoc accuracy, and misleading comments#26233

Merged
noanflaherty merged 1 commit into
noanflaherty/assistant-browser-cli-decouplingfrom
run-plan/browser-cli/fix-r1-1
Apr 17, 2026
Merged

fix: IPC timeout for browser operations, JSDoc accuracy, and misleading comments#26233
noanflaherty merged 1 commit into
noanflaherty/assistant-browser-cli-decouplingfrom
run-plan/browser-cli/fix-r1-1

Conversation

@noanflaherty
Copy link
Copy Markdown
Contributor

@noanflaherty noanflaherty commented Apr 17, 2026

Summary

Fixes gaps identified during plan review for assistant-browser-cli-decoupling.md.

Gap 1: IPC timeout mismatch — browser CLI commands now use 180s timeout to accommodate long-running operations like wait_for_download (max 120s)
Gap 2: JSDoc @throws on executeBrowserOperation replaced with accurate return-value docs
Gap 3: Misleading comment about browser_mode/activity as CLI global options corrected


Open with Devin

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: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@noanflaherty noanflaherty merged commit 45aae52 into noanflaherty/assistant-browser-cli-decoupling Apr 17, 2026
12 checks passed
@noanflaherty noanflaherty deleted the run-plan/browser-cli/fix-r1-1 branch April 17, 2026 21:51
noanflaherty added a commit that referenced this pull request Apr 17, 2026
…26235)

* refactor: add browser operations contract decoupled from tool wrappers (#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

* refactor: make browser skill wrappers thin adapters over browser operations (#26199)

* feat: add browser_execute CLI IPC route for daemon-side browser operations (#26200)

* feat: add `assistant browser` command namespace with 17 browser subcommands (#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>

* refactor: centralize browser identifier sets across permissions and side-effects (#26223)

* fix: stale comment and error message wording for wait_for_download (#26234)

* fix: IPC timeout for browser operations, JSDoc accuracy, and misleading comments (#26233)

* fix: update stale JSDoc on executeWaitForDownload to describe current state

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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