Preserve background tasks across daemon reconnects - #1165
Conversation
|
🚨 SLOP COP 🚨 · ELI5: A daemon reconnect is like a phone call that drops. This change keeps its long task alive after the new call starts. I am SlopCop. I am reviewing this pull request for security, code quality, performance, tests, and architectural duplication. Three BB child threads will inspect these areas in parallel. I will post one final review after I verify their findings. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: A daemon reconnect is like a phone call that drops. This change keeps a long job alive after the new call starts.
I found no blocking product defect. The change keeps server policy in the server. It also adds a small socket-only hub method. I found no useful duplicate code or larger refactor.
I left two low-risk test comments:
- Wait for the replacement WebSocket before the follow-up turn.
- Give the real-provider test a larger timeout.
Security, code-quality, and performance threads reviewed the change. A separate GPT-5.6 gate confirmed the two test risks and found no product defect.
Targeted server tests passed with 11 tests. Contract tests passed with 49 tests. Server, contract, and integration type checks passed. The dev app loaded in a browser. All GitHub checks are green.
The real Claude test stopped before the reconnect scenario. The local Claude OAuth session had expired. This failure does not show a pull-request defect.
Summary
session-closeand task reconciliation behavior when a different daemon instance takes overRoot cause
Opening a replacement server session always sent
session-close:replacedto the previous socket. On an ordinary reconnect, that socket belonged to the same live daemon instance. The daemon treated the message as terminal and shut down every environment runtime, which killed background commands before its supervisor restarted it.The server already has both session instance IDs, so it can tell these cases apart. A same-instance replacement now closes only the stale transport. A different-instance replacement still shuts down the superseded daemon and settles its work.
Reproduction
I reproduced this with standalone dev servers and the
bbCLI against both the v0.35.1 tag and this branch:session-close:replaceddaemon shutdownTests
@bb/serverlifecycle and hub regression tests: 32 passed@bb/host-daemon-contracttests: 49 passed@bb/server,@bb/host-daemon-contract, and@bb/integration-testsPATHCloses #1127