Skip to content

relayburn-sdk: gate gap-writer override behind test-utils feature#348

Merged
willwashburn merged 1 commit into
mainfrom
rust-gap-cfg-test
May 7, 2026
Merged

relayburn-sdk: gate gap-writer override behind test-utils feature#348
willwashburn merged 1 commit into
mainfrom
rust-gap-cfg-test

Conversation

@willwashburn

Copy link
Copy Markdown
Member

Summary

Gate set_ingest_gap_writer / restore_ingest_gap_writer (and their pub use re-exports) behind cfg(any(test, feature = "test-utils")). The functions are test-only API for capturing stderr in unit tests; today they ship in the SDK's default public surface, where embedders can hijack the global gap-warning writer for the whole process.

  • Add test-utils feature to relayburn-sdk/Cargo.toml.
  • Gate the two function definitions in ingest/gap.rs.
  • Split the pub use in ingest.rs so only the writer hooks fall under the gate; the other gap helpers (emit_gap_warning, record_session_gap, reset_ingest_gap_warnings, etc.) stay on the default surface.

reset_ingest_gap_warnings is left ungated because its doc explicitly says it's safe to invoke from production code.

The audit for "other test-only-flavored exports" (per #338's scope item 5) found none — the only Test-only: doc-tagged items in the SDK source are the two gated here and reset_ingest_gap_warnings (intentionally not gated).

Closes #338.

Test plan

  • cargo build --workspace --all-targets clean (default features — gate active)
  • cargo test --workspace passes — 729 tests, all green; in-crate tests reach the API via cfg(test)
  • cargo build -p relayburn-sdk --features test-utils --all-targets clean
  • No external callsites: rg "set_ingest_gap_writer|restore_ingest_gap_writer" crates/relayburn-cli crates/relayburn-sdk-node returns nothing

🤖 Generated with Claude Code

Move `set_ingest_gap_writer` / `restore_ingest_gap_writer` (and their
`pub use` re-exports) behind `cfg(any(test, feature = "test-utils"))`.
The functions are test-only API for capturing stderr in unit tests;
today they ship in the SDK's default public surface, where embedders
can hijack the global gap-warning writer for the whole process.

- Add `test-utils` feature to relayburn-sdk/Cargo.toml.
- Gate the two function definitions in `ingest/gap.rs`.
- Split the `pub use` block in `ingest.rs` so the writer hooks only
  re-export under the gate; non-writer items (`emit_gap_warning`,
  `record_session_gap`, `reset_ingest_gap_warnings`, etc.) stay public.

`reset_ingest_gap_warnings` is left ungated because its doc explicitly
calls out that it's safe to invoke from production code.

Closes #338.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: aa7dbd6f-2e6e-484f-95e7-e925ea6d49c9

📥 Commits

Reviewing files that changed from the base of the PR and between c871a04 and 3cf2c62.

📒 Files selected for processing (3)
  • crates/relayburn-sdk/Cargo.toml
  • crates/relayburn-sdk/src/ingest.rs
  • crates/relayburn-sdk/src/ingest/gap.rs

📝 Walkthrough

Walkthrough

The PR restricts test-only gap writer override functions by introducing a test-utils Cargo feature and gating their public availability with cfg(any(test, feature = "test-utils")). This prevents accidental exposure of mutable process-global state to production code.

Changes

Test-Utils Feature Gate

Layer / File(s) Summary
Feature Declaration
crates/relayburn-sdk/Cargo.toml
New opt-in test-utils feature is declared with documentation explaining it exposes test-only SDK hooks.
Core Function Gating
crates/relayburn-sdk/src/ingest/gap.rs
Functions set_ingest_gap_writer and restore_ingest_gap_writer are gated with #[cfg(any(test, feature = "test-utils"))] to restrict compilation to in-crate tests or feature-enabled builds.
Re-export Wiring
crates/relayburn-sdk/src/ingest.rs
The gap module's unconditional re-export is split: core APIs remain public, while writer overrides are conditionally re-exported only under the test/feature gate.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • AgentWorkforce/burn#298: Integration tests that install/restore a buffer-backed gap warning writer depend on the now-gated set_ingest_gap_writer and restore_ingest_gap_writer functions.
  • AgentWorkforce/burn#305: Ingest test rewiring uses the same writer override functions that are gated behind cfg(any(test, feature = "test-utils")).
  • AgentWorkforce/burn#296: Earlier work that introduced the gap writer functions now restricted by this PR's visibility gating.

Poem

🐰 A fox once swapped the writer's pen,
And chaos spread through logs and men.
Now gated safe behind cfg(test),
The SDK rests, unbothered and blessed!
Squeak — test helpers stay where they belong! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: gating the gap-writer override functions behind a test-utils feature.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, explaining the purpose, implementation, and testing of the gap-writer API gating.
Linked Issues check ✅ Passed The PR fully addresses issue #338's requirements: gates set_ingest_gap_writer and restore_ingest_gap_writer behind cfg(any(test, feature = 'test-utils')), adds the test-utils feature, updates re-exports, and leaves reset_ingest_gap_warnings ungated as intended.
Out of Scope Changes check ✅ Passed All changes are directly in scope: the Cargo.toml feature addition, gap.rs function gating, and ingest.rs re-export splitting align precisely with issue #338's scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rust-gap-cfg-test

Comment @coderabbitai help to get the list of available commands and usage tips.

@willwashburn willwashburn merged commit 08fe38a into main May 7, 2026
8 checks passed
@willwashburn willwashburn deleted the rust-gap-cfg-test branch May 7, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rust hygiene: gap.rs writer should be #[cfg(test)]-gated, not a public swap

1 participant