Skip to content

fix(daemon): enforce session idle timeout#22

Open
NianJiuZst wants to merge 3 commits into
Tencent:mainfrom
NianJiuZst:codex/enforce-session-idle-timeout
Open

fix(daemon): enforce session idle timeout#22
NianJiuZst wants to merge 3 commits into
Tencent:mainfrom
NianJiuZst:codex/enforce-session-idle-timeout

Conversation

@NianJiuZst

@NianJiuZst NianJiuZst commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

bsk daemon start --session-idle <duration> exposed a session inactivity setting and stored it in DaemonConfig, but the runtime never read that value. Sessions therefore remained registered indefinitely until a caller explicitly stopped them, the browser disconnected, or the Agent Window closed.

That is more than a stale-list issue. A live session owns an Agent Window and may own tabs borrowed from the user's normal browser windows. When an agent disappears without calling session stop, the documented five-minute default did not release those resources. The presence of the leaked session also prevented the daemon's separate idle shutdown from firing.

How This Fix Works

  • Track a monotonic last-activity timestamp for every registered session without changing the public Session status/wire shape.
  • Refresh activity when a tool request is accepted and again when it completes. The completion touch is important for long-running calls: an active call may cross the idle threshold, but it should receive a fresh inactivity window after it finishes.
  • Start a cooperative session-idle reaper in both the production foreground/background runtime and the embedded/test daemon entry point.
  • When a session reaches the configured threshold, use the existing normal stop_session path rather than deleting registry state directly. This asks the extension to tear down the Agent Window and return borrowed tabs before unregistering the session.
  • If a tool is still in flight, stop_session reports SessionBusy; the reaper leaves the session intact and retries on a later sweep instead of cancelling active work.
  • Reconcile sessions whose browser has already gone, while retaining sessions when teardown reports tab-return failures or another recoverable extension error.
  • Abort and join the reaper during daemon shutdown so the embedded runtime does not leak a detached task.

User Impact

The existing --session-idle flag and its five-minute default now behave as documented. Abandoned Agent Windows and borrowed tabs are cleaned up automatically, active tools are not force-stopped, and successfully reaped sessions no longer keep the daemon alive forever.

CI Baseline Compatibility

This branch also contains the one-line Biome 2.4 formatting update required by the repository's current lockfile. upstream/main currently fails the Frontend CI job on this exact pre-existing formatting mismatch; carrying the minimal formatter output is necessary for this PR's checks to execute past lint.

Validation

  • Added an end-to-end WebSocket/daemon test with a 50 ms idle threshold. It verifies that the reaper sends tool.session_stop, accepts the extension response, and unregisters the session.
  • cargo fmt --check
  • cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test --workspace --locked
  • node --test scripts/*.test.mjs
  • pnpm lint
  • pnpm --filter @browser-skill/extension compile
  • pnpm ext:test (35 files, 373 tests)
  • pnpm ext:build

CI Reliability Follow-up

GitHub Actions exposed a pre-existing test-only port allocation race: integration helpers probed an ephemeral port, released it, and then asked the daemon to bind the same number, allowing another process to claim it in between. The resulting Address already in use failure was unrelated to the functional changes. The PR now lets each daemon bind port 0 directly and reads the assigned address from its handle, removing the TOCTOU window across all affected integration helpers.

@NianJiuZst NianJiuZst marked this pull request as ready for review July 10, 2026 14:58
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