Skip to content

Commit 91e90d4

Browse files
simionclaude
andcommitted
fix(pty): hold a terminal's first output until the webview is listening
CI's Activity spec died in its before-all: the fixture agent never drove its OSC title, so the tab had no live title to bridge. The title was not late, it was gone. pty_spawn starts the reader and flusher threads before it returns, and the webview can only call listen("pty://<id>") after that round trip resolves. Tauri events are fire-and-forget, so every byte emitted in the gap reaches nobody and leaves no trace. The child starts writing as soon as it is forked, which on a loaded runner beats the round trip. Any CLI that paints a banner plus one OSC title at startup and then blocks on stdin loses BOTH permanently, since nothing ever repaints: a blank terminal and an untitled tab, forever. Our agent fixture is exactly that shape, which is why this spec found it and the ones that submit a prompt (and get a fresh title) did not. So the frontend now acks: pty_attached flips a flag under the same buffer mutex the reader's `done` store uses, and the flusher holds its first emit until the ack lands or a 3s grace expires. The reader's final drain waits on the same gate, or a process that prints and exits inside the gap loses everything it wrote. Late, never lost: a caller that never acks costs one delay instead of a wedged terminal. Both spawn sites send it; a new one that forgets shows nothing until the grace runs out, which docs/ipc.md and docs/gotchas.md now say. Also on this run, "Upload artifacts on failure" failed with EACCES: the /var/folders/** glob for termic-debug.log walked the whole tree into an unreadable LaunchServices store, so a failing run uploaded nothing at all, screenshots included. The log is copied in by a bounded find now. And the hook that failed reports what the tab actually held, instead of leaving "never drove its OSC title" to cover a fixture that never ran, a spawn that dropped --name, and this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F2JzhUsFa9YLjBBvUJQGYj
1 parent 16247a3 commit 91e90d4

8 files changed

Lines changed: 178 additions & 8 deletions

File tree

.github/workflows/test.yml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,31 @@ jobs:
7171
run: npm run e2e:build
7272
- name: Run e2e suite
7373
run: npm run test:e2e
74+
# The debug log lives in the per-user temp dir (`std::env::temp_dir()`),
75+
# i.e. somewhere under /var/folders on macOS. Copying it here rather
76+
# than globbing it in upload-artifact: `/var/folders/**` made the
77+
# uploader walk the whole tree, where it hit an unreadable
78+
# LaunchServices store and failed the step with EACCES, so a failing
79+
# run uploaded NOTHING (not the log, not the screenshots).
80+
- name: Collect the debug log
81+
if: failure()
82+
run: |
83+
mkdir -p .e2e/artifacts
84+
cp "${TMPDIR%/}/termic-debug.log" .e2e/artifacts/ 2>/dev/null || true
85+
# The app may run under a different TMPDIR than this shell. Bounded
86+
# depth so this stays a lookup, not a filesystem walk, and errors
87+
# are swallowed so an unreadable sibling cannot fail the job.
88+
find /var/folders -maxdepth 4 -name termic-debug.log 2>/dev/null \
89+
| head -5 | while read -r f; do cp "$f" ".e2e/artifacts/$(basename "$(dirname "$f")")-termic-debug.log" 2>/dev/null || true; done
90+
ls -la .e2e/artifacts || true
91+
7492
- name: Upload artifacts on failure
7593
if: failure()
7694
uses: actions/upload-artifact@v4
7795
with:
7896
name: e2e-artifacts
7997
path: |
8098
.e2e/artifacts
81-
/var/folders/**/termic-debug.log
8299
if-no-files-found: ignore
83100
# Same trap as perf.yml: `.e2e/` is hidden, so every screenshot and
84101
# debug log from a failing run was silently dropped. With

