Skip to content

feat(stelae): Stelae protocol crate — walking skeleton - #1147

Merged
scarmuega merged 5 commits into
mainfrom
feat/stelae-core-crate
Jul 31, 2026
Merged

feat(stelae): Stelae protocol crate — walking skeleton#1147
scarmuega merged 5 commits into
mainfrom
feat/stelae-core-crate

Conversation

@scarmuega

@scarmuega scarmuega commented Jul 31, 2026

Copy link
Copy Markdown
Member

Stelae protocol crate — walking skeleton

Implements the dolos-stelae-core-crate plan, the
first slice of Phase 1a of adrs/004_stelae_snapshots.md. Base: main at c7507799.

The slice is deliberately narrow: the smallest thing that writes a stele to a
directory and reads it back, for a profile the core knows nothing about, with a
byte-reproducible inscription digest. The point is not the feature — nothing here is
reachable from the CLI yet — it is to retire the determinism risks the rest of the
protocol is built on, before any profile code exists to bias the design.

The five unknowns, answered

The plan makes answering these as much the deliverable as the code. Each answer names
the test that establishes it.

1. Does minicbor emit RFC 8949 §4.2.1 canonical output by default, and can the reader reject non-canonical input?

Yes to the first, with two documented gaps; the reader rejects, because the crate
implements the check itself.

minicbor 0.26 emits shortest-form integers and definite lengths for the whole subset
the format uses — verified against byte strings taken from RFC 8949 §3 and Appendix A
at every width boundary (0, 23, 24, 255, 256, 65535, 65536, 2^32−1, 2^32, u64::MAX,
and the negatives), in frame::tests::minicbor_emits_shortest_form_integers and
frame::tests::minicbor_emits_definite_lengths.

Two things the encoder cannot know are wrong, because the caller asked for them:
an explicitly opened indefinite-length container, and map keys written out of order.
Both are caught by validating in frame::encode, so they cannot reach a blob —
frame::tests::encode_rejects_indefinite_lengths_from_minicbor and
frame::tests::encode_rejects_unsorted_map_keys.

On the read side minicbor's decoder is permissive by design, so no fallback
hand-rolled encoder was needed, but a canonical-form validator was
: frame::scan_item
walks the bytes and rejects non-shortest integers, indefinite lengths, floats, tags,
undefined, non-UTF-8 text, unsorted or duplicate map keys, and nesting past a depth
bound. Rejection tests: frame::tests::rejects_non_shortest_integers,
rejects_indefinite_lengths, rejects_floats_tags_and_undefined,
rejects_unsorted_and_duplicate_map_keys, rejects_invalid_utf8_and_truncation,
rejects_excessive_nesting. Write→read→write byte identity:
frame::tests::sequence_roundtrip_is_byte_identical, and at layer level
toy_profile::layers_round_trip_byte_identically.

This matters because a layer's identity is the sha256 of these bytes. A producer
emitting a sloppy encoding would publish a diffId that no independent re-encoding
reproduces — which is a determinism failure that only shows up when a second publisher
tries to attest. Rejecting at the door keeps "reproduce the digest" decidable.

2. Does the chosen JCS crate actually implement RFC 8785, and how does it render integers?

serde_jcs 0.2.0 — the crate ADR-004 names — passes. No swap needed.

All six official vectors from the RFC 8785 reference suite pass byte for byte
(rfc8785::official_vectors), including weird, which is the one that matters: it
pins sorting by UTF-16 code units, so 😂 (surrogate pair D83D DE02) must sort
before דּ (FB33) — the opposite of what UTF-8 bytewise ordering gives. The full
Appendix B number-serialization table passes too (rfc8785::appendix_b_number_serialization),
which is where two "conformant" implementations most plausibly diverge, since number
rendering is ECMAScript's algorithm rather than anything JSON specifies. Canonicalization
is also a fixed point (rfc8785::canonicalization_is_idempotent).

All three candidates were measured before choosing:

crate 6 official vectors integer > 2^53−1 i64::MIN
serde_jcs 0.2.0 pass silently rounds silently rounds
serde_json_canonicalizer 0.3.2 pass silently rounds silently rounds
json-canon 0.1.3 pass hard error panics

json-canon's hard error is the nicer behaviour, but it panics on i64::MIN — and
an inscription is untrusted input by definition, so a panic there is a denial of service
on any verifier. Disqualifying. serde_jcs was kept, matching the ADR.

