Skip to content

feat(keep): add an optional ReferenceStore content adapter - #770

Merged
flyingrobots merged 2 commits into
mainfrom
feat/keep-reference-adapter
Oct 7, 2026
Merged

feat(keep): add an optional ReferenceStore content adapter#770
flyingrobots merged 2 commits into
mainfrom
feat/keep-reference-adapter

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Adds a disabled-by-default Keep ReferenceStore backend through Echo's complete-object physical-content port. Publication verifies both independent identities on sealed bytes and validates the Keep commit receipt; reconstruction checks private bindings and the Keep receipt before independently sealing Echo bytes and atomically promoting the destination.

Backend errors retain their concrete upstream causes privately behind adapter-owned sanitized formatting and source/downcast boundaries. Echo ingress and destination I/O behavior stays compatible.

The isolated Rust 1.96 package preserves the root Rust 1.90 CAS graph and all existing consumers. Explicit physical-payload, object-size, object-count and layout-entry limits bound supported work. The adapter and bindings are volatile; receipts establish no restart durability, authenticated absence, retention or durable generation. There is no implicit fallback or production cutover.

Closes #761. Prerequisites #759 and #760 are merged in #768 and #769.

Validation: guarded Docker, six default identity tests, 13 feature-enabled tests, 36 existing CAS tests, strict all-feature/all-target Clippy, formatting, dependency graph, inherited policy, audit-lockfile mocks, MSRV and SPDX checks passed. The same backend-neutral conformance source is used by Keep, MemoryTier and DiskTier. Actual upstream missing-chunk and malformed-layout refusals use public Keep APIs through the same quarantine helper; they do not inject private upstream chunk corruption. No physical power-loss or study timing evidence is claimed.

Documentation accuracy gate: the canonical physical-content boundary, experimental README/package posture and changelog now describe the implemented optional adapter and its volatile limits. Relevant README/GUIDE/docs entrances were checked; no production-adoption claim was introduced.

Current-head Code Lawyer and independent agy review plus hosted checks are required before merge. Local retained evidence: k03-private-error-green source manifest, launch contract, raw log and result. Earlier attempts recorded a formatter boundary failure and shell syntax failure; neither is counted as a RED behavior witness.

Late P2 finding: k03-private-error-red selected one lost-store test and failed on the parent downcast exposure. The current regression and all surrounding gates pass after a372574. The first agy approval is superseded; a fresh current-head review is required.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: flyingrobots/echo/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: cc5c009d-f084-4a3e-af08-695802e69072
📥 Commits

Reviewing files that changed from the base of the PR and between bb20c57 and a372574.

📒 Files selected for processing (8)
  • .github/workflows/echo-keep-experimental.yml
  • CHANGELOG.md
  • docs/architecture/echo-keep-physical-content-boundary.md
  • experiments/echo-keep/Cargo.toml
  • experiments/echo-keep/README.md
  • experiments/echo-keep/src/lib.rs
  • experiments/echo-keep/src/reference_adapter.rs
  • experiments/echo-keep/src/reference_adapter/tests.rs
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T20:32:04.725441Z a372574 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Code Lawyer inspection at de67e83b88b99af5f963bd9ce8276efdc27b903a found no actionable source defect.

Path Verified behavior Evidence
Constructor and publication Explicit payload/object/count/layout limits; same-source identity binding; expected Keep staging; commit receipt verified before private binding publication; repeated publication at count cap works reference_adapter.rs; limit and shared conformance witnesses
Borrowed reconstruction Target binding preflight; immutable Keep reconstruction; Keep blob/layout/length checked; shared quarantine independently verifies Echo hash and length; destination promotion preserves old visible output on failure Shared contract and private-coordinate substitution tests
Unavailable and restart posture Unbound requests yield unavailable capability; lost Keep state yields an operational error with its original cause; recreation loses volatile bindings; no durable receipt or fallback Lost-store and recreated-adapter test
Default and feature graphs Feature defaults empty, standalone workspace; root Rust 1.90 CAS remains unchanged; hosted feature suite and all-feature lint enabled Manifest, workflow and dependency-tree witness

Guarded Docker evidence at k03-final-gate-v3: six default identity tests, twelve feature-enabled tests, 36 existing CAS tests, strict Clippy and formatting, MSRV, SPDX and dependency-policy/audit mocks passed. Current candidate bytes independently match every file in that manifest. No merge commits occur between target main and this feature head. Existing mainline identity, port and study-recovery invariants remain intact.

The imported shared test source retains its owning Rust 1.90 formatting; only recursive formatting of that external module is skipped in the Rust 1.96 experiment. An earlier formatting failure and shell-runner syntax failure are explicitly excluded from defect RED evidence.

Coverage limits: missing chunks and malformed records use real public Keep APIs with the same quarantine helper, rather than injecting corruption into inaccessible upstream tables. Power loss, production cutover, persisted bindings, Windows runtime and original study measurements are not claimed. The canonical boundary, README and changelog describe that exact posture.

