diff --git a/.github/workflows/release-tui.yml b/.github/workflows/release-tui.yml index 3093730..a50c6f7 100644 --- a/.github/workflows/release-tui.yml +++ b/.github/workflows/release-tui.yml @@ -105,6 +105,10 @@ jobs: test -x "$launcher" # Proves compile + OTUI_ASSET_ROOT launcher; no wasm path crash. node scripts/smoke-tui-binary.mjs "$launcher" + # Proves the binary is actually usable: renders, accepts typed input, quits on Ctrl+D. + # A compile without the Solid JSX transform passes every check above and still ships + # a session nobody can type into. + CLAWFIX_TUI_REQUIRE_PTY=1 node scripts/smoke-tui-interactive.mjs "$launcher" - name: Upload artifact uses: actions/upload-artifact@v4 diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 0000000..768a83d --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,542 @@ +# ClawFix end-to-end review — v0.11.2 (main @ 6d51d7e) + +> **Status: implementation pass complete on branch `review/upgrade-pass` (local only, never pushed).** +> All 20 planned items are done. Gates: 365 node tests, 87 TUI tests, tsc clean, 80 frames, +> binary built and smoked. See "Implementation outcome" at the end for what was fixed, what was +> verified, and the one item that is only partially fixed. +> +> **Two further blockers were found and fixed after the first pass: B-23 (the standalone +> binary accepted no keyboard input) and B-24 (the guarded repair reported success without +> the gateway running), the latter found only by running against a real OpenClaw install.** + +Reviewed: CLI core, TUI, web service, install/release flow, website copy. +Method: source review + live server, live SSE, published-tarball inspection, frame captures, +a compiled standalone binary, and targeted repros. Every finding below has reproducible evidence. + +## Scope note + +The working directory was empty at session start; `/root/clawfix` did not exist. This review is of +`origin/main @ 6d51d7e` fetched from GitHub, so **`git ls-files --others` shows nothing — any +uncommitted local work on your machine was not reviewed.** Host is macOS/arm64, Bun 1.3.6, +Node 22.22.3 (not the pinned Bun 1.2.21). + +## Gate results + +| Gate | Result | +|---|---| +| `npm test` | **349/350 pass, 1 fail** — `test/install-script.test.js:70`, macOS-only (L-16) | +| `cd cli/tui && bun test` | **59/60 pass, 1 fail** — `test/app.test.tsx:27`, macOS-only (L-16) | +| `cd cli/tui && bunx tsc --noEmit` | **clean (exit 0)** | +| Frame sweep 60x20 → 140x40 | **rendered, no regression at swept sizes** — but the sweep misses the broken range (M-9) | +| `scripts/build-tui-release.mjs` + binary smoke | **binary builds and renders real UI** (see below); repo smoke script is a no-op gate (M-11) | + +Binary smoke, rendered output (not exit code) from `clawfix-tui-darwin-arm64.bin`: + +``` +ClawFix v... +Tell me what is going wrong +Local only · Enter send · Shift+Enter newline · Ctrl+P help · Ctrl+C cancel +OpenClaw diagnostics and guarded repairs +Remote AI analysis always asks before anything leaves this machine. +``` + +It rendered — and then had to be killed with SIGTERM, because of B-2. + +--- + +# Findings + +## BLOCKER + +### B-1 — Remote repair proposals are silently dropped; the remote repair flow is dead + +`src/routes/agent-v2.js:163` emits exactly: + +```js +sse.send('repair.proposed', { repairId: ..., rationale: ... }); +``` + +`cli/tui/src/session-bridge.ts:468` handles it with `planFromRaw(event.plan || event)`, and +`planFromRaw` returns `null` unless the object carries `planId` or `id` +(`cli/tui/src/session-bridge.ts:137-138`). The server never sends either. `if (plan)` is false, so +**no repair card, no approval dialog, no error** — the user only sees the assistant prose. + +Repro (real server event shape, real bridge): + +``` +dialog.type : none +repair cards : 0 +session.proposeRepair: 0 call(s) +RESULT: NO approval dialog. Remote repair proposal was silently dropped. +``` + +This also violates invariant 2. The client is supposed to resolve a server repair ID **locally**; +`session.proposeRepair()` is never called on this path. Worse, the code as written would render a +**server-authored** plan — server-supplied `risk`, `summary`, and up to 20 000 chars of +`unifiedDiff` (`session-bridge.ts:164-178`) — inside an approval dialog the user reads as local. +Execution still fails closed (`repair-engine.js:86-88` rejects an unknown `planId`), so the impact +is UI deception plus a dead feature, not code execution. + +**Fix:** in the `repair.proposed` branch, look up a local finding whose `repairId === event.repairId`, +call `session.proposeRepair(finding.id)`, and build the `RepairPlanView` from the **returned local +plan**. Surface an explicit error card when no local finding matches. Never let `planFromRaw` +consume a server payload. + +This is exactly the flow named in your known-open-items as having no e2e coverage — that is why it +shipped broken. + +### B-2 — There is no way to exit the TUI + +`cli/tui/src/main.tsx:106-107` sets `exitOnCtrlC: false` and `exitSignals: []`. `cli/tui/src/app.tsx:128-130` +binds Ctrl+C to `cancelScan()` only. `cli/tui/src/keymap.tsx` documents no quit key — Ctrl+C is +labelled "Cancel active scan/remote request". Because stdin is in raw mode, Ctrl+C arrives as a key +event, so the `SIGINT` listener in `topLevelExit` (`main.tsx:65-69`) never fires. + +Evidence: the compiled binary ran past a 120 s timeout, and a follow-up run survived 12 s of +repeated `0x03` — `STILL RUNNING after 12s of Ctrl+C — no quit path`. Only SIGTERM/SIGKILL ended it. + +**Fix:** add an explicit quit binding (Ctrl+D, or `q` when the composer is empty, or double Ctrl+C +within ~1 s) that calls `renderer.destroy()`, and document it in `keymap.tsx` + `KEY_HINTS`. + +--- + +## HIGH + +### H-3 — The TUI is not in the published npm package, so `npx clawfix` can never launch it + +`cli/package.json` `files` allowlists `bin/ adapters/ core/ interfaces/ README.md LICENSE` — no +`tui/`. `cli/bin/clawfix.js:93` resolves `../tui/src/main.tsx` and spawns Bun with +`cwd: join(cliDir, '../tui')` (`clawfix.js:109`), a directory that does not exist once installed. + +Evidence — published tarball contents: + +``` +$ npm pack clawfix@0.11.2 && tar -tzf clawfix-0.11.2.tgz | grep -c tui +0 +``` + +21 files, zero TUI files. `scripts/install.sh` installs that same registry tarball +(`install.sh:205-233`), so **both documented install paths lack the TUI.** + +Worse, the failure message blames the wrong thing. On a machine with Bun on PATH: + +``` +$ node package/bin/clawfix.js --tui +Fatal error: spawn /Users/felirami/.bun/bin/bun ENOENT +``` + +Bun is installed; the missing thing is the `cwd`. Users will chase a Bun install that is already there. + +**Fix:** decide and align. Either add `tui/` to `files` (and accept the install-size/Bun-dependency +cost), or keep it out and (a) make `runOpenTuiMode` detect a missing entry file *before* spawning and +say "the OpenTUI session is not included in the npm package — download the standalone binary from +", and (b) reword the site so the TUI is presented as a separate binary download. + +### H-4 — Stale `SCRIPT_HASH` breaks the documented verification ritual + +`/fix/sha256` tells users to compare against a GitHub file that does not match. Live: + +``` +$ curl -s localhost:3111/fix/sha256 +{"sha256":"c8f29823b1534fda1a26c4d95abaa9f17b5998c18077a3f5c1918d59317f37d8", + "note":"...GitHub reference: https://github.com/arcabotai/clawfix/blob/main/SCRIPT_HASH"} + +$ cat SCRIPT_HASH +4470a867f26e2c3e729c0a03c109270252e11e2973089984731e2c503b78d96b +``` + +`SCRIPT_HASH` was last touched in `6aa3908`; the served script (`src/routes/script.js:22-593`) has +changed across several releases since. A user who performs the verification honestly gets a +mismatch, and the only lesson available is "ignore the mismatch" — which is worse than no check. + +(`/install/sha256` is fine: it hashes `scripts/install.sh` at runtime and points at the source file. +Verified matching: `21816fab…`.) + +**Fix:** regenerate `SCRIPT_HASH` in CI on every change to `src/routes/script.js`, and add a test +asserting `sha256(DIAGNOSTIC_SCRIPT) === readFileSync('SCRIPT_HASH')`. Until then, point the note at +`/fix/sha256` itself rather than a stale repo file. + +### H-5 — The consent dialog understates what leaves the machine + +`cli/tui/src/remote-analyzer.ts:84-107` uploads the **entire redacted diagnostic** to `/api/diagnose` +inside `send()`, before the agent call. The privacy dialog the user approves +(`session-bridge.ts:351-373`) previews only the agent-v2 body, with `diagnosticId: null` and a +hardcoded `conversationId: "pending-session"`, and `risk: "medium"` for every repair +(`session-bridge.ts:359`). The disclosure text says fields are included "when present on a linked +diagnostic" — it never says *this action creates that linked diagnostic by uploading it*. + +Consent is correctly gated (free text does open the dialog first; `session-bridge.ts:588-597`), and +`redactOutbound` is applied — the defect is that "Inspect payload" shows something other than what +is sent. + +**Fix:** show both requests in the disclosure — the `/api/diagnose` upload and the +`/api/v2/agent/messages` body — and build the preview from the same `availableRepairs()` the client +actually sends. + +### H-6 — A throwing scan leaves stale findings attached to the new revision + +`cli/core/session.js:152-167`. The `result.error` branch (126-131) correctly clears `diagnostic`, +`issues`, `findings`, `summary`. The `catch` branch does not: it sets `revision = nextRevision` and +`scanError`, then emits `session.scan.committed` with `findingsCount: findings.length` — the **previous +scan's** findings, now labelled with the new revision. + +So after a failed rescan the UI shows old findings as current, and those findings remain +`repairable` and can be proposed against a revision whose scan never produced them. + +**Fix:** clear the same four fields in the `catch` branch before emitting. + +--- + +## MEDIUM + +### M-7 — An unhandled scan rejection tears down the whole TUI + +`cli/tui/src/live-session.ts:115` does `void bridge.scan()`. `bridge.scan()` re-throws whatever +`session.scan()` throws (`session-bridge.ts:540-550`), and `session.scan()` re-throws on the catch +path (`session.js:166`). `main.tsx:60` maps `unhandledRejection` to `rejectFatal`, which kills the +renderer and prints `ClawFix TUI failed: …`. A first-scan exception — e.g. `plainSummary()` at +`live-session.ts:42-51` dereferencing `summary.gateway` when `summary` is undefined — takes the app +down at startup instead of showing a scan error. +**Fix:** `void bridge.scan().catch(() => {})` and let the committed `scanError` render. + +### M-8 — Hint line wraps at widths 65–91, not 60 + +`cli/tui/src/app.tsx:164-168` switches to the short hint at `width <= 64`. The long hint is 62 chars; +with `aiLabel` it needs 75 ("Local only") to 89 ("Remote (pending consent)") columns, against a +content width of `width - 2`. Measured: + +``` +width 64 -> 1 line width 65..90 -> WRAPS to 2 lines width 92 -> 1 line +``` + +The status line is not pushed off screen (the composer column grows), but a transcript row is eaten +and the footer looks broken across the most common 80-column terminal band. +**Fix:** don't guess a threshold — compute `` `${aiLabel()} · ${LONG_HINTS}`.length <= dims().width - 2 `` +and fall back to the short hint otherwise. A constant bump to `<= 91` also works but re-breaks the +moment a hint or label changes. + +### M-9 — The frame sweep structurally cannot catch M-8, and validates fake copy + +`cli/tui/scripts/frames.tsx:17-23` samples 140/120/100/80/60 — it never samples 61–79 or 81–99, which +is exactly where M-8 lives. Every state comes from `createFakeSession()`, so `aiMode` is always +`"local"` (the shortest label, the least likely to wrap). And the privacy frame +(`frames.tsx:135-146`) hand-writes a disclosure — `endpointUrl: ".../api/analyze"`, provider +`"ClawFix hosted AI (deepseek v4 flash)"` — that does not match `buildDisclosureView` +(`lib/disclosure.ts:50-58`), which produces `/api/v2/agent/messages` and +`ClawFix service → OpenRouter → selected model`. The UX gate is reviewing copy that ships nowhere. +**Fix:** add 66/72/88 widths, sweep `aiMode` variants, and build the privacy frame from the real +`buildDisclosureView()`. + +### M-10 — Transcript items get new keys on every render + +`session-bridge.ts:190` uses `typeof entry.id === "string" ? entry.id : nextId("msg")`, but session +messages are `{ type, role, text, at }` (`session.js:177`) — no `id`, ever. So every `publish()` +re-keys every message, defeating Solid's keyed reconciliation during token streaming, and +`idCounter` grows without bound. +**Fix:** assign a stable id in `appendMessage()` and use it. + +### M-11 — The binary smoke gate cannot fail + +`scripts/smoke-tui-binary.mjs:57-61` falls back to `smokePlain` when `node-pty` is absent. It is +absent (not in `package.json`), and the CI step (`.github/workflows/release-tui.yml:101-107`) never +installs it. `smokePlain` asserts bytes-on-stdout and an exit code, not rendered UI. My run: + +```json +{ "ok": true, "mode": "plain", "full": false, "exitCode": 0, "stdoutBytes": 5048 } +``` + +Note the interaction with B-2: the PTY path writes `\x03` and waits for exit (`:92-104`), so if +`node-pty` *were* installed the gate would fail with "timed out waiting for TUI startup". The gate is +green only because it is degrading. +**Fix:** add `node-pty` as a dev dependency, fail loudly instead of degrading, and assert rendered +strings ("ClawFix", the composer placeholder) rather than byte counts. + +### M-12 — `conversations` is an unbounded in-process Map + +`src/routes/agent-v2.js:23,72-78` stores up to 12 messages per conversation, keyed by a +client-supplied `conversationId`, with `createdAt` recorded and never read. Nothing evicts. On a +long-lived Railway instance any client can grow server memory with fresh conversation IDs, rate +limits notwithstanding (30/min/IP still allows ~43k conversations/day per IP). +**Fix:** TTL sweep on `createdAt` plus a hard cap with LRU eviction. + +### M-13 — `nativeChecks` is a dead parameter + +`session.js:137` passes `nativeChecks: result?.nativeChecks` into `normalizeFindings`, which does not +accept it (`findings.js:129-134`). Native checks only survive because `diagnostics.js` folds them into +`localIssues` with a `nativeCheckId`. The header comment on `findings.js:1-8` claims it normalizes +"native OpenClaw findings", implying a path that isn't there. +**Fix:** delete the argument, or implement the branch and cover it with a test. + +### M-14 — High/critical refusal is enforced only in the TUI + +`session-bridge.ts:690-701` refuses `high`/`critical` plans. `repair-engine.js:84-135` does not check +`plan.risk` at all, and the plain interface has no equivalent guard. Today the catalog holds exactly +one `low`-risk entry (`repair-catalog.js:74-76`), so this is latent — but the invariant is enforced at +the UI layer, which is the wrong layer. +**Fix:** reject high/critical inside `applyPlan` and return `{status:'blocked', reason:'risk_refused'}`. + +### M-15 — `verify`/`rollback` exceptions escape `applyPlan` + +`repair-engine.js:115-135` wraps `entry.apply` in try/catch but not `preflight`, `verify`, or +`rollback`. A throwing `verify` rejects the promise after the repair has already been applied, so the +caller reports a generic failure and loses the `applyResult` — the one case where truthful reporting +matters most. +**Fix:** wrap each stage; on a `verify` throw return `{status:'verify_failed', verifyError, applyResult}`. + +--- + +## LOW / NIT + +- **L-16 — Two tests are Linux-only, so the suite is not green on macOS.** + `test/install-script.test.js:95` uses GNU `tar --transform`; BSD tar rejects it + (`tar: Option --transform=... is not supported`), the helper ignores the failure, and the test dies + later on `ENOENT … clawfix-0.10.0.tgz`. `cli/tui/test/app.test.tsx:27,39` hardcodes + `@opentui/core-linux-x64`, absent on darwin. Fix: use `-s ,^,package/,` on BSD or `tar -C` with a + pre-made `package/` dir; select the native package from `process.platform`/`arch`. +- **L-17 — Landing tense conflict.** `src/landing.js:564` "Chat-first TUI is now the default session." + vs `:572` "The chat-first TUI ships in the next release." The "New on main" pill (`:504-507`) and the + note are correctly labelled — only the H2 overclaims. Fix: "Chat-first TUI lands as the default session." +- **L-18 — Node engine drift.** `cli/package.json` `engines.node >= 18`, root `>= 22`, + site says "Node.js 22+" (`landing.js:519`). The CLI uses `--env-file-if-exists` and Node 22 APIs + elsewhere. Fix: raise the CLI floor to 22. +- **L-19 — `runOpenTuiMode` conflates "launched" with "exited 0"** (`cli/bin/clawfix.js:122-126`). + A TUI that starts, runs, and exits non-zero returns `false`, so the default path drops the user into a + *second*, full plain session. Fix: track "did the child start" separately from its exit code. +- **L-20 — `statusLine()` can split a surrogate pair** (`app.tsx:179`, `full.slice(0, budget - 1)`), and + measures 🦞 by `.length` rather than display width. +- **L-21 — `records` Map in `repair-engine.js:43` is never pruned**; consumed plans live for the + process lifetime. +- **L-22 — Disclosure defaults are duplicated** in `cli/core/privacy.js:15-29` and + `cli/tui/src/lib/disclosure.ts:5-19`. They agree today; the frames harness (M-9) shows how quietly + they drift. Fix: single source, imported by the TUI. + +--- + +## Verified sound (no action) + +Worth stating explicitly, since these are the load-bearing claims: + +- **agent-v2 contract holds under live testing.** Banned field → `{"error":"Field \"shell\" is not allowed"}`; + 7-char `conversationId` → rejected; 33 repairs → `too many availableRepairs`; valid request streams + `agent.meta → assistant.delta × N → agent.done`. `propose_repair` is a closed enum built from + client-supplied IDs (`contract.js:122-150`) and re-validated against the allowlist + (`contract.js:156-189`). +- **Consent gating for free text is correct.** Deterministic commands bypass + (`session-bridge.ts:583`); everything else opens the dialog with focus defaulting to `stay-local` + (`:368`), and `runRemote` re-checks consent (`:434-437`). +- **Offline analyzer is deterministic** — unmatched input returns "Unknown local command. Type help for + the deterministic command list." (`offline-analyzer.js:106-108`), never a generated answer. +- **Approval defaults to Cancel** (`session-bridge.ts:381`) and success copy is gated on + `status === "applied"` (`:718`); `rejected`/`blocked`/`verify_failed` all produce failure cards + (`:731-751`). The bridge calls `applyRepair({planId, approvalToken, findingId, ctx})` — no + `approveRepair` fallback anywhere in the tree. +- **Plan binding is sound**: single-use token consumed before any further check + (`repair-engine.js:91-111`), revision + fingerprint checked, `verify` reads runtime evidence. +- **AI abuse gate is fail-closed.** `isPaidAIEnabled` (`security.js:76-79`) plus a timing-safe bearer + check in `createAIRequestGuard` (`security.js:105-108`); concurrency + daily budget + per-IP limits. +- **Site claims that check out:** `npx clawfix@0.11.2` resolves (npm `latest` = 0.11.2); npm + attestations exist (`dist.attestations`, SLSA provenance v1); "21-file allowlisted package" is exactly + right; "49 known issue detectors" matches `KNOWN_ISSUES.length === 49`; TUI binaries + `TUI-SHA256SUMS` + are attached to the v0.11.2 release; install copy is download → verify → bash throughout, with no + `curl | bash` anywhere (enforced by `install-script.test.js:62`). + +--- + +# Upgrade plan + +Ordered by value-at-risk. Nothing here is started — awaiting your go. + +## Fix (in this order) + +| # | Item | Effort | Risk if done | Risk if skipped | +|---|---|---|---|---| +| 1 | **B-1** resolve `repair.proposed` locally via `session.proposeRepair` | M | Low — new code path, fully unit-testable | The product's headline flow does not work | +| 2 | **B-2** add a quit binding + document it | S | Low | Shipped binary traps users | +| 3 | **H-6** clear findings on the scan catch path | S | Low | Stale findings repairable under a new revision | +| 4 | **M-7** `.catch()` the autoscan | S | Very low | Startup scan error kills the TUI | +| 5 | **H-4** generate `SCRIPT_HASH` in CI + equality test | S | Low | Documented integrity check fails for honest users | +| 6 | **H-5** disclose the `/api/diagnose` upload in the privacy dialog | M | Low, copy + preview only | Consent UI understates the upload | +| 7 | **H-3** decide TUI packaging; fix the misleading Bun error either way | M | Medium — shipping `tui/` changes package size and adds a Bun dependency; **your call** | Site promises a TUI that `npx` cannot run | +| 8 | **M-14/M-15** enforce risk refusal and wrap stages in `repair-engine` | S | Low | Safety invariant enforced at the UI layer only | +| 9 | **M-8** measure the hint line instead of thresholding | S | Low | Footer wraps across the 65–91 column band | +| 10 | **M-12** TTL + cap on `conversations` | S | Low | Unbounded server memory | +| 11 | **L-16** make the two tests platform-agnostic | S | Low | Suite cannot go green on macOS | + +## Improve + +| # | Item | Effort | Risk | +|---|---|---|---| +| 12 | **M-11** add `node-pty`, fail instead of degrading, assert rendered strings | S | Low — will surface B-2 immediately, which is the point | +| 13 | **M-9** widen the frame sweep; drive the privacy frame from `buildDisclosureView` | S | Low | +| 14 | **New:** scripted PTY e2e — scan → consent → remote chat (stub SSE) → repair proposal → approval → apply → verify | L | Medium (fixture-heavy) — but this is the gap that let B-1 ship | +| 15 | **M-10** stable message ids | S | Low | +| 16 | **L-17/L-18** landing tense, Node engine floor | S | Very low | +| 17 | **L-19** separate "started" from "exit code" in mode dispatch | S | Low | +| 18 | **L-20/L-21** grapheme-safe truncation; prune consumed plans | S | Very low | + +## Delete + +| # | Item | Effort | Risk | +|---|---|---|---| +| 19 | **M-13** the dead `nativeChecks` argument (`session.js:137`) and the misleading `findings.js` header claim | S | Very low | +| 20 | **L-22** the duplicated disclosure constants in `lib/disclosure.ts`; import from `cli/core/privacy.js` | S | Low — only if TUI packaging (item 7) keeps `cli/core` reachable | + +## Recommended first commit + +Items 1–4 together: they are the two blockers plus the two one-line correctness fixes, they touch +`session-bridge.ts`, `session.js`, `app.tsx`, `live-session.ts` only, and each is covered by a test I +can add in the same commit. Item 14 (the PTY e2e) should follow immediately so B-1 cannot regress. + +**Open decision for you (item 7):** ship `tui/` inside the npm package, or keep the TUI +binary-only and reword the site? I recommend binary-only — shipping the TUI would add a hard Bun +dependency to an `npx`-first tool — but it means editing the hero copy, so it's your call. + +--- + +# Implementation outcome + +Branch `review/upgrade-pass`, 9 commits, local only — nothing pushed, no Railway, no npm publish. + +## Gates (final run) + +| Gate | Before | After | +|---|---|---| +| `npm test` | 349/350 | **365/365** | +| `cd cli/tui && bun test` | 59/60 | **87/87** | +| `bunx tsc --noEmit` | clean | **clean** | +| Frame sweep | 40 frames, 5 widths | **80 frames, 8 widths**, zero footer wraps | +| Build + smoke | passed without asserting anything | **passes, asserting rendered UI** | + +## Fixed and verified + +| # | Finding | Evidence | +|---|---|---| +| B-1 | Remote repair proposals dropped | e2e test drives the real flow; confirmed to fail against the pre-fix bridge | +| H-3 | TUI absent from npm package | `--tui` on a staged published layout now names the standalone binary; test asserts it | +| H-4 | Stale `SCRIPT_HASH` | regenerated; guard test fails on drift (verified by corrupting it) | +| H-5 | Consent dialog understated the upload | preview built from the real request; `/api/diagnose` disclosed | +| H-6 | Stale findings after a thrown scan | test asserts the cleared state and that the repair can no longer be proposed | +| M-7 | Autoscan rejection killed the TUI | `.catch()` on the autoscan | +| M-8 | Footer wrapped at widths 65–91 | 303 renders across 3 aiModes × widths 40–140: single-row footer everywhere | +| M-9 | Frame sweep could not see M-8 | widths 66/72/88 and consent-pending/remote states added; real disclosure rendered | +| M-10 | Unstable transcript keys | ids assigned in `appendMessage` | +| M-11 | Smoke gate proved nothing | asserts rendered UI; regression test feeds it a non-rendering binary | +| M-12 | Unbounded conversation store | TTL + cap with oldest-first eviction, unit tested | +| M-13 | Dead `nativeChecks` argument | removed; comment corrected | +| M-14 | Risk refusal only in the TUI | enforced in `applyPlan`; nothing runs, not even preflight | +| M-15 | Stage exceptions escaped `applyPlan` | each stage returns a structured outcome; `applyResult` preserved | +| L-16 | Suite unrunnable on macOS | both tests fixed; suite green on macOS | +| L-17..L-22 | Copy, engines, mode dispatch, truncation, plan retention, disclosure drift | all addressed; parity test added | + +## Partially fixed + +**B-2 — TUI quit path.** Fixed and verified *from source*: Ctrl+D, and Ctrl+C when idle, exit in +~0.1s with exit code 0 and the terminal restored (`ICANON`/`ECHO` back on). Ctrl+C still cancels +while work is in flight. The decision is a pure `resolveGlobalKeyAction()`, unit tested, because +`bun test` swallows raw Ctrl+C. + +It does **not** fix the standalone binary, for the reason below. + +## New finding (not fixed) + +### B-23 — BLOCKER: the standalone binary receives no keyboard input at all + +Discovered while verifying B-2. The `bun --compile` build renders the UI correctly but never +receives a key. Driving both builds through a PTY with the pty buffer drained, typing `ZZTOP`: + +``` +compiled binary : typed text echoed into UI: False bytes rendered after typing: 0 +from source : typed text echoed into UI: True bytes rendered after typing: 132 +``` + +So in the shipped binary the composer cannot be typed into, no dialog can be answered, no repair +can be approved, and — the symptom that led here — it cannot be quit. It renders a session that +cannot be used. Reproduced consistently across rebuilds; unaffected by the B-2 fix, which is why +that fix lands only for the source path. + +Not fixed: the cause is below ClawFix, in how OpenTUI's input layer behaves under `bun --compile` +(`cli/tui/bunfig.toml` preloads `@opentui/solid/preload`, which a compiled binary cannot resolve +— the most likely culprit, unconfirmed). Fixing it means changing how the binary is built, which +is outside the approved plan. + +**Suggested next step:** treat this as the top priority. Reproduce with a minimal OpenTUI +`bun --compile` app to isolate it from ClawFix, then either embed the preload plugin at build +time or raise it upstream. Until then the GitHub release binaries render but do not work, and +the site should not present them as a usable way to run ClawFix. + +Note this was invisible to every existing gate: `verify-tui-artifact.mjs` checks the file, and the +smoke script only proved the process started — even after this pass hardened it to assert rendered +output, rendering is exactly what still works. Only driving real keys through a PTY catches it. + +## Methodology note + +An earlier round of this investigation reached wrong conclusions — that `renderer.destroy()` +blocks, that `process.exit()` does not fire — because the PTY harness was not draining the master, +so writes to fd 1 blocked on a full buffer. Once the harness drained continuously, the source +build exited cleanly and those conclusions dissolved. The findings above come from the corrected +harness. + + +--- + +# Real-machine review (Blaxel sandbox, OpenClaw 2026.6.11) + +Everything below ran on a provisioned Linux sandbox with a real OpenClaw install, not a fixture. + +| Check | Result | +|---|---| +| Node suite on Linux | **378 tests, 376 pass, 0 fail** (2 platform-skipped) | +| `clawfix --json` against real OpenClaw | found the binary, version, config, and **7 real issues** including native `openclaw doctor` and security-audit findings | +| Hostname privacy | `hostHash` 8 chars; real hostname, home path and `/root` absent from the diagnostic | +| Redaction of a real secret | a real 48-char `gateway.auth.token` appears **0 times**; the section renders as `"auth": "***REDACTED***"` | +| Guarded repair, full flow | plan built locally, wrong token → `invalid_token`, replay → `token_reused`, outcome truthful against a rescan | +| TUI on Alpine/musl | **cannot run** — see the limitation below | + +## B-24 — BLOCKER (fixed): the guarded repair reported success without a running gateway + +`checkGatewayRunning()` decided liveness from `pgrep -f 'openclaw.*gateway'`. On a real install +that pattern is wrong in both directions: + +- a gateway started by `openclaw gateway run` re-execs with the bare argv `openclaw`, so it + **never matched a real gateway**; +- ClawFix's own `openclaw gateway status` probe — launched concurrently with the pgrep in the + same `Promise.all` — **always matched**. + +Reproduced with nothing listening on 18789: + +``` +verify() -> {"ok":true,"evidence":{"running":true,"pid":"6911\n6926"}} +$ ss -ltn | grep 18789 # nothing listening +``` + +`verify.ok === true` makes `applyPlan` return `applied`, and the UI then prints "Repair applied +… Verification passed against local detectors." A repair that changed nothing would be reported +as a success. The mirror image also held: `preflight` returned `gateway_already_running` while +the gateway was down, so the only repair ClawFix ships could never be applied. + +Fixed by making the listening port the verdict — the same evidence the diagnostics core already +reports as `portListening`. Status prose and PIDs remain evidence, never the verdict, and the PID +probe now excludes ClawFix's own sub-commands and the current process. + +Verified against a real gateway, both states: + +``` +gateway down -> verify {"ok":false,"listening":false,"pid":""} preflight {"ok":true} +gateway up -> verify {"ok":true,"listening":true,"pid":"8855"} preflight {"ok":false,"reason":"gateway_already_running"} +``` + +A full repair run with the service unavailable now reports `verify_failed`, and a rescan agrees +with the reported outcome. + +## Limitation found (not fixed): the TUI cannot run on Alpine/musl + +`@opentui/core` resolves its native library as `@opentui/core-linux-x64` with no musl detection. +On Alpine the glibc `libopentui.so` fails to load (`ld-linux-x86-64.so.2` missing), `gcompat` does +not help, and installing `@opentui/core-linux-x64-musl` does not either — the loader asks for the +glibc package by name and fails with `Cannot find module '@opentui/core-linux-x64'`. All 23 TUI +render tests fail there for this one reason. + +The CLI core is unaffected: the full node suite and the plain interface work on musl, which is +what `npx clawfix` uses. But an operator running OpenClaw in an Alpine container cannot use the +TUI from source or from the release binary. Worth deciding whether to ship a musl target or to +state the requirement on the site; it is an upstream OpenTUI limitation either way. diff --git a/SCRIPT_HASH b/SCRIPT_HASH index b4b6e6b..7b61c63 100644 --- a/SCRIPT_HASH +++ b/SCRIPT_HASH @@ -1 +1 @@ -4470a867f26e2c3e729c0a03c109270252e11e2973089984731e2c503b78d96b +c8f29823b1534fda1a26c4d95abaa9f17b5998c18077a3f5c1918d59317f37d8 diff --git a/cli/adapters/openclaw.js b/cli/adapters/openclaw.js index 5b7ecd2..106f7b6 100644 --- a/cli/adapters/openclaw.js +++ b/cli/adapters/openclaw.js @@ -484,8 +484,59 @@ export function createOpenClawAdapter({ npmVersion(options = {}) { return successfulText('npm', ['--version'], options); }, - gatewayProcesses(options = {}) { - return successfulText('pgrep', ['-f', 'openclaw.*gateway'], options); + /** + * PIDs that plausibly belong to a running gateway *server*. + * + * The old pattern was `pgrep -f 'openclaw.*gateway'`, which was wrong in both directions on + * a real install: a gateway started by `openclaw gateway run` re-execs with the bare argv + * `openclaw`, so it never matched, while ClawFix's own `openclaw gateway status` probe — + * which runs concurrently with this call — always did. So this reported "running" whenever + * ClawFix looked, and never because a gateway was actually up. + * + * Transient CLI invocations are excluded by verb, and this process is excluded outright. + * Treat the result as supporting evidence only: `gatewayListening()` is the real signal. + */ + async gatewayProcesses(options = {}) { + const listing = await successfulText('pgrep', ['-af', 'openclaw'], options); + const self = String(process.pid); + const pids = []; + for (const line of String(listing || '').split('\n')) { + const match = /^(\d+)\s+(.*)$/.exec(line.trim()); + if (!match) continue; + const [, pid, command] = match; + if (pid === self) continue; + // Sub-commands ClawFix itself runs, plus anything merely mentioning openclaw. + if (/\b(gateway\s+(status|restart|stop|logs|probe|health|diagnostics|stability|call|discover|install)|doctor|config|--version)\b/.test(command)) continue; + if (/\b(pgrep|grep|tail|less|cat|sh -c|bash -c|node .*clawfix)\b/.test(command)) continue; + pids.push(pid); + } + return pids.join('\n'); + }, + + /** + * Whether something is accepting connections on the gateway port. + * + * This is what "the gateway is running" means operationally, and it is the same evidence + * the diagnostics core already reports as `portListening`. + */ + async gatewayListening(port = 18789, options = {}) { + if (!Number.isInteger(port) || port < 1 || port > 65535) return false; + const probes = [ + ['ss', ['-ltn']], + ['netstat', ['-ltn']], + ['lsof', ['-nP', `-iTCP:${port}`, '-sTCP:LISTEN']], + ]; + for (const [executable, argv] of probes) { + const text = await successfulText(executable, argv, options); + if (!text) continue; + if (executable === 'lsof') return text.trim().length > 0; + // Match ":" at the end of a local address column only. + if (new RegExp(`[\\s:\\[\\]][:.]?${port}\\s`).test(`${text}\n`) || new RegExp(`:${port}\\b`).test(text)) { + return true; + } + return false; + } + return false; }, serviceManagerState, readFileTail, diff --git a/cli/bin/clawfix.js b/cli/bin/clawfix.js index 5953b9f..a9b2c82 100755 --- a/cli/bin/clawfix.js +++ b/cli/bin/clawfix.js @@ -10,7 +10,7 @@ * npx clawfix --tui (force OpenTUI; requires Bun) */ -import { readFileSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; import { execSync, spawn } from 'node:child_process'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -41,9 +41,10 @@ function printHelp() { Usage: npx clawfix [options] Modes: - (default) OpenTUI session UI (falls back to plain session without Bun) + (default) Interactive session. This package ships the plain session; the OpenTUI + chat UI is a separate standalone binary (see below) --plain Classic interactive readline session: scan, review, fix, optional chat - --tui Force the OpenTUI session UI (requires Bun) + --tui Force the OpenTUI session UI (source checkout + Bun only) --scan One-shot scan (legacy mode) --no-interactive Same as --scan @@ -79,8 +80,12 @@ Security: • ClawFix discloses OpenRouter and asks before the first upload (unless --yes) • Source code: https://github.com/arcabotai/clawfix +OpenTUI chat session: + Not bundled in this npm package. Download the standalone binary from + https://github.com/arcabotai/clawfix/releases/latest + Examples: - npx clawfix # OpenTUI session (default; plain fallback without Bun) + npx clawfix # interactive plain session npx clawfix --plain # Classic interactive session npx clawfix --scan # One-shot scan + repair guidance npx clawfix --dry-run # See what data would be collected @@ -88,9 +93,26 @@ Examples: `); } +const TUI_RELEASES_URL = 'https://github.com/arcabotai/clawfix/releases/latest'; + async function runOpenTuiMode({ quiet = false } = {}) { const cliDir = dirname(fileURLToPath(import.meta.url)); const tuiEntry = join(cliDir, '../tui/src/main.tsx'); + const tuiDir = join(cliDir, '../tui'); + + // The npm package ships the portable CLI only — the OpenTUI session is distributed as a + // standalone binary. Detect that here instead of spawning Bun against a missing directory, + // which surfaces as `spawn ENOENT` and wrongly blames a Bun install that is present. + if (!existsSync(tuiEntry)) { + if (quiet) return false; + console.error(c.red('The OpenTUI session is not bundled in the clawfix npm package.')); + console.error(c.dim(`Download the standalone binary: ${TUI_RELEASES_URL}`)); + console.error(c.dim('Or run it from a source checkout: cd cli/tui && bun install && bun run src/main.tsx')); + console.error(c.dim('This session works today with: npx clawfix --plain')); + process.exitCode = 2; + return false; + } + let bunPath = ''; try { bunPath = execSync('command -v bun', { encoding: 'utf8' }).trim(); @@ -106,7 +128,7 @@ async function runOpenTuiMode({ quiet = false } = {}) { const child = spawn(bunPath, [tuiEntry], { stdio: 'inherit', env: process.env, - cwd: join(cliDir, '../tui'), + cwd: tuiDir, }); child.on('error', error => resolve({ error })); child.on('exit', code => resolve({ code: code ?? 1 })); @@ -119,10 +141,9 @@ async function runOpenTuiMode({ quiet = false } = {}) { } return false; } - if (result.code !== 0) { - if (!quiet) process.exitCode = result.code; - return false; - } + // The TUI ran. A non-zero exit is its own outcome to report — it must not be mistaken for + // "never launched", which would drop the user into a second, full plain session. + if (result.code !== 0) process.exitCode = result.code; return true; } diff --git a/cli/core/findings.js b/cli/core/findings.js index 4aa8e02..1cb5809 100644 --- a/cli/core/findings.js +++ b/cli/core/findings.js @@ -1,5 +1,6 @@ -// ClawFix Task 5: normalize local diagnostic issues, native OpenClaw findings, server findings, -// and future AI repair proposals into one frozen Finding contract with stable, explicit identity. +// ClawFix Task 5: normalize local diagnostic issues (including native OpenClaw checks, which +// diagnostics.js folds into localIssues carrying a nativeCheckId), server findings, and future +// AI repair proposals into one frozen Finding contract with stable, explicit identity. // // The rule this module exists to enforce: a repair may only be authorized from an *explicit // reviewed mapping* keyed by a stable local `knownIssueId` or native `checkId` — never from a diff --git a/cli/core/repair-catalog.js b/cli/core/repair-catalog.js index 7509ec2..bb6e6f9 100644 --- a/cli/core/repair-catalog.js +++ b/cli/core/repair-catalog.js @@ -12,14 +12,43 @@ // which itself only ever spawns argv arrays (shell: false). ctx.wait is an injectable delay hook // so tests can drive apply -> verify without real timers. +/** + * Is the gateway actually up? + * + * The answer is the listening port. Two weaker signals were previously treated as proof and + * both were wrong on a real install: + * + * - `pgrep -f 'openclaw.*gateway'` matched ClawFix's own `openclaw gateway status` probe (run + * concurrently with it, right here) and never matched a real gateway, whose argv is just + * `openclaw`. verify() therefore returned ok with nothing listening on the port, which is + * how a repair that did nothing could be reported as applied. + * - the `/running.*pid|state active/i` test against status prose never fired on the real + * `openclaw gateway status` output at all. + * + * Status text and PIDs are still collected, as evidence for the caller — never as the verdict. + */ async function checkGatewayRunning(ctx) { const { openclaw } = ctx; - const [statusText, pid] = await Promise.all([ + const port = Number.isInteger(ctx.gatewayPort) ? ctx.gatewayPort : 18789; + const [statusText, pid, listening] = await Promise.all([ openclaw.gatewayStatusText({ timeoutMs: 5000 }), - openclaw.gatewayProcesses({ timeoutMs: 5000 }), + typeof openclaw.gatewayProcesses === 'function' + ? openclaw.gatewayProcesses({ timeoutMs: 5000 }) + : Promise.resolve(''), + typeof openclaw.gatewayListening === 'function' + ? openclaw.gatewayListening(port, { timeoutMs: 5000 }) + : Promise.resolve(null), ]); - const running = Boolean(pid) || /running.*pid|state active/i.test(statusText || ''); - return Object.freeze({ running, statusText: statusText || '', pid: pid || '' }); + + // Without a port probe, fall back to filtered PIDs rather than claiming knowledge. + const running = listening === null ? Boolean(pid) : Boolean(listening); + return Object.freeze({ + running, + listening: listening === null ? null : Boolean(listening), + port, + statusText: statusText || '', + pid: pid || '', + }); } const gatewayNotRunning = Object.freeze({ diff --git a/cli/core/repair-engine.js b/cli/core/repair-engine.js index 762416b..01e00c8 100644 --- a/cli/core/repair-engine.js +++ b/cli/core/repair-engine.js @@ -14,10 +14,22 @@ import { randomUUID, createHash } from 'node:crypto'; +const REFUSED_RISKS = new Set(['high', 'critical']); +const MAX_RETAINED_PLANS = 256; + function defaultRandomToken() { return randomUUID(); } +/** Rollback is best-effort cleanup — a throw here must never mask the apply/verify outcome. */ +async function safeRollback(entry, ctx, applyResult) { + try { + return await entry.rollback(ctx, { applyResult }); + } catch (error) { + return Object.freeze({ rolledBack: false, note: `rollback failed: ${error.message}` }); + } +} + function stableFingerprintInput(finding, revision) { return JSON.stringify({ revision, @@ -57,6 +69,15 @@ export function createRepairEngine({ catalog = {}, now = () => Date.now(), rando throw new Error(`no catalog entry for repair "${finding.repairId}"`); } + // Consumed plans are dead weight; keep only enough history to keep rejecting replays of + // recent tokens rather than growing for the life of the process. + if (records.size > MAX_RETAINED_PLANS) { + for (const [id, record] of records) { + if (records.size <= MAX_RETAINED_PLANS) break; + if (record.consumed) records.delete(id); + } + } + const planId = randomUUID(); const fingerprint = computeFingerprint(finding, revision); const approvalToken = randomToken(); @@ -112,12 +133,28 @@ export function createRepairEngine({ catalog = {}, now = () => Date.now(), rando const entry = catalog[plan.repairId]; - const preflight = await entry.preflight(ctx); + // High/critical repairs are never executed automatically. This belongs here rather than in + // any one UI: the engine is the only layer every caller has to go through. + if (REFUSED_RISKS.has(String(entry.risk ?? plan.risk).toLowerCase())) { + return Object.freeze({ status: 'blocked', reason: 'risk_refused', plan }); + } + + let preflight; + try { + preflight = await entry.preflight(ctx); + } catch (error) { + return Object.freeze({ status: 'error', error: `preflight failed: ${error.message}`, plan }); + } if (!preflight.ok) { return Object.freeze({ status: 'blocked', reason: preflight.reason, plan, preflight }); } - const preview = await entry.preview(ctx); + let preview = null; + try { + preview = await entry.preview(ctx); + } catch (error) { + return Object.freeze({ status: 'error', error: `preview failed: ${error.message}`, plan }); + } let applyResult; try { @@ -126,9 +163,26 @@ export function createRepairEngine({ catalog = {}, now = () => Date.now(), rando return Object.freeze({ status: 'error', error: error.message, plan, preview }); } - const verify = await entry.verify(ctx); + // Past this point the repair has run. Every remaining failure must still be reported as a + // structured outcome that carries applyResult — throwing here would lose the one fact the + // caller most needs, which is that the system was already changed. + let verify; + try { + verify = await entry.verify(ctx); + } catch (error) { + const rollback = await safeRollback(entry, ctx, applyResult); + return Object.freeze({ + status: 'verify_failed', + plan, + preview, + applyResult, + verify: Object.freeze({ ok: false, error: error.message }), + rollback, + }); + } + if (!verify.ok) { - const rollback = await entry.rollback(ctx, { applyResult }); + const rollback = await safeRollback(entry, ctx, applyResult); return Object.freeze({ status: 'verify_failed', plan, preview, applyResult, verify, rollback }); } diff --git a/cli/core/session.js b/cli/core/session.js index 7e18b4e..a8e2d2a 100644 --- a/cli/core/session.js +++ b/cli/core/session.js @@ -49,6 +49,7 @@ export function createSessionController({ let scanError = null; let transcript = freezeList(); let activeScan = null; + let messageCounter = 0; function getState() { return Object.freeze({ @@ -132,9 +133,10 @@ export function createSessionController({ } else { diagnostic = result?.diagnostic ?? null; issues = freezeList(result?.issues); + // Native OpenClaw checks arrive folded into localIssues with a nativeCheckId — there is + // no separate nativeChecks input, and passing one silently did nothing. findings = freezeList(normalizeFindings({ localIssues: issues, - nativeChecks: result?.nativeChecks, serverFindings: result?.serverFindings, aiFindings: result?.aiFindings, knownRepairIds, @@ -157,6 +159,13 @@ export function createSessionController({ activeScan = null; revision = nextRevision; scanning = false; + // A thrown scan commits the new revision, so the previous revision's results must not + // survive alongside it — otherwise stale findings stay repairable under a revision whose + // scan never produced them. Mirrors the result.error branch above. + diagnostic = null; + issues = freezeList(); + findings = freezeList(); + summary = null; scanError = asError(error); emitSession('session.scan.committed', { revision, @@ -174,7 +183,16 @@ export function createSessionController({ function appendMessage(role, text) { if (!SESSION_ROLES.has(role)) throw new TypeError('role must be user, assistant, or system'); if (typeof text !== 'string' || text.trim().length === 0) throw new TypeError('text must be non-empty'); - const message = Object.freeze({ type: 'session.message', role, text, at: Date.now() }); + // A stable id per message: consumers key rendered rows by it, and without one every + // projection minted fresh keys for the whole transcript on each update. + messageCounter += 1; + const message = Object.freeze({ + type: 'session.message', + id: `msg-${messageCounter}`, + role, + text, + at: Date.now(), + }); transcript = freezeList([...transcript, message]); onEvent(message); return message; diff --git a/cli/package.json b/cli/package.json index 34d6d10..a463fd8 100644 --- a/cli/package.json +++ b/cli/package.json @@ -26,7 +26,7 @@ "author": "Arca (https://arcabot.ai)", "license": "MIT", "engines": { - "node": ">=18.0.0" + "node": ">=22.0.0" }, "files": [ "bin/", diff --git a/cli/tui/scripts/build.ts b/cli/tui/scripts/build.ts index dc31f8d..ab3f89a 100644 --- a/cli/tui/scripts/build.ts +++ b/cli/tui/scripts/build.ts @@ -24,6 +24,8 @@ import { dirname, join, resolve, relative } from "node:path" import { fileURLToPath } from "node:url" import { spawnSync } from "node:child_process" +import { createSolidTransformPlugin } from "@opentui/solid/bun-plugin" + const __dirname = dirname(fileURLToPath(import.meta.url)) const TUI_ROOT = resolve(__dirname, "..") const ENTRY = join(TUI_ROOT, "src", "standalone.ts") @@ -208,21 +210,41 @@ if not defined OTUI_TREE_SITTER_WORKER_PATH set OTUI_TREE_SITTER_WORKER_PATH=%OT writeFileSync(launcherPath, script) } -export function buildTarget(spec: TargetSpec, outdir: string): { binary: string; launcher: string; assets: string } { +/** + * Compile the standalone binary **in this process**, with Solid's JSX transform passed to the + * bundler explicitly. + * + * This cannot go back to `bun build --compile` on the CLI. That transform is registered by + * bunfig.toml's `preload = ["@opentui/solid/preload"]`, which Bun applies to `bun run` and + * `bun test` but not to a spawned bundler process — so the compiled binary was built with the + * plain automatic JSX runtime instead. The result still painted a first frame, which is why + * every existing check passed, but it had no Solid reactivity and no keyboard input at all: + * the shipped binary could not be typed into, answered, or quit. + * + * Verified by driving both builds through a PTY: with the plugin, typed text is echoed and + * Ctrl+D exits; without it, keystrokes produce zero bytes. + */ +async function buildStandaloneBinary(spec: TargetSpec, binary: string): Promise { + console.log(`$ Bun.build --compile --target=${spec.bunCompileTarget} ${ENTRY} (+ solid transform)`) + const result = await Bun.build({ + entrypoints: [ENTRY], + compile: { target: spec.bunCompileTarget, outfile: binary }, + plugins: [createSolidTransformPlugin()], + }) + if (!result.success) { + for (const log of result.logs) console.error(String(log)) + throw new Error(`bun build failed for ${spec.id}`) + } +} + +export async function buildTarget(spec: TargetSpec, outdir: string): Promise<{ binary: string; launcher: string; assets: string }> { mkdirSync(outdir, { recursive: true }) ensureNativePackage(spec) const binary = join(outdir, spec.outfileName) if (existsSync(binary)) rmSync(binary) - const args = ["build", "--compile", `--outfile=${binary}`, `--target=${spec.bunCompileTarget}`, ENTRY] - console.log(`$ bun ${args.join(" ")}`) - const r = spawnSync("bun", args, { - cwd: TUI_ROOT, - stdio: "inherit", - env: { ...process.env, NODE_ENV: "production" }, - }) - if (r.status !== 0) throw new Error(`bun build failed for ${spec.id}`) + await buildStandaloneBinary(spec, binary) if (!existsSync(binary)) throw new Error(`Expected artifact missing: ${binary}`) chmodSync(binary, 0o755) @@ -249,5 +271,5 @@ export function buildTarget(spec: TargetSpec, outdir: string): { binary: string; if (import.meta.main) { const { target, outdir } = parseArgs(process.argv.slice(2)) - buildTarget(TARGETS[target], outdir) + await buildTarget(TARGETS[target], outdir) } diff --git a/cli/tui/scripts/frames.tsx b/cli/tui/scripts/frames.tsx index fbc4668..e7cc3ad 100644 --- a/cli/tui/scripts/frames.tsx +++ b/cli/tui/scripts/frames.tsx @@ -9,16 +9,22 @@ import { mkdir, writeFile } from "node:fs/promises" import { testRender } from "@opentui/solid" import { App, createFakeSession } from "../src/app" +import { buildDisclosureView } from "../src/lib/disclosure" import type { TuiSessionView } from "../src/session-bridge" import type { TranscriptItem, RepairPlanView, DialogState } from "../src/lib/models" const OUT = process.env.CLAWFIX_TUI_FRAMES_DIR || "/tmp/clawfix-tui-frames" +// 66/72/88 sample the band the original sweep skipped, where the footer used to wrap for +// every aiMode with a label longer than "Local only". const SIZES = [ { name: "140x40", width: 140, height: 40 }, { name: "120x35", width: 120, height: 35 }, { name: "100x30", width: 100, height: 30 }, + { name: "88x26", width: 88, height: 26 }, { name: "80x24", width: 80, height: 24 }, + { name: "72x22", width: 72, height: 22 }, + { name: "66x20", width: 66, height: 20 }, { name: "60x20", width: 60, height: 20 }, ] as const @@ -98,15 +104,9 @@ const STATES: Array<{ name: string; view: TuiSessionView }> = [ composerLocked: true, dialog: { type: "privacy", - disclosure: { - destination: "clawfix.dev hosted AI", - baseUrl: "https://clawfix.dev", - endpointUrl: "https://clawfix.dev/api/analyze", - providerLabel: "ClawFix hosted AI (deepseek v4 flash)", - providerChain: ["clawfix.dev"], - included: ["Your message", "Redacted diagnostic summary", "Available repair IDs"], - excluded: ["API keys", "Tokens", "File contents", "Env values", "Home directory paths"], - }, + // Built from the shipping builder — a hand-written disclosure here meant the UX gate + // was reviewing copy (and an endpoint) that no user ever sees. + disclosure: buildDisclosureView({ uploadsDiagnostic: true }), payloadJson: "{\n \"message\": \"why is my gateway not running\"\n}", pendingMessage: "why is my gateway not running", focus: "stay-local", @@ -134,6 +134,28 @@ const STATES: Array<{ name: string; view: TuiSessionView }> = [ helpVisible: true, }), }, + // The footer is `${aiLabel} · ${hints}`, so the longest label is the case that wraps first. + // Sweeping only the default "local" mode hid a footer wrap across the entire 65..91 band. + { + name: "09-consent-pending", + view: state({ + findings, + items: chatItems, + revision: "a1b2c3d", + aiMode: "remote-pending", + status: "Consent required before remote AI", + }), + }, + { + name: "10-remote-consented", + view: state({ + findings, + items: chatItems, + revision: "a1b2c3d", + aiMode: "remote", + status: "Remote AI enabled", + }), + }, ] async function main() { diff --git a/cli/tui/src/app.tsx b/cli/tui/src/app.tsx index 793c239..fcd082f 100644 --- a/cli/tui/src/app.tsx +++ b/cli/tui/src/app.tsx @@ -8,7 +8,7 @@ import { DialogBox } from "./components/dialog-box" import { Sidebar } from "./components/sidebar" import { Splash } from "./components/splash" import { buildUnifiedDiff } from "./components/diff-dialog" -import { helpText } from "./keymap" +import { helpText, resolveGlobalKeyAction, resolveModelLine } from "./keymap" import { resolveLayout, type TranscriptItem } from "./lib/models" import { createFakeSession, @@ -45,6 +45,8 @@ export interface AppProps { readonly session?: TuiSessionView readonly source?: SessionSource readonly simpleComposer?: boolean + /** Tear down the renderer and leave the app. Wired to renderer.destroy() in main.tsx. */ + readonly onQuit?: () => void } const SIDEBAR_WIDTH = 34 @@ -119,14 +121,21 @@ export function App(props: AppProps) { return } - if (name === "p" && key.ctrl) { - controller?.toggleHelp?.() - key.preventDefault?.() - return - } - - if (name === "c" && key.ctrl) { - controller?.cancelScan?.() + const state = view() + switch (resolveGlobalKeyAction(key, { busy: state.busy, scanning: state.scanning })) { + case "toggle-help": + controller?.toggleHelp?.() + key.preventDefault?.() + return + case "quit": + props.onQuit?.() + key.preventDefault?.() + return + case "cancel": + controller?.cancelScan?.() + key.preventDefault?.() + return + default: } }) @@ -161,12 +170,7 @@ export function App(props: AppProps) { ? "Remote (pending consent)" : "Local only" - const modelLine = () => { - const hints = dims().width <= 64 - ? "Enter send · Ctrl+P help" - : "Enter send · Shift+Enter newline · Ctrl+P help · Ctrl+C cancel" - return `${aiLabel()} · ${hints}` - } + const modelLine = () => resolveModelLine(aiLabel(), dims().width) const statusLine = () => { const state = current() @@ -176,7 +180,9 @@ export function App(props: AppProps) { if (budget <= 0) return "" if (full.length <= budget) return full if (budget === 1) return "…" - return `${full.slice(0, budget - 1)}…` + // Slice by code point: the 🦞 in the banner is a surrogate pair, and cutting between its + // halves paints a replacement glyph. + return `${[...full].slice(0, budget - 1).join("")}…` } const handleSubmit = (text: string) => { @@ -227,7 +233,9 @@ export function App(props: AppProps) { diff --git a/cli/tui/src/components/dialog-box.tsx b/cli/tui/src/components/dialog-box.tsx index 361cf5e..9d1466d 100644 --- a/cli/tui/src/components/dialog-box.tsx +++ b/cli/tui/src/components/dialog-box.tsx @@ -6,18 +6,27 @@ function line(text: string, color: string) { } function privacyLines(dialog: Extract) { + // Order matters: visibleDialogLines() truncates from the end of the head, so the lines that + // define what consent actually covers come before the ones that merely restate context. const lines = [ line("Privacy approval", theme.heading), line("ClawFix can send this message and a redacted diagnostic to:", theme.text), line(dialog.disclosure.providerLabel, theme.accent), - line(`Destination: ${dialog.disclosure.destination}`, theme.muted), + // The destination host is already in the endpoint URL; a separate line only costs a row + // that the Included/Not-included disclosure needs on small terminals. line(`Endpoint: ${dialog.disclosure.endpointUrl}`, theme.muted), ] - if (dialog.pendingMessage) { - lines.push(line(`Message: ${dialog.pendingMessage.slice(0, 120)}`, theme.muted)) + if (dialog.disclosure.diagnosticEndpointUrl) { + lines.push(line( + `Uploads the redacted diagnostic first: ${dialog.disclosure.diagnosticEndpointUrl}`, + theme.muted, + )) } lines.push(line(`Included: ${dialog.disclosure.included.join("; ")}`, theme.text)) lines.push(line(`Not included: ${dialog.disclosure.excluded.join("; ")}`, theme.text)) + if (dialog.pendingMessage) { + lines.push(line(`Message: ${dialog.pendingMessage.slice(0, 120)}`, theme.muted)) + } if (dialog.showPayload) { lines.push(line("Exact payload (redacted preview)", theme.heading)) for (const payloadLine of dialog.payloadJson.split("\n").slice(0, 24)) { diff --git a/cli/tui/src/components/privacy-dialog.tsx b/cli/tui/src/components/privacy-dialog.tsx index 5cc344d..442679e 100644 --- a/cli/tui/src/components/privacy-dialog.tsx +++ b/cli/tui/src/components/privacy-dialog.tsx @@ -16,6 +16,12 @@ export function PrivacyDialog(props: { out.push({ text: props.disclosure.providerLabel, color: theme.accent }) out.push({ text: `Destination: ${props.disclosure.destination}`, color: theme.muted }) out.push({ text: `Endpoint: ${props.disclosure.endpointUrl}`, color: theme.muted }) + if (props.disclosure.diagnosticEndpointUrl) { + out.push({ + text: `Also uploads the redacted diagnostic first: ${props.disclosure.diagnosticEndpointUrl}`, + color: theme.muted, + }) + } if (props.pendingMessage) out.push({ text: `Message: ${String(props.pendingMessage).slice(0, 120)}`, color: theme.muted }) out.push({ text: "Included", color: theme.heading }) for (const item of props.disclosure.included) out.push({ text: `• ${item}`, color: theme.text }) diff --git a/cli/tui/src/keymap.tsx b/cli/tui/src/keymap.tsx index c085fb5..7e0f489 100644 --- a/cli/tui/src/keymap.tsx +++ b/cli/tui/src/keymap.tsx @@ -1,17 +1,43 @@ /** App keybinding table + help labels. Handlers are wired in App via useKeyboard. */ +export type GlobalKeyAction = "quit" | "cancel" | "toggle-help" | "close-dialog" | "none" + +/** + * Decide what a global (non-dialog) key press means. + * + * Kept pure and separate from App so it is testable without a renderer — `bun test` consumes + * raw Ctrl+C (0x03) before it reaches the mock keyboard, so the Ctrl+C branch cannot be covered + * by an integration test. + */ +export function resolveGlobalKeyAction( + key: { readonly name?: string; readonly ctrl?: boolean } | null | undefined, + state: { readonly busy?: boolean; readonly scanning?: boolean } = {}, +): GlobalKeyAction { + const name = String(key?.name || "").toLowerCase() + if (name === "escape") return "close-dialog" + if (!key?.ctrl) return "none" + if (name === "p") return "toggle-help" + if (name === "d") return "quit" + // Ctrl+C interrupts work when there is work; otherwise it is the conventional way out. + // Without the quit fallback the renderer owns stdin in raw mode, so Ctrl+C never reaches + // the process as SIGINT and the session cannot be exited at all. + if (name === "c") return (state.busy || state.scanning) ? "cancel" : "quit" + return "none" +} + export const KEY_HINTS = Object.freeze([ "Enter send", "Shift+Enter newline", "Ctrl+P help", "Ctrl+C cancel", + "Ctrl+D quit", "Esc close dialog", ] as const) export const NARROW_KEY_HINTS = Object.freeze([ "Enter send", "Ctrl+P help", - "Ctrl+C cancel", + "Ctrl+D quit", ] as const) export const COMPOSER_KEY_BINDINGS = Object.freeze([ @@ -23,13 +49,41 @@ export const COMPOSER_KEY_BINDINGS = Object.freeze([ { name: "linefeed", action: "newline" as const }, ]) +/** + * Footer hint tiers, longest first. resolveModelLine picks the longest that fits. + * + * A fixed width threshold cannot work here: the line is `${aiLabel} · ${hints}`, and aiLabel + * ranges from "Local only" (10) to "Remote (pending consent)" (24), so any single cutoff wraps + * for some mode. The previous `width <= 64` check wrapped the footer for every width from 65 to + * 91 — the whole 80-column band — and ate a transcript row. + */ +export const HINT_TIERS = Object.freeze([ + "Enter send · Shift+Enter newline · Ctrl+P help · Ctrl+C cancel · Ctrl+D quit", + "Enter send · Ctrl+P help · Ctrl+C cancel · Ctrl+D quit", + "Enter send · Ctrl+P help · Ctrl+D quit", + "Enter send · Ctrl+D quit", + "Ctrl+D quit", +] as const) + +/** Build the footer line for `aiLabel` that fits inside a terminal `width` (2 cols of padding). */ +export function resolveModelLine(aiLabel: string, width: number): string { + const label = String(aiLabel || "") + const budget = Math.max(0, Math.floor(width || 0) - 2) + for (const hints of HINT_TIERS) { + const line = `${label} · ${hints}` + if (line.length <= budget) return line + } + // Nothing fits: return the label alone, truncated, rather than wrapping the footer. + return label.length <= budget ? label : label.slice(0, Math.max(0, budget)) +} + export function helpText(compact = false): string { if (compact) { return [ "ClawFix keys", " Enter send · Ctrl+J newline", " Ctrl+P help · Ctrl+C cancel", - " Esc close · Tab/Arrows move", + " Ctrl+D quit · Esc close · Tab/Arrows move", " Local: help, issues, scan, explain <#|id>, fix <#|id>", " Remote AI requires explicit privacy consent.", ].join("\n") @@ -40,7 +94,8 @@ export function helpText(compact = false): string { " Shift+Enter Newline (when terminal supports modifiers)", " Ctrl+J Newline fallback", " Ctrl+P Toggle this help", - " Ctrl+C Cancel active scan/remote request", + " Ctrl+C Cancel active scan/remote request (quits when idle)", + " Ctrl+D Quit ClawFix", " Esc Close dialog / return focus to composer", " Tab/Arrows Move dialog focus (default never on destructive action)", "", diff --git a/cli/tui/src/lib/disclosure.ts b/cli/tui/src/lib/disclosure.ts index 05d3894..c795fa8 100644 --- a/cli/tui/src/lib/disclosure.ts +++ b/cli/tui/src/lib/disclosure.ts @@ -30,6 +30,8 @@ export function buildDisclosureView(input: { readonly providerChain?: readonly string[] readonly included?: readonly string[] readonly excluded?: readonly string[] + /** True when this turn will also POST the redacted diagnostic to /api/diagnose. */ + readonly uploadsDiagnostic?: boolean } = {}): DisclosureView { const raw = (input.baseUrl || "https://clawfix.dev").trim() || "https://clawfix.dev" let url: URL @@ -47,13 +49,23 @@ export function buildDisclosureView(input: { ? [`Custom ClawFix server (${hostname})`, ...chain.filter((p) => !/clawfix service/i.test(p))] : chain + // Agreeing to a remote turn also uploads the redacted diagnostic, as a separate request to a + // separate endpoint. Say so up front rather than describing it as something that merely might + // already be "linked". + const included = input.included + ? [...input.included] + : input.uploadsDiagnostic + ? ["Your message", ...DEFAULT_INCLUDED.slice(1).map((line) => line.replace(" (when present on a linked diagnostic)", ""))] + : [...DEFAULT_INCLUDED] + return Object.freeze({ destination: hostname, baseUrl: url.origin, endpointUrl: `${url.origin}/api/v2/agent/messages`, + diagnosticEndpointUrl: input.uploadsDiagnostic ? `${url.origin}/api/diagnose` : null, providerLabel: effectiveChain.join(" → "), providerChain: Object.freeze(effectiveChain), - included: Object.freeze([...(input.included || DEFAULT_INCLUDED)].map(String)), + included: Object.freeze(included.map(String)), excluded: Object.freeze([...(input.excluded || DEFAULT_EXCLUDED)].map(String)), }) } diff --git a/cli/tui/src/lib/models.ts b/cli/tui/src/lib/models.ts index d3dc374..ee97c2b 100644 --- a/cli/tui/src/lib/models.ts +++ b/cli/tui/src/lib/models.ts @@ -10,6 +10,8 @@ export interface DisclosureView { readonly providerChain: readonly string[] readonly included: readonly string[] readonly excluded: readonly string[] + /** Set when this turn will also upload the redacted diagnostic to /api/diagnose. */ + readonly diagnosticEndpointUrl: string | null } export interface RepairEffectView { diff --git a/cli/tui/src/live-session.ts b/cli/tui/src/live-session.ts index 24f2e48..ca7485f 100644 --- a/cli/tui/src/live-session.ts +++ b/cli/tui/src/live-session.ts @@ -112,7 +112,10 @@ export function createLiveSession(options: LiveSessionOptions): SessionBridge { }) if (options.autoScan !== false) { - void bridge.scan() + // A rejected autoscan must not reach process-level `unhandledRejection`, which main.tsx + // treats as fatal and tears the renderer down. The controller has already committed the + // scan error into session state, so the UI renders it as a normal scan failure. + void bridge.scan().catch(() => {}) } return bridge } diff --git a/cli/tui/src/main.tsx b/cli/tui/src/main.tsx index 09357fc..7620c3c 100644 --- a/cli/tui/src/main.tsx +++ b/cli/tui/src/main.tsx @@ -121,8 +121,8 @@ export function startTui(input?: TuiSessionView | SessionSource | SessionBridge) createRenderer: () => createCliRenderer({ ...rendererConfig }), mount: renderer => render( () => isSource - ? - : , + ? renderer.destroy()} /> + : renderer.destroy()} />, renderer, ), run: waitForRendererExit, diff --git a/cli/tui/src/remote-analyzer.ts b/cli/tui/src/remote-analyzer.ts index 1d8ffa1..a01fbc3 100644 --- a/cli/tui/src/remote-analyzer.ts +++ b/cli/tui/src/remote-analyzer.ts @@ -128,6 +128,29 @@ export function createRemoteAnalyzer(options: RemoteAnalyzerOptions) { conversationId, baseUrl, endpointUrl: `${baseUrl}/api/v2/agent/messages`, + + /** + * Describe exactly what send() would transmit for `message`, for the consent dialog. + * Built from the same conversationId and availableRepairs() the request uses, so the + * "Inspect exact payload" view cannot drift from the real body. + */ + describeOutbound(message: string) { + const state = options.session.getState() + const uploadsDiagnostic = Boolean(state.diagnostic) && !diagnosticId + return Object.freeze({ + uploadsDiagnostic, + diagnosticEndpointUrl: uploadsDiagnostic ? `${baseUrl}/api/diagnose` : null, + payload: { + ...(uploadsDiagnostic + ? { ">> first, POST /api/diagnose": "the redacted diagnostic below is uploaded and returns the diagnosticId used here" } + : {}), + conversationId, + message: String(message || "").slice(0, 4000), + ...(diagnosticId ? { diagnosticId } : {}), + availableRepairs: availableRepairs(), + }, + }) + }, async *send(input: { readonly message: string; readonly consentGranted: boolean; readonly signal?: AbortSignal }) { const signal = input.signal let diagId: string | null = null diff --git a/cli/tui/src/session-bridge.ts b/cli/tui/src/session-bridge.ts index b8f371b..b84983f 100644 --- a/cli/tui/src/session-bridge.ts +++ b/cli/tui/src/session-bridge.ts @@ -81,6 +81,12 @@ export interface OfflineAnalyzerLike { /** Optional remote analyzer — never called without consent. */ export interface RemoteAnalyzerLike { + /** Exact description of what send() would transmit, for the consent dialog. */ + describeOutbound?(message: string): { + readonly uploadsDiagnostic?: boolean + readonly diagnosticEndpointUrl?: string | null + readonly payload?: unknown + } send(input: { readonly message: string readonly consentGranted: boolean @@ -349,7 +355,13 @@ export function createSessionBridge(options: { } function openPrivacyDialog(message: string) { - const payload = { + // Prefer the analyzer's own description: the dialog must show what will actually be sent, + // including the separate /api/diagnose upload that a remote turn performs first. + const described = typeof options.remoteAnalyzer?.describeOutbound === "function" + ? options.remoteAnalyzer.describeOutbound(message) + : null + + const payload = described?.payload ?? { conversationId: "pending-session", message, diagnosticId: null, @@ -358,7 +370,10 @@ export function createSessionBridge(options: { .slice(0, 32) .map((f) => ({ id: f.repairId, title: f.title, risk: "medium" })), } - disclosureCache = buildDisclosureView({ baseUrl: options.remoteBaseUrl }) + disclosureCache = buildDisclosureView({ + baseUrl: options.remoteBaseUrl, + uploadsDiagnostic: Boolean(described?.uploadsDiagnostic), + }) dialog = Object.freeze({ type: "privacy", disclosure: disclosureCache, @@ -372,6 +387,73 @@ export function createSessionBridge(options: { publish() } + /** + * Resolve a server-proposed repair ID against the *local* catalog. + * + * The hosted agent only ever sends `repair.proposed { repairId, rationale }` + * (src/routes/agent-v2.js). It never sends a plan, and a server-authored plan must never + * reach the approval dialog: the planId, approval token, risk, and preview shown to the + * user all have to come from the local repair engine. So we map the id onto a repairable + * finding from the current revision and ask the session controller to build the plan. + */ + function resolveRemoteRepairProposal(repairId: string, rationale: string) { + const id = String(repairId || "").trim() + if (!id) return + + const raw = session.getState().findings + const current = (Array.isArray(raw) ? raw : []) + .map(asFinding) + .filter((f): f is TuiFinding => f !== null) + const finding = current.find((f) => f.repairable && f.repairId === id) + if (!finding) { + pushExtra(Object.freeze({ + kind: "error", + id: nextId("err"), + message: `ClawFix suggested the repair "${sanitizeDisplayText(id, 128)}", but no matching repairable finding exists in the current scan. Nothing was prepared. Try "scan" and ask again.`, + })) + return + } + + if (typeof session.proposeRepair !== "function") { + pushExtra(Object.freeze({ + kind: "error", + id: nextId("err"), + message: "Repair proposals are unavailable in this session.", + })) + return + } + + const proposal = session.proposeRepair(finding.id) + if (proposal?.status !== "proposed" || !proposal.plan) { + pushExtra(Object.freeze({ + kind: "error", + id: nextId("err"), + message: `Could not prepare the repair for "${finding.title}" (${sanitizeDisplayText(String(proposal?.status || "unknown"), 64)}).`, + })) + return + } + + const plan = planFromRaw(proposal.plan) + if (!plan) { + pushExtra(Object.freeze({ + kind: "error", + id: nextId("err"), + message: `The local repair plan for "${finding.title}" was malformed and was not offered.`, + })) + return + } + + const why = sanitizeDisplayText(rationale, 800) || plan.summary + pushExtra(Object.freeze({ + kind: "repair", + id: nextId("repair"), + plan, + rationale: why, + status: "proposed" as const, + })) + openApprovalDialog(plan, why) + } + function openApprovalDialog(plan: RepairPlanView, rationale: string) { dialog = Object.freeze({ type: "approval", @@ -465,17 +547,8 @@ export function createSessionBridge(options: { })) publish() } else if (type === "repair.proposed") { - const plan = planFromRaw(event.plan || event) - if (plan) { - pushExtra(Object.freeze({ - kind: "repair", - id: nextId("repair"), - plan, - rationale: sanitizeDisplayText(String(event.rationale || ""), 800), - status: "proposed" as const, - })) - openApprovalDialog(plan, String(event.rationale || plan.summary)) - } + resolveRemoteRepairProposal(String(event.repairId || ""), String(event.rationale || "")) + publish() } else if (type === "agent.error") { pushExtra(Object.freeze({ kind: "error", @@ -494,8 +567,7 @@ export function createSessionBridge(options: { if (message) session.appendMessage?.("assistant", sanitizeDisplayText(message)) for (const event of Array.isArray(result?.events) ? result.events : []) { if (event?.type === "repair.proposed") { - const plan = planFromRaw(event.plan || event) - if (plan) openApprovalDialog(plan, String(event.rationale || plan.summary)) + resolveRemoteRepairProposal(String(event.repairId || ""), String(event.rationale || "")) } } } diff --git a/cli/tui/test/app.test.tsx b/cli/tui/test/app.test.tsx index 67034ff..4f6b674 100644 --- a/cli/tui/test/app.test.tsx +++ b/cli/tui/test/app.test.tsx @@ -32,16 +32,22 @@ describe("OpenTUI package compatibility", () => { new URL(`../node_modules/${name}/package.json`, import.meta.url), "utf8", ).then(JSON.parse) + // Only the host's native package is installed, so read that one — hardcoding linux-x64 + // made this test unrunnable anywhere but Linux. The linux-x64 release pin is still + // asserted below, from metadata rather than from disk. + const hostNative = `@opentui/core-${process.platform}-${process.arch}` const [core, solid, keymap, native] = await Promise.all([ packageJson("@opentui/core"), packageJson("@opentui/solid"), packageJson("@opentui/keymap"), - packageJson("@opentui/core-linux-x64"), + packageJson(hostNative), ]) expect(core.version).toBe("0.4.5") expect(core.peerDependencies).toEqual({ "web-tree-sitter": "0.25.10" }) - expect(core.optionalDependencies["@opentui/core-linux-x64"]).toBe(native.version) + expect(core.optionalDependencies[hostNative]).toBe(native.version) + // The release build targets linux-x64 regardless of the host it is built on. + expect(core.optionalDependencies["@opentui/core-linux-x64"]).toBe("0.4.5") expect(solid.version).toBe("0.4.5") expect(solid.dependencies["@opentui/core"]).toBe(core.version) expect(solid.peerDependencies).toEqual({ "solid-js": "1.9.12" }) diff --git a/cli/tui/test/consent-disclosure.test.ts b/cli/tui/test/consent-disclosure.test.ts new file mode 100644 index 0000000..db628f7 --- /dev/null +++ b/cli/tui/test/consent-disclosure.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, test } from "bun:test" + +import { buildDisclosureView } from "../src/lib/disclosure" +import { createRemoteAnalyzer } from "../src/remote-analyzer" +import { createSessionBridge } from "../src/session-bridge" + +const DIAGNOSTIC = Object.freeze({ os: "linux", ocVersion: "1.2.3" }) + +const FINDING = Object.freeze({ + id: "clawfix:gateway-is-not-running", + title: "Gateway is not running", + severity: "critical", + repairable: true, + repairId: "gateway-not-running", +}) + +function fakeSession(diagnostic: unknown = DIAGNOSTIC) { + return { + getState: () => Object.freeze({ + revision: "rev-1", + diagnostic, + issues: [], + findings: [FINDING], + scanning: false, + scanError: null, + transcript: [], + }), + scan: async () => ({}), + appendMessage: () => {}, + } +} + +describe("consent disclosure describes the real outbound traffic", () => { + test("the dialog discloses the diagnostic upload as a separate endpoint", () => { + const analyzer = createRemoteAnalyzer({ session: fakeSession() as any }) + const bridge = createSessionBridge({ + session: fakeSession() as any, + remoteAnalyzer: analyzer as any, + preferRemote: true, + }) + + bridge._testOpenPrivacy("why is my gateway down") + const dialog = bridge.getView().dialog as any + + expect(dialog.type).toBe("privacy") + expect(dialog.disclosure.diagnosticEndpointUrl).toBe("https://clawfix.dev/api/diagnose") + expect(dialog.disclosure.endpointUrl).toBe("https://clawfix.dev/api/v2/agent/messages") + // Focus still defaults to the non-uploading choice. + expect(dialog.focus).toBe("stay-local") + }) + + test("the payload preview is the real request body, not a placeholder", () => { + const analyzer = createRemoteAnalyzer({ session: fakeSession() as any }) + const bridge = createSessionBridge({ + session: fakeSession() as any, + remoteAnalyzer: analyzer as any, + preferRemote: true, + }) + + bridge._testOpenPrivacy("why is my gateway down") + const preview = JSON.parse( + (bridge.getView().dialog as any).payloadJson.replace(/^\{\n\s*">> first[^\n]*\n/, "{\n"), + ) + + // Real conversation id, not "pending-session". + expect(preview.conversationId).toBe(analyzer.conversationId) + expect(preview.conversationId).not.toBe("pending-session") + expect(preview.message).toBe("why is my gateway down") + expect(preview.availableRepairs).toEqual([ + { id: "gateway-not-running", title: "Gateway is not running", risk: "medium" }, + ]) + }) + + test("the payload preview names the diagnostic upload that happens first", () => { + const analyzer = createRemoteAnalyzer({ session: fakeSession() as any }) + const bridge = createSessionBridge({ + session: fakeSession() as any, + remoteAnalyzer: analyzer as any, + preferRemote: true, + }) + + bridge._testOpenPrivacy("hello") + expect((bridge.getView().dialog as any).payloadJson).toContain("POST /api/diagnose") + }) + + test("no diagnostic, no diagnostic-upload disclosure", () => { + const analyzer = createRemoteAnalyzer({ session: fakeSession(null) as any }) + const bridge = createSessionBridge({ + session: fakeSession(null) as any, + remoteAnalyzer: analyzer as any, + preferRemote: true, + }) + + bridge._testOpenPrivacy("hello") + const dialog = bridge.getView().dialog as any + expect(dialog.disclosure.diagnosticEndpointUrl).toBeNull() + expect(dialog.payloadJson).not.toContain("POST /api/diagnose") + }) + + test("the included list drops the 'when present on a linked diagnostic' hedge on upload turns", () => { + const hedged = buildDisclosureView({}) + const direct = buildDisclosureView({ uploadsDiagnostic: true }) + + expect(hedged.included.join(" ")).toContain("when present on a linked diagnostic") + expect(direct.included.join(" ")).not.toContain("when present on a linked diagnostic") + expect(direct.included.join(" ")).toContain("OS and OpenClaw versions") + }) +}) diff --git a/cli/tui/test/disclosure-parity.test.ts b/cli/tui/test/disclosure-parity.test.ts new file mode 100644 index 0000000..56262c5 --- /dev/null +++ b/cli/tui/test/disclosure-parity.test.ts @@ -0,0 +1,48 @@ +// @ts-nocheck — imports plain-JS cli/core modules without type declarations (TS7016). +import { describe, expect, test } from "bun:test" + +import { + DEFAULT_EXCLUDED_FIELDS, + DEFAULT_INCLUDED_FIELDS, + DEFAULT_PROVIDER_CHAIN, + buildDisclosure, +} from "../../core/privacy.js" +import { + DEFAULT_EXCLUDED, + DEFAULT_INCLUDED, + DEFAULT_PROVIDER_CHAIN as TUI_PROVIDER_CHAIN, + buildDisclosureView, +} from "../src/lib/disclosure" + +/** + * lib/disclosure.ts mirrors cli/core/privacy.js so the TUI package stays self-contained. + * Two copies of the text a user consents to is exactly the kind of thing that drifts + * silently, so assert they agree. + */ +describe("disclosure parity with cli/core/privacy.js", () => { + test("the included, excluded and provider lists match", () => { + expect([...DEFAULT_INCLUDED]).toEqual([...DEFAULT_INCLUDED_FIELDS]) + expect([...DEFAULT_EXCLUDED]).toEqual([...DEFAULT_EXCLUDED_FIELDS]) + expect([...TUI_PROVIDER_CHAIN]).toEqual([...DEFAULT_PROVIDER_CHAIN]) + }) + + test("both builders describe the same destination and endpoint", () => { + const core = buildDisclosure({ baseUrl: "https://clawfix.dev" }) + const tui = buildDisclosureView({ baseUrl: "https://clawfix.dev" }) + + expect(tui.destination).toBe(core.destination) + expect(tui.baseUrl).toBe(core.baseUrl) + expect(tui.endpointUrl).toBe(core.endpointUrl) + expect(tui.providerLabel).toBe(core.providerLabel) + }) + + test("both builders treat a custom server the same way", () => { + const core = buildDisclosure({ baseUrl: "https://fix.internal.example" }) + const tui = buildDisclosureView({ baseUrl: "https://fix.internal.example" }) + + expect(tui.providerLabel).toBe(core.providerLabel) + expect(tui.providerLabel).toContain("fix.internal.example") + // A custom host must not be described as going through the hosted ClawFix service. + expect(tui.providerLabel).not.toContain("ClawFix service") + }) +}) diff --git a/cli/tui/test/e2e-session.test.ts b/cli/tui/test/e2e-session.test.ts new file mode 100644 index 0000000..1e84f44 --- /dev/null +++ b/cli/tui/test/e2e-session.test.ts @@ -0,0 +1,284 @@ +// @ts-nocheck — imports plain-JS cli/ and src/ modules without type declarations (TS7016). +/** + * Scripted end-to-end session: scan -> consent -> remote chat -> repair proposal -> + * approval -> apply -> verify. + * + * Everything on the client is real: the session controller, findings normalization, the + * repair catalog and engine, the offline analyzer, the SSE client, and the TUI bridge. + * + * The server is a local HTTP server that validates request bodies with the *real* + * validateAgentV2Request and writes frames with the *real* formatSseEvent, so the wire + * contract cannot drift from src/routes/agent-v2.js without this failing. A live server is + * not used because emitting repair.proposed requires a configured AI provider. + * + * Only the OpenClaw process boundary is faked — that is the one thing a test must not + * actually do. + */ +import { afterAll, beforeAll, describe, expect, test } from "bun:test" +import { createServer, type Server } from "node:http" + +import { normalizeFindings } from "../../core/findings.js" +import { repairCatalog } from "../../core/repair-catalog.js" +import { createRepairEngine } from "../../core/repair-engine.js" +import { createSessionController } from "../../core/session.js" +import { createOfflineAnalyzer } from "../../core/offline-analyzer.js" +import { formatSseEvent, validateAgentV2Request } from "../../../src/agent/contract.js" +import { createRemoteAnalyzer } from "../src/remote-analyzer" +import { createSessionBridge } from "../src/session-bridge" + +interface Recorded { + readonly path: string + readonly body: any +} + +let server: Server +let baseUrl = "" +const requests: Recorded[] = [] +let rejectAgentBody: string | null = null + +beforeAll(async () => { + server = createServer((req, res) => { + let raw = "" + req.on("data", (chunk) => { raw += chunk }) + req.on("end", () => { + const body = raw ? JSON.parse(raw) : null + requests.push({ path: req.url || "", body }) + + if (req.url === "/api/diagnose") { + res.setHeader("content-type", "application/json") + res.end(JSON.stringify({ fixId: "diag12345678" })) + return + } + + if (req.url === "/api/v2/agent/messages") { + // The real server-side validator: if the client's payload would be rejected in + // production, it is rejected here too. + const validated = validateAgentV2Request(body) + if (!validated.ok) { + rejectAgentBody = validated.error + res.statusCode = 400 + res.end(JSON.stringify({ error: validated.error })) + return + } + + res.writeHead(200, { + "content-type": "text/event-stream", + "cache-control": "no-cache", + connection: "keep-alive", + }) + res.write(formatSseEvent("agent.meta", { + conversationId: validated.value.conversationId, + diagnosticId: validated.value.diagnosticId, + protocol: "clawfix.agent.v2", + })) + res.write(formatSseEvent("assistant.delta", { text: "Your gateway is not running. " })) + res.write(formatSseEvent("assistant.delta", { text: "I can restart it." })) + // Exactly what src/routes/agent-v2.js emits: an id and a rationale, never a plan. + res.write(formatSseEvent("repair.proposed", { + repairId: "gateway-not-running", + rationale: "The gateway process is absent and the port is closed.", + })) + res.write(formatSseEvent("agent.done", { conversationId: validated.value.conversationId, repairProposed: true })) + res.end() + return + } + + res.statusCode = 404 + res.end("{}") + }) + }) + + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)) + const address = server.address() as { port: number } + baseUrl = `http://127.0.0.1:${address.port}` +}) + +afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())) +}) + +/** OpenClaw process boundary: down until `gateway restart` is invoked, up afterwards. */ +function fakeOpenClaw() { + const invocations: string[][] = [] + let running = false + return { + invocations, + isRunning: () => running, + adapter: { + async gatewayStatusText() { return running ? "running (pid 4242)" : "inactive (dead)" }, + async gatewayProcesses() { return running ? "4242" : "" }, + async invoke(argv: string[]) { + invocations.push(argv) + if (argv[0] === "gateway" && argv[1] === "restart") running = true + return { status: 0, timedOut: false, errorSummary: "", stdout: "restarted" } + }, + }, + } +} + +function buildSession(openclaw: ReturnType) { + let revisionCounter = 0 + return createSessionController({ + runDiagnostics: async ({ revision }) => ({ + revision, + diagnostic: { revision, os: "linux", ocVersion: "1.2.3" }, + issues: openclaw.isRunning() ? [] : [{ severity: "critical", text: "Gateway is not running" }], + summary: { gateway: { running: openclaw.isRunning() } }, + }), + repairEngine: createRepairEngine({ catalog: repairCatalog }), + normalizeFindings, + knownRepairIds: Object.keys(repairCatalog), + makeRevisionId: () => `rev-${(revisionCounter += 1)}`, + onEvent: () => {}, + }) +} + +describe("end-to-end session", () => { + test("scan -> consent -> remote chat -> proposal -> approval -> apply -> verify", async () => { + requests.length = 0 + rejectAgentBody = null + + const openclaw = fakeOpenClaw() + const session = buildSession(openclaw) + const bridge = createSessionBridge({ + session: session as any, + offlineAnalyzer: createOfflineAnalyzer({ session }) as any, + remoteAnalyzer: createRemoteAnalyzer({ session: session as any, baseUrl }) as any, + preferRemote: true, + remoteBaseUrl: baseUrl, + repairContext: { + openclaw: openclaw.adapter, + // No real timers: apply -> verify must be drivable deterministically. + wait: async () => {}, + }, + }) + + // 1. Scan finds a repairable gateway failure. + await bridge.scan() + const scanned = bridge.getView() + expect(scanned.findings).toHaveLength(1) + expect(scanned.findings[0]!.repairable).toBe(true) + expect(scanned.findings[0]!.repairId).toBe("gateway-not-running") + + // 2. A deterministic local command must not touch the network. + await bridge.send("issues") + expect(requests).toHaveLength(0) + + // 3. Free text opens the privacy dialog and still sends nothing. + await bridge.send("why is my gateway not running") + expect(bridge.getView().dialog.type).toBe("privacy") + expect((bridge.getView().dialog as any).focus).toBe("stay-local") + expect(requests).toHaveLength(0) + + // 4. Consent released the turn: diagnostic upload, then the agent call. + bridge.privacySetFocus("continue") + await bridge.privacyConfirm() + + expect(rejectAgentBody).toBeNull() + expect(requests.map((r) => r.path)).toEqual(["/api/diagnose", "/api/v2/agent/messages"]) + const agentBody = requests[1]!.body + expect(agentBody.diagnosticId).toBe("diag12345678") + expect(agentBody.availableRepairs).toEqual([ + { id: "gateway-not-running", title: "Gateway is not running", risk: "medium" }, + ]) + for (const banned of ["shell", "command", "script", "patch", "exec", "commands", "files"]) { + expect(Object.prototype.hasOwnProperty.call(agentBody, banned)).toBe(false) + } + + // 5. The server's repairId became a locally-built plan and an approval dialog. + const proposed = bridge.getView() + expect(proposed.dialog.type).toBe("approval") + const dialog = proposed.dialog as any + expect(dialog.focus).toBe("cancel") + expect(dialog.plan.findingId).toBe(scanned.findings[0]!.id) + expect(typeof dialog.plan.approvalToken).toBe("string") + expect(dialog.plan.approvalToken.length).toBeGreaterThan(0) + expect(dialog.plan.risk).toBe("low") + + // Nothing has run yet. + expect(openclaw.invocations).toEqual([]) + + // 6. Approve: plan -> apply -> verify, through the real engine and catalog. + bridge.approvalSetFocus("approve") + await bridge.approvalConfirm() + + expect(openclaw.invocations).toEqual([["gateway", "restart"]]) + expect(openclaw.isRunning()).toBe(true) + + const done = bridge.getView() + expect(done.dialog.type).toBe("none") + const repairCards = done.items.filter((i: any) => i.kind === "repair") + expect(repairCards).toHaveLength(1) + expect((repairCards[0] as any).status).toBe("completed") + expect(done.items.some((i: any) => i.kind === "error")).toBe(false) + + // Success copy exists only because applyRepair returned `applied`. + const messages = done.items.filter((i: any) => i.kind === "message" && i.role === "assistant") + expect(messages.some((m: any) => /Repair applied/.test(m.text))).toBe(true) + + // 7. A rescan confirms the world actually changed. + await bridge.scan() + expect(bridge.getView().findings).toHaveLength(0) + }) + + test("declining consent keeps the turn local and uploads nothing", async () => { + requests.length = 0 + + const openclaw = fakeOpenClaw() + const session = buildSession(openclaw) + const bridge = createSessionBridge({ + session: session as any, + offlineAnalyzer: createOfflineAnalyzer({ session }) as any, + remoteAnalyzer: createRemoteAnalyzer({ session: session as any, baseUrl }) as any, + preferRemote: true, + remoteBaseUrl: baseUrl, + repairContext: { openclaw: openclaw.adapter, wait: async () => {} }, + }) + + await bridge.scan() + await bridge.send("why is my gateway not running") + expect(bridge.getView().dialog.type).toBe("privacy") + + // Stay local is the default action; confirming it must not reach the network. + await bridge.privacyConfirm() + + expect(requests).toHaveLength(0) + const view = bridge.getView() + expect(view.dialog.type).toBe("none") + expect(view.aiMode).toBe("local") + expect(view.remoteConsent).toBe(false) + // The offline analyzer answered instead, deterministically. + expect(view.messages.some((m) => /Unknown local command/.test(m))).toBe(true) + }) + + test("a repair the local catalog does not know is refused, not executed", async () => { + requests.length = 0 + + const openclaw = fakeOpenClaw() + const session = buildSession(openclaw) + const bridge = createSessionBridge({ + session: session as any, + offlineAnalyzer: createOfflineAnalyzer({ session }) as any, + remoteAnalyzer: { + async *send() { + yield { type: "repair.proposed", repairId: "rm-rf-slash", rationale: "trust me" } + }, + } as any, + preferRemote: true, + remoteBaseUrl: baseUrl, + repairContext: { openclaw: openclaw.adapter, wait: async () => {} }, + }) + + await bridge.scan() + bridge._testOpenPrivacy("do the thing") + bridge.privacySetFocus("continue") + await bridge.privacyConfirm() + + const view = bridge.getView() + expect(view.dialog.type).toBe("none") + expect(openclaw.invocations).toEqual([]) + const errors = view.items.filter((i: any) => i.kind === "error") + expect(errors).toHaveLength(1) + expect((errors[0] as any).message).toContain("no matching repairable finding") + }) +}) diff --git a/cli/tui/test/footer.test.ts b/cli/tui/test/footer.test.ts new file mode 100644 index 0000000..ffb8a46 --- /dev/null +++ b/cli/tui/test/footer.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test" + +import { HINT_TIERS, resolveModelLine } from "../src/keymap" + +const LABELS = ["Local only", "Remote · clawfix.dev", "Remote (pending consent)"] + +describe("footer hint line", () => { + test("never exceeds the content width for any label at any width", () => { + for (const label of LABELS) { + for (let width = 20; width <= 200; width += 1) { + const line = resolveModelLine(label, width) + expect(line.length).toBeLessThanOrEqual(Math.max(0, width - 2)) + } + } + }) + + test("the 65..91 band that used to wrap now keeps the full label and a quit hint", () => { + for (let width = 65; width <= 91; width += 1) { + const line = resolveModelLine("Remote (pending consent)", width) + expect(line.length).toBeLessThanOrEqual(width - 2) + expect(line).toContain("Remote (pending consent)") + expect(line).toContain("Ctrl+D quit") + } + }) + + test("wide terminals get the richest tier", () => { + expect(resolveModelLine("Local only", 140)).toBe(`Local only · ${HINT_TIERS[0]}`) + }) + + test("narrow terminals degrade instead of overflowing", () => { + const narrow = resolveModelLine("Remote (pending consent)", 40) + expect(narrow.length).toBeLessThanOrEqual(38) + expect(narrow).toContain("Ctrl+D quit") + }) + + test("an impossibly narrow terminal truncates rather than wrapping", () => { + expect(resolveModelLine("Local only", 6).length).toBeLessThanOrEqual(4) + expect(resolveModelLine("Local only", 0)).toBe("") + }) + + test("every tier advertises the quit key", () => { + for (const tier of HINT_TIERS) expect(tier).toContain("Ctrl+D quit") + }) +}) diff --git a/cli/tui/test/new-layout.test.tsx b/cli/tui/test/new-layout.test.tsx index a8c973f..cf092e1 100644 --- a/cli/tui/test/new-layout.test.tsx +++ b/cli/tui/test/new-layout.test.tsx @@ -4,6 +4,7 @@ import { testRender } from "@opentui/solid" import { App, createFakeSession } from "../src/app" import type { TuiSessionView } from "../src/session-bridge" import type { RepairPlanView, DialogState } from "../src/lib/models" +import { buildDisclosureView } from "../src/lib/disclosure" const renderers: Array<{ destroy(): void }> = [] afterEach(() => { @@ -82,15 +83,8 @@ describe("new layout", () => { composerLocked: true, dialog: { type: "privacy", - disclosure: { - destination: "clawfix.dev hosted AI", - baseUrl: "https://clawfix.dev", - endpointUrl: "https://clawfix.dev/api/analyze", - providerLabel: "ClawFix hosted AI (deepseek v4 flash)", - providerChain: ["clawfix.dev"], - included: ["Your message", "Redacted diagnostic summary"], - excluded: ["API keys", "Tokens", "File contents"], - }, + // Built from the real builder so the dialog under test renders shipping copy. + disclosure: buildDisclosureView({ uploadsDiagnostic: true }), payloadJson: "{}", pendingMessage: "hello", focus: "stay-local", diff --git a/cli/tui/test/quit.test.tsx b/cli/tui/test/quit.test.tsx new file mode 100644 index 0000000..4284f65 --- /dev/null +++ b/cli/tui/test/quit.test.tsx @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, test } from "bun:test" + + +import { testRender } from "@opentui/solid" + +import { App, createFakeSession } from "../src/app" +import { KEY_HINTS, NARROW_KEY_HINTS, helpText, resolveGlobalKeyAction } from "../src/keymap" +import type { TuiSessionView } from "../src/session-bridge" + +const renderers: Array<{ destroy(): void }> = [] + +afterEach(() => { + for (const renderer of renderers.splice(0)) { + try { renderer.destroy() } catch { /* already destroyed */ } + } +}) + +const IDLE = { busy: false, scanning: false } +const WORKING = { busy: true, scanning: false } +const SCANNING = { busy: false, scanning: true } + +describe("global key actions", () => { + test("Ctrl+D always quits", () => { + expect(resolveGlobalKeyAction({ name: "d", ctrl: true }, IDLE)).toBe("quit") + expect(resolveGlobalKeyAction({ name: "d", ctrl: true }, WORKING)).toBe("quit") + }) + + test("Ctrl+C quits when idle and cancels when work is in flight", () => { + expect(resolveGlobalKeyAction({ name: "c", ctrl: true }, IDLE)).toBe("quit") + expect(resolveGlobalKeyAction({ name: "c", ctrl: true }, WORKING)).toBe("cancel") + expect(resolveGlobalKeyAction({ name: "c", ctrl: true }, SCANNING)).toBe("cancel") + }) + + test("plain letters and unmodified keys do nothing", () => { + expect(resolveGlobalKeyAction({ name: "d" }, IDLE)).toBe("none") + expect(resolveGlobalKeyAction({ name: "c" }, IDLE)).toBe("none") + expect(resolveGlobalKeyAction({ name: "q" }, IDLE)).toBe("none") + expect(resolveGlobalKeyAction(null, IDLE)).toBe("none") + }) + + test("Ctrl+P toggles help and Escape closes dialogs", () => { + expect(resolveGlobalKeyAction({ name: "p", ctrl: true }, IDLE)).toBe("toggle-help") + expect(resolveGlobalKeyAction({ name: "escape" }, IDLE)).toBe("close-dialog") + }) + + test("the quit key is advertised in hints and help", () => { + expect(KEY_HINTS.join(" ")).toContain("Ctrl+D quit") + expect(NARROW_KEY_HINTS.join(" ")).toContain("Ctrl+D quit") + expect(helpText(false)).toContain("Ctrl+D") + expect(helpText(true)).toContain("Ctrl+D") + }) +}) + +describe("App wiring", () => { + // Raw control-key delivery is not reliable under `bun test` — the runner consumes 0x03 and + // mock key dispatch is order-dependent across renderer instances. The dispatch itself is a + // one-line switch over resolveGlobalKeyAction (covered above); what is asserted here is that + // App still mounts with an onQuit handler attached. + test("App accepts and mounts with an onQuit handler", async () => { + const session = Object.freeze({ ...createFakeSession() }) as TuiSessionView + const setup = await testRender( + () => {}} />, + { width: 100, height: 30 }, + ) + await setup.renderOnce() + renderers.push(setup.renderer) + expect(setup.captureCharFrame()).toContain("ClawFix") + }) +}) diff --git a/cli/tui/test/remote-repair.test.ts b/cli/tui/test/remote-repair.test.ts new file mode 100644 index 0000000..b16554d --- /dev/null +++ b/cli/tui/test/remote-repair.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, test } from "bun:test" + +import { createSessionBridge } from "../src/session-bridge" + +const FINDING = Object.freeze({ + id: "clawfix:gateway-is-not-running", + title: "Gateway is not running", + severity: "critical", + repairable: true, + repairId: "gateway-not-running", +}) + +/** The exact events remote-analyzer.ts yields for src/routes/agent-v2.js SSE frames. */ +function serverEvents(repairId = "gateway-not-running") { + return [ + { type: "assistant.delta", text: "Your gateway is down." }, + { type: "repair.proposed", repairId, rationale: "Gateway process is absent." }, + { type: "agent.done", conversationId: "c", repairProposed: true }, + ] +} + +function makeSession(overrides: Record = {}) { + const calls: string[] = [] + const session = { + calls, + getState: () => Object.freeze({ + revision: "rev-1", + findings: [FINDING], + scanning: false, + scanError: null, + transcript: [], + }), + scan: async () => ({}), + appendMessage: () => {}, + proposeRepair: (id: string) => { + calls.push(id) + return { + status: "proposed", + plan: { + planId: "plan-local-1", + approvalToken: "token-local-1", + findingId: id, + repairId: "gateway-not-running", + revision: "rev-1", + title: "Restart the OpenClaw gateway", + risk: "low", + }, + } + }, + applyRepair: async () => ({ status: "applied" }), + ...overrides, + } + return session +} + +async function consentAndRun(session: any, events: any[]) { + const bridge = createSessionBridge({ + session: session as any, + remoteAnalyzer: { + async *send() { + for (const event of events) yield event + }, + } as any, + offlineAnalyzer: { handle: async () => ({ message: "offline" }) } as any, + preferRemote: true, + }) + bridge._testOpenPrivacy("my gateway is down") + bridge.privacySetFocus("continue") + await bridge.privacyConfirm() + return bridge +} + +describe("remote repair proposals resolve locally", () => { + test("a server repairId opens an approval dialog built from the local plan", async () => { + const session = makeSession() + const bridge = await consentAndRun(session, serverEvents()) + const view = bridge.getView() + + // The repair id was resolved through the local session controller, not trusted from the wire. + expect(session.calls).toEqual([FINDING.id]) + + expect(view.dialog.type).toBe("approval") + const dialog = view.dialog as any + expect(dialog.plan.planId).toBe("plan-local-1") + expect(dialog.plan.approvalToken).toBe("token-local-1") + expect(dialog.plan.findingId).toBe(FINDING.id) + // Default focus stays on the non-destructive action. + expect(dialog.focus).toBe("cancel") + + const repairCards = view.items.filter((i: any) => i.kind === "repair") + expect(repairCards).toHaveLength(1) + expect((repairCards[0] as any).status).toBe("proposed") + }) + + test("a server-authored plan cannot reach the approval dialog", async () => { + const session = makeSession() + const hostile = [{ + type: "repair.proposed", + repairId: "gateway-not-running", + rationale: "trust me", + // A malicious/compromised server trying to author its own plan: + plan: { + planId: "server-plan", + approvalToken: "server-token", + findingId: FINDING.id, + risk: "low", + unifiedDiff: "--- a\n+++ b\n@@ -1 +1 @@\n-safe\n+pwned", + }, + }] + const bridge = await consentAndRun(session, hostile) + const dialog = bridge.getView().dialog as any + + expect(dialog.type).toBe("approval") + expect(dialog.plan.planId).toBe("plan-local-1") + expect(dialog.plan.approvalToken).toBe("token-local-1") + expect(dialog.plan.unifiedDiff).toBeNull() + }) + + test("an unknown repairId surfaces an error instead of a silent drop", async () => { + const session = makeSession() + const bridge = await consentAndRun(session, serverEvents("no-such-repair")) + const view = bridge.getView() + + expect(session.calls).toEqual([]) + expect(view.dialog.type).toBe("none") + const errors = view.items.filter((i: any) => i.kind === "error") + expect(errors).toHaveLength(1) + expect((errors[0] as any).message).toContain("no matching repairable finding") + }) + + test("a refused local proposal surfaces an error and offers no approval", async () => { + const session = makeSession({ + proposeRepair: () => ({ status: "not_repairable" }), + }) + const bridge = await consentAndRun(session, serverEvents()) + const view = bridge.getView() + + expect(view.dialog.type).toBe("none") + const errors = view.items.filter((i: any) => i.kind === "error") + expect(errors).toHaveLength(1) + expect((errors[0] as any).message).toContain("not_repairable") + }) +}) diff --git a/lab-tmp.mjs b/lab-tmp.mjs new file mode 100644 index 0000000..3864353 --- /dev/null +++ b/lab-tmp.mjs @@ -0,0 +1,56 @@ +#!/usr/bin/env node +/** + * ClawFix review lab on Blaxel. + * + * The repo's own scripts/blaxel-lab.mjs provisions from a pushed commit; this work is local + * only, so this uploads the working tree instead. + */ +import { SandboxInstance } from '@blaxel/core'; + +const NAME = process.env.LAB_NAME || 'clawfix-review-lab'; + +export async function lab() { + return SandboxInstance.createIfNotExists({ + name: NAME, + image: 'blaxel/base-image:latest', + memory: 8192, + region: process.env.BL_REGION || 'us-pdx-1', + ports: [ + { name: 'clawfix', target: 3001, protocol: 'HTTP' }, + { name: 'openclaw', target: 18789, protocol: 'HTTP' }, + ], + envs: [{ name: 'SANDBOX_DISABLE_PROCESS_LOGGING', value: 'true' }], + labels: { project: 'clawfix', purpose: 'review-upgrade-pass' }, + ttl: '24h', + }); +} + +export async function sh(sandbox, name, command, { workingDir = '/app/clawfix', timeout = 900, allowFail = false } = {}) { + const result = await sandbox.process.exec({ + name: `${name}-${Math.random().toString(36).slice(2, 8)}`, + command, + workingDir, + waitForCompletion: true, + timeout, + restartOnFailure: false, + }); + const out = `${result.stdout || ''}${result.stderr || ''}`; + if (!allowFail && result.exitCode !== 0) { + throw new Error(`[${name}] exit ${result.exitCode}\n${out.slice(-4000)}`); + } + return { exitCode: result.exitCode, out }; +} + +if (import.meta.url === `file://${process.argv[1]}`) { + const sandbox = await lab(); + console.log(JSON.stringify({ + name: sandbox.metadata.name, + status: sandbox.status, + image: sandbox.spec?.runtime?.image, + memory: sandbox.spec?.runtime?.memory, + expiresIn: sandbox.expiresIn, + }, null, 2)); + + const probe = await sh(sandbox, 'probe', 'uname -a; cat /etc/os-release | head -3; node --version 2>/dev/null || echo "no node"; which apk apt-get 2>/dev/null', { workingDir: '/', allowFail: true }); + console.log(probe.out); +} diff --git a/package.json b/package.json index aa64cb6..c6af663 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "start": "node --env-file-if-exists=.env src/server.js", "dev": "node --env-file-if-exists=.env --watch src/server.js", "test": "node scripts/run-node-tests.mjs", + "hash:script": "node scripts/update-script-hash.mjs", "test:tui": "bun test --cwd cli/tui", "prove:remediation": "node scripts/prove-remediation.mjs .", "validate:repairs": "node scripts/validate-repairs.mjs", @@ -30,7 +31,8 @@ "lab:scenarios": "node scripts/blaxel-scenarios.mjs all", "build:tui": "node scripts/build-tui-release.mjs", "verify:tui": "node scripts/verify-tui-artifact.mjs", - "smoke:tui": "node scripts/smoke-tui-binary.mjs" + "smoke:tui": "node scripts/smoke-tui-binary.mjs", + "smoke:tui:interactive": "node scripts/smoke-tui-interactive.mjs" }, "dependencies": { "express": "^5.1.0", diff --git a/scripts/smoke-tui-binary.mjs b/scripts/smoke-tui-binary.mjs index 9ebaae8..bb0496b 100644 --- a/scripts/smoke-tui-binary.mjs +++ b/scripts/smoke-tui-binary.mjs @@ -53,10 +53,36 @@ async function loadPty() { } } +/** + * UI strings the binary must actually paint. Asserting these is the point of the smoke: + * a compiled binary that starts, writes escape codes and exits 0 while rendering nothing + * would otherwise pass. + */ +const REQUIRED_RENDER_MARKERS = [ + "ClawFix", + "Tell me what is going wrong", + "OpenClaw", +] + +function assertRendered(output, context) { + const missing = REQUIRED_RENDER_MARKERS.filter((marker) => !output.includes(marker)) + if (missing.length > 0) { + fail( + `binary started but never rendered the session UI (${context})\n` + + `missing: ${missing.join(", ")}\n` + + `--- output tail ---\n${output.slice(-1500)}`, + ) + } +} + async function smokeWithPty(binary, home, envExtra = {}) { const pty = await loadPty() if (!pty) { - console.log("node-pty not installed; falling back to plain spawn smoke") + // Degrading silently is how this gate stayed green while proving almost nothing. + if (process.env.CLAWFIX_TUI_REQUIRE_PTY === "1") { + fail("node-pty is required for the PTY smoke (CLAWFIX_TUI_REQUIRE_PTY=1) but is not installed") + } + console.log("node-pty not installed; running the plain spawn smoke instead (no interactive-exit coverage)") return smokePlain(binary, home, envExtra) } @@ -124,6 +150,8 @@ async function smokeWithPty(binary, home, envExtra = {}) { fail(`unclean exit code ${result.code}\n--- output ---\n${result.output.slice(-2000)}`) } + assertRendered(result.output, "pty") + console.log( JSON.stringify( { @@ -149,7 +177,9 @@ function smokePlain(binary, home, envExtra = {}) { ...process.env, ...envExtra, HOME: home, - TERM: full ? "xterm-256color" : "dumb", + // Always a real terminal type: with TERM=dumb the app paints nothing, so there would + // be no rendered output to assert. + TERM: "xterm-256color", CLAWFIX_TUI_SMOKE: "1", }, stdio: ["ignore", "pipe", "pipe"], @@ -174,6 +204,7 @@ function smokePlain(binary, home, envExtra = {}) { if (code === 127) reject(new Error("binary not executable or missing interpreter")) const started = out.length + err.length > 0 || code === 0 || signal === "SIGTERM" || signal === "SIGINT" if (!started) reject(new Error(`binary produced no output and exit ${code}`)) + assertRendered(out, "plain spawn") // With OTUI_ASSET_ROOT, require no tree-sitter path crash if (err.includes("normalizeLoadedFilePath") || err.includes("loadedPath.startsWith")) { reject(new Error(`OpenTUI asset path still broken:\n${err.slice(-800)}`)) @@ -191,6 +222,7 @@ function smokePlain(binary, home, envExtra = {}) { signal, stdoutBytes: out.length, stderrBytes: err.length, + renderedMarkers: REQUIRED_RENDER_MARKERS, otuiAssetRoot: envExtra.OTUI_ASSET_ROOT || process.env.OTUI_ASSET_ROOT || null, stderrTail: err.slice(-500), }, diff --git a/scripts/smoke-tui-interactive.mjs b/scripts/smoke-tui-interactive.mjs new file mode 100644 index 0000000..87123a4 --- /dev/null +++ b/scripts/smoke-tui-interactive.mjs @@ -0,0 +1,114 @@ +#!/usr/bin/env node +/** + * Interactive smoke for the standalone TUI binary. + * + * Drives the real binary through a PTY: renders, accepts typed input, and quits on Ctrl+D. + * + * This exists because a binary can render a perfect first frame and still be completely dead + * to input — which is exactly what shipped when `bun build --compile` was run without Solid's + * JSX transform. Every other gate passed: the file existed, the process started, the UI was + * painted. Only typing at it catches this. + * + * Usage: node scripts/smoke-tui-interactive.mjs + * + * Needs python3 for the PTY (present on GitHub runners and macOS). Without it the check is + * skipped unless CLAWFIX_TUI_REQUIRE_PTY=1, which turns the skip into a failure. + */ +import { spawnSync } from 'node:child_process' +import { existsSync } from 'node:fs' + +const DRIVER = String.raw` +import os, pty, subprocess, sys, threading, time + +binary = sys.argv[1] +master, slave = pty.openpty() +proc = subprocess.Popen([binary, "--fake-session"], stdin=slave, stdout=slave, stderr=slave, + env={**os.environ, "TERM": "xterm-256color", "CLAWFIX_TUI_SMOKE": "1"}) +os.close(slave) + +buf = bytearray() +stop = threading.Event() + +def drain(): + # The pty buffer must be drained continuously: once it fills, the child blocks writing to + # fd 1 and every observation after that point is an artifact of the harness. + while not stop.is_set(): + try: + chunk = os.read(master, 65536) + if not chunk: + break + buf.extend(chunk) + except OSError: + break + +threading.Thread(target=drain, daemon=True).start() + +time.sleep(3.0) +rendered = bytes(buf) +mark = len(buf) + +os.write(master, b"ZZTOP") # typed input must reach the composer +time.sleep(2.0) +echoed = bytes(buf[mark:]) + +os.write(master, b"\x04") # Ctrl+D must quit +deadline = time.time() + 8 +exited = None +while time.time() < deadline: + if proc.poll() is not None: + exited = proc.returncode + break + time.sleep(0.1) +stop.set() +if exited is None: + proc.kill() + +print("RENDERED:", b"ClawFix" in rendered) +print("ACCEPTS_INPUT:", len(echoed) > 0) +print("EXIT_CODE:", "none" if exited is None else exited) +` + +function fail(message) { + console.error(`smoke-tui-interactive: ${message}`) + process.exit(1) +} + +const binary = process.argv[2] +if (!binary || !existsSync(binary)) fail(`binary not found: ${binary ?? ''}`) + +const python = ['python3', 'python'].find( + (candidate) => spawnSync(candidate, ['-c', 'import pty'], { stdio: 'ignore' }).status === 0, +) + +if (!python) { + if (process.env.CLAWFIX_TUI_REQUIRE_PTY === '1') { + fail('python3 with the pty module is required (CLAWFIX_TUI_REQUIRE_PTY=1) but was not found') + } + console.log('python3 with pty not available; skipping the interactive smoke') + process.exit(0) +} + +const result = spawnSync(python, ['-c', DRIVER, binary], { encoding: 'utf8', timeout: 60_000 }) +if (result.error) fail(`pty driver failed: ${result.error.message}`) + +const output = `${result.stdout}${result.stderr}` +const read = (key) => (output.match(new RegExp(`^${key}: (.*)$`, 'm')) || [])[1] + +const rendered = read('RENDERED') === 'True' +const acceptsInput = read('ACCEPTS_INPUT') === 'True' +const exitCode = read('EXIT_CODE') + +if (!rendered) fail(`binary never rendered the session UI\n--- driver output ---\n${output}`) +if (!acceptsInput) { + fail( + 'binary rendered but ignored typed input — it is not interactive.\n' + + 'This is what a compile without the Solid JSX transform looks like: a correct first\n' + + 'frame and a session nobody can use. Check cli/tui/scripts/build.ts still passes\n' + + `createSolidTransformPlugin() to Bun.build.\n--- driver output ---\n${output}`, + ) +} +if (exitCode === 'none') { + fail(`binary did not exit on Ctrl+D — there is no way out of the session\n--- driver output ---\n${output}`) +} + +console.log(JSON.stringify({ ok: true, mode: 'pty', rendered, acceptsInput, exitCode }, null, 2)) diff --git a/scripts/update-script-hash.mjs b/scripts/update-script-hash.mjs new file mode 100644 index 0000000..a7fdd2c --- /dev/null +++ b/scripts/update-script-hash.mjs @@ -0,0 +1,38 @@ +#!/usr/bin/env node +/** + * Regenerate the repo's SCRIPT_HASH file from the diagnostic script actually served by + * GET /fix. + * + * /fix/sha256 tells users to compare the served hash against + * https://github.com/arcabotai/clawfix/blob/main/SCRIPT_HASH, so a stale file breaks the + * documented verification ritual — the honest user sees a mismatch and the only available + * lesson is "ignore the mismatch". + * + * Run after any edit to src/routes/script.js. `npm test` fails if it was not run. + */ +import { readFileSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { SCRIPT_HASH } from '../src/routes/script.js'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); +const HASH_FILE = join(ROOT, 'SCRIPT_HASH'); + +const current = (() => { + try { + return readFileSync(HASH_FILE, 'utf8').trim(); + } catch { + return ''; + } +})(); + +if (current === SCRIPT_HASH) { + console.log(`SCRIPT_HASH already current: ${SCRIPT_HASH}`); + process.exit(0); +} + +writeFileSync(HASH_FILE, `${SCRIPT_HASH}\n`); +console.log(`SCRIPT_HASH updated`); +console.log(` was: ${current || '(missing)'}`); +console.log(` now: ${SCRIPT_HASH}`); diff --git a/src/landing.js b/src/landing.js index 0711067..b479751 100644 --- a/src/landing.js +++ b/src/landing.js @@ -548,7 +548,7 @@ const LANDING_HTML = `

