mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(browse): stop --force-restart kills the live daemon directly instead of booting a fresh one
`browse stop --force-restart` on a live-but-busy daemon fell through the
stop short-circuit into ensureServer(), whose force-restart path kills the
daemon and then STARTS A FRESH ONE (daemon + Chromium, multi-second churn)
just so sendCommand('stop') can shut it down again — the #2254 churn in
force clothing. gstack-upgrade's Step 4.8 sends users down exactly this
path when a stale daemon is busy after an upgrade.
The stop short-circuit now handles it: live pid + --force-restart → kill
the daemon (tree-kill on Windows, TERM→KILL on POSIX), reap the orphaned
Chromium + clear profile locks, remove the state file, exit 0 — no server
is ever started. Pinned in stop-dead-daemon.test.ts: a wedged live "daemon"
is killed, the state file stays gone (a booted daemon would have rewritten
it), and no Starting/Restarting output appears.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
68006e219c
commit
a5b6522afe
+20
-1
@@ -1602,7 +1602,26 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
|
||||
console.log('No daemon running (cleaned stale state) — nothing to stop.');
|
||||
process.exit(0);
|
||||
}
|
||||
// Live daemon → fall through to the normal sendCommand('stop') path.
|
||||
// stop --force-restart on a LIVE daemon (healthy or busy): kill it and
|
||||
// clean up right here. Falling through would hand ensureServer() the
|
||||
// force-restart flag, which kills the daemon and then BOOTS A FRESH ONE
|
||||
// (daemon + Chromium, multi-second churn) just so sendCommand('stop')
|
||||
// can shut it down again — the #2254 churn in force clothing, and
|
||||
// gstack-upgrade's Step 4.8 sends users down exactly this path when a
|
||||
// stale daemon is busy. The desired end state is "no daemon"; get there
|
||||
// directly.
|
||||
if (isProcessAlive(stopState.pid) && globalFlags.forceRestart) {
|
||||
await killServer(stopState.pid);
|
||||
// Reap the orphaned Chromium child + clear its profile locks so the
|
||||
// NEXT launch is clean (same cleanup as the disconnect force path).
|
||||
await killOrphanChromium();
|
||||
cleanChromiumProfileLocks();
|
||||
safeUnlinkQuiet(config.stateFile);
|
||||
console.log('Daemon stopped (forced — tabs/cookies/logins discarded).');
|
||||
process.exit(0);
|
||||
}
|
||||
// Live daemon without --force-restart → fall through to the normal
|
||||
// sendCommand('stop') path (graceful shutdown; busy semantics apply).
|
||||
}
|
||||
|
||||
// Special case: chain reads from stdin
|
||||
|
||||
@@ -15,6 +15,7 @@ import * as fs from 'fs';
|
||||
import * as net from 'net';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { isProcessAlive } from '../src/error-handling';
|
||||
|
||||
function runCli(args: string[], env: Record<string, string>, timeoutMs = 30_000):
|
||||
Promise<{ code: number; stdout: string; stderr: string }> {
|
||||
@@ -97,3 +98,47 @@ describe('#2254 stop on a dead daemon', () => {
|
||||
}
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe('stop --force-restart on a LIVE daemon', () => {
|
||||
test('kills it directly — never boots a fresh daemon just to stop it', async () => {
|
||||
// A live-but-BUSY daemon: the pid is alive but nothing answers /health.
|
||||
// Pre-fix, stop --force-restart fell through to ensureServer(), whose
|
||||
// force-restart path killed the daemon and then STARTED a fresh one
|
||||
// (daemon + Chromium) so sendCommand('stop') could stop it again —
|
||||
// exactly what gstack-upgrade Step 4.8 triggers on a stale-busy daemon.
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-stop-force-'));
|
||||
const stateFile = path.join(tmpDir, 'browse.json');
|
||||
// Portable long-lived child standing in for the wedged daemon process.
|
||||
const wedged = spawn('bun', ['-e', 'await Bun.sleep(300000)'], { stdio: 'ignore' });
|
||||
try {
|
||||
const port = await closedPort();
|
||||
fs.writeFileSync(stateFile, JSON.stringify({
|
||||
pid: wedged.pid,
|
||||
port,
|
||||
token: 'busy-token',
|
||||
startedAt: new Date().toISOString(),
|
||||
serverPath: '',
|
||||
mode: 'launched' as const,
|
||||
}, null, 2));
|
||||
|
||||
const result = await runCli(['stop', '--force-restart'], baseEnv(stateFile));
|
||||
|
||||
expect(result.code).toBe(0);
|
||||
expect(result.stdout).toContain('Daemon stopped (forced');
|
||||
// The load-bearing half: NO fresh daemon was booted to serve the stop.
|
||||
// A spawned daemon would have re-written the state file.
|
||||
expect(fs.existsSync(stateFile)).toBe(false);
|
||||
expect(result.stderr).not.toContain('Starting server');
|
||||
expect(result.stdout + result.stderr).not.toContain('Restarting');
|
||||
// And the live pid is actually gone.
|
||||
const deadline = Date.now() + 3000;
|
||||
while (Date.now() < deadline && isProcessAlive(wedged.pid!)) {
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
}
|
||||
expect(isProcessAlive(wedged.pid!)).toBe(false);
|
||||
} finally {
|
||||
try { wedged.kill('SIGKILL'); } catch { /* already gone */ }
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user