From 8b8582f231fdccb379dae5652f46fe17cbe324e4 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Thu, 10 Sep 2026 17:07:23 +0700 Subject: [PATCH] fix(runs): persist tmux-backed runs so a resumed session shows its own info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dashboard_runs.mode` is NOT NULL and `recordRun` swallows insert errors by design, so pty-run.js passing `mode: null` meant NO tmux-backed run was ever persisted — the newest row in a live database dated from before pty-run.js replaced run-spawner.js. `publicRun` reads cwd, model, prompt preview, started_at and session_id off that row and the handle itself is recomputed from tmux, so the runs modal rendered a live row with no cwd, no model, no prompt and a start date of Jan 1 1970. The same missing row is why a resumed session kept its separate `external` badge: the client dedupes an external session against the session ids carried by run rows, and no run row ever carried one. - pty-run.js records `mode: "interactive"` — every run here is one pane. - dashboard-runs.js still never fails a live run on a db error, but warns instead of swallowing silently; that silence hid this for six weeks. - RunHistory renders `startedAt: 0` as "-" rather than dating a row with no known start time to the epoch. Also closes the other half of the same lie: adopting a pane that is already running an agent now throws ERUNLIVE (409 from both the lane start action and POST /api/run) when the request carried a prompt or a resumeSessionId. That argv cannot be typed over a live agent, so answering 200 reported a resume that never spawned. A start asking for nothing but a pane still adopts. --- ARCHITECTURE.md | 2 +- client/src/components/run/RunHistory.tsx | 19 ++-- .../run/__tests__/RunHistory.test.tsx | 8 ++ docs/API.md | 2 +- server/README.md | 7 +- server/__tests__/pty-run.test.js | 87 +++++++++++++++++++ server/lib/dashboard-runs.js | 8 +- server/lib/pty-run.js | 20 ++++- server/routes/lanes.js | 4 +- server/routes/run.js | 4 + 10 files changed, 146 insertions(+), 15 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 7dd42910..3ca65a39 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -354,7 +354,7 @@ graph TD | `lib/codex-cli.js` | Shells out to the `codex` CLI (argv array, never a shell string) for the three surfaces with no on-disk reader: MCP servers (`mcp list\|add\|remove`), plugins (`plugin list\|add\|remove`), and marketplaces (`marketplace list\|add\|remove`). `runCodex()` parses stdout as JSON when possible; empty stdout on a zero-exit is a normal empty list, not a parse failure. A non-zero exit is returned as a structured failure (surfaced by the route as 422, never a 500) with `stdout`/`stderr` preserved | | `routes/codex-config.js` | HTTP surface for the Codex Config panel. Read endpoints for every surface (skills, agents, rules, memory, config, mcp, plugins, marketplaces, file, overview) plus mutation endpoints (`PUT /file`, `DELETE /file` delegating to `codex-mutate.js`; `POST /mcp/{add,remove}`, `/plugins/action`, `/marketplaces/{add,action}` delegating to `codex-cli.js`), plus `GET /backups`. Every successful mutation broadcasts `codex_config_changed` over the WebSocket so an open Codex settings tab refetches without polling. All errors return structured `{error: {code, message}}` shapes | | `lib/tmux.js` | Wrapper around tmux CLI for session management. `createSession(sessionName, cwd)` creates a new tmux session in the specified working directory. `sendCommand(sessionName, command)` sends a command into the session. `killSession(sessionName)` terminates the session. `listSessions()` returns all active sessions. Session management is the foundation for the PTY transport layer | -| `lib/pty-run.js` | PTY lifecycle for tmux-backed runs. Manages one tmux session per lane, named `agenthub-lane-`. Exports `startRun()` to create/attach a session and return a `runId` opaque handle; internally uses tmux to manage the pseudoterminal. Spawned `claude` processes run inside the session and fire the dashboard's hooks like any other CLI session, so they show up in `/api/sessions`, the analytics, the Kanban board, and the Workflows page automatically. The PTY frames (terminal input/output deltas) are streamed to the client over `/ws-pty/:runId` (see `server/websocket.js`) at binary frame granularity rather than as JSON envelopes; the client's xterm.js terminal widget renders these raw PTY updates live | +| `lib/pty-run.js` | PTY lifecycle for tmux-backed runs. Manages one tmux session per lane, named `agenthub-lane-`. Exports `startRun()` to create/attach a session and return a `runId` opaque handle; internally uses tmux to manage the pseudoterminal. Spawned `claude` processes run inside the session and fire the dashboard's hooks like any other CLI session, so they show up in `/api/sessions`, the analytics, the Kanban board, and the Workflows page automatically. The PTY frames (terminal input/output deltas) are streamed to the client over `/ws-pty/:runId` (see `server/websocket.js`) at binary frame granularity rather than as JSON envelopes; the client's xterm.js terminal widget renders these raw PTY updates live. Every spawn also mirrors itself into `dashboard_runs` (cwd, model, prompt preview, `resume_session_id`, lane id, start time) — that row is the ONLY source `publicRun` has for those fields, since the handle itself is recomputed from tmux on every read, so a dropped insert renders as a live run with no cwd, model or start date. `mode` is NOT NULL in that table and `recordRun` swallows insert errors by design, which is why this module passes a literal `"interactive"` rather than null. **Create-or-adopt has one refusal:** a pane already running an agent is adopted silently only when the request asked for nothing but a pane — a `resumeSessionId` or an initial prompt cannot be typed over a live agent, so it throws `ERUNLIVE` (409) rather than reporting a resume that never spawned | | `lib/pty-attach.js` | Client-side PTY attachment via WebSocket. Establishes a `/ws-pty/:runId` connection, receives binary PTY frames, and feeds them to an xterm.js terminal instance. Handles reconnection, resize events (sending `TIOCSWINSZ` ioctl to the tmux pane), and cleanup on disconnect. A single tmux session can have many simultaneous PTY clients (browser Workspace, `agenthub lanes shell` CLI, etc.), all synced live | | `routes/run.js` | HTTP surface for the tmux+PTY Run feature. **Same-origin guard** on every route — browser requests must come from a localhost-ish Origin (`localhost`, `127.0.0.1`, `::1`, `0.0.0.0`); missing-Origin (curl/CLI) requests pass. When `DASHBOARD_TOKEN` is configured it is **also** required on these routes (same as the rest of `/api/*`). `GET /api/run` lists all live runs (computed fresh from tmux state via `tmux list-sessions`). `GET /api/run/tmux` reports whether `tmux` is installed and on PATH (required for the feature to work). `GET /api/run/binary` probes whether `claude` is on `PATH`. `GET /api/run/cwds` suggests cwds (dashboard + home + recent from sessions table). `GET /api/run/history?laneId=...` returns persisted run history, optionally scoped to one lane. `GET /api/run/files?cwd=&q=` powers the Workspace page's `@`-file autocomplete: scoped fuzzy search inside `cwd` skipping `node_modules`, `.git`, `dist`, `build`, `.next`, `.cache`, `coverage`, `vendor`, etc., capped result count, ranked by basename match. `POST /api/run` requires `laneId` in the body and starts/attaches a tmux-backed run. `GET /api/run/:id` returns the run handle. `DELETE /api/run/:id` kills the tmux session (sends SIGTERM to the pane, escalates to SIGKILL after 5 s). The PTY frames are streamed to the client over `/ws-pty/:runId` as binary frames (not JSON), rendering a real interactive terminal in xterm.js on the Workspace page. `tmux` must be installed on the dashboard server's machine (same as better-sqlite3's native-module requirements) | | `routes/lanes.js` | Durable-lane API. `POST /api/lanes/worktree`, `PATCH /api/lanes/:id`, destructive actions, and `DELETE /api/lanes/:id` use the Run route's same-origin guard. Worktree provisioning validates an absolute source git repository, persists a managed lane as `provisioning`, returns `202`, then uses the per-lane lock to resolve the base and add the worktree. Completion broadcasts the existing `lane_update` payload as `idle`; a git failure leaves a row that the non-destructive delete route can forget. `GET /api/lanes/:id/preflight?action=reset\|remove\|purge` produces counted confirmation facts. Confirmed `POST /:id/{reset,remove,purge}` actions require a complete `expect`, run under the same lock, kill a recorded run and wait for the spawner's actual child-exit timestamp (or return `500 ERUNTIMEOUT` before git), clear `run_id`, reject changed facts with `409 ESTALE` including expected/current diagnostics, and require `force` for unpushed managed reset/remove work. Reset and managed removal call the worktree's independent managed-kind, realpath-within-`LANES_ROOT`, and listed-worktree guard; adopted reset is refused, while adopted remove only forgets its row and never modifies its directory, and a managed lane whose directory was deleted by hand takes a prune path that still enforces the managed-kind and inside-`LANES_ROOT` checks. `start` returns `409 ERUNLIVE` rather than overwriting a live `run_id` and orphaning its child. `kind`, `source_repo`, `slug`, `base_branch`, `slot` and `ports` are not patchable — provisioning writes them through `lanesLib.setProvisioningFacts`. Worktree provisioning also runs `lane-runtime.js:provisionLane` (A2) when the repo declares a `.agenthub/profile` — seed `.env`, `bootstrap`, create the database, migrate, seed — before the lane reports `idle`; `reset`/`remove` likewise call `resetLaneData`/`removeLaneData`, with `reset` accepting a body `keepDb: true` to skip the whole drop-recreate-migrate-reseed block. The runtime routes (`GET /:id/runtime`, `POST /:id/up`, `POST /:id/down`, `POST /:id/hook/:name`, `GET /:id/logs/:svc`) are registered **before** the `/:id/:action` catch-all so `up`/`down` are not swallowed as unknown actions, and are deliberately kept out of it: that catch-all drives a lane's Claude run, these drive the application the lane is working on. `GET /api/lanes/pipeline-stats` aggregates per-node pass counts over every lane and every archived feature. | diff --git a/client/src/components/run/RunHistory.tsx b/client/src/components/run/RunHistory.tsx index 87ff4462..91b8c38d 100644 --- a/client/src/components/run/RunHistory.tsx +++ b/client/src/components/run/RunHistory.tsx @@ -521,14 +521,17 @@ function UnifiedRunRowView({ }) { const { t } = useTranslation("run"); const startedDate = new Date(row.startedAt); - const startedLabel = isNaN(startedDate.getTime()) - ? "-" - : startedDate.toLocaleString(undefined, { - month: "short", - day: "numeric", - hour: "2-digit", - minute: "2-digit", - }); + // 0 is "unknown", not the epoch — a run whose persisted row is missing has no + // start time, and printing "Jan 1, 08:00 AM" for it reads as real data. + const startedLabel = + !row.startedAt || isNaN(startedDate.getTime()) + ? "-" + : startedDate.toLocaleString(undefined, { + month: "short", + day: "numeric", + hour: "2-digit", + minute: "2-digit", + }); // Without mode distinction, offer resume for any finished run with a session. // An external session is live but has no attachable tmux session, so Resume // (a new tmux-backed `claude --resume` in its cwd) is what it gets instead. diff --git a/client/src/components/run/__tests__/RunHistory.test.tsx b/client/src/components/run/__tests__/RunHistory.test.tsx index 1cef2c99..5a0bfdee 100644 --- a/client/src/components/run/__tests__/RunHistory.test.tsx +++ b/client/src/components/run/__tests__/RunHistory.test.tsx @@ -242,6 +242,14 @@ describe("ActiveRunsSwitcher", () => { expect(screen.queryByText(i18n.t("run:runs.externalBadge"))).toBeNull(); }); + it("shows a dash, not the epoch, for a row with no start time", () => { + // A run whose persisted row is missing arrives with startedAt 0. Rendering + // that as "Jan 1, 08:00 AM" presents a gap in the data as real data. + renderModal([row("no-start", { startedAt: 0 })]); + expect(screen.getByText("-")).toBeTruthy(); + expect(screen.queryByText(/Jan 1,/)).toBeNull(); + }); + it("fires attach with the run id of the row that was clicked", () => { const { spies } = renderSwitcher(); openModal(); diff --git a/docs/API.md b/docs/API.md index e25b33f6..cec63ea2 100644 --- a/docs/API.md +++ b/docs/API.md @@ -245,7 +245,7 @@ All fields are optional. The tool (CLI invoked) comes from the lane itself, neve - **`approval`** — one of `on-request` or `never`, or omitted/empty for no approval prompt. Passed to `codex -a `. - **`resumeSessionId`** or **`resumeLast: true`** — resume an earlier run instead of starting new. For Codex lanes specifically, `resumeLast: true` uses `codex resume --last`; `resumeSessionId` uses `codex resume `. For Claude lanes, `resumeSessionId` re-attaches a Claude session by id. -Error `409 ERUNLIVE` is returned if the lane already has a live run; stop it first with the `stop` action. +Error `409 ERUNLIVE` is returned if the lane already has a live run; stop it first with the `stop` action. The same code and status come back when the lane's tmux pane is already running an agent while a `prompt` or `resumeSessionId` was sent — that argv cannot be typed over a live agent, so the request is refused instead of answered 200 with nothing done. A start that asks for neither still adopts the existing pane silently. #### Preflight a destructive lane action diff --git a/server/README.md b/server/README.md index cc32150d..21027752 100644 --- a/server/README.md +++ b/server/README.md @@ -555,7 +555,12 @@ failures return `500` with `error.stderr`. `start` returns `409 ERUNLIVE` when the lane's recorded run is still `spawning` or `running`: overwriting `run_id` would orphan that child, and a later `reset` -would then `git clean -fd` a directory the orphan is still writing into. +would then `git clean -fd` a directory the orphan is still writing into. It +returns the same code when the lane's tmux pane is running an agent and the +request carried a `prompt` or `resumeSessionId` — `pty-run.js` will not type +over a live agent, and adopting the pane silently would answer 200 for a resume +that never spawned. `POST /api/run` maps that code to `409` too; a start with +neither field still adopts the pane. Besides the stage a skill declares explicitly, every lane also carries a server-inferred `detected_stage`: `server/lib/stage-detect.js` matches each diff --git a/server/__tests__/pty-run.test.js b/server/__tests__/pty-run.test.js index 0d440d0b..402c6c9c 100644 --- a/server/__tests__/pty-run.test.js +++ b/server/__tests__/pty-run.test.js @@ -335,4 +335,91 @@ describe("pty-run", () => { ); assert.ok(!call.some((a) => String(a).startsWith("--dangerously-bypass"))); }); + + // `dashboard_runs.mode` is NOT NULL and recordRun swallows insert errors, so a + // null there persisted nothing at all while every call still looked fine: the + // runs modal rendered a row with no cwd, model, prompt or start time, and a + // resumed session kept its separate `external` row because no run row ever + // carried its session id. These assert the row actually lands. + it("refuses to swallow a resume or a prompt when the pane already runs an agent", () => { + // Adopting silently here answered 200 while spawning no `--resume` at all: + // the dashboard reported a resumed session that never happened. + tmux.__setExecImpl((args) => { + if (args[0] === "display-message") return "claude\n"; + return ""; // has-session succeeds → pane is busy with a live agent + }); + assert.throws( + () => pty.spawnRun({ laneId: 51, cwd: "/tmp/repo", resumeSessionId: "abc12345" }), + (err) => err.code === "ERUNLIVE" + ); + assert.throws( + () => pty.spawnRun({ laneId: 51, cwd: "/tmp/repo", initialPrompt: "do the thing" }), + (err) => err.code === "ERUNLIVE" + ); + }); + + it("still adopts a busy pane when the request asked for nothing but a pane", () => { + const calls = []; + tmux.__setExecImpl((args) => { + calls.push(args); + if (args[0] === "display-message") return "claude\n"; + return ""; + }); + const handle = pty.spawnRun({ laneId: 52, cwd: "/tmp/repo" }); + assert.equal(handle.id, "agenthub-lane-52"); + assert.ok(!calls.some((c) => c[0] === "new-session")); + assert.ok(!calls.some((c) => c[0] === "send-keys")); + }); + + describe("run persistence", () => { + const dashboardRuns = require("../lib/dashboard-runs"); + + it("spawnRun persists the run so the runs list has cwd, model and prompt", () => { + tmux.__setExecImpl((args) => { + if (args[0] === "has-session") { + const e = new Error("gone"); + e.status = 1; + throw e; + } + return ""; + }); + pty.spawnRun({ + laneId: 71, + cwd: "/tmp/persisted", + model: "claude-opus-5", + initialPrompt: "ship it", + }); + + const row = dashboardRuns.getRun("agenthub-lane-71"); + assert.ok(row, "expected a dashboard_runs row for the spawned run"); + assert.equal(row.cwd, "/tmp/persisted"); + assert.equal(row.model, "claude-opus-5"); + assert.equal(row.prompt_preview, "ship it"); + assert.equal(row.lane_id, 71); + assert.ok(row.started_at, "started_at must be set, or the UI dates the row to the epoch"); + assert.ok(row.mode, "mode is NOT NULL — a null here silently drops the whole insert"); + }); + + it("a resumed run carries the session id, which is what dedupes the external row", () => { + tmux.__setExecImpl((args) => { + if (args[0] === "has-session") { + const e = new Error("gone"); + e.status = 1; + throw e; + } + return ""; + }); + const sessionId = "c2055094-09cf-4735-9712-09e5cdf7023f"; + const handle = pty.spawnRun({ + laneId: 72, + cwd: "/tmp/resumed", + resumeSessionId: sessionId, + }); + + // The UI reads sessionId off the run list; an external session whose id is + // already on a run row is not repeated as its own `external` row. + assert.equal(handle.sessionId, sessionId); + assert.equal(dashboardRuns.getRun("agenthub-lane-72").resume_session_id, sessionId); + }); + }); }); diff --git a/server/lib/dashboard-runs.js b/server/lib/dashboard-runs.js index 6b3eb4b1..8f971826 100644 --- a/server/lib/dashboard-runs.js +++ b/server/lib/dashboard-runs.js @@ -91,8 +91,12 @@ function recordRun(handle) { lane_id: typeof handle.laneId === "number" ? handle.laneId : null, tool: handle.tool === "codex" ? "codex" : "claude", }); - } catch { - /* persistence is best-effort */ + } catch (err) { + // Best-effort by design (a live run must never fail because sqlite did), + // but not silent: a swallowed insert leaves the runs modal showing rows with + // no cwd, model, prompt or start time, which reads as a UI bug rather than a + // persistence one. Say it once, here, where the cause is known. + console.warn(`[dashboard-runs] failed to record run ${handle?.id}: ${err.message}`); } } diff --git a/server/lib/pty-run.js b/server/lib/pty-run.js index c115eb4d..18e396a7 100644 --- a/server/lib/pty-run.js +++ b/server/lib/pty-run.js @@ -200,7 +200,13 @@ function spawnRun(args) { dashboardRuns.recordRun({ id, sessionId: resumeSessionId || null, - mode: null, + // `dashboard_runs.mode` is NOT NULL. Passing null made every insert throw + // inside recordRun's best-effort catch, so no tmux-backed run was ever + // persisted: publicRun then read its cwd/model/prompt/startedAt off a row + // that did not exist (a blank row dated Jan 1 1970 in the runs modal), and + // session_id never landed either, so a resumed session stayed listed as a + // separate `external` row. Every run here is one interactive pane. + mode: "interactive", cwd, tool: tool === "codex" ? "codex" : "claude", model: model || null, @@ -238,6 +244,18 @@ function spawnRun(args) { // Pane is running something (a live `claude`, an editor, a build): adopt // silently, same convention as this repo's server port-adoption logic — no // error, no duplicate session. Attaching shows the user what is running. + // + // Adopting is only honest when the request asked for nothing more than a + // pane. A Resume or an initial prompt cannot be delivered into a pane that is + // already running an agent (typing over it is what the shell-pane branch + // above exists to avoid), so answering 200 there reports work that never + // happened: the user sees "resumed" while no `--resume` was ever spawned. + else if (resumeSessionId || initialPrompt) { + throw makeErr( + "ERUNLIVE", + `lane ${laneId}'s pane is already running \`${tmux.paneCommand(id) || "a process"}\` — stop it before resuming a session or sending a prompt` + ); + } return getRun(id); } diff --git a/server/routes/lanes.js b/server/routes/lanes.js index 8de6c9f0..572cf652 100644 --- a/server/routes/lanes.js +++ b/server/routes/lanes.js @@ -898,7 +898,9 @@ function sendLifecycleError(res, err) { if (["ENOTMANAGED", "EOUTSIDEROOT", "ENOTWORKTREE", "EEXPECT"].includes(err.code)) { return res.status(400).json({ error: { code: err.code, message: err.message } }); } - if (["ESTALE", "EUNPUSHED"].includes(err.code)) { + // ERUNLIVE: the lane's pane is busy with a live agent, so a resume/prompt + // could not be delivered — a conflict, not a bad request or a server fault. + if (["ESTALE", "EUNPUSHED", "ERUNLIVE"].includes(err.code)) { return res.status(409).json({ error: { code: err.code, diff --git a/server/routes/run.js b/server/routes/run.js index b0af850b..e8f028b5 100644 --- a/server/routes/run.js +++ b/server/routes/run.js @@ -256,6 +256,10 @@ router.post("/", (req, res) => { }); return res.status(201).json(handle); } catch (err) { + // A busy pane is a conflict with live state, not a malformed request. + if (err.code === "ERUNLIVE") { + return res.status(409).json({ error: { code: err.code, message: err.message } }); + } if (err.code && err.code.startsWith("E")) { return res.status(400).json({ error: { code: err.code, message: err.message } }); }