Define Keep's authenticated reconstruction contract - #77
Conversation
Summary by CodeRabbit
WalkthroughThe PR adds a normative authenticated reconstruction contract for Keep. It documents proof scopes, exact-byte output, receipts, refusal and failure behavior, ChangesAuthenticated reconstruction
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/invariants/authenticated-reconstruction/README.md`:
- Line 203: Update the invariant wording in the authenticated reconstruction
documentation to explicitly guarantee that required supporting evidence is not
garbage-collected, deleted, or otherwise invalidated during the read, replacing
the ambiguous “collected” terminology.
- Around line 10-16: Update the contract section around the “Keep either”
statement to distinguish three outcomes: authenticated success, an evidenced
content-related refusal, and operational failure. Align the wording with
ReferenceStore::reconstruct so output or other I/O errors are classified as
operational failures, do not count as precise refusals, and cannot support a
content conclusion.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: d886dc72-4668-44aa-9fda-339cb22ceea2
📒 Files selected for processing (2)
README.mddocs/invariants/authenticated-reconstruction/README.md
📜 Review details
🔇 Additional comments (2)
README.md (1)
8-10: LGTM!docs/invariants/authenticated-reconstruction/README.md (1)
1-9: LGTM!Also applies to: 17-202, 204-240
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0469e05a5
ℹ️ 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".
| **Status:** Normative for every Keep operation that claims authenticated | ||
| reconstruction. The public non-durable `ReferenceStore` implements the | ||
| complete-object and exact-range forms. A consolidated durable logical-read | ||
| surface is not yet implemented. |
There was a problem hiding this comment.
Add the required reconstruction decision record
This normative contract makes governed decisions about proof scope, receipt durability, recovery, and public API boundaries, but a repo-wide search shows that the new concept directory contains only this README.md and the commit adds no ADR. Add the required colocated rationale.md documenting the decision and rejected alternatives; otherwise the rationale exists only in the commit message and is unavailable to future repository readers.
AGENTS.md reference: AGENTS.md:L150-L156
Useful? React with 👍 / 👎.
| Evidence anchors: | ||
|
|
||
| - [`ReferenceStore` architecture](../../architecture/reference-store/README.md) |
There was a problem hiding this comment.
Add a requirement ledger for the normative contract
Because this page declares a governed invariant, docs/Documentation Standards.md §3.2 requires its contract ledger to identify stable requirement IDs, exact laws, oracles, evidence types, status, and concrete tests. A repo-wide search finds no ledger for this new invariant, while this evidence section supplies only unversioned file links, so future changes cannot audit which tests establish each normative reconstruction claim; add a colocated requirements ledger mapping these claims to their evidence.
Useful? React with 👍 / 👎.
| It does not prove that Keep authenticated: | ||
|
|
||
| - unrequested chunks; | ||
| - the complete logical blob; | ||
| - every storage-profile boundary. |
There was a problem hiding this comment.
State that range reads verify no profile boundaries
For every exact-range entry point, range_read_execution::verify_selected verifies only each selected ChunkId and never invokes ProfileVerifier; the existing RangeReadReceipt rustdoc likewise says that storage-profile boundaries were not verified. Saying merely that the receipt does not prove “every” boundary can be read as proving the overlapping boundaries, which would let consumers promote the receipt beyond its implemented proof scope. Replace “every” with “any” so the normative contract states the actual verification limit.
AGENTS.md reference: AGENTS.md:L150-L154
Useful? React with 👍 / 👎.
| The generic reconstruction API accepts an ordinary caller-owned `Write` sink. | ||
| An ordinary sink is not transactional. It may fail after accepting a prefix, | ||
| and an emission-time storage failure may occur after a prefix has been | ||
| written. |
There was a problem hiding this comment.
Add coverage for failure after a writer accepts a prefix
This section makes the retained-but-untrusted prefix a normative failure mode, but the reconstruction and range failure tests use FailingWriter, which errors before accepting any byte; the short-write fixture only exercises eventual success. Consequently no executable evidence checks the newly documented case where a writer accepts a prefix and then errors, including the reported bytes_written coordinate and absence of a success receipt. Add a deterministic writer fixture that accepts a nonempty prefix before failing for both whole-object and range operations.
AGENTS.md reference: AGENTS.md:L103-L105
Useful? React with 👍 / 👎.
| The current durable segment, catalog, publication, retention, and recovery | ||
| surfaces do not yet form this consolidated high-level `BlobId`-to-writer | ||
| contract. Their existence must not be described as an implemented durable | ||
| logical reconstruction API. |
There was a problem hiding this comment.
Do not describe retention as a current durable surface
A repo-wide search of src finds no retention API or implementation, while the root README explicitly says retention and garbage collection remain planned and ADR-0009 requires documentation to keep describing them that way. Grouping retention among the “current durable ... surfaces” therefore overstates the implemented capabilities and can mislead integrators assessing whether evidence is protected from collection; remove retention from this current-surface list or label it explicitly as planned.
Useful? React with 👍 / 👎.
| authenticated bytes + receipt | ||
| or typed refusal |
There was a problem hiding this comment.
Include operational failure in the top-level outcome model
When a caller-owned writer fails after accepting a prefix, or when evidence is unreadable, the later failure taxonomy correctly calls this an operational failure from which no content conclusion follows; however, this normative outcome diagram offers only authenticated success or a typed refusal. Consumers following the summary can therefore classify every Err as an evidenced refusal, which is exactly the distinction the later section is intended to prohibit. Add operational failure as an explicit third outcome in the contract and diagram.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
| An exact-range operation has a deliberately narrower proof scope. Its receipt | ||
| proves that the requested bytes came from completely authenticated overlapping | ||
| chunks under the admitted layout. |
There was a problem hiding this comment.
Require a committed target-layout binding for range receipts
For a caller-supplied range layout, structural AdmittedLayout validation proves offsets and lengths but not that its target BlobId names the listed chunks; docs/architecture/reference-store/rationale.md explicitly demonstrates this forged same-length-target case. The current read_admitted_layout_range avoids it by resolving the canonical LayoutId to a committed layout, but this normative range rule requires only an “admitted layout,” allowing a future conforming implementation to issue a target-bearing receipt for an unauthenticated target association. Require the layout-to-target binding to come from the admitted store view before returning a range receipt.
AGENTS.md reference: AGENTS.md:L5-L9
Useful? React with 👍 / 👎.
Problem
Keep's core law promises exact named bytes or refusal, but the repository did
not have one governed invariant stating whole-object versus range proof scope,
ordinary
Writefailure visibility, receipt durability, or the distinctionbetween evidenced content refusals and operational failures.
Invariant affected
For a requested content identity, Keep returns authenticated exact bytes and a
receipt, returns a precise evidenced content refusal, or returns an operational
failure that makes no content claim. A failed ordinary writer may retain an
untrusted prefix; only a complete success receipt authenticates the emitted
sequence.
Approach
selected store view and state that ranges verify no profile boundaries.
prefix and then fail, verifying exact error coordinates and no receipt.
Alternatives rejected
Writeuntouched.complete durable logical-read API.
Failure modes
The contract covers untrusted output prefixes, incomplete views, unreadable or
inconsistent evidence, unsupported durable claims, exact-layout unavailability,
and the distinction between content evidence and operational failure.
Tests
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-targets --all-featurescargo test --test streaming_cascargo test --test range_read_failuresPATH="scripts/documentation-tools/node_modules/.bin:$PATH" cargo xtask documentation-integrity-checkgit diff --checkBenchmark impact
No production path changes. The new deterministic writer fixtures affect tests
only; the all-target validation benchmark binaries completed successfully.
Format and API compatibility
No durable format, canonical bytes, identity law, or public Rust API changes.
Recovery implications
The contract requires future durable receipts to bind one immutable view and
protect the supporting evidence closure. Retention remains planned; the page
does not promote current lower-level durable machinery into a logical read API.
Security implications
Operational failure, partial proof, stale physical existence, or an uncommitted
caller layout cannot be promoted into a stronger content claim. Authenticated
bytes still do not prove authorship, authorization, provenance, or meaning.
Cross-repository consumer
Echo's experimental boundary is flyingrobots/echo#721; live implementation and
cutover work is flyingrobots/echo#722. Echo vocabulary remains outside Keep
core.