Skip to content

feat(memory): model memory as a userland extension + host-managed lifecycle — implements #3537 - #6345

Merged
BenKurrek merged 52 commits into
mainfrom
stack/memory/01-userland-extension
Jul 24, 2026
Merged

BenKurrek merged 52 commits into
mainfrom
stack/memory/01-userland-extension

Conversation

@BenKurrek

@BenKurrek BenKurrek commented Jul 20, 2026 •

Copy link
Copy Markdown
Collaborator

Reborn: model memory as a userland extension + host-managed lifecycle — implements #3537

This PR subsumes and replaces #5205. It contains every commit from
reborn/memory-lift-followups plus one more — the host-managed memory
lifecycle (565b0d893, formerly #5327) — and is brought current with main.
#5205 has been closed in favor of landing everything here in one PR.

Implements the bulk of #3537 — Extension Manifest v2 architecture, source-aware trust, host-defined capability profiles + conformance, the memory profile-binding policy, and the always-on native document-store provider. It also includes a config-driven third-party document-store provider backed by a self-hosted, fully-local mem0 (mem0 OSS on localhost — no api.mem0.ai, optional key), demonstrating the provider is swappable entirely through config with no hardcoded native assumption (the host_runtime resolver names no concrete provider; construction follows the ironclaw_embeddings::create_provider idiom in composition). On top of that, it wires the host-managed retrieve-before / record-after memory loop live on the local-dev runtime path.

Memory is now a host-bundled userland extension that implements host-defined capability profiles, bound per profile through a fail-closed profile_id → extension_id policy — consolidated to a single, always-on native surface — with the host proactively surfacing memory into prompts and recording finished turns back into it.

End state: one always-on native memory extension

ironclaw.memory is a bundled v2 Extension Manifest (schema_version = "reborn.extension_manifest.v2") parsed from crates/ironclaw_host_runtime/assets/memory_native/manifest.toml and registered on the always-on first-party lane (the same lane as the builtin toolset) — not the catalog/lifecycle lane — so its tools are unconditionally available with no install/enable step. It declares four model-visible memory tools ironclaw.memory.{read,write,search,tree}; read/write implements memory.document_store.v1 (their schema refs match the profile's operation refs), and search/tree are native conveniences that implement no profile.

This replaces the prior two declarations of the one filesystem-backed provider — the live builtin.memory_* tools and a separate, dormant host_internal native manifest — with one. builtin.memory_* is removed; provider-swapping is governed by the document-store profile binding (config-driven), not by install/enable.

The live provider is filesystem-backed (it writes through Arc<dyn RootFilesystem>, i.e. it sits on top of the deployment's storage substrate — libSQL/Postgres/local-fs); input schemas are served inline (include_str! of the bundled asset files, the single source of truth) on the always-on lane rather than materialized. Behavior, I/O, the scoped /memory mount, and on-by-default availability are unchanged from builtin.memory_*; only the capability identity + derived tool names change.

#3537 machinery

  • Profile catalog + conformance (ironclaw_host_runtime::memory_profiles): the three host-defined contracts (memory.context_retrieval.v1, memory.interaction_log.v1, memory.document_store.v1) and the semantic conformance harness. The native read/write capabilities are proven to satisfy memory.document_store.v1.
  • Profile binding (MemoryBindingPolicy, fail-closed; the [memory] config section; MemoryServiceResolver single construction point): default-native, production rejects memory.disabled and unverified third-party bindings absent an (extension_id, profile_id, deployment_profile) admin override.
  • Provider-neutral facade + registry: ironclaw_host_runtime depends on the MemoryService trait and names no concrete provider; ironclaw_reborn_composition::memory_provider_factory is the sole namer of concrete providers, populating the MemoryServiceResolver. An architecture test (host_runtime_stays_memory_provider_neutral_and_only_composition_names_mem0) enforces this.
  • Config-driven mem0 provider (ironclaw_memory_mem0, feature-gated memory-mem0, off by default): a second MemoryService impl over an injected HTTP transport, self-hosted mem0 OSS only. A bound-but-unconfigured / not-compiled-in mem0 fails closed.

Host-managed memory lifecycle (formerly #5327 — the commit this PR adds on top of #5205)

Makes the previously-deferred context_retrieval / interaction_log flow live on the local-dev path:

  • Retrieve (two lanes, once per run) — ironclaw_loop_host::memory_context::load_memory_snippets_once seeds a query from the latest user message and fetches long-term (user-general, excluding threads/*) and short-term (threads/<thread_id>/) memory in disjoint lanes, concatenates them (short-term first) under a 4 KiB admission budget, and injects them into the prompt's "memory" section. A per-run OnceCell caches the outcome (including empty-on-failure) so a slow/down provider is hit at most once per run; any failure degrades to empty and never fails the turn.
  • Record (after each turn) — ironclaw_runner::after_turn_memory::AfterTurnMemoryRecorder fires at the run-end seam gated on Completed, reads the run's full ordered transcript, and hands it to MemoryService::record_interaction (the mem0 add(messages, metadata) shape). The host passes the data and lets the provider decide (verbatim / LLM-extract / nothing). Post-terminal and best-effort: failures are debug!-logged and swallowed.

Deferred (not stubbed; tracked in #5264 / #5013)

  • Production wiring of the lifecycle flow — the retrieve/record ports are wired on the local-dev runtime; the production graph passes None (the same optionality as user_profile_source), deferred under Reborn: wire production-graph composition for optional context sources (identity + profile) #5013.
  • SQL storage-port-backed native persistence — the reborn_memory_* dual-backend tables behind host.storage.sql_transaction.first_party. This is the one place the memory-provider seam and the storage substrate would meet; catalogued as deferred vocabulary. See docs/adr/0002-native-memory-uses-host-storage-ports.md.
  • Default flip + memory.semantic_search.v1 — unchanged from the issue's deferral (/memory data/API compatibility decision; host-mediated embedding port).

Validation (merge-to-main, 2026-07)

  • cargo fmt --all -- --check — clean
  • cargo clippy -D warnings — zero warnings across ironclaw_host_runtime, ironclaw_reborn_composition (test-support,libsql,memory-mem0), ironclaw_runner (libsql-secrets,libsql-restart-tests,webui-user-store), ironclaw_loop_host, ironclaw_architecture, ironclaw_reborn_cli
  • cargo check --workspace --tests — clean
  • cargo test -p ironclaw_architecture — green (incl. the mem0 provider-neutrality + boundary rules)
  • Targeted tests green: memory unit tests (cache+facade, memory_native, memory_binding, mem0 swap), runner after-turn-memory lifecycle tests, wiring_parity, planned_runtime_parts_shape, group_memory, golden_payload, first-party trace coverage
  • cargo deny check advisories — ok
  • Known pre-existing: 3 sandbox_process host_runtime tests fail locally only without Docker

🤖 Generated with Claude Code

BenKurrek and others added 30 commits June 25, 2026 21:54
…ce-wide allowlist

PR #5163 made only the memory rules allowlist-based; the other ~30
`BoundaryRule` entries stayed blocklists that under-enforce — they forbid
today's offenders but would silently admit a future internal dep (e.g.
`ironclaw_turns`, `ironclaw_product_workflow`, `ironclaw_reborn`).

Convert the whole harness to an allowlist:

- `BoundaryRule` now carries `allowed: Vec<&'static str>` instead of
  `forbidden`. The runner computes
  `forbidden = workspace_ironclaw_crates() - allowed - crate_name`, so any
  unlisted internal dependency now fails the boundary test.
- Each crate's `allowed` set is its actual normal `ironclaw_*` dependencies,
  matching the dependency guardrail documented in its CLAUDE.md/AGENTS.md.
- The three previously-inline allowlist rules (`ironclaw_host_api`,
  `ironclaw_memory`, `ironclaw_memory_native`) are folded into
  `boundary_rules()` so the test body is a single uniform loop.

Behavior-preserving on the current (correct) dependency graph; the win is
forward enforcement. Addresses serrrfirat's deferred thread on #5163
(discussion_r3468163078).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ontext

Before this change the native provider sanitized, wrapped, and hashed each
memory snippet, and the host only *asserted* the `Untrusted memory content:`
prefix in `admit_memory_context_snippet`. A future untrusted provider could
pre-attach that prefix (or pre-shape the snippet) and slip text past the host's
prompt-safety wrapper.

Move all model-visible shaping into the host so the provider can never bypass
prompt safety:

- `MemoryServiceContextSnippet` now carries RAW snippet text plus the resolved
  scope/path components (`tenant_id`, `user_id`, `agent_id`, `project_id`,
  `relative_path`) — no `snippet_ref`/`safe_summary`/`model_content`.
- The native `retrieve_context` ranks and scope-filters candidates, then returns
  them raw; it no longer sanitizes, truncates, hashes, or budgets. Removed the
  native `sanitize_snippet_text`, `truncate_to_char_boundary`,
  `validate_loop_safe_summary`, `memory_snippet_display_ref`, `feed_hash`,
  `collect_context_snippets`, the FNV/budget consts, and the prompt-envelope dep.
- The host `memory_context.rs` builds the `memory-snippet:*` reference via the
  canonical `ironclaw_turns::run_profile::memory_snippet_display_ref`, sanitizes
  + wraps the raw text (`sanitize_snippet_text` relocated here), validates through
  the loop's own `LoopSafeSummary` gate (collapsing the native denylist copy into
  one source of truth), and enforces the per-snippet (512B) + aggregate (4 KiB)
  budgets in the admission loop with the same break semantics the native
  `collect_context_snippets` used.

Behavior-preserving for the native provider: model-visible output is byte-for-byte
identical (same wrapping, same FNV trailing-separator
`memory-snippet:cb96ed00b13e6ae4` golden ref, same caps/ordering). New coverage
proves a provider that returns text merely starting with the untrusted prefix is
STILL re-sanitized + re-wrapped by the host
(`adapter_re_sanitizes_provider_supplied_untrusted_prefix`,
`sanitize_re_wraps_text_already_carrying_untrusted_prefix`), plus the legacy ref
golden lock and the host-owned aggregate-budget test.

Addresses serrrfirat's deferred security thread on #5163 (discussion_r3468163070;
ref-stability discussion_r3466587649).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…facade

Profile READS built the native repository/backend directly in
`user_profile_source.rs` and duplicated the scope/path decision
(`profile_scope_and_path` + `PROFILE_DOCUMENT_PATH`) that
`NativeMemoryService` already owns for WRITES (`profile_set`). That coupled
profile reads to the concrete native provider and left provider selection unable
to swap reads with the rest of the memory facade.

Add a provider-neutral profile read to the contract and route the host read
through it:

- New `MemoryService::profile_read(invocation) -> MemoryServiceProfileReadResponse`
  (raw document bytes) on the `ironclaw_memory` trait, with a native
  implementation that reuses the SAME `profile_scope_and_path` as `profile_set`
  — so the scope/path decision lives in exactly one place per provider.
- `MemoryBackedUserProfileSource` now holds `Arc<dyn MemoryService>` and reads
  via `profile_read`; the host keeps only the parse + 64 KiB size-cap +
  validation. Deleted the duplicate host `profile_scope_and_path` /
  `PROFILE_DOCUMENT_PATH` and their re-export.
- Production wiring uses the new `MemoryBackedUserProfileSource::from_filesystem`
  factory (host owns the native-provider choice, matching the memory capability);
  the composition layer keeps passing the workspace filesystem.

Behavior-preserving: the unit tests assert identical parse/validation outcomes
(now through a stub `MemoryService`), and the end-to-end
`user_profile_roundtrip` test still proves the agent-scoped write → user-scoped
read round trip, now through `MemoryService::profile_read`.

Addresses serrrfirat's deferred thread on #5163 (discussion_r3466587663).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n audit

Both deltas live in the (host-driven) context path and are intentional; this
commit records why so a future reader doesn't "fix" them back toward origin:

- Native `retrieve_context` uses `.with_vector(false)` while origin's
  prompt-context search left `vector=true`. `false` is correct for the FTS-only
  native backend (no embeddings wired; a vector request fails closed) and matches
  the native `search` method. Documented inline.
- Host `map_memory_service_error` maps a failed memory-scope build to
  `InvalidInvocation` (via the provider's `Input` kind) where origin used
  `Internal`. The arm is unreachable in practice — the host validates the context
  scope before calling `retrieve_context` — and `InvalidInvocation` fails closed
  on the same axis as query validation. Documented on the mapper.

Comment-only; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fest + binding policy (#3537)

Land the host-runtime side of the remaining #3537 milestones:

- M1: author the three memory CapabilityProfileContracts (context_retrieval,
  interaction_log, document_store) as host-defined code in `memory_profiles`,
  with repo conformance tests driving the real catalog through the
  `ironclaw_capabilities` harness.
- M2: register `host.storage.sql_transaction.first_party` + `host.events.audit`
  (new `ironclaw_host_api` constants) in `default_host_port_catalog()`.
- M3: bundle the `ironclaw.memory.native` v2 Extension Manifest (HostBundled,
  first_party runtime) under `assets/memory_native/`, parsed/backed from
  host_runtime so the manifest's `service` must match the registered native
  provider identity ("TOML alone is not authority"). Conformance + schema
  validation tests over the real bundled schemas.
- M4 (host side): fail-closed `MemoryBindingPolicy` (profile_id -> provider,
  default-native, production rejects disabled/unverified-third-party absent an
  (extension_id, profile_id, deployment_profile) override). The memory-tools
  dispatch site now consults the binding instead of hardwiring
  `NativeMemoryService::from_filesystem`; non-native bindings fail closed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the `[memory]` section to `RebornConfigFile` with `profile_bindings`
(profile_id -> extension_id) and `admin_overrides` (scoped to
(extension_id, profile_id, deployment_profile)). Validation is structural +
deployment-agnostic (non-empty fields, valid override deployment_profile or
`*`); profile-id validity and fail-closed production policy are owned by the
host-runtime binding resolver, which holds the profile catalog.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…3537)

Resolve the memory binding policy from the `[memory]` config section + the
deployment profile at startup (fail-closed: production rejects
memory.disabled / unverified third-party bindings without an override), and
thread the resolved document-store binding to the builtin first-party handler
registry on both the local-dev and production composition paths. The CLI
resolves the policy in `build_services_input_with_options` and attaches it to
`RebornBuildInput`; active third-party overrides are logged (redacted) at
`debug!`. Replaces the hardwired native provider selection at the dispatch
site with a config-driven, profile-bound resolution.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-profiles status (#3537)

Add the two ADRs the issue references (0001 Extension Manifest v2 hard
cutover, 0002 native memory uses host storage ports) and move
memory-profiles.md from "draft zero-behavior" to Active, with an
Implemented/Deferred split. Documents the gated remainder explicitly: the
reborn_memory_* dual-backend SQL tables + concrete storage-port adapter +
scoped HostPortView into the handler (boundary: composition crates cannot
depend on the root ironclaw crate where the SQL backends live), and the
default flip (blocked on /memory data + API compatibility tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… profile binding, fail-loud)

Apply the substantive + cheap review findings from the bot review pass:

- Scope-equality guard (security): the provider-neutral snippet admission path
  (`memory_context::admit_memory_context_snippet`) now drops any snippet whose
  tenant/user/agent/project scope does not match the request scope before it is
  hashed or admitted. Native filters earlier, but a pluggable/third-party
  provider must not inject cross-scope content. Adds drop/keep tests.
- Profile reads honor the binding: the local-dev user-profile source now builds
  the native-backed reader only when the document-store profile is bound to
  native, degrading to empty otherwise — so profile reads stay consistent with
  the memory tools instead of silently staying native. The resolved binding is
  carried on the local-dev store-graph input / local-runtime services.
- Fail-loud: `document_store_binding` returns `Result` and surfaces a missing
  document-store binding instead of silently falling back to native.
- Char-safe redaction: `MemoryActiveOverride::redacted_summary` truncates by
  characters, not bytes (the byte slice could not panic — id is ASCII — but the
  char form follows the repo rule and is encoding-robust).
- More schema coverage: valid/invalid instance fixtures for document-read,
  document-write, and interaction-record (previously only context-retrieve).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mer (#3537)

Collapse the per-call-site memory provider construction into one resolver.
Before, the memory tools and the user-profile reader each called
`NativeMemoryService::from_filesystem` and re-checked the binding inline, so
the "which provider, and is it permitted?" decision was duplicated.

`MemoryServiceResolver` (host_runtime::memory_provider) is now that decision in
one place: given a profile + per-invocation inputs (filesystem + optional
prompt-write-safety sink) it resolves the bound provider or returns None
(fail-closed) for disabled / unimplemented-third-party bindings. It wraps
`Option<MemoryBindingPolicy>` (None = native default) so it is Default and the
tool structs that hold it need no fallible constructor.

- Memory tools (MemoryCapabilityState) hold a resolver and build their service
  through it; they no longer reference NativeMemoryService or MemoryProviderBinding.
- The local-dev user-profile source builds through the same resolver (native
  → MemoryBackedUserProfileSource, disabled/third-party → EmptyUserProfileSource).
- The context retriever already takes an injected service and draws from the
  resolver once production-wired.
- Composition threads one `MemoryServiceResolver` (built once per runtime from
  the resolved policy) instead of a bare `MemoryProviderBinding`; the
  `document_store_binding` helper is removed.

Behavior-preserving for the native default; verified by an adversarial review
(completeness / fail-closed / behavior-preservation / dead-code) plus the
existing memory + user_profile_roundtrip suites. fmt + clippy clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ry.native package

Collapse the two capability declarations of the one filesystem-backed memory
provider into one. The model-facing memory tools (read/write/search/tree) now
belong to a dedicated `ironclaw.memory.native` first-party package on the same
always-on lane as `builtin` (registered directly into the builtin extension
registry, not the catalog/lifecycle extension lane), replacing the anonymous
`builtin.memory_*` capabilities.

- read/write `implements` the `memory.document_store.v1` profile; search/tree
  are native conveniences that implement no profile.
- Input schemas are served inline by `resolve_native_memory_input_schema_ref`
  via a provider-keyed `surface.rs` branch — no asset materialization, mirroring
  the builtin package.
- The package is trusted (per-turn `provider_trust` insert + a first-party
  `AdminEntry`) and granted the /memory mount via the local-dev capability
  policy, exactly as the builtin memory tools were.

Provider-swapping stays on the document-store profile binding. Behavior, I/O,
data, and on-by-default availability are unchanged; only the capability identity
and the derived model tool names change (builtin__memory_* ->
ironclaw__memory__native__*).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t (honor #3537)

Path 1 code-constructed the native package in Rust; #3537 explicitly wants a
bundled v2 Extension Manifest. This reworks native memory to parse the bundled
`assets/memory_native/manifest.toml` and register the resulting package on the
SAME always-on first-party lane (not the catalog/lifecycle lane), so it stays
unconditionally available with no install/enable step — a v2 manifest on the
always-on lane.

- The manifest is reshaped from the dormant host_internal/SQL form into four
  model-visible memory tools: `read`/`write` implement `memory.document_store.v1`
  (their schema refs match the profile op refs); `search`/`tree` are native
  conveniences. No required host ports (filesystem-backed); the SQL/audit ports
  stay catalogued vocabulary for the deferred SQL milestone.
- `memory_native_extension::native_memory_first_party_package()` parses the TOML
  (via `ExtensionManifestRecord`) into an `ExtensionPackage`; the composition
  registry insertion + trust + /memory grant from the prior commit are reused
  unchanged (same native capability ids).
- Input schemas are served inline on the always-on lane via `include_str!` of the
  bundled asset files (the single source of truth) — no materialization. Prompt
  docs are added (required for model visibility) and bundled likewise.
- Conformance tests updated to the lean scope: native satisfies
  `memory.document_store.v1`; the context_retrieval/interaction_log profiles
  remain defined for the deferred host-managed flow with no live implementer.
  `NATIVE_MEMORY_FIRST_PARTY_PROVIDER` now aliases the canonical
  `NATIVE_MEMORY_EXTENSION_ID` (single identity source).

Behavior, I/O, data, and on-by-default availability remain unchanged from the
prior commit; this changes the manifest authoring form (code -> bundled v2 TOML).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update memory-profiles.md and ADR 0002 to reflect that ironclaw.memory.native is
now live: its bundled v2 TOML manifest is parsed and registered on the always-on
first-party lane, implementing memory.document_store.v1 via model-facing
read/write tools (search/tree are native conveniences). The live provider is
filesystem-backed and declares no host ports; the SQL/audit ports stay catalogued
for the deferred SQL-backed milestone. The context_retrieval/interaction_log
profiles remain defined with no live implementer (deferred host-managed flow).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… ids

The projection display-preview summarizer keys on the `memory_<op>` short names
via `capability_matches`, which matched `builtin.memory_<op>` by suffix. The
native tools are now `ironclaw.memory.native.<op>` (suffix `.<op>`, not
`.memory_<op>`), so memory input summaries — including write-content secret
redaction — would have silently stopped rendering in production. Teach
`capability_matches` the new id shape and update the projection test fixtures to
the native ids so they exercise it. Also rename the now-misnamed
`builtin_memory_search_dispatches_*` host_runtime test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…untime

The native-memory consolidation (ffbd298, dc2fbb5) moved the memory_*
capabilities out of the builtin package into the always-on
`ironclaw.memory.native` package and reshaped the bundled input schemas to the
four live document-store tools, but the host_runtime test suite + two schemas
were left asserting the old shape — leaving `Test ironclaw_host_runtime` red on
18 memory tests.

- first_party_builtin_tools.rs: resolve the memory capabilities from the native
  package (register it in the test registry + add its first-party trust entry)
  and drop the memory ids from `all_builtin_capability_ids`.
- memory_native_schema_validation.rs: validate the four live tool schemas
  (read/write/search/tree); the removed context-retrieve / interaction-record
  schemas belong to the deferred host-managed flow and are no longer bundled.
- search.input.v1.json: accept the `q`/`text`/`pattern` aliases that
  `MemoryServiceSearchRequest::from_tool_input` already honors (anyOf), so a
  model call using an alias is not rejected at the pre-dispatch schema boundary.
- document-read.input.v1.json: reject empty and absolute paths at the schema
  (minLength + `^[^/]` pattern), matching the scoped-path contract.
- ironclaw_memory service.rs: restore the pre-lift null-target handling — an
  explicit JSON `null` write target is treated as omitted (daily_log) rather
  than rejected, the #4547 behavior the lift had regressed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ackage

Moving the memory_* capabilities into the `ironclaw.memory.native` package left
two root-crate tests red, because the e2e harness and the e2e coverage allowlist
still assumed memory lived in `builtin`:

- reborn_trace_first_party_tool_coverage.rs: build the covered-capability set
  from the union of the builtin and native-memory packages, so the always-on
  first-party surface (which now spans two packages) is fully checked.
- tests/support/reborn/harness.rs: register `native_memory_first_party_package`
  in the core-builtins runtime (via a shared `core_builtins_extension_registry`
  so the two core-builtins runtimes cannot drift), trust the native provider at
  the host-policy and per-run authority levels, and scope memory grants to
  filesystem effects so they fit the native provider's tight authority ceiling.

Fixes `reborn_builtin_first_party_capability_e2e_coverage_is_complete` and
`reborn_trace_memory_first_party_tools_parity` on `Reborn root tests` and
`Tests (all-features)`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the memory document-store provider swappable to mem0 entirely through
config, with no hardcoded native assumption in the kernel — demonstrating the
#3537 memory architecture is genuinely pluggable. Follows the established
`ironclaw_embeddings::create_provider` config-driven-factory idiom.

- ironclaw_host_runtime/memory_provider.rs: remove the hardwired
  native-or-none logic; `MemoryServiceResolver` is now a provider-agnostic
  registry (`BTreeMap<extension_id, Arc<dyn MemoryService>>` + a
  `with_third_party_document_store_provider` builder). `resolve_document_store`
  matches the binding (Native -> build native / ThirdParty(id) -> registered
  instance, None if unregistered / Disabled -> None). It names no concrete
  third-party provider, so host_runtime keeps zero provider deps.
- crates/ironclaw_memory_mem0: new provider crate implementing MemoryService
  over the mem0 REST API. Real reqwest transport behind a `Mem0Transport`
  trait (SSRF-checked base URL, `Authorization: Token` header) with a
  panic-free mock for tests. Depends only on ironclaw_memory + ironclaw_host_api.
- ironclaw_reborn_composition: `create_document_store_provider(binding, deps)`
  factory (embeddings idiom: match Native/ThirdParty/Disabled, build mem0 over
  its real transport with check_base_url, fail-closed None on missing creds)
  plus `build_memory_service_resolver` that registers third-party providers;
  wired into all three resolver-construction sites in factory.rs.
- Config: `[memory] mem0_base_url` + env MEMORY_MEM0_{API_KEY,BASE_URL,APP_ID}
  (API key as SecretString), mirroring EmbeddingsConfig.
- Tests: end-to-end swap proof (config -> policy -> factory -> registry ->
  resolve_document_store returns mem0, not native; write+search route through
  the mem0 mock transport), mem0 unit tests, and a caller-level tool-dispatch
  test proving the unchanged ironclaw.memory.native.* tools transparently route
  to mem0 under a binding. Architecture boundary allowlist updated for the new
  crate (composition may depend on it; host_runtime may not).

Scope: document-store swap only. The host-managed retrieve/record lifecycle and
SQL storage-port backing remain the deferred #5264 follow-ups.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a full-pipeline (`load_memory_snippets`) negative test proving the host
drops memory-context snippets whose resolved tenant/user scope does not match
the request scope, even when the provider returns them — keeping only the
in-scope snippet. The scope guard was unit-tested at the `admit_*` level; this
exercises it end-to-end against a malicious or buggy provider, which is now a
live possibility with config-bound third-party providers like mem0 (#5264).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-target the mem0 document-store provider from mem0's hosted cloud API to a
self-hosted mem0 open-source instance on localhost — no dependency on
api.mem0.ai and no cloud API key. Proven end-to-end against a real local stack
(mem0ai + Qdrant + an Ollama embedder): store -> search-recall -> verbatim read
round-trips, with the data physically in the local vector store.

- transport.rs: the API key is now optional (`Option<&str>` — the auth header is
  sent only when a key is present); add `.timeout(30s)` + `.redirect(none)`
  (review hardening, finding #2).
- service.rs: local OSS paths (`/memories`, `/search`, `GET /memories?user_id=`)
  instead of the hosted `/v1/memories/...`; `add` sends `infer:false` so mem0
  stores content verbatim (document-store semantics need only the embedder, not
  the extraction LLM). `profile_set` is now field-preserving (read-merge-write,
  latest selected by `created_at`) instead of last-writer-wins (review finding
  #1 — no more silent profile-field loss); merge + infer-false unit tests added.
- lib.rs: extension id `mem0.cloud.memory` -> `mem0.local.memory`; docs rewritten
  to the local OSS surface.
- composition factory: build the provider with an optional key (no longer fails
  closed on a missing key); reborn_cli defaults `MEMORY_MEM0_BASE_URL` to
  `http://localhost:8888`; config doc updated.
- swap-test fixtures updated to the local API; new `tests/live_local_mem0.rs`
  (`#[ignore]`'d) drives the real transport against a running local mem0.

The document-store swap is proven at the provider level. The full LLM-agent loop
using memory remains blocked on this branch by the pre-existing #5206
worker-pool stall (fixed on main) and is a tracked follow-up (#5264).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the `http://localhost:8888` default for the mem0 connection base URL so
mem0 is fully opt-in: it activates only when an operator both binds the
document-store profile to it AND supplies a base URL (the `[memory]` config or
`MEMORY_MEM0_BASE_URL`). A bound-but-unconfigured mem0 fails closed in the
factory, and the binding policy already defaults to native — so the shipped
default memory layer is unchanged (native filesystem); mem0 never engages
unless explicitly configured.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…loud, tests)

From a 5-agent end-to-end review of the PR. All low-risk:
- mem0 provider: reject embedded-credential base URLs (redacted in the error) and
  run `memory.mem0_base_url` through the config inline-secret guard; correct the
  stale "defaults to localhost:8888" doc-comments (there is no default — a
  bound-but-unset mem0 fails closed); add the missing `created_at` newest-profile
  test; fail loud (`CorruptProfile`) instead of silently dropping fields when an
  existing profile blob is unparseable; add the `// silent-ok:` annotation and
  drop cloud (`api.mem0.ai`, `/v1/`) remnants from test/doc strings.
- reborn_cli: `optional_nonempty_env` fails loud on a non-UTF-8 value
  (NotPresent -> None, NotUnicode -> Err) for the three MEMORY_MEM0_* reads.
- reborn_config: deployment-profile validation uses `RebornProfile::from_str`
  instead of matching string literals (types.md).
- architecture: add a boundary-test guard asserting host_runtime stays
  memory-provider-neutral (only composition may name `ironclaw_memory_mem0`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A third-party document_store binding (e.g. mem0) does not get write-time prompt-write-safety enforcement or per-write audit, since that engine lives inside the native provider. Spell out the security limitation in resolve_document_store + why it is acceptable for the off-by-default surface (third parties cannot reach the trusted prompt surface; all retrieved content is host-wrapped untrusted), and that hoisting it host-side is deferred to #5264.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (off by default)

mem0 is now opt-in at build time, mirroring ironclaw_llm/root-llm-provider: ironclaw_memory_mem0 is an optional dependency enabled by a new `memory-mem0` feature on composition, so a default build carries no mem0 code or its reqwest/rustls transport. The factory's mem0 construction (plus its test seam, tests, and the swap integration test) are #[cfg(feature = "memory-mem0")]; a mem0 binding fails closed when the feature is not compiled in. The architecture boundary test asserts the gating (mirroring root-llm-provider), and CI runs the composition suite with the feature so the mem0 tests still execute (feature-off stays covered by the --no-default-features composition run in test.yml).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… schemas, secrets)

From CodeRabbit's full re-review of the rebased PR. All low-risk:
- mem0 provider: read fragments now sort by created_at (mem0 list order is not
  chronological) so append-style docs read back in order; a replace write
  (append=false) is rejected as an Unsupported operation instead of silently
  becoming an add that misreports append:true; check_base_url fails closed on
  hosted mem0 cloud hosts (mem0.ai / *.mem0.ai), enforcing self-hosted-OSS-only;
  the InvalidUrl error no longer echoes the configured URL (drops host/query),
  keeping only the cause.
- reborn_config: mem0_base_url is validated non-empty + trimmed (check_non_empty_trimmed).
- native memory schemas: tree.input rejects absolute / .. / backslash paths (fail
  closed, empty root still allowed); document-read.output requires word_count;
  search.input requires a non-empty query and forbids conflicting aliases (oneOf).
- docs: memory-profiles.md non-goals updated (mem0 provider now exists, off by
  default, feature-gated); host_port.rs docstring no longer over-claims native
  backing (native memory is filesystem-backed, declares no host ports).
- memory_binding: regression test locking that a case variant of the native id
  fails closed (ExtensionId grammar is lowercase-only) rather than misclassifying.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, fail-loud, docs)

From CodeRabbit's fresh full re-review. Most notable: the native memory JSON
input schemas are model-facing (advertised in parameters_schema) and are NOT
host-validated against actual tool arguments before dispatch, so a traversal
target could reach a provider verbatim. Added a provider-neutral
reject_out_of_scope_target guard in MemoryServiceWriteRequest::from_tool_input
(mirrors the schema pattern) so every bound provider -- including mem0, which
stores the target verbatim -- gets containment, not just native.

Also:
- document-write.input schema rejects absolute / .. / backslash targets (fail
  closed, matching the sibling schemas).
- mem0 response_items fails loud (UnrecognizedResponse) on an unrecognized 2xx
  body instead of silently returning empty (which let a malformed list response
  overwrite existing profile fields).
- docs: manifest description scoped (search/tree implement no portable profile);
  extension_contracts + transport docstrings corrected (native is filesystem-
  backed / declares no host ports; non-JSON bodies degrade to Null, not an error).

The mem0 replace-write rejection is provider-specific (mem0 OSS is append-only);
native still supports replace, so the shared write prompt is left unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the mem0 feature-gate: ironclaw_reborn_cli now exposes a memory-mem0 feature that enables ironclaw_reborn_composition/memory-mem0, so an ironclaw-reborn binary built with --features memory-mem0 can bind memory.document_store.v1 to a self-hosted mem0 server. Without it the feature was only reachable on the composition crate, never the actual binary. Mirrors the existing root-llm-provider / webui-v2-beta forwarding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…llowups

# Conflicts:
#	crates/ironclaw_reborn_cli/Cargo.toml
…ded DoS)

New advisory on lopdf (via pdf-extract in ironclaw_extractors, PDF attachment extraction) with no patched release in our semver range yet. Bounded DoS only -- a crash of the extraction task on a hostile user-supplied PDF, not RCE and not the whole process. Matches the existing deny.toml ignore pattern; remove once lopdf/pdf-extract ship a fixed release. Unrelated to the memory work; surfaced because the main merge re-ran cargo-deny.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e + scope into user_id)

Local-dev native memory is isolated per workspace (its filesystem store lives under
local_dev_root); mem0 (a shared server) was not, since the local-dev runtime uses a
fixed scope. mem0 OSS enforces search/get_all filtering by user_id (and agent_id) but
NOT by app_id -- a top-level app_id is accepted yet silently ignored when filtering
(verified empirically: cross-app_id queries leak). So we encode the entire partition
into the one key guaranteed enforced:

- The provider folds the workspace partition (config.app_id, set per-workspace by the
  local-dev composition from local_dev_root) into the user_id namespace at all 7
  namespace sites (search/write/read/tree/profile-read/profile-set/retrieve_context).
  app_id is still stamped as forward-compat metadata but is NOT relied on for isolation.
- The local-dev composition derives a per-workspace app_id from the canonical local-dev
  root; production is untouched (no app_id -> pure scope namespace, so memory persists
  across restarts for the same scope).

Result: local-dev mem0 partitions like native (workspace x tenant/user/agent/project).
Verified by a live cross-isolation test (cross-workspace AND cross-scope both return
zero on search and list) + a regression guard locking the user_id prefix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…law.memory.*)

The bundled memory extension exposed its model-facing tools as
`ironclaw.memory.native.*`, leaking the default provider's name into the one
surface that must be backend-blind — the agent's tool list. With the mem0 swap
the model still saw `ironclaw__memory__native__*` while running on mem0,
contradicting #3537's agnostic goal.

Rename the agent-facing extension id + capabilities to `ironclaw.memory.*`
(model now sees `ironclaw__memory__{read,search,write,tree}`) and neutralize the
manifest name/description. "native" is kept only where it's true — the internal
default provider: `native_memory_provider` service, `ironclaw_memory_native`
crate, `NativeMemoryService`, the bundled asset/prompt dirs.

The last dot-segment (read/search/write/tree) is preserved, so the benchmark
retrieval matcher (keys on `rsplit('.').next()`) is unaffected.

Tests: host_runtime memory/manifest/surface, capability-profile conformance, and
reborn_composition projection all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…composition tests + re-bless surface hash

Two stragglers from the memory-package rename that only fail in CI
lanes wider than --lib: webui_v2_e2e asserted builtin.memory_write is
visible in the local-dev capability surface (composition-core bucket),
and the golden payload snapshots pinned the pre-origin-gate-matrix
surface sha256 (integration coverage lane 3). Rename the ids and
re-bless; the snapshot diff is exactly the surface-hash token — the
capability list, names, and descriptions are unchanged. Also rename the
opaque id in the local_dev result-staging test for consistency.

[skip-regression-check] test-only rename + snapshot re-bless; the
renamed assertions are themselves the regression coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W1q1QYdRY3XRg6RnYLZdNw
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6345 July 24, 2026 15:34 Destroyed
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6345 July 24, 2026 15:45 Destroyed

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
crates/ironclaw_reborn_composition/src/runtime.rs (1)

3957-3969: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Keep memory lifecycle off non-local runtimes.

local_runtime is assigned Some(&services) at Line 3513 for every profile, so this new branch is not a local-development guard. Whenever the binding resolves to native, hosted/production builds can wire prompt memory and after_turn_memory_writer, causing production turns to read/write memory despite the local-dev-only lifecycle contract. Gate this on the actual local-development substrate/deployment and add a production-profile caller regression test proving these lanes stay absent.

As per path instructions, side-effect gates must be tested through the production caller; as per coding guidelines, every Rust bug fix requires a regression test.

Also applies to: 4104-4133

🤖 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_reborn_composition/src/runtime.rs` around lines 3957 - 3969,
Gate the shared resolved_memory_document_store flow and the related
prompt-memory/after_turn_memory_writer setup around the actual local-development
substrate or deployment predicate, not merely local_runtime being Some. Ensure
hosted/production callers cannot read or write memory through these lanes, while
preserving native local behavior, and add a regression test through the
production-profile caller verifying both lanes remain absent.

Sources: Coding guidelines, Path instructions

tests/reborn_qa_smoke_scenarios_e2e.rs (1)

671-677: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Make the async-test wrapper honor the documented 64 MiB stack requirement.

tests/reborn_qa_smoke_scenarios_e2e.rs documents RUST_MIN_STACK=67108864, but run_async_test_with_stack bypasses that default by calling std::thread::Builder::stack_size(16 * 1024 * 1024) at tests/reborn_qa_smoke_scenarios_e2e.rs:1014. This contradicts the file/module invariant and can still overflow at the tested stack height for both qa_extension_lifecycle_tools_search_install_and_remove_e2e and qa_installing_bundled_extensions_exposes_complete_model_surface_e2e. Set the helper to 64 * 1024 * 1024 or remove the explicit override.

🤖 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 `@tests/reborn_qa_smoke_scenarios_e2e.rs` around lines 671 - 677, The
run_async_test_with_stack helper overrides the documented 64 MiB minimum stack
with 16 MiB. Update run_async_test_with_stack to use a 64 * 1024 * 1024 stack
size, or remove its explicit stack-size override so the documented
RUST_MIN_STACK default is honored.
🤖 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.

Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 3957-3969: Gate the shared resolved_memory_document_store flow and
the related prompt-memory/after_turn_memory_writer setup around the actual
local-development substrate or deployment predicate, not merely local_runtime
being Some. Ensure hosted/production callers cannot read or write memory through
these lanes, while preserving native local behavior, and add a regression test
through the production-profile caller verifying both lanes remain absent.

In `@tests/reborn_qa_smoke_scenarios_e2e.rs`:
- Around line 671-677: The run_async_test_with_stack helper overrides the
documented 64 MiB minimum stack with 16 MiB. Update run_async_test_with_stack to
use a 64 * 1024 * 1024 stack size, or remove its explicit stack-size override so
the documented RUST_MIN_STACK default is honored.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 99679e6d-3474-4790-82c2-3771c2485bc3

📥 Commits

Reviewing files that changed from the base of the PR and between dd39617 and 60333f1.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock, !**/Cargo.lock
📒 Files selected for processing (10)
  • Cargo.toml
  • crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs
  • crates/ironclaw_reborn_composition/Cargo.toml
  • crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
  • crates/ironclaw_reborn_composition/src/factory.rs
  • crates/ironclaw_reborn_composition/src/lib.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/tests/webui_v2_e2e.rs
  • docs/plans/composition-pubuse.snapshot
  • tests/reborn_qa_smoke_scenarios_e2e.rs
💤 Files with no reviewable changes (1)
  • docs/plans/composition-pubuse.snapshot

…in-generated-code contract

Main's #6618 refined the extension_search sanitizer to RETAIN a
generated-code channel's setup guidance (blanking only the static
failure copy) and pinned that in a unit test — but left the integration
test asserting the old strip-everything contract. The contradiction was
invisible on main because main's tip cannot compile the integration-test
closure at all: #6618 renamed the RebornRuntime field to
`_channel_host_assembly` but missed the test-support-gated accessor
(`active_channel_preference_codec_ids_for_test`), so every
integration-tier suite fails at compile and all five coverage lanes are
red on main. This branch already carries the accessor fix from the
catch-up merge; this commit carries the test-contract fix: telegram's
web_generated_code guidance must remain model-visible with
"IronClaw pairing panel" instructions and a blanked error_message,
mirroring model_visible_extension_search_projects_generated_code_without_ui_failure_copy.

[skip-regression-check] test-only contract alignment; the rewritten
assertions are the regression coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W1q1QYdRY3XRg6RnYLZdNw
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6345 July 24, 2026 16:04 Destroyed
BenKurrek and others added 2 commits July 24, 2026 12:11
…rface

Verified each open review thread against current code; fixes for the
still-valid ones:

- Harness trust parity (security): the integration harness granted the
  ironclaw.memory provider the FULL builtin effect set
  (Network/SpawnProcess/ExecuteCode/...); production grants only
  dispatch + filesystem. Narrow core_builtin + qa_smoke profiles to the
  production ceiling so harness runs can't mask authority-ceiling
  denials production would enforce.
- mem0 retrieve_context now filters the kind=profile record out of
  context snippets (profile JSON must not enter the prompt as a memory
  snippet; profile state has its own read path) + regression case.
- Caller-level proof of the retrieve-before lane: drive the REAL
  LoopContextPort::load_loop_context twice with a recording
  MemoryPromptContextService — asserts the query is the latest user
  message, snippets surface on the bundle, and the fetch happens once
  per run (cache reuse).
- [memory] manifest validation regression tests: non-first-party
  runtime, empty operations, and missing document_store all fail closed
  (+ a parsing baseline for the provider-only shape).
- surface.rs: mirrored fail-closed test — a native-memory descriptor
  without an input schema ref is rejected like a builtin one.
- Origin-gate ratchet now asserts BOTH host-bundled memory manifests are
  actually scanned (a moved manifest can no longer silently drop out).
- document-read input schema mirrors write/tree's stricter path
  not-pattern (blank/absolute/traversal/backslash); tree output schema
  types its items as path strings.

Verified-invalid threads (no change): the "duplicate [[tools]] headers"
critical is a diff-context misread (one header per tool; the package
parse is pinned by tests); the memory_provider_factory
CapabilityProfileId silent-drop refers to code removed with the
capability-profile vocabulary retirement. Deferred: mem0 unbounded list
pagination (off-by-default provider; needs a mem0 API paging design).

[skip-regression-check] review-driven test hardening; each behavioral
fix above carries its regression case in the same commit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W1q1QYdRY3XRg6RnYLZdNw
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6345 July 24, 2026 16:28 Destroyed
@BenKurrek
BenKurrek merged commit d06bde9 into main Jul 24, 2026
57 of 60 checks passed
@BenKurrek
BenKurrek deleted the stack/memory/01-userland-extension branch July 24, 2026 16:38
BenKurrek added a commit that referenced this pull request Jul 24, 2026
The three golden payload suites pin the full model-visible system prompt,
including the capability-surface sha256. #6345 refreshed these snapshots on
its own branch, but the surface hash of merged main differs (a sibling
change altered the underlying tool-definition surface), so main's tip fails
golden_context_surfacing, golden_tool_call_feedback, and
golden_parallel_tool_calls with a hash-only diff — the rendered capability
list and every other byte of the payload are unchanged. Accept the merged
surface hash per the documented cargo-insta drift flow in
tests/integration/support/golden.rs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek added a commit that referenced this pull request Jul 28, 2026
* refactor(extensions): normalize filesystem state records

* test(reborn): refresh golden payload surface hashes for merged main

The three golden payload suites pin the full model-visible system prompt,
including the capability-surface sha256. #6345 refreshed these snapshots on
its own branch, but the surface hash of merged main differs (a sibling
change altered the underlying tool-definition surface), so main's tip fails
golden_context_surfacing, golden_tool_call_feedback, and
golden_parallel_tool_calls with a hash-only diff — the rendered capability
list and every other byte of the payload are unchanged. Accept the merged
surface hash per the documented cargo-insta drift flow in
tests/integration/support/golden.rs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(reborn-tests): give group suites the root-tests stack headroom

reborn_group_triggers::triggered_gate_group overflowed the 8 MiB libtest
default stack in the group-tests job on Linux while its measured stack need
is unchanged from base (both sit in the same 1.875-2.0 MiB threshold bucket
on macOS debug): the suite runs a full Reborn runtime per scenario on the
test-thread stack and rides within a few percent of the limit, so any
unrelated code-layout shift can tip it over. The root-tests and coverage
jobs already set RUST_MIN_STACK for exactly this pathology; align the
group-tests job with the root-tests value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(extensions): close v2 store review findings on unreserved sweeps, projection failures, and health writes

Three review findings on the normalized v2 store, each with a regression
test in installations_contract.rs:

- Creation/reactivation (and compensation restores) hold no core
  reservation, so the component writer must not sweep child rows omitted
  from its aggregate: a concurrent creator's just-activated membership
  would be tombstoned while both install calls report success. Component
  sync is now mode-split (V2ComponentSync); only a reserved update of an
  active core sweeps omitted rows.
  Test: concurrent_creation_does_not_tombstone_the_other_creators_membership

- Once the v2 records (the authority) commit, a failed legacy
  compatibility-projection write no longer fails the operation — callers
  compensated against a live install and left a ghost (store says
  installed, runtime/files torn down). The projection is repaired from v2
  at the next startup.
  Test: aggregate_projection_write_failure_does_not_fail_a_committed_v2_install

- update_health now writes only the health row instead of rewriting the
  whole aggregate through the reservation path, so it can no longer race
  a final removal into resurrecting a removed core, and it matches the
  plan's health-isolation contract.
  Test: update_health_mutates_only_the_health_row

Also documents the merge/health/projection semantics and enforcing test
commands in docs/reborn/contracts/extensions.md, the deliberate
at-least-once join outcome during a removal reservation in-code, and
fixes the plan doc's stale integration-test filter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(extensions): pin one membership row per creator; docs: name the manifest row an install pin

Strengthen the concurrent-creation regression test per review: assert the
row count before the per-user status projection so a duplicate row could
never hide behind the map. Clarify in the extensions contract that the
manifest row is the hash-pinned copy of the installed package definition,
not an availability or policy declaration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(extensions): merge the install pin into the installation record and derive lifecycle state

Consolidates the v2 schema before first deployment (the last
migration-free moment), per review discussion:

- The separate manifest record folds into the installation record as an
  embedded, hash-pinned install pin; manifest_ref is derived from it so
  an installation can never disagree with its definition, and a
  free-floating definition row cannot exist.
- The three-value status enum dissolves into derived state: removed_at
  is the tombstone, an explicit V2MutationLease (member: Option) is the
  exclusive mutation gate, and a record with neither is live. The
  unexposed core installed_at field is dropped; memberships keep theirs.
- Solo upsert_manifest leaves the port: installs go through the combined
  manifest+installation write, and persist_install_plan's manifest-orphan
  compensation deletes itself. delete_manifest remains as the idempotent
  convergence verifier.
- The legacy removal-cleanup tombstone semantics are preserved as a
  first-class derived state: delete_installation marks
  removal_cleanup_pending, keeping the embedded definition authoritative
  for get_manifest so interrupted cleanups retry without the catalog and
  imports stay blocked until delete_manifest marks convergence.
  persist_removal_tombstone seeds the same state for orphan cleanups,
  and bootstrap imports orphan legacy manifest rows as pending
  tombstones so interrupted v1 removals stay retryable after migration.
- upsert_installation validates against the record's own pin, falling
  back to the extension's authoritative definition for legacy-shaped
  additional rows.
- cas_update's retry loop is boxed off caller stacks: the merged
  record's larger CAS state inlined into every store future and
  overflowed default test-thread stacks in debug builds
  (reborn_group_triggers reproduced it; it passes at default again).

Contract coverage: the full installations_contract suite is adapted to
the merged layout and extended with
removal_tombstone_keeps_manifest_authoritative_until_convergence; the
composition cleanup-tombstone tests, the multi-row canonicalization
integration test, and the restart journey pass unchanged in intent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(extensions): make lease recovery child-authoritative and import orphan legacy manifests

Closes the review findings on the consolidated schema, each with a
regression test:

- Bootstrap imports an orphaned legacy manifest row (the legacy flow's
  durable removal-cleanup marker) as a cleanup-pending tombstone, so an
  interrupted v1 removal stays retryable without the catalog after
  migration and imports stay blocked until convergence.
  Test: bootstrap_imports_orphan_legacy_manifest_as_cleanup_pending_tombstone
  (the previous attempt at this import was lost to a silently failed
  scripted edit; the new test pins the migration path directly).

- A missing compatibility snapshot is no longer treated as proof of
  removal during lease recovery — projection writes are best-effort, so
  a live record can legitimately lack one. The child rows are the
  authority: surviving active membership (or a legacy tenant owner)
  clears the lease over the children as they stand; only a record whose
  memberships were all tombstoned rolls forward to removed.
  Test: interrupted_update_without_compatibility_snapshot_stays_live_on_reopen

- repair_compatibility_views skips a lease taken by a live peer between
  startup passes instead of failing the whole store open.

- persist_removal_tombstone returns the retryable
  MembershipMutationInProgress for a leased live record, so the
  orphan-cleanup flow backs off instead of treating a peer's in-flight
  mutation as cleanup debris.

- delete_installation keeps the legacy manifest projection in place (the
  tombstone is cleanup-pending and its definition stays authoritative);
  delete_manifest retires it at convergence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(extensions): state the single-writer-per-root deployment contract

The store's concurrency machinery exists for in-process concurrency and
crash recovery, not cross-process coordination. Make the already-implied
topology explicit — one live writer per installation root, recovery
assumes an orphaned lease's writer is dead — instead of hedging about
same-version multi-writer convergence, which invited defending races in
a topology this product does not run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(extensions): sweep failed-install debris instead of merging it in

A fresh install that fails after writing its membership row leaves an
orphan row with no installation record. Creation previously merged
(activate-only) to protect concurrent cross-process creators — a
topology the deployment contract now excludes — so a later user's
successful install silently granted membership to the user whose
install failed. Every aggregate write now sweeps child rows omitted
from its member/binding sets; under one live writer per root an
omitted-but-active row can only be debris from an earlier failed write.
Deletes the V2ComponentSync merge machinery and the cross-process
creation-merge test with it (net-negative diff).

Regression test: failed_install_debris_is_not_granted_to_a_later_installer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(extensions): match the sweep contract to the shipped code

f1f503f deleted V2ComponentSync and made every aggregate write sweep
child rows omitted from its member/binding sets, but the contract still
described the removed activate-only merge ("Only a leased update sweeps
... Creation and reactivation ... merge (activate-only)"). The contract is
the normative spec for this store, so a reader following it would conclude
a fresh install's debris survives a later installer -- the exact bug that
commit fixed.

Verified live against the running stack: an injected orphan membership row
(active, no installation record) is tombstoned by a different user's
successful install, and that user is never granted membership.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(extensions): retire the vestigial installation health snapshot

The per-installation health snapshot was never wired to anything. Only one
production site ever constructed it -- `ExtensionInstallation::new`, always
`Healthy` with `message: None` -- nothing called `update_health`, nothing
constructed `Degraded`/`Unhealthy` outside tests, and it was never projected
to any API. Its `message` was wrapped in a `Serialize`/`Deserialize` pair that
substituted the literal "<redacted>", so the field could not carry information
even in principle.

Extension failure state already has an owner: a failed activation persists
`InstallationState::Failed` plus a redacted `last_error` on the host record,
which is projected through `installation_state` onto the extensions card.
Normalizing the snapshot into its own v2 record would have created a second,
durable, never-refreshed answer to "is this extension broken" that could only
diverge from the one callers actually read. Deleting it leaves exactly one.

Removes the health record, its path/index/kind, `update_health` from the port,
and the redaction machinery. The v2 layout is now three collections.

The released aggregate row does carry `health`, and `ExtensionInstallation`
denies unknown fields, so the wire struct accepts and discards it -- without
that, every existing row fails to deserialize and the catalog disappears on
upgrade.

Two fault-injection tests used the health write as their "after memberships,
before the record commit" interrupt point; that slot is now the credential
binding write, so both were retargeted at the same ordinal.

Regression test: released_aggregate_rows_carrying_health_still_deserialize.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(extensions): drop the retired health collection from the restart journey

The soft-removal assertion still walked ["installations", "memberships",
"health"] and required every collection to stay queryable. The health
collection no longer exists, so the integration journey failed while the
store contract tests passed -- my symbol grep missed it because the
collection is named by path string, not by type.

The surviving assertion is the one that matters: installation and membership
rows are still queryable after a soft removal.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
* refactor(extensions): normalize filesystem state records

* test(reborn): refresh golden payload surface hashes for merged main

The three golden payload suites pin the full model-visible system prompt,
including the capability-surface sha256. nearai#6345 refreshed these snapshots on
its own branch, but the surface hash of merged main differs (a sibling
change altered the underlying tool-definition surface), so main's tip fails
golden_context_surfacing, golden_tool_call_feedback, and
golden_parallel_tool_calls with a hash-only diff — the rendered capability
list and every other byte of the payload are unchanged. Accept the merged
surface hash per the documented cargo-insta drift flow in
tests/integration/support/golden.rs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(reborn-tests): give group suites the root-tests stack headroom

reborn_group_triggers::triggered_gate_group overflowed the 8 MiB libtest
default stack in the group-tests job on Linux while its measured stack need
is unchanged from base (both sit in the same 1.875-2.0 MiB threshold bucket
on macOS debug): the suite runs a full Reborn runtime per scenario on the
test-thread stack and rides within a few percent of the limit, so any
unrelated code-layout shift can tip it over. The root-tests and coverage
jobs already set RUST_MIN_STACK for exactly this pathology; align the
group-tests job with the root-tests value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(extensions): close v2 store review findings on unreserved sweeps, projection failures, and health writes

Three review findings on the normalized v2 store, each with a regression
test in installations_contract.rs:

- Creation/reactivation (and compensation restores) hold no core
  reservation, so the component writer must not sweep child rows omitted
  from its aggregate: a concurrent creator's just-activated membership
  would be tombstoned while both install calls report success. Component
  sync is now mode-split (V2ComponentSync); only a reserved update of an
  active core sweeps omitted rows.
  Test: concurrent_creation_does_not_tombstone_the_other_creators_membership

- Once the v2 records (the authority) commit, a failed legacy
  compatibility-projection write no longer fails the operation — callers
  compensated against a live install and left a ghost (store says
  installed, runtime/files torn down). The projection is repaired from v2
  at the next startup.
  Test: aggregate_projection_write_failure_does_not_fail_a_committed_v2_install

- update_health now writes only the health row instead of rewriting the
  whole aggregate through the reservation path, so it can no longer race
  a final removal into resurrecting a removed core, and it matches the
  plan's health-isolation contract.
  Test: update_health_mutates_only_the_health_row

Also documents the merge/health/projection semantics and enforcing test
commands in docs/reborn/contracts/extensions.md, the deliberate
at-least-once join outcome during a removal reservation in-code, and
fixes the plan doc's stale integration-test filter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(extensions): pin one membership row per creator; docs: name the manifest row an install pin

Strengthen the concurrent-creation regression test per review: assert the
row count before the per-user status projection so a duplicate row could
never hide behind the map. Clarify in the extensions contract that the
manifest row is the hash-pinned copy of the installed package definition,
not an availability or policy declaration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(extensions): merge the install pin into the installation record and derive lifecycle state

Consolidates the v2 schema before first deployment (the last
migration-free moment), per review discussion:

- The separate manifest record folds into the installation record as an
  embedded, hash-pinned install pin; manifest_ref is derived from it so
  an installation can never disagree with its definition, and a
  free-floating definition row cannot exist.
- The three-value status enum dissolves into derived state: removed_at
  is the tombstone, an explicit V2MutationLease (member: Option) is the
  exclusive mutation gate, and a record with neither is live. The
  unexposed core installed_at field is dropped; memberships keep theirs.
- Solo upsert_manifest leaves the port: installs go through the combined
  manifest+installation write, and persist_install_plan's manifest-orphan
  compensation deletes itself. delete_manifest remains as the idempotent
  convergence verifier.
- The legacy removal-cleanup tombstone semantics are preserved as a
  first-class derived state: delete_installation marks
  removal_cleanup_pending, keeping the embedded definition authoritative
  for get_manifest so interrupted cleanups retry without the catalog and
  imports stay blocked until delete_manifest marks convergence.
  persist_removal_tombstone seeds the same state for orphan cleanups,
  and bootstrap imports orphan legacy manifest rows as pending
  tombstones so interrupted v1 removals stay retryable after migration.
- upsert_installation validates against the record's own pin, falling
  back to the extension's authoritative definition for legacy-shaped
  additional rows.
- cas_update's retry loop is boxed off caller stacks: the merged
  record's larger CAS state inlined into every store future and
  overflowed default test-thread stacks in debug builds
  (reborn_group_triggers reproduced it; it passes at default again).

Contract coverage: the full installations_contract suite is adapted to
the merged layout and extended with
removal_tombstone_keeps_manifest_authoritative_until_convergence; the
composition cleanup-tombstone tests, the multi-row canonicalization
integration test, and the restart journey pass unchanged in intent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(extensions): make lease recovery child-authoritative and import orphan legacy manifests

Closes the review findings on the consolidated schema, each with a
regression test:

- Bootstrap imports an orphaned legacy manifest row (the legacy flow's
  durable removal-cleanup marker) as a cleanup-pending tombstone, so an
  interrupted v1 removal stays retryable without the catalog after
  migration and imports stay blocked until convergence.
  Test: bootstrap_imports_orphan_legacy_manifest_as_cleanup_pending_tombstone
  (the previous attempt at this import was lost to a silently failed
  scripted edit; the new test pins the migration path directly).

- A missing compatibility snapshot is no longer treated as proof of
  removal during lease recovery — projection writes are best-effort, so
  a live record can legitimately lack one. The child rows are the
  authority: surviving active membership (or a legacy tenant owner)
  clears the lease over the children as they stand; only a record whose
  memberships were all tombstoned rolls forward to removed.
  Test: interrupted_update_without_compatibility_snapshot_stays_live_on_reopen

- repair_compatibility_views skips a lease taken by a live peer between
  startup passes instead of failing the whole store open.

- persist_removal_tombstone returns the retryable
  MembershipMutationInProgress for a leased live record, so the
  orphan-cleanup flow backs off instead of treating a peer's in-flight
  mutation as cleanup debris.

- delete_installation keeps the legacy manifest projection in place (the
  tombstone is cleanup-pending and its definition stays authoritative);
  delete_manifest retires it at convergence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(extensions): state the single-writer-per-root deployment contract

The store's concurrency machinery exists for in-process concurrency and
crash recovery, not cross-process coordination. Make the already-implied
topology explicit — one live writer per installation root, recovery
assumes an orphaned lease's writer is dead — instead of hedging about
same-version multi-writer convergence, which invited defending races in
a topology this product does not run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(extensions): sweep failed-install debris instead of merging it in

A fresh install that fails after writing its membership row leaves an
orphan row with no installation record. Creation previously merged
(activate-only) to protect concurrent cross-process creators — a
topology the deployment contract now excludes — so a later user's
successful install silently granted membership to the user whose
install failed. Every aggregate write now sweeps child rows omitted
from its member/binding sets; under one live writer per root an
omitted-but-active row can only be debris from an earlier failed write.
Deletes the V2ComponentSync merge machinery and the cross-process
creation-merge test with it (net-negative diff).

Regression test: failed_install_debris_is_not_granted_to_a_later_installer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(extensions): match the sweep contract to the shipped code

f1f503f deleted V2ComponentSync and made every aggregate write sweep
child rows omitted from its member/binding sets, but the contract still
described the removed activate-only merge ("Only a leased update sweeps
... Creation and reactivation ... merge (activate-only)"). The contract is
the normative spec for this store, so a reader following it would conclude
a fresh install's debris survives a later installer -- the exact bug that
commit fixed.

Verified live against the running stack: an injected orphan membership row
(active, no installation record) is tombstoned by a different user's
successful install, and that user is never granted membership.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(extensions): retire the vestigial installation health snapshot

The per-installation health snapshot was never wired to anything. Only one
production site ever constructed it -- `ExtensionInstallation::new`, always
`Healthy` with `message: None` -- nothing called `update_health`, nothing
constructed `Degraded`/`Unhealthy` outside tests, and it was never projected
to any API. Its `message` was wrapped in a `Serialize`/`Deserialize` pair that
substituted the literal "<redacted>", so the field could not carry information
even in principle.

Extension failure state already has an owner: a failed activation persists
`InstallationState::Failed` plus a redacted `last_error` on the host record,
which is projected through `installation_state` onto the extensions card.
Normalizing the snapshot into its own v2 record would have created a second,
durable, never-refreshed answer to "is this extension broken" that could only
diverge from the one callers actually read. Deleting it leaves exactly one.

Removes the health record, its path/index/kind, `update_health` from the port,
and the redaction machinery. The v2 layout is now three collections.

The released aggregate row does carry `health`, and `ExtensionInstallation`
denies unknown fields, so the wire struct accepts and discards it -- without
that, every existing row fails to deserialize and the catalog disappears on
upgrade.

Two fault-injection tests used the health write as their "after memberships,
before the record commit" interrupt point; that slot is now the credential
binding write, so both were retargeted at the same ordinal.

Regression test: released_aggregate_rows_carrying_health_still_deserialize.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(extensions): drop the retired health collection from the restart journey

The soft-removal assertion still walked ["installations", "memberships",
"health"] and required every collection to stay queryable. The health
collection no longer exists, so the integration journey failed while the
store contract tests passed -- my symbol grep missed it because the
collection is named by path string, not by type.

The surviving assertion is the one that matters: installation and membership
rows are still queryable after a soft removal.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-6345 — 2ec58d61 Deployed Jul 24, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: ci CI/CD workflows scope: dependencies Dependency updates scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants