feat(session-ingest): route fenced browser providers - #5648
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| this.ensureState(); | ||
|
|
||
| const now = Date.now(); | ||
| await this.runBrowserOperation(async () => { |
There was a problem hiding this comment.
WARNING: Browser expire/cleanup failures abort the entire alarm() handler
expire() / cleanup() (and restoreBrowserState() inside runBrowserOperation) can throw BrowserJobStoreError on parse failure, a deadline row whose job is missing, or capacity. That rejection skips expirePendingCommands, stale-CLI eviction, and scheduleNextAlarm.
Cloudflare retries a throwing alarm(), so a persistent ledger fault becomes a poison pill: heartbeat timeouts and pending-command TTLs never run. fireReadyPushes and the durable pending-command sweep in this same handler already isolate failures; this new block should too.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 431610a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 431610a)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Reviewed by grok-4.6 · Input: 94.3K · Output: 13.5K · Cached: 446.3K Review guidance: REVIEW.md from base branch |
1 participant
No new behavior — this change prepares browser tasks but does not add controls that people can use.
Summary
browserJobsV1now negotiates authenticatedBrowserRequestandBrowserProviderOutboundMessagerouting throughheartbeat_ackandpong; legacy clients retain unchanged responses.WSAttachmentandBrowserProviderBindingpreserve parent-authorized delivery, provider generations, and dispatch acknowledgement through hibernation and subscription races; reconnects alone cannot transfer authority.The ledger enforces approval, cancellation, and recovery without dispatch replay; one composed alarm preserves browser deadlines, retention, and legacy alarms.
Files
services/session-ingest/src/dos/UserConnectionDO.ts— Source; modified (M); 608 changed lines (+506/−102). Routeslist,invoke,status,cancel, andrecover. Missing capabilities and failed acknowledgements disable browser traffic. Roles separate requests from provider updates; parent proofs, not heartbeats or connection identifiers, authorize access. Discovery lists labels, identifiers, availability, and queue counts separately from command-line interface (CLI) instances. Registration binds one provider and generation per socket; replies, progress, results, lease acknowledgements, and bounded snapshots remain private. Serialized commits precede sends; acceptance precedes dispatch. Each invocation needs tab approval; only quiescence advances the queue. Cancellation settles before forwarding and rejects late success; uncertain cancellation interrupts queued jobs. Provider loss, replacement, revocation, disablement, or shutdown settles jobs without clearing uncertain execution fences. Provider recovery requires proof plus confirmation of tab closure and drained locks. CLI loss preserves jobs; proven lookup or recovery rebinds delivery. Hibernation preserves acknowledged work, resends pending cancellations, and interrupts unacknowledged dispatch without replay. Heartbeats preserve socket identity; reconstruction ignores replaced sockets, and CLI lookups select open, unreplaced sockets. Delayed subscriptions reload attachments to preserve negotiation, registration, dispatch acknowledgement, and close or replacement markers. The scheduler replaces competing writers and combines queue, approval, execution, lease, and retention deadlines with heartbeats, commands, and ready pushes. It restores ready pushes, includes completed commands, ignores malformed deadlines, schedules overdue work now, and deletes empty alarms. Legacy dispatch does not wait for alarm reads. Browser errors preserve retryability and omit proofs; provider errors retain the legacy response envelope.services/session-ingest/src/dos/UserConnectionDO.test.ts— Test; modified (M); 1,421 changed lines (+1,411/−10). Adds coverage for negotiation, private discovery, parent isolation, provider generations, approval, cancellation, recovery, deadlines, hibernation, reconnects, and delayed subscriptions. The mocks clone attachments, track closed sockets, serialize atomic transactions, and expose alarm state. The helpers accept authenticated CLI identities; legacy assertions await scheduling, and a regression case checks command dispatch during delayed alarm reads.The default suite restricts
UserConnectionDOalarm writes toscheduleNextAlarmthrough a TypeScript abstract syntax tree (AST) inventory.setAlarmanddeleteAlarmaliases count as writers; unrelated text and other Durable Objects do not.Independent writers, a missing target class, and an empty inventory fail the check, so new scheduling code must use the composed scheduler.
Files
services/session-ingest/src/dos/browser-alarm-inventory.test.ts— Test; added (A); 187 changed lines (+187/−0). Adds the inventory and synthetic checks for direct, computed, destructured, bound, chained, and class-field aliases. Checks reject missing or empty inventories and exclude other Durable Object classes.Tests: 2 files changed —
UserConnectionDO.test.tsmodified andbrowser-alarm-inventory.test.tsadded; 1,598 insertions and 10 deletions.Generated: 0 files changed.
Visual Changes
Visual Changes: N/A
Verification
Reviewer Notes
Human steps
Automated evidence
pnpm --filter cloudflare-session-ingest exec vitest run --no-cache --configLoader runner src/dos/UserConnectionDO.test.ts src/dos/browser-alarm-inventory.test.ts.Scope and dependencies
/Users/igor/Projects/.worktrees/browser-task-0787, branchbrowser-task-0787-s3, basebrowser-task-0787-s2. This description covers level 3 only./Users/igor/Projects/.worktrees/browser-task-0787-kilocode, branchbrowser-task-0787. Its matching contracts have landed; this worktree provides contract context only.Notes
Runtime verification remains pending on the stack tips. Browser traffic requires explicit capability negotiation; client integration follows in later levels.
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Runtime verification (E2E, user advocacy, simplify) runs on the tip PR over every level.
Every level keeps its own checks, its own bot review, and its own threads; each one is answered on its own PR.
Each level is its own deliverable: it builds and passes its own checks alone.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
browser-task-0787— feat(browser-task): define negotiated job contracts #5638browser-task-0787-s2— feat(session-ingest): persist owned browser jobs #5644browser-task-0787-s3— feat(session-ingest): route fenced browser providers #5648 ← this PRbrowser-task-0787-s4— feat(cloud-agent-sdk): support browser provider jobs #5653browser-task-0787-s7— fix(extension): expose honest browser runner outcomes #5681 (tip)