chaitanyagiri/munder-difflin

**Finding: substantially FIXED on `main` since 0.2.4 — your exact guard plus more landed (verified in v0.2.6 source). The structurally-inert classes (3 & 4) are closed. One residual remains for live-but-busy worker agents; firsthand repro + a concrete mitigation below.**

### What's already fixed on main (post-0.2.4)
Your root-cause analysis was spot on, and the fix you suggested is implemented — plus more:

- **`runBreakerBeat` now skips the inert classes** (`src/main/index.ts:485,494,495`):
```ts
if (a.archived) continue;
if (a.isAssistant) continue; // class 3: send-only 'Dwight'
if (id !== reg.godId && !ptyForAgent(id)) continue; // class 4: orphaned shells, no live PTY
```
This is exactly your "mirror the cost-ledger guard" one-liner, extended with a no-live-PTY guard for orphaned shells.
- **Startup migration `archiveOrphanedAgents()`** (`index.ts:288-309`, tagged `#57/#58`) archives every `archived:false` entry with no live PTY *before* the...

**Finding: the PRIMARY repro (unquoted `C:\Program Files\…` exe path) is very likely fixed by PR #71 — needs on-Windows verification. Two secondary symptoms in this report are separate and NOT addressed by #71.**

### 1. Unquoted executable path → `'C:\Program' is not recognized` — addressed by #71
PR #71 adds `buildCmdCommandLine()` in `src/main/pty.ts`, whose doc comment explicitly cites **#55** and your exact `C:\Program Files\nodejs\node` → "C:\Program is not recognized" failure. The fix:
- A non-`.exe`/`.com` target on Windows is routed through `cmd.exe /d /s /c ""` (CreateProcess can't directly run `.cmd`/extensionless shims — that's the error-193 family the 0.2.4 fix handled).
- The command line is built by quoting **every token including the resolved target path** on whitespace/quote/metachar (`/[ \t"&|^()%!]/`), then wrapping the whole inner command in one outer quote pair that cmd's `/s` strips. So `C:\Program Files\…\node.exe` is emitted as `"C:\Program Files\…\no...

**Addressed in v0.2.5** (commit `5fc2161`) — recommend close.

Two parts, as discussed:
- The visible Dwight → headless Haiku enrichment swap (commit `acc13a3`, shipped earlier) was **by design**, which is why the on-floor Dwight + Enrich button are gone.
- The actual bug — a stale pre-swap `assistant` registry entry left `archived:false` with no PTY (the ghost that fed the #56/#57 symptoms) — is now cleaned up by the boot migration `archiveOrphanedAgents()` in `5fc2161`.

Worth a one-line CHANGELOG/README note that the assistant became headless, since the swap was undocumented. Otherwise ok to close.

— @chaitanyagiri

**Structural cause fixed in v0.2.5** (commit `5fc2161`) — recommend close (see caveat).

Two guards land the fix I outlined:
1. `runBreakerBeat` now **skips** assistant entries and any orphaned entry that is `archived:false` with no live PTY (`ptyForAgent(id) === undefined`) — so the breaker never steers a send-only/inert agent whose steer would bounce to god as a non-clearable `requires_reply` and flood the inbox.
2. New boot migration `archiveOrphanedAgents()` archives every stale `archived:false`-but-no-live-PTY entry before the renderer respawns anything, removing the ghost that triggered the loop in the first place.

Caveat (same honesty as my earlier note): I could only confirm the **structural exposure** statically, not reproduce a strictly-0-token continuous 30–60s loop, since the no-progress trip is gated on `dOut>0`. The fix removes the structural source (orphaned inert entries + breaker evaluating them), which is the right root-cause fix. Suggest closing after a soak with th...

**Verified fixed in v0.2.5** (commit `650c568`) — recommend close.

The breaker-beat ledger write is now guarded by `if (sample?.sessionId)` — the exact one-liner proposed here. A dead/orphaned agent with a frozen transcript still produces a sample via the transcript fallback, but with an empty `sessionId` (a truthy one is set only by a live session), so it no longer appends a duplicate row every 30s. The sample is still pushed into the breaker inputs, so liveness logic is unaffected. This eliminates the 2,417-dupe class.

— @chaitanyagiri ok to close.

**Verified fixed in v0.2.5** (commit `e5c144e`) — recommend close.

The new `buildCmdCommandLine()` in `src/main/pty.ts` routes non-`.exe` Windows targets through `cmd.exe /d /s /c ""`, double-quoting the resolved target **and** each arg only when it contains a space/tab/quote, then wrapping the whole inner command in one outer pair that `/s` strips. So `C:\Program Files\...\claude.cmd` now survives its space instead of splitting at `C:\Program` — exactly the repro in this issue. The JSDoc also confirms node-pty receives this as a pre-escaped CommandLine and passes it through verbatim (no double-wrapping), which was the concern when I first reviewed the #51 fix.

Separate latent item (not this bug): cmd.exe still expands `%VAR%` inside quotes, so the generated `-p` prompt path in `assistant.ts`/`reflect.ts` would ideally move to **stdin** as a follow-up hardening. The crash this issue reports is resolved.

— @chaitanyagiri ok to close once you've confirmed on Win11.

**Answering your direct question: Dwight was *intentionally* removed — this is by design, not a broken Windows respawn.**

Commit `acc13a3 feat(enrich): replace visible Dwight assistant with invisible headless Haiku enrichment` (shipped in 0.2.4) replaced the visible Dwight agent + the Enrich toggle with a headless Haiku enrichment pass. So on a clean 0.2.4 install, no Dwight on the floor and no Enrich button is *correct*. The `enrichMessage` preload entry is a vestige of the old surface; the real documentation miss is that the 0.2.4 changelog doesn't record the swap (you correctly noted 0.2.4 isn't in the changelog at all).

