Repository navigation
Conversation
sendCommand's ECONNRESET handler treats every dropped connection as a crash and restarts the daemon. But stop and restart are user-initiated shutdowns: the daemon intentionally closes the HTTP connection before the response body flushes (meta-commands.ts:423-433). The restart attempt sends stop again, the new daemon exits again, and retries >= 1 throws 'Server crashed twice in a row — aborting.' Fix: add a short-circuit check for command === 'stop' || command === 'restart' at the top of the ECONNRESET handler. When user-intended, call process.exit(0) to skip main()'s surrounding .catch(), which would print '[browse] fetch failed' and exit 1. 'command' is already in closure from sendCommand's signature (cli.ts:527). The recursive calls at cli.ts:596 and :613 pass 'command' explicitly. Daemon-side handlers (meta-commands.ts) are unchanged. No 'chore: rebuild' commit — CI's windows-setup-e2e.yml rebuilds browse binary on every push touching browse/src/cli.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mands Four test cases for the sendCommand ECONNRESET fix: 1. stop command on ECONNRESET does not print crash warning — runs browse stop against a real daemon; asserts exit 0 and absence of 'crashed' / 'aborting' / 'fetch failed' in stderr. 2. restart command on ECONNRESET does not print crash warning — same assertion for the restart command. 3. guard is scoped to stop + restart only (regression guard) — reads browse/src/cli.ts and asserts the new short-circuit checks both 'stop' and 'restart', and that the original ECONNREFUSED / ECONNRESET handler is preserved. 4. guard preserves original crash-retry path (regression guard) — asserts 'Server crashed twice in a row' and the 'retries >= 1' guard are still present. Cases 1-2 use describe.skipIf(!HAS_BINARY) so shards without a built binary gracefully skip. Cases 3-4 always run as source-shape checks, providing a safety net against future refactors that accidentally widen the short-circuit or strip the crash-retry path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Just reports succesful whenevr the connection fails. Even if the daemon never recieved the stop command |
|
@16francej — recommend closing this PR because it can report a successful stop or restart without proving the daemon processed the command. The available coverage checks only the client exit behavior, not handler execution or resulting process state, so it does not safely resolve #1732. |
|
Thanks for this, @TTmo123. The fix is now on main: stop is acknowledged before the daemon exits, a stop against a dead daemon exits cleanly, and stop short-circuits when no daemon runs (browse/src/cli.ts, meta-commands.ts) (fixed in v1.65.0.0, #2577 and v1.67.0.0, #2604). Closing as superseded; if you still see the problem on the latest gstack, please open an issue with ./setup --status output. |
2 participants
What
browse stopandbrowse restartno longer trigger the "Server crashed twice in a row — aborting" warning.Why
Today's
sendCommandECONNRESET handler (browse/src/cli.ts:586) treats every dropped connection as a crash. Butstopandrestartare user-initiated shutdowns (daemon-side atbrowse/src/meta-commands.ts:423,:428): the daemon intentionally exits before the HTTP response body flushes. The restart path then sendsstopagain, the new daemon exits again, andretries >= 1throws the misleading crash message.How
One 4-line branch at the top of the ECONNRESET handler:
commandis already in closure fromsendCommand's function signature (cli.ts:527) — no new param passing.process.exit(0)(not a barethrow) is intentional:main()'s.catch()(cli.ts:1357-1362) would print[browse] fetch failedand exit 1 on a thrown error. Clean exit 0 is what the user intended.Verification
bun test test/send-command-stop-skip-restart.test.ts— 4 new cases pass (2 integration + 2 regression-guard)bun test— full Tier 1 suite passesbun build browse/src/cli.ts— TypeScript compiles cleanlybun run test:e2e(Tier 2) — requires EVALS secret, maintainer-side gatebrowse stop— exits 0, no stderr outputbrowse goto <url>— crash-retry still fires (unchanged)Notes for reviewer
meta-commands.ts:423-433) are unchanged. The"Server stopped"/"Restarting..."strings rarely reach the client becauseshutdown()exits before the response body flushes; the fix respects that as an intentional daemon behavior.chore: rebuildcommit:.gitignoreexcludesbrowse/dist/, and CI'swindows-setup-e2e.ymlrebuilds binaries on every push touchingbrowse/src/cli.ts(trigger path at.github/workflows/windows-setup-e2e.yml:22).describe.skipIf(!HAS_BINARY)so shards without a built binary gracefully skip; cases 3-4 are code-shape regression guards that always run, catching future refactors that accidentally widen the short-circuit or strip the crash-retry path.