Integer rendering: integers render as plain digits, no exponent and no decimal
point, for everything inside ±(2^53−1) — rfc8785::integers_render_as_plain_digits.
Past that the crate keeps working and starts lying: u64::MAX renders as
18446744073709552000. That behaviour is pinned by
rfc8785::beyond_the_safe_range_rendering_is_lossy, so the plan's JCS-safe-integer rule
reads as load-bearing rather than as belt-and-braces. inscription::check_safe_numbers
refuses any non-integer or out-of-range number including inside the profile's opaque
position, parameters and scope
, which is exactly where a vendor would most
plausibly park a raw u64
(inscription::tests::rejects_numbers_outside_the_jcs_safe_integer_range,
rejects_non_integer_numbers, parse_rejects_unsafe_numbers_in_raw_bytes).

3. Does identity really survive compression variance?

Yes. The same records compressed at zstd levels 1, 9 and 19 produce one diffId
and three distinct blob digests, and all three decompress back to identical bytes —
digest::tests::identity_survives_compression_variance. So a publisher who compresses
differently still reproduces the inscription, while a registry still addresses each blob
by its own bytes. Amendment 4 of the parent plan holds as specified.

The pipeline computes both digests and both sizes in a single streaming pass over the
data, nothing buffered (digest::tests::one_pass_yields_both_digests,
read_back_reproduces_the_digests).

4. Can a profile plug in with the core knowing nothing about it?

Yes, and the toy profile is the only profile in the tree — there is no Dolos
profile yet, by design, so the core cannot have been written against one.

tests/toy_profile.rs defines dev.example.toy, which publishes chapters of notes: no
chain, no epochs, no blocks, no ledger. It drives the full path — build records, write
two layers, write the inscription, read the whole thing back, verify.

  • the_core_composes_no_vendor_owned_string asserts every media type and tag in the
    artifact is character-for-character what the profile returned, and that the canonical
    document contains neither dolos nor cardano.
  • opaque_fields_are_carried_not_interpreted pushes arbitrary shapes (nested objects,
    arrays, strings, null) through position, parameters and both scopes and gets
    them back unchanged.
  • a_foreign_profile_is_refused and a_profile_cannot_claim_the_protocols_namespace
    cover the two failure directions.

The trait wanted nothing Dolos-shaped, so the escalation trigger the plan names for
this did not fire. Profile answers only naming questions — name, major version, kinds,
layer media type, tag for a sequence. The protocol asks for every vendor-owned string
and validates the answer against the normative rules (checked_layer_media_type,
checked_tag_for_sequence); it composes none of them, and refuses a profile that tries
to claim vnd.stelae.*.

5. Does the crate boundary hold mechanically?

The boundary holds; the mechanical guard was dropped on review.
cargo tree -p stelae -e normal yields a 33-package tree with no dolos-* in it, and
nothing in the crate imports one.

A stelae-boundary CI job originally asserted this, and I did negative-test it —
temporarily adding dolos-core = { path = "../core" } to [dependencies] made it fire,
then reverted. It has since been removed at the author's request, so the boundary is
now a convention documented in crates/stelae/Cargo.toml and lib.rs rather than
something CI enforces. cargo tree -p stelae -e normal shows it holding on demand.

(Worth recording, since the job is gone: CodeRabbit correctly caught that its
grep -E '^dolos-' would have missed the root crate, which is named exactly dolos.
The guard would have passed on a real break. Anyone reinstating it wants
^dolos(-|$).)

Dependency cost is two new packages in the whole lockfile: serde_jcs and its
ryu-js. minicbor is pinned to the 0.26 line Pallas already pulls in, so no second
CBOR implementation enters the tree; sha2, zstd, hex, serde*, thiserror and
tempfile were already there.

Done criterion

Four of five met as written; item 3 was deliberately narrowed after the plan was
released — see the note below the table.

# Criterion Status
1 cargo test -p stelae passes, incl. CBOR-seq roundtrip + write→read→write identity, non-canonical rejection, RFC 8785 vectors, zstd-levels identity, history gap/duplicate/out-of-order, fail-closed on unknown generic key / unknown profile / higher profile major Met — 71 tests, 0 failures
2 tests/toy_profile.rs writes a stele to a temp dir and reads it back; two independent write runs give an identical inscription digest Metwrites_a_stele_and_reads_it_back, two_independent_writes_produce_the_same_inscription_digest
3 cargo tree -p stelae -e normal free of dolos-*, asserted by a CI step Partial — the tree is free of dolos-*; the CI assertion was removed at the author's request
4 clippy -D warnings, cargo +nightly fmt --check, cargo deny check advisories pass with the new deps Met — see below
5 PR body answers all five unknowns, naming the test for each Met — above

