mirror of
https://github.com/ruvnet/RuView
synced 2026-08-10 20:31:42 +00:00
rUv
e6f26e9ac9
* docs(adr): deep review of the RuView npm surface — ADR-263/264/265 optimization strategies
ADR-263 — @ruvnet/ruview@0.1.0 harness review (O1–O9):
- HIGH: claim-check CLI fails open on empty input (no --text/--file -> PASS exit 0)
- HIGH: MCP stdio server head-of-line blocking (spawnSync verify/calibrate up to 600s)
- MEASURED: optionalDependencies triple the cold npx install (4 pkgs/620kB/71 files
vs 1 pkg/172kB/22 files with --omit=optional) for a path that never imports them
- maxBuffer truncation, python -c port interpolation, version drift, duplicate skills,
guardrail METRIC_TERMS substring false positives ('map'/'F1' — found by dogfooding
claim-check on these very ADRs), zero CI
ADR-264 — @ruvnet/rvagent@0.1.0 + @ruv/ruview-cli review (O1–O9), verified against
the published registry tarball:
- HIGH: exports.require -> dist/index.cjs which is never built nor published
- MEASURED: 44 dead source-map files = 62,698B of the 188kB unpacked payload
- stdio-only server described as dual-transport; mixed dot/underscore tool names;
double Zod validation + hand-duplicated advertised schemas; 2-fd leak per training
job; unbounded body in the unwired HTTP scaffold; dead detectCogBinary candidates;
ruview bin-name collision
ADR-265 — cross-cutting npm distribution strategy: npm-packages.yml CI matrix
(test + pack-content/size gate + tarball-install smoke test), publish-from-CI-only
with npm provenance, version single-sourcing from package.json, bin/namespace
ownership (ruview bin belongs to @ruvnet/ruview), claim-check on package READMEs.
Docs only — no runtime code changed. Index/CHANGELOG/CLAUDE.md/README counts updated.
Co-Authored-By: claude-flow <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_01WrGfTGKv1oWZ6iwXZACULz
* fix(npm): implement ADR-263/264/265 — harness fail-closed + async MCP, rvagent packaging/transport/naming, npm CI+provenance gate
ADR-263 (@ruvnet/ruview 0.2.0), O1-O9:
- claim-check fails closed on empty input (CLI exit 2, empty_text tool error)
- MCP stdio server dispatches tools/call asynchronously (promise-based spawn);
ping answers while a 3s fake verify runs — pinned by new e2e test
- optionalDependencies dropped: cold npx installs exactly 1 package
(MEASURED: was 4 pkgs/620kB/71 files via npm i in a clean prefix)
- bounded rolling output tails replace spawnSync 1MiB maxBuffer
- node_monitor port passed via sys.argv, never spliced into python -c source
- serverInfo.version read from package.json; resources/prompts stubs
- skills single-sourced: prepack sync script generates .claude/skills/ copies
- which() = memoized dep-free PATH scan
- tools underscore-canonical (ruview_claim_check, ...) + dotted aliases
- guardrail precision: word-boundary map/f1/auc/iou, code-span + F1/O2 label
scrubbing, quantitative-claims-only; packaging reproducer hints
- 30/30 tests (was 17), incl. concurrency e2e + fail-open regression pins
ADR-264 (@ruvnet/rvagent 0.2.0), O1-O9:
- exports fixed: types-first, phantom dist/index.cjs require target removed
- tarball map-free: 127,704B unpacked / 46 files / 0 maps (MEASURED,
npm pack --dry-run; was 188kB incl. 44 maps referencing unshipped src)
- Streamable HTTP actually wired behind RVAGENT_HTTP_PORT: one transport +
one MCP server per session (mcp-session-id routing), 1MiB body cap (413),
port-aware localhost origin gate; dual-transport description now true
- tools renamed underscore-canonical with dotted router-only aliases
- single Zod validation gate; advertised inputSchema generated from the same
Zod source (zod-to-json-schema)
- train_count: parent log fds closed (was leaking 2/job); job records
persisted to <jobsDir>/<id>.json (job_status survives restarts); bounded
log-tail reads
- detectCogBinary probes its candidates instead of dead-coding them
- version from package.json; @types/express dropped; @types/jest -> 29
- README rewritten to match reality (no phantom subcommands/policy layer)
- 99/99 jest tests (incl. new session/body-cap suite + previously-broken
manifest suite); stdio handshake + HTTP session flow smoke-tested live
ADR-265 D1-D4:
- .github/workflows/npm-packages.yml: 3-package x Node 20/22 gate — tests,
version-literal grep (D3), pack-content/size gate, tarball-install smoke
test (catches the ADR-264 F1 class), README claim-check (D4)
- .github/workflows/ruview-npm-release.yml: publish from CI only with
npm publish --provenance
- @ruv/ruview-cli bin renamed ruview-cli (ruview bin belongs to
@ruvnet/ruview); version single-sourced
- ci.yml NODE_VERSION 18 -> 20
ADR statuses updated to Accepted/implemented; harness manifest re-pinned;
ADR-263/264/265 + both package READMEs pass claim-check.
Co-Authored-By: claude-flow <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_01WrGfTGKv1oWZ6iwXZACULz
* perf(rvagent): lazy-load HTTP transport + memoize generated tool schemas
stdio time-to-first-response ~242ms -> ~189ms (-22%; MEASURED, median of
repeated initialize round-trips against dist/index.js in this container).
- ./http-transport.js now imported lazily inside the RVAGENT_HTTP_PORT
branch: it chain-loads the MCP SDK streamableHttp module (~48ms MEASURED
via per-module import() timing) which the default stdio path never uses
- toolInputJsonSchema memoized per tool: schemas are static for the process
lifetime; under the session-per-server HTTP model every session calls
tools/list, so stop re-walking the Zod tree each time
No behavior change: 99/99 jest tests; HTTP session flow re-smoke-tested
through the lazy import path (initialize -> 200 + mcp-session-id).
Profiled @ruvnet/ruview too and left it alone: 50ms CLI startup vs ~29ms
bare 'node -e ""' floor on the same box (MEASURED) — already near the
interpreter floor with zero dependencies.
Co-Authored-By: claude-flow <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_01WrGfTGKv1oWZ6iwXZACULz
* ci(ruview-cli): pass jest --passWithNoTests so the private no-test package doesn't fail the npm-packages matrix
Co-Authored-By: claude-flow <ruv@ruv.net>
* fix(npm): address 10 verified review findings in harness + rvagent before 0.2.0 publish
harness/ruview (@ruvnet/ruview):
- guardrails: digit gate now sees numbers inside code spans; F1-style
metric tokens followed by ':' or a nearby number are no longer scrubbed
(fail-open regressions in the honesty gate)
- mcp-server: tools/call requests serialize through a FIFO promise chain
(hardware/mutating tools never overlap) while ping/tools/list stay
immediate; stdin close drains in-flight responses before exit
- tools: which() no longer memoizes negative lookups
tools/ruview-mcp (@ruvnet/rvagent):
- index: realpath invoked-directly guard — library import no longer
connects a stdio transport to the consumer's process
- http-transport: explicit allowedOrigins is exact-match only (localhost
any-port convenience applies only with no configured allowlist);
session map gains maxSessions=64 + 5min idle TTL sweep
- train-count: job records persist the child pid and reconcile stale
'running' status after a server restart (exit-code marker or dead pid)
- config: cog binary candidates ordered by process.arch
.github/workflows/ruview-npm-release.yml: port the full ADR-265 D1 gate
(version-literal check, unpacked-size budget, tarball-install smoke test)
from npm-packages.yml so the publish path enforces what the header claims.
Tests: harness 30→36, rvagent 99→112, all passing.
Co-Authored-By: claude-flow <ruv@ruv.net>
---------
Co-authored-by: Claude <noreply@anthropic.com>
192 lines
11 KiB
Markdown
192 lines
11 KiB
Markdown
# ADR-263: `@ruvnet/ruview` npm Harness — Deep Review + Optimization Strategy
|
||
|
||
| Field | Value |
|
||
|-------|-------|
|
||
| **Status** | Accepted — **implemented** (O1–O9, `@ruvnet/ruview@0.2.0`): fail-closed `claim-check`, async MCP dispatch (ping answered mid-`verify`, pinned by e2e test), zero-dependency install, bounded output tails, argv-passed monitor port, package.json-sourced version, prepack skill sync, memoized `which()`, underscore-canonical tools with dotted aliases, word-boundary guardrail matching. 30/30 tests (MEASURED, `node --test test/*.test.mjs`); CI gate in ADR-265's `npm-packages.yml` |
|
||
| **Date** | 2026-07-02 |
|
||
| **Deciders** | ruv |
|
||
| **Codename** | **RUVIEW-NPM-REVIEW-1** |
|
||
| **Supersedes / amends** | none (records review of the ADR-182 P1+P2 artifact; feeds ADR-265 distribution strategy) |
|
||
|
||
## Context
|
||
|
||
ADR-182 minted and published **`@ruvnet/ruview@0.1.0`** (`harness/ruview/`) — the
|
||
`npx ruview` operator harness: a dependency-free ESM CLI + minimal MCP stdio server
|
||
exposing six `ruview.*` tools (onboard / claim_check / verify / node_monitor /
|
||
calibrate / node_flash), five skill playbooks, and the executable
|
||
MEASURED-vs-CLAIMED guardrail (`src/guardrails.js`). The package is live on npm
|
||
(0.1.0, 49.5 kB unpacked / 21 files — MEASURED, `npm view @ruvnet/ruview` +
|
||
`npm pack --dry-run`) and is the recommended MCP registration path
|
||
(`npx -y @ruvnet/ruview mcp start` in the bundled `.claude/settings.json`).
|
||
|
||
This ADR is the first dedicated deep review of that npm artifact: correctness,
|
||
fail-open/fail-closed posture, performance (cold start + request handling),
|
||
packaging hygiene, and security of the subprocess surface. All 17 bundled tests
|
||
pass on Node 22 (MEASURED, `node --test test/*.test.mjs`, 17/17, ~108 ms).
|
||
|
||
## Findings
|
||
|
||
Severity reflects impact on the package's stated contract: *fail-closed operator
|
||
tools + an honesty guardrail that must never fail open*.
|
||
|
||
### F1 (HIGH, fail-open): `claim-check` passes silently on empty input
|
||
|
||
`bin/cli.js` `claim-check` with **neither `--text` nor `--file`** sends
|
||
`text: undefined` → `claimCheck(String(args.text ?? ''))` → `''` → `ok: true`,
|
||
**exit 0**. A CI hook wired as `npx ruview claim-check --text "$BODY"` where
|
||
`$BODY` expands empty therefore reports PASS. This is the single tool whose whole
|
||
purpose is to fail closed; empty input must be an error, not a pass.
|
||
Reproducer: `node bin/cli.js claim-check` → `{"ok": true}`, exit 0.
|
||
|
||
### F2 (HIGH, head-of-line blocking): MCP server is fully synchronous
|
||
|
||
`src/mcp-server.js` dispatches `tools/call` inside the readline `line` handler,
|
||
and every heavyweight handler in `src/tools.js` uses **`spawnSync`**
|
||
(`ruview.verify` up to 180 s, `ruview.calibrate` up to 300–600 s,
|
||
`ruview.node_monitor` up to `seconds+10`). While one call runs, the event loop is
|
||
blocked: `ping`, `tools/list`, and concurrent `tools/call` requests are not even
|
||
read from stdin. Hosts that health-check with `ping` during a long `calibrate`
|
||
will conclude the server is dead and kill it mid-run.
|
||
|
||
### F3 (MEDIUM, cold start): optionalDependencies triple the `npx` install for a path that never uses them
|
||
|
||
`package.json` declares `optionalDependencies` on `@metaharness/kernel` and
|
||
`@metaharness/host-claude-code`. npm installs optional deps **by default**, so
|
||
every cold `npx -y @ruvnet/ruview mcp start` fetches 3 extra packages (kernel +
|
||
host + transitive `@ruvector/emergent-time`). MEASURED (npm 10.9.7, this
|
||
container): default install = **4 packages, 620 kB, 71 files**; with
|
||
`--omit=optional` = **1 package, 172 kB, 22 files**. The operator-tool and MCP
|
||
paths never import these — only `doctor`/`install` do, and both already
|
||
dynamic-import inside `try/catch` and degrade gracefully when absent
|
||
(`kernel/host: not installed (ok…)`). The optional deps buy nothing on the hot
|
||
path and cost 3 registry round-trips + ~450 kB on every cold start.
|
||
|
||
### F4 (MEDIUM, silent truncation): `spawnSync` default `maxBuffer` (1 MiB)
|
||
|
||
`run()` in `src/tools.js` never sets `maxBuffer`. `cargo run -p
|
||
wifi-densepose-cli` (the `calibrate` fallback path) and a chatty `verify.py` can
|
||
exceed 1 MiB of stdout, at which point the child is killed with `ENOBUFS` and the
|
||
tool reports a spawn error that looks like a proof/calibration failure. The
|
||
handlers only ever consume the last 8 kB/1.5 kB; buffering should be bounded but
|
||
generous (e.g. `maxBuffer: 16 MiB`) or streamed with a tail ring.
|
||
|
||
### F5 (MEDIUM, injection surface): `node_monitor` interpolates the port into Python source
|
||
|
||
The handler builds a `python -c` script by string interpolation:
|
||
`` `ser=serial.Serial(${JSON.stringify(port)},115200,…)` `` and
|
||
`` `while time.time()-t<${dur}:` ``. `JSON.stringify` produces a *JavaScript*
|
||
string literal; Python string-literal semantics differ at the edges (`\uXXXX` is
|
||
shared, but e.g. JS emits raw U+2028/U+2029 unescaped pre-ES2019 rules aside, and
|
||
any future non-JSON-safe field added the same way would be executable). `port`
|
||
arrives from the MCP caller (an agent), so this is an agent-controlled string
|
||
concatenated into an interpreter invocation. `dur` is `Number()`-guarded; `port`
|
||
should be passed out-of-band (`sys.argv`/env), never spliced into source.
|
||
|
||
### F6 (LOW, drift): server version hardcoded
|
||
|
||
`SERVER_INFO = { name: 'ruview', version: '0.1.0' }` in `src/mcp-server.js`
|
||
duplicates `package.json.version` (the CLI's `--version` already reads
|
||
package.json at runtime). First release bump will drift the MCP handshake
|
||
version.
|
||
|
||
### F7 (LOW, duplication): every skill ships twice
|
||
|
||
`skills/*.md` and `.claude/skills/*/SKILL.md` are byte-identical (same sha256 in
|
||
`.harness/manifest.json`). ~8 kB of the 49.5 kB unpacked payload is duplicate
|
||
content, and — worse than size — two copies must be kept in sync by hand.
|
||
|
||
### F8 (LOW, perf + portability): `which()` is uncached and shells out
|
||
|
||
`which()` runs up to twice per tool call (`python` then `python3`), each a
|
||
blocking `spawnSync`; the POSIX branch spawns a shell (`shell: true`). Results
|
||
are stable for the process lifetime and should be memoized; the lookup can be
|
||
done dep-free with a PATH scan instead of a shell.
|
||
|
||
### F9 (LOW, interop): dot-named tools + minimal protocol surface
|
||
|
||
Tool names (`ruview.onboard`, `ruview.claim_check`, …) contain dots. MCP itself
|
||
does not restrict names, but downstream host APIs commonly enforce
|
||
`^[a-zA-Z0-9_-]{1,64}$` for tool names; hosts must then sanitize or reject.
|
||
The server also answers `resources/list` / `prompts/list` with `-32601` (it does
|
||
not advertise those capabilities, so this is spec-legal, but empty-list stubs are
|
||
cheaper than every host's error path). Protocol version is pinned to
|
||
`2024-11-05` with no negotiation fallback. None of this breaks Claude Code today;
|
||
it narrows portability, which is the harness's whole pitch (9 hosts, ADR-182).
|
||
|
||
### F10 (LOW, CI gap): the published package has zero CI
|
||
|
||
No workflow under `.github/workflows/` runs `harness/ruview` tests (checked:
|
||
no workflow references `harness/ruview`, `ruview-mcp`, or `ruview-cli`), and
|
||
`ci.yml` pins `NODE_VERSION: '18'` while the package declares
|
||
`engines.node >= 20`. Note also `node --test test/` (directory form) fails on
|
||
Node 22 while the documented glob form passes — CI should pin the working
|
||
invocation. Consolidated CI/publish strategy is ADR-265.
|
||
|
||
### F11 (MEDIUM, guardrail precision): `METRIC_TERMS` substring matching false-positives on ordinary prose
|
||
|
||
Found by dogfooding this review: `claimCheck` matches metric terms with
|
||
`lower.includes(t)`, so the two-character terms `'map'` and `'f1'` fire inside
|
||
ordinary words and labels — "source **map**s", "the **map**s can never
|
||
resolve", finding IDs like "**F1** (HIGH…)". MEASURED reproducer: running
|
||
`npx ruview claim-check --file` over this ADR and ADR-264 yields 4 and 16
|
||
medium findings respectively, the majority of which are `map`/`F1`
|
||
false positives on lines carrying no accuracy claim. A guardrail that cries
|
||
wolf trains people to ignore it — precision is part of its fail-closed
|
||
contract. Short/ambiguous terms need word-boundary matching (`\bmap\b`,
|
||
`\bf1\b`, likewise `auc`, `iou`), and section-heading label patterns
|
||
(`F\d+`, `O\d+`) should not count as metric mentions.
|
||
|
||
## Decision
|
||
|
||
Adopt the following optimization strategy, in priority order. Each item is
|
||
independently shippable; F-numbers map to findings.
|
||
|
||
- **O1 (F1):** `claim-check` with no `--text`/`--file` (or empty text after read)
|
||
exits 2 with a usage error. Add a regression test pinning exit ≠ 0.
|
||
- **O2 (F2):** make the MCP dispatch async: convert `run()`/`which()` to
|
||
promise-based `spawn`, make `tools/call` handlers `async`, and keep reading
|
||
stdin while calls run (respond to `ping`/`tools/list` concurrently; serialize
|
||
only same-tool hardware operations). Acceptance: `ping` round-trips < 50 ms
|
||
while a synthetic 30 s `calibrate` is in flight.
|
||
- **O3 (F3):** drop the two `optionalDependencies`; `doctor`/`install` already
|
||
degrade and should print the exact `npm i @metaharness/kernel
|
||
@metaharness/host-claude-code` hint on the miss path. Acceptance: cold
|
||
`npm i @ruvnet/ruview` installs exactly 1 package (MEASURED baseline above).
|
||
- **O4 (F4):** set `maxBuffer: 16 * 1024 * 1024` in `run()` (or stream + tail).
|
||
- **O5 (F5):** pass `port` to the monitor script via `sys.argv`
|
||
(`python -c script -- <port>`), never by source interpolation.
|
||
- **O6 (F6):** read the MCP `serverInfo.version` from `package.json` once at
|
||
startup (same pattern the CLI already uses).
|
||
- **O7 (F7):** make `skills/*.md` the single source and generate
|
||
`.claude/skills/*/SKILL.md` in a `prepack` script (or vice versa); manifest
|
||
hashes then pin one canonical set.
|
||
- **O8 (F8, F9):** memoize `which()`; add underscore aliases for the dot-named
|
||
tools (accept both in `tools/call`, advertise the underscore form) and add
|
||
empty `resources/list` / `prompts/list` stubs.
|
||
- **O9 (F11):** switch `METRIC_TERMS` matching to word-boundary regexes for
|
||
short terms (`map`, `f1`, `auc`, `iou`) and skip label tokens matching
|
||
`\b[FO]\d+\b`. Acceptance: `claim-check --file` over ADR-263/264/265 reports
|
||
only the genuinely tagged-or-taggable percentage lines, and the existing 17
|
||
guardrail tests still pass plus new false-positive pins ("source maps",
|
||
"F1 (HIGH)" → no finding).
|
||
|
||
Non-goals: no new runtime dependencies (the zero-dep MCP server is a feature,
|
||
not an accident — keep it), no build step, no change to the fail-closed tool
|
||
contracts.
|
||
|
||
## Consequences
|
||
|
||
- The honesty guardrail becomes fail-closed end-to-end (its current empty-input
|
||
pass is the exact failure mode the guardrail exists to prevent).
|
||
- `npx` cold start drops ~450 kB / 3 packages (MEASURED baseline in F3) with no
|
||
feature loss; `doctor` output already communicates the optional-dep story.
|
||
- Long-running `verify`/`calibrate` no longer starve the MCP channel — the
|
||
harness survives host health checks during real calibration runs.
|
||
- Two-copy skill drift becomes impossible at pack time.
|
||
- Costs: async conversion touches every handler signature in `src/tools.js`
|
||
(mechanical, ~6 handlers); alias tools add a small compatibility table.
|
||
- Verification for the implementing PR: bundled tests extended for O1/O2/O5
|
||
(target ≥ 20 tests), `npm pack --dry-run` file-count asserted, and the F3
|
||
install measurement re-run and quoted MEASURED in the PR body — which must
|
||
itself pass `npx ruview claim-check`.
|