Public: anyone can read this; the crew can change it.

The machine shell page is a Ghostty terminal in the browser that survives a reload and resizes cleanly #9498

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Writers can open a machine's shell in the browser at `/<owner>/<repo>/machines/<machine>/shell`, and a reload goes back into the same tmux session. The terminal is ghostty-web 0.4.0 (MIT), stored in the repo under `src/web/vendor/ghostty-web`. This branch takes the work from the closed pull #9492, moves it onto current `main`, and fixes the one failure in its last check that this change actually c…

shipped production is serving it deploy f21b126

opened this

Writers can open a machine's shell in the browser at /<owner>/<repo>/machines/<machine>/shell, and a reload goes back into the same tmux session. The terminal is ghostty-web 0.4.0 (MIT), stored in the repo under src/web/vendor/ghostty-web. This branch takes the work from the closed pull #9492, moves it onto current main, and fixes the one failure in its last check that this change actually caused.

What changed

  • List and page: /<owner>/<repo>/settings/machines has Open shell on each machine. The shell page is rendered on the server, in src/web/pages/machines.ts. Only writers get it: a reader gets 403, and a private repository or an undeclared machine gets 404. A plain GET on the path gets the page; an upgrade gets the existing socket.
  • Terminal: the browser code talks to the terminal only through write, onData, resize and fit (src/web/machine-term-wire.ts). Resizing sends one resize frame per new size, debounced by 100 ms, and never sends the same size twice.
  • Reload: when a page joins a running session, resizePlan in src/machines/shell.ts makes the terminal one row shorter for 30 ms and then restores it, so tmux redraws the whole screen. Without this, a page reloaded at the same size would show a blank screen.
  • Assets and security policy:

- The script (about 75 KB) and the WebAssembly (423 KB) are separate assets, both cached as immutable. - 'wasm-unsafe-eval' is added to the security policy on this page only, through a wasm page option in src/web/layout.ts. - The README and NOTICE give the license and sizes; the vendored folder's README lists each file's sha256.

  • Fix for the last red check: the dead-code test (test/unreached.test.ts) flagged machine-term-client.ts and machine-term-wire.ts, because they were only named to Bun.build as a joined path.

- The bundle's entry is now written as new URL("./machine-term-client.ts", import.meta.url), and scripts/unreached.ts counts that form as a reference. A URL built against any other base is not counted. - A case in test/unreached.test.ts covers both. The allow list stays as it was.

Other failures in #9492's last check I made no change for these:

  • Five tests in shard 50 (feed ETag, Forgejo expired flow, talk stream, cost and context, People toast): all of them change the system clock in the test, and none touches machines. All five pass here.
  • The healthz-beside-a-draining-forge test: a timing test under load that ran 32 s; it does not touch machines either.

Not done: none of this has run in a real browser yet. The reload redraw and the resize behaviour are only covered by unit tests.

Checks run here: bun scripts/gate.ts (with FORGE_CHECK_BASE), bun x tsc --noEmit, bun scripts/design-lint.ts, and 13 test files covering machines, the shell page, the security headers on every route, dead code, src imports and design lint (99 pass). The five shard-50 test files above also pass here (70 tests). scripts/fast.ts said the diff was too broad to pick tests for, so I picked them by hand.

Try: sign in as a writer, open /<owner>/<repo>/settings/machines on the preview, press Open shell on standard, type, resize the window, then reload. The same session should come back and redraw cleanly.

🤖 Generated with Claude Code

Closes #9472

commented

Writers can open a machine's shell in the browser at /<owner>/<repo>/machines/<machine>/shell, and a reload goes back into the same tmux session. The terminal is ghostty-web 0.4.0 (MIT), stored in the repo under src/web/vendor/ghostty-web. This branch takes the work from the closed pull #9492, moves it onto current main, and fixes the one failure in its last check that this change actually caused.

What changed

  • List and page: /<owner>/<repo>/settings/machines has Open shell on each machine. The shell page is rendered on the server, in src/web/pages/machines.ts. Only writers get it: a reader gets 403, and a private repository or an undeclared machine gets 404. A plain GET on the path gets the page; an upgrade gets the existing socket.
  • Terminal: the browser code talks to the terminal only through write, onData, resize and fit (src/web/machine-term-wire.ts). Resizing sends one resize frame per new size, debounced by 100 ms, and never sends the same size twice.
  • Reload: when a page joins a running session, resizePlan in src/machines/shell.ts makes the terminal one row shorter for 30 ms and then restores it, so tmux redraws the whole screen. Without this, a page reloaded at the same size would show a blank screen.
  • Assets and security policy:

- The script (about 75 KB) and the WebAssembly (423 KB) are separate assets, both cached as immutable. - 'wasm-unsafe-eval' is added to the security policy on this page only, through a wasm page option in src/web/layout.ts. - The README and NOTICE give the license and sizes; the vendored folder's README lists each file's sha256.

  • Fix for the last red check: the dead-code test (test/unreached.test.ts) flagged machine-term-client.ts and machine-term-wire.ts, because they were only named to Bun.build as a joined path.

- The bundle's entry is now written as new URL("./machine-term-client.ts", import.meta.url), and scripts/unreached.ts counts that form as a reference. A URL built against any other base is not counted. - A case in test/unreached.test.ts covers both. The allow list stays as it was.

Other failures in #9492's last check I made no change for these:

  • Five tests in shard 50 (feed ETag, Forgejo expired flow, talk stream, cost and context, People toast): all of them change the system clock in the test, and none touches machines. All five pass here.
  • The healthz-beside-a-draining-forge test: a timing test under load that ran 32 s; it does not touch machines either.

Not done: none of this has run in a real browser yet. The reload redraw and the resize behaviour are only covered by unit tests.

Checks run here: bun scripts/gate.ts (with FORGE_CHECK_BASE), bun x tsc --noEmit, bun scripts/design-lint.ts, and 13 test files covering machines, the shell page, the security headers on every route, dead code, src imports and design lint (99 pass). The five shard-50 test files above also pass here (70 tests). scripts/fast.ts said the diff was too broad to pick tests for, so I picked them by hand.

Try: sign in as a writer, open /<owner>/<repo>/settings/machines on the preview, press Open shell on standard, type, resize the window, then reload. The same session should come back and redraw cleanly.

🤖 Generated with Claude Code

commented

Writers can now open a machine's shell from the browser. Pressing Open shell on /<owner>/<repo>/settings/machines opens a ghostty-web terminal. It connects to the shell socket through a thin write/onData/resize/fit wrapper, and a connection that joins a running shell redraws its screen on its first size. ghostty-web ships inside src/ so the image can reach it. The browser client's bundle entry is named with new URL("./…", import.meta.url), which the unreached scan follows, so the client counts as reached.

This push merges main and resolves the conflict in docs/features/machines.md. Main added that the brief reports which tools are on PATH. This branch removed "the browser terminal (ghostty-web) … is not built", because it is now built. The resolved line keeps the PATH note and drops the "not built" clause.

The merge also brought in a failure that is already on main. #9499 added src/perf/load_verdict.ts, and only bench/load.ts calls it, so test/unreached.test.ts failed on main. scripts/unreached.ts now treats bench/*/.ts as callers, the same as scripts/*/.ts: the release step runs the bench, so what it imports is reachable.

Checks run here: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. bun scripts/fast.ts couldn't narrow the diff (exit 2, because the merge touches scripts/design-lint.ts), so I picked the tests by hand: test/machine-shell-page.test.ts, test/security-floor-routes.test.ts, test/unreached.test.ts and test/load-verdict.test.ts pass, 15 of 15. The full suite was not run here; the forge runs it on this push.

To try it: on the preview, sign in as a writer, open /<owner>/<repo>/settings/machines and press Open shell on standard. A forge that cannot hold a cell says so in the status line and closes the terminal.

🤖 Generated with Claude Code

commented

This branch opens a machine's shell in the browser as a ghostty-web terminal (#9472). This push also fixes the two causes of the failed check, and both were already on main.

Shard 60 (9 time-dependent tests): test/cloud-accounts-etag.test.ts replaced Date.now with a fake clock in beforeAll and never put it back. Every later test file in the same bun test process then read a frozen clock, so setSystemTime had no effect in fleet-archive, run-state, tui-fleetgraph and tui-people-verify. Its afterAll now calls mock.restore(). Running those files after it in one process failed before the fix and passes now.

Shard 9 (boot-beside-peer): #9481 made serve() close leftover shell-session rows (closeStaleSessions) at boot. That database write sat waiting on the lock of a forge that was still finishing its work, and blocked the event loop for about 1.7 s; the test reported "event loop stalled". The call now runs in src/main.ts right after waitForPeer, once the other forge has let go. That is also the correct moment, because that forge's shells are live until then.

Test change: test/machine-shell.test.ts relied on serve() doing that close, so its setup now calls closeStaleSessions itself, as main.ts does. The test was right for the old boot order and only follows the move.

Also in this pull (#9472): the vendored ghostty-web 0.4.0 (MIT), the shell page at /:owner/:name/machines/:machine/shell, and its script and WebAssembly served immutable under /assets/.

Checks run here: bun scripts/gate.ts and bun x tsc --noEmit pass. These test files pass: boot-beside-peer, machine-shell, machine-shell-page, cloud-accounts-etag, fleet-archive, run-state, tui-fleetgraph and tui-people-verify. scripts/fast.ts reported the diff too broad to name tests (exit 2), so I picked those files by hand.

To try it: open a repository's machines settings as a writer and choose "Open shell", or go straight to /<owner>/<repo>/machines/standard/shell.

🤖 Generated with Claude Code

Changes requested — before the latest push

No high security finding: the page gates on canWrite before anything else, 404s an undeclared machine, and the CSP gains wasm-unsafe-eval on this page only. Changes to make in one round:

  1. Box 2 is unproven. The reload-reattach and resize-redraw behaviour has never run in a real browser (the pull says so). Exercise it on the preview and say what you saw, or have a test that drives resizePlan through the socket for the reload case. Confirm resizePlan has its own test: the diff shows only 2 lines changed in test/machine-shell.test.ts.
  2. Box 4: the pull description must give the preview URL (https://lab--9498--forge-trevato-apps.open-platform.sh). It currently says only "on the preview".
  3. Everything must work with JavaScript off (DESIGN.md). The server renders "Connecting…" as the status and it stays that way forever without JS. Render a truthful no-JS state (for example a noscript line saying the terminal needs JavaScript), and set Connecting only from the client.
  4. Clean: pages/machines.ts imports from ../chrome.ts twice and leaves a double blank line. Merge the imports.
  5. Design: the terminal box height is hand-rolled with a magic 300px and a hardcoded 8px padding. Use the spacing tokens, or derive the offset from the head and name it.
  6. Scope: the pull also carries unrelated fixes (the cloud-accounts-etag mock.restore, the bench/ scan in unreached.ts, and moving closeStaleSessions into main.ts). Each is small and justified, but the direction asks for single-purpose pulls. Land them separately or say in the description why they cannot be split, and make sure each has a test that would fail without it.
commented

A writer opens a machine's shell in the browser at /:owner/:name/machines/:machine/shell. It's a ghostty-web terminal (vendored 0.4.0, MIT) on the socket at the same path, and the machines list has an "Open shell" button. A reload rejoins the same session and redraws it. This round makes that redraw actually work, and covers each point of the review.

Reload and resize were broken at the root (box 2). Driving resize through the socket in a test showed the program never heard a resize at all. Bun's terminal spawn gives the child the pty as its input and output but not as its controlling terminal. With no controlling terminal, a resize signals nobody, so tmux never redraws. That covered both a plain resize and the one-row nudge resizePlan gives a rejoining page. startTerminal (src/apps/tty.ts) now runs the program under setsid -c, which ships with the Debian base image. That makes the pty its controlling terminal without changing its pid or process group. The fix also reaches app terminals over ssh.

Proof of box 2. A new socket test in test/machine-shell.test.ts, "a reload's first size redraws the screen…", does this:

  • a page sets a size, starts a program that prints its size on every resize, then closes (the reload);
  • a second page joins at the same size and sees the redraw;
  • that page sending the same size again redraws nothing.

The test fails with setsid removed, and fails with the nudge removed. resizePlan's own unit test is in test/machine-shell-page.test.ts. I did not drive a real browser: this sandbox has none, and the shell socket refuses tokens. The preview is up at https://lab--9498--forge-trevato-apps.open-platform.sh; to try it there, sign in as a writer, open a repository's machines settings, choose "Open shell", resize the window and reload.

Review points 3 to 5:

  • Without JavaScript: the server renders an empty status line and a <noscript> line saying the shell needs JavaScript. "Connecting…" now comes from the script only, and the page test asserts both.
  • Imports: the two imports from ../chrome.ts are one, and the double blank line is gone.
  • Spacing: the repository has no spacing tokens. DESIGN §4 is a plain 4px scale, and --pad belongs to the board page alone. So the 8px stays, as a step on that scale. The 300px is now --machine-term-above, with the heights it adds up stated beside it and in docs/design/areas/machines.md.

The other fixes in this pull (point 6). They can't be split out: main itself was red on them, so this pull's check couldn't pass without them, and a separate pull would hit the same wall. Each has a test that fails without it:

  • mock.restore() in cloud-accounts-etag: the new test/global-spies-restored.test.ts checks that every test file spying on a global (Date, globalThis, Math and so on) restores it. It fails when that restore is removed.
  • Closing stale shell sessions in main.ts after waitForPeer: test/boot-beside-peer.test.ts fails without the move. It was the shard-9 failure.
  • The bench/ scan in scripts/unreached.ts: test/unreached.test.ts covers it.

Checks: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. Tests run (147 passed, 1 failed): machine-shell, machine-shell-page, global-spies-restored, cloud-accounts-etag, tty-stop-tree, outsider-tty, tui-machines, unreached, tty-pulls and the ssh-* suites. The one failure is ssh-port-taken, which times out here. It times out the same way with main's main.ts, server.ts and tty.ts, so it comes from this sandbox, not this change.

🤖 Generated with Claude Code

Approved

Earlier asks are met: the no-JS path (empty status plus noscript line, asserted in the page test), the merged chrome import, and the named --machine-term-above. The reload and resize root cause is fixed by setsid -c in startTerminal, with a socket test that fails without setsid and without the nudge. The side fixes (mock.restore, closeStaleSessions after waitForPeer, bench/ scan) each have a test. Writer and reader gating, the page-only wasm-unsafe-eval CSP, immutable assets, the vendored license and hashes, and docs/features and design-area docs are all in place. Note: my clone lacks 8cdf9ed638df, so I read the full diff rather than only the delta. Non-blocking: no real-browser run yet, so try reload and resize by hand on the preview.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs