Release 0.19.3 — context registration lifecycle#120
Conversation
|
Warning Review limit reached
More reviews will be available in 23 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe daemon now claims context IDs through a shared helper for extension and native registrations. The extension background code interprets registration control messages, updates conflict badges, and changes WebSocket readiness flow around registration acknowledgements. The native agent handles the same control messages, and tests plus docs were added. ChangesContext Name Uniqueness Guard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
vitorfhc
force-pushed
the
fix/context-name-uniqueness
branch
from
739a993 to
54f2b5e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/dist-mv2/background-electron.js`:
- Around line 1670-1744: The window mutation paths in handleWindowActions
currently treat withWindowTimeout failures from
chrome.windows.create/update/remove as hard errors, but these operations can
still complete after the timeout. Update the window_create, window_close,
window_focus, and window_resize cases to distinguish mutation timeouts from real
failures and return an unknown/in-flight result instead of failed when the
timeout is hit, while keeping timeout-based failure handling for
non-mutation/read-only calls. Use the existing withWindowTimeout,
handleWindowActions, and windowIdFromAction helpers to localize the change.
In `@extension/src/background/transport.ts`:
- Line 380: The storage-driven re-registration path in transport.ts ignores the
result of sendWsRegistration(), which can leave the WebSocket open while wsReady
stays false. Update the re-registration handling around
sendWsRegistration(wsChannel, newId) to check the returned boolean and, on
failure, follow the same recovery flow as the onopen branch by closing the
socket and triggering reconnect logic. Use the existing wsChannel, newId, and
registration/reconnect helpers in this transport flow so the channel does not
remain stuck unregistered.
🪄 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
Run ID: 771273cc-7d39-42cb-a102-dea6e6c19a09
📒 Files selected for processing (10)
README.mddaemon/context-registration.tsdaemon/index.tsextension/dist-mv2/background-electron.jsextension/src/background/context-registration.tsextension/src/background/transport.tsinterceptor-agent/Sources/InterceptorAgent/AgentClient.swiftprd/PRD-113-PR120-CONTEXT-UNIQUENESS-FOLLOWUP.mdtest/context-uniqueness.test.tstest/extension-context-registration.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- daemon/index.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 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/dist-mv2/background-electron.js`:
- Around line 1670-1744: The window mutation paths in handleWindowActions
currently treat withWindowTimeout failures from
chrome.windows.create/update/remove as hard errors, but these operations can
still complete after the timeout. Update the window_create, window_close,
window_focus, and window_resize cases to distinguish mutation timeouts from real
failures and return an unknown/in-flight result instead of failed when the
timeout is hit, while keeping timeout-based failure handling for
non-mutation/read-only calls. Use the existing withWindowTimeout,
handleWindowActions, and windowIdFromAction helpers to localize the change.
In `@extension/src/background/transport.ts`:
- Line 380: The storage-driven re-registration path in transport.ts ignores the
result of sendWsRegistration(), which can leave the WebSocket open while wsReady
stays false. Update the re-registration handling around
sendWsRegistration(wsChannel, newId) to check the returned boolean and, on
failure, follow the same recovery flow as the onopen branch by closing the
socket and triggering reconnect logic. Use the existing wsChannel, newId, and
registration/reconnect helpers in this transport flow so the channel does not
remain stuck unregistered.
🪄 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
Run ID: 771273cc-7d39-42cb-a102-dea6e6c19a09
📒 Files selected for processing (10)
README.mddaemon/context-registration.tsdaemon/index.tsextension/dist-mv2/background-electron.jsextension/src/background/context-registration.tsextension/src/background/transport.tsinterceptor-agent/Sources/InterceptorAgent/AgentClient.swiftprd/PRD-113-PR120-CONTEXT-UNIQUENESS-FOLLOWUP.mdtest/context-uniqueness.test.tstest/extension-context-registration.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- daemon/index.ts
🛑 Comments failed to post (2)
extension/dist-mv2/background-electron.js (1)
1670-1744: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Are Chrome extensionchrome.windows.create,chrome.windows.update, andchrome.windows.removePromise-returning APIs abortable or cancelable once started?💡 Result:
No, the Chrome extension APIs chrome.windows.create, chrome.windows.update, and chrome.windows.remove are not abortable or cancelable [1][2][3]. While these methods return Promises in Manifest V3 [1][4][5], they do not support the AbortSignal pattern or any other built-in mechanism to abort or cancel the operation once it has been initiated [1][2][6]. Because these APIs are asynchronous, once a call is made, the browser processes the request according to its internal state [3]. There is no native interface to stop a pending "create," "update," or "remove" action once the function has been called [7][6]. If you need to manage window lifecycle flows (e.g., conditional execution or sequential actions), you must use event-driven patterns, such as listening to events like chrome.windows.onRemoved, or implement your own state management logic to ignore results or handle secondary actions if an initial operation becomes obsolete [3]. Discussions regarding the addition of AbortSignal support to WebExtensions APIs have occurred within the developer community, but such functionality has not been implemented for these specific window management methods [7][6].
Citations:
- 1: https://developer.chrome.com/docs/extensions/reference/api/windows
- 2: https://developer.chrome.com/docs/extensions/reference/api/windows.md.txt
- 3: https://codemia.io/knowledge-hub/path/chrome_extensions_synchronous_calls_-_create_window_only_after_window_closed
- 4: https://developer.chrome.com/docs/extensions/mv2/reference/windows.md.txt
- 5: GoogleChrome/chrome-types#69
- 6: w3c/webextensions#415
- 7: https://groups.google.com/a/chromium.org/g/chromium-extensions/c/smBGekWAxQA/m/vaKhSWv1AwAJ
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "withWindowTimeout\\(|chrome\\.windows\\.(create|update|remove|getCurrent|get|query)" extension/dist-mv2/background-electron.js sed -n '1640,1795p' extension/dist-mv2/background-electron.jsRepository: Hacker-Valley-Media/Interceptor
Length of output: 6850
Treat timed-out window mutations as unknown, not failed.
chrome.windows.create/update/removecan’t be cancelled, so a timeout can leave the mutation running after the caller already sees an error. That makes retries unsafe and can duplicate creates or apply delayed closes/resizes out of order; keep the timeout for read-only calls, but surface mutation timeouts as in-flight/unknown instead.🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 1671-1671: Avoid using the initial state variable in setState
Context: setTimeout(() => reject(new WindowOperationTimeoutError(op, ms)), ms)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(setstate-same-var)
🤖 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/dist-mv2/background-electron.js` around lines 1670 - 1744, The window mutation paths in handleWindowActions currently treat withWindowTimeout failures from chrome.windows.create/update/remove as hard errors, but these operations can still complete after the timeout. Update the window_create, window_close, window_focus, and window_resize cases to distinguish mutation timeouts from real failures and return an unknown/in-flight result instead of failed when the timeout is hit, while keeping timeout-based failure handling for non-mutation/read-only calls. Use the existing withWindowTimeout, handleWindowActions, and windowIdFromAction helpers to localize the change.extension/src/background/transport.ts (1)
380-380: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle storage-driven re-registration send failures.
If
sendWsRegistration()fails here, it has already flipped the channel back to unregistered. Ignoring thefalsereturn leaves an open socket withwsReady = falseand no recovery path until some later close/error happens. Mirror theonopenbranch and close/reconnect on failure.Suggested fix
- sendWsRegistration(wsChannel, newId) + if (!sendWsRegistration(wsChannel, newId)) { + try { + wsChannel.close() + } catch {} + wsChannel = null + scheduleWsReconnect() + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (!sendWsRegistration(wsChannel, newId)) { try { wsChannel.close() } catch {} wsChannel = null scheduleWsReconnect() }🤖 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/background/transport.ts` at line 380, The storage-driven re-registration path in transport.ts ignores the result of sendWsRegistration(), which can leave the WebSocket open while wsReady stays false. Update the re-registration handling around sendWsRegistration(wsChannel, newId) to check the returned boolean and, on failure, follow the same recovery flow as the onopen branch by closing the socket and triggering reconnect logic. Use the existing wsChannel, newId, and registration/reconnect helpers in this transport flow so the channel does not remain stuck unregistered.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/extension-ws-lifecycle.test.ts`:
- Line 33: The test teardown is restoring globalThis.chrome to undefined even
when it did not exist originally, which changes the global shape for later
tests. In the extension-ws-lifecycle test, use the saved originalChrome value to
restore the property exactly: if it was present, assign it back; if it was
absent, remove chrome from globalThis instead of setting it to undefined. Keep
this logic in the setup/teardown path where originalChrome is captured and
restored.
- Around line 78-89: The test name promises behavior before onopen, but it only
asserts that connectWsChannel coalesces to one WebSocket instance. Update the
test to explicitly verify that no registration/send work happens until the
FakeWebSocket is opened, using connectWsChannel and the FakeWebSocket instance
to assert the pre-onopen state. This should guard the transport flow in
extension/src/background/transport.ts so a premature send() call would fail the
test.
🪄 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
Run ID: af209204-8337-4929-a06a-83c9658212ae
📒 Files selected for processing (4)
extension/dist-mv2/background-electron.jsextension/src/background/transport.tsprd/PRD-113-PR120-CONTEXT-UNIQUENESS-FOLLOWUP.mdtest/extension-ws-lifecycle.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- extension/src/background/transport.ts
- extension/dist-mv2/background-electron.js
ronaldeddings
force-pushed
the
fix/context-name-uniqueness
branch
from
551f393 to
0dedda8
Compare
ronaldeddings
changed the title
ronaldeddings
left a comment
ronaldeddings
left a comment
There was a problem hiding this comment.
Approved for merge after sanitation, version bump, and passing validation.
Bumps every tracked surface to 0.19.3. This release hardens context registration so duplicate browser and native context names fail explicitly without stealing routing ownership, and it makes extension WebSocket registration resilient to overlapping service-worker connection attempts and failed re-registration sends. Context registration lifecycle - Reject duplicate browser and native context claims through a shared production registration helper. - Emit explicit context_registered acknowledgements before the extension marks WebSocket routing ready. - Keep same-socket re-registration, context rename, and disconnected context reclaim behavior working. Extension recovery behavior - Show a red toolbar badge when a duplicate context is rejected. - Clear the conflict badge after a successful registration. - Coalesce overlapping WebSocket connection attempts before onopen so one extension instance cannot falsely conflict with itself. - Recover storage-driven context re-registration send failures by closing the stale socket and scheduling reconnect. Release prep - Bump package, CLI source version, MV3 manifest, and tracked MV2 manifest to 0.19.3. - Keep Sparkle publishing and GitHub Release creation out of this change. Validation - Focused context and WebSocket lifecycle tests: 13/0. - Typecheck clean. - Suite: 537/0, with 10 existing skips. - Build complete.
ronaldeddings
force-pushed
the
fix/context-name-uniqueness
branch
from
0dedda8 to
73fff74
Compare
ronaldeddings
left a comment
ronaldeddings
left a comment
There was a problem hiding this comment.
Approved final sanitized 0.19.3 context registration lifecycle update after passing CI.
|
@vitorfhc wanted to drop in and say AWESOME contribution. Great for quality of life. @pedramamini just encouraged me to open #124 which is going to open up the world of possibilities. This is the predecessor to making this happen. 🔥 |
2 participants
Summary
This prepares Interceptor
0.19.3and hardens context registration across the daemon, browser extension, MV2 app bridge bundle, and native runtime-agent path.What Changed
0.19.3: package metadata, CLI source version, MV3 manifest, and tracked MV2 manifest.context_registeredacknowledgements so the extension only marks WebSocket routing ready after daemon acceptance.onopen, preventing one extension instance from falsely conflicting with itself.Validation
bun test test/context-uniqueness.test.ts test/extension-context-registration.test.ts test/extension-ws-lifecycle.test.tsbun run typecheckbun testbun run buildRelease Scope