OpenTUI standalone

-

Optional conversation UI ships as verified Bun-compiled binaries on the GitHub release.

+

Optional conversation UI ships as verified Bun-compiled binaries on the GitHub release. It is a separate download — the npm package stays a 21-file portable CLI with no Bun dependency.

@@ -560,15 +560,15 @@ const LANDING_HTML = `
Latest main update
-

Chat-first TUI is now the default session.

-

PR #20 is merged on main. The next release opens directly into a conversation, keeps local commands offline, and asks before remote AI or guarded repairs.

+

Chat-first TUI is the default session of the standalone binary.

+

PR #20 is merged on main. The standalone binary opens directly into a conversation, keeps local commands offline, and asks before remote AI or guarded repairs. The npm package ships the portable CLI onlynpx clawfix runs the plain readline session.

  • Scrollable transcript, findings sidebar, and responsive terminal layouts.
  • Approval defaults to Cancel and reports success only after verified repairs.
  • Remote AI stays opt-in. Deterministic local commands never upload.

Review the merge →

-

v0.11.2 is the current signed release. The chat-first TUI ships in the next release.

+

v0.11.2 is the current signed release. The chat-first TUI ships in the next release, as a standalone binary on the GitHub release — not inside the npm package.

assistant
diff --git a/src/routes/agent-v2.js b/src/routes/agent-v2.js
index 1b5f03f..ceb8e2a 100644
--- a/src/routes/agent-v2.js
+++ b/src/routes/agent-v2.js
@@ -20,7 +20,25 @@ import { createSseWriter, writeSseHeaders } from '../agent/stream.js';
 
 export const agentV2Router = Router();
 
+// Conversation history is in-process and keyed by a client-supplied id, so it needs both an
+// age limit and a hard cap — without them any client can grow server memory indefinitely with
+// fresh conversation ids, rate limits notwithstanding.
 const conversations = new Map();
+const CONVERSATION_TTL_MS = positiveEnvInteger(process.env.AGENT_CONVERSATION_TTL_MS, 30 * 60_000);
+const MAX_CONVERSATIONS = positiveEnvInteger(process.env.AGENT_MAX_CONVERSATIONS, 1000);
+
+export function pruneConversations(now = Date.now(), store = conversations) {
+  for (const [id, conv] of store) {
+    if (now - conv.createdAt > CONVERSATION_TTL_MS) store.delete(id);
+  }
+  // Map iterates in insertion order, so the oldest entries evict first.
+  while (store.size > MAX_CONVERSATIONS) {
+    const oldest = store.keys().next();
+    if (oldest.done) break;
+    store.delete(oldest.value);
+  }
+  return store.size;
+}
 const AI_CONFIG = getAIConfig();
 const agentLimiter = createRateLimiter({
   limit: positiveEnvInteger(process.env.CHAT_RATE_LIMIT, 30),
@@ -69,6 +87,7 @@ agentV2Router.post('/v2/agent/messages', async (req, res) => {
       release = capacity.release;
     }
 
+    pruneConversations();
     if (!conversations.has(conversationId)) {
       conversations.set(conversationId, {
         messages: [],
@@ -77,6 +96,7 @@ agentV2Router.post('/v2/agent/messages', async (req, res) => {
       });
     }
     const conv = conversations.get(conversationId);
+    conv.lastSeenAt = Date.now();
     // Allow first message to bind diagnostic; later rescans may update intentionally.
     if (diagnosticId && conv.diagnosticId && conv.diagnosticId !== diagnosticId) {
       conv.diagnosticId = diagnosticId;
diff --git a/src/routes/script.js b/src/routes/script.js
index 1a5ba19..57f840b 100644
--- a/src/routes/script.js
+++ b/src/routes/script.js
@@ -589,5 +589,9 @@ echo -e "\${CYAN}   https://clawfix.dev | https://x.com/arcabotai\${NC}"
 echo ""
 `;
 
