refactor(reborn-memory): native-isolated guardrails + lib.rs module split (PR 1 of #3118) by nickpismenkov · Pull Request #3180 · nearai/ironclaw

nickpismenkov · 2026-05-01T23:27:16Z

Summary

Full Reborn memory substrate for #3118, collapsed into a single PR. Originally opened as the first of a 7-PR stack, then PRs 2–6 (#3181–#3185) were squash-merged into this branch on May 4 — the rest of the stack is closed and the work lives here.

What lands with this PR:

Native-isolated guardrails (crates/ironclaw_memory/CLAUDE.md) — full (tenant, user, agent, project, path) scope on every operation, empty-string DB sentinel for absent agent/project, _none is virtual-path-only, legacy migration deferred, Postgres needs real (not compile-only) coverage.
lib.rs module split — the 5,560-line monolith becomes a 47-line re-export hub plus 13 focused modules (backend, chunking, embedding, filesystem, indexer, metadata, path, repo/{mod,in_memory,libsql,postgres,native_libsql,native_postgres}, safety, schema, search). Public API surface preserved.
Prompt-write safety policy (safety.rs, 824 LoC) — PromptWriteSafetyPolicy trait, DefaultPromptWriteSafetyPolicy, PromptProtectedPathRegistry (covers SOUL.md, AGENTS.md, USER.md, IDENTITY.md, MEMORY.md, HEARTBEAT.md, …), versioned policy + sanitized reason codes + redacted event sink. Wired through RepositoryMemoryBackend, MemoryBackendFilesystemAdapter, and MemoryDocumentFilesystem. Defense-in-depth: any warn/bypass outcome that persists requires a durable audit sink before persistence.
Native reborn_memory_* schema — reborn_memory_documents, reborn_memory_chunks (with generated tsvector + HNSW pgvector index), reborn_memory_document_versions. Migrations are idempotent; no legacy memory_documents row migration.
Native libSQL repository (native_libsql.rs, 1,037 LoC) — full read/write/list/search/version/chunk on the new schema. FTS via MATCH, vector search via byte-encoded embeddings + cosine similarity. Transactional via BEGIN IMMEDIATE.
Native Postgres repository (native_postgres.rs, 805 LoC) — same surface on Postgres + pgvector. Row-level locking via LOCK TABLE ... IN SHARE ROW EXCLUSIVE MODE + FOR UPDATE.
RepositoryMemoryBackend scope guards — ensure_path_matches_context / ensure_scope_matches_context fail closed when a caller of the public seam passes a path or scope that doesn't match the authorized MemoryContext.
Search fusion fixes — min_score filter applied after normalization (so default RRF k=60 doesn't drop every result); with_limit() re-clamps pre_fusion_limit so the invariant holds regardless of builder call order; tied scores break deterministically by path ascending.
Race-safe chunk clearing — both skip_indexing and empty-chunks paths in the indexer route through replace_document_chunks_if_current(path, hash, &[]). The unguarded delete_document_chunks API was removed entirely from the trait + all 4 impls.
Five test files — schema contract (libSQL + Postgres smoke), native libSQL repo contract, native Postgres repo contract, RepositoryMemoryBackend behavior contract, and CompositeRootFilesystem vertical integration.

Test posture

Postgres tests fail loud by default. Set IRONCLAW_SKIP_POSTGRES_TESTS=1 to opt into skipping when no DB is available — the previous "silent skip + green pass" pattern violated the ironclaw_memory guardrail that Postgres coverage must be real.

Test plan

cargo fmt --all -- --check — clean
cargo clippy --workspace --all-features --tests — zero warnings
cargo test -p ironclaw_memory --all-features — all libSQL tests pass
End-to-end against pgvector/pgvector:pg16 container with DATABASE_URL=postgres://postgres:postgres@localhost:5432/ironclaw_test — all 142 tests pass (109 unit/lib + ~33 contract/integration)
Verified fail-loud-without-DB (panics with actionable message) and IRONCLAW_SKIP_POSTGRES_TESTS=1 opt-in (clean skip)
cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold — pass
python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD — clean (no panics in changed production code)

Out of scope

PR 7 (product-tool migration) is not included; it remains a separate decision after this lands.
No WorkspaceMemoryAdapter is introduced. No legacy memory_documents rows are migrated.

🤖 Generated with Claude Code

gemini-code-assist · 2026-05-01T23:27:20Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112 and asks for a native, isolated Reborn memory subsystem with explicit tenant/user/agent/project scope columns rather than synthetic workspace user_id strings. Update crates/ironclaw_memory/CLAUDE.md to reflect that decision: - Drop reuse-first / preserve-existing-tables wording. - State that src/workspace is reference material only and that ironclaw_memory must not depend on the main app crate. - Spell out the full (tenant, user, agent, project, path) scope invariant on every read/list/search/write/version/chunk operation. - Document the empty-string DB sentinel for absent agent/project columns and call out that _none is the virtual-path-only sentinel. - Note that legacy migration/coexistence is explicitly deferred and that Postgres needs real (not compile-only) behavioral coverage. No code changes; this PR is purely doc reorientation that unblocks the native-substrate work in PR 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Phase 2 of #3118. Make the crate agent-friendly before the deeper native-substrate changes by splitting the 3801-line crates/ironclaw_memory/src/lib.rs into 13 focused modules. No behavior change: every public name keeps its identity, the four test files under crates/ironclaw_memory/tests/ are untouched, and all 42 existing tests still pass. New layout: src/lib.rs (34 LoC) re-export hub + crate docs src/path.rs scope, path, parsing, validation, error helpers src/metadata.rs DocumentMetadata, .config inheritance src/embedding.rs EmbeddingProvider + vector helpers src/schema.rs JSON schema validation src/search.rs search request/result + RRF/weighted fusion src/chunking.rs ChunkConfig + chunk_document + content_sha256 src/backend.rs MemoryBackend + RepositoryMemoryBackend src/filesystem.rs RootFilesystem adapters src/indexer.rs MemoryDocumentIndexer + ChunkingIndexer src/repo/mod.rs MemoryDocumentRepository + shared repo helpers src/repo/in_memory.rs src/repo/libsql.rs (cfg(libsql)) src/repo/postgres.rs (cfg(postgres)) Visibility rules: - Public items keep their public names (verified against the imports in all four crates/ironclaw_memory/tests/*.rs files). - Helpers shared across modules become pub(crate). - libSQL- and Postgres-only schema constants and helpers stay inside their respective repo files. Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration This split is the foundation for the follow-up PRs that add the native reborn_memory_* schema, libSQL/Postgres repositories on the native schema, ported semantic/search/versioning tests, and host wiring (#3118 phases 3-7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Three medium-severity issues raised on the split refactor: 1. search.rs: `min_score` was filtering raw RRF/weighted scores before the later normalization pass. With default RRF k=60 the top single hit's raw score is ~0.016, so a caller-supplied `min_score=0.5` would drop every result before normalization could lift the best hit to 1.0. Move filtering to after normalization, matching the workspace fusion contract; add 3 regression tests covering RRF, weighted, and `min_score=0`. 2. indexer.rs: empty/whitespace documents routed through the unconditional `delete_document_chunks`, which races with a concurrent writer that has already produced fresh chunks for newer content. Route empty chunk sets through the same hash-checked `replace_document_chunks_if_current` path used for non-empty sets; the hash guard makes a stale delete a no-op once the document has moved on. Add 2 regression tests using a recording mock. 3. backend.rs: `RepositoryMemoryBackend` ignored its host-resolved `MemoryContext` and forwarded the separately supplied path/scope to the repository, so a direct caller of this public seam that authorized one context but passed a path or scope for a different tenant/user/agent/project would bypass the boundary. Add `ensure_path_matches_context` / `ensure_scope_matches_context` guards before any repository side effect on read, write, compare-and-append, and list. Add 5 regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

#3181) * docs(reborn-memory): reorient guardrails to native-isolated direction Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112 and asks for a native, isolated Reborn memory subsystem with explicit tenant/user/agent/project scope columns rather than synthetic workspace user_id strings. Update crates/ironclaw_memory/CLAUDE.md to reflect that decision: - Drop reuse-first / preserve-existing-tables wording. - State that src/workspace is reference material only and that ironclaw_memory must not depend on the main app crate. - Spell out the full (tenant, user, agent, project, path) scope invariant on every read/list/search/write/version/chunk operation. - Document the empty-string DB sentinel for absent agent/project columns and call out that _none is the virtual-path-only sentinel. - Note that legacy migration/coexistence is explicitly deferred and that Postgres needs real (not compile-only) behavioral coverage. No code changes; this PR is purely doc reorientation that unblocks the native-substrate work in PR 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn-memory): split lib.rs into focused modules Phase 2 of #3118. Make the crate agent-friendly before the deeper native-substrate changes by splitting the 3801-line crates/ironclaw_memory/src/lib.rs into 13 focused modules. No behavior change: every public name keeps its identity, the four test files under crates/ironclaw_memory/tests/ are untouched, and all 42 existing tests still pass. New layout: src/lib.rs (34 LoC) re-export hub + crate docs src/path.rs scope, path, parsing, validation, error helpers src/metadata.rs DocumentMetadata, .config inheritance src/embedding.rs EmbeddingProvider + vector helpers src/schema.rs JSON schema validation src/search.rs search request/result + RRF/weighted fusion src/chunking.rs ChunkConfig + chunk_document + content_sha256 src/backend.rs MemoryBackend + RepositoryMemoryBackend src/filesystem.rs RootFilesystem adapters src/indexer.rs MemoryDocumentIndexer + ChunkingIndexer src/repo/mod.rs MemoryDocumentRepository + shared repo helpers src/repo/in_memory.rs src/repo/libsql.rs (cfg(libsql)) src/repo/postgres.rs (cfg(postgres)) Visibility rules: - Public items keep their public names (verified against the imports in all four crates/ironclaw_memory/tests/*.rs files). - Helpers shared across modules become pub(crate). - libSQL- and Postgres-only schema constants and helpers stay inside their respective repo files. Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration This split is the foundation for the follow-up PRs that add the native reborn_memory_* schema, libSQL/Postgres repositories on the native schema, ported semantic/search/versioning tests, and host wiring (#3118 phases 3-7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): add native reborn_memory_* schema + empty repo wiring Phase 3 of #3118, PR 2 of the stack. Add the dedicated reborn_memory_* substrate that the rest of the stack will build on, without yet implementing any read/write/list/search behavior. The native schema lives inside the crate (consistent with the existing `LibSqlMemoryDocumentRepository::run_migrations` pattern where the crate manages its own DDL in isolation from the main app's migration system). The legacy `memory_documents` table is deliberately untouched — Reborn memory must not depend on or piggy-back on the legacy schema. Added: src/repo/native_libsql.rs - RebornLibSqlMemoryDocumentRepository (compiles, errors on every op) - run_migrations() materializes the substrate - REBORN_LIBSQL_MEMORY_DOCUMENTS_SCHEMA (4 tables + indexes + triggers) src/repo/native_postgres.rs - RebornPostgresMemoryDocumentRepository (compiles, errors on every op) - run_migrations() materializes the substrate - REBORN_POSTGRES_MEMORY_DOCUMENTS_SCHEMA (4 tables + GIN/HNSW indexes) tests/reborn_native_schema_contract.rs - reborn_libsql_run_migrations_creates_native_substrate_idempotently - reborn_libsql_repository_fails_closed_until_behavior_lands Schema design (matches issue #3118 exactly): reborn_memory_documents tenant_id TEXT NOT NULL user_id TEXT NOT NULL agent_id TEXT NOT NULL DEFAULT '' -- empty string is the project_id TEXT NOT NULL DEFAULT '' -- DB-only "absent" sentinel; path TEXT NOT NULL -- _none stays virtual-path-only UNIQUE (tenant_id, user_id, agent_id, project_id, path) reborn_memory_chunks -- chunks with content_hash + optional embedding reborn_memory_chunks_fts -- FTS5 virtual table on content (libSQL) -- TSVECTOR + GIN + HNSW (Postgres) reborn_memory_document_versions -- monotonic version per document, cascade Indexes cover full-scope lookup, scope+path lookup, list/search by scope, chunk lookup by document_id, FTS lookup, and version lookup by (document_id, version DESC). Repositories return a clear "reborn-native ... is not yet implemented" `FilesystemError::Backend` for every read/write/list/ search/index operation. Callers fail closed instead of treating an empty result as authoritative. Verification (all green, 44 tests pass): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): native libSQL repository behavior (PR 3 of #3118) (#3182) * feat(reborn-memory): implement native libSQL repository behavior Phase 4 of #3118, PR 3 of the stack. Stacks on PR 2 (#3181). Replace the `not yet implemented` stubs in `RebornLibSqlMemoryDocumentRepository` with a full read/write/list/search/version/chunk implementation against the `reborn_memory_*` substrate. Every query carries the full `(tenant_id, user_id, agent_id, project_id)` scope tuple. Absent agent/project IDs use the empty-string DB sentinel; the constructor rejects empty / `_none` user-supplied IDs, so equality (`agent_id = ?`) is unambiguous and NULL/IS NULL gymnastics are gone. Implementation: - read_document, read_document_metadata, list_documents: filter by full scope tuple + path; native rows reconstruct via `reborn_memory_document_from_row` (which round-trips empty-string sentinel back to `None`). - write_document, write_document_with_options: BEGIN IMMEDIATE -> reborn_libsql_list_paths_for_scope -> conflict check -> insert-or-update; computes content_hash on every write; writes prior content to reborn_memory_document_versions if the content changed and skip_versioning is not set. - write_document_metadata: scoped UPDATE of metadata column. - search_documents: FTS branch joins through reborn_memory_chunks_fts with full-scope WHERE; vector branch reads embeddings, computes cosine, sorts deterministically (path tiebreaker), then RRF / weighted-score fusion. - replace_document_chunks_if_current: re-reads content_hash inside the transaction; no-op if it has drifted (prevents corrupting the index after a between-read-and-refresh rewrite). - delete_document_chunks: scoped DELETE. Reusable helpers added to `repo/mod.rs` and shared by both native backends: - REBORN_SCOPE_NONE_SENTINEL ("") - reborn_agent_id_db_value, reborn_project_id_db_value - reborn_memory_document_from_row(tenant, user, agent_db, project_db, db_path) 13 new behavioral tests in `tests/reborn_native_libsql_repository_contract.rs`: - round_trips_a_document_within_full_scope - returns_none_when_document_is_missing - upsert_replaces_content_for_same_full_scope_and_path - full_scope_isolates_tenant_user_agent_project_independently - top_level_projects_path_is_a_normal_user_path_not_project_scope - rejects_file_directory_prefix_conflicts_within_scope - writes_metadata_and_reads_it_back_for_native_documents - write_with_options_creates_version_row_only_when_not_skipped - version_numbers_are_monotonic_and_content_hash_matches_archived_content - replace_chunks_if_current_is_a_noop_when_document_was_rewritten - full_text_search_returns_only_chunks_within_full_scope - fts_query_escapes_punctuation_and_handles_empty_input_gracefully - full_text_search_uses_rrf_when_only_full_text_branch_returns_results The `reborn_libsql_repository_fails_closed_until_behavior_lands` smoke test from PR 2 is removed (its purpose — proving stubs were fail-closed before behavior landed — is now obsolete). Postgres native repository remains stubbed; PR 4 implements it on the same substrate with a behavioral testcontainer harness. Verification (all green; 56 memory tests pass): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) (#3183) * feat(reborn-memory): implement native Postgres repository behavior Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e482). Replace the `not yet implemented` stubs in `RebornPostgresMemoryDocumentRepository` with a full read/write/list/search/version/chunk implementation against the `reborn_memory_*` Postgres substrate. Every query carries the full `(tenant_id, user_id, agent_id, project_id)` scope tuple. Absent agent/project IDs use the empty-string DB sentinel; the constructor rejects empty / `_none` user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and NULL/IS NULL gymnastics are unnecessary. Implementation: - read_document, read_document_metadata, list_documents: filter by full scope tuple + path; native rows reconstruct via `reborn_memory_document_from_row` (which round-trips empty-string sentinels back to `None`). - write_document, write_document_with_options: BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE -> reborn_postgres_list_paths_for_scope -> conflict check -> upsert -> optional version row -> COMMIT. Computes content_hash on every write; archives prior content into `reborn_memory_document_versions` if the content changed and `skip_versioning` is not set. - write_document_metadata: scoped UPDATE of metadata column. - search_documents: FTS branch joins through `reborn_memory_chunks` with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector branch uses pgvector `<=>` ordering. Both branches are pre-fusion ranked; results are fused through the shared `search::fuse_memory_search_results`, honoring `FusionStrategy`. - replace_document_chunks_if_current: SELECT … FOR UPDATE row lock, hash drift check (no-op if the document was rewritten under it), delete-then-insert chunks with optional pgvector embedding. - delete_document_chunks: scoped DELETE through `reborn_memory_documents` join. - run_migrations: wrapped in a session-level `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent processes (parallel test runs) serialize on `CREATE EXTENSION pgcrypto/vector`. Behavioral coverage in `tests/reborn_native_postgres_repository_contract.rs` (13 tests at parity with the libSQL contract). Tests use the standard `DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern with `try_connect` skipping cleanly when Postgres is unreachable (matches `tests/workspace_integration.rs`); each test scopes a unique tenant prefix and cleans up via `cleanup_tenant` so parallel runs do not collide: - round_trips_a_document_within_full_scope - returns_none_when_document_is_missing - upsert_replaces_content_for_same_full_scope_and_path - full_scope_isolates_user_agent_project_independently - top_level_projects_path_is_a_normal_user_path_not_project_scope - rejects_file_directory_prefix_conflicts_within_scope - writes_metadata_and_reads_it_back - write_with_options_creates_version_row_only_when_not_skipped - version_numbers_are_monotonic_and_content_hash_matches_archived_content - replace_chunks_if_current_is_a_noop_when_document_was_rewritten - full_text_search_returns_only_chunks_within_full_scope - fts_query_escapes_punctuation_and_handles_empty_input_gracefully - full_text_search_uses_rrf_when_only_full_text_branch_returns_results `plainto_tsquery('english', $5)` already tolerates arbitrary punctuation, so the Postgres FTS-escape test asserts the same contract as the libSQL counterpart against the unescaped path. Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo clippy --all --tests --examples --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --features postgres cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Postgres tests run for real against a local DB when DATABASE_URL is reachable; otherwise skip via `try_connect` so CI without a database remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn-memory): port pure-behavior contract over native repos (PR 5 of #3118) (#3184) * test(reborn-memory): port pure-behavior contract over native repos Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a behavior contract for `RepositoryMemoryBackend` layered over both `RebornLibSqlMemoryDocumentRepository` and `RebornPostgresMemoryDocumentRepository`. The semantics under test live above the bare repository — they belong to the `RepositoryMemoryBackend` composition (`.config` inheritance, schema validation, indexer best-effort, capability fail-closed, embedding-dimension guard). These behaviors already exist in production code; PR 5 is purely test coverage that ports the matching contract from the legacy `db_memory_repository_contract.rs` over to the native `reborn_memory_*` substrate. No production-code changes. New file `tests/reborn_native_repository_backend_contract.rs`: libSQL (10 tests, in-process temp DB): - libsql_backend_inherits_config_metadata_for_skip_indexing - libsql_backend_closer_config_overrides_parent_config - libsql_backend_honors_skip_versioning_from_config - libsql_backend_validates_schema_from_config_before_write - libsql_backend_reports_write_success_when_indexer_fails_after_persist - libsql_backend_search_fails_closed_for_unsupported_vector_search - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch - libsql_backend_hybrid_search_fuses_full_text_and_vector_results - libsql_backend_weighted_score_fusion_orders_results_by_weights - libsql_backend_search_honors_limit Postgres (4 tests, skip-on-unreachable per `tests/workspace_integration.rs:26-33`; each test scopes a unique tenant prefix and cleans up via `pg_cleanup_tenant`): - postgres_backend_validates_schema_from_config_before_write - postgres_backend_honors_skip_versioning_from_config - postgres_backend_reports_write_success_when_indexer_fails_after_persist - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch Coverage delta vs. PR 4: | Behavior in #3118 phase 6 | PR 5 | |--------------------------------------------------------|------| | `.config` metadata inheritance precedence | ✓ | | Closer `.config` overrides parent `.config` | ✓ | | `skip_indexing` from `.config` | ✓ | | `skip_versioning` from `.config` | ✓ | | Schema validation before persistence | ✓ | | Indexer failure after persist => write success | ✓ | | Capability fail-closed for vector search | ✓ | | Query embedding dimension mismatch fails closed | ✓ | | RRF fusion (hybrid) | ✓ | | WeightedScore fusion | ✓ | | `limit` honored | ✓ | | FTS query escaping (already covered in PR 3 / PR 4) | n/a | | `version` hash semantics (already covered in PR 3/4) | n/a | Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo clippy --all --tests --examples --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Postgres tests run for real against a local DB when DATABASE_URL is reachable; otherwise skip via `pg_try_connect` so CI without a database remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185) * test(reborn-memory): vertical integration through public seams Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add caller-facing vertical-integration coverage that drives the full public-seam stack against the Reborn-native repositories: reborn-native repository -> ChunkingMemoryDocumentIndexer -> RepositoryMemoryBackend -> MemoryBackendFilesystemAdapter -> CompositeRootFilesystem mounted at /memory These behaviors already exist in production code; PR 6 is purely test coverage that proves the Phase 7 invariants hold against the native substrate. No production-code changes. `tests/reborn_native_filesystem_vertical_integration.rs` — public-seam vertical: libSQL (6 tests, in-process temp DB): - libsql_round_trips_authorized_read_write_through_composite_mount - libsql_lists_direct_children_through_composite_mount - libsql_search_through_composite_mount_returns_only_same_scope_results - libsql_capability_denied_search_fails_closed_before_repository_is_called - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error - libsql_composite_mount_rejects_duplicate_root_at_registration Postgres (2 tests, skip-on-unreachable; per-tenant cleanup): - postgres_round_trips_authorized_read_write_through_composite_mount - postgres_search_through_composite_mount_returns_only_same_scope_results This amend additionally closes thoroughness gaps identified in a Required-Tests audit of #3118. None of these are production behavior changes — every gap is a missing assertion on behavior the production code already implements: `tests/reborn_native_repository_backend_contract.rs`: - Strengthen skip_indexing test to assert chunk-count = 0 in DB instead of indexer-call-count = 1 (uses real ChunkingMemoryDocumentIndexer that respects skip_indexing). Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing` -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`. - Add `libsql_backend_document_metadata_overrides_inherited_config`: parent .config says skip_versioning=true, doc-level metadata overrides to false; overwrite must produce a version row. - Strengthen weighted-score test to assert ordering changes when weights flip (FT-heavy vs vector-heavy produces different top result, proving weights actually steer the score). - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch` asserting result count is bounded by an effective pre_fusion_limit. - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test locking the clamp invariant of `with_pre_fusion_limit`). `tests/reborn_native_libsql_repository_contract.rs`: - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`: two `tokio::join!`-launched writes serialize via `BEGIN IMMEDIATE`; assert exactly one row in `list_documents`. - Add `fts_query_with_only_stopwords_does_not_error` covering the issue's "empty/stopword-ish" FTS case. `tests/reborn_native_postgres_repository_contract.rs`: - Add `same_path_in_different_tenants_stores_separate_rows` (Postgres parity for the tenant axis the libSQL contract already covered). - Add Postgres mirrors of the concurrent-writes and stopword-FTS tests above. `src/search.rs`: - Add unit tests for `fuse_memory_search_results` deterministic tiebreak: tied scores must sort by relative path ascending, independent of insertion order. Coverage delta for issue #3118 "Required tests": | Required test (issue) | Status | |--------------------------------------------------------------|--------| | Same path different tenants/users/agents/projects | ✓ | | Search/list scope isolation | ✓ | | projects/ prefix as user path | ✓ | | Same scope+path upserts | ✓ | | Different scope+same path -> different docs | ✓ | | File/directory prefix conflict | ✓ | | Concurrent writes -> exactly one row | ✓ | | Indexer failure after persist -> write succeeds | ✓ | | .config applies to children | ✓ | | Closer .config overrides parent | ✓ | | Document metadata overrides inherited .config | ✓ | | skip_indexing -> chunk count = 0 | ✓ | | skip_versioning -> no version row | ✓ | | Schema validation rejects before persistence | ✓ | | Write -> chunk -> FTS end-to-end | ✓ | | FTS escaping (punct + stopwords) | ✓ | | Vector dim mismatch | ✓ | | Hybrid ranking deterministic (path tiebreak) | ✓ | | RRF + WeightedScore fusion | ✓ | | limit + pre_fusion_limit | ✓ | | Version monotonic + content hash | ✓ | | Adapter / RootFilesystem vertical integration | ✓ | | Unsupported capability fails closed before side effects | ✓ | Acceptance criteria from #3118 closed by this PR: - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` / `RootFilesystem`) have vertical integration coverage against native repositories. Verification (all green; second run confirms reliability — first run hit a pre-existing parallel-execution flake in the legacy `db_memory_repository_contract` target, unrelated to this PR; passes in isolation and on re-run): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo clippy --all --tests --examples --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Postgres tests run for real against a local DB when DATABASE_URL is reachable; otherwise skip via `pg_try_connect` so CI without a database remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3185 review findings Three medium-severity issues raised on the vertical integration PR: 1. Postgres vertical tests in reborn_native_filesystem_vertical_integration were silently skipped when the pool could not hand out a connection, so the test binary reported them as passed even when Postgres was unreachable. Replace the silent-skip helper with `pg_require_connection` that panics with an actionable message unless the new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the `ironclaw_memory` guardrail that Postgres behavioral coverage must be real, not compile/skip coverage. 2. The Postgres vertical search test built `RepositoryMemoryBackend` without a `ChunkingMemoryDocumentIndexer`, so writes through `CompositeRootFilesystem` never populated `reborn_memory_chunks`. Search returned an empty Vec, and the assertion `paths.iter().all(|p| p == "visible.md")` was vacuously true even without proving anything about the write -> index -> search flow. Wire the same chunking indexer the libSQL vertical stack uses (FTS-only — no embedding provider), and add an explicit non-empty assertion before the scope check. Verified end-to-end against a `pgvector/pgvector:pg16` container. 3. The `pre_fusion_limit_is_clamped_up_to_limit` test in reborn_native_repository_backend_contract claimed the invariant `pre_fusion_limit >= limit` held regardless of caller order, but only exercised `with_limit().with_pre_fusion_limit()`. The reverse case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch SQL `LIMIT` shrink below the requested final limit. Make `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3184 review findings Three medium-severity issues raised on the behavior-contract PR: 1. Postgres backend tests in reborn_native_repository_backend_contract were silently skipped when the pool could not hand out a connection, so the test binary reported all Postgres tests as passed without running migrations or exercising any of the schema validation, versioning, indexer, or search behavior against Postgres. Replace the silent-skip helper with `pg_require_connection` that panics with an actionable message unless the new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the `ironclaw_memory` guardrail that Postgres repository coverage must be real, not compile/skip coverage. 2. The `libsql_backend_search_honors_limit` and `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests only asserted `results.len() <= 2`, which is vacuously satisfied by 0 or 1 results — a regression that broke FTS or indexing entirely would still pass. Establish the precondition with a larger-limit baseline search asserting the full match count is visible, then assert the bounded search returns exactly 2. 3. The new search/fusion/limit contract was libSQL-only, while `RebornPostgresMemoryDocumentRepository` has its own FTS (`tsvector`) and pgvector query implementations. Add Postgres counterparts for hybrid search, weighted-score fusion, pre-fusion-limit per-branch, and the `limit` truncation cases. The Postgres weighted-score test uses asymmetric FTS content (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks the FTS leader unambiguously above the hybrid match — without this, `ts_rank_cd` ties non-deterministically. Generalize `RecordingEmbeddingProvider` to be parametric on `DIM` so the same provider semantics work against libSQL (DIM=3, blob embeddings) and Postgres (DIM=1536, fixed-width pgvector column). Verified end-to-end against a `pgvector/pgvector:pg16` container: all 105 memory tests pass, zero clippy warnings, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3183 review findings Two review issues raised on the native Postgres repository PR: 1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional `delete_document_chunks` for both `skip_indexing == Some(true)` and the empty-chunks case. A stale reindex (older bytes, older skip_indexing flag) racing with a concurrent writer that has already produced fresh chunks for newer content could clobber the newer write's chunk rows, leaving the latest content unsearchable. Compute `content_hash_at_read` up front and route both paths through the same hash-guarded `replace_document_chunks_if_current` call. The hash guard turns a stale clear into a no-op once the document has moved on. `delete_document_chunks` is no longer reached by the indexer; the trait method stays in place to avoid churning the API across the stack. Add 3 regression tests in `mod tests` covering the skip_indexing path, the empty-chunks path, and the non-empty path, each asserting routing through `replace_document_chunks_if_current` with the read-time content hash. 2. The Postgres contract harness silently skipped every test when the pool could not hand out a connection — the binary reported all tests as passed without running migrations or exercising any read/write/search/chunk/version behavior, violating the `ironclaw_memory` guardrail that Postgres coverage must be real. Make `try_connect` panic with an actionable message unless the new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set. Verified end-to-end against a `pgvector/pgvector:pg16` container: all 108 memory tests pass, zero clippy warnings, fmt clean. Verified fail-loud and opt-in-skip behavior with and without DATABASE_URL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn-memory): remove unguarded delete_document_chunks API Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous fix routed the indexer through `replace_document_chunks_if_current` so the live race the reviewer described (stale reindex clobbering chunks from a newer write) was closed, but the unguarded `delete_document_chunks` method was still on the trait and in 4 implementations as a footgun for any future caller. Remove it entirely: - Drop the trait method from `MemoryDocumentIndexRepository` and document at the trait level that chunk clearing is folded into `replace_document_chunks_if_current` (called with an empty `chunks` slice). A hash-guarded clear is the only safe shape here. - Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`, `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the `libsql_document_id_and_content` helper that became dead with the legacy libsql impl. - Remove the test mock's stub and the `IndexerCall::Delete` variant in the indexer regression tests; the recording repo now only implements the hash-checked path, so any future regression that starts calling something else fails to compile. Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory tests pass, zero clippy warnings, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3181 review findings Issue #2 of the two raised on the native-schema PR — silent skip in the schema contract tests. (Issue #1, fail-closed metadata methods, was mooted when PR3 squash-merged into this branch's history and replaced the trait-default Ok stubs with real implementations on both `RebornLibSqlMemoryDocumentRepository` and `RebornPostgresMemoryDocumentRepository`.) The reviewer flagged that `reborn_native_schema_contract.rs` only defined libSQL tests — running with `--features postgres` compiled and ran zero tests, leaving the `pgcrypto`/`vector` extensions, the generated `tsvector` column, the HNSW vector index, and the `reborn_memory_*` tables as compile-only DDL coverage. Add `reborn_postgres_run_migrations_creates_native_substrate_idempotently`: - Uses the now-standard `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var. Without it, an unreachable Postgres panics with an actionable message instead of skipping silently. - Calls `run_migrations()` twice to lock in idempotency. - Verifies both required extensions are installed. - Verifies the three `reborn_memory_*` tables exist. - Specifically verifies the generated `content_tsv` column has type `tsvector` and `is_generated = ALWAYS`. - Specifically verifies the `idx_reborn_memory_chunks_embedding` index uses HNSW. - Re-asserts the legacy `memory_documents` table is not created. Verified end-to-end against `pgvector/pgvector:pg16`: both schema tests pass, all 109 memory tests pass, zero clippy warnings, fmt clean. Verified the fail-loud-without-DB and opt-in-skip paths explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

zmanian

Verdict: APPROVE-WITH-NITS — well-structured split, scope guards and hash-CAS index refresh are correctly designed and tested; SQL is parameterized; issues are real but not catastrophic.

Spot-checked and correct

No SQL injection. Every native_libsql/native_postgres call uses bound params (?N / $N). The only format! in native_postgres is pg_advisory_lock($1) — also parameterized.
Scope equality is typed. MemoryDocumentScope is (String, String, Option<String>, Option<String>) with derived PartialEq (path.rs:9); the agent_id="" vs "None" collision can't occur because comparison happens at the typed Rust level before the repo's _db_value sentinel translation. _none is explicitly rejected at construction (path.rs:37–53).
Hash-CAS race is sound. ChunkingMemoryDocumentIndexer::reindex_document (indexer.rs:97–110) computes content_hash_at_read and passes it to replace_document_chunks_if_current, which inside one transaction selects content_hash FOR UPDATE (postgres native_postgres.rs:430–458) / BEGIN IMMEDIATE + check (native_libsql.rs:444–459) and bails if it has moved. A stale reindex degrades to a no-op rather than wiping fresh chunks. Contract test exists on both backends.
Search fusion fix correct. fuse_memory_search_results (search.rs:255–279) normalizes max-to-1 before applying min_score, with regression tests at search.rs:378 and search.rs:403. with_limit re-clamps pre_fusion_limit (search.rs:65).
Tied-score determinism. Tiebreaker is path-asc (search.rs:285), with two tests proving it isn't insertion-order coincidence (search.rs:336, 357).
Vector encoding (libsql) is explicit LE (to_le_bytes/from_le_bytes, embedding.rs:106–124). cosine_similarity returns None on dim mismatch, empty inputs, zero norms, and any non-finite result (NaN/Inf).
Search-time embedding dim check. backend.rs:513–527 fails fast on caller-supplied embedding dim ≠ provider dim.
API surface preservation. lib.rs re-exports cover the previously-public surface; the new safety / Reborn types are additive.

High

H1. Trigger update_reborn_memory_documents_updated_at is dropped/recreated on every start() (native_postgres.rs:765–769). pg_advisory_lock(REBORN_MIGRATION_LOCK_ID) at line 55 serializes parallel start() calls — but only if start() is the only place this batch runs. Worth a comment at the schema block stating the advisory-lock invariant, plus an explicit test that two parallel RebornPostgresMemoryDocumentRepository::new(...).start() calls succeed.
H2. ⚠️ LOCK TABLE … IN SHARE ROW EXCLUSIVE is table-level (native_postgres.rs:175). Every tenant's writes serialize through one global lock. Combined with the per-chunk INSERT loop in replace_document_chunks_if_current running under a separate transaction, write throughput will fall over fast under multi-tenant load. The unique constraint already prevents the actual data race; the prefix check is best-effort. Recommend dropping the LOCK TABLE and relying on FOR UPDATE + ON CONFLICT, or scoping with pg_advisory_xact_lock(hashtext(scope_id)) per-scope. Flag for follow-up before any prod load.
H3. libsql vector search is full-table-scan-in-Rust (native_libsql.rs:807–940) — selects every chunk in scope, loads every blob into Rust, computes cosine in a loop, then sorts. No index. Fine for single user with 10k chunks; multi-tenant trap. Probably acceptable since libsql doesn't have a vector index, but please add a TODO so this isn't mistaken for the postgres-class hot path.

Medium

M1. PromptProtectedPathRegistry::default() matches by lowercased exact relative path (safety.rs:125–137), not basename. If the runtime ever writes agents/AGENTS.md or _user/USER.md, those would not match. Confirm with the author whether the contract is "exact relative path only" (fine) or "any path whose basename is one of these" (insufficient).
M2. PromptProtectedPathClass::as_str() always returns "system_prompt_file" regardless of which path matched (safety.rs:62–64). The "stable protected-path class" is currently a single bucket — telemetry can't distinguish AGENTS.md vs SOUL.md vs HEARTBEAT.md events. Either remove as_str or have it return distinct stable strings per class.
M3. Audit-sink-required-before-persistence is enforced for Warn/BypassAllowed (safety.rs:643, 671, 750–763). Reject and Allow paths use require_sink: false, so a successful protected-path write with event_sink: None leaves no audit trail. That's the documented contract, but worth re-examining: a successful write to AGENTS.md without a sink is silent. Recommend either making the sink mandatory whenever a protected_path_class is matched, or documenting this as an explicit operator opt-in.
M4. PromptWriteSafetyEvent carries the full MemoryDocumentScope (safety.rs:766) — tenant/user/agent/project IDs. Validated by validated_memory_segment so they can't carry arbitrary content, but they are user-derived. Confirm sink consumers are OK with that.
M5. tsvector hardwired to 'english' (native_postgres.rs:777). Non-English content gets degraded full-text relevance. Fine for v1 if all current users are English; worth a TODO/issue link.
M6. HNSW m=16, ef_construction=64 are pgvector's typical defaults — fine. Search-time ef_search is unset, so pgvector uses whatever session default applies. Call out the recall/build-cost tradeoff at the schema or in docs.

Low / nits

L1. safety.rs:469 — request.content.trim().is_empty() rejects content that is e.g. " \n". Probably intended; worth a one-line comment.
L2. safety.rs:30 — validates trimmed-empty version but stores the un-trimmed value. Inconsistent.
L3. native_libsql.rs:209–214 and :345 hand-format strftime('%Y-%m-%dT%H:%M:%fZ', 'now') — hoist into a constant.
L4. native_libsql.rs mixes raw BEGIN IMMEDIATE/COMMIT/ROLLBACK strings (140–268) with transaction_with_behavior (433). Pick one style.
L5. PromptSafetyAllowanceId::empty_prompt_file_clear() (safety.rs:219) hardcodes a string here and matches against == at safety.rs:471. A single const would be clearer.

Test gaps worth filling

Concurrent replace_document_chunks_if_current with same expected_content_hash and different chunk sets — one wins, one no-ops.
Embedding dimension mismatch at write/index time (search-time check exists, write-time doesn't have a test).
Safety-policy bypass paths: (a) BypassAllowed for empty_prompt_file_clear actually persisting; (b) require_sink: true with event_sink: None returning PromptWriteSafetyEventUnavailable and not writing the document.
Policy version bump (v1 client vs v2 writer reconciliation).
tsvector with non-English (e.g. CJK) content — smoke test that write doesn't fail.
Idempotent start() called twice in one process and from two processes (postgres advisory-lock contract).
End-to-end search exercising pre_fusion_limit clamp (>50 candidates).

Bottom line

Ship it after addressing H2 (the table-level LOCK TABLE is too coarse for multi-tenant — at minimum file a follow-up issue before any prod traffic) and M2 (PromptProtectedPathClass::as_str is degenerate as written). Everything else is fine for follow-up. The split itself is clean, the trait surface is right, and the scope guards close a real hole.

# Conflicts: # crates/ironclaw_memory/src/lib.rs

Address three high/medium severity unresolved review comments on the native Reborn memory repositories: - libSQL/Postgres native repos now implement `compare_and_append_document_with_options` directly with hash conflict checks, version archival, path-conflict checks on insert, and the same transaction/locking the legacy backends use (BEGIN IMMEDIATE on libSQL; LOCK TABLE IN SHARE ROW EXCLUSIVE MODE + FOR UPDATE on Postgres). Without the override, atomic appends silently fell back to the trait default. - Direct-repo `write_document` now populates `changed_by` from a scoped owner key so version rows are never NULL-attributed when callers bypass the backend/filesystem seam. - `reborn_memory_chunks.embedding` is now `vector` (unbounded) instead of `vector(1536)` so non-1536 providers (Ollama 768/1024-dim, OpenAI 3072-dim, Claude 1024-dim, …) can write embeddings. The prior fixed dimension was a footgun. Schema contract test asserts the unbounded shape and that no HNSW index is reintroduced (HNSW requires fixed dimension). Adds tests at every tier the reviewer asked for: native repository contract tests (append + changed_by), filesystem vertical-integration tests that drive `append_file` through the composite mount, and a Postgres test that writes mixed-dimension embeddings to lock in the unbounded vector contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address zmanian's two must-fix items on PR #3180 and the seven test gaps he flagged for follow-up. Must-fix: - H2: Replace the table-level `LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE` on the Postgres write/append paths with a per-scope advisory lock keyed on `hashtext('tenant:user:agent:project')`. Concurrent writes for different scopes no longer serialize globally; the unique constraint still prevents same-path duplicates and `FOR UPDATE` still pins the row. Exposes a latent H1 deadlock between concurrent `run_migrations` (DROP TRIGGER → AccessExclusiveLock) and writers (RowExclusiveLock); fixed by short-circuiting `run_migrations` when the schema is already present and gating the trigger setup behind a `pg_trigger` existence check so the table is never re-locked at AccessExclusive level on re-runs. - M2: `PromptProtectedPathClass::as_str()` now maps each default protected path to a distinct stable class string (`agents_md`, `soul_md`, `heartbeat_md`, …) instead of the single `system_prompt_file` bucket. Custom paths fall through to `custom_protected_path`; consumers that need the exact path use `relative_path()`. Test gaps: 1. Concurrent `replace_document_chunks_if_current` with the same `expected_content_hash` and different chunk sets — exactly one writer's set lands, no partial union (libSQL + Postgres). 2. Write/index-time embedding dimension mismatch — provider declares one dim, returns another; indexer fails closed before any chunks land while the durable document row still persists (libSQL + Postgres). 3. `BypassAllowed` path on `empty_prompt_file_clear` actually overwrites the prior content with the cleared (empty) content and records a `BypassAllowed` audit event; the `require_sink: true` ⇒ no-sink fail-loud path was already covered by existing tests. 4. Audit event records the *classifying* registry's policy version per-path, not always the default — locks in v1/v2 reconciliation during a policy bump. 5. Postgres tsvector with CJK content writes cleanly under the `'english'` config; English tokens remain searchable alongside CJK chunks. 6. Two `RebornPostgresMemoryDocumentRepository` instances on independent pools call `run_migrations` concurrently against a *cold* schema (per-test isolated Postgres schema) — both succeed under `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)`; the schema fingerprint count is exactly one. 7. End-to-end search with 60 candidates exercises the per-branch `pre_fusion_limit` SQL `LIMIT` cap and asserts a unique limit-respecting result set. Also surfaces tokio_postgres error source chains via a `pg_error_chain` helper so `FilesystemError::Backend.reason` fields contain SQLSTATE instead of the opaque `"db error"` string. This was load-bearing for diagnosing the H1 deadlock during this work and remains useful for future operators. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… MED data-destruction) Each fix has a regression test driving the original bug shape. HIGH `backend.rs:79` — public forgeable prompt-safety bypass `MemoryContext::with_prompt_write_safety_enforced()` was `pub`, so any direct backend caller could construct an "already-enforced" context and persist high-risk content to SOUL.md/BOOTSTRAP.md through `RepositoryMemoryBackend::write_document` without ever passing through policy. Made the builder `pub(crate)` so only the in-crate filesystem adapter can produce an enforced context, and added an anchor test for the default-unenforced invariant. HIGH `filesystem.rs:47` — sink config no longer flips the policy override `with_prompt_write_safety_event_sink()` was setting `prompt_safety_config_overridden = true`, which caused the adapter to take over enforcement and skip a stricter wrapped backend's policy. A host that wanted only a durable audit sink ended up bypassing the policy. Sink config is observability and is now scoped to itself; the override flag is only flipped by the explicit policy/registry builders. Regression: stricter `AlwaysRejectProtectedPolicy` on the backend still fires when the adapter only adds a sink. HIGH `filesystem.rs:196` — adapter must check the path-level enforce result `adapter_should_enforce_prompt_safety` was using `!backend_capabilities.prompt_write_safety` instead of `!backend_will_enforce_protected_path`. A backend that advertised the capability while reporting `prompt_write_safety_protects_path() == false` for adapter-classified protected files (SOUL.md, AGENTS.md, …) caused the adapter to skip its own check and the backend to do nothing. Both `write_file` and `append_file` now consult the path-level result. Regression: a custom backend with capability=true and protects_path=false is rejected by the adapter for SOUL.md high-risk content. MED `native_libsql.rs:801` — LIKE wildcards in path segments `MemoryDocumentPath` allows `%` and `_` in valid segments, but the metadata-clear path interpolated the parent prefix raw into a `LIKE pattern`. A `team_%/.config` write would also clear chunks for unrelated `team-a/note.md`. Added shared `escape_like_pattern` helper; both libSQL and Postgres metadata-clear queries now use `LIKE … ESCAPE '\\'`. Regression on both backends drives `team_%/` vs `team-a/`. MED `native_libsql.rs:531` — chunk-clear must be gated on row update + honor descendant overrides Two related issues: 1. `write_document_metadata` used to run the descendant chunk-clear even when the UPDATE matched zero rows. For a missing root `.config` the LIKE pattern was bare `%` → every chunk in the scope wiped. Now both backends capture `rows_affected` and gate the clear on it. 2. The descendant clear deleted chunks for documents whose own document-level metadata explicitly set `skip_indexing=false` (an override of the inherited parent .config). Both backends now exclude descendants with explicit `skip_indexing=false` via `json_extract` (libSQL) and `metadata->>'skip_indexing'` (Postgres). Two libSQL regressions cover both fixes. Already-closed items verified during review Two MED items zmanian flagged were already addressed by earlier work in this branch — verified with assertions and an existing test pass: - MED `backend.rs:342` — direct backend file ops fail closed when `file_documents=false` (`ensure_file_documents_supported` covers read/write/compare-and-append/list; existing `file_document_capability_rejects_direct_backend_file_operations`). - MED `backend.rs:561` — search path checks `capabilities.embeddings` before any `embed_text` call (existing `vector_request_fails_closed_when_embedding_generation_is_disabled`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Each fix has a regression test driving the bug shape. MED `native_postgres.rs:985` — vector search bound type cast The dimension-filtered vector search query bound `$5` without an explicit `::vector` cast, causing pgvector to fail with "function vector_dims(unknown) is not unique" because the function is overloaded on `vector` and `halfvec`. Added `$5::vector` casts on both the filter and the ORDER BY so the overload resolves cleanly. The filter itself (already in place) excludes mismatched-dim chunks. Postgres regression `vector_search_with_mixed_dimension_chunks_does_not_error` proves a mixed-dim scope returns the matching chunk and excludes the mismatched one without erroring the whole search. MED `native_libsql.rs:1232` — FTS backfill regression test The current code already issues `INSERT INTO reborn_memory_chunks_fts (reborn_memory_chunks_fts) VALUES ('rebuild')` after migration. Added a regression that drops the FTS table+triggers, leaves chunk rows intact, re-runs migrations, and asserts FTS search surfaces the pre-existing chunks. MED `indexer.rs:117` — embedding outage degrades to text-only chunks When `build_chunk_writes` fails (provider outage, dimension mismatch, …), the indexer used to clear chunk rows then return the error; backend writes intentionally swallow indexer errors after persistence, so a successfully-overwritten document disappeared from FTS too. Indexer now persists *text-only* chunks (embedding = NULL) via the hash-guarded replace, then surfaces the embedding error for observability. FTS stays current; vector search degrades. Updated the existing `embedding_failure_*_with_hash_guard` test to lock in the new contract; updated the dim-mismatch backend test to assert chunks have NO embedding rather than no chunk rows. MED `indexer.rs:122` — race-narrowing for concurrent skip_indexing Reindex used to read content + metadata, build chunks, and finally replace chunks guarded only by the content hash. A concurrent metadata write that flipped `skip_indexing=true` between the initial resolve and the replace had its chunk-clear undone (content hash unchanged → reindex re-inserted chunks). Indexer now re-resolves metadata immediately before the final replace and short-circuits to an empty chunk set when skip_indexing is now true. New `FlipsSkipIndexingMidwayRepo` mock + regression `concurrent_skip_indexing_metadata_write_wins_against_in_flight_reindex`. A truly race-free fix (atomic metadata-version-checked replace at the repo layer) is documented as a follow-up. MED `native_libsql.rs:491` — metadata-only writes invalidate chunks Trait method `write_document_metadata` already invalidates chunk rows when the new metadata sets skip_indexing=true (covered by the MED #33 fix). Documented the contract on the trait and added a libSQL regression `libsql_metadata_write_setting_skip_indexing_true_clears_existing_chunks` that pins it. Re-enabling indexing (true → false) requires the caller to drive a reindex; the repo can't run the chunker/embedder itself. MED `filesystem.rs:245` — backend skips re-rejection of adapter approval Already addressed: `RepositoryMemoryBackend::write_document/append` guards on `if !context.prompt_write_safety_enforced()`, and the adapter sets that flag on the backend context after running its own enforcement. Added regression `backend_does_not_re_reject_adapter_approved_warn_or_bypass` composing an `AlwaysRejectIfRecheckedPolicy` on the backend with a permissive default policy on the adapter — benign content reaches the wrapped repository because the backend's policy is short-circuited. MED `safety.rs:809` — sink errors only fatal when require_sink=true Already addressed: `emit_prompt_write_safety_event` checks `parts.require_sink` before converting a sink error into `PromptWriteSafetyEventUnavailable`. Added regression `configured_sink_failure_does_not_block_clean_allow_writes` proving an Allow outcome on a protected path with a failing sink still persists (only warn/bypass require a durable audit event). MED `postgres.rs:121` — legacy postgres agent_id binds as Uuid Already addressed: the legacy schema declares `agent_id UUID` in this file's `CREATE TABLE IF NOT EXISTS` (matching the V1 migration), and `scoped_memory_agent_uuid` parses the string into `Option<uuid::Uuid>` before binding. Added behavioural regression `postgres_memory_repository_round_trips_agent_scoped_writes_against_uuid_schema` that creates an agent-scoped path with a real UUID, writes through the repo, reads it back, and asserts the listing surfaces it — proving the UUID bindings work against a real Postgres instance. Bonus close-out The earlier fix in MED `indexer.rs:117` exposed that one existing backend test asserted "0 chunk rows after dim mismatch" — the new contract is "chunk rows exist but with NULL embeddings". Updated the assertion to count `embedding IS NOT NULL`, which is the actually load-bearing invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Consolidated fixes for serrrfirat's 10 unresolved review threads plus zmanian's CHANGES_REQUESTED review (3 blockers + 7 mediums + 3 follow-up items + title typo). ## Blockers 1. CI now runs the package tests (zmanian blocker 2 / serrrfirat #7). The matrix `cargo test ${{ matrix.flags }}` runs from workspace root which only covers the `ironclaw` package; added an explicit step `cargo test -p ironclaw_memory --features libsql --tests` so the Tier A guards for PR #3180 invariants actually fire. 2. `#[ignore]` markers converted to `#[cfg_attr(not(feature = "pr3180-ready"), ignore = ...)]` (zmanian blocker 1 / serrrfirat #1). Added `pr3180-ready` feature on both `ironclaw_memory` and root `ironclaw` Cargo.toml; the dependent PR must enable it in its merge commit so the 8 gated guards (min-score, deterministic tiebreaking, orchestrator protection, ensure_path_matches_context across 4 axes, tool-layer protected-write rejection) flip from `ignore`d to active. 3. Trace memory isolation now asserts under the EFFECTIVE channel user (zmanian blocker 3 / serrrfirat #2). Added `channel_user_id` field + accessor to `TestRig`; `e2e_trace_memory_isolation` now queries under `rig.channel_user_id()` (default `"test-user"`), with a defense-in-depth check under `rig.owner_id()` for mis-routing regressions. ## Test-correctness mediums 4. Min-score test pins `with_query_embedding([1,0,0])` to favor hybrid.md (serrrfirat #3 / zmanian #4). Removed the permissive `* 0.99` fallback — FTS-only-below-hybrid is now a hard assertion. 5. Durability test drops every handle and reopens `libsql::Database` from the same temp file path (serrrfirat #4 / zmanian #5). Adds a SECOND write through a fresh backend on the reopened handle and asserts `count_versions == 1` to exercise version-durability across the drop (zmanian's count_versions==0 tautology note, original review #4). 6. Append versioning asserts exact row count `== 1`, not `!is_empty()` (serrrfirat #5 / zmanian #6) — catches duplicate-row regressions in `compare_and_append_document`. 7. Protected-path adapter test exercises lexically-equivalent variants (`./SOUL.md`, `//SOUL.md`) in addition to canonical (serrrfirat #6 / zmanian #7). VirtualPath rejects `..` so no `..` variant; in-loop `count_documents_total == 0` after EACH variant. 8. Hybrid search isolation now varies all four scope axes (serrrfirat #8 / zmanian #8): tenant, user, agent, project. 5 documents seeded; search from caller scope must return exactly one. 9. Tool round-trip asserts EXACT persisted content via direct DB read (serrrfirat #9 / zmanian #9). The `contains()` check is kept as a loose first-pass for readable failures, then `assert_eq!` on the exact byte string is the load-bearing assertion. 10. Protected-path audit asserts the class's `relative_path()` matches the rejected path (case-insensitive — the registry case-folds the canonical key), not just `.is_some()` (serrrfirat #10 / zmanian #10). A regression that emits the wrong path class now fails. ## zmanian follow-ups Z1. Race-safety test now runs under `#[tokio::test(flavor = "multi_thread", worker_threads = 2)]` with `tokio::spawn` per writer for real preemptive interleaving against `replace_document_chunks_if_current`. Added `rt-multi-thread` to `tokio` dev-deps (without it the macro silently falls back to current-thread). Z2. `write_to_protected_path_rejected.json` trace fixture sets `all_tools_succeeded: false` explicitly. Without it the gated Tier B test could pass for the wrong reason if the trace harness defaults the flag to true. Z3. Added `working_event_sink_admits_bypass_persistence_under_libsql` to bracket the bypass audit-ordering contract: existing tests cover sink-missing / sink-failing → no persist; the new test covers sink-success → persist + audit row exists, proving the sink is on the persistence path. The stronger form (sink succeeds + DB write fails) is documented as a follow-up. ## Cleanup - Removed `_link_in_memory_repo_for_unused_imports` shim and the `InMemoryMemoryDocumentRepository` import that only existed to feed it (zmanian original-review #3). - Fixed PR title typo `momery` → `memory` via gh. Helper-consolidation into `tests/common/libsql_helpers.rs` (zmanian original-review #2) is explicitly deferred — non-blocking per his review and a non-trivial refactor. ## Verified - `cargo fmt --all -- --check` clean - `cargo clippy -p ironclaw_memory --features libsql --all-targets -- -D warnings` zero warnings - `cargo test -p ironclaw_memory --features libsql` all suites green (gated tests stay `ignored` without `--features pr3180-ready`)

Henry's review on PR #3303 (2026-05-12T04:20:05Z) flagged that the SOUL.md tool-layer rejection guard does not run in CI because the trace test is gated on `pr3180-ready` but no workflow enables that feature. His proposed fix — enable the feature in CI now that the substrate (#3180) is merged — would actually break CI: running with `--features pr3180-ready` against `reborn-integration` HEAD surfaces 7 failing tests, because #3180 as merged is narrower than what the gated tests anticipate. None of `ensure_path_matches_context`, deterministic tiebreaking, `.system/engine/orchestrator/*` registration, or min_score-after-normalization exist in the substrate yet (verified by grep against `crates/ironclaw_memory/src/`). ## Changes 1. **Split the feature flag.** `pr3180-ready` now gates substrate-level guards that #3180 was anticipated to deliver but didn't (5 tests). New `pr7-ready` gates the tool-dispatcher → substrate integration tests (1 test: the SOUL.md trace test) — distinct because the tool routing migration (PR 7) is a separate landing from the substrate work. The trace test's gate moves to `pr7-ready` accordingly. 2. **Updated gate messages with empirical reality.** Each gated test's ignore message now states what specifically is missing in the merged substrate (e.g., "no `ensure_path_matches_context` function in `crates/ironclaw_memory/src/`") so a future maintainer doesn't waste cycles enabling the flag and being confused by the failures. 3. **Always-running substrate-level SOUL.md rejection test.** Added `soul_md_high_risk_write_rejected_at_substrate_for_caller_tier_contract` in `e2e_scope_isolation_safety.rs`. Drives `RepositoryMemoryBackend` directly with the same scope+payload+policy shape PR 7's tool routing will use, asserts rejection + non-persistence + correct audit class. Closes Henry's CI gap at the substrate tier even before PR 7 lands; the trace test stays as the caller-tier guard for when the tool routing arrives. Distinct from `protected_paths_high_risk_writes_blocked_at_libsql_backend` (which loops over all 11 protected paths): a SOUL.md-by-name test catches a regression that selectively drops only SOUL.md from the protected-path registry — a real concrete attack surface that's worth pinning by name. ## Not changed - Neither feature is enabled in any CI workflow. Doing so today would surface the 7 substrate-test failures listed above, which aren't ours to fix in this PR. The followup substrate PR(s) must enable `pr3180-ready` in their merge commit; PR 7 must enable `pr7-ready`. - Henry's Low-priority note about `memory_read` tool-output exactness is acknowledged but not addressed here — DB-read covers persistence, and a tool-output helper is a non-blocking follow-up. - The Z3 bypass audit-ordering "sink succeeds + DB write fails" case remains a documented gap in `working_event_sink_admits_bypass_ persistence_under_libsql`; Henry flagged it but didn't escalate. ## Verified - `cargo fmt --all -- --check` clean - `cargo clippy -p ironclaw_memory --features libsql --all-targets -- -D warnings` zero warnings - `cargo test -p ironclaw_memory --features libsql` all suites green (gated tests stay `ignored` without `--features pr3180-ready` / `--features pr7-ready`) - New `soul_md_high_risk_write_rejected_at_substrate_for_caller_tier_ contract` test passes under default flags Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…plit (PR 1 of nearai#3118) (nearai#3180) * docs(reborn-memory): reorient guardrails to native-isolated direction Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112 and asks for a native, isolated Reborn memory subsystem with explicit tenant/user/agent/project scope columns rather than synthetic workspace user_id strings. Update crates/ironclaw_memory/CLAUDE.md to reflect that decision: - Drop reuse-first / preserve-existing-tables wording. - State that src/workspace is reference material only and that ironclaw_memory must not depend on the main app crate. - Spell out the full (tenant, user, agent, project, path) scope invariant on every read/list/search/write/version/chunk operation. - Document the empty-string DB sentinel for absent agent/project columns and call out that _none is the virtual-path-only sentinel. - Note that legacy migration/coexistence is explicitly deferred and that Postgres needs real (not compile-only) behavioral coverage. No code changes; this PR is purely doc reorientation that unblocks the native-substrate work in PR 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn-memory): split lib.rs into focused modules Phase 2 of #3118. Make the crate agent-friendly before the deeper native-substrate changes by splitting the 3801-line crates/ironclaw_memory/src/lib.rs into 13 focused modules. No behavior change: every public name keeps its identity, the four test files under crates/ironclaw_memory/tests/ are untouched, and all 42 existing tests still pass. New layout: src/lib.rs (34 LoC) re-export hub + crate docs src/path.rs scope, path, parsing, validation, error helpers src/metadata.rs DocumentMetadata, .config inheritance src/embedding.rs EmbeddingProvider + vector helpers src/schema.rs JSON schema validation src/search.rs search request/result + RRF/weighted fusion src/chunking.rs ChunkConfig + chunk_document + content_sha256 src/backend.rs MemoryBackend + RepositoryMemoryBackend src/filesystem.rs RootFilesystem adapters src/indexer.rs MemoryDocumentIndexer + ChunkingIndexer src/repo/mod.rs MemoryDocumentRepository + shared repo helpers src/repo/in_memory.rs src/repo/libsql.rs (cfg(libsql)) src/repo/postgres.rs (cfg(postgres)) Visibility rules: - Public items keep their public names (verified against the imports in all four crates/ironclaw_memory/tests/*.rs files). - Helpers shared across modules become pub(crate). - libSQL- and Postgres-only schema constants and helpers stay inside their respective repo files. Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration This split is the foundation for the follow-up PRs that add the native reborn_memory_* schema, libSQL/Postgres repositories on the native schema, ported semantic/search/versioning tests, and host wiring (#3118 phases 3-7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3180 review findings Three medium-severity issues raised on the split refactor: 1. search.rs: `min_score` was filtering raw RRF/weighted scores before the later normalization pass. With default RRF k=60 the top single hit's raw score is ~0.016, so a caller-supplied `min_score=0.5` would drop every result before normalization could lift the best hit to 1.0. Move filtering to after normalization, matching the workspace fusion contract; add 3 regression tests covering RRF, weighted, and `min_score=0`. 2. indexer.rs: empty/whitespace documents routed through the unconditional `delete_document_chunks`, which races with a concurrent writer that has already produced fresh chunks for newer content. Route empty chunk sets through the same hash-checked `replace_document_chunks_if_current` path used for non-empty sets; the hash guard makes a stale delete a no-op once the document has moved on. Add 2 regression tests using a recording mock. 3. backend.rs: `RepositoryMemoryBackend` ignored its host-resolved `MemoryContext` and forwarded the separately supplied path/scope to the repository, so a direct caller of this public seam that authorized one context but passed a path or scope for a different tenant/user/agent/project would bypass the boundary. Add `ensure_path_matches_context` / `ensure_scope_matches_context` guards before any repository side effect on read, write, compare-and-append, and list. Add 5 regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): native schema + empty repo wiring (PR 2 of #3118) (#3181) * docs(reborn-memory): reorient guardrails to native-isolated direction Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112 and asks for a native, isolated Reborn memory subsystem with explicit tenant/user/agent/project scope columns rather than synthetic workspace user_id strings. Update crates/ironclaw_memory/CLAUDE.md to reflect that decision: - Drop reuse-first / preserve-existing-tables wording. - State that src/workspace is reference material only and that ironclaw_memory must not depend on the main app crate. - Spell out the full (tenant, user, agent, project, path) scope invariant on every read/list/search/write/version/chunk operation. - Document the empty-string DB sentinel for absent agent/project columns and call out that _none is the virtual-path-only sentinel. - Note that legacy migration/coexistence is explicitly deferred and that Postgres needs real (not compile-only) behavioral coverage. No code changes; this PR is purely doc reorientation that unblocks the native-substrate work in PR 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn-memory): split lib.rs into focused modules Phase 2 of #3118. Make the crate agent-friendly before the deeper native-substrate changes by splitting the 3801-line crates/ironclaw_memory/src/lib.rs into 13 focused modules. No behavior change: every public name keeps its identity, the four test files under crates/ironclaw_memory/tests/ are untouched, and all 42 existing tests still pass. New layout: src/lib.rs (34 LoC) re-export hub + crate docs src/path.rs scope, path, parsing, validation, error helpers src/metadata.rs DocumentMetadata, .config inheritance src/embedding.rs EmbeddingProvider + vector helpers src/schema.rs JSON schema validation src/search.rs search request/result + RRF/weighted fusion src/chunking.rs ChunkConfig + chunk_document + content_sha256 src/backend.rs MemoryBackend + RepositoryMemoryBackend src/filesystem.rs RootFilesystem adapters src/indexer.rs MemoryDocumentIndexer + ChunkingIndexer src/repo/mod.rs MemoryDocumentRepository + shared repo helpers src/repo/in_memory.rs src/repo/libsql.rs (cfg(libsql)) src/repo/postgres.rs (cfg(postgres)) Visibility rules: - Public items keep their public names (verified against the imports in all four crates/ironclaw_memory/tests/*.rs files). - Helpers shared across modules become pub(crate). - libSQL- and Postgres-only schema constants and helpers stay inside their respective repo files. Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration This split is the foundation for the follow-up PRs that add the native reborn_memory_* schema, libSQL/Postgres repositories on the native schema, ported semantic/search/versioning tests, and host wiring (#3118 phases 3-7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): add native reborn_memory_* schema + empty repo wiring Phase 3 of #3118, PR 2 of the stack. Add the dedicated reborn_memory_* substrate that the rest of the stack will build on, without yet implementing any read/write/list/search behavior. The native schema lives inside the crate (consistent with the existing `LibSqlMemoryDocumentRepository::run_migrations` pattern where the crate manages its own DDL in isolation from the main app's migration system). The legacy `memory_documents` table is deliberately untouched — Reborn memory must not depend on or piggy-back on the legacy schema. Added: src/repo/native_libsql.rs - RebornLibSqlMemoryDocumentRepository (compiles, errors on every op) - run_migrations() materializes the substrate - REBORN_LIBSQL_MEMORY_DOCUMENTS_SCHEMA (4 tables + indexes + triggers) src/repo/native_postgres.rs - RebornPostgresMemoryDocumentRepository (compiles, errors on every op) - run_migrations() materializes the substrate - REBORN_POSTGRES_MEMORY_DOCUMENTS_SCHEMA (4 tables + GIN/HNSW indexes) tests/reborn_native_schema_contract.rs - reborn_libsql_run_migrations_creates_native_substrate_idempotently - reborn_libsql_repository_fails_closed_until_behavior_lands Schema design (matches issue #3118 exactly): reborn_memory_documents tenant_id TEXT NOT NULL user_id TEXT NOT NULL agent_id TEXT NOT NULL DEFAULT '' -- empty string is the project_id TEXT NOT NULL DEFAULT '' -- DB-only "absent" sentinel; path TEXT NOT NULL -- _none stays virtual-path-only UNIQUE (tenant_id, user_id, agent_id, project_id, path) reborn_memory_chunks -- chunks with content_hash + optional embedding reborn_memory_chunks_fts -- FTS5 virtual table on content (libSQL) -- TSVECTOR + GIN + HNSW (Postgres) reborn_memory_document_versions -- monotonic version per document, cascade Indexes cover full-scope lookup, scope+path lookup, list/search by scope, chunk lookup by document_id, FTS lookup, and version lookup by (document_id, version DESC). Repositories return a clear "reborn-native ... is not yet implemented" `FilesystemError::Backend` for every read/write/list/ search/index operation. Callers fail closed instead of treating an empty result as authoritative. Verification (all green, 44 tests pass): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): native libSQL repository behavior (PR 3 of #3118) (#3182) * feat(reborn-memory): implement native libSQL repository behavior Phase 4 of #3118, PR 3 of the stack. Stacks on PR 2 (#3181). Replace the `not yet implemented` stubs in `RebornLibSqlMemoryDocumentRepository` with a full read/write/list/search/version/chunk implementation against the `reborn_memory_*` substrate. Every query carries the full `(tenant_id, user_id, agent_id, project_id)` scope tuple. Absent agent/project IDs use the empty-string DB sentinel; the constructor rejects empty / `_none` user-supplied IDs, so equality (`agent_id = ?`) is unambiguous and NULL/IS NULL gymnastics are gone. Implementation: - read_document, read_document_metadata, list_documents: filter by full scope tuple + path; native rows reconstruct via `reborn_memory_document_from_row` (which round-trips empty-string sentinel back to `None`). - write_document, write_document_with_options: BEGIN IMMEDIATE -> reborn_libsql_list_paths_for_scope -> conflict check -> insert-or-update; computes content_hash on every write; writes prior content to reborn_memory_document_versions if the content changed and skip_versioning is not set. - write_document_metadata: scoped UPDATE of metadata column. - search_documents: FTS branch joins through reborn_memory_chunks_fts with full-scope WHERE; vector branch reads embeddings, computes cosine, sorts deterministically (path tiebreaker), then RRF / weighted-score fusion. - replace_document_chunks_if_current: re-reads content_hash inside the transaction; no-op if it has drifted (prevents corrupting the index after a between-read-and-refresh rewrite). - delete_document_chunks: scoped DELETE. Reusable helpers added to `repo/mod.rs` and shared by both native backends: - REBORN_SCOPE_NONE_SENTINEL ("") - reborn_agent_id_db_value, reborn_project_id_db_value - reborn_memory_document_from_row(tenant, user, agent_db, project_db, db_path) 13 new behavioral tests in `tests/reborn_native_libsql_repository_contract.rs`: - round_trips_a_document_within_full_scope - returns_none_when_document_is_missing - upsert_replaces_content_for_same_full_scope_and_path - full_scope_isolates_tenant_user_agent_project_independently - top_level_projects_path_is_a_normal_user_path_not_project_scope - rejects_file_directory_prefix_conflicts_within_scope - writes_metadata_and_reads_it_back_for_native_documents - write_with_options_creates_version_row_only_when_not_skipped - version_numbers_are_monotonic_and_content_hash_matches_archived_content - replace_chunks_if_current_is_a_noop_when_document_was_rewritten - full_text_search_returns_only_chunks_within_full_scope - fts_query_escapes_punctuation_and_handles_empty_input_gracefully - full_text_search_uses_rrf_when_only_full_text_branch_returns_results The `reborn_libsql_repository_fails_closed_until_behavior_lands` smoke test from PR 2 is removed (its purpose — proving stubs were fail-closed before behavior landed — is now obsolete). Postgres native repository remains stubbed; PR 4 implements it on the same substrate with a behavioral testcontainer harness. Verification (all green; 56 memory tests pass): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) (#3183) * feat(reborn-memory): implement native Postgres repository behavior Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e4821). Replace the `not yet implemented` stubs in `RebornPostgresMemoryDocumentRepository` with a full read/write/list/search/version/chunk implementation against the `reborn_memory_*` Postgres substrate. Every query carries the full `(tenant_id, user_id, agent_id, project_id)` scope tuple. Absent agent/project IDs use the empty-string DB sentinel; the constructor rejects empty / `_none` user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and NULL/IS NULL gymnastics are unnecessary. Implementation: - read_document, read_document_metadata, list_documents: filter by full scope tuple + path; native rows reconstruct via `reborn_memory_document_from_row` (which round-trips empty-string sentinels back to `None`). - write_document, write_document_with_options: BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE -> reborn_postgres_list_paths_for_scope -> conflict check -> upsert -> optional version row -> COMMIT. Computes content_hash on every write; archives prior content into `reborn_memory_document_versions` if the content changed and `skip_versioning` is not set. - write_document_metadata: scoped UPDATE of metadata column. - search_documents: FTS branch joins through `reborn_memory_chunks` with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector branch uses pgvector `<=>` ordering. Both branches are pre-fusion ranked; results are fused through the shared `search::fuse_memory_search_results`, honoring `FusionStrategy`. - replace_document_chunks_if_current: SELECT … FOR UPDATE row lock, hash drift check (no-op if the document was rewritten under it), delete-then-insert chunks with optional pgvector embedding. - delete_document_chunks: scoped DELETE through `reborn_memory_documents` join. - run_migrations: wrapped in a session-level `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent processes (parallel test runs) serialize on `CREATE EXTENSION pgcrypto/vector`. Behavioral coverage in `tests/reborn_native_postgres_repository_contract.rs` (13 tests at parity with the libSQL contract). Tests use the standard `DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern with `try_connect` skipping cleanly when Postgres is unreachable (matches `tests/workspace_integration.rs`); each test scopes a unique tenant prefix and cleans up via `cleanup_tenant` so parallel runs do not collide: - round_trips_a_document_within_full_scope - returns_none_when_document_is_missing - upsert_replaces_content_for_same_full_scope_and_path - full_scope_isolates_user_agent_project_independently - top_level_projects_path_is_a_normal_user_path_not_project_scope - rejects_file_directory_prefix_conflicts_within_scope - writes_metadata_and_reads_it_back - write_with_options_creates_version_row_only_when_not_skipped - version_numbers_are_monotonic_and_content_hash_matches_archived_content - replace_chunks_if_current_is_a_noop_when_document_was_rewritten - full_text_search_returns_only_chunks_within_full_scope - fts_query_escapes_punctuation_and_handles_empty_input_gracefully - full_text_search_uses_rrf_when_only_full_text_branch_returns_results `plainto_tsquery('english', $5)` already tolerates arbitrary punctuation, so the Postgres FTS-escape test asserts the same contract as the libSQL counterpart against the unescaped path. Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo clippy --all --tests --examples --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --features postgres cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Postgres tests run for real against a local DB when DATABASE_URL is reachable; otherwise skip via `try_connect` so CI without a database remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn-memory): port pure-behavior contract over native repos (PR 5 of #3118) (#3184) * test(reborn-memory): port pure-behavior contract over native repos Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a behavior contract for `RepositoryMemoryBackend` layered over both `RebornLibSqlMemoryDocumentRepository` and `RebornPostgresMemoryDocumentRepository`. The semantics under test live above the bare repository — they belong to the `RepositoryMemoryBackend` composition (`.config` inheritance, schema validation, indexer best-effort, capability fail-closed, embedding-dimension guard). These behaviors already exist in production code; PR 5 is purely test coverage that ports the matching contract from the legacy `db_memory_repository_contract.rs` over to the native `reborn_memory_*` substrate. No production-code changes. New file `tests/reborn_native_repository_backend_contract.rs`: libSQL (10 tests, in-process temp DB): - libsql_backend_inherits_config_metadata_for_skip_indexing - libsql_backend_closer_config_overrides_parent_config - libsql_backend_honors_skip_versioning_from_config - libsql_backend_validates_schema_from_config_before_write - libsql_backend_reports_write_success_when_indexer_fails_after_persist - libsql_backend_search_fails_closed_for_unsupported_vector_search - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch - libsql_backend_hybrid_search_fuses_full_text_and_vector_results - libsql_backend_weighted_score_fusion_orders_results_by_weights - libsql_backend_search_honors_limit Postgres (4 tests, skip-on-unreachable per `tests/workspace_integration.rs:26-33`; each test scopes a unique tenant prefix and cleans up via `pg_cleanup_tenant`): - postgres_backend_validates_schema_from_config_before_write - postgres_backend_honors_skip_versioning_from_config - postgres_backend_reports_write_success_when_indexer_fails_after_persist - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch Coverage delta vs. PR 4: | Behavior in #3118 phase 6 | PR 5 | |--------------------------------------------------------|------| | `.config` metadata inheritance precedence | ✓ | | Closer `.config` overrides parent `.config` | ✓ | | `skip_indexing` from `.config` | ✓ | | `skip_versioning` from `.config` | ✓ | | Schema validation before persistence | ✓ | | Indexer failure after persist => write success | ✓ | | Capability fail-closed for vector search | ✓ | | Query embedding dimension mismatch fails closed | ✓ | | RRF fusion (hybrid) | ✓ | | WeightedScore fusion | ✓ | | `limit` honored | ✓ | | FTS query escaping (already covered in PR 3 / PR 4) | n/a | | `version` hash semantics (already covered in PR 3/4) | n/a | Verification (all green): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo clippy --all --tests --examples --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Postgres tests run for real against a local DB when DATABASE_URL is reachable; otherwise skip via `pg_try_connect` so CI without a database remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185) * test(reborn-memory): vertical integration through public seams Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add caller-facing vertical-integration coverage that drives the full public-seam stack against the Reborn-native repositories: reborn-native repository -> ChunkingMemoryDocumentIndexer -> RepositoryMemoryBackend -> MemoryBackendFilesystemAdapter -> CompositeRootFilesystem mounted at /memory These behaviors already exist in production code; PR 6 is purely test coverage that proves the Phase 7 invariants hold against the native substrate. No production-code changes. `tests/reborn_native_filesystem_vertical_integration.rs` — public-seam vertical: libSQL (6 tests, in-process temp DB): - libsql_round_trips_authorized_read_write_through_composite_mount - libsql_lists_direct_children_through_composite_mount - libsql_search_through_composite_mount_returns_only_same_scope_results - libsql_capability_denied_search_fails_closed_before_repository_is_called - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error - libsql_composite_mount_rejects_duplicate_root_at_registration Postgres (2 tests, skip-on-unreachable; per-tenant cleanup): - postgres_round_trips_authorized_read_write_through_composite_mount - postgres_search_through_composite_mount_returns_only_same_scope_results This amend additionally closes thoroughness gaps identified in a Required-Tests audit of #3118. None of these are production behavior changes — every gap is a missing assertion on behavior the production code already implements: `tests/reborn_native_repository_backend_contract.rs`: - Strengthen skip_indexing test to assert chunk-count = 0 in DB instead of indexer-call-count = 1 (uses real ChunkingMemoryDocumentIndexer that respects skip_indexing). Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing` -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`. - Add `libsql_backend_document_metadata_overrides_inherited_config`: parent .config says skip_versioning=true, doc-level metadata overrides to false; overwrite must produce a version row. - Strengthen weighted-score test to assert ordering changes when weights flip (FT-heavy vs vector-heavy produces different top result, proving weights actually steer the score). - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch` asserting result count is bounded by an effective pre_fusion_limit. - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test locking the clamp invariant of `with_pre_fusion_limit`). `tests/reborn_native_libsql_repository_contract.rs`: - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`: two `tokio::join!`-launched writes serialize via `BEGIN IMMEDIATE`; assert exactly one row in `list_documents`. - Add `fts_query_with_only_stopwords_does_not_error` covering the issue's "empty/stopword-ish" FTS case. `tests/reborn_native_postgres_repository_contract.rs`: - Add `same_path_in_different_tenants_stores_separate_rows` (Postgres parity for the tenant axis the libSQL contract already covered). - Add Postgres mirrors of the concurrent-writes and stopword-FTS tests above. `src/search.rs`: - Add unit tests for `fuse_memory_search_results` deterministic tiebreak: tied scores must sort by relative path ascending, independent of insertion order. Coverage delta for issue #3118 "Required tests": | Required test (issue) | Status | |--------------------------------------------------------------|--------| | Same path different tenants/users/agents/projects | ✓ | | Search/list scope isolation | ✓ | | projects/ prefix as user path | ✓ | | Same scope+path upserts | ✓ | | Different scope+same path -> different docs | ✓ | | File/directory prefix conflict | ✓ | | Concurrent writes -> exactly one row | ✓ | | Indexer failure after persist -> write succeeds | ✓ | | .config applies to children | ✓ | | Closer .config overrides parent | ✓ | | Document metadata overrides inherited .config | ✓ | | skip_indexing -> chunk count = 0 | ✓ | | skip_versioning -> no version row | ✓ | | Schema validation rejects before persistence | ✓ | | Write -> chunk -> FTS end-to-end | ✓ | | FTS escaping (punct + stopwords) | ✓ | | Vector dim mismatch | ✓ | | Hybrid ranking deterministic (path tiebreak) | ✓ | | RRF + WeightedScore fusion | ✓ | | limit + pre_fusion_limit | ✓ | | Version monotonic + content hash | ✓ | | Adapter / RootFilesystem vertical integration | ✓ | | Unsupported capability fails closed before side effects | ✓ | Acceptance criteria from #3118 closed by this PR: - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` / `RootFilesystem`) have vertical integration coverage against native repositories. Verification (all green; second run confirms reliability — first run hit a pre-existing parallel-execution flake in the legacy `db_memory_repository_contract` target, unrelated to this PR; passes in isolation and on re-run): cargo fmt --all -- --check cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings cargo clippy --all --tests --examples --all-features -- -D warnings cargo test -p ironclaw_memory --features libsql cargo test -p ironclaw_memory --all-features cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD git diff --check origin/reborn-integration Postgres tests run for real against a local DB when DATABASE_URL is reachable; otherwise skip via `pg_try_connect` so CI without a database remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3185 review findings Three medium-severity issues raised on the vertical integration PR: 1. Postgres vertical tests in reborn_native_filesystem_vertical_integration were silently skipped when the pool could not hand out a connection, so the test binary reported them as passed even when Postgres was unreachable. Replace the silent-skip helper with `pg_require_connection` that panics with an actionable message unless the new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the `ironclaw_memory` guardrail that Postgres behavioral coverage must be real, not compile/skip coverage. 2. The Postgres vertical search test built `RepositoryMemoryBackend` without a `ChunkingMemoryDocumentIndexer`, so writes through `CompositeRootFilesystem` never populated `reborn_memory_chunks`. Search returned an empty Vec, and the assertion `paths.iter().all(|p| p == "visible.md")` was vacuously true even without proving anything about the write -> index -> search flow. Wire the same chunking indexer the libSQL vertical stack uses (FTS-only — no embedding provider), and add an explicit non-empty assertion before the scope check. Verified end-to-end against a `pgvector/pgvector:pg16` container. 3. The `pre_fusion_limit_is_clamped_up_to_limit` test in reborn_native_repository_backend_contract claimed the invariant `pre_fusion_limit >= limit` held regardless of caller order, but only exercised `with_limit().with_pre_fusion_limit()`. The reverse case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch SQL `LIMIT` shrink below the requested final limit. Make `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3184 review findings Three medium-severity issues raised on the behavior-contract PR: 1. Postgres backend tests in reborn_native_repository_backend_contract were silently skipped when the pool could not hand out a connection, so the test binary reported all Postgres tests as passed without running migrations or exercising any of the schema validation, versioning, indexer, or search behavior against Postgres. Replace the silent-skip helper with `pg_require_connection` that panics with an actionable message unless the new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the `ironclaw_memory` guardrail that Postgres repository coverage must be real, not compile/skip coverage. 2. The `libsql_backend_search_honors_limit` and `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests only asserted `results.len() <= 2`, which is vacuously satisfied by 0 or 1 results — a regression that broke FTS or indexing entirely would still pass. Establish the precondition with a larger-limit baseline search asserting the full match count is visible, then assert the bounded search returns exactly 2. 3. The new search/fusion/limit contract was libSQL-only, while `RebornPostgresMemoryDocumentRepository` has its own FTS (`tsvector`) and pgvector query implementations. Add Postgres counterparts for hybrid search, weighted-score fusion, pre-fusion-limit per-branch, and the `limit` truncation cases. The Postgres weighted-score test uses asymmetric FTS content (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks the FTS leader unambiguously above the hybrid match — without this, `ts_rank_cd` ties non-deterministically. Generalize `RecordingEmbeddingProvider` to be parametric on `DIM` so the same provider semantics work against libSQL (DIM=3, blob embeddings) and Postgres (DIM=1536, fixed-width pgvector column). Verified end-to-end against a `pgvector/pgvector:pg16` container: all 105 memory tests pass, zero clippy warnings, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3183 review findings Two review issues raised on the native Postgres repository PR: 1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional `delete_document_chunks` for both `skip_indexing == Some(true)` and the empty-chunks case. A stale reindex (older bytes, older skip_indexing flag) racing with a concurrent writer that has already produced fresh chunks for newer content could clobber the newer write's chunk rows, leaving the latest content unsearchable. Compute `content_hash_at_read` up front and route both paths through the same hash-guarded `replace_document_chunks_if_current` call. The hash guard turns a stale clear into a no-op once the document has moved on. `delete_document_chunks` is no longer reached by the indexer; the trait method stays in place to avoid churning the API across the stack. Add 3 regression tests in `mod tests` covering the skip_indexing path, the empty-chunks path, and the non-empty path, each asserting routing through `replace_document_chunks_if_current` with the read-time content hash. 2. The Postgres contract harness silently skipped every test when the pool could not hand out a connection — the binary reported all tests as passed without running migrations or exercising any read/write/search/chunk/version behavior, violating the `ironclaw_memory` guardrail that Postgres coverage must be real. Make `try_connect` panic with an actionable message unless the new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set. Verified end-to-end against a `pgvector/pgvector:pg16` container: all 108 memory tests pass, zero clippy warnings, fmt clean. Verified fail-loud and opt-in-skip behavior with and without DATABASE_URL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn-memory): remove unguarded delete_document_chunks API Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous fix routed the indexer through `replace_document_chunks_if_current` so the live race the reviewer described (stale reindex clobbering chunks from a newer write) was closed, but the unguarded `delete_document_chunks` method was still on the trait and in 4 implementations as a footgun for any future caller. Remove it entirely: - Drop the trait method from `MemoryDocumentIndexRepository` and document at the trait level that chunk clearing is folded into `replace_document_chunks_if_current` (called with an empty `chunks` slice). A hash-guarded clear is the only safe shape here. - Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`, `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the `libsql_document_id_and_content` helper that became dead with the legacy libsql impl. - Remove the test mock's stub and the `IndexerCall::Delete` variant in the indexer regression tests; the recording repo now only implements the hash-checked path, so any future regression that starts calling something else fails to compile. Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory tests pass, zero clippy warnings, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): address PR #3181 review findings Issue #2 of the two raised on the native-schema PR — silent skip in the schema contract tests. (Issue #1, fail-closed metadata methods, was mooted when PR3 squash-merged into this branch's history and replaced the trait-default Ok stubs with real implementations on both `RebornLibSqlMemoryDocumentRepository` and `RebornPostgresMemoryDocumentRepository`.) The reviewer flagged that `reborn_native_schema_contract.rs` only defined libSQL tests — running with `--features postgres` compiled and ran zero tests, leaving the `pgcrypto`/`vector` extensions, the generated `tsvector` column, the HNSW vector index, and the `reborn_memory_*` tables as compile-only DDL coverage. Add `reborn_postgres_run_migrations_creates_native_substrate_idempotently`: - Uses the now-standard `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var. Without it, an unreachable Postgres panics with an actionable message instead of skipping silently. - Calls `run_migrations()` twice to lock in idempotency. - Verifies both required extensions are installed. - Verifies the three `reborn_memory_*` tables exist. - Specifically verifies the generated `content_tsv` column has type `tsvector` and `is_generated = ALWAYS`. - Specifically verifies the `idx_reborn_memory_chunks_embedding` index uses HNSW. - Re-asserts the legacy `memory_documents` table is not created. Verified end-to-end against `pgvector/pgvector:pg16`: both schema tests pass, all 109 memory tests pass, zero clippy warnings, fmt clean. Verified the fail-loud-without-DB and opt-in-skip paths explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): native append, version attribution, unbounded vector Address three high/medium severity unresolved review comments on the native Reborn memory repositories: - libSQL/Postgres native repos now implement `compare_and_append_document_with_options` directly with hash conflict checks, version archival, path-conflict checks on insert, and the same transaction/locking the legacy backends use (BEGIN IMMEDIATE on libSQL; LOCK TABLE IN SHARE ROW EXCLUSIVE MODE + FOR UPDATE on Postgres). Without the override, atomic appends silently fell back to the trait default. - Direct-repo `write_document` now populates `changed_by` from a scoped owner key so version rows are never NULL-attributed when callers bypass the backend/filesystem seam. - `reborn_memory_chunks.embedding` is now `vector` (unbounded) instead of `vector(1536)` so non-1536 providers (Ollama 768/1024-dim, OpenAI 3072-dim, Claude 1024-dim, …) can write embeddings. The prior fixed dimension was a footgun. Schema contract test asserts the unbounded shape and that no HNSW index is reintroduced (HNSW requires fixed dimension). Adds tests at every tier the reviewer asked for: native repository contract tests (append + changed_by), filesystem vertical-integration tests that drive `append_file` through the composite mount, and a Postgres test that writes mixed-dimension embeddings to lock in the unbounded vector contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): per-scope advisory lock + per-file path class Address zmanian's two must-fix items on PR #3180 and the seven test gaps he flagged for follow-up. Must-fix: - H2: Replace the table-level `LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE` on the Postgres write/append paths with a per-scope advisory lock keyed on `hashtext('tenant:user:agent:project')`. Concurrent writes for different scopes no longer serialize globally; the unique constraint still prevents same-path duplicates and `FOR UPDATE` still pins the row. Exposes a latent H1 deadlock between concurrent `run_migrations` (DROP TRIGGER → AccessExclusiveLock) and writers (RowExclusiveLock); fixed by short-circuiting `run_migrations` when the schema is already present and gating the trigger setup behind a `pg_trigger` existence check so the table is never re-locked at AccessExclusive level on re-runs. - M2: `PromptProtectedPathClass::as_str()` now maps each default protected path to a distinct stable class string (`agents_md`, `soul_md`, `heartbeat_md`, …) instead of the single `system_prompt_file` bucket. Custom paths fall through to `custom_protected_path`; consumers that need the exact path use `relative_path()`. Test gaps: 1. Concurrent `replace_document_chunks_if_current` with the same `expected_content_hash` and different chunk sets — exactly one writer's set lands, no partial union (libSQL + Postgres). 2. Write/index-time embedding dimension mismatch — provider declares one dim, returns another; indexer fails closed before any chunks land while the durable document row still persists (libSQL + Postgres). 3. `BypassAllowed` path on `empty_prompt_file_clear` actually overwrites the prior content with the cleared (empty) content and records a `BypassAllowed` audit event; the `require_sink: true` ⇒ no-sink fail-loud path was already covered by existing tests. 4. Audit event records the *classifying* registry's policy version per-path, not always the default — locks in v1/v2 reconciliation during a policy bump. 5. Postgres tsvector with CJK content writes cleanly under the `'english'` config; English tokens remain searchable alongside CJK chunks. 6. Two `RebornPostgresMemoryDocumentRepository` instances on independent pools call `run_migrations` concurrently against a *cold* schema (per-test isolated Postgres schema) — both succeed under `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)`; the schema fingerprint count is exactly one. 7. End-to-end search with 60 candidates exercises the per-branch `pre_fusion_limit` SQL `LIMIT` cap and asserts a unique limit-respecting result set. Also surfaces tokio_postgres error source chains via a `pg_error_chain` helper so `FilesystemError::Backend.reason` fields contain SQLSTATE instead of the opaque `"db error"` string. This was load-bearing for diagnosing the H1 deadlock during this work and remains useful for future operators. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(memory): address serrrfirat review — harden reborn memory (#3180) * fix(reborn-memory): close 5 serrrfirat review items (3 HIGH safety, 2 MED data-destruction) Each fix has a regression test driving the original bug shape. HIGH `backend.rs:79` — public forgeable prompt-safety bypass `MemoryContext::with_prompt_write_safety_enforced()` was `pub`, so any direct backend caller could construct an "already-enforced" context and persist high-risk content to SOUL.md/BOOTSTRAP.md through `RepositoryMemoryBackend::write_document` without ever passing through policy. Made the builder `pub(crate)` so only the in-crate filesystem adapter can produce an enforced context, and added an anchor test for the default-unenforced invariant. HIGH `filesystem.rs:47` — sink config no longer flips the policy override `with_prompt_write_safety_event_sink()` was setting `prompt_safety_config_overridden = true`, which caused the adapter to take over enforcement and skip a stricter wrapped backend's policy. A host that wanted only a durable audit sink ended up bypassing the policy. Sink config is observability and is now scoped to itself; the override flag is only flipped by the explicit policy/registry builders. Regression: stricter `AlwaysRejectProtectedPolicy` on the backend still fires when the adapter only adds a sink. HIGH `filesystem.rs:196` — adapter must check the path-level enforce result `adapter_should_enforce_prompt_safety` was using `!backend_capabilities.prompt_write_safety` instead of `!backend_will_enforce_protected_path`. A backend that advertised the capability while reporting `prompt_write_safety_protects_path() == false` for adapter-classified protected files (SOUL.md, AGENTS.md, …) caused the adapter to skip its own check and the backend to do nothing. Both `write_file` and `append_file` now consult the path-level result. Regression: a custom backend with capability=true and protects_path=false is rejected by the adapter for SOUL.md high-risk content. MED `native_libsql.rs:801` — LIKE wildcards in path segments `MemoryDocumentPath` allows `%` and `_` in valid segments, but the metadata-clear path interpolated the parent prefix raw into a `LIKE pattern`. A `team_%/.config` write would also clear chunks for unrelated `team-a/note.md`. Added shared `escape_like_pattern` helper; both libSQL and Postgres metadata-clear queries now use `LIKE … ESCAPE '\\'`. Regression on both backends drives `team_%/` vs `team-a/`. MED `native_libsql.rs:531` — chunk-clear must be gated on row update + honor descendant overrides Two related issues: 1. `write_document_metadata` used to run the descendant chunk-clear even when the UPDATE matched zero rows. For a missing root `.config` the LIKE pattern was bare `%` → every chunk in the scope wiped. Now both backends capture `rows_affected` and gate the clear on it. 2. The descendant clear deleted chunks for documents whose own document-level metadata explicitly set `skip_indexing=false` (an override of the inherited parent .config). Both backends now exclude descendants with explicit `skip_indexing=false` via `json_extract` (libSQL) and `metadata->>'skip_indexing'` (Postgres). Two libSQL regressions cover both fixes. Already-closed items verified during review Two MED items zmanian flagged were already addressed by earlier work in this branch — verified with assertions and an existing test pass: - MED `backend.rs:342` — direct backend file ops fail closed when `file_documents=false` (`ensure_file_documents_supported` covers read/write/compare-and-append/list; existing `file_document_capability_rejects_direct_backend_file_operations`). - MED `backend.rs:561` — search path checks `capabilities.embeddings` before any `embed_text` call (existing `vector_request_fails_closed_when_embedding_generation_is_disabled`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn-memory): close remaining 9 serrrfirat review items Each fix has a regression test driving the bug shape. MED `native_postgres.rs:985` — vector search bound type cast The dimension-filtered vector search query bound `$5` without an explicit `::vector` cast, causing pgvector to fail with "function vector_dims(unknown) is not unique" because the function is overloaded on `vector` and `halfvec`. Added `$5::vector` casts on both the filter and the ORDER BY so the overload resolves cleanly. The filter itself (already in place) excludes mismatched-dim chunks. Postgres regression `vector_search_with_mixed_dimension_chunks_does_not_error` proves a mixed-dim scope returns the matching chunk and excludes the mismatched one without erroring the whole search. MED `native_libsql.rs:1232` — FTS backfill regression test The current code already issues `INSERT INTO reborn_memory_chunks_fts (reborn_memory_chunks_fts) VALUES ('rebuild')` after migration. Added a regression that drops the FTS table+triggers, leaves chunk rows intact, re-runs migrations, and asserts FTS search surfaces the pre-existing chunks. MED `indexer.rs:117` — embedding outage degrades to text-only chunks When `build_chunk_writes` fails (provider outage, dimension mismatch, …), the indexer used to clear chunk rows then return the error; backend writes intentionally swallow indexer errors after persistence, so a successfully-overwritten document disappeared from FTS too. Indexer now persists *text-only* chunks (embedding = NULL) via the hash-guarded replace, then surfaces the embedding error for observability. FTS stays current; vector search degrades. Updated the existing `embedding_failure_*_with_hash_guard` test to lock in the new contract; updated the dim-mismatch backend test to assert chunks have NO embedding rather than no chunk rows. MED `indexer.rs:122` — race-narrowing for concurrent skip_indexing Reindex used to read content + metadata, build chunks, and finally replace chunks guarded only by the content hash. A concurrent metadata write that flipped `skip_indexing=true` between the initial resolve and the replace had its chunk-clear undone (content hash unchanged → reindex re-inserted chunks). Indexer now re-resolves metadata immediately before the final replace and short-circuits to an empty chunk set when skip_indexing is now true. New `FlipsSkipIndexingMidwayRepo` mock + regression `concurrent_skip_indexing_metadata_write_wins_against_in_flight_reindex`. A truly race-free fix (atomic metadata-version-checked replace at the repo layer) is documented as a follow-up. MED `native_libsql.rs:491` — metadata-only writes invalidate chunks Trait method `write_document_metadata` already invalidates chunk rows when the new metadata sets skip_indexing=true (covered by the MED #33 fix). Documented the contract on the trait and added a libSQL regression `libsql_metadata_write_setting_skip_indexing_true_clears_existing_chunks` that pins it. Re-enabling indexing (true → false) requires the caller to drive a reindex; the repo can't run the chunker/embedder itself. MED `filesystem.rs:245` — backend skips re-rejection of adapter approval Already addressed: `RepositoryMemoryBackend::write_document/append` guards on `if !context.prompt_write_safety_enforced()`, and the adapter sets that flag on the backend context after running its own enforcement. Added regression `backend_does_not_re_reject_adapter_approved_warn_or_bypass` composing an `AlwaysRejectIfRecheckedPolicy` on the backend with a permissive default policy on the adapter — benign content reaches the wrapped repository because the backend's policy is short-circuited. MED `safety.rs:809` — sink errors only fatal when require_sink=true Already addressed: `emit_prompt_write_safety_event` checks `parts.require_sink` before converting a sink error into `PromptWriteSafetyEventUnavailable`. Added regression `configured_sink_failure_does_not_block_clean_allow_writes` proving an Allow outcome on a protected path with a failing sink still persists (only warn/bypass require a durable audit event). MED `postgres.rs:121` — legacy postgres agent_id binds as Uuid Already addressed: the legacy schema declares `agent_id UUID` in this file's `CREATE TABLE IF NOT EXISTS` (matching the V1 migration), and `scoped_memory_agent_uuid` parses the string into `Option<uuid::Uuid>` before binding. Added behavioural regression `postgres_memory_repository_round_trips_agent_scoped_writes_against_uuid_schema` that creates an agent-scoped path with a real UUID, writes through the repo, reads it back, and asserts the listing surfaces it — proving the UUID bindings work against a real Postgres instance. Bonus close-out The earlier fix in MED `indexer.rs:117` exposed that one existing backend test asserted "0 chunk rows after dim mismatch" — the new contract is "chunk rows exist but with NULL embeddings". Updated the assertion to count `embedding IS NOT NULL`, which is the actually load-bearing invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(memory): address reborn review guardrails --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: serrrfirat <f@nuff.tech>

Consolidated fixes for serrrfirat's 10 unresolved review threads plus zmanian's CHANGES_REQUESTED review (3 blockers + 7 mediums + 3 follow-up items + title typo). ## Blockers 1. CI now runs the package tests (zmanian blocker 2 / serrrfirat #7). The matrix `cargo test ${{ matrix.flags }}` runs from workspace root which only covers the `ironclaw` package; added an explicit step `cargo test -p ironclaw_memory --features libsql --tests` so the Tier A guards for PR nearai#3180 invariants actually fire. 2. `#[ignore]` markers converted to `#[cfg_attr(not(feature = "pr3180-ready"), ignore = ...)]` (zmanian blocker 1 / serrrfirat #1). Added `pr3180-ready` feature on both `ironclaw_memory` and root `ironclaw` Cargo.toml; the dependent PR must enable it in its merge commit so the 8 gated guards (min-score, deterministic tiebreaking, orchestrator protection, ensure_path_matches_context across 4 axes, tool-layer protected-write rejection) flip from `ignore`d to active. 3. Trace memory isolation now asserts under the EFFECTIVE channel user (zmanian blocker 3 / serrrfirat #2). Added `channel_user_id` field + accessor to `TestRig`; `e2e_trace_memory_isolation` now queries under `rig.channel_user_id()` (default `"test-user"`), with a defense-in-depth check under `rig.owner_id()` for mis-routing regressions. ## Test-correctness mediums 4. Min-score test pins `with_query_embedding([1,0,0])` to favor hybrid.md (serrrfirat #3 / zmanian #4). Removed the permissive `* 0.99` fallback — FTS-only-below-hybrid is now a hard assertion. 5. Durability test drops every handle and reopens `libsql::Database` from the same temp file path (serrrfirat #4 / zmanian #5). Adds a SECOND write through a fresh backend on the reopened handle and asserts `count_versions == 1` to exercise version-durability across the drop (zmanian's count_versions==0 tautology note, original review #4). 6. Append versioning asserts exact row count `== 1`, not `!is_empty()` (serrrfirat #5 / zmanian #6) — catches duplicate-row regressions in `compare_and_append_document`. 7. Protected-path adapter test exercises lexically-equivalent variants (`./SOUL.md`, `//SOUL.md`) in addition to canonical (serrrfirat #6 / zmanian #7). VirtualPath rejects `..` so no `..` variant; in-loop `count_documents_total == 0` after EACH variant. 8. Hybrid search isolation now varies all four scope axes (serrrfirat #8 / zmanian #8): tenant, user, agent, project. 5 documents seeded; search from caller scope must return exactly one. 9. Tool round-trip asserts EXACT persisted content via direct DB read (serrrfirat #9 / zmanian #9). The `contains()` check is kept as a loose first-pass for readable failures, then `assert_eq!` on the exact byte string is the load-bearing assertion. 10. Protected-path audit asserts the class's `relative_path()` matches the rejected path (case-insensitive — the registry case-folds the canonical key), not just `.is_some()` (serrrfirat #10 / zmanian #10). A regression that emits the wrong path class now fails. ## zmanian follow-ups Z1. Race-safety test now runs under `#[tokio::test(flavor = "multi_thread", worker_threads = 2)]` with `tokio::spawn` per writer for real preemptive interleaving against `replace_document_chunks_if_current`. Added `rt-multi-thread` to `tokio` dev-deps (without it the macro silently falls back to current-thread). Z2. `write_to_protected_path_rejected.json` trace fixture sets `all_tools_succeeded: false` explicitly. Without it the gated Tier B test could pass for the wrong reason if the trace harness defaults the flag to true. Z3. Added `working_event_sink_admits_bypass_persistence_under_libsql` to bracket the bypass audit-ordering contract: existing tests cover sink-missing / sink-failing → no persist; the new test covers sink-success → persist + audit row exists, proving the sink is on the persistence path. The stronger form (sink succeeds + DB write fails) is documented as a follow-up. ## Cleanup - Removed `_link_in_memory_repo_for_unused_imports` shim and the `InMemoryMemoryDocumentRepository` import that only existed to feed it (zmanian original-review #3). - Fixed PR title typo `momery` → `memory` via gh. Helper-consolidation into `tests/common/libsql_helpers.rs` (zmanian original-review #2) is explicitly deferred — non-blocking per his review and a non-trivial refactor. ## Verified - `cargo fmt --all -- --check` clean - `cargo clippy -p ironclaw_memory --features libsql --all-targets -- -D warnings` zero warnings - `cargo test -p ironclaw_memory --features libsql` all suites green (gated tests stay `ignored` without `--features pr3180-ready`)

Henry's review on PR nearai#3303 (2026-05-12T04:20:05Z) flagged that the SOUL.md tool-layer rejection guard does not run in CI because the trace test is gated on `pr3180-ready` but no workflow enables that feature. His proposed fix — enable the feature in CI now that the substrate (nearai#3180) is merged — would actually break CI: running with `--features pr3180-ready` against `reborn-integration` HEAD surfaces 7 failing tests, because nearai#3180 as merged is narrower than what the gated tests anticipate. None of `ensure_path_matches_context`, deterministic tiebreaking, `.system/engine/orchestrator/*` registration, or min_score-after-normalization exist in the substrate yet (verified by grep against `crates/ironclaw_memory/src/`). ## Changes 1. **Split the feature flag.** `pr3180-ready` now gates substrate-level guards that nearai#3180 was anticipated to deliver but didn't (5 tests). New `pr7-ready` gates the tool-dispatcher → substrate integration tests (1 test: the SOUL.md trace test) — distinct because the tool routing migration (PR 7) is a separate landing from the substrate work. The trace test's gate moves to `pr7-ready` accordingly. 2. **Updated gate messages with empirical reality.** Each gated test's ignore message now states what specifically is missing in the merged substrate (e.g., "no `ensure_path_matches_context` function in `crates/ironclaw_memory/src/`") so a future maintainer doesn't waste cycles enabling the flag and being confused by the failures. 3. **Always-running substrate-level SOUL.md rejection test.** Added `soul_md_high_risk_write_rejected_at_substrate_for_caller_tier_contract` in `e2e_scope_isolation_safety.rs`. Drives `RepositoryMemoryBackend` directly with the same scope+payload+policy shape PR 7's tool routing will use, asserts rejection + non-persistence + correct audit class. Closes Henry's CI gap at the substrate tier even before PR 7 lands; the trace test stays as the caller-tier guard for when the tool routing arrives. Distinct from `protected_paths_high_risk_writes_blocked_at_libsql_backend` (which loops over all 11 protected paths): a SOUL.md-by-name test catches a regression that selectively drops only SOUL.md from the protected-path registry — a real concrete attack surface that's worth pinning by name. ## Not changed - Neither feature is enabled in any CI workflow. Doing so today would surface the 7 substrate-test failures listed above, which aren't ours to fix in this PR. The followup substrate PR(s) must enable `pr3180-ready` in their merge commit; PR 7 must enable `pr7-ready`. - Henry's Low-priority note about `memory_read` tool-output exactness is acknowledged but not addressed here — DB-read covers persistence, and a tool-output helper is a non-blocking follow-up. - The Z3 bypass audit-ordering "sink succeeds + DB write fails" case remains a documented gap in `working_event_sink_admits_bypass_ persistence_under_libsql`; Henry flagged it but didn't escalate. ## Verified - `cargo fmt --all -- --check` clean - `cargo clippy -p ironclaw_memory --features libsql --all-targets -- -D warnings` zero warnings - `cargo test -p ironclaw_memory --features libsql` all suites green (gated tests stay `ignored` without `--features pr3180-ready` / `--features pr7-ready`) - New `soul_md_high_risk_write_rejected_at_substrate_for_caller_tier_ contract` test passes under default flags Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions Bot added scope: docs Documentation size: XL 500+ changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels May 1, 2026

serrrfirat reviewed May 2, 2026

View reviewed changes

Comment thread crates/ironclaw_memory/src/search.rs Outdated

serrrfirat reviewed May 2, 2026

View reviewed changes

Comment thread crates/ironclaw_memory/src/indexer.rs Outdated

serrrfirat reviewed May 2, 2026

View reviewed changes

Comment thread crates/ironclaw_memory/src/backend.rs

nickpismenkov and others added 3 commits May 4, 2026 13:59

nickpismenkov force-pushed the feat/reborn-memory branch from 0a74642 to 83a4c53 Compare May 4, 2026 21:26

nickpismenkov requested review from serrrfirat and zmanian May 4, 2026 23:07

zmanian reviewed May 5, 2026

View reviewed changes

serrrfirat reviewed May 5, 2026

View reviewed changes

Comment thread crates/ironclaw_memory/src/repo/native_libsql.rs

serrrfirat reviewed May 5, 2026

View reviewed changes

Comment thread crates/ironclaw_memory/src/repo/native_libsql.rs Outdated

serrrfirat reviewed May 5, 2026

View reviewed changes

Comment thread crates/ironclaw_memory/src/repo/native_postgres.rs Outdated

nickpismenkov and others added 3 commits May 5, 2026 14:26

Merge branch 'reborn-integration' into feat/reborn-memory

1175975

# Conflicts: # crates/ironclaw_memory/src/lib.rs

nickpismenkov requested review from serrrfirat and zmanian May 5, 2026 22:11

github-actions Bot mentioned this pull request May 6, 2026

🦞 OpenClaw 生态日报 2026-05-06 ivanweng2077/big_model_radar#2

Open