fix(daemon): require uuid proof for ALL kill escalation; restartDaemon aborts on failed stop

Round-3 review findings on PR #18, both verified real:

1. The accepted-shutdown branch treated any 2xx as identity proof — a
   catch-all local service on a stale lock's reused port can 200 a POST
   /daemon/shutdown while ignoring the bearer header, and after the
   lock-release wait expired the recorded (possibly recycled) pid was
   killed. The accepted/rejected distinction no longer matters for
   escalation: EVERY kill now requires /daemon/health (authenticated
   with the lockfile's bearer) to echo the lockfile's uuid. Costs
   nothing in the legit path: a healthy daemon dies during the wait, a
   wedged one still answers health. Graceful wait is test-tunable.

2. restartDaemon ignored stopDaemon's StopResult: when the old daemon
   survived SIGKILL (stopped:false leaves the lock in place),
   ensureDaemon would reconnect to that same wedged process and report
   a successful restart. It now throws with the stop reason; the
   restartDaemon command surfaces the error instead of claiming success.

3 new tests: impostor-2xx must not kill; wedged-but-proven daemon still
escalates; restart aborts when stop fails.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: ARHAEEM

packages/extension/src/extension.ts

@@ -571,8 +571,12 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
             dashboardProvider.refresh();
         }),
         vscode.commands.registerCommand('airtable-formula.restartDaemon', async () => {
-            await daemonManager.restartDaemon();
-            vscode.window.showInformationMessage('Airtable Formula: Daemon restarted.');
+            try {
+                await daemonManager.restartDaemon();
+                vscode.window.showInformationMessage('Airtable Formula: Daemon restarted.');
+            } catch (err) {
+                vscode.window.showErrorMessage(`Airtable Formula: Daemon restart failed — ${err instanceof Error ? err.message : String(err)}`);
+            }
             dashboardProvider.refresh();
         }),
         vscode.commands.registerCommand('airtableFormula.tunnel.disable', async () => {

packages/extension/src/mcp/daemon-manager.ts

@@ -51,6 +51,8 @@ export class DaemonManager implements vscode.Disposable {
    * any explicit start/restart.
    */
   private _userStopped = false;
+  /** Graceful-shutdown wait before kill escalation (overridable in tests). */
+  private _stopWaitMs = 10_000;
 
   constructor(
     private readonly configDir: string,
@@ -185,28 +187,27 @@ export class DaemonManager implements vscode.Disposable {
 
     // 2) Accepted — the daemon releases its lockfile as it exits; wait for it.
     if (outcome === 'accepted') {
-      const deadline = Date.now() + 10_000;
+      const deadline = Date.now() + this._stopWaitMs;
       while (Date.now() < deadline) {
         if (!this._lockfileExists()) return { stopped: true, forced: false };
         await this._delay(200);
       }
-      // Daemon acknowledged but never exited — fall through to escalation.
+      // Acknowledged but the lock never released — fall through to escalation.
     }
 
     const pid = status.pid;
     const pidAlive = typeof pid === 'number' && pid > 0 && this._isPidAlive(pid);
 
-    // 3) Escalate to kill ONLY with proven daemon identity. An accepted
-    //    (bearer-authenticated) shutdown proves it. A rejected response does
-    //    NOT — it only proves SOMETHING answered on the port; a stale lock
-    //    whose port was reused by an unrelated HTTP service also rejects,
-    //    while the recorded pid may belong to an innocent recycled process.
-    //    For rejected, require /daemon/health to echo the lockfile's uuid.
-    const provenOurDaemon = outcome === 'accepted' || (
-      outcome === 'rejected'
+    // 3) Escalate to kill ONLY with proven daemon identity: /daemon/health,
+    //    authenticated with the lockfile's bearer, echoing the lockfile's
+    //    uuid. NEITHER an accepted nor a rejected shutdown response proves
+    //    identity by itself — a stale lock whose port was reused by an
+    //    unrelated local service can produce either (catch-all routes 200
+    //    anything and ignore the bearer header), while the recorded pid may
+    //    belong to an innocent recycled process.
+    const provenOurDaemon = outcome !== 'unreachable'
       && status.port != null && status.bearerToken != null
-      && await this._verifyDaemonIdentity(status.port, status.bearerToken, status.uuid)
-    );
+      && await this._verifyDaemonIdentity(status.port, status.bearerToken, status.uuid);
 
     if (provenOurDaemon && pidAlive && typeof pid === 'number') {
       this._killPid(pid);
@@ -264,7 +265,12 @@ export class DaemonManager implements vscode.Disposable {
   }
 
   async restartDaemon(): Promise<DaemonConnectionInfo> {
-    await this.stopDaemon();
+    const stop = await this.stopDaemon();
+    if (!stop.stopped) {
+      // Proceeding would let ensureDaemon() find the old daemon's lockfile
+      // and "restart" by reconnecting to the very process that refused to die.
+      throw new Error(`Restart aborted — the running daemon could not be stopped: ${stop.reason ?? 'unknown reason'}`);
+    }
     await this._delay(500);
     return this.ensureDaemon();
   }

packages/extension/src/test/daemon-manager.test.ts

@@ -177,6 +177,49 @@ describe('DaemonManager.stopDaemon', () => {
     expect(lockExists()).toBe(false);
   });
 
+  it('accepted shutdown from an impostor (2xx, no uuid proof): does NOT kill, reclaims the lock', async () => {
+    // A catch-all local service can 200 a POST /daemon/shutdown while
+    // ignoring the bearer header — and it will never release our lockfile.
+    const port = await listen((req, res) => {
+      res.writeHead(200, { 'Content-Type': req.url === '/daemon/health' ? 'text/html' : 'application/json' });
+      res.end(req.url === '/daemon/health' ? '<html>not the daemon</html>' : '{"ok":true}');
+    });
+    writeLock(port, process.pid);
+    (dm as any)._stopWaitMs = 300;
+    (dm as any)._killPid = vi.fn();
+    (dm as any)._isPidAlive = vi.fn(() => true);
+
+    const result = await dm.stopDaemon();
+    expect((dm as any)._killPid).not.toHaveBeenCalled();
+    expect(result.stopped).toBe(true);
+    expect(result.reason).toBeTruthy();
+    expect(lockExists()).toBe(false);
+  });
+
+  it('accepted shutdown but wedged daemon (uuid proven): escalates to kill', async () => {
+    const port = await listen((req, res) => {
+      if (req.method === 'GET' && req.url === '/daemon/health') {
+        res.writeHead(200, { 'Content-Type': 'application/json' });
+        res.end(JSON.stringify({ ok: true, uuid: 'uuid-1' }));
+        return;
+      }
+      res.writeHead(200, { 'Content-Type': 'application/json' });
+      res.end('{"ok":true}');
+      // Wedged: never releases the lockfile
+    });
+    writeLock(port, 23456);
+    (dm as any)._stopWaitMs = 300;
+    let killed = false;
+    (dm as any)._killPid = vi.fn(() => { killed = true; });
+    (dm as any)._isPidAlive = vi.fn(() => !killed);
+
+    const result = await dm.stopDaemon();
+    expect((dm as any)._killPid).toHaveBeenCalledWith(23456);
+    expect(result.stopped).toBe(true);
+    expect(result.forced).toBe(true);
+    expect(lockExists()).toBe(false);
+  });
+
   it('unreachable daemon with dead pid: reclaims the stale lock without killing anything', async () => {
     writeLock(1, 999_999); // port 1 — nothing listening
     (dm as any)._killPid = vi.fn();
@@ -201,6 +244,27 @@ describe('DaemonManager.stopDaemon', () => {
   });
 });
 
+describe('DaemonManager.restartDaemon', () => {
+  let tmpDir: string;
+  let dm: InstanceType<typeof DaemonManager>;
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-daemon-restart-'));
+    dm = new DaemonManager(tmpDir, '/tmp/test-ext-path');
+  });
+
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it('aborts (throws) when the running daemon could not be stopped', async () => {
+    (dm as any).stopDaemon = vi.fn(async () => ({ stopped: false, forced: true, reason: 'process 1 did not exit' }));
+    (dm as any)._spawnDetached = vi.fn();
+    await expect(dm.restartDaemon()).rejects.toThrow(/could not be stopped/);
+    expect((dm as any)._spawnDetached).not.toHaveBeenCalled();
+  });
+});
+
 describe('DaemonManager user-stopped latch', () => {
   let tmpDir: string;
   let dm: InstanceType<typeof DaemonManager>;