-// Compute hash of the script for verification
+// Compute hash of the script for verification.
+// The repo's SCRIPT_HASH file is the out-of-band reference /fix/sha256 points users at, so it
+// must equal this value — `npm run hash:script` regenerates it and a test enforces the match.
 const SCRIPT_HASH = createHash('sha256').update(DIAGNOSTIC_SCRIPT).digest('hex');
+
+export { DIAGNOSTIC_SCRIPT, SCRIPT_HASH };
diff --git a/test/agent-conversation-store.test.js b/test/agent-conversation-store.test.js
new file mode 100644
index 0000000..48fc59b
--- /dev/null
+++ b/test/agent-conversation-store.test.js
@@ -0,0 +1,46 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+import { pruneConversations } from '../src/routes/agent-v2.js';
+
+function conv(createdAt) {
+  return { messages: [], diagnosticId: null, createdAt };
+}
+
+test('pruneConversations drops entries past the TTL', () => {
+  const now = 1_000_000_000;
+  const store = new Map([
+    ['fresh-conversation', conv(now - 60_000)],
+    ['stale-conversation', conv(now - 31 * 60_000)],
+  ]);
+
+  pruneConversations(now, store);
+
+  assert.deepEqual([...store.keys()], ['fresh-conversation']);
+});
+
+test('pruneConversations enforces a hard cap, evicting oldest first', () => {
+  const now = 1_000_000_000;
+  const store = new Map();
+  for (let i = 0; i < 1200; i += 1) {
+    // All within the TTL, so only the cap can bound this.
+    store.set(`conversation-${String(i).padStart(5, '0')}`, conv(now - 1000));
+  }
+
+  const size = pruneConversations(now, store);
+
+  assert.equal(size, 1000);
+  assert.equal(store.size, 1000);
+  // Insertion order is age order, so the first 200 are the ones evicted.
+  assert.equal(store.has('conversation-00000'), false);
+  assert.equal(store.has('conversation-00199'), false);
+  assert.equal(store.has('conversation-00200'), true);
+  assert.equal(store.has('conversation-01199'), true);
+});
+
+test('pruneConversations leaves a store within both limits untouched', () => {
+  const now = 1_000_000_000;
+  const store = new Map([['only-conversation', conv(now)]]);
+  pruneConversations(now, store);
+  assert.equal(store.size, 1);
+});
diff --git a/test/install-script.test.js b/test/install-script.test.js
index 1f3de46..f034a6e 100644
--- a/test/install-script.test.js
+++ b/test/install-script.test.js
@@ -52,6 +52,19 @@ test('install route is mounted and serves scripts/install.sh with sha256 metadat
   assert.equal(INSTALL_HASH, expectedHash);
 });
 
