Skip to content

fix: PTY exit handling, error handling, sidebar persistence - #7

Merged
aterrylu merged 1 commit into
mainfrom
terry/project-session-view
Mar 8, 2026
Merged

fix: PTY exit handling, error handling, sidebar persistence#7
aterrylu merged 1 commit into
mainfrom
terry/project-session-view

Conversation

@aterrylu

@aterrylu aterrylu commented Mar 8, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up fixes from code reviewer and simplifier on PR #6:

  • PTY exit notification: Server sends WS close (code 4010) when PTY exits; client immediately clears the dead session instead of waiting for next poll
  • onExit handler guard: Register PTY exit handler only once per session to prevent duplicate close notifications on reconnect
  • Error handling: /api/projects endpoint now catches SDK errors gracefully instead of crashing
  • Sidebar persistence: sidebarOpen persisted in Zustand — survives page refresh. Defaults to open for discoverability
  • Code cleanup: Extract PageTheme type alias, type listSessions result to avoid implicit any

Test plan

  • Start a session, Ctrl+C to quit Claude Code — view should snap back to empty state immediately
  • Refresh page — sidebar should remain open/closed as left
  • Kill the server — /api/projects should return 500 not crash

🤖 Generated with Claude Code

… cleanup

- Fix onExit handler accumulation: register once per session via guard set
- Add try/catch on /api/projects for SDK errors
- Persist sidebarOpen in Zustand (survives refresh), default to open
- Client clears session immediately on PTY exit (WS close code 4010)
- Extract PageTheme type alias in Sidebar (simplifier)
- Type the listSessions result to avoid implicit any

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aterrylu
aterrylu enabled auto-merge (squash) March 8, 2026 19:55

@nox-0x nox-0x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid follow-up to #6. PTY exit notification is correctly guarded against duplicate handlers (the exitHandlerRegistered set + cleanup in onExit handles reconnect races cleanly), error handling on /api/projects is proper, and the sidebarOpen persistence is straightforward. The removed env/CLAUDECODE deletion was dead code (the local copy was never passed to listSessions), so that removal is correct. LGTM.

@aterrylu
aterrylu merged commit ec7bd72 into main Mar 8, 2026
1 check passed
@aterrylu
aterrylu deleted the terry/project-session-view branch March 8, 2026 19:59
aterrylu added a commit that referenced this pull request Jun 10, 2026
…sktop gap (#195)

The desktop app had 1 test file vs the server's 19 — which is why 'the hosted
server works great but the desktop app is always problematic.' Goes 1 → 6 files
(48 cases), testing the highest-risk main-process logic with electron +
child_process fully MOCKED (zero real spawns).

Coverage:
- server-supervisor (THE critical module): acquireOrConnect Built-in-vs-Always-on
  decision — live owner → connect, NEVER spawn (asserted); dead pid/port → Built-in
  spawn; AUTONOMOS_READY parsing + argv; AUTONOMOS_ALREADY_RUNNING race; early-exit/
  spawn-error/missing-bundle → clean reject, no spawn.
- config/tokens (security): safeStorage round-trip, plaintext fallback, the
  encrypted-flag keychain-flip edge, decrypt-failure → null, tokens.dat 0o600.
- migrate + config/migration: install-service shellout argv; migrateConfig
  validation (garbage → defaults, bad connections dropped, localhost dedup).
- drag-controller: extracted from window-manager (1:1, electron-free) — offset
  reposition + timer-leak guards (failure-class #7).

Testability seams (mirror the server's _setDependencies pattern, prod defaults to
real): electron-deps.ts (lazy real require, _setElectronForTesting stub),
_setDependenciesForTesting on server-supervisor/migrate, drag-controller injection.
window-manager keeps identical public signatures — no callers changed.

FINDING (pinned with a FINDING-commented test): migration.ts isLocalhostRemote
never strips IPv6-loopback dups — new URL('http://[::1]:..').hostname is '[::1]'
(bracketed), so the '::1' compare is dead code. Behavior preserved; flagged for a
deliberate future fix.

make check green 4x (392 node:test + 203 vitest, 0 fail); biome + tsc clean.
Zero process spawns (claude PIDs unchanged across runs).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants