Skip to content

feat: phase 1 — one terminal in the browser#2

Merged
srid merged 15 commits intomasterfrom
claude/elastic-raman
Mar 20, 2026
Merged

feat: phase 1 — one terminal in the browser#2
srid merged 15 commits intomasterfrom
claude/elastic-raman

Conversation

@srid
Copy link
Copy Markdown
Member

@srid srid commented Mar 19, 2026

image

srid added 3 commits March 19, 2026 18:09
Full terminal stack: Axum spawns a PTY (portable-pty), pipes output
through a broadcast channel to WebSocket clients. Browser renders via
ghostty-web through a thin JS bridge (ghostty-bridge.js) with 1:1
wasm-bindgen bindings.

- server: pty.rs (spawn/read/write/resize via channels), ws.rs
  (bidirectional WS↔PTY pipe with scrollback replay), state.rs
- client: ghostty-bridge.js, terminal.rs (FFI bindings),
  terminal_view.rs (Leptos component with WS status signal)
- common: WsClientMessage/WsServerMessage enums, DEFAULT_COLS/ROWS
- nix: fetchurl ghostty-web tarball, inject into buildTrunkPackage
- UI: compact header with branding + WS status indicator
- e2e: smoke (branding + canvas), terminal input, resize survival
- justfile: test-dev recipe for fast testing against running dev server
- Add ResizeObserver so terminal reflows when browser viewport changes
- Extract send_resize() helper to deduplicate 3x WS resize pattern
- Create e2e test DSL (tests/e2e/dsl/) inspired by openspatial:
  scenario() entry point, TerminalView/AppView typed abstractions
- Rewrite smoke + terminal tests using the DSL
- Add border around terminal container
- Inline ghosttyWebTgz into single nix derivation
nix build compiles Rust + WASM from scratch, which can exceed the
previous 120s timeout. Bump to 600s.
Comment thread client/src/main.rs Outdated
Comment thread client/src/terminal_view.rs Outdated
Comment thread client/src/terminal_view.rs
Comment thread client/src/terminal_view.rs
Comment thread client/src/terminal_view.rs Outdated
Comment thread server/src/state.rs
Comment thread server/src/ws.rs
srid added 9 commits March 19, 2026 18:46
Address PR review: split terminal_view.rs monolith into focused modules,
extract named functions from closure nests, add doc comments throughout.
Update code-review skill with learnings on module structure and readability.
Replace manual Closure::wrap + forget with leptos-use hooks:
- use_websocket_with_options for WS connection
- use_resize_observer for container resize
- use_event_listener_with_options for font zoom keydown
- on_cleanup for terminal dispose on unmount

Extract bridge.rs for non-Leptos JS helpers (wait_animation_frame,
extract_size, localStorage, build_ws_url). Simplify ws.rs to just
WsStatus type with From<ConnectionReadyState>.
- measureCells() now uses actual grid dimensions instead of hardcoded
  80x24, so cell size is correct after font size changes
- stop_propagation() on zoom shortcuts prevents =/-/+ leaking to PTY
- Add e2e tests: canvas fills container, fills after zoom, no keystroke leaks
ws_open() is non-blocking — the resize message sent immediately after
was silently dropped. Now watches ready_state signal and sends resize
when WS transitions to Open. Adds e2e test verifying cols > 80.
@srid srid marked this pull request as ready for review March 20, 2026 00:23
srid and others added 3 commits March 19, 2026 20:24
- bridge::extract_size now returns Option<(u16, u16)> instead of
  hardcoding defaults from kolu_common
- Collapse nested if in ws.rs (clippy::collapsible_if)
- Update CLAUDE.md with clippy and pre-commit quality gates
@srid srid merged commit 9b93616 into master Mar 20, 2026
5 checks passed
@srid srid deleted the claude/elastic-raman branch March 20, 2026 00:51
srid added a commit that referenced this pull request Apr 17, 2026
**The Debug → Diagnostic info dialog now surfaces the memory and WebGL
axes that were invisible before** — JS heap (used/total/limit), DOM node
count, canvas count, per-terminal scrollback depth, and, for the focused
WebGL tile, the char-atlas canvas dimensions. Copy-JSON becomes a
forensic-grade payload a user can paste into a bug report and pair with
Chrome's Task Manager numbers to see the whole picture in one glance.

Motivated by **issue #591** (WebGL _zombie-context_ leak — kolu tabs
growing from 300 MB to ~2 GB over hours). Live monitoring showed the
kolu PWA sitting at 622 MB total footprint with **500 MB of GPU memory**
but only 55 MB of JS heap — _six times_ the next-highest tab's GPU use,
matching a ~16 × ~30 MB zombie-context ceiling almost exactly. The JS
side told us nothing; the leak lives outside the heap. This dialog now
shows enough to confirm that gap without reaching for DevTools.

A small structural side note: the per-terminal `webglAtlas` probe is
volatile (it flips on/off as focus moves between tiles), while
`TerminalRefs` so far only held permanent-lifetime handles (`xterm`,
`serialize`). The PR namespaces the volatile accessors under a new
`probes: { webglAtlas }` sub-object so the _stability contrast stays
visible in the type_ and the next diagnostic probe doesn't quietly land
next to `xterm` as a grab-bag field. While touching the area, I also
hoisted the WebGL2-support probe to module scope — the old code created
a throwaway WebGL2 context on every dialog mount, which is itself a tiny
zombie-context leak. _The exact pattern #591 exists to diagnose, fixed
in passing._

> This PR is diagnostic tooling, not the fix. The prevailing hypothesis
(from the #591 investigation) is that `Terminal.tsx`'s `unloadWebgl()`
needs to call `w.dispose()` _before_ `loseContext()` — xterm's
`webglcontextlost` listener `preventDefault()`s while the addon is still
registered, which likely keeps contexts alive past their disposal. That
fix is out of scope here.

> **Deferred:** Centralized GPU/WebGL introspection surface (future
`MAX_TEXTURE_SIZE`, `WEBGL_debug_renderer_info`, etc.) — today's
`webglAtlas` is the first probe; probe #2 should get a dedicated home
rather than accreting onto `TerminalProbes`.

See #591.

### Try it locally

```sh
nix run github:juspay/kolu/feat/diagnostic-memory-fields
```
srid added a commit that referenced this pull request Apr 18, 2026
Per Lowy review: lookup-fn props are unenforced coupling — every
consumer must be hand-wired identically, and the indirection looks like
encapsulation but isn't.

- useTerminalStore: cache + createRoot singleton, every consumer reads
  the same store
- useThemeManager: drop deps argument; pulls active id / metadata /
  setTheme from the store + RPC client directly. Cached + createRoot.
- New canvas/useTileTheme.ts: singleton accessor mapping id → TileTheme,
  centralising the (xterm ITheme → bg/fg with fallbacks) adapter that
  used to be inline in App.tsx as `getChromeTileTheme`.
- ChromeBar / PillTree / TerminalCanvas / CanvasMinimap /
  MobileChromeSheet / MobileTileView: drop the lookup-fn props,
  read from the singletons. Their prop surface shrinks substantially.
- App.tsx: stops threading store accessors through layout components.

Hickey #2: while we were in TerminalCanvas, fold the duplicated
CanvasTile prop-spell-out (tiled vs maximized) into a single
renderTile(id, maximized) helper.
@ThisIsMani ThisIsMani mentioned this pull request Apr 28, 2026
3 tasks
srid added a commit that referenced this pull request Apr 30, 2026
…n autosave clobber (#796)

**Three independent macOS CI failures, each with a distinct root
cause.** Locally each one repros at 10–25% under parallel load on
darwin; on x86_64-linux they're either masked by faster timing or
genuinely don't apply. Together they account for every red step the e2e
+ unit suites were posting on darwin.

| # | Symptom | Root cause | Fix |
| --- | --- | --- | --- |
| 1 | `Close terminal on worktree …` times out clicking the Remove
button | `closeTerminal()` snapshots `meta.git?.isWorktree` once at
dialog-open; the test only waited for CWD, not the async git-metadata
stream | Wait for the worktree indicator before initiating close |
| 2 | 9 `watchGitHead` / `subscribeGitInfo` unit tests assert
`_sharedHeadWatcherCount() = 0/1/2`, get 2/4/7… | `git rev-parse
--git-dir` returns relative `.git` from the repo root but a
realpath-resolved absolute path from a subdir; on macOS `/tmp` ↔
`/private/tmp` collides the registry | Pipe the resolved path through
`fs.realpathSync` |
| 3 | `session-restore` scenarios — restore card flashes visible then
disappears mid-scenario | Pending autosave timer scheduled by a stale
`terminals:dirty` event fires 500ms later, runs `saveSession([])` and
writes `null`, clobbering the test-set session | `setSavedSession`
cancels the pending autosave timer before writing |

### #2 in detail

The shared-watcher registry is keyed by the resolved gitDir string. When
two callers reach the same repo via different paths — one from the root,
one from a subdir — `git rev-parse --git-dir` reports them differently:

```
cwd=/tmp/repo            → "git rev-parse --git-dir" → ".git"           → /tmp/repo/.git
cwd=/tmp/repo/src/deep   → "git rev-parse --git-dir" → "/private/...   → /private/tmp/repo/.git
                                                       tmp/repo/.git"
```

Different keys → two `fs.watch` handles for one `.git`.
`fs.realpathSync` collapses both to `/private/tmp/repo/.git` and the
singleton invariant holds.

### #3 in detail

The Before hook drains terminals and posts `session: null`; the Given
step posts the test session; the page loads. _Between those_, a
lingering provider event from a drained terminal in the previous
scenario fires `terminals:dirty`, the leading-edge throttle (#767)
schedules a save 500ms out, and the callback runs with an empty terminal
snapshot. `saveSession([])` rewrites the session to `null`, the client's
`session.get` subscription pushes `null`, and the `<Show
when={savedSession}>` unmounts the restore card the test was about to
assert on.

> _The fix is local to `setSavedSession`. Production callers (test-only
path today) get a free upgrade: an explicit set always wins over a
pending autosave._

### Verification

- `CUCUMBER_PARALLEL=1 just test-quick features/worktree.feature` — 5/5
(was 5/5; fix is for a parallel-only race in CI)
- `just test-quick features/session-restore.feature` ×5 with parallel=4
— 5/5 (was 1–2/5 failing on master and on this branch before the
autosave fix)
- `pnpm -r test:unit` — all suites green, including the 9
previously-failing `watchGitHead` / `subscribeGitInfo` cases

_Generated by [`/do`](https://github.com/srid/agency) on Claude Code
(model \`claude-opus-4-7\`)._
srid added a commit that referenced this pull request May 1, 2026
… runtime

Both Lowy F2 and Hickey #2 flagged Cell.name as dual-use (contract key
AND publisher channel). Re-checking the actual code: cell.name is used
only for type identity and one error string in collectionHandlers.
Channel names are explicit strings in cellBus ("preferences:changed",
"terminal-list"); contract dispatch goes through procedure refs the
caller passes to hooks. The dual-use risk was theoretical, never actual.
Update the doc to say so — name is a descriptor identifier, not a
runtime-load-bearing dispatch key.
srid added a commit that referenced this pull request May 6, 2026
…a renderer factory

Hickey #1: the virtualization-mode predicate was decided at the
constructor site (vanilla vs. virtualized class) and re-checked at the
render site (containerWrapper vs. fileContainer), with nothing enforcing
that the two branches stay in sync. Extract the pairing into a
`createDiffRenderer` / `createFileRenderer` factory so a future
render-option addition can't be applied to one arm and missed on the
other. Also addresses Hickey #2 (collapsed ternary) and Lowy #3 (the
branch point is now named and commented inside the factory).
srid added a commit that referenced this pull request May 6, 2026
Lowy #2: virtualization on/off is decided ambiently by whether
<Virtualizer> sits in the JSX tree, but the contract that the root
must scroll is implicit — a caller who wraps in <Virtualizer> without
`overflow-auto` activates the virtualized Pierre class but gets
full-DOM rendering anyway, because the IntersectionObserver root
computes against the layout box and sees everything as visible.

Catch the misconfig with a dev-mode console warning at the boundary
where the contract lives, not in informal documentation.
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.

1 participant