fix: settle a finished agent, and pin the dashboard markup to one bundle

src/index.ts

@@ -1917,8 +1917,18 @@ A design decision, in this shape:
         options: { codemode: true },
         execute: async (input: unknown, toolCtx: any) => {
           const { role, task, model, wait, timeout } = input as { role: string; task: string; model?: string; wait?: boolean; timeout?: number }
+          // Held OUTSIDE the try so the catch below can settle it. The agent is
+          // in `orchestrator.agents` from the moment `spawnAgent` returns, and
+          // this block marks it `working`; every exit that is not a deliberate
+          // settle — a `session.wait` that rejects, a throw from any read, a
+          // failing `storage.set` — used to leave it at `working` in
+          // `getState().agents` for the rest of the process. A finished agent
+          // that still claims to be working is a lie the page cannot see
+          // through, so the failure path settles it rather than leaking it.
+          let spawned: SpawnedAgent | null = null
           try {
             const agent = await spawnAndDeliver({ role, task, model }, toolCtx)
+            spawned = agent
 
             agent.status = 'working'
             orchestrator.notifyStateChange()
@@ -1927,23 +1937,42 @@ A design decision, in this shape:
             if (wait) {
               const waitTimeout = timeout || 120000
               const waitPromise = ctx.session.wait({ sessionID: agent.sessionID! })
-              const timeoutPromise = new Promise((_, reject) =>
-                setTimeout(() => reject(new Error(`Timed out after ${waitTimeout}ms`)), waitTimeout)
-              )
+              // A TIMEOUT and a REJECTED POLL arrive at the same `catch` and
+              // mean opposite things, so they are tagged apart here rather than
+              // conflated below. A timeout is our own deadline firing: the
+              // session was never told to stop and may still be generating. A
+              // rejection is `session.wait` itself dying — an aborted, evicted
+              // or unknown session — and a dead poll is not a running session,
+              // which is the distinction `executeTask` already draws when it
+              // wraps the same call in a guard that cannot reject.
+              const timeoutPromise = new Promise((_, reject) => {
+                const expiry = new Error(`Timed out after ${waitTimeout}ms`)
+                ;(expiry as Error & { nexusTimedOut?: boolean }).nexusTimedOut = true
+                setTimeout(() => reject(expiry), waitTimeout)
+              })
 
               try {
                 await Promise.race([waitPromise, timeoutPromise])
               } catch (waitError: any) {
-                // Timeout or cancellation — agent may still be running
-                agent.status = 'working'
+                // A TIMEOUT means the session may still be running, and
+                // `working` is the honest status for it — left alone on
+                // purpose. A REJECTED POLL means the wait itself died, and no
+                // status here can be trusted to survive: this agent is not
+                // working, and publishing it as though it were is the stuck-
+                // `working` row from issue #100. What it settled AS depends on
+                // whether the session produced anything, so the settle happens
+                // at each exit below rather than here — one settle per outcome,
+                // whichever is actually known.
+                const pollDied = !(waitError as Error & { nexusTimedOut?: boolean })?.nexusTimedOut
+                if (!pollDied) agent.status = 'working'
                 orchestrator.notifyStateChange()
 
                 // Try to get whatever results are available
                 try {
                   const messages = await ctx.session.context({ sessionID: agent.sessionID! })
                   const partialText = lastAssistantText(messages)
                   if (partialText) {
-                    agent.status = 'completed'
+                    if (pollDied) orchestrator.settleAgent(agent.id, 'completed')
                     await ctx.storage.set("orchestrator-state", JSON.parse(JSON.stringify(orchestrator.getState())))
                     const taskPreview = task.length > 80 ? task.substring(0, 77) + '...' : task
                     return {
@@ -1961,6 +1990,7 @@ A design decision, in this shape:
                   // Context read also failed
                 }
 
+                if (pollDied) orchestrator.settleAgent(agent.id, 'failed')
                 return {
                   content: [
                     `${agent.name}`,
@@ -1977,7 +2007,7 @@ A design decision, in this shape:
                 const messages = await ctx.session.context({ sessionID: agent.sessionID! })
                 const result = lastAssistantText(messages) || 'Task completed (no output captured)'
 
-                agent.status = 'completed'
+                orchestrator.settleAgent(agent.id, 'completed')
                 await ctx.storage.set("orchestrator-state", JSON.parse(JSON.stringify(orchestrator.getState())))
 
                 const taskPreview = task.length > 80 ? task.substring(0, 77) + '...' : task
@@ -1993,7 +2023,7 @@ A design decision, in this shape:
                 }
               } catch {
                 // Context read failed after successful wait
-                agent.status = 'completed'
+                orchestrator.settleAgent(agent.id, 'completed')
                 await ctx.storage.set("orchestrator-state", JSON.parse(JSON.stringify(orchestrator.getState())))
                 return {
                   content: [
@@ -2007,6 +2037,13 @@ A design decision, in this shape:
             }
 
             // Non-wait: return spawn info with complexity analysis
+            //
+            // `working` is left in place deliberately, and is the one case where
+            // that is honest rather than a leak: this tool has NOT been told the
+            // session finished, and a non-wait spawn has no waiter to hear it.
+            // The caller is pointed at `nexus.result()`, which is the surface
+            // that reports the outcome. Inventing a terminal status here would
+            // be a worse lie than a running row.
             const complexity = orchestrator.analyzeComplexity({
               id: `spawn-${Date.now()}`,
               name: task,
@@ -2034,6 +2071,11 @@ A design decision, in this shape:
             ].join('\n')
             return { content: output }
           } catch (error: any) {
+            // The spawn itself never reached `spawned` on a spawn failure, and
+            // there is then no agent in the map to settle. Once it HAS spawned,
+            // this is a task that is over — by failure rather than by completion
+            // — and leaving it at `working` is the bug this settles.
+            if (spawned) orchestrator.settleAgent(spawned.id, 'failed')
             return { content: `Failed to spawn agent: ${error.message}` }
           }
         }
@@ -2056,8 +2098,16 @@ A design decision, in this shape:
         options: { codemode: true },
         execute: async (input: unknown, toolCtx: any) => {
           const { role, task, model, timeout } = input as { role: string; task: string; model?: string; timeout?: number }
+          // See the `spawn` tool's own note: the agent is in the map and marked
+          // `working` before anything below can throw, and this catch is the
+          // exit that used to leave it there. `session.wait` REJECTING is the
+          // ordinary way in here — an aborted or gone session rejects rather
+          // than timing out — and that is not a session still working, so it
+          // must not keep reporting one.
+          let spawned: SpawnedAgent | null = null
           try {
             const agent = await spawnAndDeliver({ role, task, model }, toolCtx)
+            spawned = agent
 
             agent.status = 'working'
             orchestrator.notifyStateChange()
@@ -2075,7 +2125,11 @@ A design decision, in this shape:
               const messages = await ctx.session.context({ sessionID: agent.sessionID! })
               const resultContent = lastAssistantText(messages) || (outcome === 'timeout' ? 'Timed out — agent may still be running' : 'Completed with no output')
 
-              agent.status = outcome === 'timeout' ? 'working' : 'completed'
+              // `timeout` leaves the agent `working` and it is right to: the
+              // session was never told to stop and is still spending. Only a
+              // completed wait is an outcome, and it settles like every other.
+              if (outcome === 'completed') orchestrator.settleAgent(agent.id, 'completed')
+              else agent.status = 'working'
               orchestrator.notifyStateChange()
               await ctx.storage.set("orchestrator-state", JSON.parse(JSON.stringify(orchestrator.getState())))
 
@@ -2091,7 +2145,8 @@ A design decision, in this shape:
                 ].join('\n')
               }
             } catch {
-              agent.status = outcome === 'timeout' ? 'working' : 'completed'
+              if (outcome === 'completed') orchestrator.settleAgent(agent.id, 'completed')
+              else agent.status = 'working'
               orchestrator.notifyStateChange()
               return {
                 content: [
@@ -2103,6 +2158,9 @@ A design decision, in this shape:
               }
             }
           } catch (error: any) {
+            // See the `spawn` tool's catch. A rejected `session.wait` lands here
+            // and the agent is not working any more.
+            if (spawned) orchestrator.settleAgent(spawned.id, 'failed')
             return { content: `Delegate failed: ${error.message}` }
           }
         }

src/orchestrator.ts

@@ -1947,7 +1947,14 @@ export class NexusOrchestrator {
       })
     }
 
-    return [...views.values()]
+    // The published rows, and a guard rather than a projection. `id` is the key
+    // of every row and `state` is written by all three passes, so neither can be
+    // absent or null through those constructors — but `sessions[]` is the one
+    // collection a reader is entitled to treat as fully populated, and an empty
+    // key reaching it would be a row that names no session and claims no state.
+    // A row with no id is dropped here rather than published; a `running`/`settled`
+    // row with a real id is not a phantom and stays.
+    return [...views.values()].filter(view => view.id.length > 0)
   }
 
   /**
@@ -1968,6 +1975,13 @@ export class NexusOrchestrator {
       observedUncollected: number
     }
   ): void {
+    // A record with no session id is not a session. `sessionViews()` pass 1
+    // already skips an agent with no session for the same reason — there is
+    // nothing to key a row by, and a row with an empty `id` is a phantom in a
+    // list a user is meant to trust. Passes 2 and 3 did not have this check,
+    // which is the one place a session-keyed row could be built with a
+    // non-session key.
+    if (!record.sessionID) return
     const existing = views.get(record.sessionID)
     // `record.agentId` is deliberately NOT used to fill a null `agentId` on an
     // existing orphan row, and is not used to decide `owned` either: both are
@@ -3887,6 +3901,50 @@ export class NexusOrchestrator {
     return agent
   }
 
+  /**
+   * Put a spawned agent into a TERMINAL status — and leave it in `this.agents`.
+   *
+   * WHY THIS EXISTS. `working` is written by the two spawn tools in `index.ts`
+   * (and by `executeTask`), and it was read back by nobody who could clear it:
+   * the only writer of a terminal status on the tool path was the tool body
+   * itself, at the end of its own success branch. Every path that reached the
+   * tool's OUTER `catch` — a `session.wait` that rejects rather than timing out,
+   * a read that throws, a `storage.set` that fails — returned "Failed to spawn
+   * agent" to the caller and left the agent sitting at `working` in
+   * `getState().agents` for the rest of the process. A finished agent that
+   * claims to still be working is worse than a missing one: the page's `Running`
+   * filter stays lit, and `sessionViews()` maps `working` to `running`, so the
+   * session row claims the session is spending too.
+   *
+   * WHY IT DOES NOT DELETE. The row has to survive, because the row is what
+   * makes the outcome readable: an agent removed from `this.agents` takes its
+   * session row with it (pass 1 of `sessionViews()` reads this map), and the
+   * only reason a session outlives its agent — the orphan that
+   * `SessionStateView` exists to make visible — is `terminateAgent` deleting a
+   * session that is STILL RUNNING. Settling is not that case, and pretending it
+   * were would put a fabricated `agentId: null` row on a session that has an
+   * owner. So: terminal status, row kept, and `cleanupStaleData()` still owns
+   * the removal — an hour of `lastActivity`, which this refreshes.
+   *
+   * First terminal outcome wins. `completed` and `failed` are two readings of
+   * the same task, not two stages of it, so a late `failed` must not overwrite a
+   * `completed`. `terminated` does outrank both: it is the escalation path, and
+   * it is what `terminateAgent` would have written.
+   */
+  settleAgent(agentId: string, status: 'completed' | 'failed'): void {
+    const agent = this.agents.get(agentId)
+    // Unknown agent: nothing to settle. Callers pass an id they were handed, but
+    // a terminated agent is legitimately gone by the time a spawn tool unwinds.
+    if (!agent) return
+    if (agent.status === 'terminated') return
+    if (agent.status === 'completed' || agent.status === 'failed') return
+    agent.status = status
+    // Without this the row is evicted a full hour after it was SPAWNED, which
+    // for a long run removes rows that only just finished.
+    agent.lastActivity = new Date()
+    this.notifyStateChange()
+  }
+
   async terminateAgent(agentId: string): Promise<void> {
     const agent = this.agents.get(agentId)
     if (agent) {

test/dashboard-entrypoints.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, afterEach, mock } from 'bun:test'
-import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'
+import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'
 import { join, resolve } from 'node:path'
 import { tmpdir } from 'node:os'
 import * as realOs from 'node:os'
@@ -927,6 +927,56 @@ describe('dashboard entry points', () => {
     })
   })
 
+  // The parser test above keeps the two functions in agreement. It cannot
+  // keep them in separate BUNDLES, which is the constraint that made them two
+  // functions in the first place: `src/dashboard.ts:11` inlines
+  // `dashboard/index.html` with a text import, so importing that module from
+  // `src/tui.tsx` would pull ~170 KB of page markup into `dist/tui.js`, which
+  // serves no dashboard and reads none of it. The comments at both sites say
+  // so, and a comment is not a test — the next reader who sees the duplication
+  // merges them, and the cost lands at build time where nothing fails.
+  describe('the built bundles keep the dashboard markup in one of them', () => {
+    // Strings taken from `dashboard/index.html`. Short and specific enough that
+    // a coincidence in application code is not the reason a bundle matches,
+    // and from the head of the file so a later edit to the body cannot quietly
+    // make this pass for the wrong reason.
+    const MARKUP = ['--surface-hover', '0d1117', '161b22', '30363d'] as const
+
+    const dist = (file: string): string | null => {
+      const path = join(import.meta.dir, '..', 'dist', file)
+      return existsSync(path) ? readFileSync(path, 'utf-8') : null
+    }
+
+    it('inlines the dashboard page into the server bundle, which serves it', () => {
+      const server = dist('index.js')
+      if (server === null) return // a source checkout with no build; see below
+      for (const mark of MARKUP) expect(server).toContain(mark)
+    })
+
+    it('keeps every byte of that markup out of the TUI bundle', () => {
+      const tui = dist('tui.js')
+      if (tui === null) return
+      for (const mark of MARKUP) {
+        expect(tui).not.toContain(mark)
+      }
+    })
+
+    it('and the TUI bundle is nowhere near large enough to be carrying it', () => {
+      // The size claim is what makes the two tests above a real assertion
+      // rather than a coincidence about four strings. `dist/tui.js` is ~72 KB
+      // and the page is ~170 KB, so a bundle that had inlined the markup
+      // would be roughly 240 KB. The bar is set at the sum, not below it, so
+      // the current bundle keeps a wide margin and a regression is not one
+      // that has to be timed to notice.
+      const tui = dist('tui.js')
+      if (tui === null) return
+      const page = join(import.meta.dir, '..', 'dashboard', 'index.html')
+      const pageBytes = existsSync(page) ? statSync(page).size : 0
+      expect(pageBytes).toBeGreaterThan(0)
+      expect(Buffer.byteLength(tui)).toBeLessThan(pageBytes)
+    })
+  })
+
   describe('probeDashboard', () => {
     it('distinguishes a dashboard, a squatter and a closed port', async () => {
       const squatter = Bun.serve({ port: 14994, hostname: '127.0.0.1', fetch: () => new Response('<html>') })