Merge eligibility remains pending current-head independent agy approval and hosted checks.

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 7, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de67e83b88

ℹ️ 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".

Comment thread experiments/echo-keep/src/reference_adapter.rs Outdated
@flyingrobots flyingrobots mentioned this pull request Oct 7, 2026
Merged
@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Independent Adversarial Review: PR #770 (flyingrobots/echo)

1. Findings (P0–P5) & Review Coverage Limitations

Demonstrated Source Defects: None (0 actionable findings)

Exhaustive line-by-line inspection of experiments/echo-keep/src/reference_adapter.rs, experiments/echo-keep/src/reference_adapter/tests.rs, experiments/echo-keep/src/lib.rs, and crates/echo-cas/src/physical_content.rs identified no actionable defects (P0–P5) at commit de67e83b88b99af5f963bd9ce8276efdc27b903a.

  • Memory safety: #![forbid(unsafe_code)] remains strictly enforced; all operations are safe Rust edition 2024.
  • Arithmetic: All length calculations use checked operations (checked_add, saturating_add, checked try_from).
  • Error handling: No unwrap(), expect(), panic!(), or unchecked indexing exists in production paths. Errors retain their typed causes (ReconstructionError, IngestionError, PublishError) wrapped cleanly in ContentError::Backend or mapped to ContentError::ResourceLimit.
  • Coordinate leaks: Keep physical coordinates remain completely redacted from public APIs and Debug implementations.
  • Refusal and quarantine: Overlong, mismatched, or corrupt bytes are rejected prior to destination promotion, leaving destination visible state untouched.

Explicit Review Coverage Limitations & Missing Evidence (Not Defects)

The following boundaries are explicitly outside the claims of PR #770 and are recorded as verified coverage limits:

  1. Volatile / Non-Durable In-Memory Storage Only:
  2. Public API Quarantine Testing vs. Inaccessible Upstream Tables:
  3. Absence of Host Platform Evidence (Windows / Power Loss / Study Latency):
    • Limitation: No evidence exists for native Windows execution, catastrophic hardware power-loss survival, or production latency benchmarks.
    • Evidence: All retained execution evidence (k03-final-gate-v3.log) originates from a hermetic Linux Docker container (echo-read-runtime:red).

2. Code Path Analysis & Production Parity Mapping

PR #770 delivers an experimental, default-off complete-object physical content adapter implementing echo_cas::physical_content::PhysicalContentBackend. Production Echo CAS uses MemoryTier and DiskTier.

Operation / Visible Behavior Experimental Keep Path Production Echo CAS Parallel Path Parity & Invariant Verification
Backend Construction & Caps reference_adapter.rs:43-58: KeepReferenceAdapter::new(physical_bytes, object_bytes, objects, layout_entries) memory.rs:32-34: MemoryTier::new() / DiskTier::open(path) Production memory backend has unbounded advisory limits by default; Keep adapter requires explicit limits for physical payload, per-object size, object count, and layout entries. Validates LayoutEntryLimit::new before allocating.
Expected Ingestion Staging physical_content.rs:171-184: StagedContent::read_expected physical_content.rs:171-184: StagedContent::read_expected Shared type: Preflights expected_length > byte_limit before reading source; reads into PrivateContentBuffer; seals BLAKE3 hash and length.
Content Publication reference_adapter.rs:102-152: KeepReferenceAdapter::publish_content physical_content.rs:336-342 (MemoryTier) & 370-375 (DiskTier) Preflights object-size and object-count caps. Computes same-source IdentityBinding from sealed bytes; verifies Echo identity and length. Checks duplicate binding equality. Stages and commits to Keep ReferenceStore. Validates commit receipt against target and layout ID before registering private binding.
Immutable View Borrow reference_adapter.rs:99-101: KeepReferenceView<'a>(&'a KeepReferenceAdapter) physical_content.rs:333-335 (MemoryContentView) & 367-369 (DiskContentView) Borrows underlying store immutably (&self); lifetime bounded to store borrow.
Reconstruction & Quarantine reference_adapter.rs:64-95: KeepReferenceView::reconstruct physical_content.rs:316-329 (MemoryContentView) & 347-363 (DiskContentView) All backends delegate to reconstruct_quarantined. Checks destination atomic promotion support and destination byte limit. Preflights binding. Calls Keep store reconstruct. Verifies Keep receipt target, layout ID, and byte count. Shared quarantine re-hashes BLAKE3 before destination.promote.
Missing Content Handling reference_adapter.rs:70-74: Unbound target returns CapabilityUnavailable physical_content.rs:322-325 (MemoryTier) & 355-357 (DiskTier) Parity verified: Missing/unbound items refuse with ContentError::CapabilityUnavailable. No authenticated absence claim is manufactured. Bound items whose Keep store lost state return operational ContentError::Backend(ReconstructionError::BlobMissing).
Receipt Posture physical_content.rs:135-161: ContentReceipt physical_content.rs:135-161: ContentReceipt Exact parity: All receipts return establishes_durability() == false and establishes_complete_view() == false.

