Skip to content

fix(browser): boot reconciliation must not orphan another replica's live session (F-06) - #273

Open
anilguleroglu wants to merge 1 commit into
mainfrom
fix/p1-replica-job-ownership
Open

fix(browser): boot reconciliation must not orphan another replica's live session (F-06)#273
anilguleroglu wants to merge 1 commit into
mainfrom
fix/p1-replica-job-ownership

Conversation

@anilguleroglu

Copy link
Copy Markdown
Collaborator

Summary

P1 finding from the 2026-09-05 finance-institution assessment.

reconcileOrphanedBrowserSessions decided a session was dead purely on whether THIS process's in-memory browserManager knew about it. A replica that just booted knows about none of them, so it marked every active-status session across every tenant "expired" the moment it ran — including sessions genuinely alive on a different, still-running replica. The assessment's isolated check reproduced exactly this: a second node's empty local session map was enough to write another node's live session as expired.

The cluster already has the infrastructure this needed — a node registry with heartbeat + online/offline tracking (src/lib/core/cluster). This PR:

  • Adds ownerNode to IBrowserSession (SQLite migration via the existing ensureTableColumn mechanism + schema.ts for fresh installs; Mongo needs no schema change).
  • Stamps it once, in the single service function all six call sites already funnel through (browserSessionService.createBrowserSession) — so no call site had to remember to set it.
  • reconcileOrphanedBrowserSessions now reads the node registry once per sweep (listClusterNodes({status:'online'})) and only expires a session when its owner node is not currently online, or the session predates this field (pre-migration rows keep the exact prior behavior — no regression for the single-node case this already worked correctly for).

Explicitly out of scope for this PR

crawlerJobReconciler.ts has the exact same bug pattern (requeues every running/queued job tenant-wide with no owner/heartbeat check, and can delete partial results) — but job requeue has failure modes session-expiry doesn't (in-flight partial results, retry/idempotency semantics), so it deserves its own review rather than being folded into this diff. Flagging as a direct follow-up using the identical ownerNode + node-registry pattern proven here.

Test plan

  • New browser-operations-reconcile.test.ts: owner node still online → not touched (the exact F-06 scenario), owner node gone → expired, legacy row with no ownerNode → expired (matches prior behavior), session known to local browserManager → never touched regardless of registry state
  • Manually verified the "owner node still online" test fails against the pre-fix code (reverted the check, confirmed red, restored)
  • npx tsc --noEmit clean
  • npx eslint clean
  • Full vitest run (including db-parity.test.ts, which exercises the SQLite/Mongo schema in both backends): 5050 passed, 0 failed, 5 skipped

🤖 Generated with Claude Code

https://claude.ai/code/session_01KQq6TnVNHRNU6Wz1eQzPpD

…ive session (F-06)

reconcileOrphanedBrowserSessions decided a session was dead purely on
whether THIS process's in-memory browserManager knew about it. A
replica that just booted knows about none of them, so it marked every
active-status session across every tenant "expired" the moment it ran
-- including ones genuinely alive on a different, still-running
replica. An isolated check reproduced this: a second node's empty
local session map was enough to write another node's live session as
expired.

The cluster already has what this needed: a node registry with
heartbeat + online/offline tracking (src/lib/core/cluster). Sessions
now record which node's browserManager actually launched them
(ownerNode, stamped once in the single service function all six call
sites already funnel through -- browserSessionService.createBrowserSession
-- so no call site had to remember to set it). Boot reconciliation
reads the node registry once per sweep and only expires a session when
its owner node is not currently online, or the session predates this
field (pre-migration rows keep the old behavior, matching what was
already correct for a genuinely single-node deployment).

Scoped to browser sessions only. crawlerJobReconciler.ts has the exact
same bug pattern (requeues every 'running'/'queued' job tenant-wide
with no owner/heartbeat check, and can delete partial results) but is
a separate, larger fix -- job requeue has failure modes session-expiry
doesn't (in-flight partial results, retry semantics) and deserves its
own review rather than being folded into this diff.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQq6TnVNHRNU6Wz1eQzPpD
@anilguleroglu
anilguleroglu requested a review from a team September 6, 2026 12:18
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