On item 3. The plan asked for the boundary to be asserted by a CI step, and the
first push did that. The author subsequently asked for the job to be removed, so the
criterion is now met in substance (the tree is clean) but not in mechanism (nothing
fails the build if that changes). Recorded rather than quietly restated: reinstating it
is a ten-line workflow job, and the regex it needs is ^dolos(-|$).

Verification

$ cargo test -p stelae
running 53 tests   test result: ok. 53 passed; 0 failed; 0 ignored     (lib)
running 5 tests    test result: ok.  5 passed; 0 failed; 0 ignored     (tests/rfc8785.rs)
running 12 tests   test result: ok. 12 passed; 0 failed; 0 ignored     (tests/toy_profile.rs)
running 1 test     test result: ok.  1 passed; 0 failed; 0 ignored     (doc-tests)

$ cargo test --workspace --all-targets
TOTAL passed=703 failed=0 ignored=16          # 16 ignored = the e2e suite CI runs separately

$ cargo clippy --all-targets --all-features -- -D warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 18.26s

$ cargo +nightly fmt --all -- --check
(no output)

$ cargo deny check advisories
advisories ok

cargo tree -p stelae -e normal:

stelae v1.6.0
├── hex v0.4.3
├── minicbor v0.26.4
├── serde v1.0.228
├── serde_jcs v0.2.0
│   ├── ryu-js v0.2.2
│   ├── serde v1.0.228 (*)
│   └── serde_json v1.0.150
├── serde_json v1.0.150 (*)
├── sha2 v0.10.9
├── thiserror v2.0.18
└── zstd v0.13.3

Finding: three pre-existing cargo deny yanked warnings

cargo deny check advisories exits 0, but warns on three yanked crates: fjall 3.1.0,
lsm-tree 3.1.0 and spin 0.9.8 (via flume). All three are pre-existing and
unrelated to this branch
— they reach the tree through dolos-fjall and
mithril-client, and deny.toml already anticipates them by name in its rationale for
yanked = "warn". Neither serde_jcs nor ryu-js appears anywhere in the output.
Reported rather than acted on: nothing was pinned or added to ignore.

What changed

  • crates/stelae/ (new, package stelae) — lib.rs (errors, protocol constants,
    the three envelope media types), frame.rs, digest.rs, inscription.rs,
    profile.rs, dir.rs; tests/toy_profile.rs, tests/rfc8785.rs, and the vendored
    RFC 8785 vectors under tests/data/rfc8785/ with their provenance in a README.
  • Cargo.tomlcrates/stelae added to workspace members; minicbor, serde_jcs,
    sha2, zstd added to [workspace.dependencies].

Nothing outside crates/stelae and the workspace manifest is touched; the branch no
longer changes .github/workflows/ci.yml at all.

Two design notes worth a reviewer's eye:

dir.rs scans to rebuild a map a manifest would give it. An inscription lists
diffIds (identity) and deliberately not compressed digests (transport, which live in the
OCI manifest). With no manifest, SteleDir::blob_index recovers the diffId → blob map by
decompressing every blob once. That is the right cost for a fixture and the wrong one for
a registry; oci.rs supplies the map for free in Phase 3. Flagged in the module docs.

Golden digests are pinned. toy_profile::golden_digests_pin_the_encoding asserts the
exact inscription digest, the two layer diffIds, and the full canonical JSON string. Every
value is over spec-determined bytes, so they are stable across machines and zstd versions.
They are the drift alarm for the whole encoding stack: a deliberate format change updates
them in the same commit as the spec, an accidental one shows up here first.

Review round

Two follow-up commits since the first push.

dfdca306 removes the stelae-boundary CI job (see unknown 5 above).

c9d05d28 closes five gaps CodeRabbit surfaced. Each has a test that was confirmed
to fail without its fix, not merely to pass with it.