3. Merge & Git History Audit

Commit Graph Verification

  • Merge Base: bb20c57345374f476fa938789294a0890341eadd (git merge-base bb20c573 de67e83b confirmed).
  • Commit Count: Exactly 1 commit between base and head (de67e83b88b99af5f963bd9ce8276efdc27b903a).
  • Merge Commits in PR: 0. The branch is a clean linear fast-forward on top of mainline bb20c573.

Audit of Merged Ancestors & Integrated Invariants

  1. PR feat(keep): verify the experimental Echo content identity bridge #768 (bb20c57345374f476fa938789294a0890341eadd, K01 - Issue Prove the Echo and Keep content identity bridge #759):
    • Parents: 2056c95f (main) and 64b3a42d (feat/keep-identity-bridge).
    • Introduced Invariants: Bounded dual-identity stream hashing (IdentityBinding::from_source), private Keep coordinate redaction from Debug, isolated package experiments/echo-keep pinned to Keep 3165890e9291cfb5fe10e81a9d7cd151f3e59464, and dependency boundary enforcement.
    • PR feat(keep): add an optional ReferenceStore content adapter #770 Integration: Fully honored. IdentityBinding is reused unmodified; Keep coordinates remain strictly private; Keep pin is unchanged in Cargo.toml.
  2. PR feat(cas): verify complete objects before atomic output promotion #769 (2056c95fb891125cbfcc8405e438542e020b7f4c, K02 - Issue Add the Echo physical-content port and existing CAS adapters #760):
    • Parents: Preceding main commit and 7802d898 (feat/physical-content-port).
    • Introduced Invariants: Complete-object physical-content port (echo_cas::physical_content), StagedContent, reconstruct_quarantined, atomic destination promotion, and shared backend-neutral conformance suite (tests/common/physical_content.rs).
    • PR feat(keep): add an optional ReferenceStore content adapter #770 Integration: Fully honored. KeepReferenceAdapter implements PhysicalContentBackend; KeepReferenceView implements PhysicalContentView; common::conformance is run against KeepReferenceAdapter in reference_adapter/tests.rs:30-34.

4. Claim and Numeric Verification

Independent Manifest Verification

  • Retained Manifest: [retained evidence: k03-final-gate-v3.manifest.json]
  • Base Head Recorded in Manifest: bb20c57345374f476fa938789294a0890341eadd
  • Total Files Recorded in Manifest: 979
  • Independent Hash Comparison against Candidate Tree:
    • Computed SHA-256 for all 979 files against the reviewed checkout.
    • Mismatches: 0
    • Missing files: 0
    • Result: 100% exact match across all 979 files.

Test Count Claims vs. Raw Logs

  • 6 Default Tests (Package: echo-keep-experimental, default features):
    • Verified in [k03-final-gate-v3.log:5-13]. All 6 passed:
      1. tests::complete_binding_budget_preflight_does_not_read_source
      2. tests::binding_debug_keeps_backend_coordinates_private
      3. tests::identity_reader_stops_at_one_overlength_probe
      4. tests::interrupted_short_source_retries_but_failed_source_returns_no_binding
      5. tests::substitution_and_length_mismatch_refuse
      6. tests::pinned_vectors_bind_distinct_laws_and_reconstructed_bytes
  • 12 Feature-Enabled Tests (Package: echo-keep-experimental, --features reference-adapter):
    • Verified in [k03-final-gate-v3.log:24-38]. Exactly 12 passed:
      • 6 identity tests above, plus:
        1. reference_adapter::tests::interrupted_ingress_retries_before_explicit_publication
        1. reference_adapter::tests::losing_the_store_or_recreating_the_adapter_refuses_without_a_receipt
        1. reference_adapter::tests::private_coordinate_substitutions_never_promote_output
        1. reference_adapter::tests::real_keep_missing_chunks_and_malformed_layouts_stay_quarantined
        1. reference_adapter::tests::publication_limits_leave_existing_objects_readable
        1. reference_adapter::tests::keep_and_existing_memory_share_the_exact_contract
  • 36 Root CAS Tests (Package: echo-cas under Rust 1.90.0):
    • Verified in [k03-final-gate-v3.log:50-106]:
      • src/lib.rs (memory tier): 17 passed
      • tests/disk_tier.rs: 4 passed
      • tests/physical_content.rs: 5 passed
      • tests/semantic_retention.rs: 10 passed
      • Total: $17 + 4 + 5 + 10 = \mathbf{36}$ passed.
  • 25 Package MSRVs Matching Policy:

History of Gate Runs (v1, v2, v3)

  • k03-final-gate.log (v1 failure): Rust 1.96 cargo fmt --check failed on crates/echo-cas/tests/common/physical_content.rs because the 1.96 formatter formatted the imported 1.90 shared source differently. Fixed by annotating mod common; with #[rustfmt::skip] in experiments/echo-keep/src/reference_adapter/tests.rs:9. Confirmed: This was a formatting runner boundary, not a defect RED claim.
  • k03-final-gate-v2.log (v2 failure): Tests completed green, but runner failed at line 119 (sh: 17: Syntax error: redirection unexpected) due to a Bash here-string executed under sh.
  • k03-final-gate-v3.log (v3 pass): Runner syntax corrected; all steps exited with code 0.

