fix(discord): keep the thread of a discarded rebuild, close one made for a closed tab, unique tmp names, wider prune keep set

docs/discord-bridge.md

@@ -122,7 +122,7 @@ The text explains `codeman notify "message" --urgent`. `codeman` is on the PATH
 ## Rotating and revoking
 
 - **Rotate:** in the Developer Portal choose **Bot**, **Reset Token**, paste the new token into Codeman and press **Send test**.
-- **Revoke:** kick the bot from the server or delete the application, then delete `~/.codeman/discord.json`.
+- **Revoke:** kick the bot from the server or delete the application, then choose **Remove token** in Settings. Deleting `~/.codeman/discord.json` alone does not reach a running bridge, which keeps the token it loaded until Codeman restarts.
 - If the token is ever pasted somewhere it should not be, reset it straight away.
 
 ## Troubleshooting

src/discord-bridge.ts

@@ -22,6 +22,7 @@
 
 import { existsSync, mkdirSync } from 'node:fs';
 import fs from 'node:fs/promises';
+import { randomUUID } from 'node:crypto';
 import { join } from 'node:path';
 import { probePathKind } from './utils/bounded-path-probe.js';
 import WebSocket from 'ws';
@@ -294,7 +295,8 @@ export async function readDiscordConfig(configDir: string): Promise<DiscordConfi
 }
 
 async function writePrivate(target: string, data: unknown, mode: number): Promise<void> {
-  const tmp = `${target}.${process.pid}.tmp`;
+  // A unique name: two writes at once (two admin tabs) must not share one tmp file.
+  const tmp = `${target}.${randomUUID()}.tmp`;
   try {
     await fs.writeFile(tmp, JSON.stringify(data, null, 2), { mode });
     await fs.rename(tmp, target);
@@ -679,6 +681,8 @@ export class DiscordBridge {
   /** Brand-new sessions that have not yet been sent the startup hint (non-Claude CLIs). */
   private fresh = new Set<string>();
   private pending = new Map<string, Promise<string | DiscordResult>>();
+  /** Tabs closed while their thread was still being created: the creation applies `onSessionDelete` to it. */
+  private closedWhilePending = new Set<string>();
   /** The parent channel the thread map was built under. */
   private threadsParent = '';
   /** True from boot until the restored tabs exist (see hold). */
@@ -822,6 +826,10 @@ export class DiscordBridge {
         if (!t?.id) return this.fail(new Error('Discord did not return a thread'));
         // A save moved the bridge to another channel meanwhile: this thread belongs to the old one.
         if (parent !== this.cfg.parentChannelId) return this.fail(new Error('The parent channel changed'));
+        if (this.closedWhilePending.has(sessionId)) {
+          await this.closeThread(t.id);
+          return this.fail(new Error('The tab was closed'));
+        }
         this.threads[sessionId] = t.id;
         await this.persistThreads();
         return t.id;
@@ -829,7 +837,10 @@ export class DiscordBridge {
         return this.fail(err);
       } finally {
         // Only this creation's own entry: a newer one for the same tab may have replaced it.
-        if (this.pending.get(sessionId) === mine.p) this.pending.delete(sessionId);
+        if (this.pending.get(sessionId) === mine.p) {
+          this.pending.delete(sessionId);
+          this.closedWhilePending.delete(sessionId);
+        }
       }
     })());
     this.pending.set(sessionId, p);
@@ -940,16 +951,31 @@ export class DiscordBridge {
     await this.api.rename(thread, threadNameFor(name, sessionId)).catch(() => undefined);
   }
 
-  /** Session tab deleted: apply `onSessionDelete` and forget the mapping. */
-  async sessionDeleted(sessionId: string): Promise<void> {
-    // Per-tab state goes whether or not the tab ever had a thread, so these maps cannot grow for the life of the server.
+  /**
+   * Drop the per-tab state only, keeping the thread and its map entry. For a session that is discarded
+   * but can come back under the same id (a failed reboot-restore rebuild stays on offer for a retry).
+   * Per-tab state goes whether or not the tab ever had a thread, so these maps cannot grow for the life of the server.
+   */
+  forgetTab(sessionId: string): void {
     this.lastReply.delete(sessionId);
     this.names.delete(sessionId);
     this.fresh.delete(sessionId);
+  }
+
+  /** Session tab deleted: apply `onSessionDelete` and forget the mapping. */
+  async sessionDeleted(sessionId: string): Promise<void> {
+    this.forgetTab(sessionId);
+    // A thread still being created is closed by its creation, which finds no entry to remove here.
+    if (this.pending.has(sessionId)) this.closedWhilePending.add(sessionId);
     const thread = this.threads[sessionId];
     if (!thread) return;
     delete this.threads[sessionId];
     await this.persistThreads();
+    await this.closeThread(thread);
+  }
+
+  /** Apply `onSessionDelete` to a thread. */
+  private async closeThread(thread: string): Promise<void> {
     if (!this.cfg.enabled) return;
     if (this.cfg.onSessionDelete === 'delete') await this.api.remove(thread).catch(() => undefined);
     else if (this.cfg.onSessionDelete === 'archive') await this.api.archive(thread).catch(() => undefined);

src/web/server.ts

@@ -3261,7 +3261,14 @@ export class WebServer extends EventEmitter {
       await this.finalizeRestoredState(restored);
       void this.discordBridge
         .release()
-        .then(() => this.discordBridge.prune(this.sessions.keys()))
+        .then(() =>
+          // Same keep set as the agent inbox prune: a reboot-restore rebuild reuses the old id.
+          this.discordBridge.prune([
+            ...this.sessions.keys(),
+            ...Object.keys(this.store.getSessions()),
+            ...rebootRestoreRegistry.list(() => true).map((entry) => entry.sessionId),
+          ])
+        )
         .catch(() => undefined);
 
       // Instance-scoped reaper: after restore, `docker rm -f` managed containers of
@@ -3847,8 +3854,9 @@ export class WebServer extends EventEmitter {
     } catch (err) {
       console.warn(`[Server] releasing the tab layout slot failed: ${getErrorMessage(err)}`);
     }
-    // A half-built session never had a thread, but its per-tab bridge state goes with it.
-    void this.discordBridge.sessionDeleted(sessionId).catch(() => undefined);
+    // Per-tab bridge state only: a reboot-restore rebuild reuses the old id and stays on offer for a
+    // retry, so its thread must survive the discard.
+    this.discordBridge.forgetTab(sessionId);
     // Any `session:updated` the half-built session emitted before it failed left a
     // tab on every other open board, and the client's handler is an upsert.
     this.broadcast(SseEvent.SessionDeleted, { id: sessionId });

test/agent-inbox-server-wiring.test.ts

@@ -50,7 +50,7 @@ function bareServer(live: string[] = [], persisted: string[] = []): CleanupServe
     persistDeb: { cancelKey: () => {} },
     sse: { cleanupSessionBatches: () => {} },
     // A real delete also tells the Discord bridge, which frees its per-tab state and may archive the thread.
-    discordBridge: { sessionDeleted: async () => {} },
+    discordBridge: { sessionDeleted: async () => {}, forgetTab: () => {} },
     broadcast: () => {},
   });
   return server;

test/discord-bridge.test.ts

@@ -663,6 +663,41 @@ describe('DiscordBridge', () => {
       b.stop();
     });
 
+    it('forgetTab drops the per-tab state but keeps the thread and its map entry', async () => {
+      await writeDiscordConfig(dir, { ...CFG, onSessionDelete: 'delete' });
+      const b = bridge();
+      await b.notify(MSG);
+      posts.length = 0;
+      b.forgetTab('s1');
+      await b.sessionRenamed('s1', 'other'); // first sight again: recorded, no PATCH
+      expect(posts).toHaveLength(0);
+      expect(await readThreadMap(dir)).toEqual({ s1: THREAD });
+      b.stop();
+    });
+
+    it('a tab closed while its thread is being created has that thread closed and no entry kept', async () => {
+      await writeDiscordConfig(dir, { ...CFG, onSessionDelete: 'archive' });
+      let release!: () => void;
+      const gate = new Promise<void>((r) => (release = r));
+      const b = bridge({}, async (url, init) => {
+        const path = new URL(String(url)).pathname.replace('/api/v10', '');
+        posts.push({ path, method: String(init.method), body: init.body ? JSON.parse(String(init.body)) : undefined });
+        if (path.endsWith('/threads')) {
+          await gate;
+          return jsonRes({ id: THREAD });
+        }
+        return jsonRes({ id: 'msg' });
+      });
+      const first = b.notify(MSG);
+      await vi.waitFor(() => expect(posts.some((p) => p.path.endsWith('/threads'))).toBe(true));
+      await b.sessionDeleted('s1');
+      release();
+      await first;
+      expect(posts).toContainEqual({ path: `/channels/${THREAD}`, method: 'PATCH', body: { archived: true } });
+      expect(await readThreadMap(dir)).toEqual({});
+      b.stop();
+    });
+
     it('renames the thread only when the name really changed', async () => {
       const b = bridge();
       await b.notify(MSG);

test/discord-push-glue.test.ts

@@ -25,6 +25,7 @@ type Internals = {
   sendPushNotifications: (event: string, data: Record<string, unknown>) => Promise<void>;
   broadcast: (event: string, data: unknown) => void;
   cleanupSession: (id: string, killMux?: boolean, reason?: string) => Promise<void>;
+  discardPartiallyBuiltSession: (id: string) => Promise<void>;
 };
 
 function server(): Internals {
@@ -35,6 +36,7 @@ function server(): Internals {
     sessionCreated: vi.fn(async () => undefined),
     sessionRenamed: vi.fn(async () => undefined),
     sessionDeleted: vi.fn(async () => undefined),
+    forgetTab: vi.fn(),
     sessionIdle: vi.fn(async () => undefined),
     stop: vi.fn(),
   };
@@ -125,6 +127,19 @@ describe('broadcast -> Discord tab lifecycle', () => {
     expect(s.discordBridge.sessionDeleted).toHaveBeenCalledWith('gone');
   });
 
+  it('a discarded half-built session clears the per-tab state but keeps its thread', async () => {
+    const s = server();
+    s.sessions.set('rebuild', {
+      owner: 'u',
+      ralphTracker: { stopWatchingFixPlan: () => undefined, fullReset: () => undefined },
+      removeAllListeners: () => undefined,
+      stop: async () => undefined,
+    });
+    await s.discardPartiallyBuiltSession('rebuild');
+    expect(s.discordBridge.forgetTab).toHaveBeenCalledWith('rebuild');
+    expect(s.discordBridge.sessionDeleted).not.toHaveBeenCalled();
+  });
+
   it('ignores an event with no session id', () => {
     const s = server();
     s.broadcast(SseEvent.SessionUpdated, {});