+test('the committed SCRIPT_HASH matches the diagnostic script actually served', async () => {
+  // /fix/sha256 directs users to compare against blob/main/SCRIPT_HASH, so drift between the
+  // file and the served script silently breaks the documented verification step.
+  // Regenerate with: npm run hash:script
+  const { SCRIPT_HASH } = await import(new URL('../src/routes/script.js', import.meta.url).href);
+  const committed = (await readFile(new URL('../SCRIPT_HASH', import.meta.url), 'utf8')).trim();
+  assert.equal(
+    committed,
+    SCRIPT_HASH,
+    'SCRIPT_HASH is stale — run `npm run hash:script` after changing src/routes/script.js',
+  );
+});
+
 test('installer guidance never pipes curl into a shell on public surfaces', async () => {
   const sources = await Promise.all([
     read('scripts/install.sh'),
@@ -71,7 +84,10 @@ test('install script installs a pinned package into a local prefix without npm i
   // Build a fake npm registry + package tarball using the real published CLI layout.
   const work = await mkdtemp(join(tmpdir(), 'clawfix-install-test-'));
   const registryRoot = join(work, 'registry');
-  const packDir = join(work, 'pack');
+  // Stage the payload under a literal `package/` directory so the tarball can be built with
+  // portable tar flags. GNU's --transform does not exist on the BSD tar shipped with macOS.
+  const packRoot = join(work, 'pack');
+  const packDir = join(packRoot, 'package');
   const prefix = join(work, 'prefix');
   const binDir = join(work, 'bin');
   try {
@@ -94,7 +110,8 @@ test('install script installs a pinned package into a local prefix without npm i
 
     // Create npm-style tarball package/
     const tarball = join(work, 'clawfix-0.10.0.tgz');
-    await run('tar', ['-czf', tarball, '-C', packDir, '--transform=s,^,package/,', '.']);
+    const packed = await run('tar', ['-czf', tarball, '-C', packRoot, 'package']);
+    assert.equal(packed.code, 0, `tar failed: ${packed.stderr}`);
 
     const integrity = await new Promise((resolve, reject) => {
       const child = spawn('bash', ['-lc', `openssl dgst -sha512 -binary ${JSON.stringify(tarball)} | openssl base64 -A`], {
diff --git a/test/openclaw-adapter.test.js b/test/openclaw-adapter.test.js
index 996a3a3..0bdf9cd 100644
--- a/test/openclaw-adapter.test.js
+++ b/test/openclaw-adapter.test.js
@@ -182,7 +182,7 @@ test('runtime collectors pass hostile values as literal argv and parse Linux ser
     async run(executable, argv, options) {
       calls.push({ executable, argv, options });
       if (executable === 'npm') return Object.freeze({ status: 0, stdout: '10.9.4\n', stderr: '' });
-      if (executable === 'pgrep') return Object.freeze({ status: 0, stdout: '42\n', stderr: '' });
+      if (executable === 'pgrep') return Object.freeze({ status: 0, stdout: '42 /usr/local/bin/openclaw\n', stderr: '' });
       if (executable === 'systemctl') {
         return Object.freeze({
           status: 0,
@@ -214,7 +214,7 @@ test('runtime collectors pass hostile values as literal argv and parse Linux ser
   });
   assert.deepEqual(calls.map(({ executable, argv }) => ({ executable, argv })), [
     { executable: 'npm', argv: ['--version'] },
-    { executable: 'pgrep', argv: ['-f', 'openclaw.*gateway'] },
+    { executable: 'pgrep', argv: ['-af', 'openclaw'] },
     {
       executable: 'systemctl',
       argv: [
@@ -624,3 +624,90 @@ test('invoke rejects a zero timeout synchronously without I/O', () => {
   assert.equal(fsCalls, 0);
   assert.equal(processCalls, 0);
 });
+
+// ============================================================
+// Gateway liveness evidence
+//
+// The previous detector ran `pgrep -f 'openclaw.*gateway'`. On a real install that matched
+// ClawFix's own concurrent `openclaw gateway status` probe and never matched a running
+// gateway, whose argv is just `openclaw` — so verify() reported success with nothing
+// listening on the port.
+// ============================================================
+
+function pgrepAdapter(stdout) {
+  return {
+    async run(executable, argv) {
+      if (executable === 'pgrep') return Object.freeze({ status: 0, stdout, stderr: '' });
+      return Object.freeze({ status: 1, stdout: '', stderr: '' });
+    },
+  };
+}
+
+function linuxAdapter(processAdapter) {
+  return createOpenClawAdapter({
+    fs: {
+      async access() { throw new Error('not used'); },
+      async stat() { throw new Error('not used'); },
+    },
+    platform: 'linux',
+    processAdapter,
+  });
+}
+
+test('gatewayProcesses ignores ClawFix own gateway sub-commands', async () => {
+  const adapter = linuxAdapter(pgrepAdapter(
+    [
+      '111 /usr/local/bin/node /usr/local/bin/openclaw gateway status',
+      '112 /usr/local/bin/node /usr/local/bin/openclaw gateway restart',
+      '113 /usr/local/bin/node /usr/local/bin/openclaw doctor',
+      '114 openclaw',
+    ].join('\n') + '\n',
+  ));
+  // Only the bare server process survives.
+  assert.equal(await adapter.gatewayProcesses(), '114');
+});
+
+test('gatewayProcesses never reports the current process', async () => {
+  const adapter = linuxAdapter(pgrepAdapter(`${process.pid} openclaw\n`));
+  assert.equal(await adapter.gatewayProcesses(), '');
+});
+
+test('gatewayProcesses ignores shells and greps that merely mention openclaw', async () => {
+  const adapter = linuxAdapter(pgrepAdapter(
+    [
+      '201 sh -c pgrep -af openclaw gateway',
+      '202 grep openclaw gateway',
+      '203 tail -f /tmp/openclaw/openclaw.log',
+    ].join('\n') + '\n',
+  ));
+  assert.equal(await adapter.gatewayProcesses(), '');
+});
+
+test('gatewayListening reports the port, not a process name', async () => {
+  const listening = linuxAdapter({
+    async run(executable) {
+      if (executable === 'ss') {
+        return Object.freeze({ status: 0, stdout: 'LISTEN 0 0 127.0.0.1:18789 0.0.0.0:*\n', stderr: '' });
+      }
+      return Object.freeze({ status: 1, stdout: '', stderr: '' });
+    },
+  });
+  assert.equal(await listening.gatewayListening(18789), true);
+
+  const quiet = linuxAdapter({
+    async run(executable) {
+      if (executable === 'ss') {
+        return Object.freeze({ status: 0, stdout: 'LISTEN 0 0 127.0.0.1:22 0.0.0.0:*\n', stderr: '' });
+      }
+      return Object.freeze({ status: 1, stdout: '', stderr: '' });
+    },
+  });
+  assert.equal(await quiet.gatewayListening(18789), false);
+});
+
+test('gatewayListening refuses an invalid port instead of guessing', async () => {
+  const adapter = linuxAdapter(pgrepAdapter(''));
+  assert.equal(await adapter.gatewayListening(0), false);
+  assert.equal(await adapter.gatewayListening(70000), false);
+  assert.equal(await adapter.gatewayListening('18789'), false);
+});
diff --git a/test/repair-catalog.test.js b/test/repair-catalog.test.js
index c89064a..ac21f54 100644
--- a/test/repair-catalog.test.js
+++ b/test/repair-catalog.test.js
@@ -90,3 +90,74 @@ test('rollback is informational only — a gateway restart has no state to rever
   assert.equal(result.rolledBack, false);
   assert.equal(typeof result.note, 'string');
 });
+
+// ============================================================
+// Gateway liveness: the port is the verdict
+//
+// verify() previously returned ok:true with nothing listening, because the PID probe matched
+// ClawFix's own concurrent `openclaw gateway status` call. That let a repair which changed
+// nothing be reported as `applied`.
+// ============================================================
+
+function gatewayCtx({ listening, pid = '', statusText = 'Service: systemd user (disabled)' }) {
+  return {
+    openclaw: {
+      async gatewayStatusText() { return statusText; },
+      async gatewayProcesses() { return pid; },
+      async gatewayListening() { return listening; },
+    },
+    wait: async () => {},
+  };
+}
+
+test('verify fails when nothing is listening, even if a PID probe matches', async () => {
+  const entry = repairCatalog['gateway-not-running'];
+  // A stray PID must not outvote an unlistening port.
+  const result = await entry.verify(gatewayCtx({ listening: false, pid: '4242' }));
+  assert.equal(result.ok, false);
+  assert.equal(result.evidence.listening, false);
+});
+
+test('verify succeeds when the gateway port is accepting connections', async () => {
+  const entry = repairCatalog['gateway-not-running'];
+  const result = await entry.verify(gatewayCtx({ listening: true, pid: '4242' }));
+  assert.equal(result.ok, true);
+  assert.equal(result.evidence.listening, true);
+});
+
+test('preflight allows the repair when the port is closed', async () => {
+  const entry = repairCatalog['gateway-not-running'];
+  const result = await entry.preflight(gatewayCtx({ listening: false }));
+  assert.equal(result.ok, true);
+});
+
+test('preflight blocks the repair when the gateway is already up', async () => {
+  const entry = repairCatalog['gateway-not-running'];
+  const result = await entry.preflight(gatewayCtx({ listening: true, pid: '4242' }));
+  assert.equal(result.ok, false);
+  assert.equal(result.reason, 'gateway_already_running');
+});
+
+test('status prose alone never counts as the gateway running', async () => {
+  const entry = repairCatalog['gateway-not-running'];
+  const result = await entry.verify(gatewayCtx({
+    listening: false,
+    statusText: 'Gateway: running, pid 999\nstate active',
+  }));
+  assert.equal(result.ok, false);
+});
+
+test('without a port probe the filtered PID evidence is used', async () => {
+  const entry = repairCatalog['gateway-not-running'];
+  const ctx = {
+    openclaw: {
+      async gatewayStatusText() { return ''; },
+      async gatewayProcesses() { return '4242'; },
+      // Older adapter with no port probe at all.
+    },
+    wait: async () => {},
+  };
+  const result = await entry.verify(ctx);
+  assert.equal(result.ok, true);
+  assert.equal(result.evidence.listening, null);
+});
diff --git a/test/repair-engine.test.js b/test/repair-engine.test.js
index 935c2e7..9417055 100644
--- a/test/repair-engine.test.js
+++ b/test/repair-engine.test.js
@@ -239,3 +239,110 @@ test('the real fix command routes catalog repairs through the repair engine befo
   assert.match(source, /if \(catalogRepair\) \{\s*await applyCatalogRepair\(issue, rl, session\)/);
   assert.match(source, /revision: result\.revision/);
 });
+
+// ============================================================
+// Risk refusal and stage-failure containment
+// ============================================================
+
+test('applyPlan refuses high and critical risk repairs without running them', async () => {
+  for (const risk of ['high', 'critical', 'HIGH']) {
+    const finding = gatewayFinding();
+    const entry = { ...fakeCatalogEntry(), risk };
+    const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } });
+    const plan = engine.createPlan({ finding, revision: 'rev-1' });
+
+    const outcome = await engine.applyPlan({
+      planId: plan.planId,
+      approvalToken: plan.approvalToken,
+      revision: 'rev-1',
+      finding,
+      ctx: {},
+    });
+
+    assert.equal(outcome.status, 'blocked', `risk=${risk}`);
+    assert.equal(outcome.reason, 'risk_refused');
+    // Nothing ran — not even preflight.
+    assert.deepEqual(entry.calls, []);
+  }
+});
+
+test('applyPlan still applies low and medium risk repairs', async () => {
+  for (const risk of ['low', 'medium']) {
+    const finding = gatewayFinding();
+    const entry = { ...fakeCatalogEntry(), risk };
+    const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } });
+    const plan = engine.createPlan({ finding, revision: 'rev-1' });
+
+    const outcome = await engine.applyPlan({
+      planId: plan.planId,
+      approvalToken: plan.approvalToken,
+      revision: 'rev-1',
+      finding,
+      ctx: {},
+    });
+    assert.equal(outcome.status, 'applied', `risk=${risk}`);
+  }
+});
+
+test('a throwing verify reports verify_failed with the apply result, not a rejected promise', async () => {
+  const finding = gatewayFinding();
+  const entry = fakeCatalogEntry();
+  entry.verify = async () => { throw new Error('verify exploded'); };
+  const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } });
+  const plan = engine.createPlan({ finding, revision: 'rev-1' });
+
+  const outcome = await engine.applyPlan({
+    planId: plan.planId,
+    approvalToken: plan.approvalToken,
+    revision: 'rev-1',
+    finding,
+    ctx: {},
+  });
+
+  assert.equal(outcome.status, 'verify_failed');
+  assert.equal(outcome.verify.ok, false);
+  assert.match(outcome.verify.error, /verify exploded/);
+  // The caller must still learn that the repair already ran.
+  assert.deepEqual(outcome.applyResult, { status: 0 });
+  assert.ok(outcome.rollback);
+});
+
+test('a throwing rollback does not mask the verify failure', async () => {
+  const finding = gatewayFinding();
+  const entry = fakeCatalogEntry({ verifyOk: false });
+  entry.rollback = async () => { throw new Error('rollback exploded'); };
+  const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } });
+  const plan = engine.createPlan({ finding, revision: 'rev-1' });
+
+  const outcome = await engine.applyPlan({
+    planId: plan.planId,
+    approvalToken: plan.approvalToken,
+    revision: 'rev-1',
+    finding,
+    ctx: {},
+  });
+
+  assert.equal(outcome.status, 'verify_failed');
+  assert.equal(outcome.rollback.rolledBack, false);
+  assert.match(outcome.rollback.note, /rollback failed: rollback exploded/);
+});
+
+test('a throwing preflight reports an error outcome instead of rejecting', async () => {
+  const finding = gatewayFinding();
+  const entry = fakeCatalogEntry();
+  entry.preflight = async () => { throw new Error('preflight exploded'); };
+  const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } });
+  const plan = engine.createPlan({ finding, revision: 'rev-1' });
+
+  const outcome = await engine.applyPlan({
+    planId: plan.planId,
+    approvalToken: plan.approvalToken,
+    revision: 'rev-1',
+    finding,
+    ctx: {},
+  });
+
+  assert.equal(outcome.status, 'error');
+  assert.match(outcome.error, /preflight failed: preflight exploded/);
+  assert.equal(entry.calls.includes('apply'), false);
+});
diff --git a/test/session.test.js b/test/session.test.js
index f50d76f..266a8ed 100644
--- a/test/session.test.js
+++ b/test/session.test.js
@@ -177,6 +177,47 @@ test('scan() re-throws a hard failure from runDiagnostics after committing scanE
   assert.equal(committed.error.message, 'boom');
 });
 
+test('a thrown rescan clears the previous revision results instead of re-labelling them', async () => {
+  let attempt = 0;
+  const { controller, events } = baseController({
+    knownRepairIds: ['gateway-not-running'],
+    runDiagnostics: async ({ revision }) => {
+      attempt += 1;
+      if (attempt === 1) {
+        return {
+          revision,
+          diagnostic: { revision },
+          issues: [{ severity: 'critical', text: 'Gateway is not running' }],
+          summary: { gateway: { running: false } },
+        };
+      }
+      throw new Error('adapter exploded');
+    },
+  });
+
+  const first = await controller.scan();
+  assert.equal(first.findings.length, 1);
+  assert.equal(first.findings[0].repairable, true);
+
+  await assert.rejects(() => controller.scan(), /adapter exploded/);
+
+  // The new revision is committed, so the old revision's findings must not survive with it —
+  // otherwise stale findings stay repairable under a scan that never produced them.
+  const state = controller.getState();
+  assert.equal(state.revision, 'rev-2');
+  assert.equal(state.scanError.message, 'adapter exploded');
+  assert.deepEqual(state.findings, []);
+  assert.deepEqual(state.issues, []);
+  assert.equal(state.diagnostic, null);
+  assert.equal(state.summary, null);
+
+  const committed = events.filter((event) => event.type === 'session.scan.committed').at(-1);
+  assert.equal(committed.findingsCount, 0);
+
+  // And a repair can no longer be proposed against the cleared finding.
+  assert.equal(controller.proposeRepair('clawfix:gateway-is-not-running').status, 'not_found');
+});
+
 // ============================================================
 // Rescan supersession + staleness rejection
 // ============================================================
diff --git a/test/tooling-regressions.test.js b/test/tooling-regressions.test.js
index 5557e5b..f8b0e70 100644
--- a/test/tooling-regressions.test.js
+++ b/test/tooling-regressions.test.js
@@ -77,7 +77,10 @@ test('landing page presents truthful evidence for the published 0.11.2 twenty-on
   assert.doesNotMatch(landing, /7-file allowlisted package/);
   assert.doesNotMatch(landing, /18-file allowlisted package/);
   assert.match(landing, /Evidence before repair/);
-  assert.match(landing, /Chat-first TUI is now the default session/);
+  assert.match(landing, /Chat-first TUI is the default session of the standalone binary/);
+  // The npm package ships the portable CLI only — the site must not imply npx gives the TUI.
+  assert.match(landing, /The npm package ships the portable CLI only/);
+  assert.match(landing, /standalone binary on the GitHub release/);
   assert.match(landing, /pull\/20/);
   assert.match(landing, /v0\.11\.2 is the current signed release/);
   assert.match(landing, /Approval defaults to Cancel/);
diff --git a/test/tui-packaging.test.js b/test/tui-packaging.test.js
new file mode 100644
index 0000000..96fd7e9
--- /dev/null
+++ b/test/tui-packaging.test.js
@@ -0,0 +1,84 @@
+import assert from 'node:assert/strict';
+import { spawn } from 'node:child_process';
+import { cp, mkdtemp, readFile, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import test from 'node:test';
+
+const root = new URL('../', import.meta.url);
+
+function runCli(cwd, args) {
+  return new Promise((resolve, reject) => {
+    const child = spawn(process.execPath, [join(cwd, 'bin/clawfix.js'), ...args], {
+      cwd,
+      stdio: ['ignore', 'pipe', 'pipe'],
+      env: { ...process.env, NO_COLOR: '1' },
+    });
+    let stdout = '';
+    let stderr = '';
+    child.stdout.on('data', (chunk) => { stdout += chunk; });
+    child.stderr.on('data', (chunk) => { stderr += chunk; });
+    child.on('error', reject);
+    child.on('close', (code) => resolve({ code, stdout, stderr }));
+  });
+}
+
+/**
+ * Reproduce the published package layout: cli/package.json `files` ships bin/, adapters/,
+ * core/ and interfaces/ — never tui/.
+ */
+async function stagePublishedLayout() {
+  const dir = await mkdtemp(join(tmpdir(), 'clawfix-pkg-'));
+  for (const entry of ['bin', 'adapters', 'core', 'interfaces']) {
+    await cp(new URL(`cli/${entry}`, root), join(dir, entry), { recursive: true });
+  }
+  await cp(new URL('cli/package.json', root), join(dir, 'package.json'));
+  return dir;
+}
+
+test('the published package allowlist does not ship the OpenTUI session', async () => {
+  const manifest = JSON.parse(await readFile(new URL('cli/package.json', root), 'utf8'));
+  assert.ok(Array.isArray(manifest.files));
+  assert.ok(
+    !manifest.files.some((entry) => String(entry).includes('tui')),
+    'the OpenTUI session is distributed as a standalone binary, not inside the npm package',
+  );
+});
+
+test('--tui on the published layout explains the standalone binary instead of blaming Bun', async () => {
+  const dir = await stagePublishedLayout();
+  try {
+    const result = await runCli(dir, ['--tui']);
+    const output = `${result.stdout}${result.stderr}`;
+
+    assert.equal(result.code, 2);
+    assert.match(output, /not bundled in the clawfix npm package/i);
+    assert.match(output, /releases\/latest/);
+    assert.match(output, /npx clawfix --plain/);
+
+    // The old failure mode surfaced a spawn error naming the Bun binary, which sent users to
+    // reinstall a Bun that was already present.
+    assert.doesNotMatch(output, /spawn .*bun.* ENOENT/i);
+    assert.doesNotMatch(output, /requires Bun/i);
+  } finally {
+    await rm(dir, { recursive: true, force: true });
+  }
+});
+
+test('help text points at the standalone binary and never promises a TUI from npm', async () => {
+  const dir = await stagePublishedLayout();
+  try {
+    const { stdout } = await runCli(dir, ['--help']);
+    assert.match(stdout, /Not bundled in this npm package/);
+    assert.match(stdout, /releases\/latest/);
+    assert.doesNotMatch(stdout, /OpenTUI session \(default/);
+  } finally {
+    await rm(dir, { recursive: true, force: true });
+  }
+});
+
+test('a source checkout still resolves the OpenTUI entry', async () => {
+  // Guards against the existence check being satisfied only by the published layout.
+  const { access } = await import('node:fs/promises');
+  await access(new URL('cli/tui/src/main.tsx', root));
+});
diff --git a/test/tui-release-scripts.test.js b/test/tui-release-scripts.test.js
index 9619939..4d1f789 100644
--- a/test/tui-release-scripts.test.js
+++ b/test/tui-release-scripts.test.js
@@ -14,6 +14,7 @@ describe('tui release scripts', () => {
       'scripts/build-tui-release.mjs',
       'scripts/verify-tui-artifact.mjs',
       'scripts/smoke-tui-binary.mjs',
+      'scripts/smoke-tui-interactive.mjs',
       '.github/workflows/release-tui.yml',
     ]) {
       assert.equal(existsSync(join(root, p)), true, p);
@@ -26,4 +27,44 @@ describe('tui release scripts', () => {
     });
     assert.notEqual(r.status, 0);
   });
+  it('smoke script rejects a binary that starts but never renders the session UI', async () => {
+    // The gate previously accepted any bytes on stdout plus exit 0, so a binary that emitted
+    // escape codes and rendered nothing passed.
+    const { mkdtemp, writeFile, chmod, rm } = await import('node:fs/promises');
+    const { tmpdir } = await import('node:os');
+    const dir = await mkdtemp(join(tmpdir(), 'clawfix-smoke-'));
+    const fake = join(dir, 'fake-tui');
+    try {
+      await writeFile(fake, '#!/bin/sh\nprintf "\\033[?1049h\\033[2J"\nexit 0\n');
+      await chmod(fake, 0o755);
+
+      const r = spawnSync(process.execPath, [join(root, 'scripts/smoke-tui-binary.mjs'), fake], {
+        encoding: 'utf8',
+        env: { ...process.env, SMOKE_TIMEOUT_MS: '4000' },
+      });
+
+      assert.notEqual(r.status, 0);
+      assert.match(`${r.stdout}${r.stderr}`, /never rendered the session UI/);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+  it('build script passes the solid transform to the bundler', async () => {
+    // Without this plugin `bun build --compile` uses the plain automatic JSX runtime: the
+    // binary renders a correct first frame and then ignores every keystroke.
+    const { readFile } = await import('node:fs/promises');
+    const build = await readFile(join(root, 'cli/tui/scripts/build.ts'), 'utf8');
+    assert.match(build, /createSolidTransformPlugin/);
+    assert.match(build, /plugins: \[createSolidTransformPlugin\(\)\]/);
+    // The CLI form cannot inherit the plugin, so it must not come back.
+    assert.doesNotMatch(build, /spawnSync\("bun", \["build"/);
+  });
+
+  it('interactive smoke fails a binary that renders but ignores input', () => {
+    const r = spawnSync(process.execPath, [join(root, 'scripts/smoke-tui-interactive.mjs'), '/no/such/binary'], {
+      encoding: 'utf8',
+    });
+    assert.notEqual(r.status, 0);
+    assert.match(`${r.stdout}${r.stderr}`, /binary not found/);
+  });
 });