Resource Usage Accounting

Verified against [k03-final-gate-v3.result.json] and [k03-final-gate-v3.launch.json]:

  • Build Cache: 13,366,132,955 bytes (budget: 21,474,836,480 bytes / 20 GiB) — Passed (62.2% of budget).
  • Data Storage: 4,267,811,035 bytes (budget: 4,294,967,296 bytes / 4 GiB) — Passed (99.4% of budget).
  • Logs: 19,172,996 bytes (budget: 134,217,728 bytes / 128 MiB) — Passed (14.3% of budget).
  • Host Free Space: 724,038,213,632 bytes (> 50 GiB floor) — Passed.
  • VM Free Space: 688,110,047,232 bytes (> 50 GiB floor) — Passed.

5. Constants, Buffers, and Limits Trace

  1. Keep Revision Pin:
  2. Buffer and Staging Limits:
  3. CI Execution Timeout:

6. State Machines, Error Propagation, and Refusal Invariants

State Transitions Traced

  1. Interrupted Ingress Retries:
  2. Atomic Destination Promotion & Quarantine:
    • reconstruct_quarantined writes candidate bytes into PrivateContentBuffer.
    • If an overlength write occurs, PrivateContentBuffer::write flags identity_failure = true, returning ContentError::Mismatch.
    • If allocation limit is reached, it flags resource_failure = true, returning ContentError::ResourceLimit.
    • On error, destination.promote is bypassed; the destination visible buffer remains identical to its prior state.
  3. Repeated Publication Idempotence:
    • When an object is re-published at the object-count limit, reference_adapter.rs:110 checks !self.bindings.contains_key(&target.hash) && self.bindings.len() >= self.object_count. Existing keys bypass the count limit. Lines 120–126 confirm identity equality before staging. Verified in reference_adapter/tests.rs:59-60.
  4. Refusal without Silent Fallback:
    • Missing Keep store state returns operational ContentError::Backend(ReconstructionError::BlobMissing).
    • Unbound targets return ContentError::CapabilityUnavailable.
    • No fallback to MemoryTier or DiskTier occurs under any condition.

7. Repository Standards & Pre-PR Gate Conformance

8. Execution State of Validation Checks

In accordance with review protocols requiring transparent accounting of validation execution:

  • Inspected Only:
    • All source files, Git commit tree, and PR metadata.
    • Manifest [k03-final-gate-v3.manifest.json] (all 979 file hashes compared via read-only local hash verification).
    • Retained execution logs: k03-final-gate-v3.log, k03-final-gate-v2.log, k03-final-gate.log.
    • Retained launch and result files: k03-final-gate-v3.launch.json, k03-final-gate-v3.result.json.
    • Live GitHub PR state and issue dependencies via gh CLI.
    • Pinned Keep commit in /Users/j/git/keep via read-only git -C /Users/j/git/keep show.
  • Executed Directly by Reviewer:
    • None. No compiler builds, test suites, or Docker commands were run during this review (strict compliance with read-only review constraints).
  • Skipped / Unavailable:
    • Live process-death recovery tests, host Windows tests, and hardware power-loss verification (unavailable; explicitly excluded from PR claims).

9. Mandatory Verification Checklist

APPROVE

@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Current-head merge gate at de67e83b88b99af5f963bd9ce8276efdc27b903a: Code Lawyer found no actionable defect; independent agy returned APPROVE with the complete path/merge/claim/number checklist; all hosted checks passed and no review threads are unresolved.

Two review-language qualifications do not change code: private binding fields and IdentityBinding::Debug hide Keep coordinates, but opaque backend causes can still expose diagnostic identities through their error formatting. This is not a public semantic identity conversion or persisted ABI. Agy performed read-only inspections and hash/API commands, while inspecting the author's test logs; it did not execute builds or tests itself. Its broad “executed none” wording refers to compiler/test validation. No review wording establishes absence of all defects, crash durability or original study timings.

All acceptance checks for #761 pass under the experimental, volatile ReferenceStore posture. Required protections remain in force; user authorization already covers the ordinary merge.

@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Confirmed late review finding (P2): reference_adapter.rs boxes concrete Keep errors into the public physical-content error. The missing-store test demonstrates successful downstream downcast to ReconstructionError, exposing the binding's private Keep coordinates. A generic boxed type alone is insufficient isolation. My earlier diagnostic qualification and the first agy approval did not close this gap; that approval is superseded as a merge gate.

Acceptance: a real lost-store reconstruction must return an operational error while preserving prior visible bytes, with no Keep coordinate accessible through Display, Debug, downcast or the public source chain. Apply one shared adapter-owned opaque failure wrapper to identity, staging, publication and reconstruction. Retain original causes privately and leave Echo ingress/destination I/O errors unchanged. Obtain fresh current-head review and validation before merge. @codex

@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Fixed P2 private coordinate/type exposure in a3725747: every adapter identity, staging, publication and reconstruction failure now uses one adapter-owned opaque error. Original causes remain privately owned; Display/Debug and the public source/downcast chain expose no Keep fields. Echo ingress and destination I/O errors retain their existing API.

k03-private-error-red executed exactly one lost-store test on parent de67e83b, failing the expected downstream downcast assertion. k03-private-error-green passes that regression, a cross-error rendering/type oracle, all 13 feature tests, six default tests, 36 root CAS tests and directly relevant strict Clippy/format/policy/MSRV/SPDX checks. Primary Code Lawyer verifies the shared mapper and its four callers; the first agy approval is superseded. Fresh current-head review remains required. @codex

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: a37257474c

ℹ️ 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".

@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Fresh Adversarial Review: Pull Request #770 (flyingrobots/echo)

  • Repository: flyingrobots/echo
  • Branch: feat/keep-reference-adapter
  • Head Commit: a37257474c1759ae1b9971199300844f1e909d2d
  • Base / Target Commit: bb20c57345374f476fa938789294a0890341eadd (main)
  • Superseded Head: de67e83b88b99af5f963bd9ce8276efdc27b903a (earlier review approval superseded due to P2 downcast coordinate leak)
  • Working Tree: the reviewed checkout (clean checkout at a3725747, confirmed HEAD detached at a3725747, nothing dirty)
  • Review Mode: Read-only adversarial inspection; no code edits, commits, pushes, comments, host test/build execution, subagents, or Docker invocations.

1. Resolution of Late P2 Finding (Commit a3725747)

Background & Flaw Diagnosis

At commit de67e83b, KeepReferenceAdapter boxed upstream Keep errors directly into ContentError::Backend(Box::new(error)). While private struct fields and IdentityBinding::Debug redacted Keep coordinates, the public ContentError::Backend held raw upstream error types (ReconstructionError, IngestionError, PublishError). Callers could downcast cause.downcast_ref::<ReconstructionError>() or format the error via Display/Debug, exposing private Keep BlobId, LayoutId, and ChunkId coordinates.

Red Regression Evidence (k03-private-error-red)

  • Target Commit: de67e83b88b99af5f963bd9ce8276efdc27b903a
  • Test Executed: reference_adapter::tests::losing_the_store_or_recreating_the_adapter_refuses_without_a_receipt
  • Assertion Tested: 
    assertion failed: matches!(error, ContentError::Backend(ref cause) if
    cause.downcast_ref::<ReconstructionError>().is_none() &&
    cause.source().is_none())
  • Result: Failed as expected (test result: FAILED. 0 passed; 1 failed; finished in 0.00s), confirming the P2 downcast leak defect.

Fix Verification at a3725747

  1. Opaque Error Boundary (reference_adapter.rs:172-197):
    • struct BackendFailure: Private struct with private fields operation: &'static str and _cause: Box<dyn Error + Send + Sync>.
    • Display implementation: Formats solely "Keep reference {} failed", self.operation.
    • Debug implementation: Uses custom debug_struct("KeepBackendFailure").field("operation", &self.operation).finish_non_exhaustive(). The upstream cause is never rendered.
    • Error implementation: Implements std::error::Error without overriding fn source(&self). It defaults to returning None.
    • Downcast immunity: Because BackendFailure is private and unexported, external crates cannot name or downcast to BackendFailure. Downcasting to ReconstructionError, IngestionError, or PublishError returns None.
  2. Comprehensive Operation Coverage:
    • Identity Verification (reference_adapter.rs:118): Maps IdentityBinding::from_source errors to backend_failure("identity verification", error).
    • Staging (reference_adapter.rs:136, 150-160): ingestion_error maps resource limits (CapacityExceeded, Allocation, Layout(EntryLimitExceeded | Allocation)) to ContentError::ResourceLimit, and all other errors to backend_failure("staging", error).
    • Publication (reference_adapter.rs:141, 162-167): publication_error maps PublishError::CapacityExceeded to ContentError::ResourceLimit, and all other errors to backend_failure("publication", error).
    • Reconstruction (reference_adapter.rs:86): Maps store.reconstruct errors to backend_failure("reconstruction", error).
  3. Echo Ingress & Destination I/O Compatibility:
    • Ingress io::Error during StagedContent::read_expected remains mapped to ContentError::Io(error) via PrivateContentBuffer::map_error.
    • Destination promotion errors (e.g., io::Error in TransactionalContentDestination::promote) pass through directly in reconstruct_quarantined without modification.
  4. Adversarial Test Suite (reference_adapter/tests.rs:121-156, 224-259):
    • losing_the_store_or_recreating_the_adapter_refuses_without_a_receipt: Validates live adapter reconstruction failure against a cleared store. Asserts downcast_ref::<ReconstructionError>().is_none(), source().is_none(), neither Display nor Debug contains Keep coordinates, and prior visible destination bytes are preserved.
    • upstream_error_types_and_coordinates_remain_private: Adversarial unit oracle iterating over IngestionError::BlobIdentityMismatch, PublishError::ConflictingLayout, and ReconstructionError::BlobMissing. Asserts downcasting fails for all three upstream types, source().is_none(), and formatted strings contain neither blob nor layout coordinates.
  5. Review Thread Resolution:
    • Live GitHub review thread PRRT_kwDOQH8Wr86qFEJ_ by chatgpt-codex-connector is confirmed resolved and outdated (isResolved: true, isOutdated: true). Codex re-reviewed a37257474c and commented: "Didn't find any major issues. 🚀".

2. Findings (P0–P5) & Review Coverage Limitations

Demonstrated Source Defects: None (0 actionable findings)

Exhaustive line-by-line inspection of all PR 770 files at a3725747 identified 0 actionable defects (P0–P5).

  • Memory Safety: Strict #![forbid(unsafe_code)] in experiments/echo-keep/src/lib.rs:8.
  • Arithmetic & Sizing: Checked conversions (usize::try_from, u64::try_from, checked_add, saturating_add) throughout.
  • Error Propagation: No unhandled unwrap(), expect(), panic!(), or unbounded recursion in production paths.
  • Coordinate Privacy: Keep physical coordinates are completely hidden from public types, downcast, source chains, Display, and Debug.

Explicit Review Coverage Limitations & Invariant Boundaries (Not Defects)

  1. Volatile In-Memory Storage Only (No Restart Durability):
  2. Public API Quarantine Testing vs. Inaccessible Upstream Tables:
    • Chunk-missing and corrupt-layout tests in reference_adapter/tests.rs:158-195 exercise Keep's public entry points (reconstruct_admitted_layout and reconstruct_record) through reconstruct_quarantined.
    • They do not mutate Keep's internal, inaccessible private chunk tables.
  3. Absence of Host Platform Evidence:
    • No evidence exists for native Windows host execution, hardware power-loss survival, or production latency adoption.
    • All retained gate runs were executed in a hermetic Linux Docker container (echo-read-runtime:red).

3. Code Path Analysis & Production Parity Mapping

PR #770 delivers an experimental, default-off complete-object physical content adapter implementing echo_cas::physical_content::PhysicalContentBackend. Production Echo CAS uses MemoryTier and DiskTier.

Operation / Behavior Experimental Keep Path Production Echo CAS Parallel Path Parity & Invariant Verification
Backend Construction & Caps reference_adapter.rs:45-60: KeepReferenceAdapter::new(physical_bytes, object_bytes, objects, layout_entries) memory.rs:32-34: MemoryTier::new() / DiskTier::open(path) Keep adapter requires explicit caps for physical payload, per-object size, object count, and layout entries. Validates LayoutEntryLimit::new before allocating.
Expected Ingestion Staging physical_content.rs:171-184: StagedContent::read_expected physical_content.rs:171-184: StagedContent::read_expected Shared type: Preflights expected_length > byte_limit before reading source; reads into PrivateContentBuffer; seals BLAKE3 hash and length.
Content Publication reference_adapter.rs:104-148: KeepReferenceAdapter::publish_content physical_content.rs:336-342 (MemoryTier) & 370-375 (DiskTier) Preflights object-size and object-count caps. Computes same-source IdentityBinding; verifies Echo identity and length. Checks duplicate binding equality. Stages and commits to Keep ReferenceStore. Validates commit receipt against target and layout ID before registering private binding.
Immutable View Borrow reference_adapter.rs:100-103: KeepReferenceView<'a>(&'a KeepReferenceAdapter) physical_content.rs:333-335 (MemoryContentView) & 367-369 (DiskContentView) Borrows underlying store immutably (&self); lifetime bounded to store borrow.
Reconstruction & Quarantine reference_adapter.rs:65-97: KeepReferenceView::reconstruct physical_content.rs:315-329 (MemoryContentView) & 346-363 (DiskContentView) All backends delegate to reconstruct_quarantined. Checks destination atomic promotion support and destination byte limit. Preflights binding. Calls Keep store reconstruct. Verifies Keep receipt target, layout ID, and byte count. Shared quarantine re-hashes BLAKE3 before promotion.
Missing Content Handling reference_adapter.rs:72-76: Unbound target returns CapabilityUnavailable physical_content.rs:322-325 (MemoryTier) & 355-357 (DiskTier) Parity verified: Missing/unbound items refuse with ContentError::CapabilityUnavailable. Bound items whose Keep store lost state return operational ContentError::Backend(Box<BackendFailure>).
Receipt Posture physical_content.rs:135-161: ContentReceipt physical_content.rs:135-161: ContentReceipt Exact parity: All receipts return establishes_durability() == false and establishes_complete_view() == false.

4. Merge & Git History Audit

Commit Graph Verification

  • Merge Base: bb20c57345374f476fa938789294a0890341eadd (git merge-base bb20c573 a3725747 confirmed).
  • PR Commits: Exactly 2 standard commits on top of base bb20c573:
    1. de67e83b88b99af5f963bd9ce8276efdc27b903a: feat(keep): add an optional ReferenceStore content adapter
    2. a37257474c1759ae1b9971199300844f1e909d2d: fix(keep): hide upstream coordinates behind adapter-owned errors
  • Merge Commits in PR Branch: 0. The branch is a clean linear chain.

Audit of Merged Ancestors & Integrated Invariants

  1. PR feat(keep): verify the experimental Echo content identity bridge #768 (bb20c573, K01 - Issue Prove the Echo and Keep content identity bridge #759):
    • Merged feat/keep-identity-bridge into main.
    • Introduced: Bounded dual-identity stream hashing (IdentityBinding), private Keep coordinate redaction from Debug, isolated workspace experiments/echo-keep pinned to Keep 3165890e9291cfb5fe10e81a9d7cd151f3e59464, and dependency boundary enforcement.
    • Preserved in PR feat(keep): add an optional ReferenceStore content adapter #770: IdentityBinding is reused unmodified; Keep coordinates remain strictly private; Keep pin is unchanged.
  2. PR feat(cas): verify complete objects before atomic output promotion #769 (2056c95f, K02 - Issue Add the Echo physical-content port and existing CAS adapters #760):
    • Merged feat/physical-content-port into main.
    • Introduced: Complete-object physical-content port (echo_cas::physical_content), StagedContent, reconstruct_quarantined, atomic destination promotion, and shared backend-neutral conformance suite (tests/common/physical_content.rs).
    • Preserved in PR feat(keep): add an optional ReferenceStore content adapter #770: KeepReferenceAdapter implements PhysicalContentBackend; KeepReferenceView implements PhysicalContentView; common::conformance is run against KeepReferenceAdapter in reference_adapter/tests.rs:30-34.

5. Constants, Buffers, and Limits Evidence Trace

  1. Keep Revision Pin:
  2. Buffer and Staging Limits:
    • IdentityBinding read buffer: 8192 bytes ([0_u8; 8192] in experiments/echo-keep/src/lib.rs:67).
    • Overlength probe: remaining.saturating_add(1) in lib.rs:70. Verified by identity_reader_stops_at_one_overlength_probe.
    • Test harness adapter capacity: 1_048_576 bytes (1 MiB) in reference_adapter/tests.rs:24. Matches Keep's largest golden fixture (1_048_576 bytes).
    • Layout Entry Limit: Bounded to Keep's PROTOCOL_MAXIMUM (1_048_576 entries). Tested in reference_adapter/tests.rs:81-84 with u32::MAX, correctly rejecting with ContentError::ResourceLimit.
  3. CI Execution Timeout:

6. Numeric Claims vs. Raw Retained Evidence

Candidate Manifest Verification

  • Manifest: retained evidence: k03-private-error-green.manifest.json
  • Base/Head Recorded: de67e83b88b99af5f963bd9ce8276efdc27b903a with dirty working tree matching the exact 5 files committed in a3725747.
  • File Count: Exactly 979 files.
  • Candidate Tree Hash Verification: All 979 files in the reviewed checkout at a3725747 were independently hashed (SHA-256).
    • Missing files: 0
    • Hash mismatches: 0
    • Match rate: 100% (979 / 979)

Test Count Claims vs. Raw Logs (k03-private-error-green.log)

  • 6 Default Tests (echo-keep-experimental, default features):
    • Verified lines 6–14: all 6 passed (complete_binding_budget_preflight_does_not_read_source, identity_reader_stops_at_one_overlength_probe, binding_debug_keeps_backend_coordinates_private, interrupted_short_source_retries_but_failed_source_returns_no_binding, substitution_and_length_mismatch_refuse, pinned_vectors_bind_distinct_laws_and_reconstructed_bytes).
  • 13 Feature-Enabled Tests (echo-keep-experimental, --features reference-adapter):
    • Verified lines 26–41: exactly 13 passed (6 identity tests above + 7 adapter tests):
      1. reference_adapter::tests::losing_the_store_or_recreating_the_adapter_refuses_without_a_receipt
      2. reference_adapter::tests::interrupted_ingress_retries_before_explicit_publication
      3. reference_adapter::tests::real_keep_missing_chunks_and_malformed_layouts_stay_quarantined
      4. reference_adapter::tests::private_coordinate_substitutions_never_promote_output
      5. reference_adapter::tests::publication_limits_leave_existing_objects_readable
      6. reference_adapter::tests::upstream_error_types_and_coordinates_remain_private
      7. reference_adapter::tests::keep_and_existing_memory_share_the_exact_contract
  • 36 Root CAS Tests (echo-cas, Rust 1.90.0):
    • Verified lines 54–110:
      • src/lib.rs (memory tier): 17 passed
      • tests/disk_tier.rs: 4 passed
      • tests/physical_content.rs: 5 passed
      • tests/semantic_retention.rs: 10 passed
      • Total: $17 + 4 + 5 + 10 = \mathbf{36}$ passed.
  • 25 Package MSRVs:
    • Verified line 126 and matched against scripts/rust-msrv-policy.tsv (exactly 25 entries).

