fix: preview panes in split groups + restart all sessions

[aterrylu]

Summary

• Preview panes in groups: Sidebar now shows preview panes as part of split-pane groups. Previously, the two-pass rendering skipped group lookup for previews, so they always appeared as standalone items even when in a split. GroupContainer now renders mixed members (sessions + previews) with appropriate icons.
• Restart All Sessions: New button in settings panel that kills all PTYs and respawns them with fresh environment (picks up new API key/settings). Preserves session IDs so layout, groups, and splits remain intact. Includes confirmation step to prevent accidental restarts.
• Settings UX: Updated help text to clarify save-then-restart workflow.

Test plan

• Split a session with a markdown preview — both should appear under the same group in sidebar
• Drag a preview out of a group — should ungroup correctly
• Set Anthropic Auth Token in settings, save, then click "Restart All Sessions" — sessions should restart with new key
• Clear the token (empty string), save, restart — sessions should use default Claude Code auth
• After restart, verify layout/splits/groups are preserved

🤖 Generated with Claude Code

[nox-0x]

🔴 Critical

Problem: shuttingDown is reset to false synchronously immediately after managed.pty.kill(), but the old PTY's onExit callback fires asynchronously — on the next event loop tick, after this synchronous loop iteration has already cleared the flag. That means every old onExit handler fires with shuttingDown === false, triggering both sessions.delete(id) and removePersistedSession(session.claudeSessionId) — which permanently removes each restarted session from the map and deletes the Claude session file the new PTY needs for --resume.

Why it matters: restartAllSessions() will silently destroy every session instead of restarting them. The dashboard will show all sessions as gone after the restart completes.

Suggested fix: Track restarting IDs separately so the old onExit can skip cleanup:

// module-level
const restartingIds = new Set<string>();

// In restartAllSessions, replace the shuttingDown toggle:
restartingIds.add(id);
try {
  managed.pty.kill();
} catch {
  // best-effort
}
// (remove the shuttingDown = false line entirely)

// In the original spawn's onExit (and the new one below):
pty.onExit(() => {
  session.status = "stopped";
  session.updatedAt = Date.now();
  if (!shuttingDown && !restartingIds.has(id)) {
    if (session.claudeSessionId) removePersistedSession(session.claudeSessionId);
    sessions.delete(id);
  }
  restartingIds.delete(id); // always clean up
});

This requires the same guard in the original createSession spawn path too — or alternatively, add a restarting?: boolean field to ManagedSession and check that instead.

[aterrylu]

Already addressed in the latest push — rewrote to snapshot all session info first, then kill everything under shuttingDown=true, clear the map, set shuttingDown=false, then respawn via createSession. The old per-session toggle approach is gone.

[nox-0x]

🟡 Warning

Problem: The catch block in handleRestart silently swallows errors — both network failures and non-2xx responses — and just resets state to "idle" with no user feedback.

Why it matters: If the server returns a 500 or the fetch fails entirely, the user sees the button return to normal and has no idea whether the restart succeeded, partially ran, or never executed. With destructive-adjacent actions (killing all live PTYs), silent failure is especially bad.

Suggested fix:

const [error, setError] = useState<string | null>(null);

// In handleRestart catch:
} catch (err) {
  setError(err instanceof Error ? err.message : "Restart failed");
  setState("idle");
}

// In render, show the error:
{error && (
  <div className="text-xs mt-1" style={{ color: "#ea6c73" }}>
    {error}
  </div>
)}

Also: the data variable from await res.json() on line 97 is assigned but never used — can safely remove it.

[aterrylu]

Fixed — added error state to RestartAllButton. Now shows error message on failure instead of silently resetting. Also removed unused data variable.

[nox-0x]

Preview-panes-in-groups logic looks solid — the GroupMember union type and two-pass rendering refactor are clean. One critical bug in restartAllSessions: shuttingDown is reset to false synchronously before the old PTYs' async onExit handlers fire, so those handlers see shuttingDown === false and call sessions.delete(id) + removePersistedSession() on the just-restarted sessions — effectively destroying them all. Also flagged silent failure in the restart button's catch block.

[nox-0x]

Reviewed the new commit (de9b304) — the remapSessionIds store logic and the Sidebar GroupMember refactor look correct. However, both issues flagged in the previous review remain unresolved in the newly added code:

1. 🔴 shuttingDown async race (sessions.ts): shuttingDown is still reset to false synchronously before the old PTYs' onExit handlers fire. Those handlers see !shuttingDown === true and call removePersistedSession(session.claudeSessionId) — deleting the Claude session files the newly-spawned PTYs need for --resume. See the inline comment for the suggested fix using a restartingIds: Set<string> guard.

2. 🟡 Silent failure in RestartAllButton (SettingsStatusBarItem.tsx): The catch block still resets to "idle" with no user feedback. On a destructive operation like killing all PTYs, silent failure is especially bad.

Neither is in new code paths added by this commit — they were both present and called out before. Please address before merging.

[nox-0x]

New commit (8e0e42d) correctly adds remapSessionIds to the store and threads closePreview through GroupContainer — both look good. However the two issues flagged in prior reviews are still unresolved in this commit:

🔴 Critical — shuttingDown async race (sessions.ts): shuttingDown is reset to false synchronously before the old PTYs' onExit callbacks fire. Those handlers see !shuttingDown === true and call removePersistedSession(session.claudeSessionId), deleting the Claude session files the newly-spawned PTYs need for --resume. The practical result is every session is permanently destroyed instead of restarted. Fix: use a restartingIds: Set<string> guard as described in the inline comment.

🟡 Warning — silent failure in RestartAllButton (SettingsStatusBarItem.tsx): catch block still resets to "idle" with no user-visible error. For an operation that kills all live PTYs, silent failure is especially bad.

Please address the critical race before merging — it will cause real data loss in production.

[nox-0x]

New commit (f512ed9) adds remapSessionIds and closePreview correctly — both look good. However the two issues flagged across all prior review rounds remain unresolved:

🔴 Critical — shuttingDown async race (sessions.ts): shuttingDown is reset to false synchronously before the old PTYs' onExit callbacks fire. Those handlers see !shuttingDown === true and call removePersistedSession(session.claudeSessionId), deleting the Claude session files the newly-spawned PTYs need for --resume. Every restarted session will be permanently destroyed instead of resumed. Fix: use a restartingIds: Set<string> guard as described in the inline comment on sessions.ts.

🟡 Warning — silent failure in RestartAllButton (SettingsStatusBarItem.tsx): The catch block resets to "idle" with no user-visible error. For an operation that kills all live PTYs, silent failure is especially dangerous — user has no idea if the restart succeeded or not.

The sidebar GroupMember refactor and the store remap logic are solid. Ready to approve once the critical race is addressed.

[nox-0x]

🟡 Silent failure in RestartAllButton — FIXED ✅ Error state is now rendered.

🔴 Critical — shuttingDown async race in sessions.ts — Still present in this commit.

The sequence is: shuttingDown = true → kill PTYs → sessions.clear() → shuttingDown = false → respawn. The old PTYs onExit callbacks fire after this synchronous block, when shuttingDown is already false. So !shuttingDown is true in every handler, and removePersistedSession(session.claudeSessionId) runs — which removes the newly-created session from the persistence store (the new session was just persisted under the same claudeSessionId). Sessions work fine in-memory for the current run but will not be restored on server restart.

Fix: use a restartingIds: Set<string> guard as documented in the inline comment. Guard removePersistedSession with !restartingIds.has(id) in the onExit handler, and delete from the set inside the callback.

Sidebar GroupMember refactor, store remapSessionIds, and error handling all look good. One issue blocking merge.

[nox-0x]

🟡 Warning

Problem: onChange("") is called immediately when the user clicks the edit (pencil) button — before they have typed or confirmed anything.

Why it matters: onChange presumably triggers a debounced save to the server. Clicking "edit" on a field that contains a valid API key instantly overwrites it with an empty string on the server, even if the user changes their mind and navigates away without submitting. The current value is silently destroyed on the first click.

Suggested fix: Remove the eager onChange("") call from the edit-button handler. Only call onChange when the user explicitly saves (e.g., on Enter / blur with a confirmed value):

onClick={() => {
  setEditing(true);
  setDraft(""); // pre-fill draft as empty so user types fresh
  // do NOT call onChange here — only call it on confirmed save
}}

If clearing immediately is intentional UX (show-as-blank-while-editing), add a local displayValue derived from editing ? draft : savedValue so the visual state and the persisted state stay decoupled.

[nox-0x]

Error handling in RestartAllButton is fixed ✅ — setError is wired up and the error div renders correctly. Sidebar GroupMember refactor and remapSessionIds store logic look solid.

Two issues still blocking:

🔴 Critical — shuttingDown async race (sessions.ts): The synchronous shuttingDown = false reset happens before old PTY onExit callbacks fire. Those handlers see !shuttingDown === true and call removePersistedSession(session.claudeSessionId), deleting the Claude session files the newly-spawned PTYs need for --resume. Sessions work in-memory for the current run but will not restore on server restart. Fix documented in the existing inline comment: use a restartingIds: Set<string> guard.

🟡 Warning — premature onChange("") in SettingRow: Clicking the edit pencil now calls onChange("") before the user types anything, silently overwriting a valid saved value (e.g., an API key) with an empty string on the server. New inline comment with fix on line 46.