Skip to content

[REFACTOR] Freeze the capsule-core public API and remove the dead surface - #426

Open
justin13888 wants to merge 9 commits into
chore/merge-v1-head-397from
chore/freeze-capsule-core-api-399
Open

[REFACTOR] Freeze the capsule-core public API and remove the dead surface#426
justin13888 wants to merge 9 commits into
chore/merge-v1-head-397from
chore/freeze-capsule-core-api-399

Conversation

@justin13888

@justin13888 justin13888 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Description

Freezes the capsule-core public API: exactly one path per public type, nothing public
without a call site, and cargo doc promoted to a CI gate on the four crates #399 declares
frozen.

Behaviour-preserving throughout. The only non-mechanical edits are execute_streaming's
options struct and the utils::hash fold, and neither had a caller outside capsule-core.

Six commits, one per slice, each green on its own before the next was written.

Summary

# Commit What
1 refactor(core)!: delete the dead public surface Removes models, constants::IGNORE_RULES, validation::idempotency, metadata::export_policy — all with zero call sites. Folds utils::hash into crypto::hash as hash_file(&Path) -> io::Result<Hash32> and drops the String twin of hash_bytes. Two KATs cover the folded entry point.
2 refactor(core)!: make the six barrel modules' submodules crate-private 59 pub modpub(crate) mod across library, import, db, crypto::keys, sidecar, domain; barrels completed (all 11 sidecar_v1 items, library::thumbnail_path, library::ReceiptStoreError); 22 stranded pub items demoted rather than exported; 32 consumer imports in 21 files moved to the barrel path.
3 refactor(core)!: give execute_streaming an options struct One entry point taking StreamingOptions replaces the two eight-positional twins. All three #[allow(clippy::too_many_arguments)] in the crate are gone.
4 docs(core): fix the intra-doc links the rustdoc gate rejects 73 rustdoc errors cleared on the public surface (commit 8 extends the gate to private items and clears 23 more): 6 crate::media links + 3 false media-feature claims, 14 private-item links, 7 ambiguous verify_asset links, 13 redundant explicit targets, 14 scope-resolution failures, and 22 doc paths that spelled a now-private submodule.
5 ci(rust): gate rustdoc on the frozen crates [tasks.doc-check-rust]cargo doc --no-deps under RUSTDOCFLAGS="-D warnings" — inside check-rust, right after clippy.
6 test(wasm): add host-runnable unit tests; widen default-members 5 host tests over capsule-wasm's pure functions (including the wrong-passphrase/wrong-fragment oracle property); capsule-sdk joins default-members.
7 fix(core): correct the S-C50 claim and keep hash_file off the wasm surface Four defects an adversarial read of this branch found, fixed before review. See Findings raised against this diff below.
8 fix(core): gate rustdoc over private items, and repair what that reveals Review round: doc-check-rust gains --document-private-items (the gate had gone blind to the 59 modules this PR privatised) and the 23 errors that reveals are repaired; plus three smaller review findings. See Review round below.

Two corrections to the issue's dead-item list

The issue named two items as dead that are not, found by grep before any deletion:

  • utils::hash is livehash_bytes at capsule-sdk/src/recovery/mod.rs:678,
    get_file_hash at capsule-core/src/import/planner.rs:237 and
    capsule-core/src/metadata/file.rs:8. It is folded, not deleted.
  • ffi::p256_hardware_dek_round_trip is live — 4 call sites in
    capsule-core-swift/Tests/CapsuleHardwareTests/SmokeTests.swift:197,217. Retained.

What the issue asked for and this does not do

Findings raised against this diff, and what was done

The branch was read adversarially before this PR was handed over. Everything in the code was
clean — no lost ?, no changed hash or encoding, no dropped tracing field, no changed error
variant, no swapped test argument, no type divergence from the privatization. Four real defects
were in what the diff wrote, and commit 7 fixes all four:

  1. S-C50 does not implement the privacy strip (the worst of the four). Slices 1's prose
    edits said "S-C50 is the slice that implements it". SLICES.md:280 records S-C50 as a
    docs slice, already done: it settled where the strip belongs and wrote no code. The
    prose now says the strip is the issuing client's obligation, nothing implements it, and no
    slice owns writing one — and that gap is filed as core: the share-link privacy strip has no implementation and no slice owns writing one #432. The same status note also called
    the export surface one that "v1 ships" two sentences after calling it unimplemented; it now
    separates what is specified from what is built.
  2. hash_file landed on the always-compiled wasm surface. utils::hash was native;
    crypto::hash deliberately is not, because it is the --no-default-features wasm32 sealing
    surface. std::fs is stubbed on wasm32-unknown-unknown, not absent, so an ungated
    hash_file compiled there and would have failed at runtime on every call. Now
    #[cfg(feature = "native")]; both callers are native anyway.
  3. StreamingOptions' doc miscounted. It said both replaced entry points carried eight
    arguments; the twin carried nine.
  4. A tautological assertion. The new capsule-wasm exhaustiveness test asserted
    !code.is_empty() on functions returning non-empty &'static str consts. It now pins both
    codes to the declared err:: set, so a newly added variant cannot answer with an ad-hoc
    literal, and checks the hand-written variant list does not repeat itself.