**The actual bug is the one you found underneath:** upgrading from a pre-`acc13a3` version leaves a stale `assistant` entry in `registry.json` with `archived:false` and no PTY. The current code neither respawns it (correct) nor migrates it away (the bug) → a ghost entry that then feeds the circuit-breaker loop (#57) and the cost-ledger duplication (#56).

**Fix:**...

**Structural defect confirmed. One correction, and one honest caveat: I could not reproduce a *self-sustaining* loop for a strictly 0-token agent from static reading — see below.**

Verified in v0.2.4:

1. **No `isAssistant` / no-live-PTY guard.** `runBreakerBeat` (`src/main/index.ts`) builds breaker `inputs` for every agent except `archived` and `god` — so `isAssistant` agents and orphaned non-archived shells ARE evaluated. **Correction:** in 0.2.4 the cost-ledger loop does *not* already skip `isAssistant` — it's the *same* loop and only guards `if (a.archived) continue` + `if (id === reg.godId) continue`. So adding the `isAssistant` guard helps both the ledger (#56) and the breaker.
2. **Steers to a send-only assistant bounce to GOD.** `hive.ts:647-651` bounces any message targeting an `isAssistant` agent with the `[bounced — "assistant" is the send-only prep assistant…]` subject you saw, delivered `requires_reply:true`. GOD can't clear a sibling's breaker state → exactly your noise ...

**Confirmed, and your one-line fix is correct.** Verified in v0.2.4:

- `runBreakerBeat` (`src/main/index.ts`) calls `if (sample) hive.appendCostLedger(sample)` every 30s tick for every non-archived agent — no session/dedup check.
- `transcriptFallback` (`src/main/telemetry.ts:389`) returns a sample with `sessionId: ''` (telemetry.ts:396) whenever there's no live OTel session but a transcript dir exists → identical frozen totals forever.
- `appendCostLedger` (`src/main/hive.ts`) appends unconditionally, no dedup.

So an ended-but-non-archived agent gets the same frozen row written every beat — exactly your 2,417-row pattern. Your fix:

```ts
if (sample?.sessionId) hive.appendCostLedger(sample);
```

is right: `sample` is still pushed into the breaker `inputs` after this line, so guardrails are unaffected — only the ledger write is gated. I'd pair it with a dedup guard in `appendCostLedger` (skip if `agentId`+`sessionId`+`usd` equals the last row for that agent) as belt-and-suspenders, ...

**Confirmed — this is a follow-on gap in the 0.2.4 Windows spawn fix (#51): `cmd.exe /c` is handed an unquoted path containing spaces.**

Verified in `src/main/pty.ts`. The #51 fix routes non-`.exe`/`.com` targets through `cmd.exe /c` (correct — that's what lets `.cmd`/extensionless shims run at all), but it passes the resolved path as a bare argument:

```ts
// pty.ts:153-154
const file = needsCmd ? (process.env.ComSpec || 'cmd.exe') : resolved;
const spawnArgs = needsCmd ? ['/c', resolved, ...(opts.args ?? [])] : (opts.args ?? []);
```

When `resolved` is e.g. `C:\Program Files\nodejs\npm-global\claude.cmd`, `cmd.exe` parses up to the first space and tries to run `C:\Program` → exactly your `'C:\Program' is not recognized`. The `.exe` wrapper sidesteps it because its path has no spaces, so as you said the bug was masked, not fixed.

`resolveCommand` (pty.ts:73) takes the first PATHEXT-eligible `where` hit, which on an npm-global AVD install is the `claude.cmd` under `Program Files` —...