From c7ffe9ee1b9d7ba97bf9d64b9e22b3dcf467f350 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 29 Aug 2026 18:43:29 +0000 Subject: [PATCH 1/2] fix(mirror): capture shell + launcher output to the session page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/merge` on app.moshcode.sh/sessions/ showed the echoed command line and the exit note with nothing in between. Not a stderr problem — a shell command's whole output was invisible. `/merge` is a shell-valued alias (`gh-prs-merge-all --apply`), so it expands to `!cmd` and lands in runShell, which spawned with `stdio: "inherit"`: those bytes go to the tty's own file descriptors and never pass through this process, so neither the teeOutput hook nor the pty capture ever saw them. Only engines and workflow tools had been taught to run under script(1); every other launcher had not. So the capture moves into one place — captureSpec in src/pty.mjs — and defaults its sink to the live mirror rather than waiting to be handed one. runShell (`!cmd`, /shell, every shell alias), /install, and runCmd (the upgrader, /plugin, /skill, /mcp) all reach the session page now, stdout and stderr alike, and a launcher written tomorrow gets it without knowing the mirror exists. Nothing changes for an unmirrored pit or a box with no usable script(1): the spec comes back untouched. Second fix, same symptom from the other end: POST /output stored `chunk.slice(0, 20000)` while publishing the full chunk to watchers, so a burst bigger than the row cap was on screen live and gone after a reload. The cap stays; the overflow becomes more rows in seq order. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SN7dw64sxjSEyyA2JErCPH --- apps/pwa/src/routes/sessions.mjs | 32 ++++- apps/pwa/test/sessions-output-seq.test.mjs | 29 ++++- src/engines.mjs | 53 +++----- src/mirror.mjs | 22 ++++ src/pty.mjs | 59 ++++++++- src/tui.mjs | 39 ++++-- test/mirror-shell-capture.test.mjs | 137 +++++++++++++++++++++ 7 files changed, 319 insertions(+), 52 deletions(-) create mode 100644 test/mirror-shell-capture.test.mjs diff --git a/apps/pwa/src/routes/sessions.mjs b/apps/pwa/src/routes/sessions.mjs index 3983b3c3..ea56a9b2 100644 --- a/apps/pwa/src/routes/sessions.mjs +++ b/apps/pwa/src/routes/sessions.mjs @@ -43,6 +43,26 @@ const LONG_POLL_MS = readLongPollMs(); // enough that a mis-paste of a whole file can't queue thousands of lines // against a prompt that runs them one at a time. const MAX_PASTED_LINES = 50; +// The most output one scrollback row may hold. The CLI flushes at ~16k, so a +// normal chunk is one row; a burst bigger than this becomes several rows in +// order rather than one truncated one. +export const MAX_CHUNK = 20000; + +/** + * One posted chunk as the rows it will be stored in, in order. + * + * Every byte survives: nothing is dropped for being past the cap, which is + * what the old `slice(0, MAX_CHUNK)` did — quietly, and only to the copy the + * scrollback kept, so it showed up as a session that read differently after a + * reload than it had live. + */ +export function splitChunk(text, max = MAX_CHUNK) { + const s = String(text ?? ""); + if (!s) return []; + const parts = []; + for (let i = 0; i < s.length; i += max) parts.push(s.slice(i, i + max)); + return parts; +} // Arrow keys pressed on the session page. The command queue carries opaque // strings, so a key rides it as a sentinel the CLI decodes — ESC leads it @@ -172,7 +192,13 @@ sessionsRouter.post("/api/sessions/:id/output", cliAuth, async (req, res) => { // at the old column and stays wrong in the scrollback for good. if (resized) publish(session.id, { type: "size", cols, rows }); - if (chunk) { + // Split rather than truncate. A row is capped so one flush can't put an + // unbounded string in the table, but a chunk that overran the cap used to be + // stored short while the live watchers got all of it — so the same session + // read two different ways (watching, or reloading the page) disagreed, and + // the bytes past the cap were gone for good. A build that prints a wall of + // output in one 150ms window is exactly when that happens. + for (const part of splitChunk(chunk)) { // seq is per-session and monotonic so a reconnecting browser can ask for // "everything after N" instead of replaying the whole scrollback. // @@ -187,14 +213,14 @@ sessionsRouter.post("/api/sessions/:id/output", cliAuth, async (req, res) => { `INSERT INTO session_output (session_id,seq,chunk,created_at) SELECT ?, COALESCE(MAX(seq), 0) + 1, ?, ? FROM session_output WHERE session_id = ? RETURNING seq`, - [session.id, chunk.slice(0, 20000), now, session.id] + [session.id, part, now, session.id] ); const seq = Number(inserted.seq); await run( `DELETE FROM session_output WHERE session_id = ? AND seq <= ?`, [session.id, seq - SCROLLBACK] ); - publish(session.id, { type: "out", seq, chunk }); + publish(session.id, { type: "out", seq, chunk: part }); } if (engine !== session.engine) publish(session.id, { type: "engine", engine }); res.json({ ok: true }); diff --git a/apps/pwa/test/sessions-output-seq.test.mjs b/apps/pwa/test/sessions-output-seq.test.mjs index e3152e6d..89b5dd3f 100644 --- a/apps/pwa/test/sessions-output-seq.test.mjs +++ b/apps/pwa/test/sessions-output-seq.test.mjs @@ -216,20 +216,41 @@ test("sessions: an empty chunk writes no output row", skip, async () => { assert.deepEqual(await rowsFor(sid), [], "no row for an empty chunk"); }); -test("sessions: a long chunk is still truncated to the stored limit", skip, async () => { +test("sessions: an ordinary chunk is one row, at the stored limit", skip, async () => { const { one, rowsFor } = await app(); - const reg = await one("/api/sessions", { name: "truncate" }); + const reg = await one("/api/sessions", { name: "at-cap" }); const sid = reg.body.id; - await one(`/api/sessions/${sid}/output`, { chunk: "x".repeat(20050) }); + await one(`/api/sessions/${sid}/output`, { chunk: "x".repeat(20000) }); const rows = await rowsFor(sid); assert.equal(rows.length, 1); - assert.equal(rows[0].chunk.length, 20000, "chunk is capped at 20000 chars"); + assert.equal(rows[0].chunk.length, 20000, "a row is still capped at 20000 chars"); assert.equal(Number(rows[0].seq), 1); }); +test("sessions: a chunk past the row limit is split, not truncated", skip, async () => { + const { one, rowsFor } = await app(); + + // The row cap keeps one flush from writing an unbounded string. It used to be + // applied with slice(), so the overflow was published live to every watcher + // and then thrown away — the same session read one way while you watched it + // and another way after a reload, with no sign anything was missing. A build + // that dumps a wall of output inside one 150ms flush is exactly that case. + const chunk = `${"x".repeat(20000)}TAIL-MUST-SURVIVE\n`; + const reg = await one("/api/sessions", { name: "oversized" }); + const sid = reg.body.id; + + await one(`/api/sessions/${sid}/output`, { chunk }); + + const rows = await rowsFor(sid); + assert.equal(rows.length, 2, "the overflow becomes another row"); + assert.ok(rows.every((r) => r.chunk.length <= 20000), "each row still respects the cap"); + assert.equal(rows.map((r) => r.chunk).join(""), chunk, "every byte posted must be stored"); + assert.deepEqual(rows.map((r) => Number(r.seq)), [1, 2], "and in order, so a replay reassembles it"); +}); + test("sessions: a foreign key still cannot append output", skip, async () => { const { one, two, rowsFor } = await app(); diff --git a/src/engines.mjs b/src/engines.mjs index 5e393786..3872e1b4 100644 --- a/src/engines.mjs +++ b/src/engines.mjs @@ -32,11 +32,11 @@ // a session that starts fresh is a small disappointment, and one that starts // with a flag the engine does not have is a crash. import { spawn } from "node:child_process"; -import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; -import { homedir, tmpdir } from "node:os"; +import { existsSync, readFileSync, statSync } from "node:fs"; +import { homedir } from "node:os"; import path from "node:path"; -import { followFile, ptyEnabled, ptySpec, scriptFlavor, stripScriptBanner } from "./pty.mjs"; +import { captureSpec } from "./pty.mjs"; export const ENGINES = { opencode: { @@ -439,21 +439,29 @@ export function runCmd(cmd, args = [], { capture = false } = {}) { let child; const spec = spawnSpec(cmd, args); const stdio = capture ? ["inherit", "pipe", "pipe"] : "inherit"; - try { child = spawn(spec.cmd, spec.args, { stdio }); } - catch (e) { resolve({ ok: false, error: e }); return; } + // The `capture` branch already reaches a watching browser: it re-writes + // every byte through this process's own stdout/stderr, which the mirror + // tees. The inherited branch does not — those bytes go to the tty and + // nowhere else — so it goes under a pty when a mirror is live. This is what + // an upgrade, a plugin install and an `mcp add` all run through, and all + // three used to be a rule, a blank stretch, and a result line. + const launch = capture ? { ...spec, stop: () => {} } : captureSpec(spec); + const finish = (result) => { try { launch.stop(); } catch { /* already drained */ } resolve(result); }; + try { child = spawn(launch.cmd, launch.args, { stdio }); } + catch (e) { finish({ ok: false, error: e }); return; } let output = ""; if (capture) { for (const [stream, sink] of [[child.stdout, process.stdout], [child.stderr, process.stderr]]) { stream?.on("data", (chunk) => { output += chunk.toString(); sink.write(chunk); }); } } - child.on("error", (e) => resolve({ ok: false, error: e, output })); + child.on("error", (e) => finish({ ok: false, error: e, output })); // "exit" fires as soon as the process is gone, which with pipes can leave // the last chunk still queued — the one line we are trying to read. "close" // waits for the streams too. With stdio inherited there are no streams, so // the two are the same moment and existing callers are unaffected; the // distinction is kept explicit so neither branch changes by accident. - child.on(capture ? "close" : "exit", (code, signal) => resolve({ ok: true, code, signal, output })); + child.on(capture ? "close" : "exit", (code, signal) => finish({ ok: true, code, signal, output })); }); } @@ -501,35 +509,8 @@ export function openPassthrough(target, args = [], { onOutput } = {}) { // the child the tty's own file descriptors, so none of its bytes ever pass // through this process. See src/pty.mjs for why this is script(1) and not // a pipe or node-pty. - let transcript = null; - let workDir = null; - let stopFollow = null; - let launch = { ...spec, stdio: "inherit" }; - if (ptyEnabled(onOutput)) { - try { - workDir = mkdtempSync(path.join(tmpdir(), "moshcode-pty-")); - transcript = path.join(workDir, "transcript"); - writeFileSync(transcript, ""); - const wrapped = ptySpec(spec.cmd, spec.args, transcript, scriptFlavor()); - if (wrapped) { - launch = { ...wrapped, stdio: "inherit" }; - let first = true; - stopFollow = followFile(transcript, (chunk) => { - const clean = stripScriptBanner(chunk, first); - first = false; - if (clean) onOutput(clean); - }); - } - } catch { - // Capture is a nicety; never let it stop the session from opening. - transcript = null; - } - } - - const cleanup = () => { - try { stopFollow?.(); } catch { /* nothing left to drain */ } - if (workDir) { try { rmSync(workDir, { recursive: true, force: true }); } catch { /* temp dir */ } } - }; + const launch = captureSpec(spec, onOutput); + const cleanup = () => launch.stop(); let child; try { child = spawn(launch.cmd, launch.args, { stdio: "inherit", env }); } diff --git a/src/mirror.mjs b/src/mirror.mjs index 793808a7..f0a5c09b 100644 --- a/src/mirror.mjs +++ b/src/mirror.mjs @@ -56,6 +56,28 @@ export function pressKey(name, rl = null, stdin = process.stdin) { const FLUSH_MS = 150; // batch writes so a busy render is one request, not fifty const MAX_BUFFER = 16000; // flush early once a batch gets big +// Where a child process's output should be copied while a mirror is watching. +// +// Module-level rather than threaded through every call, because "is anyone +// watching this pit" is one fact about the process and the launchers that need +// it are scattered: the shell, the installers, the upgrader, the plugin/skill/ +// MCP hand-offs. Passing it down by hand is what left most of them writing +// straight to the tty with the session page showing nothing — each new launcher +// had to remember, and none of them did. src/pty.mjs reads this as its default, +// so capture is what a launcher gets for free and opting out is the deliberate +// act. +let activeSink = null; + +/** Point child capture at this mirror (or null when the pit stops mirroring). */ +export function setActiveSink(sink) { + activeSink = typeof sink === "function" ? sink : null; +} + +/** The sink a child's output should be copied to, or null when unmirrored. */ +export function activeChildSink() { + return activeSink; +} + export function createMirror({ version = "", cwd = process.cwd(), diff --git a/src/pty.mjs b/src/pty.mjs index ff2a8321..fc6b161d 100644 --- a/src/pty.mjs +++ b/src/pty.mjs @@ -19,8 +19,11 @@ // `script` disagree on both flag names and argument order, and anything we // cannot positively identify falls back to today's plain `inherit`. import { spawnSync } from "node:child_process"; -import { closeSync, existsSync, openSync, readSync, statSync } from "node:fs"; +import { closeSync, existsSync, mkdtempSync, openSync, readSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; import { StringDecoder } from "node:string_decoder"; +import { activeChildSink } from "./mirror.mjs"; /** * POSIX single-quote escaping, for argv that has to survive being flattened @@ -178,3 +181,57 @@ export function ptyEnabled(sink, flavor = scriptFlavor()) { if (process.env.MOSHCODE_MIRROR_PTY === "0") return false; return Boolean(flavor); } + +/** + * Wrap a spawn spec so a copy of everything the child prints reaches `onOutput` + * while the child still owns the real terminal. + * + * The whole capture dance in one place — temp transcript, the flavour-specific + * `script` argv, the follower, the banner strip, the cleanup — because every + * launcher in the pit needs it, and each one growing its own copy is how a + * shell command ended up invisible in the mirror while `/agents claude` was + * captured: both spawn `inherit`, and only one of them had been taught this. + * + * `onOutput` defaults to whatever the live mirror is (src/mirror.mjs), so a + * launcher gets capture without having to know the mirror exists — the reverse + * of how this started, where each launcher had to be taught separately and only + * two ever were. Pass `null` to opt a launch out. + * + * Returns `{ cmd, args, stop }`. With nothing watching, or on a box with no + * `script(1)` we can drive, `cmd`/`args` come back exactly as passed in and + * `stop` is a no-op — the caller spawns what it always spawned. `stop()` must + * be called once the child exits: it drains the tail of the transcript (the + * last lines of a command are usually the ones you were waiting for) and + * removes the temp dir. + */ +export function captureSpec({ cmd, args = [] }, onOutput = activeChildSink(), { flavor = scriptFlavor() } = {}) { + const plain = { cmd, args, stop: () => {} }; + if (!ptyEnabled(onOutput, flavor)) return plain; + let workDir = null; + try { + workDir = mkdtempSync(path.join(tmpdir(), "moshcode-pty-")); + const transcript = path.join(workDir, "transcript"); + writeFileSync(transcript, ""); + const wrapped = ptySpec(cmd, args, transcript, flavor); + if (!wrapped) throw new Error("no script(1) spec for this flavour"); + let first = true; + const stopFollow = followFile(transcript, (chunk) => { + const clean = stripScriptBanner(chunk, first); + first = false; + if (clean) onOutput(clean); + }); + const dir = workDir; + return { + cmd: wrapped.cmd, + args: wrapped.args, + stop() { + try { stopFollow(); } catch { /* nothing left to drain */ } + try { rmSync(dir, { recursive: true, force: true }); } catch { /* temp dir */ } + }, + }; + } catch { + // Capture is a nicety; never let it stop a command from running. + if (workDir) { try { rmSync(workDir, { recursive: true, force: true }); } catch { /* temp dir */ } } + return plain; + } +} diff --git a/src/tui.mjs b/src/tui.mjs index 55618983..94e36204 100644 --- a/src/tui.mjs +++ b/src/tui.mjs @@ -18,7 +18,7 @@ import { createPrd, listPrds, authoringPrompt } from "./prd.mjs"; import { loginAuto, whoami, logout } from "./auth.mjs"; import { startAutoSync } from "./autosync.mjs"; import { loadCommand, saveCommand } from "./settings-sync.mjs"; -import { createMirror, pressKey, teeOutput } from "./mirror.mjs"; +import { createMirror, pressKey, setActiveSink, teeOutput } from "./mirror.mjs"; import { fetchMotdAd } from "./ads.mjs"; import { runScript } from "./runtime.mjs"; import { moshVocabulary } from "./commands.mjs"; @@ -28,6 +28,7 @@ import { cryptoCommand } from "./crypto.mjs"; import { gamesCommand } from "./games.mjs"; import { canOpenBrowser, openBrowser } from "./open-url.mjs"; import { shellInvocation } from "./shell.mjs"; +import { captureSpec } from "./pty.mjs"; import { needsRootHere, primeEscalation } from "./escalate.mjs"; import { banner, hr, acid, ash, bone, dim, ok, err, warn, info, moshcodeVersion } from "./ui.mjs"; import { CORE_CLI_COMMAND_NAMES } from "./cli-schema.mjs"; @@ -678,14 +679,23 @@ async function openWorkflowTool(key, tool, args) { // command string → `$SHELL +m -ic ""` (one-off). Interactive so the command // can see the aliases and functions in ~/.zshrc — see src/shell.mjs for why // that is not optional. Resolves { ok, code, signal }. -function runShell(rawCmd) { +// +// Captured through a pty when the mirror is watching, for the same reason an +// engine is: a shell command is where most of what a pit does actually happens +// — `!cmd`, /shell, and every shell-valued /alias land here — and with plain +// `inherit` none of its bytes, on stdout or stderr, ever pass through this +// process. The session page was showing the echoed command line and the exit +// note with nothing in between. +export function runShell(rawCmd, { onOutput } = {}) { return new Promise((resolve) => { const { shell, args } = shellInvocation(rawCmd); + const launch = captureSpec({ cmd: shell, args }, onOutput); + const done = (result) => { try { launch.stop(); } catch { /* already drained */ } resolve(result); }; let child; - try { child = spawn(shell, args, { stdio: "inherit" }); } - catch (e) { resolve({ ok: false, error: e }); return; } - child.on("error", (e) => resolve({ ok: false, error: e })); - child.on("exit", (code, signal) => resolve({ ok: true, code, signal })); + try { child = spawn(launch.cmd, launch.args, { stdio: "inherit" }); } + catch (e) { done({ ok: false, error: e }); return; } + child.on("error", (e) => done({ ok: false, error: e })); + child.on("exit", (code, signal) => done({ ok: true, code, signal })); }); } @@ -700,7 +710,7 @@ async function openShell(rawCmd) { ? `${bone(shellName)} ${ash(flags)} ${ash(rawCmd)}` : `dropping to ${bone(shellName)} — ${ash("`exit` or Ctrl-D brings you back to the pit")}`)); console.log(hr()); - const r = await runShell(rawCmd); + const r = await runShell(rawCmd, { onOutput: childSink() }); console.log(hr()); if (!r.ok) { console.log(err(`couldn't start shell: ${r.error?.message || r.error}`)); @@ -720,14 +730,22 @@ function installTarget(key) { // something the installer's output scrolled into view. if (needsRootHere(target)) primeEscalation({ what: key, out: (s) => console.log(info(s.replace(/^· /, ""))) }); console.log(hr()); - const child = spawn(target.install.cmd, target.install.args, { stdio: "inherit" }); + // Installers are long, chatty, and the thing you most want to read from a + // phone — so they go through the mirror's pty like everything else. + const launch = captureSpec( + { cmd: target.install.cmd, args: target.install.args }, + childSink(), + ); + const child = spawn(launch.cmd, launch.args, { stdio: "inherit" }); child.on("error", (e) => { + launch.stop(); console.log(hr()); console.log(err(`install failed: ${e.message}`)); if (e.code === "ENOENT" && target.installHelp) console.log(info(target.installHelp)); resolve(); }); child.on("exit", (code) => { + launch.stop(); console.log(hr()); if (code !== 0) { console.log(err(`install exited ${code}`)); return resolve(); } console.log(ok(`${key} installed. 🤘`)); @@ -1292,6 +1310,10 @@ async function startMirror() { if (!started) return noop; activeMirror = mirror; + // Every launcher that spawns a child reads this rather than being handed a + // sink, so a command run from the pit is captured whether or not whoever + // wrote that launcher knew the mirror existed. + setActiveSink((chunk) => activeMirror?.write(chunk)); const restoreTee = teeOutput((chunk) => mirror.write(chunk)); // Commands arrive whenever; the prompt is only ready between engine @@ -1326,6 +1348,7 @@ async function startMirror() { async function stopMirror(restoreTee) { const mirror = activeMirror; activeMirror = null; + setActiveSink(null); try { restoreTee?.(); } catch { /* noop */ } try { await mirror?.stop(); } catch { /* best effort */ } } diff --git a/test/mirror-shell-capture.test.mjs b/test/mirror-shell-capture.test.mjs new file mode 100644 index 00000000..b1e836c9 --- /dev/null +++ b/test/mirror-shell-capture.test.mjs @@ -0,0 +1,137 @@ +// A pit spends most of its time running shell commands — `!cmd`, /shell, and +// every shell-valued /alias (`/merge` → `gh-prs-merge-all --apply`) land in +// runShell. Those spawned with `stdio: "inherit"`, so the session page at +// app.moshcode.sh/sessions/ showed the echoed command line and the exit +// note with nothing at all in between: not the command's stdout, and not its +// stderr, which is where a merge sweep reports what it skipped and why. +// +// These pin the capture end-to-end through a real script(1), because the parts +// were already unit-tested individually while the path that matters was not +// wired to them at all. +import assert from "node:assert/strict"; +import test from "node:test"; + +import { captureSpec, scriptFlavor } from "../src/pty.mjs"; +import { activeChildSink, createMirror, setActiveSink } from "../src/mirror.mjs"; +import { runCmd } from "../src/engines.mjs"; +import { runShell } from "../src/tui.mjs"; + +const CAPTURABLE = Boolean(scriptFlavor()); + +test("captureSpec hands back the launch untouched when nothing is watching", () => { + const spec = captureSpec({ cmd: "gh", args: ["pr", "list"] }, undefined); + assert.equal(spec.cmd, "gh"); + assert.deepEqual(spec.args, ["pr", "list"]); + // stop() is always callable, so no caller has to know whether it captured. + assert.doesNotThrow(() => spec.stop()); +}); + +test("captureSpec falls back rather than failing on a box with no script(1)", () => { + const spec = captureSpec({ cmd: "gh", args: ["pr", "list"] }, () => {}, { flavor: null }); + assert.equal(spec.cmd, "gh"); + assert.deepEqual(spec.args, ["pr", "list"]); +}); + +test("captureSpec wraps the launch in script(1) when a sink is attached", { skip: !CAPTURABLE }, () => { + const spec = captureSpec({ cmd: "gh", args: ["pr", "list"] }, () => {}); + assert.equal(spec.cmd, "script"); + assert.ok(spec.args.includes("gh") || spec.args.some((a) => a.includes("'gh'")), + "the real command has to survive into the script argv"); + spec.stop(); +}); + +test("a shell command's stdout AND stderr both reach the mirror", { skip: !CAPTURABLE }, async () => { + let seen = ""; + // stderr first, so a run that only captured stdout can't pass by accident of + // ordering — both have to be there. + const r = await runShell( + "printf 'to-stderr\\n' >&2; printf 'to-stdout\\n'", + { onOutput: (chunk) => { seen += chunk; } }, + ); + assert.equal(r.ok, true); + assert.equal(r.code, 0); + assert.match(seen, /to-stdout/, "stdout was the half that was never missing"); + assert.match(seen, /to-stderr/, "stderr is where a merge sweep says what it skipped"); +}); + +test("a failing shell command still reports its exit code through the pty", { skip: !CAPTURABLE }, async () => { + // script -e is what forwards the child's status; without it every command + // would read as a success in the pit, which is worse than no capture at all. + const r = await runShell("printf 'nope\\n' >&2; exit 3", { onOutput: () => {} }); + assert.equal(r.ok, true); + assert.equal(r.code, 3); +}); + +test("an unmirrored shell command runs exactly as it did before", async () => { + const r = await runShell("exit 0"); + assert.equal(r.ok, true); + assert.equal(r.code, 0); +}); + +test("the active sink is what an unaware launcher captures through", { skip: !CAPTURABLE }, async () => { + // A launcher should not have to know the mirror exists. runShell is handed a + // sink by the pit; runCmd — the upgrader, /plugin, /skill, /mcp — is not, and + // reads the live mirror off the module instead. That is deliberate: every + // launcher taught by hand is one more that can be written without being. + let seen = ""; + setActiveSink((chunk) => { seen += chunk; }); + try { + const r = await runCmd("sh", ["-c", "printf 'from-a-launcher\\n' >&2"]); + assert.equal(r.ok, true); + assert.match(seen, /from-a-launcher/); + } finally { + setActiveSink(null); + } +}); + +test("with no mirror running, a launcher spawns exactly what it always did", () => { + setActiveSink(null); + const spec = captureSpec({ cmd: "gh", args: ["pr", "list"] }); + assert.equal(spec.cmd, "gh", "an unmirrored pit stays on the untouched inherit path"); + assert.deepEqual(spec.args, ["pr", "list"]); +}); + +test("a shell command's stderr survives the whole chain to the session POST", { skip: !CAPTURABLE }, async () => { + // The parts each work; this is the chain the operator actually watches — + // runShell → the pty → the mirror's sink → a batched POST to + // /api/sessions//output. Anything that reaches here reaches the page. + const posted = []; + const fetchImpl = async (url, options = {}) => { + const { pathname } = new URL(url); + if (pathname === "/api/sessions") { + return new Response(JSON.stringify({ id: "session-1" }), { headers: { "content-type": "application/json" } }); + } + if (pathname.endsWith("/commands")) { + // Park forever; stop() aborts it. + return new Promise((_resolve, reject) => { + options.signal.addEventListener("abort", () => reject(options.signal.reason), { once: true }); + }); + } + posted.push(JSON.parse(options.body).chunk); + return new Response(JSON.stringify({ ok: true }), { headers: { "content-type": "application/json" } }); + }; + + const mirror = createMirror({ + credentials: { api: "https://app.example.test", token: "mck_test" }, + fetchImpl, + }); + assert.equal(await mirror.start(), true); + setActiveSink((chunk) => mirror.write(chunk)); + try { + await runShell("printf 'merge-skipped-because\\n' >&2"); + await mirror.stop(); // flushes what is still pending + } finally { + setActiveSink(null); + } + + assert.match(posted.join(""), /merge-skipped-because/, + "stderr from a pit shell command has to reach the session page"); +}); + +test("setActiveSink refuses anything that isn't callable", () => { + // A bad value would throw inside followFile's poll — on a timer, with nobody + // to catch it — so it is turned away at the door instead. + setActiveSink("not a function"); + assert.equal(activeChildSink(), null); + setActiveSink(null); +}); From 669a18badd5e6257fabcc6a59f57aa5ef0afd26a Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 29 Aug 2026 19:01:07 +0000 Subject: [PATCH 2/2] fix(mirror): capture the blocking launchers too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified against the build actually installed (v0.74.0, ~/.moshcode/pkg) by pointing a real mirrored pit at a stand-in app and recording what it posts: a shell command's stdout AND stderr both reach the terminal and neither reaches the session page. Not stderr-specific, and not /merge -specific — every shell-valued alias went the same way. The async launchers were the first pass. These are the spawnSync ones, equally invisible and equally pit-reachable: moshscript's shell() (so `/run`), /billing handing an invoice to coinpay, and /payments connect. A blocking spawn holds the event loop, so the follower's poll never runs and the output arrives in the drain stop() does — batched rather than live, which is still the difference between reading it from a phone and not. Left uncaptured on purpose: escalate.mjs (a sudo password prompt is not something to route through a web page) and the tmux hand-offs, which hand over the terminal wholesale. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SN7dw64sxjSEyyA2JErCPH --- src/billing.mjs | 8 +++++++- src/commands.mjs | 11 ++++++++++- src/payments.mjs | 6 +++++- test/mirror-shell-capture.test.mjs | 19 +++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/billing.mjs b/src/billing.mjs index ca110972..d656cc77 100644 --- a/src/billing.mjs +++ b/src/billing.mjs @@ -19,6 +19,7 @@ import { spawnSync } from "node:child_process"; import { loadBusiness, loadTimers, newId, updateBusiness, updateTimers } from "./business-store.mjs"; import { clientLabel, parseFields, resolveClient } from "./clients.mjs"; +import { captureSpec } from "./pty.mjs"; import { GATEWAYS, defaultGateway, gatewayState } from "./payments.mjs"; import { chargeFor, describeRate, formatMoney, isDollarPegged, isFiat, rateFor } from "./rates.mjs"; import { humanDuration, selectEntries, windowFrom } from "./timer.mjs"; @@ -289,7 +290,12 @@ function handOff(record, invoice, business, fields, write, run) { return 0; } - const result = run("coinpay", args, { stdio: "inherit" }); + // Mirrored like every other hand-off: sending an invoice is exactly the kind + // of thing you want to read back from the session page afterwards. + const launch = captureSpec({ cmd: "coinpay", args }); + let result; + try { result = run(launch.cmd, launch.args, { stdio: "inherit" }); } + finally { launch.stop(); } if (result?.error) { write(err(String(result.error.message || result.error))); return 1; } if (result?.status) { write(err(`coinpay exited ${result.status} — invoice ${record.id} is still a local draft`)); return result.status; } updateBusiness((data) => { diff --git a/src/commands.mjs b/src/commands.mjs index 79fa86a9..dc57099b 100644 --- a/src/commands.mjs +++ b/src/commands.mjs @@ -22,6 +22,7 @@ import { capture, killSession, remoteStatus, sendPrompt } from "./herd.mjs"; import { herdStart, isRemoteMember, roster, waitForMany, waitMember } from "./herd-cli.mjs"; import { endTask, findTask, readTasks, startTask } from "./herd-tasks.mjs"; import { shellInvocation } from "./shell.mjs"; +import { captureSpec } from "./pty.mjs"; import { identity, loginAuto, logout as forgetCreds } from "./auth.mjs"; import { expandAlias, getAlias, loadAliases, removeAlias, setAlias } from "./aliases.mjs"; import { CORE_CLI_COMMAND_NAMES, PIT_COMMANDS } from "./cli-schema.mjs"; @@ -116,7 +117,15 @@ const SHELL = { // has the reasoning, including why a headless run stays non-interactive. const { shell: sh, args: shArgs } = shellInvocation(cmd); ctx.out(` ▶ shell: ${cmd}`); - const res = spawnSync(sh, shArgs, { stdio: "inherit" }); + // Captured for the session mirror like the pit's own `!cmd`. A blocking + // spawn holds the event loop, so the follower's poll never runs and the + // whole command arrives in the drain stop() does — batched rather than + // live, which is still the difference between reading it from a phone and + // not. + const launch = captureSpec({ cmd: sh, args: shArgs }); + let res; + try { res = spawnSync(launch.cmd, launch.args, { stdio: "inherit" }); } + finally { launch.stop(); } if (res.error) throw res.error; const code = res.status ?? 1; if (code !== 0) { diff --git a/src/payments.mjs b/src/payments.mjs index 5d6bf271..1ceaeca3 100644 --- a/src/payments.mjs +++ b/src/payments.mjs @@ -22,6 +22,7 @@ // `/payments connect stripe` records a *reference* — vault and key name — and // says out loud where the secret should go. import { spawnSync } from "node:child_process"; +import { captureSpec } from "./pty.mjs"; import { loadBusiness, updateBusiness } from "./business-store.mjs"; import { parseFields } from "./clients.mjs"; @@ -178,7 +179,10 @@ function connectGateway(args, write, run) { return 1; } write(info(`handing you to ${bone(gateway.bin)} — it owns its own session`)); - const result = run(gateway.bin, gateway.connect, { stdio: "inherit" }); + const launch = captureSpec({ cmd: gateway.bin, args: gateway.connect }); + let result; + try { result = run(launch.cmd, launch.args, { stdio: "inherit" }); } + finally { launch.stop(); } if (result?.error) { write(err(String(result.error.message || result.error))); return 1; } if (result?.status) { write(err(`${gateway.bin} ${gateway.connect.join(" ")} exited ${result.status} — nothing recorded`)); diff --git a/test/mirror-shell-capture.test.mjs b/test/mirror-shell-capture.test.mjs index b1e836c9..a64bdf66 100644 --- a/test/mirror-shell-capture.test.mjs +++ b/test/mirror-shell-capture.test.mjs @@ -9,6 +9,7 @@ // were already unit-tested individually while the path that matters was not // wired to them at all. import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; import test from "node:test"; import { captureSpec, scriptFlavor } from "../src/pty.mjs"; @@ -128,6 +129,24 @@ test("a shell command's stderr survives the whole chain to the session POST", { "stderr from a pit shell command has to reach the session page"); }); +test("a blocking spawn still delivers its output, on stop", { skip: !CAPTURABLE }, async () => { + // moshscript's shell() (so `/run`), /billing and /payments connect are all + // spawnSync: the event loop is held for the whole command, so the follower's + // poll never gets to run and everything arrives in the drain stop() does. + // Batched rather than live — but the alternative here was nothing at all. + let seen = ""; + setActiveSink((chunk) => { seen += chunk; }); + try { + const launch = captureSpec({ cmd: "sh", args: ["-c", "printf 'SYNC-STDERR\\n' >&2"] }); + spawnSync(launch.cmd, launch.args, { stdio: "ignore" }); + assert.equal(seen, "", "nothing can arrive while the loop is blocked"); + launch.stop(); + assert.match(seen, /SYNC-STDERR/, "and all of it arrives on the drain"); + } finally { + setActiveSink(null); + } +}); + test("setActiveSink refuses anything that isn't callable", () => { // A bad value would throw inside followFile's poll — on a timer, with nobody // to catch it — so it is turned away at the door instead.