Repository navigation
test(reborn): annotate the §4.3 store ratchet with per-entry achievable-floor status - #6216
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe ratchet test’s contract comments now describe completion as empty debt, document bounded-cache exceptions and inventory status, and reorder entries in the frozen in-memory store list. Test logic and public declarations are unchanged. ChangesIn-memory store ratchet
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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.
Code Review
This pull request updates the documentation and comments in reborn_inmemory_store_ratchet.rs to reflect the current status of the remaining in-memory stores after the mechanical consolidations (A1–A8) have been completed. It categorizes the remaining stores into deferred, justified keeps (bounded caches), blocked, security-sensitive, and build-first categories, providing clear context for future refactoring efforts. There are no review comments, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 5 | 5 | cf8a84512d6b |
Head: cf8a84512d6b9cc7ae7409e119073e34047276ce
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
This is a bounded, reviewable stack layer changing one architecture-test documentation file (52 additions, 15 deletions). The enforced allowlist set is unchanged and no runtime behavior changes. Static checks passed, with five non-blocking accuracy/completeness issues in the new annotations.
Findings
Blocking: 0 / Notes: 5
Non-blocking notes (5)
1. 💬 [LOW] Referenced store-triage worklog is absent
Location: crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs:64
The target tree contains no .worklog/reborn-refactor.md, so future contributors cannot access the cited “full analysis.” Commit/link an available source of truth or remove this reference.
2. 💬 [LOW] CheckpointState filesystem variant already exists
Location: crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs:70-71
FilesystemCheckpointStateStore already exists in ironclaw_loop_host, has contract tests, and is wired in composition. Separate InMemoryCheckpointStateStore from the stores that genuinely still need a filesystem implementation so the next task is scoped correctly.
3. 💬 [LOW] Justified-cache rationale contradicts production filesystem stores
Location: crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs:76-79
The blanket claim that durable variants would be semantically wrong conflicts with the code: FilesystemSubagentGoalStore is explicitly the production store for libSQL/Postgres, and serving wires FilesystemOpenAiCompatRefStore. The in-memory implementations may remain justified test/fallback implementations, but the annotation should state that rationale instead.
4. 💬 [LOW] OpenAI compatibility store is not LRU
Location: crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs:83-85
The implementation evicts the mapping with the minimum created_at; lookups and mutations do not refresh recency. Describe it as bounded oldest-created eviction rather than LRU.
5. 💬 [LOW] Three remaining entries are explicitly untriaged
Location: crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs:102-107
This block says the three crate-private stores are “not yet individually triaged,” contradicting the preceding claim that every entry has a verified per-entry status and a scoped next task. Add a concrete status for each entry or soften the overall completeness claim.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
| /// justified keep — so the §10 "shrink to empty" goal is not reachable by a | ||
| /// swap. Each entry is annotated with WHAT it needs, so the next contributor | ||
| /// picks up a scoped task instead of re-deriving the blocker. See | ||
| /// `../.worklog/reborn-refactor.md` (store triage) for the full analysis. |
There was a problem hiding this comment.
This referenced worklog is not present anywhere in the target tree, so future contributors cannot consult the claimed full analysis. Please link an available source of truth or remove the reference.
There was a problem hiding this comment.
Fixed on this branch: the worklog reference is removed — the annotations themselves are now the source of truth, and the completeness claim was softened to match.
| // `inmemory-turn-state` production runtime authority (pessimistic Mutex, | ||
| // no-CAS-livelock); a `FilesystemTurnStateStore<InMemoryBackend>` swap needs | ||
| // a concurrency stress test PROVING it keeps the no-livelock property first. | ||
| // Checkpoint/LoopCheckpoint/InstructionMaterialization also need a filesystem |
There was a problem hiding this comment.
FilesystemCheckpointStateStore is already implemented in ironclaw_loop_host, contract-tested, and wired by composition. Please distinguish InMemoryCheckpointStateStore from the stores that still need a filesystem variant.
There was a problem hiding this comment.
Fixed: the turns-cluster note now distinguishes InMemoryCheckpointStateStore (its FilesystemCheckpointStateStore already exists in ironclaw_loop_host, contract-tested and composition-wired — only the test-seam swap + allowlist trim remain) from LoopCheckpoint/InstructionMaterialization, which still need filesystem variants built.
| // --- peripheral stores (outside §4.3's five core domains; listed so the | ||
| // ratchet stays exhaustive and no new InMemory store slips in) --- | ||
| // --- JUSTIFIED KEEPS — bounded in-memory CACHES, not persistence-store debt. | ||
| // A durable `Filesystem*` variant would be semantically wrong (you do not |
There was a problem hiding this comment.
This rationale conflicts with existing production adapters: libSQL/Postgres use FilesystemSubagentGoalStore, and OpenAI-compatible serving uses FilesystemOpenAiCompatRefStore. The in-memory types may be justified test/fallback implementations, but durable storage is not semantically wrong for these contracts.
There was a problem hiding this comment.
Fixed: the justified-keeps section now states that durable production variants already exist and are wired (FilesystemSubagentGoalStore in the libSQL/Postgres runner adapters, FilesystemOpenAiCompatRefStore in OpenAI-compatible serving), and frames the in-memory types as the bounded volatile role beside those stores rather than missing consolidations.
| // order) cache of in-flight subagent-spawn goals — goal_store.rs. | ||
| "InMemoryBoundedSubagentGoalStore", | ||
| "InMemoryExtensionInstallationStore", | ||
| // OpenAiCompatRef: bounded-LRU (`max_mappings`/`with_capacity`) AND the |
There was a problem hiding this comment.
This is not LRU: eviction selects the minimum created_at, and accesses do not update recency. Please describe it as oldest-created eviction.
There was a problem hiding this comment.
Fixed: described as capacity-bounded with oldest-created eviction (evicts the minimum created_at; reads do not refresh recency — explicitly noted as NOT an LRU), matching refs.rs's min_by_key(created_at).
| "InMemorySessionStore", | ||
| // --- pub(crate) stores the visibility-aware scanner also inventories | ||
| // (same debt class, just crate-private) --- | ||
| // (crate-private; not yet individually triaged — assess build-vs-justified |
There was a problem hiding this comment.
These entries are explicitly untriaged, which conflicts with the preceding claim that every remaining entry has a verified status and scoped task. Please annotate each one or soften the completeness claim.
There was a problem hiding this comment.
Fixed: the header claim is softened — triaged entries carry their scoped task, and the pub(crate) trio is explicitly labeled untriaged instead of being covered by a blanket verified-status claim.
|
🚅 Deployed to the ironclaw-pr-6216 environment in ironclaw-ci-preview
|
66d7cf2 to
154913a
Compare
cf8a845 to
843cfba
Compare
…> CompositeRootFilesystem (§4.4.1) First §4.4.1 slice (deployment-mode-as-type cleanup), on a fresh axis now that the mechanical §4.3 store family is complete. `LocalDevRootFilesystem` was a `pub(crate) type LocalDevRootFilesystem = CompositeRootFilesystem;` alias — a pure redundant indirection whose `LocalDev` prefix falsely read as a deployment tier when it is just the composition's `CompositeRootFilesystem` (the same type factory.rs already used directly, interchangeably, in dozens of places). This is the doc's §4.4.1 bucket-(b): mis-prefixed shared substrate → de-prefix to the honest type. Inlined the alias to `ironclaw_filesystem::CompositeRootFilesystem` across the 4 composition files that used it (factory/runtime/openai_compat_serve/turn_run_snapshot, ~54 sites), deleted the alias, and repointed the imports (the alias was exported from `crate::factory`; consumers now import the real type from `ironclaw_filesystem`). The private, genuinely-local-dev `LocalDevRootFilesystemBundle` struct keeps its name (word-boundary rename left it untouched; it is not on the ratchet — visibility-aware scanner skips private types). R2 ratchet (`reborn_localdev_typename`): drop `LocalDevRootFilesystem` from the frozen allowlist — one fewer deployment-mode-as-type leak. Pure type-alias inline, semantically identical. Verified: `cargo build -p ironclaw_reborn_composition` (default + libsql+slack) clean; localdev ratchet 4; clippy -D warnings clean; local_dev composition tests 233 pass; fmt + pre-commit clean. Stacked on #6216. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
154913a to
b375388
Compare
…dings on #6216) - Drop the reference to a worklog not present in the tree. - InMemoryCheckpointStateStore: FilesystemCheckpointStateStore already exists in ironclaw_loop_host (contract-tested, composition-wired) — the entry needs a test-seam swap, not a store built; LoopCheckpoint/ InstructionMaterialization still need variants built. - Justified-keeps section acknowledges the production filesystem variants that already exist and are wired (FilesystemSubagentGoalStore, FilesystemOpenAiCompatRefStore) — the in-memory types are the bounded volatile role beside them, not missing consolidations. - OpenAiCompatRef eviction described as oldest-created (min created_at, reads do not refresh recency), not LRU. - Completeness claim softened: the pub(crate) trio is explicitly untriaged rather than claimed verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
843cfba to
636f29d
Compare
…> CompositeRootFilesystem (§4.4.1) First §4.4.1 slice (deployment-mode-as-type cleanup), on a fresh axis now that the mechanical §4.3 store family is complete. `LocalDevRootFilesystem` was a `pub(crate) type LocalDevRootFilesystem = CompositeRootFilesystem;` alias — a pure redundant indirection whose `LocalDev` prefix falsely read as a deployment tier when it is just the composition's `CompositeRootFilesystem` (the same type factory.rs already used directly, interchangeably, in dozens of places). This is the doc's §4.4.1 bucket-(b): mis-prefixed shared substrate → de-prefix to the honest type. Inlined the alias to `ironclaw_filesystem::CompositeRootFilesystem` across the 4 composition files that used it (factory/runtime/openai_compat_serve/turn_run_snapshot, ~54 sites), deleted the alias, and repointed the imports (the alias was exported from `crate::factory`; consumers now import the real type from `ironclaw_filesystem`). The private, genuinely-local-dev `LocalDevRootFilesystemBundle` struct keeps its name (word-boundary rename left it untouched; it is not on the ratchet — visibility-aware scanner skips private types). R2 ratchet (`reborn_localdev_typename`): drop `LocalDevRootFilesystem` from the frozen allowlist — one fewer deployment-mode-as-type leak. Pure type-alias inline, semantically identical. Verified: `cargo build -p ironclaw_reborn_composition` (default + libsql+slack) clean; localdev ratchet 4; clippy -D warnings clean; local_dev composition tests 233 pass; fmt + pre-commit clean. Stacked on #6216. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b375388 to
8d453ff
Compare
…dings on #6216) - Drop the reference to a worklog not present in the tree. - InMemoryCheckpointStateStore: FilesystemCheckpointStateStore already exists in ironclaw_loop_host (contract-tested, composition-wired) — the entry needs a test-seam swap, not a store built; LoopCheckpoint/ InstructionMaterialization still need variants built. - Justified-keeps section acknowledges the production filesystem variants that already exist and are wired (FilesystemSubagentGoalStore, FilesystemOpenAiCompatRefStore) — the in-memory types are the bounded volatile role beside them, not missing consolidations. - OpenAiCompatRef eviction described as oldest-created (min created_at, reads do not refresh recency), not LRU. - Completeness claim softened: the pub(crate) trio is explicitly untriaged rather than claimed verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
636f29d to
bb310b2
Compare
…> CompositeRootFilesystem (§4.4.1) First §4.4.1 slice (deployment-mode-as-type cleanup), on a fresh axis now that the mechanical §4.3 store family is complete. `LocalDevRootFilesystem` was a `pub(crate) type LocalDevRootFilesystem = CompositeRootFilesystem;` alias — a pure redundant indirection whose `LocalDev` prefix falsely read as a deployment tier when it is just the composition's `CompositeRootFilesystem` (the same type factory.rs already used directly, interchangeably, in dozens of places). This is the doc's §4.4.1 bucket-(b): mis-prefixed shared substrate → de-prefix to the honest type. Inlined the alias to `ironclaw_filesystem::CompositeRootFilesystem` across the 4 composition files that used it (factory/runtime/openai_compat_serve/turn_run_snapshot, ~54 sites), deleted the alias, and repointed the imports (the alias was exported from `crate::factory`; consumers now import the real type from `ironclaw_filesystem`). The private, genuinely-local-dev `LocalDevRootFilesystemBundle` struct keeps its name (word-boundary rename left it untouched; it is not on the ratchet — visibility-aware scanner skips private types). R2 ratchet (`reborn_localdev_typename`): drop `LocalDevRootFilesystem` from the frozen allowlist — one fewer deployment-mode-as-type leak. Pure type-alias inline, semantically identical. Verified: `cargo build -p ironclaw_reborn_composition` (default + libsql+slack) clean; localdev ratchet 4; clippy -D warnings clean; local_dev composition tests 233 pass; fmt + pre-commit clean. Stacked on #6216. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Ready for mergeCI green (18 pass / 0 fail) on the restacked head. All five IronLoop factual findings fixed with replies: the missing-worklog reference removed; 🤖 Generated with Claude Code |
fb2bc20 to
6bbbbd2
Compare
…dings on #6216) - Drop the reference to a worklog not present in the tree. - InMemoryCheckpointStateStore: FilesystemCheckpointStateStore already exists in ironclaw_loop_host (contract-tested, composition-wired) — the entry needs a test-seam swap, not a store built; LoopCheckpoint/ InstructionMaterialization still need variants built. - Justified-keeps section acknowledges the production filesystem variants that already exist and are wired (FilesystemSubagentGoalStore, FilesystemOpenAiCompatRefStore) — the in-memory types are the bounded volatile role beside them, not missing consolidations. - OpenAiCompatRef eviction described as oldest-created (min created_at, reads do not refresh recency), not LRU. - Completeness claim softened: the pub(crate) trio is explicitly untriaged rather than claimed verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bb310b2 to
86e9f90
Compare
…> CompositeRootFilesystem (§4.4.1) First §4.4.1 slice (deployment-mode-as-type cleanup), on a fresh axis now that the mechanical §4.3 store family is complete. `LocalDevRootFilesystem` was a `pub(crate) type LocalDevRootFilesystem = CompositeRootFilesystem;` alias — a pure redundant indirection whose `LocalDev` prefix falsely read as a deployment tier when it is just the composition's `CompositeRootFilesystem` (the same type factory.rs already used directly, interchangeably, in dozens of places). This is the doc's §4.4.1 bucket-(b): mis-prefixed shared substrate → de-prefix to the honest type. Inlined the alias to `ironclaw_filesystem::CompositeRootFilesystem` across the 4 composition files that used it (factory/runtime/openai_compat_serve/turn_run_snapshot, ~54 sites), deleted the alias, and repointed the imports (the alias was exported from `crate::factory`; consumers now import the real type from `ironclaw_filesystem`). The private, genuinely-local-dev `LocalDevRootFilesystemBundle` struct keeps its name (word-boundary rename left it untouched; it is not on the ratchet — visibility-aware scanner skips private types). R2 ratchet (`reborn_localdev_typename`): drop `LocalDevRootFilesystem` from the frozen allowlist — one fewer deployment-mode-as-type leak. Pure type-alias inline, semantically identical. Verified: `cargo build -p ironclaw_reborn_composition` (default + libsql+slack) clean; localdev ratchet 4; clippy -D warnings clean; local_dev composition tests 233 pass; fmt + pre-commit clean. Stacked on #6216. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…evable-floor status The mechanical §4.3 store consolidations are complete (A1–A8: approvals, authorization, processes, run-state, budget-gate, and the whole outbound family — OutboundState/TriggeredRunDelivery/DeliveredGateRoute). Every entry still in `FROZEN_INMEMORY_STORES` is blocked on non-mechanical work OR is a justified keep, so the §10 "shrink to empty" goal is not reachable by a swap. This annotates each remaining allowlist entry with its VERIFIED status so the next contributor picks up a scoped task instead of re-deriving the blocker (comments are stripped by the scanner — documentation only, the enforced string set is unchanged; entries reordered to group the justified keeps): - turns cluster — DEFERRED (production `inmemory-turn-state` authority; needs a no-livelock concurrency proof + a built filesystem variant, some cross-crate). - `InMemoryBoundedSubagentGoalStore`, `InMemoryOpenAiCompatRefStore` — JUSTIFIED bounded CACHES (capacity-bounded evict-oldest / bounded-LRU + filesystem-free contract boundary), NOT persistence debt; a durable variant would be wrong. - `InMemoryExtensionInstallationStore` — BLOCKED cross-crate (the Filesystem variant in composition depends on a composition-internal contract registry; can't move down to `ironclaw_extensions`). - `InMemorySecretStore` — security-sensitive. - `InMemorySessionStore` — BUILD-FIRST (no filesystem variant; auth-adjacent). Also reconciles the module-doc "definition of done" to note the two justified caches. No production code changes; ratchet self-tests + the frozen-set contract still pass (4 tests). Stacked on #6214. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dings on #6216) - Drop the reference to a worklog not present in the tree. - InMemoryCheckpointStateStore: FilesystemCheckpointStateStore already exists in ironclaw_loop_host (contract-tested, composition-wired) — the entry needs a test-seam swap, not a store built; LoopCheckpoint/ InstructionMaterialization still need variants built. - Justified-keeps section acknowledges the production filesystem variants that already exist and are wired (FilesystemSubagentGoalStore, FilesystemOpenAiCompatRefStore) — the in-memory types are the bounded volatile role beside them, not missing consolidations. - OpenAiCompatRef eviction described as oldest-created (min created_at, reads do not refresh recency), not LRU. - Completeness claim softened: the pub(crate) trio is explicitly untriaged rather than claimed verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
86e9f90 to
5985c26
Compare
…> CompositeRootFilesystem (§4.4.1) First §4.4.1 slice (deployment-mode-as-type cleanup), on a fresh axis now that the mechanical §4.3 store family is complete. `LocalDevRootFilesystem` was a `pub(crate) type LocalDevRootFilesystem = CompositeRootFilesystem;` alias — a pure redundant indirection whose `LocalDev` prefix falsely read as a deployment tier when it is just the composition's `CompositeRootFilesystem` (the same type factory.rs already used directly, interchangeably, in dozens of places). This is the doc's §4.4.1 bucket-(b): mis-prefixed shared substrate → de-prefix to the honest type. Inlined the alias to `ironclaw_filesystem::CompositeRootFilesystem` across the 4 composition files that used it (factory/runtime/openai_compat_serve/turn_run_snapshot, ~54 sites), deleted the alias, and repointed the imports (the alias was exported from `crate::factory`; consumers now import the real type from `ironclaw_filesystem`). The private, genuinely-local-dev `LocalDevRootFilesystemBundle` struct keeps its name (word-boundary rename left it untouched; it is not on the ratchet — visibility-aware scanner skips private types). R2 ratchet (`reborn_localdev_typename`): drop `LocalDevRootFilesystem` from the frozen allowlist — one fewer deployment-mode-as-type leak. Pure type-alias inline, semantically identical. Verified: `cargo build -p ironclaw_reborn_composition` (default + libsql+slack) clean; localdev ratchet 4; clippy -D warnings clean; local_dev composition tests 233 pass; fmt + pre-commit clean. Stacked on #6216. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 `@crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs`:
- Around line 26-35: The ratchet in the test around FROZEN_INMEMORY_STORES does
not detect newly added frozen stores. Add a checked-in baseline and compare the
current frozen set against it, failing when entries are added while allowing
removals; alternatively remove the “only shrinks” guarantee from the surrounding
comment and failure text.
- Around line 57-111: Add inline source anchors to the durable-variant and
wiring claims in FROZEN_INMEMORY_STORES, citing the defining and integration
symbols for FilesystemCheckpointStateStore, FilesystemSubagentGoalStore,
FilesystemOpenAiCompatRefStore, and FilesystemExtensionInstallationStore. Keep
FilesystemSessionStore described as an unimplemented open gap because no crates/
source exists, and trim any unsupported claims rather than leaving them uncited.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf4571cc-557f-43ae-b73d-7747c87b4b45
📒 Files selected for processing (1)
crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs
| //! Definition of done for this axis (§10): the **debt** shrinks to empty — every | ||
| //! persistence-store duplicate becomes `Filesystem*Store<InMemoryBackend>` in | ||
| //! tests. The mechanical consolidations (A1–A8: approvals, authorization, | ||
| //! processes, run-state, budget-gate, and the whole outbound family) are done; | ||
| //! see the annotated `FROZEN_INMEMORY_STORES` below for the per-entry status of | ||
| //! the remainder. Note two entries (`InMemoryBoundedSubagentGoalStore`, | ||
| //! `InMemoryOpenAiCompatRefStore`) are **justified bounded caches**, not | ||
| //! persistence debt — a future PR may formally split them into a justified-keep | ||
| //! list; for now they stay frozen with a do-not-consolidate note. Until then | ||
| //! this frozen set is the contract. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg '(^|/)reborn_inmemory_store_ratchet\.rs$|(^|/)CLAUDE\.md$|(^|/)AGENTS\.md$|(^|/)\.claude/rules'
printf '\n--- file outline ---\n'
ast-grep outline crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs --view expanded
printf '\n--- relevant file slice ---\n'
sed -n '1,240p' crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rsRepository: nearai/ironclaw
Length of output: 17702
Enforce the “only shrinks” ratchet.
added.is_empty() only rejects names outside the current allowlist. If a new InMemory*Store is added to both the code and FROZEN_INMEMORY_STORES, this test still passes, so the “never add” / “only shrinks” contract is not enforced. Add a checked-in baseline/diff check, or drop that guarantee from the comment and failure text.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs` around
lines 26 - 35, The ratchet in the test around FROZEN_INMEMORY_STORES does not
detect newly added frozen stores. Add a checked-in baseline and compare the
current frozen set against it, failing when entries are added while allowing
removals; alternatively remove the “only shrinks” guarantee from the surrounding
comment and failure text.
| /// **Status of the remainder (assessed 2026-07-18, after the mechanical §4.3 | ||
| /// slices A1–A8 landed the approvals/authorization/processes/run-state/budget-gate | ||
| /// and the whole outbound family).** The clean mechanical consolidations are | ||
| /// DONE; every entry still here is blocked on non-mechanical work OR is a | ||
| /// justified keep — except the trailing pub(crate) trio, which is not yet | ||
| /// individually triaged. Triaged entries are annotated with WHAT they need, so | ||
| /// the next contributor picks up a scoped task instead of re-deriving the | ||
| /// blocker; untriaged entries say so explicitly. | ||
| const FROZEN_INMEMORY_STORES: &[&str] = &[ | ||
| // --- remaining Slice-A domain store: turns (do last, reconcile-then-delete; | ||
| // `InMemoryTurnStateStore` is also the `inmemory-turn-state` production | ||
| // runtime authority, so it is not a mechanical delete) --- | ||
| // --- turns cluster: DEFERRED, not mechanical. `InMemoryTurnStateStore` is the | ||
| // `inmemory-turn-state` production runtime authority (pessimistic Mutex, | ||
| // no-CAS-livelock); a `FilesystemTurnStateStore<InMemoryBackend>` swap needs | ||
| // a concurrency stress test PROVING it keeps the no-livelock property first. | ||
| // `FilesystemCheckpointStateStore` already EXISTS in `ironclaw_loop_host` | ||
| // (contract-tested, composition-wired) — that entry only needs the test-seam | ||
| // swap + allowlist trim; LoopCheckpoint/InstructionMaterialization still need | ||
| // a filesystem variant BUILT (cross-crate in `ironclaw_loop_host`). --- | ||
| "InMemoryTurnStateStore", | ||
| "InMemoryCheckpointStateStore", | ||
| "InMemoryLoopCheckpointStore", | ||
| "InMemoryInstructionMaterializationStore", | ||
| // --- peripheral stores (outside §4.3's five core domains; listed so the | ||
| // ratchet stays exhaustive and no new InMemory store slips in) --- | ||
| // --- JUSTIFIED KEEPS — bounded in-memory caches serving the test/no-durable | ||
| // fallback role. Durable production variants ALREADY EXIST and are wired | ||
| // (`FilesystemSubagentGoalStore` in the libSQL/Postgres runner adapters; | ||
| // `FilesystemOpenAiCompatRefStore` in OpenAI-compatible serving) — these | ||
| // in-memory types are not missing consolidations, they are the bounded | ||
| // volatile role next to those stores. Do NOT swap them for a durable | ||
| // store in tests that specifically exercise the bounded/evicting cache | ||
| // semantics. --- | ||
| // BoundedSubagentGoal: capacity-bounded, evict-oldest (VecDeque insertion | ||
| // order) cache of in-flight subagent-spawn goals — goal_store.rs. | ||
| "InMemoryBoundedSubagentGoalStore", | ||
| "InMemoryExtensionInstallationStore", | ||
| // OpenAiCompatRef: capacity-bounded with oldest-created eviction (evicts the | ||
| // minimum `created_at`; reads do not refresh recency — NOT an LRU) AND the | ||
| // crate's documented filesystem-free default so contract-only consumers pull | ||
| // no `ironclaw_filesystem` dep (openai_compat CLAUDE.md). | ||
| "InMemoryOpenAiCompatRefStore", | ||
| // --- BLOCKED — cross-crate placement. `FilesystemExtensionInstallationStore` | ||
| // exists but in high `ironclaw_reborn_composition` and depends on a | ||
| // composition-internal contract registry, so it can't move DOWN to | ||
| // `ironclaw_extensions` (whose own tests need an in-memory store). Needs the | ||
| // filesystem store (or its contract dep) relocated first. Prod already wires | ||
| // Filesystem. --- | ||
| "InMemoryExtensionInstallationStore", | ||
| // --- SECURITY-SENSITIVE — secrets subsystem; deliberate careful work, not a | ||
| // mechanical swap. --- | ||
| "InMemorySecretStore", | ||
| // --- BUILD-FIRST — no filesystem variant exists. `InMemorySessionStore` | ||
| // (webui login sessions, TTL-expiring bearer tokens) would gain restart | ||
| // durability from a `FilesystemSessionStore`, but that store must be BUILT | ||
| // (auth-adjacent — handle with care). ~63 usages. --- | ||
| "InMemorySessionStore", | ||
| // --- pub(crate) stores the visibility-aware scanner also inventories | ||
| // (same debt class, just crate-private) --- | ||
| // (crate-private; not yet individually triaged — assess build-vs-justified | ||
| // when picked up, same as the peripheral set above). --- |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== File outline ==\n'
wc -l crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs
sed -n '1,220p' crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs
printf '\n== Search referenced symbols ==\n'
rg -n "FilesystemCheckpointStateStore|FilesystemSubagentGoalStore|FilesystemOpenAiCompatRefStore|FilesystemExtensionInstallationStore|InMemoryTurnStateStore|InMemoryCheckpointStateStore|InMemoryLoopCheckpointStore|InMemoryInstructionMaterializationStore|InMemoryBoundedSubagentGoalStore|InMemoryOpenAiCompatRefStore|InMemoryExtensionInstallationStore|InMemorySecretStore|InMemorySessionStore" crates -g '!target'
printf '\n== CLAUDE/AGENTS invariant mentions ==\n'
rg -n "load-bearing claims|Everything Goes Through Tools|LLM data is never deleted|Trusted-ingress seal|Test through the caller|filesystem variant|bounded in-memory caches|composition-internal contract registry|no-CAS-livelock|oldest-created eviction|FilesystemSessionStore|FilesystemExtensionInstallationStore" CLAUDE.md AGENTS.md .claude -g '!target'Repository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
set -euo pipefail
sed -n '1,220p' crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rsRepository: nearai/ironclaw
Length of output: 11287
🏁 Script executed:
set -euo pipefail
printf '\n== FilesystemCheckpointStateStore definition/re-export ==\n'
rg -n "pub struct FilesystemCheckpointStateStore|pub use filesystem_checkpoint_state::FilesystemCheckpointStateStore" \
crates/ironclaw_loop_host/src crates/ironclaw_loop_host/tests
printf '\n== FilesystemSubagentGoalStore definition/wiring ==\n'
rg -n "pub struct FilesystemSubagentGoalStore|FilesystemSubagentGoalStore::new|FilesystemSubagentGoalStore" \
crates/ironclaw_runner crates/ironclaw_reborn_composition/src/runtime.rs crates/ironclaw_reborn_composition/tests
printf '\n== FilesystemOpenAiCompatRefStore definition/wiring ==\n'
rg -n "pub struct FilesystemOpenAiCompatRefStore|FilesystemOpenAiCompatRefStore::with_root|FilesystemOpenAiCompatRefStore" \
crates/ironclaw_reborn_composition/src crates/ironclaw_reborn_composition/tests crates/ironclaw_reborn_openai_compat crates/ironclaw_reborn_composition/src/llm_admin
printf '\n== FilesystemExtensionInstallationStore definition/wiring ==\n'
rg -n "pub(crate) struct FilesystemExtensionInstallationStore|FilesystemExtensionInstallationStore::load_at|FilesystemExtensionInstallationStore::default_state_path|FilesystemExtensionInstallationStore" \
crates/ironclaw_reborn_composition/src crates/ironclaw_reborn_composition/tests crates/ironclaw_extensions crates/ironclaw_reborn_composition/src/extension_host
printf '\n== Bounded-cache semantics ==\n'
rg -n "capacity-bounded|evict-oldest|oldest-created|LRU|reads do not refresh recency|minimum `created_at`|VecDeque insertion order" \
crates/ironclaw_runner/src crates/ironclaw_reborn_openai_compat crates/ironclaw_reborn_composition/src crates/ironclaw_reborn_composition/tests
printf '\n== FilesystemSessionStore search ==\n'
rg -n "FilesystemSessionStore" cratesRepository: nearai/ironclaw
Length of output: 12381
Add source anchors to the ratchet notes The status block makes load-bearing claims about durable variants and wiring; cite the defining/writing sites inline, or trim the prose. For example: FilesystemCheckpointStateStore (crates/ironclaw_loop_host/src/filesystem_checkpoint_state.rs:44, re-exported in crates/ironclaw_loop_host/src/lib.rs:79), FilesystemSubagentGoalStore (crates/ironclaw_runner/src/subagent/goal_store.rs:79, wired in crates/ironclaw_reborn_composition/src/runtime.rs:305,368), FilesystemOpenAiCompatRefStore (crates/ironclaw_reborn_openai_compat/src/refs_storage.rs:37, wired in crates/ironclaw_reborn_composition/src/llm_admin/openai_compat_serve.rs:176), and FilesystemExtensionInstallationStore (crates/ironclaw_reborn_composition/src/extension_host/extension_installation_store.rs:19, loaded in crates/ironclaw_reborn_composition/src/factory.rs:1925,3326,3367). FilesystemSessionStore still has no crates/ hit, so keep that one framed as an open gap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_architecture/tests/reborn_inmemory_store_ratchet.rs` around
lines 57 - 111, Add inline source anchors to the durable-variant and wiring
claims in FROZEN_INMEMORY_STORES, citing the defining and integration symbols
for FilesystemCheckpointStateStore, FilesystemSubagentGoalStore,
FilesystemOpenAiCompatRefStore, and FilesystemExtensionInstallationStore. Keep
FilesystemSessionStore described as an unimplemented open gap because no crates/
source exists, and trim any unsupported claims rather than leaving them uncited.
Source: Coding guidelines
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.59% — 306535 / 358149 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
Post-restack confirmation (after #6213's merge): rebased onto main, CI fully green on the current head. Still ready for merge. |
…> CompositeRootFilesystem (§4.4.1) (#6218) * test(reborn): annotate the §4.3 store ratchet with the per-entry achievable-floor status The mechanical §4.3 store consolidations are complete (A1–A8: approvals, authorization, processes, run-state, budget-gate, and the whole outbound family — OutboundState/TriggeredRunDelivery/DeliveredGateRoute). Every entry still in `FROZEN_INMEMORY_STORES` is blocked on non-mechanical work OR is a justified keep, so the §10 "shrink to empty" goal is not reachable by a swap. This annotates each remaining allowlist entry with its VERIFIED status so the next contributor picks up a scoped task instead of re-deriving the blocker (comments are stripped by the scanner — documentation only, the enforced string set is unchanged; entries reordered to group the justified keeps): - turns cluster — DEFERRED (production `inmemory-turn-state` authority; needs a no-livelock concurrency proof + a built filesystem variant, some cross-crate). - `InMemoryBoundedSubagentGoalStore`, `InMemoryOpenAiCompatRefStore` — JUSTIFIED bounded CACHES (capacity-bounded evict-oldest / bounded-LRU + filesystem-free contract boundary), NOT persistence debt; a durable variant would be wrong. - `InMemoryExtensionInstallationStore` — BLOCKED cross-crate (the Filesystem variant in composition depends on a composition-internal contract registry; can't move down to `ironclaw_extensions`). - `InMemorySecretStore` — security-sensitive. - `InMemorySessionStore` — BUILD-FIRST (no filesystem variant; auth-adjacent). Also reconciles the module-doc "definition of done" to note the two justified caches. No production code changes; ratchet self-tests + the frozen-set contract still pass (4 tests). Stacked on #6214. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn): correct the ratchet per-entry annotations (IronLoop findings on #6216) - Drop the reference to a worklog not present in the tree. - InMemoryCheckpointStateStore: FilesystemCheckpointStateStore already exists in ironclaw_loop_host (contract-tested, composition-wired) — the entry needs a test-seam swap, not a store built; LoopCheckpoint/ InstructionMaterialization still need variants built. - Justified-keeps section acknowledges the production filesystem variants that already exist and are wired (FilesystemSubagentGoalStore, FilesystemOpenAiCompatRefStore) — the in-memory types are the bounded volatile role beside them, not missing consolidations. - OpenAiCompatRef eviction described as oldest-created (min created_at, reads do not refresh recency), not LRU. - Completeness claim softened: the pub(crate) trio is explicitly untriaged rather than claimed verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(reborn): inline the redundant LocalDevRootFilesystem alias -> CompositeRootFilesystem (§4.4.1) First §4.4.1 slice (deployment-mode-as-type cleanup), on a fresh axis now that the mechanical §4.3 store family is complete. `LocalDevRootFilesystem` was a `pub(crate) type LocalDevRootFilesystem = CompositeRootFilesystem;` alias — a pure redundant indirection whose `LocalDev` prefix falsely read as a deployment tier when it is just the composition's `CompositeRootFilesystem` (the same type factory.rs already used directly, interchangeably, in dozens of places). This is the doc's §4.4.1 bucket-(b): mis-prefixed shared substrate → de-prefix to the honest type. Inlined the alias to `ironclaw_filesystem::CompositeRootFilesystem` across the 4 composition files that used it (factory/runtime/openai_compat_serve/turn_run_snapshot, ~54 sites), deleted the alias, and repointed the imports (the alias was exported from `crate::factory`; consumers now import the real type from `ironclaw_filesystem`). The private, genuinely-local-dev `LocalDevRootFilesystemBundle` struct keeps its name (word-boundary rename left it untouched; it is not on the ratchet — visibility-aware scanner skips private types). R2 ratchet (`reborn_localdev_typename`): drop `LocalDevRootFilesystem` from the frozen allowlist — one fewer deployment-mode-as-type leak. Pure type-alias inline, semantically identical. Verified: `cargo build -p ironclaw_reborn_composition` (default + libsql+slack) clean; localdev ratchet 4; clippy -D warnings clean; local_dev composition tests 233 pass; fmt + pre-commit clean. Stacked on #6216. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
The mechanical §4.3 store consolidations are complete (A1–A8: approvals, authorization, processes, run-state, budget-gate, and the whole outbound family). Every entry still in
FROZEN_INMEMORY_STORESis blocked on non-mechanical work OR is a justified keep — so the §10 "shrink to empty" goal is not reachable by a swap.This annotates each remaining allowlist entry with its verified status so the next contributor picks up a scoped task instead of re-deriving the blocker. Comments are stripped by the scanner — documentation only, the enforced string set is unchanged (entries reordered to group the justified keeps).
Per-entry status
inmemory-turn-stateauthority; needs a no-livelock concurrency proof + a built filesystem variant, some cross-crate).InMemoryBoundedSubagentGoalStore,InMemoryOpenAiCompatRefStore— JUSTIFIED bounded caches (capacity-bounded evict-oldest / bounded-LRU + filesystem-free contract boundary), NOT persistence debt; a durable variant would be semantically wrong.InMemoryExtensionInstallationStore— BLOCKED cross-crate (theFilesystemvariant in composition depends on a composition-internal contract registry; can't move down toironclaw_extensions).InMemorySecretStore— security-sensitive.InMemorySessionStore— BUILD-FIRST (no filesystem variant; auth-adjacent).Also reconciles the module-doc "definition of done" to note the two justified caches (a future PR may formally split them into a justified-keep list).
Verification
No production code changes. Ratchet self-tests + the frozen-set contract still pass (4 tests); fmt + pre-commit clean.
Stack
Stacked on #6214. Closes out the §4.3 store-consolidation axis with documented evidence of the achievable floor.
🤖 Generated with Claude Code