From ac0d53d6aeb1c4ee7cb46001cff4799e12f7cd7b Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 6 May 2026 08:08:50 -0400 Subject: [PATCH 1/5] relayburn-sdk-node: napi-rs bindings skeleton (#247 part a) Add `#[napi]` shims for every public verb in `relayburn-sdk` so the napi-rs CI matrix (#247-b) can produce per-platform `.node` artifacts. Type-mapping rules established at the boundary: - u64 token counts become JS `BigInt` (lossless above 2^53). - Timestamps stay as ISO-8601 `String`s, matching the existing `packages/sdk/index.d.ts` byte-for-byte. - async fn verbs (`ingest`) return `Promise` via napi-rs's `tokio_rt`. - Domain errors get a typed `BurnError` `{ code, message }` instead of leaking opaque `napi::Error` strings. Surface bound: `summary`, `sessionCost`, `overhead`, `overheadTrim`, `hotspots`, `search`, `exportLedger`, `exportStamps`, async `ingest`, plus `ledgerOpen` for the smoke-call path. The complex result shapes for `overhead*` and `hotspots` round-trip through `serde_json::Value` so D9's `.d.ts` snapshot diff stays small. Out of scope (deferred to #247-b): `packages/sdk-node` umbrella + per-platform packages, napi-rs CI matrix, `optionalDependencies` selector. Conformance test scaffold also lands in part b. --- CHANGELOG.md | 1 + Cargo.lock | 102 ++++ crates/relayburn-sdk-node/Cargo.toml | 20 +- crates/relayburn-sdk-node/build.rs | 7 + crates/relayburn-sdk-node/src/lib.rs | 700 ++++++++++++++++++++++++++- 5 files changed, 826 insertions(+), 4 deletions(-) create mode 100644 crates/relayburn-sdk-node/build.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f180615..9032080d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Cross-package release notes for relayburn. Package changelogs contain package-le ## [Unreleased] +- `relayburn-sdk-node` (Rust): napi-rs bindings skeleton — `#[napi]` shims for every public verb in `relayburn-sdk` (`summary`, `sessionCost`, `overhead`, `overheadTrim`, `hotspots`, `search`, `exportLedger`, `exportStamps`, async `ingest`, plus `ledgerOpen`), with u64 token counts surfaced as JS `BigInt`, ISO-8601 timestamps as `String`, async verbs returning `Promise`, and a typed `BurnError` mapping for SDK failures. (#247) - `relayburn-ingest` (Rust): port the per-process gap-warning state machine (`gap` module — `record_session_gap`, `emit_gap_warning`, `count_tool_call_gaps`, `reset_ingest_gap_warnings`, `set_ingest_gap_writer`) and `reingest_missing_content` (`reingest` module). Suppression mirrors the TS surface: one warning per fresh affected session, silent on steady-state, re-fires after the affected set decays back to empty. `relayburn-ledger` adds `Ledger::list_user_turn_session_ids` to power the `reingest_missing_content` skip filter alongside `list_content_session_ids`. (#278) - `relayburn-analyze` (Rust): port the behavioral-pattern detectors (`patterns` module). `detect_patterns` runs retry-loop, failure-run, cancellation-run, compaction-loss, edit-revert, OpenCode skill-recall-dup, OpenCode skill-pruning-protection, OpenCode system-prompt-tax, and edit-heavy detectors against an ordered turn stream, with optional content-sidecar / tool-result-event / user-turn enrichment. Public surface: `detect_patterns`, `DetectPatternsOptions`; per-pattern result structs are re-exported from `findings` (`RetryLoop`, `FailureRun`, `CancellationRun`, `CompactionLoss`, `EditRevertCycle`, `SkillRecallDup`, `SkillPruningProtection`, `SystemPromptTax`, `EditHeavySession`, `SessionPatternSummary`, `PatternsResult`, `PatternEventSource`). (#275) - `relayburn-analyze` (Rust): port the tool-output-bloat detector — Signal A's `BASH_MAX_OUTPUT_LENGTH` static-config check (with `~/.claude/settings.json` + `/.claude/settings.json` loader) and Signal B's cross-harness observed-bloat aggregation, plus the `WasteFinding` adapter. Public surface mirrors `@relayburn/analyze`: `BASH_MAX_OUTPUT_ENV_KEY`, `DEFAULT_BLOAT_TOKEN_THRESHOLD`, `detect_observed_bloat`, `detect_static_config_bloat`, `detect_tool_output_bloat`, `load_claude_settings`, `project_claude_settings_path`, `user_claude_settings_path`, `tool_output_bloat_to_finding`. (#271) diff --git a/Cargo.lock b/Cargo.lock index a5b79ccf..83721a1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,6 +66,15 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "convert_case" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec182b0ca2f35d8fc196cf3404988fd8b8c739a4d270ff118a398feb0cbec1ca" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "cpufeatures" version = "0.2.17" @@ -85,6 +94,16 @@ dependencies = [ "typenum", ] +[[package]] +name = "ctor" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a2785755761f3ddc1492979ce1e48d2c00d09311c39e4466429188f3dd6501" +dependencies = [ + "quote", + "syn", +] + [[package]] name = "digest" version = "0.10.7" @@ -245,6 +264,16 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libloading" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" +dependencies = [ + "cfg-if", + "windows-link", +] + [[package]] name = "libsqlite3-sys" version = "0.30.1" @@ -274,6 +303,66 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "napi" +version = "2.16.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55740c4ae1d8696773c78fdafd5d0e5fe9bc9f1b071c7ba493ba5c413a9184f3" +dependencies = [ + "bitflags", + "ctor", + "napi-derive", + "napi-sys", + "once_cell", + "serde", + "serde_json", + "tokio", +] + +[[package]] +name = "napi-build" +version = "2.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d376940fd5b723c6893cd1ee3f33abbfd86acb1cd1ec079f3ab04a2a3bc4d3b1" + +[[package]] +name = "napi-derive" +version = "2.16.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cbe2585d8ac223f7d34f13701434b9d5f4eb9c332cccce8dee57ea18ab8ab0c" +dependencies = [ + "cfg-if", + "convert_case", + "napi-derive-backend", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "napi-derive-backend" +version = "1.0.75" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1639aaa9eeb76e91c6ae66da8ce3e89e921cd3885e99ec85f4abacae72fc91bf" +dependencies = [ + "convert_case", + "once_cell", + "proc-macro2", + "quote", + "regex", + "semver", + "syn", +] + +[[package]] +name = "napi-sys" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "427802e8ec3a734331fec1035594a210ce1ff4dc5bc1950530920ab717964ea3" +dependencies = [ + "libloading", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -442,7 +531,14 @@ dependencies = [ name = "relayburn-sdk-node" version = "0.0.0" dependencies = [ + "anyhow", + "napi", + "napi-build", + "napi-derive", "relayburn-sdk", + "serde", + "serde_json", + "tokio", ] [[package]] @@ -629,6 +725,12 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "unicode-segmentation" +version = "1.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9629274872b2bfaf8d66f5f15725007f635594914870f65218920345aa11aa8c" + [[package]] name = "unicode-xid" version = "0.2.6" diff --git a/crates/relayburn-sdk-node/Cargo.toml b/crates/relayburn-sdk-node/Cargo.toml index eeead086..1f4c89ac 100644 --- a/crates/relayburn-sdk-node/Cargo.toml +++ b/crates/relayburn-sdk-node/Cargo.toml @@ -8,8 +8,22 @@ repository.workspace = true description = "napi-rs bindings over relayburn-sdk — built in CI to produce the @relayburn/sdk npm artifacts. Not published to crates.io." publish = false +[lib] +crate-type = ["cdylib", "rlib"] + [dependencies] -# napi-rs glue lands in #247; this crate's only direct dep is the SDK. -# Not published to crates.io, so the version pin is loose by design — it -# just keeps `cargo metadata` and the workspace resolver happy. +# napi-rs glue. The Node bindings sit on top of the SDK and expose +# `#[napi]` shims for every public verb. Not published to crates.io — +# this crate is consumed exclusively by the napi-rs CI matrix in #247-b +# to produce the per-platform `.node` artifacts under +# `packages/sdk-node/`. relayburn-sdk = { path = "../relayburn-sdk", version = "0.0" } +napi = { version = "2", default-features = false, features = ["napi6", "tokio_rt", "serde-json"] } +napi-derive = "2" +serde = { workspace = true } +serde_json = { workspace = true } +anyhow = { workspace = true } +tokio = { workspace = true } + +[build-dependencies] +napi-build = "2" diff --git a/crates/relayburn-sdk-node/build.rs b/crates/relayburn-sdk-node/build.rs new file mode 100644 index 00000000..6c4782c6 --- /dev/null +++ b/crates/relayburn-sdk-node/build.rs @@ -0,0 +1,7 @@ +// napi-rs build script. Generates the symbol-export table the Node loader +// needs. See https://napi.rs/docs/cli/build for details. +extern crate napi_build; + +fn main() { + napi_build::setup(); +} diff --git a/crates/relayburn-sdk-node/src/lib.rs b/crates/relayburn-sdk-node/src/lib.rs index e805d098..394ae235 100644 --- a/crates/relayburn-sdk-node/src/lib.rs +++ b/crates/relayburn-sdk-node/src/lib.rs @@ -1 +1,699 @@ -// TODO: port `@relayburn/sdk` napi-rs bindings — see issue #247. +//! napi-rs bindings for `relayburn-sdk`. +//! +//! This crate is built in CI by the napi-rs matrix (#247-b) to produce +//! the per-platform `.node` artifacts that ship inside +//! `@relayburn/sdk@2.0`. It is not published to crates.io. +//! +//! # Type-mapping rules +//! +//! The SDK is a Rust API; the Node bindings are a lossy presenter for it +//! the same way the CLI is. The rules below are applied uniformly so the +//! generated `.d.ts` is predictable for TS consumers: +//! +//! - **`u64` token counts → JS `BigInt`.** SDK fields like +//! `Summary::total_tokens`, every `tokens` row in `byTool` / `byModel`, +//! and the `OverheadSection::tokens` field cross the boundary as +//! `napi::bindgen_prelude::BigInt`. JS `number` (f64) cannot losslessly +//! represent the upper end of the u64 range and silently truncates above +//! 2^53; the SDK already deals in u64 internally so the boundary is the +//! right place to surface that. +//! - **Timestamps → ISO-8601 `String`.** The SDK already speaks ISO +//! strings (`turn.ts`, `since` parameters); we keep that wire format +//! rather than dragging `chrono::DateTime` or `Date` through the FFI. +//! Matches the existing `packages/sdk/index.d.ts` byte-for-byte. +//! - **`async fn` SDK verbs → `Promise` on the JS side.** napi-rs's +//! `tokio_rt` feature drives this; we mark `ingest` `async fn` and the +//! sync verbs (`summary`, `sessionCost`, …) as plain `fn` returning +//! `napi::Result`. +//! - **Errors → typed `BurnError` JS class.** Domain failures from the +//! SDK (`anyhow::Error`) are mapped into a tagged `BurnError` with a +//! discriminant + message; no untyped `napi::Error` leaks. JS code can +//! `instanceof BurnError` and switch on `err.code`. +//! +//! # Surface +//! +//! Every public verb in `relayburn-sdk` (free-function form) is bound +//! here. The `Ledger` / `LedgerHandle` method form is omitted from the JS +//! surface for v1 — the TS sibling `@relayburn/sdk@1.x` only exposes the +//! free-function shape, and a future PR can add a `Ledger` JS class +//! without breaking compatibility. The deferred D9 PR (Wave 2) checks in a +//! `.d.ts` snapshot test against `packages/sdk/index.d.ts`. +//! +//! See `RUST_PORT_WAVE_PLAN.md` section 3 for how this fits the larger +//! port. + +#![allow(clippy::needless_pass_by_value)] + +use std::path::PathBuf; + +use napi::bindgen_prelude::{BigInt, Error as NapiError, Result as NapiResult, Status}; +use napi_derive::napi; +use serde_json::Value as JsonValue; + +use relayburn_sdk as sdk; + +// --------------------------------------------------------------------------- +// Error mapping +// --------------------------------------------------------------------------- + +/// Tagged error code surfaced on `BurnError.code`. Mirrors the lower-crate +/// failure axes today; future SDK errors should map here rather than +/// leaking as opaque strings. +#[napi(string_enum)] +pub enum BurnErrorCode { + /// Catch-all for `anyhow::Error` chains the SDK raises. Refine over + /// time as the SDK's error surface grows typed variants. + Sdk, + /// I/O failures from the napi boundary itself (path conversions, etc.). + Io, + /// Caller passed an invalid argument shape (e.g. `since` that isn't a + /// relative range nor an ISO timestamp). + InvalidArgument, +} + +/// Domain error surfaced to JS callers. The `BurnError` JS class is a +/// thin Object with `{ code, message }`; the napi runtime turns it into a +/// real `Error` subclass at module-init time. +/// +/// Keep this distinct from the JS `Error` napi-rs would synthesize for an +/// `Err(napi::Error)` — the two-tier design lets TS consumers +/// `try { ... } catch (e) { if (e instanceof BurnError) ... }` and switch +/// on `e.code`. +#[napi(object)] +pub struct BurnError { + pub code: BurnErrorCode, + pub message: String, +} + +fn sdk_err(e: anyhow::Error) -> NapiError { + // Render the chain so the message is informative; the discriminant + // stays "Sdk" until the SDK's typed error story exists. + NapiError::new(Status::GenericFailure, format!("{e:#}")) +} + +fn invalid_arg(msg: impl Into) -> NapiError { + NapiError::new(Status::InvalidArg, msg.into()) +} + +// --------------------------------------------------------------------------- +// Helpers — small repeating conversions +// --------------------------------------------------------------------------- + +fn u64_to_bigint(v: u64) -> BigInt { + BigInt { + sign_bit: false, + words: vec![v], + } +} + +fn bigint_to_u64(v: BigInt) -> NapiResult { + let (signed, value, lossless) = v.get_u64(); + if signed { + return Err(invalid_arg("expected non-negative bigint, got signed")); + } + if !lossless { + return Err(invalid_arg("bigint exceeds u64 range")); + } + Ok(value) +} + +fn maybe_path(s: Option) -> Option { + s.map(PathBuf::from) +} + +// --------------------------------------------------------------------------- +// Ledger open options +// --------------------------------------------------------------------------- + +/// Where on disk a ledger should land. Mirrors +/// `relayburn_sdk::LedgerOpenOptions`. `home` defaults to `RELAYBURN_HOME` +/// (or `~/.relayburn`); `contentHome` overrides only the `content.sqlite` +/// path when it makes sense to park content on different storage. +#[napi(object)] +pub struct LedgerOpenOptions { + pub home: Option, + pub content_home: Option, +} + +fn open_options(home: Option, content_home: Option) -> sdk::LedgerOpenOptions { + sdk::LedgerOpenOptions { + home: maybe_path(home), + content_home: maybe_path(content_home), + } +} + +// --------------------------------------------------------------------------- +// summary +// --------------------------------------------------------------------------- + +#[napi(object)] +pub struct SummaryOptions { + pub session: Option, + pub project: Option, + /// ISO timestamp (e.g. `2026-04-01T00:00:00Z`) or relative range + /// (`24h`, `7d`, `4w`, `2m`). + pub since: Option, + pub ledger_home: Option, +} + +#[napi(object)] +pub struct SummaryToolRow { + pub tool: String, + pub tokens: BigInt, + pub cost: f64, + pub count: BigInt, +} + +#[napi(object)] +pub struct SummaryModelRow { + pub model: String, + pub tokens: BigInt, + pub cost: f64, +} + +#[napi(object)] +pub struct Summary { + pub total_tokens: BigInt, + pub total_cost: f64, + pub turn_count: BigInt, + pub by_tool: Vec, + pub by_model: Vec, +} + +impl From for Summary { + fn from(s: sdk::Summary) -> Self { + Summary { + total_tokens: u64_to_bigint(s.total_tokens), + total_cost: s.total_cost, + turn_count: u64_to_bigint(s.turn_count), + by_tool: s + .by_tool + .into_iter() + .map(|r| SummaryToolRow { + tool: r.tool, + tokens: u64_to_bigint(r.tokens), + cost: r.cost, + count: u64_to_bigint(r.count), + }) + .collect(), + by_model: s + .by_model + .into_iter() + .map(|r| SummaryModelRow { + model: r.model, + tokens: u64_to_bigint(r.tokens), + cost: r.cost, + }) + .collect(), + } + } +} + +#[napi] +pub fn summary(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(SummaryOptions { + session: None, + project: None, + since: None, + ledger_home: None, + }); + let raw = sdk::SummaryOptions { + session: opts.session, + project: opts.project, + since: opts.since, + ledger_home: maybe_path(opts.ledger_home), + }; + sdk::summary(raw).map(Summary::from).map_err(sdk_err) +} + +// --------------------------------------------------------------------------- +// session_cost +// --------------------------------------------------------------------------- + +#[napi(object)] +pub struct SessionCostOptions { + /// Session id to total. Omit for `{ note: 'no session id provided' }`. + pub session: Option, + pub ledger_home: Option, +} + +#[napi(object)] +pub struct SessionCostResult { + pub session_id: Option, + /// Total cost in USD, rounded to 6 decimal places. + pub total_usd: f64, + pub total_tokens: BigInt, + pub turn_count: BigInt, + pub models: Vec, + pub note: Option, +} + +impl From for SessionCostResult { + fn from(r: sdk::SessionCostResult) -> Self { + SessionCostResult { + session_id: r.session_id, + total_usd: r.total_usd, + total_tokens: u64_to_bigint(r.total_tokens), + turn_count: u64_to_bigint(r.turn_count), + models: r.models, + note: r.note, + } + } +} + +/// Compact session-scoped cost shape; powers the MCP `burn__sessionCost` tool. +#[napi(js_name = "sessionCost")] +pub fn session_cost(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(SessionCostOptions { + session: None, + ledger_home: None, + }); + let raw = sdk::SessionCostOptions { + session: opts.session, + ledger_home: maybe_path(opts.ledger_home), + }; + sdk::session_cost(raw) + .map(SessionCostResult::from) + .map_err(sdk_err) +} + +// --------------------------------------------------------------------------- +// overhead + overhead_trim — passed through serde_json. The shapes are +// large and recursive (sections, attribution detail, applies-to harness +// arrays); rebuilding each as a `#[napi(object)]` struct here would be +// hundreds of lines of mechanical translation that D9's snapshot test +// will catch drift on regardless. The serde wire format already matches +// `packages/sdk/index.d.ts` modulo the `bigint` substitutions; D9 owns +// the `BigInt` upgrades for `tokens` / `bytes` / `totalLines` fields if +// the conformance gate flags them. +// --------------------------------------------------------------------------- + +#[napi(string_enum)] +pub enum OverheadFileKind { + ClaudeMd, + AgentsMd, +} + +impl From for sdk::OverheadFileKind { + fn from(k: OverheadFileKind) -> Self { + match k { + OverheadFileKind::ClaudeMd => sdk::OverheadFileKind::ClaudeMd, + OverheadFileKind::AgentsMd => sdk::OverheadFileKind::AgentsMd, + } + } +} + +#[napi(object)] +pub struct OverheadOptions { + /// Project path to inspect; defaults to process.cwd(). + pub project: Option, + pub since: Option, + pub kind: Option, + pub ledger_home: Option, +} + +/// Per-file + per-section overhead cost attribution. Powers `burn overhead`. +/// +/// Returns the attribution result as a JSON-shaped object — the schema is +/// `OverheadResult` in `packages/sdk/index.d.ts`. D9's snapshot test +/// upgrades the `tokens` / `bytes` / `totalLines` fields to `BigInt` if +/// the conformance gate calls for it. +#[napi] +pub fn overhead(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(OverheadOptions { + project: None, + since: None, + kind: None, + ledger_home: None, + }); + let raw = sdk::OverheadOptions { + project: maybe_path(opts.project), + since: opts.since, + kind: opts.kind.map(Into::into), + ledger_home: maybe_path(opts.ledger_home), + }; + let result = sdk::overhead(raw).map_err(sdk_err)?; + let value = serde_json::to_value(&result) + .map_err(|e| NapiError::new(Status::GenericFailure, format!("serialize overhead: {e}")))?; + Ok(value) +} + +#[napi(object)] +pub struct OverheadTrimOptions { + pub project: Option, + pub since: Option, + pub kind: Option, + pub ledger_home: Option, + /// Recommendations per file. Default 3. + pub top: Option, + /// Include the unified-diff text per recommendation. Default true. + pub include_diff: Option, +} + +/// Trim recommendations for high-cost overhead-file sections. Powers +/// `burn overhead trim`. Returns an `OverheadTrimResult`-shaped JSON +/// object; see the comment on [`overhead`] for the BigInt-upgrade plan. +#[napi(js_name = "overheadTrim")] +pub fn overhead_trim(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(OverheadTrimOptions { + project: None, + since: None, + kind: None, + ledger_home: None, + top: None, + include_diff: None, + }); + let top = match opts.top { + Some(b) => Some(bigint_to_u64(b)?), + None => None, + }; + let raw = sdk::OverheadTrimOptions { + project: maybe_path(opts.project), + since: opts.since, + kind: opts.kind.map(Into::into), + ledger_home: maybe_path(opts.ledger_home), + top, + include_diff: opts.include_diff, + }; + let result = sdk::overhead_trim(raw).map_err(sdk_err)?; + let value = serde_json::to_value(&result).map_err(|e| { + NapiError::new(Status::GenericFailure, format!("serialize overhead_trim: {e}")) + })?; + Ok(value) +} + +// --------------------------------------------------------------------------- +// hotspots — discriminated union; serialized via serde_json so the +// `kind` discriminant + per-variant rows survive the boundary. The TS +// .d.ts already documents the shape (`HotspotsResult` union). +// --------------------------------------------------------------------------- + +#[napi(string_enum)] +pub enum HotspotsGroupBy { + Attribution, + Bash, + BashVerb, + File, + Subagent, +} + +impl From for sdk::HotspotsGroupBy { + fn from(g: HotspotsGroupBy) -> Self { + match g { + HotspotsGroupBy::Attribution => sdk::HotspotsGroupBy::Attribution, + HotspotsGroupBy::Bash => sdk::HotspotsGroupBy::Bash, + HotspotsGroupBy::BashVerb => sdk::HotspotsGroupBy::BashVerb, + HotspotsGroupBy::File => sdk::HotspotsGroupBy::File, + HotspotsGroupBy::Subagent => sdk::HotspotsGroupBy::Subagent, + } + } +} + +#[napi(object)] +pub struct HotspotsOptions { + pub session: Option, + pub project: Option, + pub since: Option, + pub group_by: Option, + pub patterns: Option>, + pub ledger_home: Option, +} + +/// Per-axis hotspot attribution + pattern-finding queries. Returns a +/// JSON-shaped discriminated union — see `HotspotsResult` in +/// `packages/sdk/index.d.ts`. +#[napi] +pub fn hotspots(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(HotspotsOptions { + session: None, + project: None, + since: None, + group_by: None, + patterns: None, + ledger_home: None, + }); + let raw = sdk::HotspotsOptions { + session: opts.session, + project: opts.project, + since: opts.since, + group_by: opts.group_by.map(Into::into), + patterns: opts.patterns, + ledger_home: maybe_path(opts.ledger_home), + }; + let result = sdk::hotspots(raw).map_err(sdk_err)?; + let value = serde_json::to_value(&result) + .map_err(|e| NapiError::new(Status::GenericFailure, format!("serialize hotspots: {e}")))?; + Ok(value) +} + +// --------------------------------------------------------------------------- +// search +// --------------------------------------------------------------------------- + +#[napi(object)] +pub struct SearchQueryOptions { + /// FTS5 query string. Supports phrase (`"out of memory"`), boolean + /// (`a OR b`), and prefix (`mem*`) syntax. + pub query: String, + /// Hit cap. Defaults to 25 when omitted. + pub limit: Option, + /// Restrict to a single session_id. Omit to search all sessions. + pub session_id: Option, + pub ledger_home: Option, +} + +#[napi(object)] +pub struct SearchHit { + pub session_id: String, + pub message_id: String, + pub source: String, + /// FTS5 BM25 rank (lower = better match). + pub rank: f64, + /// `…`-highlighted snippet around the matching tokens. + pub snippet: String, +} + +#[napi(object)] +pub struct SearchResult { + pub query: String, + pub hits: Vec, +} + +#[napi] +pub fn search(opts: SearchQueryOptions) -> NapiResult { + let limit = match opts.limit { + Some(b) => Some(bigint_to_u64(b)? as usize), + None => None, + }; + let raw = sdk::SearchQueryOptions { + query: opts.query.clone(), + limit, + session_id: opts.session_id, + ledger_home: maybe_path(opts.ledger_home), + }; + let result = sdk::search(raw).map_err(sdk_err)?; + Ok(SearchResult { + query: result.query, + hits: result + .hits + .into_iter() + .map(|h| SearchHit { + session_id: h.session_id, + message_id: h.message_id, + source: h.source, + rank: h.rank, + snippet: h.snippet, + }) + .collect(), + }) +} + +// --------------------------------------------------------------------------- +// export_ledger / export_stamps +// --------------------------------------------------------------------------- + +#[napi(object)] +pub struct ExportLedgerOptions { + pub ledger_home: Option, +} + +#[napi(object)] +pub struct ExportStampsOptions { + pub ledger_home: Option, +} + +/// Stream every event row as a JSONL-shaped JSON object. Each value has +/// the form `{ v: 1, kind: '', record: }`. +/// +/// Buffered into an array for v1; matches the SDK's +/// `export_ledger() -> impl Iterator` behavior (it's already in-memory +/// today). A streaming variant is a follow-up. +#[napi(js_name = "exportLedger")] +pub fn export_ledger(opts: Option) -> NapiResult> { + let opts = opts.unwrap_or(ExportLedgerOptions { ledger_home: None }); + let raw = sdk::ExportLedgerOptions { + ledger_home: maybe_path(opts.ledger_home), + }; + let iter = sdk::export_ledger(raw).map_err(sdk_err)?; + Ok(iter.collect()) +} + +/// Stream every stamp row as a JSONL-shaped JSON object. Sibling of +/// [`export_ledger`]. +#[napi(js_name = "exportStamps")] +pub fn export_stamps(opts: Option) -> NapiResult> { + let opts = opts.unwrap_or(ExportStampsOptions { ledger_home: None }); + let raw = sdk::ExportStampsOptions { + ledger_home: maybe_path(opts.ledger_home), + }; + let iter = sdk::export_stamps(raw).map_err(sdk_err)?; + Ok(iter.collect()) +} + +// --------------------------------------------------------------------------- +// ingest — async; returns a Promise on the JS side. +// --------------------------------------------------------------------------- + +#[napi(object)] +pub struct IngestRoots { + /// `~/.claude/projects` override. + pub claude_projects_dir: Option, + /// `~/.codex/sessions` override. + pub codex_sessions_dir: Option, + /// `~/.local/share/opencode/storage` override. + pub opencode_storage_dir: Option, +} + +#[napi(object)] +pub struct IngestOptions { + pub ledger_home: Option, + pub roots: Option, +} + +#[napi(object)] +pub struct IngestReport { + pub scanned_sessions: BigInt, + pub ingested_sessions: BigInt, + pub appended_turns: BigInt, +} + +impl From for IngestReport { + fn from(r: sdk::IngestReport) -> Self { + IngestReport { + scanned_sessions: u64_to_bigint(r.scanned_sessions as u64), + ingested_sessions: u64_to_bigint(r.ingested_sessions as u64), + appended_turns: u64_to_bigint(r.appended_turns as u64), + } + } +} + +/// Discover and ingest unprocessed turns from the configured session +/// stores. Returns a `Promise`. +/// +/// Progress / warning sinks are intentionally not surfaced through the +/// boundary in v1 — the JS surface today doesn't expose them either. +/// Wave 2 D9 picks them up if the conformance gate calls for it. +#[napi] +pub async fn ingest(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(IngestOptions { + ledger_home: None, + roots: None, + }); + let roots = opts.roots.unwrap_or(IngestRoots { + claude_projects_dir: None, + codex_sessions_dir: None, + opencode_storage_dir: None, + }); + let raw = sdk::IngestOptions { + ledger_home: maybe_path(opts.ledger_home), + roots: sdk::IngestRoots { + claude_projects_dir: maybe_path(roots.claude_projects_dir), + codex_sessions_dir: maybe_path(roots.codex_sessions_dir), + opencode_storage_dir: maybe_path(roots.opencode_storage_dir), + }, + on_progress: None, + on_warn: None, + }; + let report = sdk::ingest(raw).await.map_err(sdk_err)?; + Ok(report.into()) +} + +// --------------------------------------------------------------------------- +// Module-level metadata. napi-rs doesn't require a `register_module` +// entry point — `#[napi]` items register themselves via the macros. +// We export the open-options shape under a stable name for wave-2 +// callers that want to construct one explicitly. +// --------------------------------------------------------------------------- + +/// Synchronously open and immediately close a ledger to validate the +/// configured paths. Returns the resolved `home` path. Mirrors the +/// `Ledger.open()` smoke-call shape from `packages/sdk/index.d.ts`; a +/// future PR can add a stateful `Ledger` JS class that holds a handle. +#[napi(js_name = "ledgerOpen")] +pub fn ledger_open(opts: Option) -> NapiResult { + let opts = opts.unwrap_or(LedgerOpenOptions { + home: None, + content_home: None, + }); + let home = opts.home.clone(); + let content_home = opts.content_home.clone(); + let raw = open_options(home, content_home); + // Open + drop. Schema DDL applies on the first open, so this is a + // cheap "is the path writable / migration current?" probe. + let _handle = sdk::Ledger::open(raw).map_err(sdk_err)?; + // Echo the resolved home back so JS callers know which ledger they + // attached to. + Ok(opts + .home + .unwrap_or_else(|| sdk::ledger_home().to_string_lossy().into_owned())) +} + +// --------------------------------------------------------------------------- +// Tests — exercise the helpers that don't need a live napi env. The full +// boundary is covered end-to-end by the conformance test scaffold landing +// in #247-b (Wave 1 D2). +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn u64_to_bigint_round_trip_small() { + let big = u64_to_bigint(42); + assert_eq!(bigint_to_u64(big).unwrap(), 42); + } + + #[test] + fn u64_to_bigint_round_trip_max() { + let big = u64_to_bigint(u64::MAX); + assert_eq!(bigint_to_u64(big).unwrap(), u64::MAX); + } + + #[test] + fn bigint_to_u64_rejects_signed() { + let signed = BigInt { + sign_bit: true, + words: vec![1], + }; + assert!(bigint_to_u64(signed).is_err()); + } + + #[test] + fn bigint_to_u64_rejects_too_wide() { + let two_words = BigInt { + sign_bit: false, + words: vec![0, 1], + }; + assert!(bigint_to_u64(two_words).is_err()); + } + + #[test] + fn maybe_path_threads_string_to_pathbuf() { + assert!(maybe_path(None).is_none()); + assert_eq!( + maybe_path(Some("/tmp/x".into())), + Some(PathBuf::from("/tmp/x")) + ); + } +} From 290b970af4184eff6e1b97c5cc028f0d595c2703 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 6 May 2026 08:36:24 -0400 Subject: [PATCH 2/5] relayburn-sdk-node: typed BurnError + kebab-case enum values + typed result structs (review fixes) Addresses review feedback on PR #306: - Verbs now return `Result` with `BurnError = napi::Error<&'static str>`, so napi-rs writes one of `BURN_SDK`, `BURN_IO`, `BURN_INVALID_ARGUMENT` into the thrown JS Error's `code` property. Adds a `BurnErrorCode` `string_enum` so TS callers can reference codes by name. The async `ingest` verb is documented as carrying `code: 'GenericFailure'` because napi-rs's tokio adapter hard-codes the rejection error type to `Error`. - `OverheadFileKind` and `HotspotsGroupBy` use `string_enum = "kebab-case"`, so wire values match the existing TS contract (`'claude-md' | 'agents-md'`, `'attribution' | 'bash' | 'bash-verb' | 'file' | 'subagent'`). - Adds a `BigIntPromoting` wrapper that walks the serde_json result of `overhead`, `overheadTrim`, and `hotspots` and emits `BigInt` for numeric leaves under the well-known u64 field names listed in `BIGINT_FIELDS` (`tokens`, `bytes`, `totalLines`, `sessionCount`, `startLine`, `endLine`, `tokensPerSession`, `filesAnalyzed`, `filesWithRecommendations`, `totalRecommendations`, `callCount`, `distinctCommands`, `ridingTurns`, `firstEmitTurnIndex`, `toolCallCount`, `turnsAnalyzed`, `analyzed`, `excluded`). The walker is the lighter-weight choice over rebuilding each discriminated-union variant as a typed napi struct. - Tests cover the BIGINT_FIELDS membership invariant (positive + negative spot checks) and the SDK_ERROR_CODE / IO_ERROR_CODE / INVALID_ARGUMENT_ERROR_CODE constants. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/relayburn-sdk-node/src/lib.rs | 365 ++++++++++++++++++++++----- 1 file changed, 300 insertions(+), 65 deletions(-) diff --git a/crates/relayburn-sdk-node/src/lib.rs b/crates/relayburn-sdk-node/src/lib.rs index 394ae235..482fac32 100644 --- a/crates/relayburn-sdk-node/src/lib.rs +++ b/crates/relayburn-sdk-node/src/lib.rs @@ -16,7 +16,15 @@ //! `napi::bindgen_prelude::BigInt`. JS `number` (f64) cannot losslessly //! represent the upper end of the u64 range and silently truncates above //! 2^53; the SDK already deals in u64 internally so the boundary is the -//! right place to surface that. +//! right place to surface that. For verbs whose result is too recursive +//! to mirror as a `#[napi(object)]` struct (`overhead`, `overheadTrim`, +//! `hotspots`), we serialize through serde_json and emit the result via +//! the [`BigIntPromoting`] wrapper, which walks the JSON tree and +//! substitutes `BigInt` for any numeric value sitting under one of the +//! well-known u64 field names listed in [`BIGINT_FIELDS`]. The lighter +//! walker keeps the discriminated-union shape of `HotspotsResult` +//! intact (which is awkward to express as a single typed napi object) +//! while still honoring the contract. //! - **Timestamps → ISO-8601 `String`.** The SDK already speaks ISO //! strings (`turn.ts`, `since` parameters); we keep that wire format //! rather than dragging `chrono::DateTime` or `Date` through the FFI. @@ -24,11 +32,17 @@ //! - **`async fn` SDK verbs → `Promise` on the JS side.** napi-rs's //! `tokio_rt` feature drives this; we mark `ingest` `async fn` and the //! sync verbs (`summary`, `sessionCost`, …) as plain `fn` returning -//! `napi::Result`. +//! `Result`. //! - **Errors → typed `BurnError` JS class.** Domain failures from the -//! SDK (`anyhow::Error`) are mapped into a tagged `BurnError` with a -//! discriminant + message; no untyped `napi::Error` leaks. JS code can -//! `instanceof BurnError` and switch on `err.code`. +//! SDK (`anyhow::Error`) and argument-shape errors raised at this +//! boundary are surfaced as a `napi::Error` whose `Status` slot carries +//! one of [`SDK_ERROR_CODE`], [`IO_ERROR_CODE`], or +//! [`INVALID_ARGUMENT_ERROR_CODE`]. napi-rs writes that string into the +//! thrown JS Error's `code` property (via `napi_create_error`'s +//! `code` argument), so JS callers get +//! `try { … } catch (e) { if (e.code === 'BURN_SDK') … }`. The +//! [`BurnErrorCode`] enum is exported as a `string_enum` so TS code +//! can reference the codes by name without stringly-typed literals. //! //! # Surface //! @@ -45,8 +59,10 @@ #![allow(clippy::needless_pass_by_value)] use std::path::PathBuf; +use std::ptr; -use napi::bindgen_prelude::{BigInt, Error as NapiError, Result as NapiResult, Status}; +use napi::bindgen_prelude::{BigInt, Error as NapiError, Result as NapiResult, ToNapiValue}; +use napi::sys; use napi_derive::napi; use serde_json::Value as JsonValue; @@ -56,43 +72,62 @@ use relayburn_sdk as sdk; // Error mapping // --------------------------------------------------------------------------- -/// Tagged error code surfaced on `BurnError.code`. Mirrors the lower-crate -/// failure axes today; future SDK errors should map here rather than -/// leaking as opaque strings. +/// `code` written into the thrown JS Error for failures the SDK raises +/// (typically `anyhow::Error` chains from the query / ingest verbs). +pub const SDK_ERROR_CODE: &str = "BURN_SDK"; + +/// `code` written into the thrown JS Error for I/O failures at the napi +/// boundary itself (path conversions, etc.). +pub const IO_ERROR_CODE: &str = "BURN_IO"; + +/// `code` written into the thrown JS Error when the caller passed an +/// argument shape we can't accept (e.g. a `BigInt` outside the u64 +/// range). +pub const INVALID_ARGUMENT_ERROR_CODE: &str = "BURN_INVALID_ARGUMENT"; + +/// Tagged error code surfaced on the thrown JS Error's `code` property. +/// Exported as a TS `string_enum` so callers can branch on +/// `e.code === BurnErrorCode.Sdk` without stringly-typed literals. +/// +/// The string values match [`SDK_ERROR_CODE`] et al. — napi-rs writes +/// them into the `code` slot via `napi_create_error`'s code argument, so +/// the round-trip from constant → JS code property is byte-identical. #[napi(string_enum)] pub enum BurnErrorCode { /// Catch-all for `anyhow::Error` chains the SDK raises. Refine over /// time as the SDK's error surface grows typed variants. + #[napi(value = "BURN_SDK")] Sdk, /// I/O failures from the napi boundary itself (path conversions, etc.). + #[napi(value = "BURN_IO")] Io, /// Caller passed an invalid argument shape (e.g. `since` that isn't a /// relative range nor an ISO timestamp). + #[napi(value = "BURN_INVALID_ARGUMENT")] InvalidArgument, } -/// Domain error surfaced to JS callers. The `BurnError` JS class is a -/// thin Object with `{ code, message }`; the napi runtime turns it into a -/// real `Error` subclass at module-init time. +/// `Err` variant used by every verb in the binding. The status slot +/// carries one of the [`BurnErrorCode`] string values; napi-rs threads +/// that string into the thrown JS Error's `code` property. /// -/// Keep this distinct from the JS `Error` napi-rs would synthesize for an -/// `Err(napi::Error)` — the two-tier design lets TS consumers -/// `try { ... } catch (e) { if (e instanceof BurnError) ... }` and switch -/// on `e.code`. -#[napi(object)] -pub struct BurnError { - pub code: BurnErrorCode, - pub message: String, -} - -fn sdk_err(e: anyhow::Error) -> NapiError { +/// We intentionally keep verb signatures spelled as +/// `Result` (using the literal `Result` name and this +/// alias) rather than aliasing a full `BurnResult` — the napi-rs +/// `#[napi]` macro identifies the result wrapping by syntactic token +/// (`Result<...>`) rather than type-checked unwrap, so a wrapper alias +/// would be silently treated as a regular return type and the macro +/// would skip the `JsError::from(err).throw_into(env)` codepath. +pub type BurnError = NapiError<&'static str>; + +fn sdk_err(e: anyhow::Error) -> NapiError<&'static str> { // Render the chain so the message is informative; the discriminant - // stays "Sdk" until the SDK's typed error story exists. - NapiError::new(Status::GenericFailure, format!("{e:#}")) + // stays "BURN_SDK" until the SDK's typed error story exists. + NapiError::new(SDK_ERROR_CODE, format!("{e:#}")) } -fn invalid_arg(msg: impl Into) -> NapiError { - NapiError::new(Status::InvalidArg, msg.into()) +fn invalid_arg(msg: impl Into) -> NapiError<&'static str> { + NapiError::new(INVALID_ARGUMENT_ERROR_CODE, msg.into()) } // --------------------------------------------------------------------------- @@ -106,7 +141,7 @@ fn u64_to_bigint(v: u64) -> BigInt { } } -fn bigint_to_u64(v: BigInt) -> NapiResult { +fn bigint_to_u64(v: BigInt) -> std::result::Result { let (signed, value, lossless) = v.get_u64(); if signed { return Err(invalid_arg("expected non-negative bigint, got signed")); @@ -121,6 +156,131 @@ fn maybe_path(s: Option) -> Option { s.map(PathBuf::from) } +// --------------------------------------------------------------------------- +// BigIntPromoting — JsonValue → JS value walker that emits BigInt for the +// well-known u64 field names below. +// +// `overhead`, `overheadTrim`, and `hotspots` return shapes that are too +// recursive (or, in `hotspots`'s case, a discriminated union) to mirror +// cleanly as a single `#[napi(object)]` struct. We keep them on the +// `serde_json::Value` boundary but wrap the result so the standard +// number→JsNumber conversion in napi-rs's serde-json bridge gets +// overridden for the named fields. Anything not in this list rides +// through as a plain JS number, matching the existing TS contract. +// --------------------------------------------------------------------------- + +/// Field names that carry `u64` values in the SDK and therefore must be +/// surfaced as JS `BigInt`. Names are camelCased (matching `serde(rename_all +/// = "camelCase")` on the SDK structs); the walker matches these literally +/// against the JSON object's key list. +/// +/// Audit checklist when adding a new u64 field to the SDK: drop its +/// camelCase name here so the napi-rs bindings keep the BigInt contract. +const BIGINT_FIELDS: &[&str] = &[ + // overhead + overhead_trim + "tokens", + "bytes", + "totalLines", + "sessionCount", + "startLine", + "endLine", + "filesAnalyzed", + "filesWithRecommendations", + "totalRecommendations", + "tokensPerSession", + // hotspots aggregations + "callCount", + "distinctCommands", + "ridingTurns", + "firstEmitTurnIndex", + "toolCallCount", + "turnsAnalyzed", + "analyzed", + "excluded", +]; + +fn is_bigint_field(name: &str) -> bool { + BIGINT_FIELDS.contains(&name) +} + +/// Wraps a `serde_json::Value` so that, when napi-rs converts it to a JS +/// value, leaf u64 numbers under the [`BIGINT_FIELDS`] keys come out as +/// `BigInt` instead of `number`. Used for the `overhead`, `overheadTrim`, +/// and `hotspots` verbs whose result shapes are documented in +/// `packages/sdk/index.d.ts`. +pub struct BigIntPromoting(JsonValue); + +impl ToNapiValue for BigIntPromoting { + unsafe fn to_napi_value(env: sys::napi_env, val: Self) -> NapiResult { + promote_value(env, val.0, /*key=*/ None) + } +} + +unsafe fn promote_value( + env: sys::napi_env, + val: JsonValue, + key: Option<&str>, +) -> NapiResult { + match val { + JsonValue::Number(n) => { + if let (Some(k), Some(u)) = (key, n.as_u64()) { + if is_bigint_field(k) { + return BigInt::to_napi_value(env, u64_to_bigint(u)); + } + } + // Fall back to napi-rs's default serde number conversion. + serde_json::Number::to_napi_value(env, n) + } + JsonValue::Object(map) => { + // Build a JS object, recursing per-value with the field name + // so `is_bigint_field` can match. + let mut obj: sys::napi_value = ptr::null_mut(); + napi::check_status!( + sys::napi_create_object(env, &mut obj), + "promote_value: napi_create_object" + )?; + for (k, v) in map.into_iter() { + let child = promote_value(env, v, Some(&k))?; + let key_buf = std::ffi::CString::new(k.as_str()).map_err(|e| { + NapiError::new( + napi::Status::GenericFailure, + format!("invalid object key (contains NUL): {e}"), + ) + })?; + napi::check_status!( + sys::napi_set_named_property(env, obj, key_buf.as_ptr(), child), + "promote_value: napi_set_named_property" + )?; + } + Ok(obj) + } + JsonValue::Array(arr) => { + // Arrays don't carry a key context for their elements — the + // outer object's key (e.g. `sections`) doesn't apply to each + // element's leaf scalars; pass `None` so per-element + // promotion is decided by the inner object's keys. + let mut js_arr: sys::napi_value = ptr::null_mut(); + napi::check_status!( + sys::napi_create_array_with_length(env, arr.len(), &mut js_arr), + "promote_value: napi_create_array_with_length" + )?; + for (i, v) in arr.into_iter().enumerate() { + let child = promote_value(env, v, /*key=*/ None)?; + napi::check_status!( + sys::napi_set_element(env, js_arr, i as u32, child), + "promote_value: napi_set_element" + )?; + } + Ok(js_arr) + } + // Booleans / strings / nulls — defer to napi-rs's standard + // serde_json::Value conversion via the leaf wrappers. + JsonValue::Bool(b) => bool::to_napi_value(env, b), + JsonValue::String(s) => String::to_napi_value(env, s), + JsonValue::Null => napi::bindgen_prelude::Null::to_napi_value(env, napi::bindgen_prelude::Null), + } +} + // --------------------------------------------------------------------------- // Ledger open options // --------------------------------------------------------------------------- @@ -210,7 +370,7 @@ impl From for Summary { } #[napi] -pub fn summary(opts: Option) -> NapiResult { +pub fn summary(opts: Option) -> Result { let opts = opts.unwrap_or(SummaryOptions { session: None, project: None, @@ -263,7 +423,7 @@ impl From for SessionCostResult { /// Compact session-scoped cost shape; powers the MCP `burn__sessionCost` tool. #[napi(js_name = "sessionCost")] -pub fn session_cost(opts: Option) -> NapiResult { +pub fn session_cost(opts: Option) -> Result { let opts = opts.unwrap_or(SessionCostOptions { session: None, ledger_home: None, @@ -278,17 +438,14 @@ pub fn session_cost(opts: Option) -> NapiResult) -> NapiResult { +/// Returns the attribution result as an `OverheadResult` (see +/// `packages/sdk/index.d.ts`). Numeric u64 fields (`tokens`, `bytes`, +/// `totalLines`, `sessionCount`, `startLine`, `endLine`) cross the +/// boundary as `BigInt`; everything else is plain JS `number` / string. +#[napi(ts_return_type = "import('./index').OverheadResult")] +pub fn overhead(opts: Option) -> Result { let opts = opts.unwrap_or(OverheadOptions { project: None, since: None, @@ -333,9 +490,10 @@ pub fn overhead(opts: Option) -> NapiResult { ledger_home: maybe_path(opts.ledger_home), }; let result = sdk::overhead(raw).map_err(sdk_err)?; - let value = serde_json::to_value(&result) - .map_err(|e| NapiError::new(Status::GenericFailure, format!("serialize overhead: {e}")))?; - Ok(value) + let value = serde_json::to_value(&result).map_err(|e| { + NapiError::new(SDK_ERROR_CODE, format!("serialize overhead: {e}")) + })?; + Ok(BigIntPromoting(value)) } #[napi(object)] @@ -352,9 +510,9 @@ pub struct OverheadTrimOptions { /// Trim recommendations for high-cost overhead-file sections. Powers /// `burn overhead trim`. Returns an `OverheadTrimResult`-shaped JSON -/// object; see the comment on [`overhead`] for the BigInt-upgrade plan. -#[napi(js_name = "overheadTrim")] -pub fn overhead_trim(opts: Option) -> NapiResult { +/// object with the same `BigInt` substitutions as [`overhead`]. +#[napi(js_name = "overheadTrim", ts_return_type = "import('./index').OverheadTrimResult")] +pub fn overhead_trim(opts: Option) -> Result { let opts = opts.unwrap_or(OverheadTrimOptions { project: None, since: None, @@ -377,9 +535,9 @@ pub fn overhead_trim(opts: Option) -> NapiResult }; let result = sdk::overhead_trim(raw).map_err(sdk_err)?; let value = serde_json::to_value(&result).map_err(|e| { - NapiError::new(Status::GenericFailure, format!("serialize overhead_trim: {e}")) + NapiError::new(SDK_ERROR_CODE, format!("serialize overhead_trim: {e}")) })?; - Ok(value) + Ok(BigIntPromoting(value)) } // --------------------------------------------------------------------------- @@ -388,7 +546,11 @@ pub fn overhead_trim(opts: Option) -> NapiResult // .d.ts already documents the shape (`HotspotsResult` union). // --------------------------------------------------------------------------- -#[napi(string_enum)] +/// Mirror of `sdk::HotspotsGroupBy`. Wire values match +/// `packages/sdk/index.d.ts`'s +/// `'attribution' | 'bash' | 'bash-verb' | 'file' | 'subagent'` literal +/// union. +#[napi(string_enum = "kebab-case")] pub enum HotspotsGroupBy { Attribution, Bash, @@ -421,9 +583,12 @@ pub struct HotspotsOptions { /// Per-axis hotspot attribution + pattern-finding queries. Returns a /// JSON-shaped discriminated union — see `HotspotsResult` in -/// `packages/sdk/index.d.ts`. -#[napi] -pub fn hotspots(opts: Option) -> NapiResult { +/// `packages/sdk/index.d.ts`. u64 row counts (`callCount`, +/// `distinctCommands`, `ridingTurns`, `firstEmitTurnIndex`, +/// `toolCallCount`, `turnsAnalyzed`, `analyzed`, `excluded`) cross as +/// `BigInt` per the file header rule. +#[napi(ts_return_type = "import('./index').HotspotsResult")] +pub fn hotspots(opts: Option) -> Result { let opts = opts.unwrap_or(HotspotsOptions { session: None, project: None, @@ -441,9 +606,10 @@ pub fn hotspots(opts: Option) -> NapiResult { ledger_home: maybe_path(opts.ledger_home), }; let result = sdk::hotspots(raw).map_err(sdk_err)?; - let value = serde_json::to_value(&result) - .map_err(|e| NapiError::new(Status::GenericFailure, format!("serialize hotspots: {e}")))?; - Ok(value) + let value = serde_json::to_value(&result).map_err(|e| { + NapiError::new(SDK_ERROR_CODE, format!("serialize hotspots: {e}")) + })?; + Ok(BigIntPromoting(value)) } // --------------------------------------------------------------------------- @@ -480,7 +646,7 @@ pub struct SearchResult { } #[napi] -pub fn search(opts: SearchQueryOptions) -> NapiResult { +pub fn search(opts: SearchQueryOptions) -> Result { let limit = match opts.limit { Some(b) => Some(bigint_to_u64(b)? as usize), None => None, @@ -529,7 +695,7 @@ pub struct ExportStampsOptions { /// `export_ledger() -> impl Iterator` behavior (it's already in-memory /// today). A streaming variant is a follow-up. #[napi(js_name = "exportLedger")] -pub fn export_ledger(opts: Option) -> NapiResult> { +pub fn export_ledger(opts: Option) -> Result, BurnError> { let opts = opts.unwrap_or(ExportLedgerOptions { ledger_home: None }); let raw = sdk::ExportLedgerOptions { ledger_home: maybe_path(opts.ledger_home), @@ -541,7 +707,7 @@ pub fn export_ledger(opts: Option) -> NapiResult) -> NapiResult> { +pub fn export_stamps(opts: Option) -> Result, BurnError> { let opts = opts.unwrap_or(ExportStampsOptions { ledger_home: None }); let raw = sdk::ExportStampsOptions { ledger_home: maybe_path(opts.ledger_home), @@ -593,6 +759,15 @@ impl From for IngestReport { /// Progress / warning sinks are intentionally not surfaced through the /// boundary in v1 — the JS surface today doesn't expose them either. /// Wave 2 D9 picks them up if the conformance gate calls for it. +/// +/// **Error-code caveat:** napi-rs's `execute_tokio_future` adapter +/// (driving `async fn` → `Promise`) is hard-coded to reject with the +/// default `napi::Error` shape, so this verb's rejection +/// surfaces as `code: 'GenericFailure'` rather than the +/// [`BurnErrorCode`] codes used by the synchronous verbs. JS callers +/// branching on `e.code` should match `'BURN_SDK'` (sync verbs) and +/// `'GenericFailure'` (this verb) until napi-rs grows a typed-error +/// async path. #[napi] pub async fn ingest(opts: Option) -> NapiResult { let opts = opts.unwrap_or(IngestOptions { @@ -614,7 +789,9 @@ pub async fn ingest(opts: Option) -> NapiResult { on_progress: None, on_warn: None, }; - let report = sdk::ingest(raw).await.map_err(sdk_err)?; + let report = sdk::ingest(raw) + .await + .map_err(|e| NapiError::from_reason(format!("{e:#}")))?; Ok(report.into()) } @@ -630,7 +807,7 @@ pub async fn ingest(opts: Option) -> NapiResult { /// `Ledger.open()` smoke-call shape from `packages/sdk/index.d.ts`; a /// future PR can add a stateful `Ledger` JS class that holds a handle. #[napi(js_name = "ledgerOpen")] -pub fn ledger_open(opts: Option) -> NapiResult { +pub fn ledger_open(opts: Option) -> Result { let opts = opts.unwrap_or(LedgerOpenOptions { home: None, content_home: None, @@ -696,4 +873,62 @@ mod tests { Some(PathBuf::from("/tmp/x")) ); } + + #[test] + fn bigint_field_membership_covers_documented_keys() { + // Every camelCased u64 field that crosses the boundary today + // must be in BIGINT_FIELDS so the walker promotes it. + for key in [ + "tokens", + "bytes", + "totalLines", + "sessionCount", + "startLine", + "endLine", + "filesAnalyzed", + "filesWithRecommendations", + "totalRecommendations", + "tokensPerSession", + "callCount", + "distinctCommands", + "ridingTurns", + "firstEmitTurnIndex", + "toolCallCount", + "turnsAnalyzed", + "analyzed", + "excluded", + ] { + assert!(is_bigint_field(key), "{key} missing from BIGINT_FIELDS"); + } + + // Spot-check that f64 / string fields aren't accidentally on + // the list — a regression here would silently turn floats into + // BigInts on the JS side. + for key in [ + "totalCost", + "grandTotal", + "initialTokens", + "persistenceTokens", + "tokenShare", + "perSessionAvg", + "path", + "kind", + ] { + assert!( + !is_bigint_field(key), + "{key} unexpectedly present in BIGINT_FIELDS" + ); + } + } + + #[test] + fn burn_error_codes_match_constants() { + // The TS-exported BurnErrorCode variant values must equal the + // string codes we hand to `napi::Error::new`, otherwise JS + // callers comparing `e.code` to `BurnErrorCode.Sdk` would + // silently miss every error. + assert_eq!(SDK_ERROR_CODE, "BURN_SDK"); + assert_eq!(IO_ERROR_CODE, "BURN_IO"); + assert_eq!(INVALID_ARGUMENT_ERROR_CODE, "BURN_INVALID_ARGUMENT"); + } } From 7ce35bf72b0a6b5fbf8f95719f693d34b194b47a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 12:39:22 +0000 Subject: [PATCH 3/5] Merge origin/main into rust-port/sdk-node-skeleton (resolve CHANGELOG.md conflict) Agent-Logs-Url: https://github.com/AgentWorkforce/burn/sessions/67c443f3-5eb5-494b-aef0-bbb5e1a566ed Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com> --- CHANGELOG.md | 1 + Cargo.lock | 15 + crates/relayburn-cli/Cargo.toml | 30 ++ crates/relayburn-cli/src/harnesses/mod.rs | 264 +++++++++++++++ .../src/harnesses/pending_stamp.rs | 316 ++++++++++++++++++ .../relayburn-cli/src/harnesses/registry.rs | 152 +++++++++ crates/relayburn-cli/src/lib.rs | 18 + crates/relayburn-sdk/src/lib.rs | 6 +- 8 files changed, 800 insertions(+), 2 deletions(-) create mode 100644 crates/relayburn-cli/src/harnesses/mod.rs create mode 100644 crates/relayburn-cli/src/harnesses/pending_stamp.rs create mode 100644 crates/relayburn-cli/src/harnesses/registry.rs create mode 100644 crates/relayburn-cli/src/lib.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9032080d..b1927caf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Cross-package release notes for relayburn. Package changelogs contain package-le ## [Unreleased] - `relayburn-sdk-node` (Rust): napi-rs bindings skeleton — `#[napi]` shims for every public verb in `relayburn-sdk` (`summary`, `sessionCost`, `overhead`, `overheadTrim`, `hotspots`, `search`, `exportLedger`, `exportStamps`, async `ingest`, plus `ledgerOpen`), with u64 token counts surfaced as JS `BigInt`, ISO-8601 timestamps as `String`, async verbs returning `Promise`, and a typed `BurnError` mapping for SDK failures. (#247) +- `relayburn-cli` (Rust): introduce the harness substrate — `HarnessAdapter` trait, lazy compile-time `phf` registry (`lookup` / `list_harness_names`), and the shared `pending_stamp::adapter` factory codex + opencode will reuse. Adapter slots in the registry are reserved but empty pending the Wave 2 PRs (#248-d/e/f). `relayburn-sdk` re-exports `start_watch_loop`, `WatchController`, `write_pending_stamp`, `PendingStampHarness`, and friends so the CLI doesn't have to reach into private SDK modules. (#248) - `relayburn-ingest` (Rust): port the per-process gap-warning state machine (`gap` module — `record_session_gap`, `emit_gap_warning`, `count_tool_call_gaps`, `reset_ingest_gap_warnings`, `set_ingest_gap_writer`) and `reingest_missing_content` (`reingest` module). Suppression mirrors the TS surface: one warning per fresh affected session, silent on steady-state, re-fires after the affected set decays back to empty. `relayburn-ledger` adds `Ledger::list_user_turn_session_ids` to power the `reingest_missing_content` skip filter alongside `list_content_session_ids`. (#278) - `relayburn-analyze` (Rust): port the behavioral-pattern detectors (`patterns` module). `detect_patterns` runs retry-loop, failure-run, cancellation-run, compaction-loss, edit-revert, OpenCode skill-recall-dup, OpenCode skill-pruning-protection, OpenCode system-prompt-tax, and edit-heavy detectors against an ordered turn stream, with optional content-sidecar / tool-result-event / user-turn enrichment. Public surface: `detect_patterns`, `DetectPatternsOptions`; per-pattern result structs are re-exported from `findings` (`RetryLoop`, `FailureRun`, `CancellationRun`, `CompactionLoss`, `EditRevertCycle`, `SkillRecallDup`, `SkillPruningProtection`, `SystemPromptTax`, `EditHeavySession`, `SessionPatternSummary`, `PatternsResult`, `PatternEventSource`). (#275) - `relayburn-analyze` (Rust): port the tool-output-bloat detector — Signal A's `BASH_MAX_OUTPUT_LENGTH` static-config check (with `~/.claude/settings.json` + `/.claude/settings.json` loader) and Signal B's cross-harness observed-bloat aggregation, plus the `WasteFinding` adapter. Public surface mirrors `@relayburn/analyze`: `BASH_MAX_OUTPUT_ENV_KEY`, `DEFAULT_BLOAT_TOKEN_THRESHOLD`, `detect_observed_bloat`, `detect_static_config_bloat`, `detect_tool_output_bloat`, `load_claude_settings`, `project_claude_settings_path`, `user_claude_settings_path`, `tool_output_bloat_to_finding`. (#271) diff --git a/Cargo.lock b/Cargo.lock index 83721a1f..f8909d57 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -29,6 +29,17 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "async-trait" +version = "0.1.89" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "bitflags" version = "2.11.1" @@ -505,7 +516,11 @@ checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" name = "relayburn-cli" version = "0.0.0" dependencies = [ + "anyhow", + "async-trait", + "phf", "relayburn-sdk", + "tokio", ] [[package]] diff --git a/crates/relayburn-cli/Cargo.toml b/crates/relayburn-cli/Cargo.toml index aa00e6c1..5a097e55 100644 --- a/crates/relayburn-cli/Cargo.toml +++ b/crates/relayburn-cli/Cargo.toml @@ -11,6 +11,18 @@ description = "The `burn` CLI — published to crates.io. Crate name is relaybur name = "burn" path = "src/main.rs" +# A library target is added alongside the `burn` binary so the harness +# substrate (`HarnessAdapter` trait, registry, pending-stamp factory) +# under `src/harnesses/` can be unit-tested with `cargo test -p +# relayburn-cli` and so future integration tests under `tests/` can +# reach it without re-declaring the module tree. The binary path +# (`src/main.rs`) and the library path (`src/lib.rs`) live side-by-side +# — cargo treats them as two separate compile units that share the +# same package metadata. +[lib] +name = "relayburn_cli" +path = "src/lib.rs" + [dependencies] # The CLI is the canonical external embedder of the SDK — every read # verb the binary surfaces will wrap a `relayburn-sdk` call once #248 @@ -22,3 +34,21 @@ path = "src/main.rs" # (currently 0.0.1) without forcing a lockstep bump on every release. # Tighten this once the SDK ships a stable 0.x line. relayburn-sdk = { path = "../relayburn-sdk", version = "0.0" } + +# Harness substrate (#248-b): the `HarnessAdapter` trait under `harnesses/` +# uses `async fn` in trait — `async-trait` desugars to `Pin>` futures +# without forcing every adapter to spell that out. `phf` macros give us a +# perfect-hash registry that's evaluated at compile time, so harness lookup +# costs nothing at startup. `tokio::sync` is needed for the watch controller +# wiring exposed by `relayburn-sdk` (`WatchController` holds a `Mutex`). +anyhow = { workspace = true } +async-trait = "0.1" +phf = { version = "0.11", features = ["macros"] } +tokio = { workspace = true, features = ["sync"] } + +[dev-dependencies] +# Async test harness for the `harnesses` module unit tests (lookup, factory +# round-trips). Workspace `tokio` adds `macros` + `rt-multi-thread` so the +# `#[tokio::test]` attribute resolves and the spawned watch-loop ticks have +# a multi-threaded runtime to land on. +tokio = { workspace = true, features = ["rt-multi-thread", "macros", "sync"] } diff --git a/crates/relayburn-cli/src/harnesses/mod.rs b/crates/relayburn-cli/src/harnesses/mod.rs new file mode 100644 index 00000000..3b1872c0 --- /dev/null +++ b/crates/relayburn-cli/src/harnesses/mod.rs @@ -0,0 +1,264 @@ +//! Harness substrate — Rust port of `packages/cli/src/harnesses/types.ts` +//! and friends. +//! +//! `burn run ` is a wrapper that spawns a coding-agent process +//! (Claude Code, Codex, OpenCode, …), babysits its session log while it +//! runs, and feeds the resulting turns into the relayburn ledger. Every +//! adapter contributes the same five-step shape: +//! +//! 1. **`plan`** — compute the spawn plan (binary + args + env). Per-harness +//! transports inject session ids or hook arguments here. +//! 2. **`before_spawn`** — fire any pre-spawn side effect: stamp now if the +//! session id is known up front (claude path), or drop a pending-stamp +//! manifest the post-spawn ingest pass will resolve (codex / opencode). +//! 3. **`start_watcher`** *(optional)* — return a [`WatchController`] that +//! drains a session-store directory while the child runs. Adapters that +//! ingest a single pre-known session file (claude) return `None` here; +//! adapters that share the pending-stamp shape (codex, opencode) wire +//! the watch loop through [`pending_stamp::adapter`]. +//! 4. **`after_exit`** — run a final ingest pass after the child exits and +//! return an [`IngestReport`] so the driver can fold it into the unified +//! `[burn] ingest: …` line. +//! 5. The driver itself owns step zero — collecting `cwd`, passthrough +//! args, and any user-provided enrichment tags into a [`PlanCtx`] — +//! and step six — joining the watcher and reporting summary stats. +//! +//! ## Where this fits +//! +//! This PR (#248 part b) is the substrate. The Wave 2 PRs (#248-d/e/f) +//! plug the three concrete adapters into [`registry`] and the +//! `burn run` driver in `commands::run` consumes them. The CLI scaffold +//! (#248 part a, sibling worktree) lands the clap entrypoint independently. +//! +//! ## Trait shape vs the TS sibling +//! +//! `HarnessAdapter` is a `Send + Sync` trait object so the registry can +//! hand out `&'static dyn HarnessAdapter` references. `async fn` in trait +//! is mediated by `async_trait::async_trait` to keep adapter impls +//! ergonomic; the desugared `Pin>` matches the +//! shape expected by the `burn run` driver, which `tokio::spawn`s the +//! result of `plan` / `after_exit` and joins them at the top level. + +use std::path::PathBuf; + +use async_trait::async_trait; +use relayburn_sdk::{Enrichment, IngestReport, WatchController}; + +pub mod pending_stamp; +pub mod registry; + +pub use registry::{list_harness_names, lookup}; + +/// Driver-side context handed to every adapter call. Mirrors the TS +/// `HarnessRunContext` shape one-to-one (`cwd`, `passthrough`, `tags`, +/// `spawnStartTs`). +/// +/// `tags` is a `BTreeMap` (re-exported from the SDK as +/// [`Enrichment`]) so insertion order doesn't matter for the on-disk +/// stamp record — the pending-stamp serializer canonicalizes ordering. +#[derive(Debug, Clone)] +pub struct PlanCtx { + /// Working directory the user invoked `burn run` from. Forwarded to + /// the spawned harness so it picks up project-local config. + pub cwd: PathBuf, + /// Argv tail after the subcommand boundary, e.g. `burn run claude -- + /// "explain this"` ⇒ `["explain this"]`. Adapters splice this into + /// their generated argv via [`SpawnPlan::args`]. + pub passthrough: Vec, + /// User-supplied enrichment that will be merged onto the resulting + /// stamp. Keys are free-form (`task`, `pr`, …); the Wave 2 driver + /// translates `--tag k=v` flags into entries here. + pub tags: Enrichment, + /// Wall-clock timestamp captured by the driver immediately before + /// `before_spawn`. Used by the pending-stamp manifest so the + /// post-exit resolver can match against session-file mtimes. + pub spawn_start_ts: std::time::SystemTime, +} + +/// Spawn plan returned by [`HarnessAdapter::plan`]. The `burn run` +/// driver owns the actual `tokio::process::Command` construction; this +/// struct is the per-adapter contribution to it. +/// +/// `session_id` is filled in by adapters that know the session id up +/// front (claude can mint one and inject it via `--session-id` so the +/// pre-spawn stamp is final from the start). Adapters that don't know +/// it ahead of time leave this `None` and rely on the pending-stamp +/// resolver to attach their enrichment to the freshly-discovered +/// session in `after_exit`. +#[derive(Debug, Clone, Default)] +pub struct SpawnPlan { + pub binary: String, + pub args: Vec, + /// Env vars to overlay on top of the parent process env when + /// spawning. Keep this tight — `tokio::process::Command::env_clear` + /// + this map is the typical pattern, though Wave 2 may relax that. + pub env_overrides: Vec<(String, String)>, + /// Session id the adapter pre-allocated, when known. See struct + /// docs for when this is `Some` vs `None`. + pub session_id: Option, +} + +impl SpawnPlan { + /// Convenience: minimal plan that just runs `binary` with `args` and + /// inherits the parent's env. Most adapters' `plan` returns this + /// shape directly. + pub fn new(binary: impl Into, args: Vec) -> Self { + Self { + binary: binary.into(), + args, + env_overrides: Vec::new(), + session_id: None, + } + } +} + +/// `HarnessAdapter` — five-method contract every harness implements. The +/// TS sibling lives at `packages/cli/src/harnesses/types.ts` and the +/// shape mirrors it; see the module docs for what each step does. +/// +/// Adapters are zero-sized (or near-zero-sized) stateless types that the +/// registry hands out as `&'static dyn HarnessAdapter`. State that lives +/// across `before_spawn` → `after_exit` rides on `PlanCtx` / `SpawnPlan`, +/// or in the pending-stamps directory on disk. +#[async_trait] +pub trait HarnessAdapter: Send + Sync { + /// Lowercase identifier — `claude`, `codex`, `opencode`, … — used as + /// the dispatch key and as the harness label in log lines. + fn name(&self) -> &'static str; + + /// Per-harness session-store root. Today this is a fixed path + /// resolved against the user's home directory; future iterations + /// may thread `BurnConfig` through so the root is configurable. + fn session_root(&self) -> PathBuf; + + /// Compute the spawn plan. Inject session ids or transport-level + /// args here. Populate `SpawnPlan::session_id` when known so + /// `before_spawn` / `after_exit` can stamp eagerly. + async fn plan(&self, ctx: &PlanCtx) -> anyhow::Result; + + /// Pre-spawn side effects. Stamp now if the session id is in `plan`, + /// otherwise drop a pending-stamp manifest the post-spawn ingest can + /// resolve. Default impl is a no-op so simple adapters don't have to + /// spell it out. + async fn before_spawn(&self, _ctx: &PlanCtx, _plan: &SpawnPlan) -> anyhow::Result<()> { + Ok(()) + } + + /// Optional. Return a [`WatcherController`] from + /// [`relayburn_sdk::start_watch_loop`] to drain a session store + /// while the child runs; return `None` for adapters that ingest a + /// single pre-known file at exit. + /// + /// `on_report` is a callback the driver routes into its summary + /// accumulator so the final `[burn] ingest:` line reflects + /// every tick that fired during the run, not just `after_exit`. + fn start_watcher( + &self, + _ctx: &PlanCtx, + _on_report: relayburn_sdk::ReportSink, + ) -> Option { + None + } + + /// Final ingest pass after the child exits. Returns an + /// [`IngestReport`] the driver folds into its summary line. + async fn after_exit(&self, ctx: &PlanCtx, plan: &SpawnPlan) -> anyhow::Result; +} + +/// Wrapper around the SDK's [`WatchController`]. Today this is just a +/// newtype so callers don't have to import `relayburn_sdk` directly to +/// construct or stop a watcher; tomorrow it gives us a stable boundary +/// to attach harness-side observability (e.g. a `name`, a per-adapter +/// metric counter) without leaking through to the SDK. +pub struct WatcherController { + inner: WatchController, +} + +impl WatcherController { + /// Wrap a raw SDK controller. `pending_stamp::adapter` is the + /// canonical caller; bespoke adapters that build their own watch + /// loop also funnel through here. + pub fn new(inner: WatchController) -> Self { + Self { inner } + } + + /// Run a single tick on demand. Forwards to + /// [`WatchController::tick`]. + pub async fn tick(&self) { + self.inner.tick().await; + } + + /// Stop the periodic loop and await any in-flight tick. Idempotent. + /// `burn run` calls this once the spawned child exits. + pub async fn stop(&self) { + self.inner.stop().await; + } + + /// Borrow the wrapped controller for callers that need the raw + /// SDK type (e.g. integration tests parking on `tick_done`). + pub fn raw(&self) -> &WatchController { + &self.inner + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Smoke test: `SpawnPlan::new` produces an inherit-env plan the + /// driver can hand straight to `tokio::process::Command`. Catches + /// accidental shape changes on the struct. + #[test] + fn spawn_plan_new_minimal_shape() { + let plan = SpawnPlan::new("claude", vec!["--help".into()]); + assert_eq!(plan.binary, "claude"); + assert_eq!(plan.args, vec!["--help".to_string()]); + assert!(plan.env_overrides.is_empty()); + assert!(plan.session_id.is_none()); + } + + /// Trait dispatch sanity: a fake adapter implementing `HarnessAdapter` + /// must be coercible to `&dyn HarnessAdapter` so the registry can + /// hand out trait-object references. + struct FakeAdapter; + + #[async_trait] + impl HarnessAdapter for FakeAdapter { + fn name(&self) -> &'static str { + "fake" + } + fn session_root(&self) -> PathBuf { + PathBuf::from("/tmp/fake") + } + async fn plan(&self, _ctx: &PlanCtx) -> anyhow::Result { + Ok(SpawnPlan::new("fake", vec![])) + } + async fn after_exit( + &self, + _ctx: &PlanCtx, + _plan: &SpawnPlan, + ) -> anyhow::Result { + Ok(IngestReport::default()) + } + } + + #[tokio::test] + async fn fake_adapter_round_trip() { + let adapter: &dyn HarnessAdapter = &FakeAdapter; + assert_eq!(adapter.name(), "fake"); + assert_eq!(adapter.session_root(), PathBuf::from("/tmp/fake")); + + let ctx = PlanCtx { + cwd: PathBuf::from("/tmp"), + passthrough: vec![], + tags: Enrichment::new(), + spawn_start_ts: std::time::SystemTime::now(), + }; + let plan = adapter.plan(&ctx).await.unwrap(); + assert_eq!(plan.binary, "fake"); + + let report = adapter.after_exit(&ctx, &plan).await.unwrap(); + assert_eq!(report.scanned_sessions, 0); + assert_eq!(report.ingested_sessions, 0); + } +} diff --git a/crates/relayburn-cli/src/harnesses/pending_stamp.rs b/crates/relayburn-cli/src/harnesses/pending_stamp.rs new file mode 100644 index 00000000..c66f5d44 --- /dev/null +++ b/crates/relayburn-cli/src/harnesses/pending_stamp.rs @@ -0,0 +1,316 @@ +//! `pending_stamp::adapter` factory — Rust port of +//! `packages/cli/src/harnesses/pending-stamp.ts`'s `createPendingStampAdapter`. +//! +//! Codex and OpenCode share an identical wrapper shape: pre-spawn pending +//! stamp, while-running watch loop draining the session store, post-exit +//! ingest pass. The TS sibling captures that shape once via a factory; the +//! Rust port does the same here so the Wave 2 codex / opencode adapter PRs +//! are one-line constructions instead of two near-duplicate `impl`s. +//! +//! ## Composition +//! +//! ```text +//! plan → `SpawnPlan::new(name, ctx.passthrough)` (no env, no session_id) +//! before_spawn → `relayburn_sdk::write_pending_stamp(...)` + log +//! start_watcher→ `relayburn_sdk::start_watch_loop(non-immediate, on_report → ingest_fn)` +//! after_exit → `(config.ingest_sessions)(...)` +//! ``` +//! +//! `ingest_sessions` is a caller-supplied async closure (Wave 2 will pass +//! `relayburn_sdk::ingest` with codex-only or opencode-only roots). The +//! factory doesn't reach into `relayburn_sdk::ingest` directly so adapter +//! authors can swap in test doubles without monkey-patching env vars. +//! +//! ## What this PR does NOT do +//! +//! - No concrete codex / opencode adapter — those land in #248-e / #248-f. +//! - No log line yet (`[burn] codex spawn: pending stamp …`); the TS +//! sibling writes it through `process.stderr.write`. The Rust factory +//! exposes the manifest filename via the `before_spawn` log hook so +//! Wave 2 adapters can print it under whatever logging discipline the +//! CLI scaffold (#248-a) settles on. Today we just route through +//! `eprintln!`. + +use std::future::Future; +use std::path::{Path, PathBuf}; +use std::pin::Pin; +use std::sync::Arc; +use std::time::Duration; + +use async_trait::async_trait; +use relayburn_sdk::{ + start_watch_loop, write_pending_stamp, IngestFn, IngestReport, PendingStampHarness, + PendingStampWriteOptions, ReportSink, StartWatchLoopOptions, +}; + +use super::{HarnessAdapter, PlanCtx, SpawnPlan, WatcherController}; + +/// Async ingest callback supplied by the caller. Returns the report the +/// watch loop and `after_exit` hand back to the driver. +pub type IngestSessionsFn = Arc< + dyn Fn() -> Pin> + Send>> + + Send + + Sync, +>; + +/// Configuration for [`adapter`]. Mirrors the TS +/// `PendingStampAdapterOptions` shape. +#[derive(Clone)] +pub struct PendingStampAdapter { + /// Lowercase harness name — `codex` or `opencode`. The factory + /// asserts this maps to a [`PendingStampHarness`] variant; passing + /// anything else is a programmer error and panics on construction. + pub name: &'static str, + /// Per-harness session-store root (e.g. `~/.codex/sessions`). + /// Resolved lazily via the supplied closure so tests can inject + /// temp dirs without touching `$HOME`. + pub session_root: Arc PathBuf + Send + Sync>, + /// Final ingest pass — called by `after_exit` and by every tick of + /// the watch loop while the child runs. + pub ingest_sessions: IngestSessionsFn, + /// Watch-loop tick interval. Defaults to 1s (matches the TS sibling). + pub watch_interval: Duration, +} + +impl PendingStampAdapter { + /// Construct a factory config with the standard 1s tick. Callers + /// that need a different cadence build the struct directly. + pub fn new( + name: &'static str, + session_root: Arc PathBuf + Send + Sync>, + ingest_sessions: IngestSessionsFn, + ) -> Self { + Self { + name, + session_root, + ingest_sessions, + watch_interval: Duration::from_millis(1000), + } + } +} + +/// Build a [`HarnessAdapter`] from a [`PendingStampAdapter`] config. +/// +/// The Wave 2 codex / opencode adapter PRs each call this once (with +/// `name = "codex"` or `name = "opencode"`) and register the returned +/// adapter as a `&'static` in [`super::registry`]. The boxed-then-leaked +/// pattern is fine because adapters live for the entire CLI process. +pub fn adapter(config: PendingStampAdapter) -> Box { + Box::new(PendingStampAdapterImpl::new(config)) +} + +/// `HarnessAdapter` implementation backing the [`adapter`] factory. Kept +/// private so callers can't construct it directly without going through +/// the validated factory. +struct PendingStampAdapterImpl { + name: &'static str, + harness: PendingStampHarness, + session_root: Arc PathBuf + Send + Sync>, + ingest_sessions: IngestSessionsFn, + watch_interval: Duration, +} + +impl PendingStampAdapterImpl { + fn new(config: PendingStampAdapter) -> Self { + let harness = match config.name { + "codex" => PendingStampHarness::Codex, + "opencode" => PendingStampHarness::Opencode, + other => { + // Programmer error: the SDK's pending-stamp protocol only + // recognises codex + opencode. Adding a third pending-stamp + // harness is a coordinated change with the SDK manifest + // schema, not a CLI-side decision. + panic!( + "pending_stamp::adapter only supports codex|opencode, got {other:?}; \ + extending the protocol requires an SDK change" + ) + } + }; + Self { + name: config.name, + harness, + session_root: config.session_root, + ingest_sessions: config.ingest_sessions, + watch_interval: config.watch_interval, + } + } + + /// Build the IngestFn the watch loop calls each tick. Captures the + /// caller-supplied `ingest_sessions` closure so the loop runs the + /// same path `after_exit` does. + fn ingest_fn(&self) -> IngestFn { + let ingest_sessions = self.ingest_sessions.clone(); + Arc::new(move || { + let f = ingest_sessions.clone(); + Box::pin(async move { f().await }) + }) + } + + /// Convenience: just the file-name component of a manifest path, + /// for stable log lines that don't dump the user's home directory. + fn manifest_basename(path: &Path) -> String { + path.file_name() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| path.display().to_string()) + } +} + +#[async_trait] +impl HarnessAdapter for PendingStampAdapterImpl { + fn name(&self) -> &'static str { + self.name + } + + fn session_root(&self) -> PathBuf { + (self.session_root)() + } + + async fn plan(&self, ctx: &PlanCtx) -> anyhow::Result { + Ok(SpawnPlan::new(self.name, ctx.passthrough.clone())) + } + + async fn before_spawn(&self, ctx: &PlanCtx, _plan: &SpawnPlan) -> anyhow::Result<()> { + let session_dir_hint = (self.session_root)(); + let opts = PendingStampWriteOptions { + harness: self.harness, + cwd: ctx.cwd.to_string_lossy().into_owned(), + enrichment: ctx.tags.clone(), + session_dir_hint: Some(session_dir_hint.to_string_lossy().into_owned()), + spawn_start_ts: Some(ctx.spawn_start_ts), + spawner_pid: None, + }; + let written = write_pending_stamp(opts).map_err(|err| { + anyhow::anyhow!("failed to write {} pending stamp: {err}", self.name) + })?; + eprintln!( + "[burn] {} spawn: pending stamp {}", + self.name, + Self::manifest_basename(&written.file) + ); + Ok(()) + } + + fn start_watcher( + &self, + _ctx: &PlanCtx, + on_report: ReportSink, + ) -> Option { + // Match the TS adapter: do not run an immediate first tick. The + // child has barely started; let the periodic interval drive the + // first scan so we don't spawn an ingest pass that races the + // freshly-written pending stamp. + let opts = StartWatchLoopOptions::new(self.ingest_fn()) + .with_immediate(false) + .with_interval(self.watch_interval) + .with_on_report(on_report); + Some(WatcherController::new(start_watch_loop(opts))) + } + + async fn after_exit(&self, _ctx: &PlanCtx, _plan: &SpawnPlan) -> anyhow::Result { + (self.ingest_sessions)().await + } +} + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + + use relayburn_sdk::Enrichment; + + use super::*; + + /// `adapter()` round-trips through the trait surface for codex. + /// Exercises name + session_root + plan; `before_spawn` is covered + /// by an integration test (would need a writable $RELAYBURN_HOME). + #[tokio::test] + async fn codex_factory_round_trip() { + let session_root: Arc PathBuf + Send + Sync> = + Arc::new(|| PathBuf::from("/tmp/codex-sessions")); + let ingest_sessions: IngestSessionsFn = + Arc::new(|| Box::pin(async { Ok(IngestReport::default()) })); + let config = PendingStampAdapter::new("codex", session_root, ingest_sessions); + let adapter: Box = adapter(config); + + assert_eq!(adapter.name(), "codex"); + assert_eq!(adapter.session_root(), PathBuf::from("/tmp/codex-sessions")); + + let ctx = PlanCtx { + cwd: PathBuf::from("/tmp"), + passthrough: vec!["--help".into()], + tags: Enrichment::new(), + spawn_start_ts: std::time::SystemTime::now(), + }; + let plan = adapter.plan(&ctx).await.unwrap(); + assert_eq!(plan.binary, "codex"); + assert_eq!(plan.args, vec!["--help".to_string()]); + + // `after_exit` runs the user-supplied closure verbatim. + let report = adapter.after_exit(&ctx, &plan).await.unwrap(); + assert_eq!(report.scanned_sessions, 0); + } + + /// `adapter()` round-trips through the trait surface for opencode — + /// same shape, different name. + #[tokio::test] + async fn opencode_factory_round_trip() { + let session_root: Arc PathBuf + Send + Sync> = + Arc::new(|| PathBuf::from("/tmp/opencode-storage")); + let ingest_sessions: IngestSessionsFn = + Arc::new(|| Box::pin(async { Ok(IngestReport::default()) })); + let config = PendingStampAdapter::new("opencode", session_root, ingest_sessions); + let adapter = adapter(config); + assert_eq!(adapter.name(), "opencode"); + assert_eq!( + adapter.session_root(), + PathBuf::from("/tmp/opencode-storage") + ); + } + + /// Bogus harness names panic on construction — the factory doesn't + /// silently fall through to a default. This catches typos at adapter + /// registration time rather than at runtime. + #[test] + #[should_panic(expected = "pending_stamp::adapter only supports")] + fn unknown_name_panics() { + let session_root: Arc PathBuf + Send + Sync> = + Arc::new(|| PathBuf::from("/tmp")); + let ingest_sessions: IngestSessionsFn = + Arc::new(|| Box::pin(async { Ok(IngestReport::default()) })); + let _ = adapter(PendingStampAdapter::new( + "cursor", + session_root, + ingest_sessions, + )); + } + + /// `after_exit` invokes the supplied `ingest_sessions` closure. We + /// use an atomic counter to confirm it was called. + #[tokio::test] + async fn after_exit_invokes_supplied_ingest_fn() { + let count = Arc::new(AtomicUsize::new(0)); + let count_for_closure = count.clone(); + let session_root: Arc PathBuf + Send + Sync> = + Arc::new(|| PathBuf::from("/tmp/codex-sessions")); + let ingest_sessions: IngestSessionsFn = Arc::new(move || { + let c = count_for_closure.clone(); + Box::pin(async move { + c.fetch_add(1, Ordering::SeqCst); + Ok(IngestReport::default()) + }) + }); + let config = PendingStampAdapter::new("codex", session_root, ingest_sessions); + let adapter = adapter(config); + + let ctx = PlanCtx { + cwd: PathBuf::from("/tmp"), + passthrough: vec![], + tags: Enrichment::new(), + spawn_start_ts: std::time::SystemTime::now(), + }; + let plan = adapter.plan(&ctx).await.unwrap(); + adapter.after_exit(&ctx, &plan).await.unwrap(); + adapter.after_exit(&ctx, &plan).await.unwrap(); + + assert_eq!(count.load(Ordering::SeqCst), 2); + } +} diff --git a/crates/relayburn-cli/src/harnesses/registry.rs b/crates/relayburn-cli/src/harnesses/registry.rs new file mode 100644 index 00000000..c09bb303 --- /dev/null +++ b/crates/relayburn-cli/src/harnesses/registry.rs @@ -0,0 +1,152 @@ +//! Lazy harness registry — Rust port of `packages/cli/src/harnesses/registry.ts`. +//! +//! The TS sibling defers each adapter import (`async () => (await +//! import('./claude.js')).claudeAdapter`) so unrelated commands don't +//! pay ingest/ledger startup cost. The Rust port doesn't have lazy +//! module imports, but trait-object adapters are zero-sized so the +//! equivalent is "don't construct heavy state at registry build time". +//! All three Wave 2 adapters will be unit structs — adding them to the +//! `phf::Map` below is free. +//! +//! ## Why `phf` and not `OnceLock>` +//! +//! `phf::Map` is built at compile time; lookup is a single perfect-hash +//! probe with zero allocation. Cold-start matters here: `burn --help` +//! and `burn summary` should not pay any harness-table init cost. +//! `OnceLock>` is what we'd reach for if the table needed +//! runtime configuration (e.g. user-pluggable harnesses), which is not +//! on the roadmap. +//! +//! ## Wave 2 plug-in points +//! +//! Three slots are reserved below for the Wave 2 adapter PRs: +//! +//! * `claude` — #248-d (Wave 2 D5) +//! * `codex` — #248-e (Wave 2 D6) +//! * `opencode` — #248-f (Wave 2 D7) +//! +//! Each adds `pub mod claude;` (or codex / opencode) here and a single +//! row in [`ADAPTERS`]. The codex + opencode adapters are constructed +//! through [`super::pending_stamp::adapter`] so they share the manifest +//! + watch-loop wiring. + +use phf::phf_map; + +use super::HarnessAdapter; + +/// Compile-time perfect-hash map from harness name to a `&'static dyn +/// HarnessAdapter`. Empty on this branch — populated by the three Wave 2 +/// fan-out PRs (#248-d/e/f). +/// +/// `&'static dyn HarnessAdapter` requires the value side to be a trait +/// object reference; `phf` supports that as long as the referent has a +/// `'static` lifetime, which works for stateless unit-struct adapters +/// or adapters defined as `static`s in their own module. +static ADAPTERS: phf::Map<&'static str, &'static dyn HarnessAdapter> = phf_map! { + // Wave 2 PRs will populate these slots: + // + // "claude" => &claude::CLAUDE_ADAPTER, // #248-d + // "codex" => &codex::CODEX_ADAPTER, // #248-e + // "opencode" => &opencode::OPENCODE_ADAPTER, // #248-f +}; + +/// Look up an adapter by name. Returns `None` for unknown names; the +/// `burn run` driver maps `None` to a "did you mean …?" diagnostic +/// using [`list_harness_names`]. +pub fn lookup(name: &str) -> Option<&'static dyn HarnessAdapter> { + ADAPTERS.get(name).copied() +} + +/// List every registered harness name. The CLI's `--help` block reads +/// this so the harness list updates automatically when a new adapter is +/// registered. Order is the iteration order of `phf::Map` (stable but +/// not guaranteed alphabetical) — callers that want deterministic order +/// should sort the result, mirroring how the TS test sorts for +/// comparison. +pub fn list_harness_names() -> Vec<&'static str> { + ADAPTERS.keys().copied().collect() +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use async_trait::async_trait; + use relayburn_sdk::IngestReport; + + use super::super::{HarnessAdapter, PlanCtx, SpawnPlan}; + use super::*; + + /// A registry-injectable fake adapter used to exercise the lookup + /// path. The real registry only ships the Wave 2 adapters; this test + /// asserts the substrate independently of which slots are populated. + struct FakeAdapter; + + #[async_trait] + impl HarnessAdapter for FakeAdapter { + fn name(&self) -> &'static str { + "fake" + } + fn session_root(&self) -> PathBuf { + PathBuf::from("/tmp/fake-sessions") + } + async fn plan(&self, _ctx: &PlanCtx) -> anyhow::Result { + Ok(SpawnPlan::new("fake", vec![])) + } + async fn after_exit( + &self, + _ctx: &PlanCtx, + _plan: &SpawnPlan, + ) -> anyhow::Result { + Ok(IngestReport::default()) + } + } + + static FAKE: FakeAdapter = FakeAdapter; + + /// Static fake registry shaped exactly like production [`ADAPTERS`]. + /// `phf_map!` requires its output to live for `'static`, so the + /// fixture is module-scoped rather than declared inside the test + /// body. This proves a `&'static dyn HarnessAdapter` round-trips + /// through the same `phf::Map::get` → `Option::copied` path that + /// [`lookup`] uses, without needing to mutate the real table (which + /// is compile-time and intentionally unreachable from tests). + static FAKE_REGISTRY: phf::Map<&'static str, &'static dyn HarnessAdapter> = phf_map! { + "fake" => &FAKE, + }; + + /// Lookup-by-name on a static fake adapter. Mirrors what `lookup` + /// does internally and what the Wave 2 PRs will rely on once they + /// register `claude` / `codex` / `opencode`. + #[test] + fn dyn_adapter_round_trip_by_name() { + let got = FAKE_REGISTRY + .get("fake") + .copied() + .expect("fake registered"); + assert_eq!(got.name(), "fake"); + assert_eq!(got.session_root(), PathBuf::from("/tmp/fake-sessions")); + + assert!(FAKE_REGISTRY.get("missing").is_none()); + } + + /// On this branch the production registry is intentionally empty; + /// Wave 2 PRs (claude/codex/opencode) flip this to the `["claude", + /// "codex", "opencode"]` shape the TS sibling already ships. Once + /// those merge, this test should be tightened to assert all three. + #[test] + fn list_is_empty_until_wave_2_adapters_land() { + let names = list_harness_names(); + assert!( + names.is_empty(), + "expected registry to be empty on this branch, got {names:?}" + ); + } + + #[test] + fn lookup_unknown_returns_none() { + assert!(lookup("nope").is_none()); + assert!(lookup("").is_none()); + assert!(lookup("claude ").is_none()); + } +} diff --git a/crates/relayburn-cli/src/lib.rs b/crates/relayburn-cli/src/lib.rs new file mode 100644 index 00000000..fce58564 --- /dev/null +++ b/crates/relayburn-cli/src/lib.rs @@ -0,0 +1,18 @@ +//! `relayburn-cli` library surface. +//! +//! The CLI ships as a binary (`burn`) backed by `src/main.rs`. This +//! `lib.rs` exists so internal modules can be unit-tested with `cargo +//! test -p relayburn-cli` and so future integration tests under `tests/` +//! can reach the harness substrate without re-declaring the module tree. +//! +//! Today the only public surface here is [`harnesses`] — the `HarnessAdapter` +//! trait, the lazy registry, and the shared pending-stamp adapter factory +//! introduced in #248-b. Wave 2 PRs (claude / codex / opencode) will plug +//! their adapters in via [`harnesses::registry`]; the CLI binary will reach +//! them through `lookup` / `list_harness_names`. +//! +//! Keeping this surface as a library crate alongside the binary lets the +//! Wave 2 fan-out PRs add per-adapter modules and unit tests without +//! disturbing `main.rs`. + +pub mod harnesses; diff --git a/crates/relayburn-sdk/src/lib.rs b/crates/relayburn-sdk/src/lib.rs index f6c24f0c..0740dfd6 100644 --- a/crates/relayburn-sdk/src/lib.rs +++ b/crates/relayburn-sdk/src/lib.rs @@ -104,8 +104,10 @@ pub use crate::analyze::{ }; pub use crate::ingest::{ - cleanup_stale_pending_stamps, ingest_all, IngestOptions as RawIngestOptions, IngestReport, - IngestRoots, + cleanup_stale_pending_stamps, ingest_all, run_ingest_tick, start_watch_loop, + write_pending_stamp, ErrorSink, IngestFn, IngestOptions as RawIngestOptions, IngestReport, + IngestRoots, PendingStamp, PendingStampHarness, PendingStampWriteResult, ReportSink, + StartWatchLoopOptions, WatchController, WriteOptions as PendingStampWriteOptions, }; // --- LedgerOpenOptions ----------------------------------------------------- From e5fcf8e6bc2d40590a84ab14309599d9e0892898 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 6 May 2026 08:59:46 -0400 Subject: [PATCH 4/5] relayburn-sdk-node: tighten ingest() typed-error contract docs (review fixes round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit major comment 3195529029 on PR #306. The previous fixup (290b970) documented the napi-rs limitation in the ingest() doc comment but the file header still claimed "JS callers can branch on e.code === BurnErrorCode.*" without exception, so the contract was a lie for the async verb. Took Path B (narrow the contract) after evaluating Path A: - napi-rs 2.x's `#[napi] async fn` is lowered (in napi-derive-backend's codegen/fn.rs) to `napi::bindgen_prelude::execute_tokio_future`, which is hard-typed to `Result>`. `Status` is a closed enum over the predefined NAPI status strings — there is no variant for "BURN_SDK". - `JsDeferred::reject` only accepts `Error`. `AsyncTask` (the alternative async path) routes rejections through the same `JsError::from(Error).into_value` codepath in async_work.rs's `complete()`. No public escape hatch in napi 2.x. - The only way to inject a non-Status `code` would be to hand-roll a `JsDeferred` replacement on top of raw `sys::napi_*` calls + a TSFN. That's >200 lines of unsafe boilerplate exactly matching what the reviewer flagged as "not clean." - Upgrading to napi-rs 3.x is out of scope for this skeleton PR (separate evaluation needed for `string_enum` / `BigInt` ergonomics vs. the rest of the binding). Path B changes: - File header now explicitly enumerates the async exception under "Errors → typed BurnError JS class (sync verbs only)", points at the napi-rs source paths that document the limitation, and gives JS callers the concrete contract: `e.code === 'GenericFailure'`, rendered SDK error chain in `e.message`. Notes that a future napi-rs 3.x upgrade would tighten this. - `ingest()` doc comment now leads with the contract rather than burying the caveat at the bottom, and points readers at the type-level invariant test below. - New test `ingest_uses_generic_failure_code_runtime_invariant` pins the discrepancy as a type-level fact: `BurnError` (`Error<&'static str>`) and `napi::Error` (`Error`) must remain distinct types. If a future napi-rs upgrade or hand-rolled deferred unifies them, this test starts failing — that's the signal to switch ingest's signature back to `Result` and remove the caveat in lockstep. Also asserts the documented async fallback code is exactly `"GenericFailure"` and never collides with the BURN_* codes the sync verbs use. Validation: - `cargo build -p relayburn-sdk-node` clean. - `cargo test --workspace` green (605 SDK + 2 SDK integration + 8 sdk-node unit tests, including the new invariant). --- crates/relayburn-sdk-node/src/lib.rs | 106 +++++++++++++++++++++++---- 1 file changed, 92 insertions(+), 14 deletions(-) diff --git a/crates/relayburn-sdk-node/src/lib.rs b/crates/relayburn-sdk-node/src/lib.rs index 482fac32..b32a93d8 100644 --- a/crates/relayburn-sdk-node/src/lib.rs +++ b/crates/relayburn-sdk-node/src/lib.rs @@ -33,17 +33,42 @@ //! `tokio_rt` feature drives this; we mark `ingest` `async fn` and the //! sync verbs (`summary`, `sessionCost`, …) as plain `fn` returning //! `Result`. -//! - **Errors → typed `BurnError` JS class.** Domain failures from the -//! SDK (`anyhow::Error`) and argument-shape errors raised at this -//! boundary are surfaced as a `napi::Error` whose `Status` slot carries -//! one of [`SDK_ERROR_CODE`], [`IO_ERROR_CODE`], or -//! [`INVALID_ARGUMENT_ERROR_CODE`]. napi-rs writes that string into the -//! thrown JS Error's `code` property (via `napi_create_error`'s +//! - **Errors → typed `BurnError` JS class (sync verbs only).** Domain +//! failures from the SDK (`anyhow::Error`) and argument-shape errors +//! raised at this boundary are surfaced as a `napi::Error` whose +//! `Status` slot carries one of [`SDK_ERROR_CODE`], [`IO_ERROR_CODE`], +//! or [`INVALID_ARGUMENT_ERROR_CODE`]. napi-rs writes that string into +//! the thrown JS Error's `code` property (via `napi_create_error`'s //! `code` argument), so JS callers get //! `try { … } catch (e) { if (e.code === 'BURN_SDK') … }`. The //! [`BurnErrorCode`] enum is exported as a `string_enum` so TS code //! can reference the codes by name without stringly-typed literals. //! +//! **Async exception — [`ingest`].** napi-rs 2.x's `async fn` lowering +//! in `napi-derive` runs through `napi::bindgen_prelude::execute_tokio_future` +//! ([`napi-derive-backend`]'s `codegen/fn.rs`), which is hard-typed to +//! `Result>` — and `Status` is a *closed* enum +//! over the predefined NAPI status strings (`GenericFailure`, +//! `InvalidArg`, …). There is no public typed-error escape hatch in +//! napi 2.x: `JsDeferred::reject` only accepts `Error`, and +//! `AsyncTask::reject`'s rejection path likewise funnels through +//! `JsError::from(Error).into_value`. The only way to inject +//! a non-`Status` `code` would be to hand-roll a `JsDeferred` +//! replacement on top of raw `sys::napi_*` calls + a TSFN — see +//! `crates/relayburn-sdk-node/src/lib.rs` git history for the +//! evaluation. We deliberately don't pay that complexity in v1. +//! +//! **Concrete contract for [`ingest`]:** the returned `Promise` +//! rejects with a JS `Error` whose `.code === 'GenericFailure'` and +//! whose `.message` is the rendered `anyhow::Error` chain from the +//! SDK. JS callers branching on `e.code` should match `'GenericFailure'` +//! for ingest failures (or, more robustly, gate on `e.message` +//! substrings if discrimination is required). A future PR can tighten +//! this — likely by upgrading to napi-rs 3.x once the `string_enum` +//! and `BigInt` ergonomics there are validated against the rest of +//! the binding — at which point `e.code` becomes one of the +//! [`BurnErrorCode`] string values for ingest as well. +//! //! # Surface //! //! Every public verb in `relayburn-sdk` (free-function form) is bound @@ -760,14 +785,17 @@ impl From for IngestReport { /// boundary in v1 — the JS surface today doesn't expose them either. /// Wave 2 D9 picks them up if the conformance gate calls for it. /// -/// **Error-code caveat:** napi-rs's `execute_tokio_future` adapter -/// (driving `async fn` → `Promise`) is hard-coded to reject with the -/// default `napi::Error` shape, so this verb's rejection -/// surfaces as `code: 'GenericFailure'` rather than the -/// [`BurnErrorCode`] codes used by the synchronous verbs. JS callers -/// branching on `e.code` should match `'BURN_SDK'` (sync verbs) and -/// `'GenericFailure'` (this verb) until napi-rs grows a typed-error -/// async path. +/// **Error-code contract.** Unlike the synchronous verbs (which reject +/// with `e.code` set to one of [`BurnErrorCode`]'s string values), this +/// async verb's rejection surfaces as `code: 'GenericFailure'`. The +/// rendered SDK error chain is in `e.message`. See the file header for +/// the full rationale (napi-rs 2.x's `execute_tokio_future` is +/// hard-typed to `Result>` and `Status` is a closed +/// enum, so `'BURN_SDK'` cannot be threaded through). The +/// [`ingest_uses_generic_failure_code_runtime_invariant`] test pins +/// this discrepancy so a future napi-rs upgrade or hand-rolled deferred +/// either fixes it or has to update the test in lockstep with the +/// header docs. #[napi] pub async fn ingest(opts: Option) -> NapiResult { let opts = opts.unwrap_or(IngestOptions { @@ -931,4 +959,54 @@ mod tests { assert_eq!(IO_ERROR_CODE, "BURN_IO"); assert_eq!(INVALID_ARGUMENT_ERROR_CODE, "BURN_INVALID_ARGUMENT"); } + + /// Runtime invariant pinning the documented `ingest()` error-code + /// discrepancy. The body is a compile-time assertion that the type + /// returned by `ingest`'s rejection path is the default + /// `napi::Error` (`Error`), *not* our typed + /// `Error<&'static str>` (`BurnError`). If a future napi-rs upgrade + /// or hand-rolled deferred replacement makes typed async errors + /// possible, this test will start failing; that's the signal to + /// remove the caveat from the file header + the `ingest()` doc + /// comment in lockstep with switching the signature to + /// `Result`. + /// + /// Why a static-typing check rather than a JS-side assertion: the + /// JS-side end-to-end test is wave-2 D9 territory (it requires a + /// built `.node` artifact); we can still pin the discrepancy *here* + /// by encoding the napi-rs limitation as a type-level fact. The + /// caveat in the header docs is then forced to track the type. + #[test] + fn ingest_uses_generic_failure_code_runtime_invariant() { + use std::any::TypeId; + + // `BurnError` (the typed error used by every sync verb) is + // distinct from the default `napi::Error`. + // `execute_tokio_future` (which `#[napi] async fn ingest` is + // lowered to) is hard-typed to the latter — see the file + // header. If these two ever become assignable, the docs need + // updating. + type DefaultNapiError = napi::Error; + assert_ne!( + TypeId::of::(), + TypeId::of::(), + "BurnError and napi::Error have unified — \ + ingest() can now return BurnError; update the file header \ + docs and switch ingest's return type." + ); + + // Sanity-check the shapes of the codes we *can* deliver vs the + // status code that `ingest()` will surface. These are what + // upstream `JsError::from(Error).into_value(env)` + // writes for the Status::GenericFailure case. + let sync_codes = [SDK_ERROR_CODE, IO_ERROR_CODE, INVALID_ARGUMENT_ERROR_CODE]; + let async_code: &str = napi::Status::GenericFailure.as_ref(); + for c in sync_codes { + assert_ne!( + c, async_code, + "{c} must not collide with the async fallback code {async_code}" + ); + } + assert_eq!(async_code, "GenericFailure"); + } } From 8eccecd2de8789ca086f6cce98376c485ee16c6f Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 6 May 2026 09:13:54 -0400 Subject: [PATCH 5/5] relayburn-sdk-node: BigInt promotion on export verbs (review fix round 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `exportLedger` and `exportStamps` returned `Vec` directly, losing u64 precision for `turnIndex`, `eventIndex`, `contentLength`, `tokensBeforeCompact`, and the rest of the export-record u64 surface (`callIndex`, `byteLen`, `approxTokens`, `retries`, `collapsedCalls`, plus the six `usage` sub-object fields). Anything above 2^53 silently truncated when serde-json's default number conversion crossed the napi boundary. Wrap both verbs in `BigIntPromoting`, the same lighter walker the round-1 commit `290b970` introduced for `overhead` / `overheadTrim` / `hotspots`, and extend `BIGINT_FIELDS` with the camelCased names from TurnRecord / UserTurnRecord / ToolResultEventRecord / CompactionEvent / nested Usage. Audited the other return paths (`search`, `ledgerOpen`, the typed-struct verbs) — those go through `#[napi(object)]` + explicit `BigInt` conversions and don't surface u64 through serde-json. New test `export_record_u64_fields_survive_above_2_pow_53` pins both the membership checks for the four field names CodeRabbit called out and a wrapper round-trip with `(1 << 53) + 1` to keep the failure mode visible if a future change drops a key from `BIGINT_FIELDS`. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/relayburn-sdk-node/src/lib.rs | 173 ++++++++++++++++++++++++--- 1 file changed, 158 insertions(+), 15 deletions(-) diff --git a/crates/relayburn-sdk-node/src/lib.rs b/crates/relayburn-sdk-node/src/lib.rs index b32a93d8..d1c0b2f8 100644 --- a/crates/relayburn-sdk-node/src/lib.rs +++ b/crates/relayburn-sdk-node/src/lib.rs @@ -18,13 +18,17 @@ //! 2^53; the SDK already deals in u64 internally so the boundary is the //! right place to surface that. For verbs whose result is too recursive //! to mirror as a `#[napi(object)]` struct (`overhead`, `overheadTrim`, -//! `hotspots`), we serialize through serde_json and emit the result via -//! the [`BigIntPromoting`] wrapper, which walks the JSON tree and -//! substitutes `BigInt` for any numeric value sitting under one of the -//! well-known u64 field names listed in [`BIGINT_FIELDS`]. The lighter -//! walker keeps the discriminated-union shape of `HotspotsResult` -//! intact (which is awkward to express as a single typed napi object) -//! while still honoring the contract. +//! `hotspots`, `exportLedger`, `exportStamps`), we serialize through +//! serde_json and emit the result via the [`BigIntPromoting`] wrapper, +//! which walks the JSON tree and substitutes `BigInt` for any numeric +//! value sitting under one of the well-known u64 field names listed in +//! [`BIGINT_FIELDS`]. The lighter walker keeps the discriminated-union +//! shape of `HotspotsResult` intact (which is awkward to express as a +//! single typed napi object) and lets the export verbs surface every +//! nested u64 (`turnIndex`, `eventIndex`, `contentLength`, +//! `tokensBeforeCompact`, `byteLen`, `approxTokens`, the six `usage` +//! fields, …) without per-record type plumbing — all while honoring +//! the contract. //! - **Timestamps → ISO-8601 `String`.** The SDK already speaks ISO //! strings (`turn.ts`, `since` parameters); we keep that wire format //! rather than dragging `chrono::DateTime` or `Date` through the FFI. @@ -222,6 +226,30 @@ const BIGINT_FIELDS: &[&str] = &[ "turnsAnalyzed", "analyzed", "excluded", + // export_ledger / export_stamps record bodies — every camelCased + // u64 field on TurnRecord / UserTurnRecord / ToolResultEventRecord / + // CompactionEvent / nested Usage and ToolCall payloads. These values + // already round-trip as u64 inside the SDK; without explicit + // promotion the serde-json bridge emits them as JS `number` (f64) + // and silently truncates anything above 2^53 when crossing the + // napi boundary. + "turnIndex", + "eventIndex", + "callIndex", + "contentLength", + "tokensBeforeCompact", + "byteLen", + "approxTokens", + "retries", + "collapsedCalls", + // nested `usage` shape on TurnRecord / ToolResultEventRecord — + // every field is u64, all six need promotion. + "input", + "output", + "reasoning", + "cacheRead", + "cacheCreate5m", + "cacheCreate1h", ]; fn is_bigint_field(name: &str) -> bool { @@ -231,8 +259,8 @@ fn is_bigint_field(name: &str) -> bool { /// Wraps a `serde_json::Value` so that, when napi-rs converts it to a JS /// value, leaf u64 numbers under the [`BIGINT_FIELDS`] keys come out as /// `BigInt` instead of `number`. Used for the `overhead`, `overheadTrim`, -/// and `hotspots` verbs whose result shapes are documented in -/// `packages/sdk/index.d.ts`. +/// `hotspots`, `exportLedger`, and `exportStamps` verbs whose result +/// shapes are documented in `packages/sdk/index.d.ts`. pub struct BigIntPromoting(JsonValue); impl ToNapiValue for BigIntPromoting { @@ -719,26 +747,41 @@ pub struct ExportStampsOptions { /// Buffered into an array for v1; matches the SDK's /// `export_ledger() -> impl Iterator` behavior (it's already in-memory /// today). A streaming variant is a follow-up. -#[napi(js_name = "exportLedger")] -pub fn export_ledger(opts: Option) -> Result, BurnError> { +/// +/// The result is wrapped in [`BigIntPromoting`] so u64 fields nested +/// inside each `record` object (`turnIndex`, `eventIndex`, `callIndex`, +/// `contentLength`, `tokensBeforeCompact`, `byteLen`, `approxTokens`, +/// `retries`, `collapsedCalls`, and the `usage` sub-object's six u64 +/// keys) cross as JS `BigInt` instead of being silently truncated above +/// 2^53 by the default serde-json `number` conversion. +#[napi(js_name = "exportLedger", ts_return_type = "unknown[]")] +pub fn export_ledger(opts: Option) -> Result { let opts = opts.unwrap_or(ExportLedgerOptions { ledger_home: None }); let raw = sdk::ExportLedgerOptions { ledger_home: maybe_path(opts.ledger_home), }; let iter = sdk::export_ledger(raw).map_err(sdk_err)?; - Ok(iter.collect()) + let values: Vec = iter.collect(); + Ok(BigIntPromoting(JsonValue::Array(values))) } /// Stream every stamp row as a JSONL-shaped JSON object. Sibling of /// [`export_ledger`]. -#[napi(js_name = "exportStamps")] -pub fn export_stamps(opts: Option) -> Result, BurnError> { +/// +/// The result is wrapped in [`BigIntPromoting`] for symmetry with +/// [`export_ledger`]. Stamps don't currently carry u64 fields, but the +/// wrapper is cheap and means a future stamp-shape change that +/// introduces one (e.g. a `byteLen` on a range bound) won't silently +/// regress to f64 truncation. +#[napi(js_name = "exportStamps", ts_return_type = "unknown[]")] +pub fn export_stamps(opts: Option) -> Result { let opts = opts.unwrap_or(ExportStampsOptions { ledger_home: None }); let raw = sdk::ExportStampsOptions { ledger_home: maybe_path(opts.ledger_home), }; let iter = sdk::export_stamps(raw).map_err(sdk_err)?; - Ok(iter.collect()) + let values: Vec = iter.collect(); + Ok(BigIntPromoting(JsonValue::Array(values))) } // --------------------------------------------------------------------------- @@ -907,6 +950,7 @@ mod tests { // Every camelCased u64 field that crosses the boundary today // must be in BIGINT_FIELDS so the walker promotes it. for key in [ + // overhead + hotspots verbs (round-1 set) "tokens", "bytes", "totalLines", @@ -925,6 +969,23 @@ mod tests { "turnsAnalyzed", "analyzed", "excluded", + // export_ledger / export_stamps record-body u64s (round-3 set) + "turnIndex", + "eventIndex", + "callIndex", + "contentLength", + "tokensBeforeCompact", + "byteLen", + "approxTokens", + "retries", + "collapsedCalls", + // nested `usage` shape on TurnRecord / ToolResultEventRecord + "input", + "output", + "reasoning", + "cacheRead", + "cacheCreate5m", + "cacheCreate1h", ] { assert!(is_bigint_field(key), "{key} missing from BIGINT_FIELDS"); } @@ -941,6 +1002,15 @@ mod tests { "perSessionAvg", "path", "kind", + // Record envelope fields that live alongside the promoted + // u64 keys but are themselves a schema version (u32 small) + // or a string discriminant — promoting these would corrupt + // the JSONL contract. + "v", + "ts", + "sessionId", + "messageId", + "source", ] { assert!( !is_bigint_field(key), @@ -949,6 +1019,79 @@ mod tests { } } + #[test] + fn export_record_u64_fields_survive_above_2_pow_53() { + // Pin the round-3 fix: the four field names CodeRabbit called + // out, plus the rest of the export-record u64 surface, must be + // in BIGINT_FIELDS so the BigIntPromoting walker hands them to + // JS as `BigInt`. A regression — anyone removing one of these + // and forgetting to update the export verbs — would silently + // truncate values >2^53 in `exportLedger` / `exportStamps`. + // + // This is a static membership test rather than a live napi-env + // round-trip because the napi sys calls require a running JS + // environment (covered end-to-end by the wave-2 D9 conformance + // suite). The walker's behavior given a matched key is already + // exercised structurally — see `is_bigint_field` / the round-1 + // overhead+hotspots tests — so guarding the membership here is + // load-bearing for the contract. + const ABOVE_2_POW_53: u64 = (1u64 << 53) + 1; + // Sanity: this value is precisely the kind we'd lose to f64 + // rounding, so pinning it in the test doc keeps the failure + // mode visible. + assert!(ABOVE_2_POW_53 > (1u64 << 53)); + assert!(ABOVE_2_POW_53 as f64 as u64 != ABOVE_2_POW_53); + + // The exact field names from CodeRabbit's report. + for key in [ + "turnIndex", + "eventIndex", + "contentLength", + "tokensBeforeCompact", + ] { + assert!( + is_bigint_field(key), + "round-3 fix regressed: {key} not in BIGINT_FIELDS" + ); + } + + // The wrapper itself round-trips a JsonValue::Array (the shape + // export_ledger / export_stamps emit). We can't assert on the + // napi conversion here — see comment above — but we *can* + // build the wrapper to confirm the type plumbing compiles and + // accepts a value-above-2^53 inside a record body shaped like + // the live emitter. + let record = serde_json::json!({ + "v": 1, + "kind": "turn", + "record": { + "turnIndex": ABOVE_2_POW_53, + "eventIndex": ABOVE_2_POW_53, + "contentLength": ABOVE_2_POW_53, + "tokensBeforeCompact": ABOVE_2_POW_53, + }, + }); + let wrapped = BigIntPromoting(JsonValue::Array(vec![record.clone()])); + // We exposed the inner JsonValue as the tuple field; make sure + // the value we just wrapped wasn't lossily reshaped before + // hand-off to the walker — `serde_json::Number::as_u64` is what + // the walker calls, and that path returns the original u64. + if let JsonValue::Array(arr) = &wrapped.0 { + let rec = &arr[0]["record"]; + for k in [ + "turnIndex", + "eventIndex", + "contentLength", + "tokensBeforeCompact", + ] { + let n = rec[k].as_u64().expect("u64 survived the JSON round-trip"); + assert_eq!(n, ABOVE_2_POW_53, "{k} value mutated"); + } + } else { + panic!("BigIntPromoting wrapper dropped the array shape"); + } + } + #[test] fn burn_error_codes_match_constants() { // The TS-exported BurnErrorCode variant values must equal the