docs/gotchas.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
- **Anything termic shells out to must get `shell_env::spawn_env()`, and `sh -lc` is NOT a substitute (GH #243, GH #181).** A GUI-launched `.app` inherits launchd's bare PATH (`/usr/bin:/bin:/usr/sbin:/sbin`), and `-l` only sources bash's profile chain, never `~/.zshrc` or `~/.zprofile` where Homebrew, nvm, volta and opencode's own installer put their PATH export. So `sh -lc "opencode session list"` is `command not found` from the shipped app and works from every `npm run tauri:dev` you test it in. This has now bitten three sites for the same reason: CLI detection (a freshly installed agent stayed invisible across relaunches, 8b03dbf), the find-in-files backend (#181), and `run_capture_command`, where the damage was invisible because an empty capture is indistinguishable from "the agent hasn't created a session yet" — opencode tabs silently never stored a session id and started a fresh conversation on every relaunch. Use `shell_env::spawn_env()` (PATH + the rc delta, one snapshot) for a spawn, `shell_env::resolved_path()` when you only need PATH, and pass `-c` rather than `-lc` once you have: re-sourcing the profile chain on top of an injected env only risks re-stripping it.
2727

2828
- **A sidebar folder stuck on "Loading…" forever (GH #159).** The tree's settle reload (`FileTree.tsx`, driven by `fsRevision`) re-read root + every expanded dir, dropped the ones whose read rejected, and then REPLACED the whole children map with what was left. `task_dir_list` rejects on a transient miss (`safe_task_path` canonicalizes, so a dir that is momentarily absent while a build or a generator rewrites it is ENOENT), which is exactly the moment the reload fires. The dir stayed in `expanded` with no listing, nothing in flight, and no retry, and the row rendered "Loading…" off `isOpen && !kids` alone, so a dropped listing looked identical to a slow read. Three separate paths could produce that state (the failed reload, a folder expanded WHILE a reload was in flight and clobbered by the replace, the same drop-on-failure shape in the mount effect), so the fix targets the state, not the trigger: reloads MERGE into the cache (`mergeReload` keeps a failed dir's old listing and anything expanded mid-flight, prunes only what was collapsed), and a reconcile effect enforces the invariant that every expanded dir has a listing, a read in flight, or a `failed` mark. A read gets one automatic retry, then the row says it failed and offers a retry rather than spinning. Logic in `lib/explorer/dirCache.ts` so it is unit-testable: there was never a repro, the argument is the state machine.
29+
- **A terminal that starts blank because its first bytes were emitted to nobody.** `pty_spawn` starts the reader and flusher threads before it returns, and the webview can only call `listen("pty://<id>")` after the spawn round trip resolves. Tauri events have no buffering: everything emitted in that gap is dropped silently. A CLI that paints a banner plus one OSC title at startup and then blocks on stdin (every agent fixture, and real agents at their first prompt) can lose BOTH, leaving an empty terminal and a tab with no live title, permanently, because nothing ever repaints. It reproduced as an Activity-spec flake on a loaded CI runner, where the spawn round trip lost its race with `bash`. The fix is an ack: `pty_attached` flips a flag under the reader/flusher buffer mutex, and the flusher holds its first emit (`wait_for_attach`) until then, or 3s, whichever comes first. **A new `pty_spawn` call site must send the ack**, see [docs/ipc.md](ipc.md).
2930
- **A file-tree error a user cannot report (GH #250).** The retry row above shipped saying only "Couldn't read this folder", which is exactly as diagnosable as the "Loading…" it replaced: the report that followed had a screenshot and nothing else. The tree now keeps the rejection message (`failed` is a `Map<rel, message>`, not a `Set`) and the row renders it through `lib/explorer/dirError.ts`: a headline ("Permission denied", "This folder links outside the task") plus the raw Rust error underneath and in the title. The Rust side names the path in every `safe_task_path` / `read_dir` error, so an ENOENT says WHICH path went missing, and a containment rejection says where the symlink pointed. **Anything that surfaces a `task_dir_list` rejection to the user goes through `explainDirError`** (the sidebar tree and `DirListingPane` both do), or the next report is a screenshot of a sentence again. Note the class the message makes visible: a folder that is a symlink out of the task lists as a directory but can never be read, so its retry is hopeless by construction, and it is the most likely thing behind a permanently failing folder in a real repo.
3031

3132

docs/ipc.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- **Tasks**: `task_create`/`task_create_multi` (async, spawn_blocking; the frontend never blocks on the returned promise — see "Non-blocking task creation" below) stream the WHOLE creation timeline — worktree add, file copy, port allocation, then the setup script — on one channel, `setup-output://<id>` (`{ line }`) + `setup-done://<id>` (`{ code, success }`), keyed by the client-generated task id the New Task dialog sends as `args.id` (so the frontend can subscribe before invoking). `task_archive`/`task_delete` (async, spawn_blocking), `task_open_repo`, `task_run_script_stream` + `task_stop_script` (PIDs in `RUNNING_SCRIPTS`, child has `process_group(0)` for clean SIGTERM tree-kill), `task_ensure_extra_ports` (GH #196: tops up frozen named ports from the current config, called by the frontend before every tab spawn).
66
- **PTYs**: `pty_spawn`/`pty_write`/`pty_resize`/`pty_kill`. Emits `pty://<id>` (`PtyChunk { data: Vec<u8> }`) and `pty-exit://<id>` (`PtyExit { code: Option<i32> }`). `SpawnArgs.role` (`{ task_id, kind: "agent"|"aux", is_default }`) is the CLI attach/logs identity and allocates the 256 KiB output ring; it is deliberately separate from `task_id`, which doubles as the sandbox trigger (the aux shell carries a role but never a task_id). `SpawnArgs.owner` (`{ task_id?, tab_id?, kind: "agent"|"shell"|"aux"|"run"|"setup"|"custom" }`) is a THIRD identity and a reporting field only: the Activity monitor groups rows by project → task → tab with it. Every spawn sets it, including the ones the other two must skip — a scratch shell pegging a core is exactly what the monitor exists to find. Nothing may branch on it.
7+
- **PTY attach ack**: `pty_attached { id }`, called by the webview the instant `listen("pty://<id>")` resolves. Tauri events are fire-and-forget, so everything the flusher emits before that listener exists is dropped with no trace, and the child starts writing the moment it is forked. Rust therefore holds a PTY's FIRST flush (and the reader's final drain, for a process that exits immediately) until the ack lands or a 3s grace expires. **Every caller of `pty_spawn` must send it** (`TerminalPane`, `AuxTerminal` today), or that terminal shows nothing until the grace runs out. The gate itself is `wait_for_attach` in `lib.rs`, unit-tested for all three exits.
78
- **Activity monitor**: `procmon_open_window` creates or re-focuses the `procmon` window; `procmon_start` → `ProcSnapshot { session, rows, sampleMs, webkitUnavailable }`, `procmon_sample { session }`, `procmon_stop { session }`, `procmon_signal { pid, signal }` (TERM/KILL/INT/STOP/CONT only, and only for a pid inside one of OUR PTY subtrees — the webview must not be an arbitrary `kill(2)` gadget). Sampling is PULL-based: there is no sampler thread, the Activity window's own interval is the clock, and `stop` leaves the module holding nothing. `session` is a guard, not decoration: a mismatched id errors so a reloaded webview restarts cleanly instead of reading another window's deltas. Only ever called from the Activity window (`activity.html`), never the main one. `mod procmon` in `lib.rs` is a 3-way `#[cfg(target_os = …)]` split resolving to `procmon.rs` (macOS, libproc/mach FFI, `ri_phys_footprint` for memory), `procmon_linux.rs` (`/proc`, plain text, `VmRSS` for memory — no phys_footprint equivalent, no WebKit-sidecar attribution), or `procmon_other.rs` (every other OS: a stub reporting "unsupported"). All three share row shapes + OS-agnostic logic (subtree walk, `cpu_ratio`, `label_for`, `signal_from_name`) from `procmon_common.rs`. The macOS FFI genuinely fails to LINK if it ends up compiled into a non-macOS build — this split exists because that shipped broken once (the Linux release build failing at link time with undefined libproc/mach symbols).
89
- **Scripts**: emit `script-output://<wsId>:<kind>` (`{ line }`) + `script-done://<wsId>:<kind>` (`{ code, success }`). `kind` in `setup`/`run`.
910
- **Settings/discovery**: `settings_load`/`settings_save`/`agents_save`/`discover_repos`/`detect_clis`/`list_monospace_fonts`/`list_font_families` (async + spawn_blocking + OnceLock cache — font-kit is 7s synchronous). `list_font_families` is the unfiltered family list (installed-ness checks); `list_monospace_fonts` is the `is_monospace()` subset (picker extras) — the latter trusts the post-table isFixedPitch bit, so it misses real monospace fonts with sloppy metadata.

e2e/specs/activity.e2e.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,26 @@ describe("Activity monitor", () => {
6969
// the BRIDGE carries a live title, so the main window has to actually have
7070
// one before the Activity window opens — otherwise the case is really
7171
// racing fake-agent.sh's first `set_title`, which is how it flaked in CI.
72+
// Report what the tab ACTUALLY had. A bare timeout here cannot tell
73+
// "the fixture never ran" from "it ran but the spawn dropped --name"
74+
// from "the title was lost before the terminal was listening", and this
75+
// only ever fails on CI, where guessing costs a round trip per attempt.
76+
let seen: unknown = null;
7277
await browser.waitUntil(
73-
async () => browser.execute((id, want) => {
74-
const tab = (window.__termic!.useApp.getState().tabs[id] ?? [])[0] as
75-
{ liveTitle?: string } | undefined;
76-
return !!tab?.liveTitle?.includes(want);
77-
}, taskId, "activity-mon"),
78-
{ timeout: 20_000, timeoutMsg: "agent never drove its OSC title to the task name" },
79-
);
78+
async () => {
79+
seen = await browser.execute((id) => {
80+
const t = (window.__termic!.useApp.getState().tabs[id] ?? [])[0] as
81+
{ liveTitle?: string; cli?: string; ptyId?: string } | undefined;
82+
return { liveTitle: t?.liveTitle ?? null, cli: t?.cli ?? null, pty: !!t?.ptyId };
83+
}, taskId);
84+
return !!(seen as { liveTitle: string | null }).liveTitle?.includes("activity-mon");
85+
},
86+
{ timeout: 20_000 },
87+
).catch(() => {
88+
throw new Error(
89+
`agent never drove its OSC title to the task name — tab was ${JSON.stringify(seen)}`,
90+
);
91+
});
8092
});
8193

8294
after(async () => {

0 commit comments

Comments
 (0)