Resource Budget & Free Space Accounting

Verified against k03-private-error-green.result.json and k03-private-error-green.launch.json:

  • Build Cache: 13,367,787,664 bytes (budget: 21,474,836,480 bytes / 20 GiB) — Passed (62.2% of budget).
  • Data Storage: 4,269,428,880 bytes (budget: 4,294,967,296 bytes / 4 GiB) — Passed (99.4% of budget).
  • Logs: 19,263,177 bytes (budget: 134,217,728 bytes / 128 MiB) — Passed (14.4% of budget).
  • Host Free Space: 722,641,600,512 bytes (~673.0 GiB > 50 GiB floor) — Passed.
  • VM Free Space: 686,833,393,664 bytes (~639.7 GiB > 50 GiB floor) — Passed.

Hosted CI Rollup

  • Live GitHub checks for PR 770 at a3725747:
    • Total checks: 43 successful, 0 failing, 0 pending, 0 cancelled, 0 skipped.
    • mergeStateStatus: CLEAN; mergeable: MERGEABLE.

7. State Machines, Error Propagation, and Refusal Invariants

  1. Interrupted Ingress Retries:
    • IdentityBinding::from_source retries on io::ErrorKind::Interrupted without consuming extra quota. Proven in tests::interrupted_short_source_retries_but_failed_source_returns_no_binding and reference_adapter::tests::interrupted_ingress_retries_before_explicit_publication.
  2. Atomic Destination Promotion & Quarantine:
    • reconstruct_quarantined writes candidate bytes into PrivateContentBuffer.
    • On overlength write, PrivateContentBuffer::write flags identity_failure = true, returning ContentError::Mismatch.
    • On allocation failure, it flags resource_failure = true, returning ContentError::ResourceLimit.
    • On any error, destination.promote is bypassed; the destination visible buffer remains identical to its prior state.
  3. Repeated Publication Idempotence:
    • When an object is re-published at the object-count limit, reference_adapter.rs:112 checks !self.bindings.contains_key(&target.hash) && self.bindings.len() >= self.object_count. Existing keys bypass the count limit. Identity equality is confirmed before staging. Verified in reference_adapter/tests.rs:59-60.
  4. Refusal without Silent Fallback:
    • Missing Keep store state returns operational ContentError::Backend(Box<BackendFailure>).
    • Unbound targets return ContentError::CapabilityUnavailable.
    • No fallback to MemoryTier or DiskTier occurs under any condition.

8. Repository Standards & Pre-PR Gate Conformance

9. Execution State of Validation Checks

  • Inspected Only:
    • Source code, Git objects, and PR metadata at a3725747.
    • Manifest k03-private-error-green.manifest.json (all 979 files verified via SHA-256).
    • Retained execution logs: k03-private-error-green.log, k03-private-error-red.log, k03-final-gate-v3.log.
    • Retained launch and result files: k03-private-error-green.launch.json, k03-private-error-green.result.json.
    • Live GitHub PR state, checks rollup, and review threads via gh CLI.
    • Pinned Keep commit in /Users/j/git/keep via read-only git -C /Users/j/git/keep show origin/main:<path>.
  • Executed Directly by Reviewer:
    • None. In accordance with read-only review rules, no host compiler builds, test suites, or Docker containers were run.
  • Skipped / Unavailable:
    • Live process-death recovery tests, host Windows tests, and hardware power-loss verification (unavailable; explicitly excluded from PR claims).

10. Mandatory Verification Checklist

APPROVE

@flyingrobots

flyingrobots commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Final Code Lawyer gate at a37257474c1759ae1b9971199300844f1e909d2d: the late P2 error leak is fixed across identity, staging, publication and reconstruction. A private wrapper owns causes, hides Debug/Display detail, and exposes no source chain or upstream downcast. Capacity/resource categories and ordinary Echo ingress/destination I/O retain their contracts. The real lost-store regression is RED on de67e83b, GREEN here; the full guarded gate passes six default, 13 feature and 36 CAS tests with relevant strict checks. All 979 source hashes match the current candidate.

The new independent agy review says APPROVE with a complete checklist for this exact head. I read the entire report; no actionable findings remain. The earlier approval is historical and superseded. Read-only reviewer inspection is separate from the author's executed Docker evidence. Hosted checks are green and the single addressed thread is resolved. Volatile ReferenceStore support, default-off integration, no fallback and no durability/production-adoption claim remain the acceptance boundary. User-authorized ordinary merge is eligible subject to the final live gate check.

@flyingrobots
flyingrobots merged commit fe87892 into main Oct 7, 2026
43 checks passed
@flyingrobots
flyingrobots deleted the feat/keep-reference-adapter branch October 7, 2026 20:39
This was referenced Oct 7, 2026
Open
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add an experimental Keep ReferenceStore adapter

1 participant

@flyingrobots