Add the TinyFlows Chrome workflow companion#15
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThe PR adds a versioned browser automation protocol, authenticated loopback companion, MV3 Chrome extension, debugger-based actions, explicit tab sharing, CLI commands, deterministic packaging, documentation, and CI coverage and end-to-end validation. ChangesChrome companion workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChromeExtension
participant CompanionServer
participant WorkflowEngine
participant BrowserRelay
User->>ChromeExtension: Pair and share tab
ChromeExtension->>CompanionServer: Authenticated WebSocket connection
User->>CompanionServer: Start workflow for tab
CompanionServer->>WorkflowEngine: Execute workflow
WorkflowEngine->>BrowserRelay: Invoke browser tool
BrowserRelay->>ChromeExtension: Send browser action
ChromeExtension-->>BrowserRelay: Return action response
BrowserRelay-->>WorkflowEngine: Return browser result
WorkflowEngine-->>User: Publish run events
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![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: ca436dacb1
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe3e7a4149
ℹ️ 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".
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
extension/src/cdp.ts (1)
105-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
evaluate()discards the underlying exception, hindering debugging.
exceptionDetailsis checked only for presence; the actual failure text/description is dropped in favor of a generic'Page evaluation failed'. This makes it hard for a workflow author to diagnose whyget_text/fill/find/waitfailed on a given page.♻️ Surface the exception text
const result = response as { result?: { value?: unknown }; exceptionDetails?: unknown }; - if (result.exceptionDetails) throw new BrowserError('browser_failure', 'Page evaluation failed'); + if (result.exceptionDetails) { + const details = result.exceptionDetails as { exception?: { description?: string }, text?: string }; + throw new BrowserError('browser_failure', details.exception?.description ?? details.text ?? 'Page evaluation failed'); + }🤖 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 `@extension/src/cdp.ts` around lines 105 - 110, Update evaluate() to extract the exception text or description from result.exceptionDetails and include it in the BrowserError message while preserving the existing browser_failure code and generic fallback when no details are available.extension/src/tab-manager.ts (1)
108-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant
tabs.getcall inannouncement(); reuse the tab already fetched byassertShared.
assertSharedalready fetches the tab (line 90) and validates its URL.announcement()then fetches it again independently and assertstab.url!non-null. Besides the extra API call, there's a small TOCTOU window where the tab could close or navigate between the two fetches.♻️ Reuse the validated tab
- async announcement(tabId: number): Promise<SharedTabAnnouncement> { - const shared = await this.assertShared(tabId); - const tab = await this.api.tabs.get(tabId); - return { - id: tabId, - window_id: shared.windowId, - url: tab.url!, - title: tab.title ?? '' - }; - } + async announcement(tabId: number): Promise<SharedTabAnnouncement> { + const shared = await this.assertShared(tabId); + const tab = await this.api.tabs.get(tabId); // still needed if assertShared doesn't return the Tab + return { + id: tabId, + window_id: shared.windowId, + url: tab.url ?? '', + title: tab.title ?? '' + }; + }(Ideally have
assertSharedreturn the already-fetchedTabalongside theSharedTabrecord to avoid the second call.)🤖 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 `@extension/src/tab-manager.ts` around lines 108 - 117, Update assertShared and announcement to reuse the tab fetched and URL-validated by assertShared: return the validated Tab alongside the SharedTab record, then use that returned tab when constructing SharedTabAnnouncement. Remove the independent api.tabs.get call and the non-null URL assertion while preserving the existing shared-tab validation behavior.extension/src/errors.ts (1)
14-21: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTab-revocation detection relies on matching Chrome's internal error wording.
toBrowserErrorclassifiestab_revokedby checkingmessage.toLowerCase().includes('no tab with id'), which is Chrome's undocumented internal phrasing for a missing tab (e.g. fromchrome.tabs.remove/chrome.debugger.sendCommand). Given how centraltab_revokedis to the fail-closed contract (persrc/browser/routing.rs'sstable_relay_error_code_reaches_engine_retry_surfacetest), a wording change in a future Chrome release would silently degrade this tobrowser_failurewithout any test failure in this repo. Consider proactively checking tab existence (e.g. viachrome.tabs.getbefore relying on string matching) rather than depending solely on error text.🤖 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 `@extension/src/errors.ts` around lines 14 - 21, Update toBrowserError and its callers so tab-revocation classification is based on an explicit chrome.tabs.get existence check rather than relying solely on the “no tab with id” message. Preserve tab_revoked for confirmed missing tabs, while retaining browser_failure for other errors and existing BrowserError instances; adjust the flow to support the required asynchronous lookup.
🤖 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 `@extension/.gitignore`:
- Around line 2-4: Update the ignore rules for the artifacts directory so
generated archive files remain ignored while checksum files matching *.sha256
remain visible and trackable by Git. Replace the broad artifacts/ rule with
specific archive exclusion and checksum negation patterns.
In `@extension/package.json`:
- Line 13: Update the package script in package.json so it runs the project
build before invoking scripts/package.mjs, ensuring packaging uses freshly
generated dist output.
In `@extension/src/relay.ts`:
- Around line 40-46: Guard the socket handlers in connect() so onerror and
onclose ignore events from sockets that are no longer this.socket, matching the
existing onopen check. Update stop() to clear this.socket when stopping, while
ensuring the active socket cleanup and pending-request rejection still occur.
Only clear timers, reject pending requests, and schedule reconnects for the
current socket and configuration.
- Around line 139-150: Update the pairingToken validation in isRelayConfig to
allow the full base64url character set, including hyphen and underscore, while
preserving the existing length requirement and rejecting other characters.
In `@extension/tests/e2e/extension.spec.ts`:
- Around line 58-144: Wrap the relay setup and test actions, including the
existing close-response assertions, in a try/finally block. In finally, close
extensionSocket if present and await relayServer.close completion so the mock
WebSocketServer is always shut down even when an assertion or action fails.
In `@src/browser/protocol.rs`:
- Around line 14-16: The BrowserAction unit variants still accept unknown
payload fields despite deny_unknown_fields. Update the BrowserAction
representation or add explicit deserialization validation so Snapshot, GetTitle,
GetUrl, and Close reject inputs such as {"action":"snapshot","foo":1}, while
preserving valid decoding; add a test covering rejection of extra fields.
In `@src/browser/routing.rs`:
- Around line 169-172: Align native Fill validation with the extension validator
by updating validate_action’s BrowserAction::Fill branch to require a non-empty
value as well as a non-empty selector. Preserve the existing selector validation
and fail native pre-validation for empty Fill values.
In `@src/lib.rs`:
- Around line 22-25: Add doc comments to the public module declarations
`browser` and `companion` in `lib.rs`, briefly describing each module’s purpose,
so they satisfy the existing `missing_docs` lint.
In `@src/main.rs`:
- Around line 273-287: Update MemoryState::load to propagate a poisoned mutex
error instead of converting it to None, matching MemoryState::store’s error
behavior. Reuse unavailable("state store") or an appropriate
state-store-specific error, and ensure the existing key lookup still returns
None only when the lock succeeds and the key is absent.
- Around line 88-117: Add a finite request timeout to the reqwest clients used
by native_get and native_run, using the existing client builder pattern and the
repository’s established timeout value or configuration. Ensure both calls
retain their current request behavior while failing instead of blocking
indefinitely.
---
Nitpick comments:
In `@extension/src/cdp.ts`:
- Around line 105-110: Update evaluate() to extract the exception text or
description from result.exceptionDetails and include it in the BrowserError
message while preserving the existing browser_failure code and generic fallback
when no details are available.
In `@extension/src/errors.ts`:
- Around line 14-21: Update toBrowserError and its callers so tab-revocation
classification is based on an explicit chrome.tabs.get existence check rather
than relying solely on the “no tab with id” message. Preserve tab_revoked for
confirmed missing tabs, while retaining browser_failure for other errors and
existing BrowserError instances; adjust the flow to support the required
asynchronous lookup.
In `@extension/src/tab-manager.ts`:
- Around line 108-117: Update assertShared and announcement to reuse the tab
fetched and URL-validated by assertShared: return the validated Tab alongside
the SharedTab record, then use that returned tab when constructing
SharedTabAnnouncement. Remove the independent api.tabs.get call and the non-null
URL assertion while preserving the existing shared-tab validation behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee98e0ce-9628-4f8d-bf1e-522e215dc97b
⛔ Files ignored due to path filters (10)
Cargo.lockis excluded by!**/*.lockextension/artifacts/tinyflows-chrome-extension-0.1.0.zipis excluded by!**/*.zipextension/dist/background.jsis excluded by!**/dist/**extension/dist/manifest.jsonis excluded by!**/dist/**extension/dist/popup.htmlis excluded by!**/dist/**extension/dist/popup.jsis excluded by!**/dist/**extension/dist/sidepanel.htmlis excluded by!**/dist/**extension/dist/sidepanel.jsis excluded by!**/dist/**extension/dist/ui.cssis excluded by!**/dist/**extension/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (53)
.github/workflows/ci.ymlCHANGELOG.mdCargo.tomlREADME.mddocs/chrome-extension.mdextension/.gitignoreextension/README.mdextension/artifacts/tinyflows-chrome-extension-0.1.0.zip.sha256extension/eslint.config.jsextension/manifest.jsonextension/package.jsonextension/playwright.config.tsextension/scripts/build.mjsextension/scripts/package.mjsextension/src/background.tsextension/src/cdp.tsextension/src/errors.tsextension/src/popup.htmlextension/src/popup.tsextension/src/protocol.tsextension/src/relay.tsextension/src/sidepanel.htmlextension/src/sidepanel.tsextension/src/tab-manager.tsextension/src/ui.cssextension/tests/cdp.test.tsextension/tests/e2e/extension.spec.tsextension/tests/errors.test.tsextension/tests/protocol.test.tsextension/tests/relay.test.tsextension/tests/tab-manager.test.tsextension/tsconfig.jsonextension/vitest.config.tsprotocol/browser-v1.schema.jsonprotocol/fixtures/browser-cancel.v1.jsonprotocol/fixtures/browser-request.v1.jsonprotocol/fixtures/browser-response.v1.jsonprotocol/fixtures/tab-shared.v1.jsonsrc/browser/mod.rssrc/browser/protocol.rssrc/browser/routing.rssrc/companion/auth.rssrc/companion/control.rssrc/companion/mod.rssrc/companion/relay.rssrc/companion/server.rssrc/companion/tabs.rssrc/engine.rssrc/lib.rssrc/main.rssrc/observability.rstests/browser_routing_e2e.rstests/cli_e2e.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b57ccb839
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13cc80422c
ℹ️ 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".
|
Also addressed the remaining review notes in 3e8b4fe: page-evaluation errors now preserve CDP exception diagnostics, shared-tab announcements reuse the already validated tab, and tab revocation is confirmed with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c379b2542
ℹ️ 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".
| this.onState('connected'); | ||
| this.heartbeatTimer = setInterval(() => this.send({ protocol_version: PROTOCOL_VERSION, type: 'heartbeat' }), 15_000); | ||
| }; | ||
| socket.onmessage = (event) => { void this.handleMessage(String(event.data)); }; |
There was a problem hiding this comment.
When a workflow fans out into two browser tool nodes for the same shared tab, the native side can send both requests over the socket as separate pending actions. This handler starts handleMessage for each WebSocket frame without awaiting or queueing the prior one, so both onBrowserRequest calls can drive CDP concurrently (for example, an open racing a click or fill) and a step can run against the wrong document while both report success; queue browser requests per tab/run before invoking onBrowserRequest.
Useful? React with 👍 / 👎.
| async function announceAllSharedTabs(): Promise<void> { | ||
| for (const { tabId } of tabs.list()) { | ||
| try { relay.send(tabSharedEvent(await tabs.announcement(tabId))); } | ||
| catch { /* assertShared revokes stale attachment metadata */ } |
There was a problem hiding this comment.
When the extension reconnects after a user closed, ungrouped, or navigated a previously shared tab while the relay was down, tabs.announcement(tabId) revokes the local record and then this catch swallows the failure without sending tab_revoked. Because the native relay keeps its shared-tab registry across disconnects, it can still list and bind workflows to that stale tab until the first browser action fails, allowing earlier non-browser workflow steps to run after tab consent was effectively revoked; send a revocation for this tabId when re-announcement fails.
Useful? React with 👍 / 👎.
| const ok = await evaluate(this.debuggerApi, target, `(() => { const e=document.querySelector(${JSON.stringify(action.selector)}); if(!e)return null; e.focus(); return true; })()`); | ||
| if (ok !== true) throw new BrowserError('element_not_found', `No element matches ${action.selector}`); | ||
| } | ||
| await this.debuggerApi.sendCommand(target, 'Input.insertText', { text: action.text }); |
There was a problem hiding this comment.
When the selector resolves to a hidden, disabled, or otherwise non-focusable element, e.focus() can leave focus on whatever control was previously active, but this expression still returns true and the following Input.insertText types there while reporting success. For workflows with a stale or broad selector, that can mutate the wrong field instead of failing/retrying; ensure the target is visible/editable and became document.activeElement before inserting text.
Useful? React with 👍 / 👎.
1 participant
Summary
browsertool routing while preserving host integration dispatchcargo installSecurity and behavior
The extension never owns workflow state, credentials, approvals, retries, or non-browser integrations. It can only control ordinary HTTP(S) tabs explicitly shared into the TinyFlows tab group. Every run is bound to one selected shared tab and fails closed on revocation, timeout, cancellation, protocol mismatch, or relay loss.
The relay binds only to loopback, authenticates the exact paired
chrome-extension://origin and every session, and carries the host-local secret in the WebSocket subprotocol rather than the URL.Validation
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo build --all-targets --all-featurescargo test --all-features(367 unit tests plus integration and doc tests)cargo packagecargo llvm-cov --all-features --workspace --summary-only --fail-under-lines 90(90.39% lines)npm run verify(20 Vitest tests; 94.19% lines across protocol/relay/CDP/tab-manager core)npm run test:e2e(3 Playwright MV3 journeys)actionlint .github/workflows/ci.yml657de561f3b6beefd6810b540885b18b77b9159944b30134251a043e36dd09a2Chrome Web Store submission and OpenHuman host wiring remain follow-up work, as scoped.
Summary by CodeRabbit
New Features
Documentation
Tests