feat(sdk): add writeStamp for exact session-id stamping#426
Conversation
Launchers that know the session id up front — for example, a Claude
launcher that preallocates `--session-id <uuid>` before spawn — can now
call `@relayburn/sdk` `writeStamp({ sessionId, enrichment })` to fold
enrichment straight onto the ledger by selector, skipping the
`writePendingStamp` sidecar manifest matching path entirely.
The companion `writePendingStamp` flow still serves harnesses that don't
expose a pre-spawn session id (Codex, OpenCode). `writeStamp` is the
more reliable path when the id is available: no cwd/pid race, no orphan
manifest if the spawn fails, and the stamp resolves immediately at
ingest time rather than after the launcher first writes a turn.
The verb is exposed as a free function and a `LedgerHandle` method in
the Rust SDK (`relayburn_sdk::write_stamp`, `LedgerHandle::write_stamp`)
plus a napi-bound `writeStamp` in `@relayburn/sdk`. Selectors are
either `sessionId` or `messageId`; empty selectors reject via
`StampError::EmptySelector` to keep `writePendingStamp`'s "no
matched-everything stamps" guarantee.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a new ChangeswriteStamp Feature
Sequence DiagramsequenceDiagram
participant Launcher as Launcher/Caller
participant write_stamp_fn as write_stamp Function
participant StampBuilder as Stamp Builder
participant Ledger
Launcher->>write_stamp_fn: WriteStampOptions{sessionId, enrichment}
write_stamp_fn->>StampBuilder: convert selector and enrichment
StampBuilder->>StampBuilder: auto-generate ISO timestamp if omitted
StampBuilder->>Ledger: Stamp::new
write_stamp_fn->>Ledger: append stamp
Ledger-->>write_stamp_fn: persisted
write_stamp_fn-->>Launcher: Result<()>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
crates/relayburn-sdk/src/stamp_verb.rs (3)
26-30: ⚡ Quick winConsider removing
Defaultderive or adding enrichment validation.
WriteStampOptionsderivesDefaultwith a non-optionalenrichmentfield. This allows callers to construct an instance with empty enrichment viaWriteStampOptions::default(), which the PR description says should be rejected. However, validation only occurs at the N-API boundary (line 521-523 incrates/relayburn-sdk-node/src/lib.rs), not in the Rust SDK layer.Consider either:
- Removing
#[derive(Default)]so invalid states cannot be constructed- Adding validation in
build_stamporwrite_stampto reject empty enrichment- Documenting that validation is the caller's responsibility
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/relayburn-sdk/src/stamp_verb.rs` around lines 26 - 30, WriteStampOptions currently derives Default even though enrichment is required, allowing invalid empty-enrichment instances; remove #[derive(Default)] from WriteStampOptions to prevent constructing default/invalid states, and/or add a runtime check in build_stamp and/or write_stamp (e.g., validate enrichment is non-empty at the start of those functions) that returns an error if enrichment is empty; update any callers/tests that relied on WriteStampOptions::default() or handle the new error path accordingly.
71-82: ⚡ Quick winTimestamp conversion truncates subsecond precision and silently falls back on errors.
The
now_isofunction has two issues:
Precision loss (lines 72-75): Converting
SystemTimeto seconds viaduration_since(UNIX_EPOCH).as_secs()truncates milliseconds/nanoseconds. The N-API layer'sparse_iso_system_timepreserves fractional seconds (lines 563-570), creating an asymmetry.Silent error handling (lines 72-77): Both
duration_sinceandfrom_unix_timestampfailures useunwrap_orto fall back to0orUNIX_EPOCH, hiding potential clock issues.If subsecond precision is not needed for stamps, document this intentionally. Otherwise, preserve milliseconds using
time::Duration::milliseconds()or similar. For error handling, consider propagating errors instead of silent fallback.
89-103: 💤 Low valueTest validates empty enrichment rejection, but Rust SDK doesn't enforce it.
The test
empty_selector_is_rejected(lines 89-103) usesenrichment: BTreeMap::new()(line 94), which is empty. However, the test only verifies selector validation; it doesn't test enrichment validation. Per the PR description, empty enrichment should also be rejected, but this validation only exists at the N-API boundary.Consider adding a test case that verifies rejection of empty enrichment at the Rust SDK level if validation is added per the previous comment, or clarify in documentation that enrichment validation is deferred to higher layers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/relayburn-sdk/src/stamp_verb.rs` around lines 89 - 103, The test points out empty enrichment maps are currently allowed at the Rust SDK level; add a validation step in the write_stamp function (or in the constructor/validation for WriteStampOptions) to reject an empty enrichment BTreeMap by returning an error when enrichment.is_empty() is true, and update or add a unit test mirroring empty_selector_is_rejected that calls write_stamp with enrichment: BTreeMap::new() and asserts the error contains "enrichment" so empty enrichment is enforced in the Rust SDK rather than only at the N-API boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 9-11: Update the changelog entry to mention both SDK surfaces: add
a line for the Rust symbol relayburn_sdk::write_stamp and the Node export
`@relayburn/sdk.writeStamp` (or combine into one line stating the feature exists
in both), and replace the implementation-detail phrase "bypassing the sidecar
`writePendingStamp` matching path" with a concise user-facing description such
as "skipping manifest matching" or "writes enrichment immediately using the
known session id" to align with existing entries like the `relayburn-sdk:`
prefix style.
---
Nitpick comments:
In `@crates/relayburn-sdk/src/stamp_verb.rs`:
- Around line 26-30: WriteStampOptions currently derives Default even though
enrichment is required, allowing invalid empty-enrichment instances; remove
#[derive(Default)] from WriteStampOptions to prevent constructing
default/invalid states, and/or add a runtime check in build_stamp and/or
write_stamp (e.g., validate enrichment is non-empty at the start of those
functions) that returns an error if enrichment is empty; update any
callers/tests that relied on WriteStampOptions::default() or handle the new
error path accordingly.
- Around line 89-103: The test points out empty enrichment maps are currently
allowed at the Rust SDK level; add a validation step in the write_stamp function
(or in the constructor/validation for WriteStampOptions) to reject an empty
enrichment BTreeMap by returning an error when enrichment.is_empty() is true,
and update or add a unit test mirroring empty_selector_is_rejected that calls
write_stamp with enrichment: BTreeMap::new() and asserts the error contains
"enrichment" so empty enrichment is enforced in the Rust SDK rather than only at
the N-API boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 048bec1b-db49-4c17-8677-fcf3f9e7bac4
📒 Files selected for processing (8)
CHANGELOG.mdcrates/relayburn-sdk-node/src/lib.rscrates/relayburn-sdk/src/lib.rscrates/relayburn-sdk/src/stamp_verb.rspackages/sdk-node/CHANGELOG.mdpackages/sdk-node/src/index.d.tspackages/sdk-node/src/index.jspackages/sdk-node/test/conformance.test.js
| - `@relayburn/sdk`: `writeStamp({ sessionId | messageId, enrichment })` for | ||
| launchers that know the session id up front (e.g. preallocated Claude | ||
| `--session-id`), bypassing the sidecar `writePendingStamp` matching path. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Changelog entry should mention both Rust and Node SDK surfaces.
The entry documents @relayburn/sdk (Node package) but omits the Rust SDK surface. Per the PR objectives, this feature adds relayburn_sdk::write_stamp (Rust) and @relayburn/sdk.writeStamp (Node). Looking at other entries in this changelog (e.g., lines 15-20, 26-29), Rust SDK changes are documented with the relayburn-sdk: prefix.
Consider splitting or expanding the entry:
- `relayburn-sdk` (Rust): `write_stamp({ sessionId | messageId, enrichment })`
for direct ledger writes by exact selector.
- `@relayburn/sdk` (Node): `writeStamp` export wraps the Rust SDK verb.Or combine them:
- `writeStamp({ sessionId | messageId, enrichment })` in both Rust SDK
and `@relayburn/sdk` (Node) for launchers that know session id up front
(e.g. preallocated Claude `--session-id`).As per coding guidelines, the phrase "bypassing the sidecar writePendingStamp matching path" leans toward implementation detail. Consider tightening to: "skipping manifest matching" or focusing on the practical benefit: "writes enrichment immediately using the known session id."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 9 - 11, Update the changelog entry to mention both
SDK surfaces: add a line for the Rust symbol relayburn_sdk::write_stamp and the
Node export `@relayburn/sdk.writeStamp` (or combine into one line stating the
feature exists in both), and replace the implementation-detail phrase "bypassing
the sidecar `writePendingStamp` matching path" with a concise user-facing
description such as "skipping manifest matching" or "writes enrichment
immediately using the known session id" to align with existing entries like the
`relayburn-sdk:` prefix style.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bae2bad9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let fmt = time::macros::format_description!( | ||
| "[year]-[month]-[day]T[hour]:[minute]:[second]Z" | ||
| ); |
There was a problem hiding this comment.
write_stamp now defaults ts to YYYY-MM-DDTHH:MM:SSZ, but existing stamp writers use millisecond ISO strings (...SS.mmmZ). Stamp application order is read from SQLite as text-ordered ORDER BY ts, written_at (collect_stamps in crates/relayburn-sdk/src/ledger/reader.rs), so mixing second-only and millisecond formats can invert precedence for stamps in the same second (e.g. .500Z sorts before Z). That can cause later enrichment to be overwritten by older data.
Useful? React with 👍 / 👎.
| if opts.session_id.is_none() && opts.message_id.is_none() { | ||
| return Err(invalid_arg( | ||
| "writeStamp requires at least one of sessionId or messageId", | ||
| )); |
There was a problem hiding this comment.
The argument guard only checks whether sessionId/messageId are None, so "" passes validation and is written as a selector. Because matching is exact, an empty id will usually match no turns, producing a successful-but-noop stamp row that is hard for callers to detect. This is especially easy when values come from env vars/flags that may be present but empty.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
….7/2.8.6 Main released 2.9.0, 2.8.7, and 2.8.6 (#421, #422, #423, #426). The auto-merge against ingest.rs and lib.rs was clean (no behavior conflict with #423's walk dedupe + run_single_harness refactor); only CHANGELOG needed manual sorting to keep this branch's items under [Unreleased] above the new release sections. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
1 participant
Summary
writeStamp({ sessionId | messageId, enrichment, ts?, ledgerHome? })to@relayburn/sdkandrelayburn_sdkfor launchers that know the session id up front — companion towritePendingStamp, skips the sidecar manifest matching path entirelyrelayburn_sdk::write_stamp) and aLedgerHandle::write_stampmethod; the Node SDK gets awriteStampasync export wired through napiStampError::EmptySelectorto preservewritePendingStamp's "no matched-everything stamps" guaranteeWhy
External launchers that already control a Claude Code session id (via
claude --session-id <uuid>) can stamp burn directly by selector instead of relying on the sidecarcwd + spawnerPid + spawnStartTsmatch. This removes a class of failure modes: cwd-rewrite races, orphan manifests on spawn failure, GC of pending stamps after 24h, and the lag between writing the manifest and burn ingest finding it.The first consumer is a Pear ↔ relay ↔ burn integration that preallocates the UUID, injects
--session-idinto the spawn args, and callswriteStampso the resulting session shows up inburn summary --tags spawner=pearwith no ambiguity about which jsonl file is which session. That integration ships as a follow-up PR inpearandrelayonce this lands and@relayburn/sdkrepublishes.What's in the change
WriteStampOptions, the free function, theLedgerHandlemethod, and 3 unit tests covering session-selector round-trip, empty-selector rejection, and default-ts ISO formattingwriteStampwith the standardBurnErrorinvariant-arg validation (empty selector, empty enrichment, empty enrichment key all reject asBURN_INVALID_ARGUMENT)writeStampexport + TS declarationexportStamps, empty-selector rejection, empty-enrichment rejection[Unreleased]entries in both the cross-package CHANGELOG and the Node SDK CHANGELOGThe committed
binding.cjs/binding.d.tsare intentionally left as their stub state — thenapi-buildworkflow regenerates them in CI with the newwriteStampexport present.Test plan
cargo test -p relayburn-sdk stamp_verb— 3 Rust unit tests passcargo test --workspace— full Rust workspace greencargo build -p relayburn-sdk-node— napi binding compilespnpm -F @relayburn/sdk run build:napi && RELAYBURN_SDK_NAPI_BUILT=1 pnpm -F @relayburn/sdk test— all 11 conformance tests pass (incl. 3 new writeStamp tests).nodeartifacts + binding.cjs / binding.d.ts with writeStamp export🤖 Generated with Claude Code