fix(respawn): do not revive a stopped controller after a cycle-step write

Each cycle step (kickstart, update, /clear, /init) checks for `stopped` before
`await session.writeViaMux(...)`, then emits `stepSent` and calls
`setState('waiting_*')` after it.

stop() is asynchronous with respect to that await. One that lands while the
write is in flight has already passed the guard that ran, so the post-await
setState() puts a stopped controller back into a waiting state — re-arming its
step timers against a session the user asked to stop.

Re-check after the await, before emitting and setting state.

The guard reads the public `state` getter rather than `_state` on purpose:
TypeScript narrows `_state` across the await from the pre-await check and cannot
see that stop() mutated it, so `this._state === 'stopped'` is rejected as a
comparison with no overlap (TS2367) at all four sites.

Adds test/respawn-stop-race.test.ts, which drives the interleaving
deterministically by calling stop() from inside the mocked write rather than
relying on timing. All four steps go red without these guards.
This commit is contained in:
Aamer Akhter
2026-08-18 10:59:44 -04:00
parent 5080390e2c
commit ce405a4cff
2 changed files with 106 additions and 0 deletions
+20
View File
@@ -1624,6 +1624,11 @@ export class RespawnController extends EventEmitter {
const prompt = this.config.kickstartPrompt!; const prompt = this.config.kickstartPrompt!;
this.logAction('command', `Sending kickstart: "${prompt.substring(0, 40)}..."`); this.logAction('command', `Sending kickstart: "${prompt.substring(0, 40)}..."`);
await this.session.writeViaMux(prompt + '\r'); // \r triggers key.return in Ink/Claude CLI await this.session.writeViaMux(prompt + '\r'); // \r triggers key.return in Ink/Claude CLI
// COD-51: stop() may have run during the await; re-check before reviving the
// state machine. Reads the public getter, not `_state`: TypeScript narrows
// `_state` across the await from the guard above and cannot see that stop()
// mutated it, so the comparison would be flagged as impossible.
if (this.state === 'stopped') return;
this.emit('stepSent', 'kickstart', prompt); this.emit('stepSent', 'kickstart', prompt);
this.setState('waiting_kickstart'); this.setState('waiting_kickstart');
this.promptDetected = false; this.promptDetected = false;
@@ -2833,6 +2838,11 @@ export class RespawnController extends EventEmitter {
const input = updatePrompt + '\r'; // \r triggers Enter in Ink/Claude CLI const input = updatePrompt + '\r'; // \r triggers Enter in Ink/Claude CLI
this.logAction('command', `Sending: "${updatePrompt.substring(0, 50)}..."`); this.logAction('command', `Sending: "${updatePrompt.substring(0, 50)}..."`);
await this.session.writeViaMux(input); await this.session.writeViaMux(input);
// COD-51: stop() may have run during the await; re-check before reviving the
// state machine. Reads the public getter, not `_state`: TypeScript narrows
// `_state` across the await from the guard above and cannot see that stop()
// mutated it, so the comparison would be flagged as impossible.
if (this.state === 'stopped') return;
this.emit('stepSent', 'update', updatePrompt); this.emit('stepSent', 'update', updatePrompt);
this.setState('waiting_update'); this.setState('waiting_update');
this.promptDetected = false; this.promptDetected = false;
@@ -2860,6 +2870,11 @@ export class RespawnController extends EventEmitter {
if (this._state === 'stopped') return; if (this._state === 'stopped') return;
this.logAction('command', 'Sending: /clear'); this.logAction('command', 'Sending: /clear');
await this.session.writeViaMux('/clear\r'); // \r triggers Enter in Ink/Claude CLI await this.session.writeViaMux('/clear\r'); // \r triggers Enter in Ink/Claude CLI
// COD-51: stop() may have run during the await; re-check before reviving the
// state machine. Reads the public getter, not `_state`: TypeScript narrows
// `_state` across the await from the guard above and cannot see that stop()
// mutated it, so the comparison would be flagged as impossible.
if (this.state === 'stopped') return;
this.emit('stepSent', 'clear', '/clear'); this.emit('stepSent', 'clear', '/clear');
this.setState('waiting_clear'); this.setState('waiting_clear');
this.promptDetected = false; this.promptDetected = false;
@@ -2902,6 +2917,11 @@ export class RespawnController extends EventEmitter {
if (this._state === 'stopped') return; if (this._state === 'stopped') return;
this.logAction('command', 'Sending: /init'); this.logAction('command', 'Sending: /init');
await this.session.writeViaMux('/init\r'); // \r triggers Enter in Ink/Claude CLI await this.session.writeViaMux('/init\r'); // \r triggers Enter in Ink/Claude CLI
// COD-51: stop() may have run during the await; re-check before reviving the
// state machine. Reads the public getter, not `_state`: TypeScript narrows
// `_state` across the await from the guard above and cannot see that stop()
// mutated it, so the comparison would be flagged as impossible.
if (this.state === 'stopped') return;
this.emit('stepSent', 'init', '/init'); this.emit('stepSent', 'init', '/init');
this.setState('waiting_init'); this.setState('waiting_init');
this.promptDetected = false; this.promptDetected = false;
+86
View File
@@ -0,0 +1,86 @@
/**
* COD-51: stop() landing DURING a cycle step's write must not revive the machine.
*
* Each cycle step (kickstart, update, /clear, /init) guards on `stopped` before
* `await session.writeViaMux(...)`, then emits `stepSent` and calls
* `setState('waiting_*')` after it. `stop()` is asynchronous with respect to
* that await: a stop that lands while the write is in flight passed the guard
* that already ran, so the post-await `setState()` puts a stopped controller
* back into a waiting state, re-arming its timers against a session the user
* asked to stop.
*
* The race is driven deterministically here by stopping from inside the mocked
* write itself — that is exactly the interleaving, without leaning on timing.
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
vi.mock('node:child_process', async (orig) => {
const actual = await orig<typeof import('node:child_process')>();
return { ...actual, exec: vi.fn((_cmd: string, cb?: (e: Error | null, o: string) => void) => cb?.(null, '')) };
});
import { RespawnController } from '../src/respawn-controller.js';
import { Session } from '../src/session.js';
import { MockSession } from './mocks/index.js';
describe('COD-51 respawn stop() race', () => {
let session: MockSession;
let controller: RespawnController;
beforeEach(() => {
session = new MockSession();
controller = new RespawnController(session as unknown as Session, {
idleTimeoutMs: 100,
interStepDelayMs: 10,
completionConfirmMs: 10,
noOutputTimeoutMs: 500,
aiIdleCheckEnabled: false,
// sendKickstart dereferences this before it reaches the write.
kickstartPrompt: 'continue',
});
});
afterEach(() => controller.stop());
/**
* Run `step`, stopping the controller from inside the write it awaits.
*
* The step methods are SYNCHRONOUS: they set `sending_*` and schedule a
* `step-delay` timer, and the write happens inside that callback. So the race
* only exists once the timer has fired — hence the settle below rather than a
* bare `await step()`, which returns before anything interesting happens.
*/
async function stopDuringWrite(step: () => void) {
const stepSent = vi.fn();
controller.on('stepSent', stepSent);
const wrote = new Promise<void>((resolve) => {
vi.spyOn(session, 'writeViaMux').mockImplementation(async () => {
controller.stop();
resolve();
return true;
});
});
step();
await wrote;
// Let the continuation after the await run before asserting on it.
await new Promise((r) => setTimeout(r, 20));
return stepSent;
}
it.each([
['update', () => (controller as never as { sendUpdateDocs(): void }).sendUpdateDocs()],
['clear', () => (controller as never as { sendClear(): void }).sendClear()],
['init', () => (controller as never as { sendInit(): void }).sendInit()],
['kickstart', () => (controller as never as { sendKickstart(): void }).sendKickstart()],
])('a stop during the %s write leaves the controller stopped', async (_label, step) => {
(controller as never as { _state: string })._state = 'watching';
const stepSent = await stopDuringWrite(step);
// The observable damage is a revived state machine: `waiting_*` re-arms the
// step timers, so the cycle keeps driving a session the user stopped.
expect(controller.state).toBe('stopped');
expect(controller.isRunning).toBe(false);
expect(stepSent).not.toHaveBeenCalled();
});
});