Add browser runtime lifecycle protocol#43
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds task-scoped browser grant context and browser runtime capability fields; introduces browser tool lifecycle/ artifact messages ( Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Daemon as Daemon
participant Cloud as Cloud
Browser->>Daemon: Send browserToolCallStarted (method/category/metadata)
Daemon->>Cloud: Relay browserToolCallStarted
Browser->>Daemon: Send browserToolCallUpdated (status/summary/metadata)
Daemon->>Cloud: Relay browserToolCallUpdated
Browser->>Daemon: Send browserArtifactCreated (artifactId/kind/url/metadata)
Daemon->>Cloud: Relay browserArtifactCreated
Browser->>Daemon: Send browserToolResult (resultJson redacted, sessionId, channelId, errorCode, redactionStatus)
Daemon->>Cloud: Relay browserToolResult
Note over Daemon,Cloud: hello.capabilities includes browserRuntime* and chrome availability
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 8 minutes and 50 seconds. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages_test.go`:
- Around line 230-271: Add tests that cover invalid-type rejection and
unknown-field compatibility for the new browser lifecycle messages:
BrowserToolCallStarted, BrowserToolCallUpdated, BrowserArtifactCreated and the
expanded browserToolResult shape (e.g., BrowserToolResult or whichever struct
represents the result). Specifically, add entries to the round-trip test table
for "browserToolCallStarted", "browserToolCallUpdated", "browserArtifactCreated"
and the browserToolResult case, then add parallel negative tests that (1) feed
an invalid "type" value for each message and assert parsing fails, and (2)
include unknown extra fields in the JSON payload for each message and assert the
parser accepts them (compatibility). Reference the existing test helpers and
table-driven test patterns in messages_test.go to mirror how other message types
validate invalid-type and unknown-field behavior.
In `@PROTOCOL.md`:
- Around line 1238-1249: The table has a malformed row with an extra table cell;
ensure every row in the "browserToolResult" table has exactly two columns (Field
and Type). Specifically, fix the "error" row so it only has two cells (e.g., "|
error | string? |") and remove any stray trailing cell/pipe; also if any
explanatory notes (like "Redacted safe result data only.") were placed in a
third cell (e.g., on "resultJson"), move that note into the Type cell after the
type (e.g., "json? — Redacted safe result data only.") so all rows remain
two-column compliant.
🪄 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: 6d00f82e-1ebf-4f0e-a004-d9b738ff68d9
📒 Files selected for processing (4)
PROTOCOL.mdenvelope.gomessages.gomessages_test.go
Summary
Verification
Dependency
Merge after browser method manifest PR. Tag protocol after merge, then bump daemon and cloud-app relay.
Summary by CodeRabbit
New Features
Tests