feat: v0 server + terminal view - #1
Conversation
Implements the v0 server + terminal view — the first working code for autonomOS. **packages/core** — shared types - Session, AgentEvent, AgentProvider interfaces - Provider abstraction from day one (ADR-006) **packages/server** — Hono + node-pty on Node.js - REST API: create, list, get, kill sessions - WebSocket endpoint streams PTY bytes bidirectionally - Spawns Claude Code as PTY subprocess - Strips CLAUDECODE env var to avoid nested session detection **packages/dashboard** — Vite + xterm.js - Dark GitHub theme, WebGL GPU rendering - macOS keyboard shortcuts (macOptionIsMeta, Cmd+K, Cmd+Backspace, etc.) - Cross-platform keybindings (Ctrl equivalents for Windows/Linux) - Unicode 11 support, FitAddon for responsive resize - Vite proxy routes /api and /ws to server **Stack:** Node.js + tsx (server), Bun (package manager + dashboard), Hono, xterm.js **Note:** node-pty 1.0.0 required — 1.1.0 has posix_spawnp regression on Node v25 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses code review findings: **Security** - Validate request body on POST /api/sessions (400 on missing/invalid fields) - Prevent prompt argument injection with "--" separator - Resolve claude binary with existence check instead of hardcoded path - Make CORS origin configurable via CORS_ORIGIN env var **Error handling** - try-catch around PTY spawn (returns 500 instead of crashing server) - try-catch around pty.write() and pty.resize() in WebSocket handler - try-catch around ws.send() in PTY onData callback - try-catch around pty.kill() (updates status even if kill fails) - Separate JSON.parse from resize operation to prevent resize JSON leaking as PTY stdin text - Dashboard fetch() wrapped in try-catch (shows "server unreachable") - WebGL addon logs actual error object on failure **Session lifecycle** - Kill PTY process on WebSocket disconnect (prevents orphan leaks) - Remove dead sessions from map on kill (prevents memory leak) - SIGINT/SIGTERM handler kills all sessions on server shutdown - Add onError handler to WebSocket (cleans up on error, not just close) **Code quality (from simplifier)** - Extract TERMINAL_THEME constant, sendResize/sendToWs helpers - WeakMap for PTY bindings instead of (ws as any)._* properties - Switch statement for key handler, extracted handleKeyEvent function - Use navigator.userAgentData with fallback for platform detection - Return type annotations on functions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Targets: - make dev — start server + dashboard together - make dev-server — start server only (port 3000) - make dev-dashboard — start dashboard only (port 5173) - make install — install dependencies - make setup — full setup including node-pty native build - make kill — kill running dev servers - make clean — remove node_modules Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nox-0x
left a comment
There was a problem hiding this comment.
Code Review: feat: v0 server + terminal view
Solid v0 foundation — the PTY streaming loop is well-structured and the architecture is clean. Here are things worth addressing before this stabilizes:
🔴 Issues
sessions.ts — Sessions never cleaned up on PTY exit
The pty.onExit handler updates the session status to "stopped" but does NOT remove it from the sessions Map. Over time this leaks memory and returns stale "stopped" sessions from GET /api/sessions.
pty.onExit(() => {
session.status = "stopped";
session.updatedAt = Date.now();
// Bug: sessions.delete(id) is missing here
});Suggestion: either delete on exit, or keep them and add a TTL cleanup.
terminal.ts — killSession called on every WebSocket close
cleanupBinding calls killSession unconditionally on both onClose and onError. This means closing your browser tab destroys the Claude Code session — likely unintentional for a resume-later scenario. Consider decoupling WebSocket lifetime from session lifetime.
sessions.ts — resolveClaudePath throws at session creation, error swallowed
The 500 response just says "Failed to spawn agent process" — the "binary not found" detail is lost. Consider validating on startup and surfacing a meaningful error message to the client.
🟡 Warnings
terminal.ts — Naive JSON detection with msg.startsWith("{")
If a user types a message starting with { into the terminal, it gets parsed as potential JSON. Consider a proper message envelope protocol (type discriminator byte, or structured framing) instead of content peeking.
sessions.ts — workingDirectory stored unexpanded
cwd expands ~ before spawning, but session.workingDirectory still stores "~". Clients get the literal ~ back. Store the expanded path or document the behavior.
terminal.ts — No bounds checking on resize cols/rows
parsed.cols as number and parsed.rows as number are cast without validation. Passing 0 or negative values to node-pty will throw. Add validation before calling managed.pty.resize().
main.ts — API_URL is an empty string, undocumented
Fine for same-origin, but confusing if someone runs the dashboard standalone. A comment or VITE_API_URL env var would help.
Makefile — setup copies to a hardcoded Bun internal path with silent failure
The || true fallback hides copy failures completely — users get a cryptic native addon error at runtime. Add a verification step or explicit error message.
🟢 Nice details
- Stripping
CLAUDECODEenv var to prevent nested session detection — smart - Pinning
node-pty@1.0.0with a documented reason (v1.1.0 regression on Node v25) — great - WebGL fallback to canvas renderer with console warning — graceful
macOptionIsMetaand key handler comments are well-documented- Graceful shutdown with
killAllSessions()on SIGINT/SIGTERM
Priority fixes: session leak on PTY exit, WebSocket/session lifetime coupling. The rest can be addressed iteratively.
Fixes from Nox's review: **Session lifecycle** - PTY onExit now removes session from map (fixes memory leak) - WebSocket disconnect no longer kills the session — sessions persist independently and can be reconnected to - Store expanded workingDirectory (not literal "~") **Validation & error handling** - Validate resize cols/rows bounds (2-500 cols, 1-200 rows) - Surface "binary not found" detail in 500 error response - Validate claude binary exists at startup (fail fast) **Tests (17 passing)** - API validation: missing body, missing/invalid fields, type checking - Session map operations: get/kill/list with unknown IDs - expandPath: ~ expansion, absolute paths, edge cases - Integration: valid session creation returns 201 with expanded path **Other** - Document API_URL empty string convention in dashboard - Add `make test` target Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addressed all review feedbackThanks Nox — great catches. All fixed in the latest push: 🔴 Issues — Fixed
🟡 Warnings — Fixed
🟡 Noted for future
Tests added17 tests covering:
Run with |
nox-0x
left a comment
There was a problem hiding this comment.
Code Review — feat: v0 server + terminal view
Great initial scaffold! The architecture is clean and well-thought-out. A few things worth flagging before this lands:
🔴 Security / Correctness
1. Prompt injection via args passthrough (sessions.ts:79)
const args = options.prompt ? ["--", options.prompt] : [];The -- separator prevents flag injection, which is good. But the raw prompt string is passed directly as a CLI argument to Claude Code. If the prompt contains shell metacharacters and claude uses a shell internally, this could be an issue. Worth documenting the assumption that node-pty (which does not use a shell by default) is the only PTY path.
2. No authentication on WebSocket or REST endpoints
Any process on the host can connect to :3000 and spawn PTY sessions or attach to existing ones. For a local-only tool this is fine short-term, but worth a TODO or a --host 127.0.0.1 bind in the server.
3. expandPath falls back to /tmp if HOME is unset (sessions.ts:51)
return path.replace(/^~/, process.env.HOME || "/tmp");Silently spawning a session in /tmp could be confusing. Consider throwing instead.
🟡 Error Handling
4. Session leak on pty.onExit (sessions.ts:93)
Sessions are removed from the map on exit — but if the PTY crashes before sessions.set() completes (unlikely but possible in tight error paths), the entry could be left dangling. Low priority, but a try/finally pattern in createSession would be safer.
5. WebglAddon fallback is silent (main.ts:113)
} catch (err) {
console.warn("WebGL addon failed, falling back to canvas renderer:", err);
}The fallback works, but there is no UI indicator. Users on a machine where WebGL is unavailable won't know why performance might be degraded. Minor, but consider a subtle status badge.
6. onMessage JSON detection via string prefix (terminal.ts:66)
if (msg.startsWith("{")) {Technically correct and the try/catch handles malformed input safely. Just note this means a user typing { in the terminal triggers JSON parsing — benign but worth a comment.
🟡 Performance / Design
7. ResizeObserver is never disconnected (main.ts:152)
const resizeObserver = new ResizeObserver(() => { ... });
resizeObserver.observe(container);Each call to connectTerminal() attaches a new observer without disconnecting the old one. After a few sessions, you'll have N observers all firing simultaneously. Store the observer and call resizeObserver.disconnect() at the top of connectTerminal(). This is the most impactful fix.
8. Makefile setup step uses a bun-version-specific path (Makefile:25)
cp -r /tmp/autonomos-pty-build/node_modules/node-pty/build \
node_modules/.bun/node-pty@1.0.0/node_modules/node-pty/ 2>/dev/null || true
If bun updates its internal directory structure, setup silently does nothing. Consider a postinstall script or a verification step that confirms the native addon loaded correctly after the copy.
9. claude binary lookup is hardcoded to two paths (sessions.ts:25-31)
Missing common paths like ~/.npm-global/bin/claude, /opt/homebrew/bin/claude. A which claude subprocess call at startup would be more robust and platform-agnostic.
✅ What's solid
- WebSocket disconnect does not kill sessions — reconnectable sessions is the right design.
- Input validation in
sessions.ts(router) is thorough and tests back it up well. CLAUDECODEenv stripping is a nice discovery and well-documented.- WebGL → canvas fallback with
try/catchis correct. macOptionIsMeta: trueis essential and well-handled.- Test coverage for validation edge cases (cols, rows, missing fields, invalid JSON) is solid.
Summary: No blockers for a v0. The ResizeObserver leak (#7) is the most impactful fix before real usage. The auth story (#2) and HOME fallback (#3) are worth follow-up issues.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the thorough reviews @nox-0x! Here's the status on each item: Already addressed (previous commit)
Fixed in this commit
Acknowledged / deferred for v0
|
…st binary lookup - Fix ResizeObserver leak: disconnect previous observer on reconnect - expandPath throws when HOME is unset instead of falling back to /tmp - resolveClaudePath adds /opt/homebrew/bin and `which` fallback - Add test for expandPath HOME-unset behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nox-0x
left a comment
There was a problem hiding this comment.
Code Review — feat: v0 server + terminal view
Solid v0! The architecture is clean and the PTY streaming loop is well-structured. A few things worth addressing before merging:
🔴 Bugs / Issues
sessions.ts — session name collision
name: `Session ${sessions.size + 1}`,After sessions are deleted and recreated, sessions.size resets, producing duplicate names ("Session 1" again). Use a monotonic counter instead.
sessions.ts — pty.onExit deletes session before callers can react
pty.onExit(() => {
session.status = "stopped";
sessions.delete(id); // ← session removed from map immediately
});Any in-flight WebSocket client that tries getSession() right after PTY exit will get undefined and silently drop. Consider a brief grace window or keeping the session in a terminal state before cleanup.
sessions.ts — hardcoded claude binary paths
const candidates = [
`${process.env.HOME}/.local/bin/claude`,
"/usr/local/bin/claude",
];which claude / $PATH lookup would be more portable. Misses ~/.npm-global/bin/claude, Homebrew (/opt/homebrew/bin), etc.
terminal.ts — JSON detection heuristic is fragile
if (msg.startsWith("{")) {A user pasting {"key": "value"} into the terminal will be silently swallowed as a resize attempt, then dropped when JSON parses but type !== "resize". Better to use a dedicated message envelope from the start (e.g., send terminal input as { type: "input", data: "..." } and resize as { type: "resize", ... }).
🟡 Error Handling
sessions.ts — expandPath falls back to /tmp
return path.replace(/^~/, process.env.HOME || "/tmp");Spawning a Claude session in /tmp silently when $HOME is unset is surprising. Better to throw a clear error.
terminal.ts — ws.close() in onOpen may be a no-op
If getSession() returns null in onOpen, closing the WS immediately may not propagate correctly depending on Hono/WS upgrade timing. Worth testing this path explicitly.
sessions.ts — killSession catches PTY kill error but still removes the session
If pty.kill() throws, the PTY process might still be running while the session is deleted from the map — creating an orphaned process. At minimum, log a warning that the process may still be running.
🟡 Security
CORS origin is hardcoded default
app.use("*", cors({ origin: process.env.CORS_ORIGIN || "http://localhost:5173" }));Fine for dev, but worth documenting that CORS_ORIGIN must be set in production.
No auth on WebSocket endpoint
Any process on the host can connect to /ws/terminal/:sessionId and get full PTY access to a Claude Code session. Even a random token in the session URL would raise the bar. Not blocking for v0, but worth tracking.
buildEnv() copies entire process.env
This passes all server environment variables (API keys, secrets) into the spawned claude subprocess. Consider allowlisting only what claude needs.
🟢 Style / Minor
packages/server/tsconfig.json uses "types": ["bun-types"] but the server runs on Node.js — this will cause type conflicts. Should be removed or swapped for @types/node (which is already a devDependency).
CI test command uses shell glob expansion
run: cd packages/server && npx tsx --test src/__tests__/*.test.tsShell glob expansion in CI is platform-dependent. Consider find src/__tests__ -name "*.test.ts" or explicit file listing.
main.ts — navigator.userAgentData?.platform is non-standard
The userAgentData API is Chromium-only. The navigator.platform fallback covers this, but the ?? chain is slightly misleading since platform is always defined (just deprecated).
✅ Looks Good
- PTY cleanup on SIGINT/SIGTERM is solid
_resetForTesting()is a clean test hygiene pattern- Input validation in session routes is thorough
- WebGL → canvas graceful fallback in xterm.js
macOptionIsMeta+attachCustomKeyEventHandleris the right approach for macOS terminal keybindingsWeakMap<WSContext, PtyBinding>for WS→PTY bindings avoids leaks nicely
|
Update on approach: This is a personal dev tool — we're not hardening against prompt injection, auth, or adversarial inputs. The dashboard itself gives full terminal access, so gating will happen at the access layer later, not at individual API endpoints. Fixes in latest push:
The rest of the review items (auth, JSON detection, Makefile verification) are fine to defer — they're not blocking a v0 personal tool. |
nox-0x
left a comment
There was a problem hiding this comment.
Code Review — feat: v0 server + terminal view
Solid v0 foundation. The architecture is clean, the monorepo structure makes sense, and the test coverage for edge cases (missing HOME, invalid JSON, PTY unavailability) is genuinely good. A few things worth addressing before or shortly after merge:
🔴 Issues
1. sessions.ts — PTY onExit deletes session before callers can observe "stopped" status
pty.onExit(() => {
session.status = "stopped";
sessions.delete(id); // ← deleted immediately; GET /api/sessions/:id returns 404 instead of "stopped"
});If the dashboard tries to poll status after the process exits, it gets a 404 rather than a stopped session it can render. Consider keeping sessions in a separate stopped map (or a TTL) for a short grace period.
2. terminal.ts — JSON prefix check is fragile
if (msg.startsWith("{")) { ... }User input (e.g. paste a JSON snippet into the terminal) will hit this branch and silently swallow valid keystrokes if parsed?.type is not "resize". The code does fall through to pty.write() in that case, but only after an unnecessary JSON.parse. Consider using a protocol envelope (e.g. length-prefix or a dedicated control channel) to separate control messages from raw terminal data.
3. sessions.ts — expandPath does not handle ~username form
path.replace(/^~/, process.env.HOME || "")This correctly handles ~ and ~/… but will turn ~otheruser/projects into $HOME/otheruser/projects. Low risk for a personal tool, but document the limitation or add a guard.
🟡 Warnings
4. No rate limiting / session cap on POST /api/sessions
With make dev exposed on LAN, a single errant client (or a hot-reload loop) could spawn unbounded PTY processes. A simple in-memory cap (e.g. MAX_SESSIONS = 10) would be prudent even for a personal tool.
5. sessions.ts — buildEnv() mutation
const env = { ...process.env } as Record<string, string>;process.env values can be undefined; the cast hides it. If a value like PATH is undefined, the string interpolation [...extraPaths, env.PATH].join(":") produces "...:undefined". Guard with env.PATH ?? "".
6. dashboard/src/main.ts — navigator.userAgentData deprecation path
/mac/i.test(navigator.userAgentData?.platform ?? navigator.platform ?? "")navigator.platform is deprecated and returns empty string in some browsers. The fallback chain is fine for now but worth a comment.
7. CI workflow — uses npx tsx instead of the pinned tsx from devDependencies
run: cd packages/server && npx tsx --test src/__tests__/*.test.tsIf npx resolves a different tsx version than bun install pinned, tests may behave differently in CI vs local. Prefer bunx tsx or ./node_modules/.bin/tsx.
🟢 Nice
WeakMap<WSContext, PtyBinding>for binding cleanup — no leaks- Resize bounds validation (
MIN_COLS/MAX_COLS) is a good guard _resetForTesting()export is clean test hygienemacOptionIsMeta: truediscovery is well-documentedCLAUDECODEenv strip to avoid nested session detection — clever, and good that it's called out in the PR description
- Add root tsconfig.json with project references across all 3 packages - Add tsconfig.base.json with shared compiler options (composite, declarations) - Fix server types: use @types/node instead of bun-types - Add Biome for linting + formatting, auto-fix all files - Consolidate Makefile to 5 targets: dev, setup, check, fmt, clean - CI runs `make check` (lint + typecheck + test) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nox-0x
left a comment
There was a problem hiding this comment.
Clean v0 — solid foundation. PTY/WebSocket plumbing is well-structured, prompt injection is handled (-- separator), env sanitization looks good. One minor note: session names use sessions.size + 1 at creation time so names can repeat after sessions exit, but no runtime impact.
nox-0x
left a comment
There was a problem hiding this comment.
Solid v0 scaffold — clean architecture, good error handling, proper test coverage. Ship it! 🚀
…LOG (pipeline PR 1/6) (#181) build(release): adopt changesets — automated versioning + single root CHANGELOG PR #1 of the professional release pipeline (approved design, macOS-only desktop). Replaces the manual `sed`-across-5-package.json version bump that caused the 0.0.2-code / v0.0.1-tag drift. ## What this adds - **changesets** (`@changesets/cli` + `@changesets/changelog-github`) with a `fixed` group covering all 5 packages → they always version in lockstep to a single version. Devs declare bumps via `bun run changeset` (a 3-line file per user-facing PR); see `.changeset/README.md`. - **Single root `CHANGELOG.md`** (Keep-a-Changelog), seeded with the hand-written 0.0.1 + 0.0.2 history. From the next release on it's generated automatically. Per-package changelogs are gitignored — `scripts/sync-changelog.ts` promotes each new version's section into the root one after `changeset version`. - **`scripts/release-notes.ts`** extracts a version's CHANGELOG section for the GitHub Release body (wired into release.yml in PR #3). - **`.github/workflows/version.yml`** — maintains the "Version Packages" PR and, when it merges, auto-tags `vX.Y.Z` to trigger the release build. Releasing becomes "merge the Version Packages PR" — no manual version edits, ever. - **`.github/workflows/changeset-check.yml`** — informational PR nudge to include a changeset (never blocks; trivial PRs use `--empty`). - Marked `cli`/`core`/`dashboard`/`server` **private** — they're internal workspace packages, not npm publishes (prevents accidental publish + clarifies intent to changesets). ## Validated locally (not just claimed) - `bun run version` with a throwaway minor changeset → all 5 packages bumped 0.0.2 → 0.1.0 in lockstep, root CHANGELOG got a dated section, then reverted. - `bun scripts/release-notes.ts 0.0.2` extracts the right section. - `make check`: 353/353 tests pass; `biome check packages/` clean. ## Dormant until PR #3 No changeset is included in this PR on purpose — the version machinery stays dormant (no Version PR / tag) until the new `release.yml` lands in PR #3. The changeset-check will warn on this infra PR; that's expected. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Personal dev tool v0 — the full "hello world" loop for controlling Claude Code from a browser terminal.
core(types),server(Hono + node-pty),dashboard(xterm.js)This is a personal tool, built for Terry first. No auth, no multi-tenancy, no public API hardening. We'll gate access at the dashboard level later.
Architecture
graph LR A[Browser<br/>xterm.js] -->|WebSocket| B[Hono Server<br/>Node.js + tsx] B -->|node-pty| C[Claude Code<br/>PTY subprocess] C -->|PTY output| B B -->|stream| A A -->|keystrokes| B B -->|PTY input| CWhat's in each package
packages/core— Shared typesSession,AgentEvent,AgentProviderinterfacespackages/server— Hono + node-ptypackages/dashboard— Vite + xterm.jsmacOptionIsMetafor proper macOS Option keyKey discoveries
posix_spawnpregression on Node v25 — pinned to 1.0.0CLAUDECODEenv var must be stripped to avoid nested session detectionmacOptionIsMeta: trueis essential on macOS — without it, Option+key produces Unicode gibberishHow to run
Test plan
🤖 Generated with Claude Code