Design the application storage and cache ownership boundary#61
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)docs/design/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR documents API-0047, defining application storage and cache ownership contracts, retention and cache semantics, compatibility requirements, validation plans, and review evidence. Release, roadmap, changelog, design-cycle, and invariant indexes are updated for the v6.2.0 direction. ChangesApplication storage boundary
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Self-reviewReviewed the complete main...design/application-storage-boundary diff against #58, the METHOD template, existing RootSet behavior, semver compatibility, and downstream goalpost git-stunts/git-warp#734.
Validation evidence is committed in docs/design/0047-application-storage-cache-boundary/witness/design-review.md. |
Code Lawyer verdictPASS after corrections Adversarial review found and resolved four contract defects before publication:
Residual constraints are explicit: witnesses are generation-scoped, streaming requires residency proof, and unknown physical attribution must remain unknown rather than guessed. Open blockers: none in the diff. CI and CodeRabbit remain external merge gates. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md`:
- Around line 184-191: Update the bundle materialization flow around
cas.bundles.put and define configurable resource limits for member count, member
path length, descriptor size, and fanout. Enforce these limits before accepting
the bundle, and return stable, documented errors for each exceeded limit so
bounded batch and residency guarantees remain valid.
- Around line 350-354: Expand the maxBytes accounting section near the existing
logical-payload definition with deterministic rules for which validated manifest
fields contribute to assets, bundles, and pages, how shared child graphs are
counted across entries, and how adopted or legacy handles without size metadata
are treated. Ensure sweep() and inspect() use the same rules and produce
consistent capacity and eviction results.
- Around line 205-212: Define a ref-safe grammar for the cache namespace used by
the cas.caches.open configuration, including normalization and rejection rules
for empty components, “..”, control characters, ambiguous separators, and
reserved names. State that invalid or ambiguous namespaces must be rejected and
valid namespaces must map uniquely within the intended refs/cas/caches/*
boundary; apply the same rules to the additional cache namespace.
- Around line 193-203: Update publications.commit validation and its generic
contract to accept a validated root handle for all supported facade kinds,
including bundle, asset, and page handles, rather than requiring only a bundle
handle. Alternatively, explicitly enumerate those supported root kinds and add
proof tests covering each; keep the commit flow in the application storage
example consistent with the revised contract.
- Around line 219-223: Define the CacheSet.get() hit shape to explicitly include
handle, key, generation/evidence, expiry, and policy fields, matching the fields
used by the cache lookup example. Add tests covering a valid hit, an expired
entry returning a miss, and evidence from the current generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12c081c7-9d7c-4ed0-98d9-a230676360fd
📒 Files selected for processing (8)
BEARING.mdCHANGELOG.mdROADMAP.mddocs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.mddocs/design/0047-application-storage-cache-boundary/witness/design-review.mddocs/design/README.mddocs/invariants/I-002-application-storage-ownership.mddocs/invariants/README.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
docs/design/**
📄 CodeRabbit inference engine (AGENTS.md)
Use
docs/design/directory for durable design contracts and proof plans
Files:
docs/design/README.mddocs/design/0047-application-storage-cache-boundary/witness/design-review.mddocs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md
BEARING.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
BEARING.mdto document current execution gravity and active tensions
Files:
BEARING.md
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
CHANGELOG.mdto record the historical truth of merged behavior
Files:
CHANGELOG.md
ROADMAP.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
ROADMAP.mdas a signpost to the GitHub release tracker
Files:
ROADMAP.md
🧠 Learnings (1)
📚 Learning: 2026-03-30T19:53:48.000Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 29
File: docs/design/TR-010-planning-index-consistency-review.md:121-131
Timestamp: 2026-03-30T19:53:48.000Z
Learning: In this repo, CHANGELOG.md entries should be user-facing and descriptive (bullet points) rather than cycle-ID headings (e.g., use a phrase like “Planning-index consistency review” instead of a “TR-010 — …” heading). When searching the changelog, don’t rely on grepping for cycle IDs (e.g., “TR-010”) because it may cause false negatives; search for the descriptive keywords/phrases from the bullet entries instead.
Applied to files:
CHANGELOG.md
🪛 LanguageTool
docs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md
[style] ~669-~669: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...sh a handle from a durability claim? 3. Can a caller publish causal history without...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~670-~670: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ry without assembling payload trees? 4. Can cache data be live now and collectible ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~706-~706: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ll objects reachable from a handle. - Very large repository attribution may need commit-...
(EN_WEAK_ADJECTIVE)
🔇 Additional comments (7)
BEARING.md (1)
17-22: LGTM!Also applies to: 124-131
ROADMAP.md (1)
32-40: LGTM!Also applies to: 45-50, 51-59
CHANGELOG.md (1)
10-18: LGTM!docs/design/README.md (1)
40-40: LGTM!docs/design/0047-application-storage-cache-boundary/witness/design-review.md (1)
1-91: LGTM!docs/invariants/I-002-application-storage-ownership.md (1)
1-46: LGTM!docs/invariants/README.md (1)
13-13: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md`:
- Around line 407-419: Align the CacheHit example’s policy representation with
the Retention Witness Contract by replacing the nested policy object in the
stable CacheHit shape with the flat cache-policy value used by retentionWitness
(such as “pinned” or “evictable”). Keep evidence: retentionWitness unchanged and
ensure the example exposes only one consistent policy shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 810bdf07-dcb9-4d9d-b018-cc1d2a4d328d
📒 Files selected for processing (2)
docs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.mddocs/design/0047-application-storage-cache-boundary/witness/design-review.md
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: test-docker (node)
- GitHub Check: test-docker (deno)
- GitHub Check: test-docker (bun)
🧰 Additional context used
📓 Path-based instructions (1)
docs/design/**
📄 CodeRabbit inference engine (AGENTS.md)
Use
docs/design/directory for durable design contracts and proof plans
Files:
docs/design/0047-application-storage-cache-boundary/witness/design-review.mddocs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md
🔇 Additional comments (7)
docs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md (6)
191-197: LGTM!Also applies to: 274-295
373-393: LGTM!
398-398: LGTM!
421-461: LGTM!
584-589: LGTM!
709-714: LGTM!docs/design/0047-application-storage-cache-boundary/witness/design-review.md (1)
71-105: LGTM! CR-001 through CR-005 accurately reflect the corresponding contract fixes now present in the design specification.
1 participant
Summary
Why
Downstream applications currently have to sequence raw CAS objects, build Git trees, maintain cache refs and indexes, and implement retention/expiry/repair themselves. This design makes git-cas the sole owner of those mechanics while preserving application domain and causal semantics.
Review evidence
The committed design witness records four resolved findings:
Validation
Closes #58
Refs #50
Downstream: git-stunts/git-warp#734