From ce71db4b87dcbe1ddf32b3b56c09a91cf3479e81 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 9 May 2026 03:13:07 +0000 Subject: [PATCH 1/3] fix(cli): stop calling unsafe std::env::set_var in compare/summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `ArchiveOverride` guard mutated `RELAYBURN_ARCHIVE` for the duration of `burn compare` / `burn summary` calls, but the Rust SDK never reads that variable — its own comment admits it's a no-op preserved "for any future archive layer." Under Rust 1.84+ `std::env::set_var` is `unsafe` because it can race with concurrent reads in the same process, so this is a footgun for parallel test runs and embedders alike with no upside. Drop the guard entirely from both call sites; `--no-archive` becomes an explicit no-op (it always was, plus a process-global env mutation). The flag stays on the CLI for parity with the TS surface. The other site flagged in #335 (`run.rs` mutating `RELAYBURN_HOME`) no longer exists — the harness adapters now resolve their ledger handle via `LedgerOpenOptions` directly, so that half of the issue is already done. Closes #335 --- CHANGELOG.md | 3 ++ crates/relayburn-cli/src/cli.rs | 4 +- crates/relayburn-cli/src/commands/compare.rs | 44 ++------------------ crates/relayburn-cli/src/commands/summary.rs | 41 ++---------------- 4 files changed, 11 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a7dc8a2..96391d6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ Cross-package release notes for relayburn. Package changelogs contain package-le - `relayburn-cli`: `burn summary` partial-coverage footers now name the token field with the largest gap and clarify that totals still include all matched turns. +- `relayburn-cli`: drop the `RELAYBURN_ARCHIVE` env mutation in `burn compare` + and `burn summary`. The Rust SDK never read the variable, and `set_var` is + unsafe under Rust 1.84+. `--no-archive` is now an explicit no-op. ## [2.6.0] - 2026-05-08 diff --git a/crates/relayburn-cli/src/cli.rs b/crates/relayburn-cli/src/cli.rs index b9cafa3a..e10e3590 100644 --- a/crates/relayburn-cli/src/cli.rs +++ b/crates/relayburn-cli/src/cli.rs @@ -244,8 +244,8 @@ pub struct CompareArgs { #[arg(long)] pub csv: bool, - /// Bypass the SQLite archive and stream the ledger directly. - /// Honored when env `RELAYBURN_ARCHIVE=0`. + /// Accepted for TS CLI flag parity; a no-op against the Rust SDK, + /// which is SQLite-native and has no archive layer to bypass. #[arg(long = "no-archive")] pub no_archive: bool, } diff --git a/crates/relayburn-cli/src/commands/compare.rs b/crates/relayburn-cli/src/commands/compare.rs index 5b43b220..94edcd2f 100644 --- a/crates/relayburn-cli/src/commands/compare.rs +++ b/crates/relayburn-cli/src/commands/compare.rs @@ -128,11 +128,9 @@ fn run_inner(globals: &GlobalArgs, args: CompareArgs) -> Result { return Err(anyhow!("invalid --min-sample: {min_sample}")); } - // 6. Honor --no-archive by exporting RELAYBURN_ARCHIVE=0 for the - // duration of this call. The Rust SDK doesn't read RELAYBURN_ARCHIVE - // today (it's SQLite-only), but we set the env so any future archive - // layer behaves identically to the TS CLI's `--no-archive`. - let _archive_guard = ArchiveOverride::activate(args.no_archive); + // 6. `--no-archive` is accepted for TS CLI flag parity but is a no-op: + // the Rust SDK is SQLite-native and has no archive layer to bypass. + let _ = args.no_archive; // 7. Build the Query. let mut q = Query::default(); @@ -490,42 +488,6 @@ fn days_to_ymd(days_from_epoch: i64) -> (i64, u32, u32) { (year, m as u32, d as u32) } -/// Drop-in for `RELAYBURN_ARCHIVE=0`. Restores the previous value on -/// drop so a panic part-way through doesn't leak the override. -struct ArchiveOverride { - previous: Option, - activated: bool, -} - -impl ArchiveOverride { - fn activate(no_archive: bool) -> Self { - if !no_archive { - return Self { - previous: None, - activated: false, - }; - } - let previous = std::env::var("RELAYBURN_ARCHIVE").ok(); - std::env::set_var("RELAYBURN_ARCHIVE", "0"); - Self { - previous, - activated: true, - } - } -} - -impl Drop for ArchiveOverride { - fn drop(&mut self) { - if !self.activated { - return; - } - match self.previous.take() { - Some(v) => std::env::set_var("RELAYBURN_ARCHIVE", v), - None => std::env::remove_var("RELAYBURN_ARCHIVE"), - } - } -} - // --------------------------------------------------------------------------- // number formatting (matches packages/cli/src/format.ts) // --------------------------------------------------------------------------- diff --git a/crates/relayburn-cli/src/commands/summary.rs b/crates/relayburn-cli/src/commands/summary.rs index f74e6ea7..61c6a22f 100644 --- a/crates/relayburn-cli/src/commands/summary.rs +++ b/crates/relayburn-cli/src/commands/summary.rs @@ -206,7 +206,9 @@ fn run_inner(globals: &GlobalArgs, args: SummaryArgs) -> anyhow::Result { None }; - let _archive_guard = ArchiveOverride::activate(args.no_archive); + // `--no-archive` is accepted for TS CLI flag parity but is a no-op: + // the Rust SDK is SQLite-native and has no archive layer to bypass. + let _ = args.no_archive; let progress = TaskProgress::new(globals, "summary"); let opts = match globals.ledger_path.as_deref() { @@ -346,43 +348,6 @@ fn run_ingest( }) } -/// Drop-in for `RELAYBURN_ARCHIVE=0`. The Rust SDK is already SQLite-native, -/// but this preserves the TS CLI flag contract for any lower layer that checks -/// the env escape hatch. -struct ArchiveOverride { - previous: Option, - activated: bool, -} - -impl ArchiveOverride { - fn activate(no_archive: bool) -> Self { - if !no_archive { - return Self { - previous: None, - activated: false, - }; - } - let previous = std::env::var("RELAYBURN_ARCHIVE").ok(); - std::env::set_var("RELAYBURN_ARCHIVE", "0"); - Self { - previous, - activated: true, - } - } -} - -impl Drop for ArchiveOverride { - fn drop(&mut self) { - if !self.activated { - return; - } - match self.previous.take() { - Some(v) => std::env::set_var("RELAYBURN_ARCHIVE", v), - None => std::env::remove_var("RELAYBURN_ARCHIVE"), - } - } -} - const COVERAGE_FIELDS: [CoverageField; 5] = [ CoverageField::Input, CoverageField::Output, From 8e6c17e8d9f71dc8592493b18b8c78f26c8a5d59 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 9 May 2026 03:15:30 +0000 Subject: [PATCH 2/3] fix(cli): align summary --no-archive help text with no-op behavior Codex review on #399 caught that the SummaryArgs flag still advertised itself as an 'archive-corruption escape hatch' even though run_inner now drops the value as a no-op. Match the wording already used on the compare flag so users don't expect behavior that isn't there. --- crates/relayburn-cli/src/commands/summary.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/relayburn-cli/src/commands/summary.rs b/crates/relayburn-cli/src/commands/summary.rs index 61c6a22f..afd63551 100644 --- a/crates/relayburn-cli/src/commands/summary.rs +++ b/crates/relayburn-cli/src/commands/summary.rs @@ -104,9 +104,8 @@ pub struct SummaryArgs { #[arg(long)] pub quality: bool, - /// Bypass the archive sidecar and stream the ledger. Kept for parity - /// with the TS CLI and as an escape hatch for archive-corruption - /// debugging. + /// Accepted for TS CLI flag parity; a no-op against the Rust SDK, + /// which is SQLite-native and has no archive layer to bypass. #[arg(long = "no-archive")] pub no_archive: bool, } From ca8e0e6ea38136681ad577a9e310252477dca830 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 9 May 2026 03:16:07 +0000 Subject: [PATCH 3/3] docs(changelog): trim --no-archive entry to impact-first form CodeRabbit nit on #399: the prior wording leaked implementation backstory (RELAYBURN_ARCHIVE, set_var, Rust 1.84+) that the repo's changelog guideline asks to drop. --- CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96391d6b..130adb9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,8 @@ Cross-package release notes for relayburn. Package changelogs contain package-le - `relayburn-cli`: `burn summary` partial-coverage footers now name the token field with the largest gap and clarify that totals still include all matched turns. -- `relayburn-cli`: drop the `RELAYBURN_ARCHIVE` env mutation in `burn compare` - and `burn summary`. The Rust SDK never read the variable, and `set_var` is - unsafe under Rust 1.84+. `--no-archive` is now an explicit no-op. +- `relayburn-cli`: `--no-archive` on `burn compare` and `burn summary` is now + an explicit no-op (accepted for TS CLI flag parity). ## [2.6.0] - 2026-05-08