Two observations accepted rather than changed, both recorded here because nothing else records
them:

  • The tracing span for a streaming run is renamed. #[tracing::instrument] sat on
    execute_streaming_with_source_metadata, and the thin execute_streaming wrapper delegated to
    it, so every streaming run emitted a span named execute_streaming_with_source_metadata. It
    is now execute_streaming. The four fields are unchanged in name, expression and meaning, and
    no in-tree consumer greps the old name — but an external log query or dashboard keyed on it
    would need updating. This is the only observable behaviour change in the PR.
  • SLICES.md:1208 and :3817 still name capsule_core::metadata::export_policy. SLICES.md
    is owned by other lanes and off-limits here; check-docs-truth does not scan it, so no gate
    fails. Recorded in core: the share-link privacy strip has no implementation and no slice owns writing one #432 and in Unresolved review notes below.

Review round (commit 8)

A review of 718bc820 raised four findings; all four are closed.

M1 — the gate was guarding least exactly what this PR changed. cargo doc --no-deps
documents public items only, so once commit 2 made 59 submodules pub(crate), their //!
headers and item docs left the gate's reach. Running the same flags with
--document-private-items fails with 23 errors: 11 pre-existing ones inside those modules that
commit 4 had hidden rather than fixed (crypto/keys/tbs.rs ×8, albumstore.rs:113,
import/streaming.rs:13,14, import/planner.rs:144), plus L2 and L3 below and nine more.
The task now carries the flag and every one of the 23 is repaired — no #[allow] was added;
each link either resolves or is a code span that states why it cannot (a tpm-feature-gated
module, a private sibling helper, a cfg(windows) extern).

L2 — library/receipts.rs:12 still said BlobRole/role_str were re-exported there after
commit 2 removed that re-export. It now names where they are reached:
crypto::receipts::BlobRole, and library::BlobRole through the storage-verify barrel.

L3 — crypto/keys/keystore.rs:44 — commit 4's rewrite was a redundant explicit target and
pointed at one type where the sentence means both byte formats. It now names both, DekKeypair
and P256HybridDek, in shortcut form.

L4 — capsule-wasm's duplicate-variant guard used Vec::dedup, which collapses only
consecutive duplicates, so [A, B, A] kept its length and passed. It uses a HashSet of
discriminants now, confirmed by introducing a non-adjacent repeat and watching the test fail.

Validation

Every command run inside the worktree at d64c2cdf.

Command Outcome
mise run check-rust pass (exit 0) — chains format-check-rust, lint-check-rust, the new doc-check-rust, i18n-check, i18n-guard, openapi-check-kynos, architecture-check, license-check, translate-readme-check, build-rust, build-check-wasm, build-ffi, lint-check-ffi, gen-bindings, verify-examples
mise run test-rust pass — 1703 + 729 + 160 tests, 0 failed, 0 skipped (1698 before slice 6)
mise run check-docs-truth pass — 473 cross-links, 84 endpoint citations, 118 module paths all resolve
mise run check-md pass — 168 files, 0 issues
mise run check-docs pass — 5 test files, docs build complete
cargo check --workspace --all-targets pass, per slice
mise run lint-check-rust pass, per slice
mise run doc-check-rust (cargo doc --no-deps --document-private-items on the four frozen crates, -D warnings) pass. Public-surface errors: 73 at f433d918 → 0 (commit 4). All-item errors: 23 remaining after commit 4 → 0 (commit 8).
mise run build-check-wasm pass — the wasm32-unknown-unknown sealing surface still type-checks with hash_file added to the always-compiled crypto::hash
mise run gen-bindings passsurface check passed: S-D9 client/session + S-P1 workspace verbs present in both languages; no FFI file is touched by this PR
CI Build Capsule.apk + :core JVM smoke fail — pre-existing, not caused. Kotlin compilation in capsule-android/src/androidMain/kotlin/**: unresolved initKoin, ListViewModel, DetailViewModel, the di package, title / artistDisplayName / objectDate. This PR touches zero Kotlin, Android, or .kts files — git diff f433d918..HEAD --name-only | grep -iE '\.kts?$|capsule-android|capsule-core-kotlin' → 0. The same job fails on the base branch's own PR #418 and on master (gh run list --branch master → `Build Android app
mise run build-ffi-apple, test-swift, test-kotlin unavailable — need a macOS and an Android toolchain, absent from this host. No slice touches capsule-core/src/ffi.rs, the harnesses, stage-bindings.sh or build.gradle.kts, so their inputs are unchanged and host-runnable gen-bindings is the proxy.

Negative tests of the gate, both reverted after running:

  • reintroducing a crate::media::image::types::RawImageFormat doc link makes
    mise run doc-check-rust exit non-zero with unresolved link;
  • a broken link placed in import/streaming.rs — a module commit 2 made crate-private — fails
    the gate as it now stands and passes the public-only form it replaced. That is the blindness
    commit 8 closes, demonstrated rather than asserted.

Success criteria, measured

Criterion Baseline f433d918 Now
grep -rE "capsule_core::(library|import|db|crypto::keys|sidecar|domain)::[a-z_0-9]+::" over the consumer crates 31 hits in 20 files 0
Intra-crate doc links through the same private paths 22 0
capsule-core/src/models/, IGNORE_RULES, validation/idempotency.rs, metadata/export_policy.rs, utils/hash.rs present gone
RUSTDOCFLAGS="-D warnings" cargo doc on the frozen crates, public items 73 errors 0
the same, all items (--document-private-items) 96 errors (73 public + 23 reachable only with the flag) 0, and this is what the gate runs
#[allow(clippy::too_many_arguments)] in capsule-core 3 0
cargo test (bare) covers capsule-sdk no yes
capsule-wasm Rust tests 0 5

Pre-existing warnings, not addressed

Two cargo check --all-targets warnings predate this branch and are left alone rather than
absorbed into a refactor commit: capsule-core/src/import/group.rs:211 (find_candidate is
never used) and capsule-core/src/library/receipts.rs:93 (two unused test imports). Neither
is in the lib target, so neither fails lint-check-rust.

Risks and rollout

No persisted data, wire format, or deployed behaviour changes. Sidecar CBOR, the OpenAPI
document, and the uniffi ABI are untouched; no generated binding is committed (they are
.gitignored and regenerated by stage-bindings.sh / build.gradle.kts). Every slice is a
source-level rename, deletion, or doc edit, reversible by git revert of that commit.

Two residual risks worth naming:

  • -D warnings on rustdoc is toolchain-sensitive. A toolchain bump can add lints and
    break doc-check-rust. rust-toolchain.toml pins the toolchain, so the gate is
    reproducible; a bump must re-run it.
  • metadata::export_policy removed a documented (but unimplemented) security control.
    capsule-docs/.../metadata.md and capsule-server/src/share/mod.rs now say the
    boundary-crossing strip is unimplemented and point at S-C50, which is what was true.

Related Issues

Refs #399 — the API freeze lands; the FFI retirement and the workspace-wide rustdoc gate are
filed as #424 and #425, and the ROADMAP.md bullet belongs to #417.

Filed by this PR: #424 (core-ffi: retire the capsule_core uniffi namespace, which is harness-only today), #425 (ci: extend the rustdoc gate to the whole workspace), #432
(core: the share-link privacy strip has no implementation and no slice owns writing one) —
the last opened after removing metadata::export_policy revealed that the strip it was cited
as implementing exists nowhere.

Contributor Checklist

  • I agree to the Contributor License Agreement for this and future contributions.
  • My code follows the project's style guidelines according to CONTRIBUTING.md.
  • Tests pass
  • No sensitive info / secrets
  • Docs updated if needed

Decisions taken

# Decision record — issue #399 (lane W-CORE)

Issue 399 - core: freeze the capsule-core public API and remove the dead surface
Plan:     r1 (against f433d918, the head of PR #418)
Branch:   chore/freeze-capsule-core-api-399
Base:     chore/merge-v1-head-397 (head of PR #418); the PR targets that branch
Worktree: /var/mnt/scratch/golem/dev/Capsulsaurus/Capsule.worktrees/Capsule-chore-freeze-capsule-core-api-399
Cause:    -
Touches:  capsule-core (14 modified, 5 deleted: models/{mod,album,asset}.rs, validation/idempotency.rs, metadata/export_policy.rs, utils/hash.rs; plus the rustdoc-span doc-only edits), 21 consumer files (import-line edits in capsule-sdk, capsule-cli, capsule-server, capsule-core/tests), Cargo.toml (default-members += capsule-sdk), mise.toml ([tasks.doc-check-rust] + one check-rust entry), capsule-wasm/src/lib.rs (#[cfg(test)] module), capsule-docs/src/content/docs/design/metadata.md (:159 export_policy prose), capsule-server/src/share/mod.rs (:17 comment). No generated bindings committed (gitignored). No SLICES.md edits (no slice row is this issue's; the API freeze is recorded in ROADMAP.md by lane #417 later).
Will not: retire the ffi/ffi-bindgen features or relocate capsule-core/src/ffi.rs; fix rustdoc in capsule-sdk/capsule-server/capsule-cli/capsule-wire/xtask; touch ROADMAP.md or SLICES.md; touch capsule-core-ffi/**, stage-bindings.sh, build.gradle.kts, mise-tasks/gen-bindings, capsule-swift/**
Lane:     singleton over capsule-core's module tree; every later Rust lane (#401, #408, #410, #411, #412, #413) stacks on this branch's head
Settled:  Base branch = head of PR #418 (run decision). ADR-0005 (lane #398) states apps link exactly the capsule_core_ffi + capsule_sdk namespace pair; the third namespace stays harness-only and is filed, consistent with decision 2 below.

Decisions taken.

1. Deliverable boundary — how much of #399 closes now.
   Taken:    Ship the API freeze without the FFI retirement, and scope the rustdoc gate to the four crates the issue calls frozen (capsule-core, capsule-core-ffi, capsule-wasm, capsule-i18n). Six slices, ~41 files. The issue's ROADMAP.md bullet is satisfied by lane #417 (ROADMAP.md is being created by lane #398 concurrently and is off-limits here). Two corrections to the issue's dead-item list, found by grep: utils::hash is NOT dead (hash_bytes used at capsule-sdk/src/recovery/mod.rs:678; get_file_hash at import/planner.rs:237 and metadata/file.rs:8) → fold into crypto::hash as hash_file(Path) -> Hash32 and delete the String twin; ffi::p256_hardware_dek_round_trip is NOT dead (SmokeTests.swift:197,217) → retained.
   Rejected: Ship the whole issue - capsule-swift/Project.swift:210-215 compiles .ffi/generated/capsule_core_ffi.swift and .ffi/generated/capsule_sdk.swift into one Swift target, and capsule-sdk/src/ffi/workspace.rs:425,171,189 already declare FfiWorkspace, FfiClientBuild and FfiVerifyOutcome; relocating those names into capsule-core-ffi is a duplicate-declaration failure in a target no host here can build.
   Rejected: Ship only the deletions and privatization - leaves the rustdoc gate unbuilt while slice 2 adds "links to private item" errors to 57 doc references naming soon-private paths.
   Reverses: Re-add slices for the ffi.rs relocation and the sdk/server/cli rustdoc spans.
   Filed:    the lane files two issues: (a) "core-ffi: retire the capsule_core uniffi namespace (harness-only today)" carrying the evidence above, (b) "ci: extend the rustdoc gate to the whole workspace (16 remaining spans)".

2. What happens to the ffi/ffi-bindgen features this run.
   Taken:    Leave them and capsule-core/src/ffi.rs untouched; the follow-up issue records the namespace as harness-only.
   Rejected: Relocate ffi.rs into capsule-core-ffi behind renamed types - 13 FfiWorkspace, 16 HardwareSigner, 22 HardwareSignerError/Exception, 5 DeviceTier, 4 HardwareKeyAgreement and 4 p256HardwareDekRoundTrip references across the Swift and Kotlin harnesses would change name, none verifiable on this host, and mise-tasks/gen-bindings:63-64 asserts FfiWorkspace + HardwareSigner in core_swift/core_kt.
   Rejected: Repoint the harnesses at the capsule_sdk namespace - its FfiWorkspace has no create_with_hardware_signer, import_asset, HardwareKeyAgreement or DEK round-trip.
   Reverses: Restore the deleted feature block in capsule-core/Cargo.toml:42-46 and the cfg(feature = "ffi") arms in lib.rs:74-80 (n/a - nothing is deleted under this decision).

3. How submodules are hidden.
   Taken:    pub(crate) mod for the 59 submodules of library, import, db, crypto::keys, sidecar, domain; complete the barrels (incl. the 11 sidecar_v1 items) using #![warn(unnameable_types)] transiently; 32 consumer import edits in 21 files. The 175 intra-crate deep references compile unchanged.
   Rejected: Plain mod, matching lifecycle/mod.rs:27-38 - lifecycle's submodules are referenced only from inside lifecycle/; applying it elsewhere rewrites all 175 references for the same external effect.
   Rejected: #[doc(hidden)] pub mod as a transition - deep paths stay compilable, so success criterion 1 is never met.
   Reverses: sed 's/^pub(crate) mod /pub mod /' across the six mod.rs files.

4. Whether capsule-wasm joins default-members.
   Taken:    Add capsule-sdk only; leave capsule-wasm out and add its five host tests inside capsule-wasm/src/lib.rs (Ok-path only: JsError::new goes through wasm-bindgen's panicking off-wasm stub).
   Rejected: Add both - capsule-wasm's real artefact needs --target wasm32-unknown-unknown; on host its cdylib links a library nothing consumes; the wasm surface is already gated by build-check-wasm.
   Rejected: Add neither - a bare cargo test would still skip capsule-sdk, whose push/staged/net/recovery tests are exactly the ones this PR's import rewrites touch.
   Reverses: Append "capsule-wasm" to default-members.

Decisions taken inside the manifest during delivery

5. What happens to the 22 `pub` items privatization stranded.
   Taken:    Demote to pub(crate). `-W unreachable_pub` (already in CLIPPY_FLAGS) named them
             exactly: db::migrate::{migrate,BASELINE_VERSION,Ddl,Step,STEPS},
             db::schema::{SCHEMA_VERSION,DDL}, import::group::{is_raw,is_primary,is_video,
             is_xmp}, import::importers::takeout, library::lock::{LockRecord,try_acquire,
             release}, library::scrub::startup_scrub,
             sidecar::library_version::CURRENT_LIBRARY_VERSION,
             crypto::keys::albumstore::{ALBUM_STORE_VERSION,ALBUM_STORE_FILE},
             crypto::keys::kem::DEK_SEED_LEN. Each was grepped across capsule-sdk,
             capsule-cli, capsule-server, capsule-wasm, xtask, capsule-core-ffi and
             capsule-core/tests: zero consumers outside capsule-core.
   Rejected: Add all 22 to their barrels - the issue's own criterion is that nothing public
             has zero call sites; a freeze that gains 22 items of surface as a side effect of
             hiding a module has failed at the thing it exists to do.
   Rejected: #[allow(unreachable_pub)] - silences the detector that found them.
   Reverses: sed 's/^pub(crate) /pub /' at the 22 sites, then re-export from the barrels.

6. Two barrel gaps `unnameable_types` found, beyond the plan's sidecar_v1 list.
   Taken:    Export library::paths::thumbnail_path (capsule-core/tests/
             local_gallery_security.rs reaches it and the barrel omitted it) and
             library::receipts::ReceiptStoreError (the error type of the already-exported
             append_receipt: reachable through that signature but unnameable without it).
             Stop re-exporting BlobRole/role_str from library::receipts - both keep their one
             public home in crypto::receipts, and library::BlobRole already reaches BlobRole
             through library::storage_verify.
   Rejected: Leave ReceiptStoreError unnameable - a caller could not write the error type of
             a public function it is expected to call.
   Reverses: drop the two `pub use` entries; restore the BlobRole/role_str re-export.

7. The 22 intra-crate doc links that spelled a now-private submodule path.
   Taken:    Rewrite each to its barrel path in slice 4. They were not rustdoc errors - the
             target items are re-exported, so they resolved - but a frozen API that documents
             a second spelling of its own paths defeats criterion 1.
   Rejected: Leave them - the gate would pass while the rendered docs taught the old path.
   Reverses: revert the corresponding hunks of 325a23e2.

8. Three false `media`-feature claims, beyond the one the plan named.
   Taken:    Correct all three (capsule-core/src/import/executor.rs:7,
             lifecycle/import.rs:307, lifecycle/mod.rs:307, plus lqip/mod.rs:11 which the plan
             found). capsule-core/Cargo.toml declares native, mls, ffi, ffi-bindgen, tpm and
             inference - never `media` - so `--features media` was a hard cargo error and the
             sentences describing behaviour "behind the `media` feature" could not be true.
   Rejected: Fix only lqip/mod.rs:11 - leaves three statements the manifest contradicts in a
             commit whose subject is doc truth.
   Reverses: revert those hunks of 325a23e2.

9. The root cause of the 14 unresolved links in sharing/client_build/lqip.
   Taken:    Fully qualify the links. capsule-core/src/lib.rs carries `///` docs on
             `pub mod sharing;`, `pub mod client_build;` and `pub mod lqip;`; rustdoc merges
             those with each module's own `//!` header and resolves the merged text in the
             crate-root scope, where LINK_SECRET_LEN, Lqip, BUILD_COMMIT and the rest are not
             in scope - which is also why rustdoc reports these with no file span.
   Rejected: Demote lib.rs's `///` blocks to plain `//` comments - fixes the class at the root
             but deletes the wasm/native feature-gating rationale from the rendered docs of
             all three modules, and that prose exists nowhere else.
   Reverses: unqualify the links and demote lib.rs's three `///` blocks instead.

10. `default-members` arithmetic.
    Taken:    The plan said 9 -> 10 entries; the list actually held 8 (capsule-cli,
              capsule-cli/entity, capsule-cli/migration, capsule-core, capsule-core-ffi,
              capsule-i18n, capsule-server, capsule-wire) and now holds 9. The decision is
              unchanged - capsule-sdk in, capsule-wasm out - only the count was wrong.
    Reverses: n/a.

11. `#![warn(unnameable_types)]` was transient, as recorded.
    Taken:    Removed after slice 2. It found one real gap `-W unreachable_pub` misses
              (ReceiptStoreError), so keeping it would be a genuine improvement - but it is a
              new workspace-wide lint under `-D warnings`, outside this PR's manifest and
              outside decision 3's wording ("transiently").
    Rejected: Keep it in lib.rs - a lint addition nobody recorded, gating every later lane.
    Reverses: re-add the attribute, or add `-W unnameable_types` to CLIPPY_FLAGS.

Decisions 12-14 are the review round's three design questions. They were briefed as "7-9";
numbered 12-14 here because 7-11 above are already taken and 1-11 are kept verbatim. The
mapping is briefed-7 = 12, briefed-8 = 13, briefed-9 = 14.

12. What `doc-check-rust` documents (review round).
    Taken:    `--document-private-items`, on the same four frozen crates. `cargo doc --no-deps`
              lints public items only, so the 59 submodules this PR made `pub(crate)` fell out
              of the gate the same PR added - it would have guarded least exactly what changed.
              Measured: with the flag the tree fails with 23 errors, 11 of them pre-existing
              links inside those very modules that slice 4 hid rather than fixed. All 23
              repaired here.
    Rejected: Public-only - see above; the gate would certify a surface nobody edited while the
              edited one rotted.
    Rejected: Defer the 23 to #425 - #425 is about widening to other *crates*; leaving 23 known
              errors (two of them introduced by this PR) unheld inside the crates this PR
              declares frozen is not a widening question.
    Reverses: drop `--document-private-items` from mise.toml and file the 23 onto #425.

13. Whether `BlobRole` keeps two public paths (review round).
    Taken:    Keep both `capsule_core::library::BlobRole` (through the `storage_verify` barrel)
              and `capsule_core::crypto::receipts::BlobRole`. The freeze criterion this PR is
              measured against is "no deep-submodule spelling", and both of these are
              parent-barrel paths; the grep in Success criteria tests exactly that and returns 0.
    Rejected: Drop it from `library` - repoints every storage-verify consumer for no reduction
              in deep paths, since neither spelling is one.
    Reverses: remove `BlobRole` from `library`'s barrel and repoint consumers.

14. The streaming span's name (review round).
    Taken:    Keep `execute_streaming`. No deployed consumer can be keyed on the old name: the
              server has no binary yet (#401 lands it) and CLI traces are local. The merged
              function is the one the new name describes.
    Rejected: Preserve the old name via `#[tracing::instrument(name = ...)]` - perpetuates a
              name for a function that no longer exists, which is the opposite of what an API
              freeze is for.
    Reverses: add `name = "execute_streaming_with_source_metadata"` to the attribute.

Unresolved review notes

`capsule-core` carried public items with zero call sites anywhere in the
workspace. Each one is a promise the crate cannot retire later without a
breaking change, so they go before the API is frozen.

Removed:

- `models` (`Asset`, `Album`) — plaintext-era types, zero references.
- `constants::IGNORE_RULES` — the only hit was its own definition.
  `SIDECAR_EXTENSIONS` stays; `metadata` reads it.
- `validation::idempotency` (`IdempotencyKey`, `session_key`, `chunk_key`)
  — zero references outside the barrel re-export.
- `metadata::export_policy` (`ExportOptions`, `strip_for_export`) — zero
  code call sites. The design docs and `capsule-server`'s share module
  both claimed it implemented the boundary-crossing strip; they now say
  the strip is unimplemented and point at `S-C50`, which is the truth.
  A documented security control with no caller is worse than an absent
  one.

`utils::hash` was *not* dead and is folded rather than deleted:
`get_file_hash` becomes `crypto::hash::hash_file`, returning `Hash32`
like its neighbours instead of a hex `String`, and the `String` twin
`utils::hash::hash_bytes` gives way to `crypto::hash::hash_bytes(..)
.to_hex()`. One hash module, one return type. `utils` keeps `paths`.

Two known-answer tests cover the folded entry point: `hash_file` over a
file larger than the 64 KiB read block equals the one-shot digest, and a
missing path surfaces `NotFound` rather than a panic.

BREAKING CHANGE: `capsule_core::models`, `constants::IGNORE_RULES`,
`validation::{idempotency, IdempotencyKey}`, `metadata::export_policy`
and `utils::hash` are removed. `utils::hash::get_file_hash` is now
`crypto::hash::hash_file` and returns `Hash32`.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 2, 2026

Copy link
Copy Markdown

Deploying capsule with  Cloudflare Pages  Cloudflare Pages

Latest commit: f508bf1
Status: ✅  Deploy successful!
Preview URL: https://eb94ab59.capsule-22k.pages.dev
Branch Preview URL: https://chore-freeze-capsule-core-ap.capsule-22k.pages.dev

View logs

`library`, `import`, `db`, `crypto::keys`, `sidecar` and `domain` each
declared every submodule `pub mod` *and* re-exported its types through a
barrel, so every public type had two paths. `capsule-sdk` and
`capsule-server` used both spellings for the same type.
`lifecycle/mod.rs` is the model this follows.

59 declarations become `pub(crate) mod`. `pub(crate)` rather than plain
`mod` because the 175 intra-crate deep references then compile unchanged;
plain `mod` would rewrite all of them for the same external effect
(`pub(crate)` items are absent from rustdoc and unreachable from every
other crate).

Barrels completed, so nothing reachable became unnameable:

- `sidecar` exports all 11 `sidecar_v1` items. `CullFlag` and `GpsSource`
  are needed by `capsule-cli`; the rest are field types of `SidecarV1`.
- `library` exports `paths::thumbnail_path` (a `capsule-core` test used
  it) and `receipts::ReceiptStoreError` (the error type of the already
  re-exported `append_receipt` — reachable but unnameable without it).

The 22 items privatization stranded — declared `pub`, now unreachable
outside the crate, and with no consumer outside it — are demoted to
`pub(crate)` rather than added to a barrel: `db::migrate::{migrate,
BASELINE_VERSION, Ddl, Step, STEPS}`, `db::schema::{SCHEMA_VERSION,
DDL}`, `import::group::{is_raw, is_primary, is_video, is_xmp}`,
`import::importers::takeout` (the barrel already re-exports
`TakeoutAdapter`), `library::lock::{LockRecord, try_acquire, release}`,
`library::scrub::startup_scrub`, `sidecar::library_version::
CURRENT_LIBRARY_VERSION`, `crypto::keys::albumstore::{ALBUM_STORE_VERSION,
ALBUM_STORE_FILE}` and `crypto::keys::kem::DEK_SEED_LEN`. A frozen API
should not gain surface as a side effect of hiding a module. `-W
unreachable_pub` (already in `CLIPPY_FLAGS`) is what found them; a
transient `#![warn(unnameable_types)]` found the two barrel gaps.

`library::receipts` no longer re-exports `BlobRole`/`role_str`; they keep
their one public home in `crypto::receipts`, which the barrel already
reaches through `library::storage_verify::BlobRole`.

32 consumer imports across 21 files move to the barrel path.
`import::scanner::scan` is `import::scan_paths`, the name the barrel
already gave it.

BREAKING CHANGE: the submodules of `capsule_core::{library, import, db,
crypto::keys, sidecar, domain}` are no longer public. Every type they
held is reached through its parent barrel instead — one path per type.
`execute_streaming` and `execute_streaming_with_source_metadata` were the
same function twice: eight positionals each, both behind
`#[allow(clippy::too_many_arguments)]`, differing only in whether the
caller supplied a `SourceMetadataIndex` or the thin twin substituted an
empty one. Neither had a caller outside the barrel.

They collapse into one entry point taking `StreamingOptions` — config,
source, uploader, verifier, headroom margin, cancellation token — leaving
four arguments. `on_event` stays positional: as a field it would force a
third type parameter on the struct for no gain.

`stream_candidate` takes the same struct rather than four of its fields,
going from eight arguments to five. All three
`#[allow(clippy::too_many_arguments)]` in the crate are gone.

Behaviour is unchanged: a caller that wants no enrichment passes
`&SourceMetadataIndex::empty()`, exactly what the thin twin did for it.

BREAKING CHANGE: `capsule_core::import::execute_streaming_with_source_metadata`
is removed and `execute_streaming` takes a `StreamingOptions` in place of
its six middle arguments.
`cargo doc` was never a gate, so 73 intra-doc link errors had accumulated
across `capsule-core`, `capsule-wasm` and `capsule-i18n`. This clears
them so the next commit can gate on it. No code changes.

Six links pointed at `crate::media`, a module that left the tree, and
three doc comments described a `media` feature `capsule-core/Cargo.toml`
has never declared. Both are demoted to prose that says what is true: the
media stack is retired to `legacy-review/` and restoring it is `S-B1`, so
this build links no still encoder and no codecs at all.

Fourteen links named genuinely private items (`PINNED_CIPHERSUITE`,
`CapsuleMlsProvider`, `MlsAppPayload`, `require_canonical_runner`,
`gcj02_to_bd09`, the `db::migrate` internals, …). Those become code
spans: a private item has no URL, and pretending otherwise is what the
lint objects to.

Seven `verify_asset` links were ambiguous — `crate::crypto::verify_asset`
is both a module and a function. Every one of them meant the function;
they are disambiguated with `fn@`.

Thirteen were redundant explicit link targets, resolving to exactly what
the shortcut already resolved to.

Fourteen were unresolved because the crate root carries its own `///`
doc on `pub mod sharing;`, `pub mod client_build;` and `pub mod lqip;`.
rustdoc merges those with each module's `//!` header and resolves the
merged text in the crate-root scope, where `LINK_SECRET_LEN`, `Lqip`,
`BUILD_COMMIT` and the rest are not in scope. Fully qualifying them makes
the link independent of which scope wins.

The remaining 22 doc links spelled a path through a submodule the
previous commit made crate-private. They still resolved — the target
items are re-exported — but a frozen API documenting a second spelling of
its own paths is the thing this series exists to remove, so each moves to
its barrel path.

`capsule-wasm` and `capsule-i18n` had one each, both naming a private
module in crate-level prose.
`cargo doc` ran nowhere, so a broken intra-doc link, an ambiguous one, or
public documentation pointing at a private item merged unnoticed — 73 of
them had accumulated by the time the previous commit cleared them.

`doc-check-rust` runs `cargo doc --no-deps` under `RUSTDOCFLAGS="-D
warnings"` and sits in `check-rust` immediately after clippy, where the
same class of failure already lives. `--no-deps` so a dependency's own
doc warnings cannot fail this build.

Scoped to the four crates #399 declares frozen. `capsule-server` (12
spans), `capsule-sdk` (8) and `capsule-cli`/`capsule-wire`/`xtask` (1
each) still fail and are widened in a follow-up rather than fixed in a
commit about the gate.

Verified negatively: reintroducing a `crate::media` link fails the task.
`capsule-wasm` had zero Rust tests, and `capsule-sdk` sat outside
`default-members`, so a bare `cargo test` skipped the crate whose push,
staged, net and recovery tests this series' import rewrites touch.

`capsule-sdk` joins `default-members`. `capsule-wasm` stays out
deliberately: its real artefact needs `--target wasm32-unknown-unknown`,
on the host its `cdylib` links a library nothing consumes, and the wasm
surface is already gated by `build-check-wasm`. `cargo nextest run
--workspace` (what `test-rust` runs) covers the new tests either way.

Five host tests over the crate's pure, JS-free functions:

- `sharing_code` and `open_code` map every `SharingError` variant, with
  the security property asserted directly: on the open path a wrong
  passphrase and a wrong fragment secret must produce the *same* code, or
  the viewer becomes an oracle for which half of a link was wrong.
- an exhaustive match that fails the build if a variant is added upstream
  without a boundary code, rather than letting it reach the viewer as an
  unmapped string.
- `hex_array` over a canonical 64-char fragment, whitespace included.
- `decode_wrapped` over a base64 round trip of a canonical `WrappedScope`.

Ok paths only, and the module comment says why: every `Err` arm builds a
`JsError`, whose `__wbindgen_error_new` extern is a panicking placeholder
off `wasm32`. Error-path behaviour at the boundary stays covered by
`capsule-web`'s bun KATs.
…rface

Four defects an adversarial read of this branch turned up. No behaviour
change beyond the `hash_file` gate, which removes a call that could only
ever fail.

**`S-C50` does not implement the privacy strip.** `SLICES.md:280` records
it as a **docs** slice, already `done`: it settled *where* the
boundary-crossing strip belongs (the issuing client) and left the
mandatory, no-opt-out rule binding that client. It writes no code.
`capsule-docs/.../metadata.md` and `capsule-server/src/share/mod.rs` both
pointed future implementation work at it. They now say what is true: the
strip is the issuing client's obligation, nothing implements it, and no
slice owns writing the real one. The same status note also claimed the
export surface "v1 ships" while calling it unimplemented two sentences
earlier; it now separates what is specified from what is built — the
server's containment half ships, the client-side strip does not.

**`crypto::hash::hash_file` is now `native`-gated.** It replaced
`utils::hash::get_file_hash`, and `utils` is `native`; `crypto::hash` is
deliberately not, because it is part of the `--no-default-features`
wasm32 sealing surface. An ungated `hash_file` therefore compiled for
`wasm32-unknown-unknown` — `std::fs` is stubbed there, not absent — and
would have failed at runtime on every call. Both callers are `native`
anyway, so the gate costs nothing and keeps the browser build's public
surface free of a filesystem API.

**Two doc-accuracy fixes.** `StreamingOptions` said both replaced entry
points carried eight arguments; the twin carried nine. The new
`capsule-wasm` exhaustiveness test asserted `!code.is_empty()` on
functions that return non-empty `&'static str` consts — an assertion that
cannot fail. It now pins both codes to the declared `err::` set, so a
newly added variant cannot answer with an ad-hoc literal the viewer has
no catalog key for, and checks that the hand-written variant list does not
repeat itself.
`cargo doc --no-deps` documents public items only. The commit that made
59 submodules of `library`, `import`, `db`, `crypto::keys`, `sidecar` and
`domain` crate-private therefore walked them out of the gate's reach: the
gate this series added went blind to exactly the modules the same series
touched, and 11 pre-existing broken links inside them were hidden rather
than fixed.

`doc-check-rust` now passes `--document-private-items`, so every item in
the four frozen crates is linted. That surfaces 23 errors, all repaired
here — 22 in `capsule-core`, 1 in `capsule-i18n`; `capsule-core-ffi` and
`capsule-wasm` were already clean.

Fourteen unresolved links, each for a stated reason:

- `HardwareSigner` is implemented in `tbs`'s `#[cfg(windows)] backend`
  submodule and never imported at module scope, so the bare name resolved
  nowhere — four links now go through `super::HardwareSigner`.
- `keys::tpm` is `#[cfg(feature = "tpm")]` and has no doc page in a
  default build; three links to it become prose that says so.
- `p256::parse_p256_public` is private to a sibling module and so is not
  nameable from `tbs` at all — code span.
- `Tbsi_Is_Tpm_Present` is a `windows-sys` extern behind `cfg(windows)`;
  it gets a Win32 URL reference, matching `Tbsip_Submit_Command` in the
  same header.
- `ingest_current_epoch` is a method, not a free item in its module;
  `ProtocolMessage`, `encrypt_asset_rekey` and `ReferenceAuthority` were
  not in their item's scope. All four now carry a resolvable path.

Seven redundant explicit link targets drop to the shortcut form, and two
ambiguous links (`crypto::verify_asset`, `crate::negotiate` — each both a
module and a function) are disambiguated with `fn@`; both meant the
function.

No `#[allow]` was added: every remaining link either resolves or was
demoted to a code span that states why it cannot.

Verified negatively: a broken link introduced in `import/streaming.rs` —
a now-crate-private module — fails the new gate and **passes** the old
public-only one.

Two further review findings:

- `library::receipts`' module doc still said `BlobRole`/`role_str` were
  re-exported there after that re-export was removed. It now says where
  they are actually reached: `crypto::receipts::BlobRole`, and
  `library::BlobRole` through the storage-verify barrel.
- `keystore`'s `DeviceDek` doc said the two byte formats are
  length-disjoint but linked one type. It now names both, `DekKeypair`
  and `P256HybridDek`, in shortcut form.

Finally, `capsule-wasm`'s duplicate-variant guard used `Vec::dedup`,
which collapses only *consecutive* duplicates — `[A, B, A]` kept its
length and passed. It uses a `HashSet` of discriminants now; confirmed by
introducing a non-adjacent repeat and watching the test fail.
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