Where Gap Fix
digest.rs, dir.rs read_blob grew a Vec until the zstd stream ended. read_layer compared the result against descriptor.uncompressed_size only afterwards, so a small blob with a hostile ratio could exhaust memory first. Content addressing is no defence — whoever produced the blob also chose the digest naming it. read_blob takes a required ceiling and raises the new Error::DecompressedTooLarge mid-stream; read_layer passes the size the descriptor already claims. scan_blob stays unbounded and documents why: it writes to io::sink(), so there is no memory to exhaust.
dir.rs read_layer counted records with Iterator::count(). SeqReader yields one Err on the first malformed record and then ends, so the failure was counted as a record and the error discarded — a descriptor written to match that inflated number read back Ok. Iterate and propagate.
inscription.rs check_profile checked that a layer's kind was one the profile defines, never that its media_type was the one the profile names for that kind. Compares vendor and kind against checked_layer_media_type. Deliberately not the whole string: version and codec are transport detail a profile may move within one major, and freezing them would refuse an inscription this implementation can otherwise read.
profile.rs checked_layer_media_type validated the shape of the profile's answer but never that it answered about the kind requested, so a descriptor could carry a kind and a mediaType whose embedded kind disagree. Compares the parsed kind against the requested one.
dir.rs blob_index skipped on any Error::Io, so a PermissionDenied or device error silently dropped a blob that exists — resurfacing later as a LayerNotFound pointing at the wrong problem. Skips only InvalidData/UnexpectedEof/Other, the kinds zstd raises for input it cannot decode.

New tests: digest::tests::read_blob_refuses_to_expand_past_its_ceiling (8 MiB of zeros
compressing over 1000:1, refused at a 64 KiB ceiling),
profile::tests::checked_media_type_refuses_a_name_for_another_kind,
toy_profile::a_malformed_record_is_reported_not_counted,
toy_profile::a_layer_media_type_that_is_not_the_profiles_is_refused.

The sixth CodeRabbit finding — the ^dolos- regex missing the root crate — was correct
but landed on the job that dfdca306 deleted. It is recorded under unknown 5 so it is
not rediscovered if the job comes back.

Error gains one variant, DecompressedTooLarge, and read_blob gains a parameter.
Nothing outside crates/stelae depends on the crate, so neither is a break.

Scope held

Out, per the plan, and not started: sign.rs, plan.rs, oci.rs, crates/snapshot and
the Dolos profile, the three store-trait additions (StateStore::iter_utxos, IndexStore
iteration, IndexWriter::append_prehashed), the CLI, and SnapshotConfig. Nothing here
is wired into the dolos binary; the crate is a workspace member and nothing more.

No escalations were opened — none of the plan's three escalation triggers fired.


🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added the Stelae protocol for deterministic, content-addressed snapshots.
    • Added canonical JSON and CBOR encoding with validation.
    • Added profile support, layer metadata, compression, digesting, and integrity verification.
    • Added tools for creating, reading, indexing, and validating local snapshot directories.
  • Tests

    • Added comprehensive conformance and end-to-end coverage for canonicalization, serialization, tamper detection, profiles, and layer handling.

scarmuega and others added 2 commits July 31, 2026 08:18
Introduces `crates/stelae` (package `stelae`), the walking skeleton of the
Stelae protocol specified in adrs/004_stelae_snapshots.md: the narrowest
thing that writes a stele to a directory and reads it back, for a profile
the core knows nothing about, with a byte-reproducible inscription digest.

- `frame.rs` — deterministic CBOR sequences (RFC 8742) under the RFC 8949
  §4.2.1 profile, plus the protocol-owned layer header record. Canonical
  form is enforced in both directions: `CanonicalCbor` cannot be built from
  non-conforming bytes, and the reader validates every record it yields.
  A layer's identity is the sha256 of these bytes, so a non-canonical
  encoding would publish a diffId no independent re-encoding reproduces.
- `digest.rs` — one streaming pass yielding both the diffId (sha256 over
  uncompressed bytes, identity) and the blob digest (sha256 over the zstd
  stream, transport), plus both sizes.
- `inscription.rs` — the schema exactly as the ADR fixes it, with
  `position`, `parameters` and `layers[].scope` held as opaque JSON and
  never typed. RFC 8785 canonicalization, the sha256 identity, the history
  contiguity invariant, and fail-closed parsing. Every number must be an
  integer within ±(2^53 − 1): past that a u64 renders as a rounded double
  and two conformant implementations diverge silently, so the value is
  refused rather than canonicalized.
- `profile.rs` — the `Profile` trait and the normative naming rules. The
  core asks the profile for every vendor-owned string and validates the
  answer; it composes none of them, and `vnd.stelae.*` is refused as a
  payload media type.
- `dir.rs` — a minimal on-disk stele (`inscription.json` plus
  `blobs/sha256/<hex>`), in OCI image-layout shape so that OCI transport
  lands beside these files rather than moving them.

Verified by 66 tests: the six official RFC 8785 vectors and the Appendix B
number table; canonical-CBOR rejection of indefinite lengths, non-shortest
integers, floats, tags and unsorted or duplicate map keys; identity across
zstd levels 1/9/19; history gap, duplicate and reordering rejection;
fail-closed rejection of unknown generic keys, unknown profiles and higher
profile majors; and a toy non-Cardano profile (`tests/toy_profile.rs`)
driving the whole path, including golden digests that pin the encoding.

