Conversation
added 22 commits
March 24, 2026 13:11
…related documentation
…ver if no sessions left
…dd configurable clipboard behavior
…ening config file
…design choices and config behavior
…d update startServer to handle logs
…ss and API for config injection
…rver to trigger shutdown
…et correctly for CLI and config tests
…tups, enhance mime type handling, and improve temporary directory management
…client import paths
There was a problem hiding this comment.
Pull request overview
This PR modernizes webtty’s UX and architecture by moving the browser client to CSP-safe static assets, expanding the config surface (including clipboard/logging behavior), tightening server/CLI lifecycle behavior, and improving the test harness to be more isolated and integration-focused.
Changes:
- Extract browser client into
src/client/*, build it viascripts/build.ts, and serve it as static assets; addGET /api/configfor runtime client config. - Polish CLI command surface and help output (
at/ls/rm/mv, no-arg default behavior,config/helpcommands), plus server lifecycle tweaks (stop when last session exits / last rm in CLI). - Improve test infrastructure (isolated HOME dirs, shared helpers, new unit + integration tests for static serving and PTY).
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Include scripts/ in TS project for typechecking/build tooling. |
| src/utils.ts | Remove shared MIME/static helpers (moved server-side). |
| src/utils.test.ts | Repurpose into shared test utilities (ports, temp HOME, server wait helpers). |
| src/server/websocket.ts | Add “last session closed” hook to enable auto-shutdown on final PTY exit. |
| src/server/websocket.test.ts | Use shared test helpers + add test for shutdown on last session exit. |
| src/server/static.ts | Move MIME + ghostty-web path helpers into server static module. |
| src/server/static.test.ts | Add unit tests for findGhosttyWeb, mimeType, serveFile. |
| src/server/routes.ts | Add GET /api/config, serve static client.html, and serve /dist/* from webtty dist first. |
| src/server/routes.test.ts | Update expectations for static HTML shell + add /api/config test. |
| src/server/index.ts | Wire clientDistPath into routes and install last-session-closed shutdown handler. |
| src/server/client.ts | Remove server-side HTML rendering template. |
| src/server/client.test.ts | Remove tests for deleted server-side client renderer. |
| src/pty/index.test.ts | Add PTY integration tests. |
| src/config.ts | Add copyOnSelect/rightClickBehavior/logs, and allow HOME override for config path. |
| src/config.test.ts | Switch to HOME override (no os.homedir spying) + add tests for new config keys. |
| src/client/index.ts | New browser TS entry: fetch config, init terminal, WS reconnect, copy behaviors. |
| src/client/index.html | New static HTML shell referencing built JS/CSS. |
| src/client/index.css | New extracted client CSS styles. |
| src/cli/index.ts | Custom help formatting and no-arg default behavior. |
| src/cli/http.ts | Harden isServerRunning + implement optional server logging to file. |
| src/cli/http.test.ts | Add coverage for logging + refactor platform stubbing. |
| src/cli/commands.ts | Rename/polish commands, add config/help, stop-on-last-rm behavior. |
| src/cli/commands.test.ts | Update CLI tests for new commands, add no-arg/help/config coverage. |
| scripts/build.ts | Build server + browser bundles, copy static HTML/CSS into dist/. |
| docs/specs/webtty.md | Update spec text and API details (notably delete header semantics). |
| docs/specs/config.md | Document new config keys and reload behavior. |
| docs/specs/client.md | Document static asset architecture and /api/config endpoint. |
| docs/specs/cli.md | Update CLI spec for new command surface and UX changes. |
| docs/adrs/012.client.static-assets.md | New ADR documenting static client build + CSP rationale. |
| docs/adrs/011.cli.default-and-help.md | Remove superseded ADR. |
| docs/adrs/011.cli.config-and-help.md | New ADR for CLI polish, config/help, command changes. |
| docs/adrs/002.cli.start-stop.md | Clarify rationale for HTTP lifecycle approach. |
| docs/adrs/001.webtty.bootstrap.md | Add single-port and ws rationale notes. |
| README.md | Update quickstart commands to match new CLI behavior. |
Comments suppressed due to low confidence (1)
src/cli/index.ts:85
program.parseAsync(process.argv)is called without being awaited/returned. Since unresolved Promises don’t keep the event loop alive, the process can exit before async command actions complete (depending on timing), and any rejection becomes unhandled. Prefer top-levelawait program.parseAsync(process.argv)(ESM supports it) or return the promise from the entrypoint.
program.action(async () => {
await program.parseAsync(['at', 'main'], { from: 'user' });
});
program.parseAsync(process.argv);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…test failure Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Re-enable --help by removing helpOption(false); custom formatHelp already omits Options section - Fix cli.md: start/stop lifecycle wording, remove stale 'down' alias mention - Export configDir() from config.ts for consistent HOME-aware path resolution - Fix logPath() to use configDir() instead of os.homedir() - Fix fd leak: close log fd in parent process after spawn - Fix config cmd: use configDir(), ensure config dir/file exists before opening editor - Improve clientDistPath derivation: use __dirname-relative path instead of ghostty-web location Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
jesse23
commented
Mar 25, 2026
jesse23
left a comment
Owner
Author
There was a problem hiding this comment.
All 7 Copilot comments addressed in e200a52:
- #1
helpOption(false)removed —--helpworks again; customformatHelpalready omits the Options section so help output is unchanged - #2/#3 cli.md updated:
start/stoplifecycle wording, staledownalias mention removed - #4
clientDistPathnow derived from__dirname(server entry location) — robust regardless of node_modules layout; also falls back tosrc/client/whendist/not built - #5
logPath()now uses exportedconfigDir()fromconfig.tswhich respectsprocess.env.HOME - #6 Log fd closed in parent process immediately after
child.unref() - #7
webtty configusesconfigDir(), creates dir and empty config file if missing before opening editor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
webtty helpas canonical entry, all-caps headings, frequency-ordered commands,at/ls/rm/mvwith 2-char names and full aliases (attach,list,remove,rename),stop/startserver lifecycle,webttyno-arg opens main sessionsrc/client/index.ts, compiled byBun.build, served as/dist/client-browser.js; zero inline scripts,script-src 'self'CSP-safe;GET /api/configendpoint replaces server-side template injectioncopyOnSelect,rightClickBehavior,logsconfig keys;$VISUAL→$EDITOR→vi/notepadeditor resolution;isServerRunning()validates JSON array to reject non-webtty processes; stop server when last session exits or is removedHOMEdirs for all subprocess tests (no more config file clobbering); shared test utilities insrc/utils.test.ts;pty/index.test.tswith real PTY integration tests;src/server/static.test.tscoveringfindGhosttyWeb/serveFile; 110 tests, 100% coverage on all unit-testable filesdevelopment.md; specs and ADRs updated throughoutADRs
at/mv, stop-on-last-rm, copy configGET /api/config, CSP rationale