This is a comment left during a code review.
Path: assets/chrome-extension/background.js
Line: 116:117

Comment:
**`onRelayClosed` can null a healthy socket**

`onRelayClosed` unconditionally sets `relayWs = null` at line 117 without checking whether `relayWs` still refers to the socket that actually closed. Here's the race:

1. Old socket `ws_old` is connected, `relayWs = ws_old`.
2. `ws_old` disconnects → `onRelayClosed` fires from `ws_old`'s `onclose` handler.
3. Meanwhile, `scheduleReconnect` fires its timer, `ensureRelayConnection` creates `ws_new`, and at line 73 sets `relayWs = ws_new`.
4. Since JS is single-threaded, the microtask from step 2 can interleave with the async flow from step 3. Specifically, after `ensureRelayConnection` yields at the `await` on line 75 (waiting for `ws_new.onopen`), `onRelayClosed` runs and sets `relayWs = null` — even though `relayWs` now points to `ws_new`, not `ws_old`.

The fix is to check identity before nulling:

```suggestion
function onRelayClosed(reason) {
  // Guard: only act if this close event is for the current socket
  if (relayWs && relayWs.readyState !== WebSocket.CLOSED) return
  relayWs = null
```

Or alternatively, capture a reference in the close handler and compare:
```js
// In ensureRelayConnection, when installing the permanent handler:
ws.onclose = () => { if (relayWs === ws) onRelayClosed('closed') }
ws.onerror = () => { if (relayWs === ws) onRelayClosed('error') }
```
The second approach is more robust since it directly ties the handler to the socket instance.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: assets/chrome-extension/background.js
Line: 666:675

Comment:
**Duplicate `chrome.alarms.create` call**

`chrome.alarms.create('relay-keepalive', ...)` is called both inside `onInstalled` (line 670) and at module top-level (line 675). The top-level call runs on every service worker startup (including restarts), while `onInstalled` only fires on install/update. Since the top-level call at line 675 already covers the `onInstalled` case, the call at line 670 is redundant. It's not harmful (calling `create` with the same name replaces the existing alarm), but it's confusing and could be removed to reduce noise.

```suggestion
chrome.runtime.onInstalled.addListener(() => {
  // Useful: first-time instructions.
  void chrome.runtime.openOptionsPage()
})

// ===== FIX #5: Keepalive via chrome.alarms =====
// Prevents MV3 service worker from being terminated, preserving debugger sessions
chrome.alarms.create('relay-keepalive', { periodInMinutes: 4 })
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: assets/chrome-extension/background.js
Line: 152:153

Comment:
**Reconnect guard disarmed during async execution**

At line 153, `reconnectTimer = null` is set at the very start of the timer callback, before `ensureRelayConnection()` is called. This disarms the `if (reconnectTimer) return` guard in `scheduleReconnect` for the entire duration of the async reconnection attempt. If `onRelayClosed` fires during this window (e.g., from an old socket), it will call `scheduleReconnect()`, which will successfully schedule a *second* timer since the guard is disarmed. This means two reconnect flows can run concurrently.

Consider adding a boolean flag like `reconnecting = true` to guard against this, or move `reconnectTimer = null` to just before `scheduleReconnect()` is called in the catch block.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: assets/chrome-extension/background.js
Line: 693:696

Comment:
**Keepalive calls `restoreState` which may clobber in-memory state**

When the keepalive ping detects a lost debugger, it calls `restoreState()` which reads from `chrome.storage.session` and rebuilds the entire in-memory `tabs`/`tabBySession` maps. However, the current in-memory state may differ from what's persisted (e.g., tabs added/removed since last `persistState` call, or a reconnection is in progress). `restoreState` also calls `ensureRelayConnection()` internally, which could conflict with an active reconnection attempt.

Consider whether `cleanupTab(tabId)` (to remove just the lost tab) or a more targeted recovery is more appropriate here than a full state restore.

How can I resolve this? If you propose a fix, please make it concise.