Refs adrs/004_stelae_snapshots.md (phase 1a).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`crates/stelae` is the protocol; `crates/snapshot` will be the Dolos
profile. The boundary between them is only real if the build checks it, so
CI fails when `cargo tree -p stelae -e normal` reports any `dolos-*`
package — which keeps a later extraction to its own repository a directory
move rather than a refactor.

Refs adrs/004_stelae_snapshots.md, "Code layout".

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

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e9daaa5b-8133-4bbb-9b54-132c8a4305a0

📥 Commits

Reviewing files that changed from the base of the PR and between c9d05d2 and 0088ef1.

📒 Files selected for processing (2)
  • crates/stelae/src/digest.rs
  • crates/stelae/tests/toy_profile.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/stelae/tests/toy_profile.rs
  • crates/stelae/src/digest.rs

📝 Walkthrough

Walkthrough

The PR adds the stelae workspace crate. It defines canonical JSON and CBOR formats, profile validation, SHA-256 and zstd layer processing, content-addressed directory storage, and protocol conformance tests.

Changes

Stelae protocol

Layer / File(s) Summary
Protocol surface and profile boundaries
Cargo.toml, crates/stelae/Cargo.toml, crates/stelae/src/lib.rs, crates/stelae/src/profile.rs
Adds the crate manifest, workspace dependencies, public exports, protocol constants, error types, profile trait, media-type parsing, and naming validation.
Canonical inscriptions and layer framing
crates/stelae/src/inscription.rs, crates/stelae/src/frame.rs, crates/stelae/tests/rfc8785.rs, crates/stelae/tests/data/rfc8785/*
Adds canonical JSON inscriptions, safe-number checks, canonical CBOR validation, sequence framing, layer headers, and RFC 8785 conformance vectors.
Compressed layers and directory storage
crates/stelae/src/digest.rs, crates/stelae/src/dir.rs, crates/stelae/tests/toy_profile.rs
Adds streaming compression and digesting, bounded decompression, blob indexing, canonical inscription persistence, verified layer reads and writes, and end-to-end profile tests.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SteleDir
  participant Profile
  participant SeqWriter
  participant LayerWriter
  participant Filesystem
  SteleDir->>Profile: Validate layer kind and media type
  SteleDir->>SeqWriter: Encode header and records
  SteleDir->>LayerWriter: Compress and hash framed bytes
  LayerWriter->>Filesystem: Store blob under blob digest
  SteleDir->>Filesystem: Read and verify inscription and layer
Loading

Possibly related PRs

  • txpipe/dolos#1146: Implements the Stelae protocol and related architecture, including inscriptions, profile-aware headers, canonical encoding, and content-addressed layers.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the new Stelae protocol crate and its walking-skeleton scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 feat/stelae-core-crate

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (7)
.github/workflows/ci.yml (1)

54-54: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Align CI rust versions with rust-toolchain.toml.

rust-toolchain.toml declares channel = "1.93" and dist-workspace.toml documents rust-toolchain-version = "1.91", while the CI jobs here use dtolnay/rust-toolchain@nightly, dtolnay/rust-toolchain@1.91, and dtolnay/rust-toolchain@1.93. Use consistent values across the workflow and the toolchain config so local builds and CI run the same Rust version.
[sub-optimum]

🤖 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 @.github/workflows/ci.yml at line 54, Align every dtolnay/rust-toolchain
version in the CI workflow with the canonical Rust channel declared by
rust-toolchain.toml, including replacing the nightly and 1.91 entries with the
consistent version. Keep all CI jobs on the same toolchain version used for
local builds.
crates/stelae/src/dir.rs (2)

268-276: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Write the inscription atomically.

fs::File::create truncates inscription.json before the new bytes land. If the process stops between the truncate and write_all, the stele keeps an empty or partial inscription. SteleDir::open still succeeds, because the path exists, and the failure then surfaces as a parse error rather than an interrupted write.

Use the same stage-then-rename pattern that write_layer uses.

♻️ Proposed atomic replace
     pub fn write_inscription(&self, inscription: &Inscription) -> Result<Digest, Error> {
         let canonical = inscription.canonicalize()?;
 
-        let mut file = fs::File::create(self.root.join(INSCRIPTION_FILE))?;
-        file.write_all(&canonical)?;
-        file.sync_all()?;
+        let staging = self.root.join(format!(".{INSCRIPTION_FILE}.tmp"));
+        let mut file = fs::File::create(&staging)?;
+        file.write_all(&canonical)?;
+        file.sync_all()?;
+        drop(file);
+        fs::rename(&staging, self.root.join(INSCRIPTION_FILE))?;
 
         Ok(Digest::compute(&canonical))
     }
🤖 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/stelae/src/dir.rs` around lines 268 - 276, Update
SteleDir::write_inscription to stage the canonical bytes in a temporary file,
flush and sync that file, then atomically rename it over INSCRIPTION_FILE using
the same pattern as write_layer. Compute and return the Digest from canonical
while preserving the existing error propagation.

236-251: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Remove the staging file when the layer write fails.

Every ? between Line 236 and Line 251 returns early and leaves .staging-<pid>-<counter> on disk. The record iterator belongs to the caller, so a failure part-way through the layer is reachable, not hypothetical. blob_index scans only blobs/sha256/, so these orphans are never reported and never reclaimed.

Wrap the staged path in a guard that removes the file unless the rename succeeds.

♻️ Proposed cleanup guard
+        struct Staged<'p>(Option<&'p Path>);
+
+        impl Drop for Staged<'_> {
+            fn drop(&mut self) {
+                if let Some(path) = self.0 {
+                    let _ = fs::remove_file(path);
+                }
+            }
+        }
+
+        let mut staged = Staged(Some(&staging));
+
         let file = fs::File::create(&staging)?;
         let mut sequence = SeqWriter::new(LayerWriter::new(file, level)?);
@@
         let path = self.blob_path(&digests.blob_digest);
         fs::rename(&staging, &path)?;
+        staged.0 = None;

Separately, consider fsyncing the blobs/sha256/ directory after the rename. Without it, a crash can lose the rename even though file.sync_all() persisted the contents.

🤖 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/stelae/src/dir.rs` around lines 236 - 251, Wrap the staged path in an
RAII cleanup guard around the layer-writing flow in the method containing
SeqWriter::new, so any early error from record encoding, writing, finishing,
syncing, or renaming removes the staging file. Mark the guard as successfully
committed only after fs::rename(&staging, &path) succeeds; leave the existing
rename behavior unchanged. Do not address the separate directory fsync
suggestion.
crates/stelae/tests/data/rfc8785/output/arrays.json (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep RFC 8785 output fixtures byte-exact.

The harness reads output/{name}.json as raw bytes and compares it directly against canonical output; output/arrays.json currently has no trailing newline, so any editor/pre-commit hook that adds one breaks the test. Use a no-newline-in-fixture rule or keep editor/pre-commit formatting out of these outputs.

🤖 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/stelae/tests/data/rfc8785/output/arrays.json` at line 1, Preserve the
byte-exact RFC 8785 fixture in arrays.json by keeping its content without a
trailing newline. Configure or adjust editor/pre-commit formatting for the
output fixtures so it does not append newlines or otherwise rewrite these
canonical files.
crates/stelae/tests/toy_profile.rs (1)

241-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the second layer assertion check record decoding.

records() yields Result items. count() counts error items as well, so this assertion passes even if every record of the index layer fails to decode. Line 227 already uses the stronger form for the notes layer.

♻️ Proposed change
     assert_eq!(index_layer.header().kind, "index");
-    assert_eq!(index_layer.records().count(), NOTES.len());
+    let index_records: Vec<&[u8]> = index_layer.records().collect::<Result<_, _>>().unwrap();
+    assert_eq!(index_records.len(), NOTES.len());
🤖 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/stelae/tests/toy_profile.rs` around lines 241 - 247, Update the
second-layer assertion in the test around index_layer to consume and validate
each Result returned by records(), matching the stronger decoding assertion used
for the notes layer near line 227. Ensure the test fails on any record-decoding
error while still verifying the expected NOTES.len() record count.
crates/stelae/src/inscription.rs (1)

243-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use error variants that describe the fault.

Two checks report a fault through a variant that names a different field:

  • Line 243: a profile.version of 0 returns Error::InvalidProfileName, and the value field carries the profile name. A caller that matches on this variant reads a name problem, not a version problem.
  • Line 250: an empty compression.algo returns the generic Error::Canonicalization, which also carries JCS encoder failures from canonical_json.

Both are part of the public failure surface of validate and parse. Add dedicated variants so callers can distinguish them.

🤖 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/stelae/src/inscription.rs` around lines 243 - 254, Add dedicated
public Error variants for invalid profile.version and empty compression.algo,
then update the corresponding checks in validate/parse to return those variants
with the relevant field values. Replace InvalidProfileName for the version check
and generic Canonicalization for the compression check, preserving their
existing validation conditions and messages.
crates/stelae/tests/rfc8785.rs (1)

152-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Track serde_jcs output explicitly.

crates/stelae/tests/rfc8785.rs:157 pins serde_jcs::to_vec output for u64::MAX, while crates/stelae/tests/rfc8785.rs:77 also assumes the crate renders negative zero as 0. Keep these exact-string assertions only with a comment that they exercise the current serde_jcs formatter, or replace them with intent-focused checks that avoid relying on the exact rounded representation.

🤖 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/stelae/tests/rfc8785.rs` around lines 152 - 158, Update the tests
around beyond_the_safe_range_rendering_is_lossy and the negative-zero assertion
to either document that their exact strings intentionally track the current
serde_jcs formatter or replace those assertions with intent-focused checks that
do not depend on exact rounded output. Preserve verification of lossy rendering
for unsafe integers and normalization of negative zero.
🤖 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 @.github/workflows/ci.yml:
- Around line 64-69: Update the cargo dependency boundary check in the workflow
to match both the exact package name dolos and dolos-prefixed packages,
replacing the current ^dolos- pattern with an equivalent boundary-safe
expression such as ^dolos(-|$).

In `@crates/stelae/src/digest.rs`:
- Around line 202-247: Bound decompressed output in digest_blob and expose the
limit through read_blob and scan_blob, using a new Error variant when the
decoded byte count exceeds it. Update read_layer to pass
descriptor.uncompressed_size and blob_index to pass the directory-wide cap,
while preserving unbounded behavior only where explicitly intended and rejecting
excess output before further allocation or processing.

In `@crates/stelae/src/dir.rs`:
- Around line 423-433: Replace the SeqReader::count() call in read_layer with
iteration that propagates each record’s Result error immediately, while counting
only successfully validated records. Preserve the existing records comparison
and LayerMismatch handling after validation completes.
- Around line 342-349: Update the error handling around scan_blob in the blob
indexing flow to classify only non-layer I/O errors with kind InvalidData,
UnexpectedEof, or Other as skippable. Propagate all other I/O errors, including
permission, interruption, and device failures, while preserving the existing
digest insertion and non-I/O error propagation.

In `@crates/stelae/src/inscription.rs`:
- Around line 210-217: Update check_profile’s layer-validation loop to call
checked_layer_media_type(profile, &layer.kind) and compare its result with
layer.media_type, returning the existing profile-validation error when they
differ. Keep the existing profile.kinds() check, and ensure layers are rejected
when their media type does not match the profile-defined descriptor.

In `@crates/stelae/src/profile.rs`:
- Around line 70-82: Update checked_layer_media_type to retain the parsed
MediaType from MediaType::parse and verify its embedded kind matches the
requested kind argument. Return an appropriate validation error on mismatch,
while preserving the existing unknown-kind handling and successful return for
matching media types.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Line 54: Align every dtolnay/rust-toolchain version in the CI workflow with
the canonical Rust channel declared by rust-toolchain.toml, including replacing
the nightly and 1.91 entries with the consistent version. Keep all CI jobs on
the same toolchain version used for local builds.

In `@crates/stelae/src/dir.rs`:
- Around line 268-276: Update SteleDir::write_inscription to stage the canonical
bytes in a temporary file, flush and sync that file, then atomically rename it
over INSCRIPTION_FILE using the same pattern as write_layer. Compute and return
the Digest from canonical while preserving the existing error propagation.
- Around line 236-251: Wrap the staged path in an RAII cleanup guard around the
layer-writing flow in the method containing SeqWriter::new, so any early error
from record encoding, writing, finishing, syncing, or renaming removes the
staging file. Mark the guard as successfully committed only after
fs::rename(&staging, &path) succeeds; leave the existing rename behavior
unchanged. Do not address the separate directory fsync suggestion.

In `@crates/stelae/src/inscription.rs`:
- Around line 243-254: Add dedicated public Error variants for invalid
profile.version and empty compression.algo, then update the corresponding checks
in validate/parse to return those variants with the relevant field values.
Replace InvalidProfileName for the version check and generic Canonicalization
for the compression check, preserving their existing validation conditions and
messages.

In `@crates/stelae/tests/data/rfc8785/output/arrays.json`:
- Line 1: Preserve the byte-exact RFC 8785 fixture in arrays.json by keeping its
content without a trailing newline. Configure or adjust editor/pre-commit
formatting for the output fixtures so it does not append newlines or otherwise
rewrite these canonical files.

In `@crates/stelae/tests/rfc8785.rs`:
- Around line 152-158: Update the tests around
beyond_the_safe_range_rendering_is_lossy and the negative-zero assertion to
either document that their exact strings intentionally track the current
serde_jcs formatter or replace those assertions with intent-focused checks that
do not depend on exact rounded output. Preserve verification of lossy rendering
for unsafe integers and normalization of negative zero.

In `@crates/stelae/tests/toy_profile.rs`:
- Around line 241-247: Update the second-layer assertion in the test around
index_layer to consume and validate each Result returned by records(), matching
the stronger decoding assertion used for the notes layer near line 227. Ensure
the test fails on any record-decoding error while still verifying the expected
NOTES.len() record count.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d6f1ae81-207f-4636-97bb-8fd0a64c52e8

📥 Commits

Reviewing files that changed from the base of the PR and between c750779 and 78eb370.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (24)
  • .github/workflows/ci.yml
  • Cargo.toml
  • crates/stelae/Cargo.toml
  • crates/stelae/src/digest.rs
  • crates/stelae/src/dir.rs
  • crates/stelae/src/frame.rs
  • crates/stelae/src/inscription.rs
  • crates/stelae/src/lib.rs
  • crates/stelae/src/profile.rs
  • crates/stelae/tests/data/rfc8785/README.md
  • crates/stelae/tests/data/rfc8785/input/arrays.json
  • crates/stelae/tests/data/rfc8785/input/french.json
  • crates/stelae/tests/data/rfc8785/input/structures.json
  • crates/stelae/tests/data/rfc8785/input/unicode.json
  • crates/stelae/tests/data/rfc8785/input/values.json
  • crates/stelae/tests/data/rfc8785/input/weird.json
  • crates/stelae/tests/data/rfc8785/output/arrays.json
  • crates/stelae/tests/data/rfc8785/output/french.json
  • crates/stelae/tests/data/rfc8785/output/structures.json
  • crates/stelae/tests/data/rfc8785/output/unicode.json
  • crates/stelae/tests/data/rfc8785/output/values.json
  • crates/stelae/tests/data/rfc8785/output/weird.json
  • crates/stelae/tests/rfc8785.rs
  • crates/stelae/tests/toy_profile.rs

Comment thread .github/workflows/ci.yml Outdated
Comment thread crates/stelae/src/digest.rs
Comment thread crates/stelae/src/dir.rs
Comment thread crates/stelae/src/dir.rs Outdated
Comment thread crates/stelae/src/inscription.rs
Comment thread crates/stelae/src/profile.rs
scarmuega and others added 3 commits July 31, 2026 08:45
Reverts the `stelae-boundary` job added earlier on this branch. The
protocol/profile boundary stays a convention rather than a CI gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bound decompression, and stop trusting three things the code claimed to
check.

`read_blob` now takes a required ceiling and raises `DecompressedTooLarge`
mid-stream. Content addressing is no defence here: whoever produced a blob
also chose the digest naming it, so nothing about a well-formed, correctly
named file bounds what it expands to, and `read_layer` was buffering the
whole expansion before comparing it against the size its descriptor claims.
It now passes that size down as the ceiling — same verdict, bounded cost.
`scan_blob` stays unbounded and says why: it writes to `io::sink()`.

`read_layer` counted records with `Iterator::count()`. `SeqReader` reports a
malformed record as one `Err` item and then ends, so the failure was tallied
as a record and the error dropped with it — a descriptor written to match
that inflated number read back clean, in the one place documented to check
everything the descriptor claims.

`checked_layer_media_type` validated the shape of the profile's answer but
never that it was an answer to the question asked, so a descriptor could
carry a `kind` and a `mediaType` whose embedded kind disagree.

`check_profile` checked only that a layer's kind was one the profile
defines, never that its media type was the one the profile names for that
kind. It now compares vendor and kind — not the whole string, since version
and codec are transport detail a profile may move within one major.

Last, `blob_index` treated every `Error::Io` as "not a layer". Only the
kinds zstd raises for input it cannot decode qualify; a `PermissionDenied`
or a device error would silently drop a blob that exists and resurface as a
`LayerNotFound` pointing at the wrong problem.

Each fix has a test that fails without it. The `dolos-*` boundary notes in
`lib.rs` and `Cargo.toml` now describe it as a convention, the CI job that
asserted it having been dropped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rustfmt.toml` enables `wrap_comments`, which only the nightly rustfmt
applies; the previous commit was formatted with stable, which ignores it.
Comment wrapping only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant