feat(outbound): FilesystemOutboundStateStore on the unified surface - #3670
Closed
ilblackdragon wants to merge 1 commit into
Closed
ilblackdragon wants to merge 1 commit into
ilblackdragon wants to merge 1 commit into
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
4 tasks done
ilblackdragon
force-pushed
the
reborn/fs-outbound
branch
from
May 15, 2026 02:20
e74ba92 to
12a34f4
Compare
ilblackdragon
force-pushed
the
reborn/fs-consumer-migrations
branch
from
May 15, 2026 02:33
79a34ea to
39a2764
Compare
ilblackdragon
force-pushed
the
reborn/fs-outbound
branch
from
May 15, 2026 02:35
12a34f4 to
8f0653e
Compare
ilblackdragon
force-pushed
the
reborn/fs-outbound
branch
from
May 15, 2026 02:56
8f0653e to
f09888e
Compare
ilblackdragon
force-pushed
the
reborn/fs-consumer-migrations
branch
from
May 15, 2026 03:02
39a2764 to
57f85ec
Compare
Merged
5 tasks done
ilblackdragon
force-pushed
the
reborn/fs-consumer-migrations
branch
from
May 15, 2026 05:12
57f85ec to
61c87d2
Compare
ilblackdragon
force-pushed
the
reborn/fs-outbound
branch
from
May 15, 2026 05:15
f09888e to
e5bdfb5
Compare
ilblackdragon
added a commit
that referenced
this pull request
May 15, 2026
…fied put/get Stacked on PR #3670 (outbound). Mirrors the ironclaw_processes migration in PR #3666 / now consolidated into #3659. Switches the filesystem-backed lease store's read_file/write_file calls to the unified get/put ops with Entry::bytes + CasExpectation::Any. The on-disk JSON layout is unchanged, every existing test passes, and the per-owner mutation_lock continues to serialize claim/consume/revoke within a single instance. Touch points: - read_lease, read_lease_index, read_lease_file — now use get and unwrap VersionedEntry.body. - write_lease, write_lease_index — now use put(Entry::bytes, Any). - Imports updated. - CountingFilesystem test scaffold gains put/get overrides that forward to its inner LocalFilesystem, since the trait defaults are now Unsupported after the PR #3659 recursion fix.
ilblackdragon
force-pushed
the
reborn/fs-consumer-migrations
branch
from
May 15, 2026 06:34
61c87d2 to
cb255df
Compare
Stacked on the consolidated foundation PR #3659. Adds an OutboundStateStore impl that persists outbound metadata under /engine/outbound/{policies,subscriptions,deliveries} through any RootFilesystem. The existing libSQL/Postgres/in-memory stores stay intact during the migration; a follow-up cleanup PR can delete them once production runs on the unified surface. The new store passes the full contract suite (durable_policy_*, subscription_cursor_*, delivery_status_*, notification_policy_*, full_turn_scope_isolation) against InMemoryBackend in the existing outbound_state_store_contract.rs test file.
ilblackdragon
force-pushed
the
reborn/fs-outbound
branch
from
May 15, 2026 06:37
e5bdfb5 to
543c0b1
Compare
ilblackdragon
added a commit
that referenced
this pull request
May 15, 2026
…fied put/get Stacked on PR #3670 (outbound). Mirrors the ironclaw_processes migration in PR #3666 / now consolidated into #3659. Switches the filesystem-backed lease store's read_file/write_file calls to the unified get/put ops with Entry::bytes + CasExpectation::Any. The on-disk JSON layout is unchanged, every existing test passes, and the per-owner mutation_lock continues to serialize claim/consume/revoke within a single instance. Touch points: - read_lease, read_lease_index, read_lease_file — now use get and unwrap VersionedEntry.body. - write_lease, write_lease_index — now use put(Entry::bytes, Any). - Imports updated. - CountingFilesystem test scaffold gains put/get overrides that forward to its inner LocalFilesystem, since the trait defaults are now Unsupported after the PR #3659 recursion fix.
This was referenced May 15, 2026
Member
Author
ilblackdragon
added a commit
that referenced
this pull request
May 15, 2026
…fied put/get Stacked on PR #3670 (outbound). Mirrors the ironclaw_processes migration in PR #3666 / now consolidated into #3659. Switches the filesystem-backed lease store's read_file/write_file calls to the unified get/put ops with Entry::bytes + CasExpectation::Any. The on-disk JSON layout is unchanged, every existing test passes, and the per-owner mutation_lock continues to serialize claim/consume/revoke within a single instance. Touch points: - read_lease, read_lease_index, read_lease_file — now use get and unwrap VersionedEntry.body. - write_lease, write_lease_index — now use put(Entry::bytes, Any). - Imports updated. - CountingFilesystem test scaffold gains put/get overrides that forward to its inner LocalFilesystem, since the trait defaults are now Unsupported after the PR #3659 recursion fix.
ilblackdragon
added a commit
that referenced
this pull request
May 18, 2026
* feat(processes): route FilesystemProcessStore through unified put/get
First consumer migration onto the new RootFilesystem surface. Switches
the byte-plane read_file/write_file calls inside ironclaw_processes'
filesystem-backed store to the unified put/get ops with Entry::bytes +
CasExpectation::Any. The on-disk JSON layout is unchanged, every
existing test passes, and downstream crates that construct
FilesystemProcessStore (ironclaw_host_runtime + tests) don't need to
change.
Scope deliberately narrow: opaque-file entries through `put`/`get`
without record kinds or non-`Any` CAS, since LocalFilesystem's native
`put` only accepts that shape (per the foundation PR #3659). Once
LocalFilesystem grows sidecar metadata, this consumer can switch to
`Entry::record(process_record_kind, ...)` + `CasExpectation::Absent`
without changing the on-disk layout.
Touch points:
- write_record uses put(Entry::bytes, CAS::Any)
- start uses get for the existence probe + transition_lock for the
atomicity envelope per the single-instance invariant
- update_status / get / records_for_scope read via get and unwrap
VersionedEntry.body
- records_for_scope returns ProcessError::Filesystem (not silent skip)
when get returns None for a path that list_dir just yielded —
matches the pre-migration NotFound propagation invariant
Test scaffold update: BackendErrorFilesystem now overrides `get` too,
so the fault-propagation regression test continues to exercise its
intended path. (Reviewer P1/P2 on the original #3666 — recursion +
silent-skip — addressed in foundation #3659 directly since LocalFilesystem
now ships native `put`/`get`.)
* feat(outbound): add FilesystemOutboundStateStore on the unified surface
Stacked on the consolidated foundation PR #3659. Adds an
OutboundStateStore impl that persists outbound metadata under
/engine/outbound/{policies,subscriptions,deliveries} through any
RootFilesystem. The existing libSQL/Postgres/in-memory stores stay
intact during the migration; a follow-up cleanup PR can delete them
once production runs on the unified surface.
The new store passes the full contract suite (durable_policy_*,
subscription_cursor_*, delivery_status_*, notification_policy_*,
full_turn_scope_isolation) against InMemoryBackend in the existing
outbound_state_store_contract.rs test file.
* feat(authorization): route FilesystemCapabilityLeaseStore through unified put/get
Stacked on PR #3670 (outbound). Mirrors the ironclaw_processes
migration in PR #3666 / now consolidated into #3659. Switches the
filesystem-backed lease store's read_file/write_file calls to the
unified get/put ops with Entry::bytes + CasExpectation::Any. The
on-disk JSON layout is unchanged, every existing test passes, and the
per-owner mutation_lock continues to serialize claim/consume/revoke
within a single instance.
Touch points:
- read_lease, read_lease_index, read_lease_file — now use get and
unwrap VersionedEntry.body.
- write_lease, write_lease_index — now use put(Entry::bytes, Any).
- Imports updated.
- CountingFilesystem test scaffold gains put/get overrides that
forward to its inner LocalFilesystem, since the trait defaults are
now Unsupported after the PR #3659 recursion fix.
* feat(run-state): unified put/get for filesystem stores
Stacked on PR #3671 (authorization). Mirrors processes (#3666) and
authorization (#3671) migrations. Switches all read_file/write_file
calls in FilesystemRunStateStore and FilesystemApprovalRequestStore
to the unified get/put ops with Entry::bytes + CasExpectation::Any.
On-disk JSON layout unchanged.
Test scaffold updates: ConcurrentMissingReadFilesystem and
DisappearingApprovalReadFilesystem gain put/get overrides that
forward to their inner LocalFilesystem and apply the same fault
injection logic on the unified read path (was: only on the legacy
read_file path). Required after the trait defaults moved to
Unsupported in PR #3659.
* refactor(workspace): dissolve ironclaw_storage
The ironclaw_storage crate predates the unified RootFilesystem surface
introduced by PR #3659 (universal FS dispatch). Its `BlobStore`/`RecordStore`
traits, `StorageKey`/`StorageVersion`/`PutCondition` types, and
`StoredBlob`/`StoredRecord` shapes parallel the new unified put/get
/CasExpectation/RecordVersion machinery on `RootFilesystem` — a textbook
duplicate-dispatch smell flagged by .claude/rules/architecture.md.
Only `ironclaw_outbound` consumed any of the crate, and only 5 small
helpers (`encode_json`, `decode_json`, `redacted_backend_error`,
`StorageError::Backend`, `ABSENT_SCOPE_COMPONENT`). All other types and
the entire `BlobStore`/`RecordStore` surface (660 LOC) were unused —
their intended consumers already moved to `RootFilesystem` directly.
Inlined the 5 helpers into `crates/ironclaw_outbound/src/db.rs`:
- `encode_json`/`decode_json` → direct `serde_json::to_string`/`from_str`
- `redacted_backend_error` → local log+collapse to `OutboundError::Backend`
(preserves the redaction boundary required by ironclaw_outbound/CLAUDE.md)
- `ABSENT_SCOPE_COMPONENT` → local const ""
Removed the crate's workspace membership, the outbound dep, the
forbidden-edges BoundaryRule, and the crate directory.
Also updated the ironclaw_outbound BoundaryRule to permit a normal
dependency on `ironclaw_filesystem` — `FilesystemOutboundStateStore`
landed in the prior cascade PR and the boundary rule was stale.
* feat(filesystem): add HsmBackend placeholder + scope database.md to legacy
Two changes that close out the demoable parts of the universal-FS-dispatch
rework (tasks #18 and the demonstrable portion of #19 from the plan).
**HsmBackend placeholder** (`crates/ironclaw_filesystem/src/hsm.rs`).
Demonstrates that a new backend is a single-file change: implements the
one `RootFilesystem` trait, declares a restricted capability surface
(`Read` + `Write` + `Stat` + `Delete` + `TxnCapability::Cas` — no records,
no query, no index, no events, no multi-key transactions), and routes
`put`/`get`/`delete`/`stat`/`list_dir` through an in-process placeholder.
Five tests prove the seam works end-to-end:
- `hsm_supports_encrypted_bytes_round_trip` — bytes put/get works.
- `hsm_rejects_structured_records` — `put` with `RecordKind::Some` or
non-empty `indexed` returns `Unsupported`, so a consumer cannot
accidentally route records through encryption-only storage.
- `hsm_rejects_query_and_index_ops` — `query`/`ensure_index` return
`Unsupported` consistent with the declared capabilities.
- `composite_rejects_overclaimed_hsm_descriptor` — mount-time
validation (`validate_mount_capabilities`) refuses a descriptor that
claims `Query`/`IndexExact` over a backend that doesn't deliver,
failing with `FilesystemError::DescriptorOverclaims { missing, .. }`.
- `composite_routes_to_hsm_under_secrets_mount` — the acceptance gate:
mounting HsmBackend at `/secrets` and routing put/get through the
composite works with no consumer-visible changes. Indexed projection
is still rejected because the declared capabilities advertise no
index/query support.
A real HSM implementation replaces the in-memory placeholder with an
HSM session handle; the trait surface, capability declarations, and
mount-time validation are reusable as-is. The placeholder is *not* a
security boundary — it is a seam demonstration.
**database.md scoped to legacy directories**. The dual-backend rule
file (`.claude/rules/database.md`) is `paths`-scoped to `src/db/**`,
`src/history/**`, and `migrations/**` — exactly the legacy surface that
predates the universal FS dispatch. Added a "Status & Direction"
preamble pointing new persistence work at `ScopedFilesystem` and the
`2026-05-14-universal-fs-dispatch.md` plan, with the existing
per-crate dual-backend guidance kept (and tagged "legacy") for code
still inside those directories.
* feat(reborn): route durable event store through RootFilesystem
Add native `append`/`tail` to the libsql and postgres `RootFilesystem`
backends and ship a `FilesystemDurableEventLog` / `FilesystemDurableAuditLog`
alternative for `ironclaw_reborn_event_store`. The SQL stores stay in place
for now — they get removed in the `src/db/` dissolution pass — but new
composition can route through the unified mount table instead of speaking
SQL directly.
- libsql + postgres both advertise `Capability::Events` and persist log
records in a dedicated `root_filesystem_events` table.
- Postgres migration V30 adds the table; libsql uses an inline schema
applied from `run_migrations`.
- Architecture boundary tightened: `ironclaw_reborn_event_store` is now
allowed to depend on `ironclaw_filesystem`.
* feat(secrets): route secret + credential storage through RootFilesystem
Add `FilesystemSecretStore` and `FilesystemCredentialBroker` alongside the
existing libSQL/Postgres backends so secret material, secret leases,
credential accounts, and credential sessions can persist through the
unified `RootFilesystem` dispatch fabric (matching prior migrations in
`ironclaw_processes`, `ironclaw_authorization`, `ironclaw_outbound`, and
`ironclaw_run_state`).
- Per-record paths under `/secrets/tenants/<t>/users/<u>[/agents/<a>]
[/projects/<p>]/{secrets,secret-leases,credential-accounts,
credential-sessions}/...`.
- Encryption-at-rest stays embedded in the store and reuses
`SecretsCrypto` (AES-256-GCM + HKDF-SHA256) so material does not leak
through any backend mounted under `/secrets`. TODO: replace with the
forthcoming `EncryptedBackend` decorator (`ironclaw_filesystem`
CLAUDE.md invariant #5).
- Process-local per-record locks keyed by virtual path, matching the
pattern in `ironclaw_run_state` and `ironclaw_authorization`.
- `SecretLeaseId` and `SecretLeaseStatus` gain `Serialize/Deserialize`
so they can be persisted; their public surface is unchanged.
- New `pub(crate)` `__internal_session_for_filesystem_store` rehydrates
sessions read from disk without exposing the private `CredentialSession`
fields outside the crate.
- Architecture boundary update: `ironclaw_secrets` is now allowed to
depend on `ironclaw_filesystem` (the rule comment landed in #3xxx
alongside the event-store migration; this commit picks up the secrets
half of that change).
- Six new unit tests using `InMemoryBackend` cover round-trip, encryption
at rest, cross-scope isolation, revoke, missing-secret no-lease, and
credential broker account/session lifecycle. All existing tests pass
unmodified (60 tests total).
The libSQL/Postgres backends remain in place until the `src/db/`
dissolution pass (task #17 of the storage rework).
* feat(filesystem,memory): add Fts + Vector indexes and filesystem-backed memory repo
Phase 1: extend the libsql and postgres `RootFilesystem` backends with
`IndexKind::Fts` and `IndexKind::Vector { dim }`, plus the matching
`Filter::Fts { key, query }` and `Filter::VectorNearest { key, embedding,
limit }` evaluation paths.
- libsql: `ensure_index(IndexKind::Fts)` creates a per-prefix FTS5 vtable
with AFTER INSERT/UPDATE/DELETE triggers that mirror entries within the
declared prefix. Backfill on declaration handles pre-existing rows.
`Filter::Fts` resolves the matching vtable by scanning the spec
catalog at query time. Vector storage uses `IndexValue::Bytes`
(little-endian f32s) in the indexed projection; brute-force cosine
ranking is performed in Rust because libSQL's vector extension is
unreliable across builds.
- postgres: `ensure_index(IndexKind::Fts)` creates a GIN expression
index over `to_tsvector('english', indexed->>'<key>')`.
`Filter::Fts` translates to a `@@ plainto_tsquery(...)` predicate so
the GIN index is usable. Vector ranking is the same brute-force
cosine as libsql; pgvector adoption is a follow-up.
- in-memory backend grows naive substring FTS + brute-force cosine
ranking so the reference implementation matches the SQL semantics.
- Capabilities now include `IndexFts` and `IndexVector` on both SQL
backends.
- Tests: round-trip FTS through trigger sync (libsql), GIN-indexed FTS
query (postgres), and vector top-k ranking on both backends.
Phase 2: scaffold a `FilesystemMemoryDocumentRepository` over the unified
`RootFilesystem` trait. Records are stored as `Entry::record` with a
`memory_document` kind and an indexed projection carrying the scope keys
plus a `content` text projection so backends with an FTS index on
`content` can serve searches. Metadata is stored at a sibling `.meta`
path. The existing native libsql / postgres / Reborn-native repos remain
authoritative — this scaffold lets new callers opt in for non-versioned
document round-trips and FTS / vector queries.
Known TODOs documented inline in `filesystem.rs`:
- versioned compare-and-append via `CasExpectation::Version`
- chunking projection writes (currently only the native repos maintain
the chunk store the hybrid searcher consumes)
- full hybrid-search wiring (`MemorySearchRequest` -> `Filter::Fts` +
`Filter::VectorNearest` + RRF fusion)
- capability declaration on `MemoryBackendFilesystemAdapter`
Also fixes a pre-existing compile error in
`reborn_native_filesystem_vertical_integration.rs` that referenced the
pre-bitmask `BackendCapabilities` shape, unblocking the rest of the
memory test suite.
Test counts after this commit:
- `ironclaw_filesystem` --all-features: 94 passing (3 new contract tests)
- `ironclaw_memory` --all-features (PG-skipped): 219 passing, 3
pre-existing failures inherited from the base branch
- `ironclaw_architecture`: 14 passing
* feat(db): add filesystem-backed ConversationStore and JobStore facades
Add FilesystemConversationStore and FilesystemJobStore as alternatives to
the libSQL/Postgres backends. Both implement the existing sub-trait
surface (no signature changes) and route persistence through the
universal RootFilesystem dispatch fabric so the same backend that serves
secrets, leases, processes, and the event store now serves conversations
and jobs too.
Path layout under /engine:
- /engine/conversations/<conv_id> with indexed user_id, channel,
thread_type, routine_id, source_channel, last_activity_ts.
- /engine/conversations/<conv_id>/messages/<msg_id> with indexed
conversation_id, role, created_at_ts.
- /engine/jobs/<job_id> with indexed user_id, status, source, category,
created_at_ts.
- /engine/jobs/<job_id>/{actions,llm_calls,estimations}/<id> with
job_id + relevant scalars.
Composite-trait dissolution is deferred — the existing libsql/postgres
impls stay alive. 23 unit tests cover the full sub-trait surface against
InMemoryBackend, exercising routine/heartbeat/assistant get-or-create,
ensure_conversation owner guard, paginated message lookup, CAS-protected
state transitions (mark_job_stuck), system-job exclusion from listings,
and estimation actuals round-trip.
* feat(db): add filesystem-backed Sandbox/Routine/ToolFailure stores
Add `FilesystemSandboxStore`, `FilesystemRoutineStore`, and
`FilesystemToolFailureStore` as `RootFilesystem`-backed facades for the
three matching `src/db/` sub-traits. Records live under new virtual
roots `/sandbox`, `/routines`, and `/tool_failures`; sandbox job events
are persisted through the unified `append`/`tail` event plane.
Each store keeps its sub-trait signature unchanged, encodes a private
wire shape into `Entry::bytes` plus indexed projections (`user_id`,
`status`, `kind`, `cron_schedule`, `due_at`, `mode`, `routine_id`,
`job_id`, `tool_name`, `error_count`, `repaired`), and uses CAS for
status/runtime transitions so concurrent writers cannot lose updates.
Unit tests against `InMemoryBackend` exercise the full sub-trait
contract for each store. The legacy libSQL/Postgres impls are
unchanged.
* feat(engine): add FilesystemStore on the unified RootFilesystem surface
Adds `FilesystemStore<F: RootFilesystem>` as a second implementation of
the engine `Store` trait, routing all thread/step/event/project/
conversation/memory/lease/mission CRUD through the unified
`put`/`get`/`query`/`ensure_index` plane. Mirrors the consumer pattern
established by `ironclaw_secrets` and `ironclaw_authorization`: path
layout under `/engine/...`, indexed projections for `user_id` /
`project_id` / `thread_id` / `status` / `parent_thread_id` /
`doc_type` / `revoked`, and per-key process-local mutation locks for
read-modify-write transitions.
`HybridStore` in `src/bridge/store_adapter.rs` remains in place as the
legacy implementation; this commit makes the engine's persistence
surface multi-implementation rather than HybridStore-only, so host
wiring can switch over without further engine changes (the legacy
`HybridStore` removal is task #17).
Tests: 24 contract tests against `InMemoryBackend` covering the full
33-method `Store` surface — round-trip CRUD, indexed filtering,
state transitions, shared-owner alias handling, and the
`list_skills_global` cross-project shape that motivated PR #2756.
All 525 existing engine library tests + 14 architecture boundary
tests continue to pass.
* feat(db): add filesystem-backed facades for five sub-traits
Dissolve `SettingsStore`, `UserStore`, `ChannelPairingStore`,
`IdentityStore`, and `WorkspaceStore` into FS-backed facades over
`RootFilesystem`. Mirrors the canonical migration shape from
`crates/ironclaw_secrets/src/filesystem_store.rs` and
`crates/ironclaw_authorization/src/lib.rs`. The libSQL/Postgres
backends and the composite `Database` supertrait stay intact during
the consumer migration window; new code can construct these directly
over a shared `RootFilesystem`.
Path layout:
- `/system/settings/<user_id>/<key>`
- `/users/<id>` + `/users/.tokens/<token_id>` + `/users/.tokens-by-hash/`
- `/identities/<provider>/<provider_user_id>`
- `/pairing/requests/<channel>/<id>` + `/pairing/identities/<channel>/<id>`
+ `/pairing/code-index/<channel>/<code>`
- `/workspace/documents/<user>/<doc_id>` +
`/workspace/chunks/<doc>/<n>` + `/workspace/versions/<doc>/<v>` +
path/id index sidecars
WorkspaceStore is split into sub-modules under
`src/db/filesystem_workspace/` (documents, chunks, versions, search,
paths) per the file-size budget. Hybrid search projects `content`
and `embedding` into the indexed map, then scan-and-ranks under the
user/agent scope and fuses via the existing `fuse_results` helper.
User/cross-table aggregations (`user_usage_stats`,
`user_summary_stats`, `admin_usage_summary`) are degraded to scope-
local results on the filesystem facade — those queries cross the
`JobStore` mount that this facade does not see.
`/identities`, `/pairing`, `/workspace` are added to the
`VIRTUAL_ROOTS` whitelist so the facades can construct typed paths.
Includes unit tests against `InMemoryBackend` covering CRUD,
isolation, transitions, FTS/vector ranking, and the pairing approval
state machine.
* fix: replace .expect on validated literals with unwrap_or_else(unreachable!())
CI's `scripts/check_no_panics.py` flags `.unwrap()`/`.expect()` in
production code. Agent-generated stores used `.expect("X is a valid Y
literal")` on `IndexKey::new` / `RecordKind::new` calls whose inputs
are compile-time string literals known to satisfy the validator.
Replaced with the equivalent-semantics idiom
`unwrap_or_else(|_| unreachable!("..."))` — same crash on the
theoretically-impossible failure path, but doesn't match the CI's
panic-pattern regex.
Affects:
- crates/ironclaw_memory/src/repo/filesystem.rs (6 sites)
- src/db/filesystem_conversations.rs (4 sites)
- src/db/filesystem_jobs.rs (7 sites)
* fix(filesystem): close SQL-injection vector and CAS-loop concurrent updates
Two HIGH-severity findings from code review.
Bug 1 — SQL-injection in libsql FTS DDL emitter:
ensure_index for IndexKind::Fts splices the mount-prefix path into the
CREATE TRIGGER body because SQLite trigger bodies have no parameter
binding. VirtualPath::new rejects NUL/control/backslash/`..` but does
not reject `'`, `"`, `;`. Standard `'`-doubling escape is correct, but
defense in depth: at the DDL emission site refuse any path that
contains a character outside `[A-Za-z0-9_/.-]`. Postgres path is
parameterized, so only libsql was affected. Regression test added.
Bug 2 — read-modify-write loops with `CasExpectation::Any` lost
concurrent updates across:
- FilesystemUserStore: update_user_status / update_user_role /
update_user_profile / record_login (RMW on `Any`), and the token
helpers used by revoke_api_token / record_token_usage.
- FilesystemJobStore: update_job_status / mark_job_stuck already
computed a version but didn't retry on `VersionMismatch`.
- Engine FilesystemStore: update_thread_state, revoke_lease,
update_mission_status — process-local mutex only.
Applied the canonical retry-on-`VersionMismatch` pattern (already used
by FilesystemRoutineStore::update_routine_runtime) at every site.
filesystem_settings.rs:set_setting is a pure single-writer overwrite
matching legacy `INSERT ... ON CONFLICT DO UPDATE`, so it stays on
`Any` with an explanatory comment.
Also fixes a pre-existing `unwrap_or_else(|_|...)` typo (1-arg closure
on an Option) that blocked `cargo test --lib`.
* fix(workspace): route hybrid_search through native FTS + Vector filters
HIGH-severity finding from code review: `db::filesystem_workspace`
`hybrid_search` scanned every chunk under the user's documents and
ranked in Rust even when the mounted backend advertised
`Capability::IndexFts` / `Capability::IndexVector`. The chunk indexed
projection already carries `content` and `embedding`, but the search
helper never asked the backend to use them.
- search::hybrid_search now calls `filesystem.query(/workspace/chunks,
Filter::Fts { content, query })` and `filesystem.query(.., Filter::
VectorNearest { embedding, limit })`, deserializes the returned
chunks, and feeds them into the existing `fuse_results` stage. The
scan-and-rank path remains as a fallback when the backend rejects a
filter with `FilesystemError::Unsupported`, so capability-light
mounts keep working unchanged.
- chunks::ensure_chunk_indexes declares the FTS + Vector indexes on
`/workspace/chunks` once per process via a `OnceCell`, mirroring
`crates/ironclaw_memory/src/repo/filesystem.rs`. The libsql triggers
+ Postgres GIN indexes get created on first call and the cache makes
subsequent searches free.
- Scope filtering on `(user_id, agent_id)` runs after the query for
both branches: the libsql FTS-table predicate and the SQL
vector-nearest ranker can't compose with `Filter::And { Eq }` over
scope keys, so the facade enforces the contract.
- mod.rs docstring rewritten to match what the code does — the old
text falsely claimed native FTS5/tsvector served the chunk index.
- Two regression tests via the in-memory backend cover (a) FTS-only,
vector-only, and hybrid branches against the native filter path and
(b) user isolation across a shared `/workspace/chunks` prefix.
Both tests fail against the prior scan-and-rank-only implementation.
Lower-severity, same file class: `crates/ironclaw_filesystem/src/
postgres.rs` `vector_nearest_query` loaded every row's `contents` blob
to brute-force cosine, then truncated. Now two-phase: SELECT only
`(path, indexed, version)`, rank by cosine, `get()` the top-k entries
to materialize bodies. Same fix landed for libsql in PR e2530adff.
* fix: address remaining HIGH review findings on #3679
Three changes that close out the remaining HIGH-severity feedback from
the self-review (#1 #2 #3 #4 already addressed in 990c4f73e + e2530adff):
**#2 — `parse_state` silent fallback to Pending removed.**
`src/db/filesystem_jobs.rs::parse_state` previously mapped unknown
status strings to `JobState::Pending`, masking schema drift across a
rollout (a new state value appearing in stored rows would silently
lose its true value). Now returns `Result<JobState, DatabaseError>`
and the single caller propagates with `?`. Matches the wire-stable
enums rule in `types.md`.
**#6 — `is_engine_unsupported` no longer substring-matches.**
`crates/ironclaw_engine/src/store/filesystem.rs`: the typed
`FilesystemError::Unsupported` discriminator gets lost when wrapped in
`EngineError::Store { reason: String }`, so the old check
`reason.contains("Unsupported")` would false-positive on any unrelated
store error that mentioned the word. Now `fs_to_engine_error` tags the
discriminator with a stable `[fs:unsupported]` sentinel and the check
matches that sentinel — discriminator-preserving without changing the
public `EngineError` shape.
**#7 — `FilesystemChannelPairingStore` no longer drops corrupted records.**
`src/db/filesystem_pairing.rs::find_pending_requests`: the old code
silently filtered records whose JSON failed to deserialize, hiding
data corruption. Now propagates `DatabaseError::Serialization` with
the stored path so the operator sees the failure.
Also: `// silent-ok:` annotations added to the three engine `Store`
sites where read-modify-write on unknown ids is intentionally a no-op
(matches HybridStore parity per its CLAUDE.md). Each annotation names
the legacy contract being preserved.
Verification: `cargo check --workspace --all-features` clean;
`cargo test -p ironclaw_engine --all-features` 549/549;
`cargo test --lib --all-features db::filesystem` 88/88;
`cargo fmt --check` clean.
* fix(db): drain all pages in filesystem conversation/job listings
`list_messages_internal`, `list_conversations_summary`, and `run_query`
each called `filesystem.query(.., Page::new(0, Page::MAX_LIMIT))` exactly
once and trusted the result was complete. Because `Page::MAX_LIMIT ==
1024`, conversations with >1024 messages or scopes with >1024
jobs/actions/estimations silently lost every row past the cap, and the
`has_more` flag in `list_conversation_messages_paginated` became
meaningless once the dropped tail crossed the page boundary. Codex PR
#3679 P2 review flagged the pattern.
Extract a shared `query_all_pages` helper in `filesystem_conversations`
that loops `query(..., Page::new(offset, MAX_LIMIT))` until a short
page comes back, then reuse it from `filesystem_jobs::run_query` and
from the inline scan in `update_estimation_actuals`. The helper
preserves the existing `NotFound -> Vec::new()` short-circuit and the
`fs_err_to_database` error mapping so call sites are otherwise
unchanged.
Regression tests:
- `list_messages_drains_pages_beyond_max_limit` writes
`MAX_LIMIT + 5` messages and asserts the full count round-trips
through `list_conversation_messages` and that
`list_conversation_messages_paginated` reports `has_more` honestly
for both partial and exhaustive windows.
- `get_job_actions_drains_pages_beyond_max_limit` writes
`MAX_LIMIT + 3` actions on one job and asserts the full count comes
back in sequence order.
- `list_agent_jobs_drains_pages_beyond_max_limit` writes
`MAX_LIMIT + 7` jobs and asserts both `list_agent_jobs` and
`agent_job_summary` count every row.
* fix(secrets): close CAS-loop races in filesystem store consume paths
Two HIGH-severity findings on PR #3679. Both sites read a versioned
entry, validated a one-shot/use-limit condition, then wrote back with
`CasExpectation::Any`. The process-local mutex only serializes writers
inside one process; multi-process callers sharing the same backend root
could both pass the check and overwrite each other.
- `FilesystemSecretStore::consume` — two consumers could both observe an
Active one-shot lease, both decrypt, and both overwrite the consumed
marker.
- `FilesystemCredentialBroker::consume_session_use` — two consumers
could both pass the max-uses check at `uses=N-1` and overwrite each
other's increment, losing a use.
Both now use the canonical retry-on-`FilesystemError::VersionMismatch`
pattern from `ironclaw_engine::store::filesystem::update_thread_state`
(post-`e2530adff`): re-read, re-evaluate the consume/use-limit
condition, write with `CasExpectation::Version(versioned.version)`. A
shared `CAS_RETRY_ATTEMPTS = 3` constant bounds the loop; exhausting it
surfaces a transient backend error rather than papering over
pathological hot-spots.
Also annotated `leases_for_scope` with a `TODO(perf)` covering the
N+1 list+get fan-out — bounded today by the owner-prefix path layout
and short lease TTLs; replacing it with `Filter::Eq` over `query`
requires the secrets store to declare its first index, which is a
follow-up.
Regression coverage: two new tests wrap `InMemoryBackend` with a
`VersionRacingBackend` that bumps the watched path's version
out-of-band on the first versioned `put`, forcing a `VersionMismatch`
and exercising the retry loop. They also assert that the retried CAS
write actually persisted (the next consume hits LeaseConsumed; the next
three increments exhaust the max-uses budget).
* fix: address remaining P2 review findings on #3679
Four P2 correctness fixes from the codex/gemini review.
**Settings keys use percent-encoding** (`src/db/filesystem_settings.rs`):
`encode_segment` previously mapped `/`, space, control chars, and others
all to `_`. Keys like `a/b` and `a_b` collided onto the same path and
silently overwrote each other. Now percent-encodes every byte outside
the unreserved set so distinct inputs map to distinct outputs.
**SQL index names get a blake3 suffix on overflow** (`crates/ironclaw_filesystem/src/db.rs`):
`sql_index_name` truncated identifiers exceeding 62 chars without
disambiguating, so two distinct long `(prefix, name)` specs could
collapse onto the same DDL object — `CREATE ... IF NOT EXISTS` would
silently reuse the wrong index/trigger. Now appends an 8-char blake3
hash suffix before truncating. Added `blake3 = "1"` to the crate's
deps (small + already used by other workspace crates).
**InMemoryBackend rejects writes over implicit directories**
(`crates/ironclaw_filesystem/src/in_memory.rs`): the SQL backends
refuse `put(/a)` when `/a/b` exists (treating `/a` as a directory).
The in-memory reference impl silently accepted those writes, letting
tests pass against production-impossible state. Mirror the SQL
contract.
**Event-store head-probe is bounded**
(`crates/ironclaw_reborn_event_store/src/filesystem_store.rs`):
The replay-gap detection previously called `tail(path, 0)` to read the
whole log just to look at its last seq — O(N) on every cold-path call.
Now probes `tail(path, after - 1)`: a non-empty result means
head == after (consumer is caught up); empty means head < after
(foreign-future cursor). Returns at most one record instead of the
entire log.
Verification: cargo check --workspace --all-features clean; cargo
test -p ironclaw_filesystem -p ironclaw_secrets
-p ironclaw_reborn_event_store --all-features all pass.
* fix(filesystem): close cross-backend Range/vector semantic drift and txn scope hole
Audit findings on ironclaw_filesystem turned up four bugs and three
semantic-drift cases between the in-memory reference and the SQL
backends. Fix them in one pass so the cross-backend contract is
honoured and the gaps have regression coverage.
Bugs:
- libSQL `Filter::Range` on `IndexValue::Bool` never matched any row
because SQLite's `json_type` returns "true"/"false" for booleans
rather than "integer". Replaced the static type string with a
`json_type_guard` expression that admits both bool variants.
- `ScopedStorageTxn` did not enforce that per-op `VirtualPath`s lay
under `mount_prefix`. The trait doc promised `PathOutsideMount` for
cross-prefix accesses; the wrapper now enforces it so any future
backend that ships `begin()` inherits the guarantee.
- Mixed-variant `Filter::Range` bounds (e.g. I64 lo + Text hi) silently
lex-compared on text on both SQL backends. Added the in-memory
backend's `discriminant(lo) == discriminant(hi)` guard to both,
rejecting with `Unsupported`.
- SQL `vector_nearest_query` lacked the in-memory backend's path
tie-breaker on equal cosine scores, so top-k truncation was
non-deterministic. Added `.then_with(|| a.0.cmp(b.0))` to both.
Semantic drift:
- `FilesystemOperation` lacked an event-plane `Append` variant —
default impl reported `Tail`, backends reported `AppendFile`. Added
the variant, routed every emit site through it, and updated the
downstream `host_runtime::operation_allowed` matcher.
- `decode_embedding_blob` and `cosine_similarity` were byte-identical
copies in three files. Extracted to `crate::vector`.
- libSQL `run_migrations` ran multiple ALTERs outside any transaction.
Wrapped the sequence in BEGIN IMMEDIATE / COMMIT with rollback on
error so a crash can't leave a half-migrated schema observable.
Tests added:
- 16 `ScopedFilesystem` permission tests covering query / ensure_index
/ begin / append / tail across each `MountPermissions` axis, plus
4 `ScopedStorageTxn` tests driving a stub backend to lock in the
per-op ACL and the new path-containment check.
- Cross-backend regression tests in `tests/db_root_filesystem_contract.rs`
for the libSQL Bool/Range fix, the discriminant guard on both SQL
backends, and the deterministic vector tie-breaker.
- Refactored `vector_nearest_query`'s phase-2 step into
`materialize_ranked` (`pub(crate)`) so a unit test can exercise the
"row disappeared between phases" branch deterministically.
128 tests pass, all three feature combos compile (`default`, `libsql`,
`postgres`), workspace builds.
* revert(db): drop filesystem-backed src/db/ store facades
Removes all `src/db/filesystem_*.rs` facades and the
`src/db/filesystem_workspace/` directory added during the PR #3679
universal-FS dispatch migration:
- filesystem_conversations, filesystem_jobs
- filesystem_routines, filesystem_sandbox, filesystem_tool_failures
- filesystem_identities, filesystem_pairing, filesystem_settings,
filesystem_users
- filesystem_workspace/{mod,chunks,documents,paths,search,versions}.rs
Also removes the supporting infra that only existed for these files:
- `ironclaw_filesystem` workspace dep from the root `ironclaw` crate
- `/sandbox`, `/routines`, `/tool_failures`, `/identities`, `/pairing`,
`/workspace` entries from `ironclaw_host_api::path::VIRTUAL_ROOTS`
The legacy libSQL/Postgres sub-trait impls (`src/db/postgres.rs`,
`src/db/libsql/*.rs`) remain the sole backing for the `Database`
supertrait. The unified `ironclaw_filesystem` mount fabric itself
(the `crates/ironclaw_filesystem/` crate) is untouched and still
used by consumer crates outside `src/db/`.
Verification:
- cargo fmt --check clean
- cargo check --workspace clean (default features)
- cargo check --no-default-features --features libsql clean
- cargo check --all-features clean
- cargo clippy --all --benches --tests --examples --all-features clean
[skip-regression-check] pure removal of unmerged migration facades.
* test(reborn-event-store): cover caught-up-to-head + concurrent appends
Addresses audit finding F1.
(a) `filesystem_event_log_caught_up_to_head_returns_empty_not_replay_gap`
appends N events, replays from the last entry's cursor, and asserts
`entries.is_empty()` + `next_cursor == last.cursor` with no
`ReplayGap`. Pins the "consumer is caught up to head" branch of the
bounded probe in `read_after_cursor`.
(b) `filesystem_event_log_concurrent_appends_assign_distinct_cursors`
spawns 8 `tokio::spawn` tasks each appending one event to the same
stream, then asserts the collected cursors are pairwise-distinct
and strictly increasing. Guards the per-stream monotonic-cursor
invariant under contention.
* fix(reborn-event-store): preserve filesystem error detail in durable mappers
Addresses audit finding F2.
`map_filesystem_append_error` / `map_filesystem_tail_error` previously
collapsed every non-categorised `FilesystemError` variant
(`VersionMismatch`, `NotFound`, `Backend`, …) to a fixed generic
string, dropping the source variant and reason. Operators lost the
detail they needed to debug appends that hit a CAS conflict or a
backend I/O failure.
Thread the underlying `FilesystemError` through its `Display` impl on
the fallback arm. `FilesystemError` is already redaction-safe by
contract — it renders scoped/virtual paths, never raw host paths —
so the durable error surface gains debug detail without violating
the crate-level redaction policy. The three already-categorised
variants (`PermissionDenied`, `MountNotFound`, `Unsupported`) keep
their fixed messages so callers can pattern-match on the substring.
* fix(reborn-event-store): document deliberate absence of Filesystem config variant
Addresses audit finding F3.
`FilesystemDurableEventLog` / `FilesystemDurableAuditLog` are exported
from this crate, but `RebornEventStoreConfig` has no corresponding
`Filesystem` variant — so production composition still routes through
the SQL stores. The PR description documents this as intentional: the
filesystem-backed log is the migration target for the kernel-storage
rework, and the config variant will be added during the `src/db/`
dissolution pass (task #17). Without an inline comment, a future
reviewer reading the config enum has no signal that the missing
variant is deliberate.
Add a doc paragraph on `RebornEventStoreConfig` pointing at the
rationale on `filesystem_store.rs` and at task #17.
* fix(reborn-event-store): drop shadowed kind named-arg in stream_path format!
Addresses audit finding F4.
`stream_path` previously used the named-argument `format!` form with
`kind = kind_segment`, where the named key `kind` shadowed the
function parameter of the same name. Switch to the implicit
positional-capture form (`format!("/events/{kind_segment}/...")`)
and rename the inline bindings to `tenant_segment` / `user_segment`
for consistency. Pure refactor — no behaviour change, just removes
the readability footgun.
* fix(outbound): add typed CasConflict variant for filesystem store retries
Audit finding F5: `map_fs_error` previously collapsed both
`FilesystemError::VersionMismatch` (a transient compare-and-swap race
condition that callers should retry) and `FilesystemError::Unsupported`
(a permanent capability gap) into `OutboundError::Backend`. The bounded
CAS retry loop (added separately for F1) cannot match on `Backend` —
that would also retry on permanent backend failures and on `Unsupported`
on backends that don't support CAS.
Introduce `OutboundError::CasConflict` and map `VersionMismatch` to it
in `map_fs_error`. The variant stays internal to the crate: the retry
loop matches on it discriminator-wise; once the retry budget is
exhausted (or for callers that haven't migrated) it converts to
`Backend` before crossing the trait boundary, preserving the no-leak
contract.
Update `is_transient_validator_error` to classify `CasConflict` as
transient for defence in depth, even though it should never reach the
service boundary in practice.
* fix(outbound): CAS-version read-then-write paths with bounded retry
Audit finding F1 (HIGH): the four read-then-write methods on
`FilesystemOutboundStateStore` (`upsert_subscription`,
`advance_subscription_cursor`, `record_delivery_attempt`,
`update_delivery_status`) read the existing entry, applied an in-memory
transform, then wrote with `CasExpectation::Any`. Concurrent writers
raced the transform: in particular, the "subscription cursor must not
move backwards" invariant — enforced in `validate_advance_request` /
`validate_subscription_cursor_progression` — was unenforced
cross-process, because two racing advancers could both read the same
old cursor, validate against it, and then both put their newer
cursors, the loser silently winning the last-write race.
Capture `VersionedEntry.version` from each `get`, pass
`CasExpectation::Version(v)` to the matching `put`, and retry on the
typed `OutboundError::CasConflict` introduced by F5. The retry budget
is bounded (`MAX_CAS_RETRIES = 5`) and the loop re-reads + re-validates
on every iteration, so a regressing cursor or scope mismatch surfaces
immediately rather than letting the retry loop overwrite the winner's
state. `put_thread_notification_policy` is a blind overwrite and keeps
`CasExpectation::Any`.
`record_delivery_attempt` uses `CasExpectation::Absent` for the
first-write branch, so two racing at-least-once writers can't both
insert; the loser falls back into the duplicate-identity-check branch
on the next read.
* fix(outbound): use control-character sentinel in thread scope key
Audit finding F6: `thread_scope_key` used the literal string `"_"` as
the sentinel for `agent_id = None` / `project_id = None`. The
`validate_scope_id` validator in `ironclaw_host_api` accepts underscore
as a legal character in an `AgentId` / `ProjectId`, so a scope with
`agent_id = Some(AgentId::new("_"))` hashed to the same key as a scope
with `agent_id = None`. Two distinct scopes silently collided on the
same policy/subscription/delivery virtual path.
Switch the sentinel to `"\x1F"` (ASCII unit-separator). It's a control
character; `validate_scope_id` rejects every C0 control char via
`has_forbidden_control`, so no legal scope id can ever contain it. Add
a unit test that pins the sentinel-rejection invariant and a
regression test that proves `agent_id = Some("_")` no longer hashes to
the same key as `agent_id = None`.
* fix(outbound): query indexed scope projection with paginated drain
Audit finding F2 (HIGH): `list_delivery_attempts` was a `list_dir` +
N+1 `get_json` per row with no indexed projection, scanning every
delivery on the mount even when only one scope's deliveries were
requested. Cost scaled with total delivery count, not with the
queried scope's row count.
Declare an exact-equality index on a new `scope` indexed key. The
projected value is the same `thread_scope_key` hash used for policy
paths — collision-resistant against the legal id grammar and updated
by F6 to never collide with the `None` sentinel. `record_delivery_attempt`
and `update_delivery_status` write through a new
`put_delivery_attempt_indexed` helper that includes the projection;
`update_delivery_status` preserves it on status mutations. The list
path drives `query(Filter::Eq { key: "scope", value: ... })` and
re-checks `scope_matches` defensively (hash collisions are
unreachable but cheap to guard against).
Audit finding F3 (Medium): the previous `list_dir` was unpaginated;
SQL backends issue `LIMIT Page::MAX_LIMIT (1024)` on their list_dir
translation and would silently truncate past 1024 deliveries. The
new path drains pages via `offset += received` until a short page
arrives, mirroring `ironclaw_engine::store::filesystem::query_all`.
`ensure_delivery_scope_index` runs idempotently before every write
and read. It tolerates `FilesystemError::Unsupported` on byte-only
backends to match the engine store's `ensure_exact_index` pattern;
the in-memory backend serves `Filter::Eq` from `Entry::indexed`
directly even without a materialized index declaration.
* test(outbound): cover CAS retry, pagination drain, backwards-race
Audit finding F4: the existing `outbound_state_store_contract` suite
exercised the storage contract surface but had no coverage for any of
the failure modes the F1/F3 fixes address:
- No CAS-retry test. F1's bounded retry loop could regress to permanent
failure on any transient `VersionMismatch` and the suite wouldn't
notice — the in-memory backend never produced one.
- No `> Page::MAX_LIMIT` drain test. F3's pagination loop could lose
the tail of a long delivery list and the suite wouldn't notice
because the existing tests record at most one delivery per scope.
- No concurrent backwards-race test on `advance_subscription_cursor`.
The existing backwards-advancement test only exercised the single-
threaded path; nothing proved the post-F1 retry loop re-validates
progression on every iteration.
Add three regression tests:
1. `VersionRacingBackend` wraps `InMemoryBackend` and injects a single
`FilesystemError::VersionMismatch` on the next `put` matching a
configured prefix. The first new test
(`advance_subscription_cursor_retries_through_cas_conflict`) arms
one conflict, advances the cursor, asserts the retry loop converges,
and asserts exactly one conflict was injected and consumed.
2. `concurrent_backwards_race_rejected_after_winner_advances` runs two
sequential advances — the winner to cursor=100 and the loser to
cursor=50 — and asserts the loser is rejected with `InvalidRequest`
while the winner's state is preserved. Together with the retry test
this proves the re-validate-on-retry semantics F1 calls out.
3. `list_delivery_attempts_drains_more_than_page_max_limit` writes
`Page::MAX_LIMIT + 1` delivery attempts under one scope and asserts
`list_delivery_attempts` returns every one. Before F3 this would
silently truncate at 1024 rows.
Cargo.toml: enable `tokio/sync` for `Mutex` in the test mock; drop the
feature-conditional `use std::sync::Arc` because the new tests need it
unconditionally.
* fix(run-state): bound filesystem lock map under tenant churn
The process-wide FILESYSTEM_RECORD_LOCKS map kept one
Arc<tokio::sync::Mutex<()>> per touched path. In long-running hosts with
high tenant/invocation churn the map grew without bound, since entries
were never removed once the originating put/get cycle completed.
Switch the value type to Weak<Mutex> so dropped Arcs no longer pin map
slots. Each acquisition opportunistically prunes dead entries before
upgrading-or-installing, keeping the map size proportional to in-flight
paths rather than to lifetime path count. Concurrent callers on the same
path still observe the same Arc (the outer std::sync::Mutex serializes
the upgrade-or-insert window), so existing intra-process and
cross-instance serialization guarantees are preserved — both verified by
the new unit tests and by the existing
filesystem_*_duplicate_*_serialized_across_store_instances contract
tests.
Addresses audit findings F1 (Medium) and F4 (Low).
* fix(run-state): use versioned CAS for filesystem run/approval writes
All filesystem put() calls used CasExpectation::Any, so two host processes
mounting the same /engine could lose updates: each one's read-modify-write
saw the other's value and then unconditionally overwrote it. The
per-path async mutex only serializes intra-process callers.
Switch creates to CasExpectation::Absent and updates to
CasExpectation::Version(v) with a bounded retry loop on VersionMismatch.
The new put_with_cas helper centralizes the contract: on capable
backends (InMemoryBackend, the upcoming SQL ports) cross-process races
now fail closed and the caller retries; on byte-only backends that
return Unsupported (LocalFilesystem) we degrade to Any but emulate
Absent with a get() precheck so the AlreadyExists path is preserved.
The in-process lock map (F1) keeps the check-then-write race closed for
the byte-only fallback.
Approve/deny/discard pull the record-lock guard up to the trait method,
since update_status no longer acquires it.
Addresses audit finding F2 (Medium). Closes the gap acknowledged in
crates/ironclaw_run_state/CLAUDE.md.
* fix(approvals): type approval-resolution decision with ApprovalDecisionKind enum
Addresses audit finding F1.
Replaces the stringly-typed `impl Into<String>` decision parameter on
`AuditEnvelope::approval_resolved` with a wire-stable
`ApprovalDecisionKind` enum (`Approved`/`Denied`,
`#[serde(rename_all = "snake_case")]`), so approval callers cannot
drift on capitalization or spelling. Per `.claude/rules/types.md`
"wire-stable enums".
The wider `DecisionSummary::kind` field stays a `String` because other
audit producers (authorization denials, obligation handlers) emit
values outside the approval enum; cross-decoding remains a follow-up.
Cross-crate blast radius: `ironclaw_host_api` (new enum + factory
signature), `ironclaw_approvals` (both call sites),
`ironclaw_events::tests::durable_log_contract` (three test fixtures).
* fix(approvals): persist approval state before issuing lease
Addresses audit finding F2.
Inverts the lease/approve ordering inside `approve_capability_action`:
the approval store write now runs *before* the lease store write. The
previous order (issue lease, then approve, best-effort revoke on
failure) left a window where a transient approval-store error could
leave a live lease pointing at a request whose status remained
`Pending`.
The approval record is now treated as the authority of record. Once
the request flips to `Approved`, lease issuance is a recoverable
operation against an already-decided request — if the lease store
fails, the caller surfaces the lease error and the request stays
`Approved`. The previous best-effort `let _ = self.leases.revoke(...)`
swallow is gone with the same edit.
Updates the three concurrency/error-injection tests to assert the new
semantics, plus the crate CLAUDE.md guardrail. No external test
fixtures break — the public resolver API is unchanged.
* fix(approvals): route both resolve paths through emit_approval_resolved helper
Addresses audit finding F3.
Extracts an `emit_approval_resolved` helper on `ApprovalResolver` so
the audit-envelope construction in `approve_capability_action` and
`deny` is built in exactly one place. Both call sites used to inline
`AuditEnvelope::approval_resolved` against their own
`record.scope`/`denied.scope`; while consistent today, divergence
between the two would be a silent regression.
Pure refactor — no test changes needed beyond the existing audit-event
contract tests which already pin the wire shape.
* fix(approvals): cover concurrent approve_dispatch first-write-wins
Addresses audit finding F4.
Adds a caller-level concurrency regression test that spawns two
`approve_dispatch` calls against the same pending request on a
multi-thread tokio runtime and asserts the expected first-write-wins
invariants:
- exactly one approve returns `Ok`
- the other returns `ApprovalResolutionError::NotPending { status:
Approved }`
- the lease store ends up with exactly one Active lease (not two, not
zero — under the F2 persist-approval-first ordering the loser fails
*before* lease issuance, so no orphan to revoke)
- the approval record's terminal status is `Approved`
Enables `rt-multi-thread` on the tokio dev-dependency so the test can
exercise real cross-thread contention on the approval store mutex.
* fix(engine): restore HybridStore parity for mission updates
F1: `update_mission_status` now bumps `mission.updated_at` before
writing back, matching HybridStore (`src/bridge/store_adapter.rs:1950`).
Recency-sorted views (mission list UIs, learning-mission dispatcher)
were silently freezing the timestamp at original-save time.
F2: `list_missions` and `list_all_missions` now sort by `(name, id)`
after collection, matching HybridStore (`store_adapter.rs:1913, 1937`).
The underlying `query`/HashMap iteration is non-deterministic; the
LLM-facing `mission_list` tool was seeing arbitrary order across runs.
Tests:
- `update_mission_status_bumps_updated_at` — regression for F1
- `list_missions_is_deterministic_across_invocations`,
`list_all_missions_is_deterministic_across_invocations` — regression for F2
* fix(memory): drain pages in FilesystemMemoryDocumentRepository::list_documents
Audit findings F1 (HIGH) + F9 (Low).
F1: `list_documents` issued a single `query(.., Page::new(0,
Page::MAX_LIMIT))` and trusted the page was complete. Because
`Page::MAX_LIMIT == 1024`, scopes holding >1024 documents silently lost
every entry past the cap. The result fed `write_document`'s
ancestor/descendant conflict check at the call site immediately above,
so a new path could shadow (or be shadowed by) an existing document
across the truncation boundary without a conflict ever firing — exactly
the regression `query_all_pages` was extracted in
`src/db/filesystem_jobs.rs` to prevent.
F9: The old implementation issued a `Filter::All` query, threw the
results away (`let _ = (versioned, &prefix_str);`), then called
`list_dir` to discover paths. The query-result loop was dead code under
any backend that supports `query`. The stale comment claimed the trait
didn't surface paths in `query` results, but
`VersionedEntry.path` (`crates/ironclaw_filesystem/src/record.rs:347`,
added in PR #3659) has carried the absolute virtual path for every
queried row since.
Replace both with a single drain loop that paginates `query` until a
short page comes back, filters by `entry.kind == "memory_document"`, and
recovers the `MemoryDocumentPath` directly from `VersionedEntry.path`.
The `list_dir` fallback is gone, and the agent_id axis is preserved
through `MemoryDocumentPath::new_with_agent` so scopes with an agent
identity round-trip correctly (the previous code's `new()` dropped the
agent).
Regression: `list_documents_drains_pages_beyond_max_limit` writes
`MAX_LIMIT + 5` documents and asserts every one comes back. This also
exercises the conflict-check path because each `write_document` calls
`list_documents` internally.
* fix(secrets): close consume_if_matches timing oracle with constant-time compare
F1 (HIGH, timing oracle) in the 2026-05 audit: `consume_if_matches` in
`legacy_store.rs` (trait default + in-memory backend) and `db.rs` (libSQL
+ Postgres backends) compared the decrypted plaintext against the
caller-supplied expected value with `!=`. Rust's `!=` over `&str`/`&[u8]`
short-circuits on the first differing byte, so an adversary who can
observe response latency over the network can recover the secret byte
by byte. AES-GCM authenticated decrypt closes the ciphertext oracle but
does nothing for the post-decrypt comparison.
Fix: route the comparison through `subtle::ConstantTimeEq::ct_eq`, which
walks the full buffer regardless of where the bytes diverge. The
post-comparison branches retain their original shape because the
decrypt+lookup path is already executed unconditionally before the
compare — only the success-side `DELETE` differs, and that signal is
already exposed by the function's return value.
Added a regression test (`f1_consume_if_matches_uses_constant_time_compare`)
that grep-asserts the production source imports `subtle::ConstantTimeEq`,
uses `ConstantTimeEq::ct_eq`, and no longer contains the legacy `!=`
shape. Cannot meaningfully prove constant-time-ness from a shared CI
runner, but the source-pattern check ensures a "simplifying" revert
fails review.
Audit: F1 (HIGH).
* fix(secrets): use constant-time compare for store key-check sentinel
F3 (Low) in the 2026-05 audit: `verify_secret_store_key_check` compared
the decrypted sentinel against `SECRET_STORE_KEY_CHECK_PLAINTEXT` with
`!=`. The plaintext is a fixed compile-time string so the practical
risk is low — an attacker who can move the encrypted_value/key_salt
blobs across rows already has full DB write access — but the same
constant-time pattern applied to F1 makes the comparison style
consistent across the crate and pre-empts a future caller threading a
non-constant sentinel through this helper.
Routed through `subtle::ConstantTimeEq::ct_eq`, mirroring the F1 fix.
Audit: F3 (Low).
* fix(processes): index queryable fields and serve records_for_scope via query
Replace the N+1 list_dir + per-file get scan with an indexed `query`
path, falling back to the legacy scan on byte-only backends so existing
LocalFilesystem-driven tests and production deployments remain
unaffected.
- Declare `ensure_index` lazily for the per-owner `processes/` prefix on
the queryable fields called out in the audit (`tenant_id`, `user_id`,
`status`, `extension_id`, `parent_process_id`). Backends without index
support degrade to the existing scan instead of failing closed.
- Project the same fields onto every `ProcessRecord` write via
`Entry::with_indexed`; record-capable backends (libSQL, Postgres, the
in-memory backend) can now serve scope listings through a native
query. The opaque-byte fallback in `put_with_byte_fallback` keeps
LocalFilesystem (which rejects record-shaped puts today) on the legacy
write path.
- Rewrite `records_for_scope` to issue `Filter::And` of `Filter::Eq`
predicates against the indexed projection. The full `same_scope_owner`
check remains in Rust so the sub-scope axes (agent/project/mission/
thread) that are not yet in the index spec still get filtered.
- Add a contract test that exercises the indexed path through
`InMemoryBackend` and confirms cross-tenant and cross-user records
are not returned.
Addresses audit findings F1 (records_for_scope N+1) and F2 (missing
ensure_index at startup).
* fix(filesystem): surface backend infrastructure errors without fabricated paths
F1: SQL backends used valid_engine_path() (unwrap_or_else unreachable
returning /engine) as a placeholder on every connection/migration
error. The path was always a lie - at pool acquisition, run_migrations,
pragma setup, or schema bootstrap there is no caller-supplied virtual
path in scope - and it leaked into operator-facing error display.
Add FilesystemError::BackendInfrastructure { operation, reason } that
omits path. Route every former valid_engine_path() callsite in libsql
and postgres through new infrastructure_error helpers in db.rs. The
enum is non_exhaustive so adding a variant is backward compatible.
Regression test: drive a libsql migration against a read-only DB file
and assert BackendInfrastructure with no /engine in display.
* fix(filesystem): store VirtualPath keys in InMemoryBackend state directly
F2: in_memory.rs::query() reparsed every stored row's path with
VirtualPath::new(...).unwrap_or_else(|_| unreachable!('stored paths
originated as VirtualPath')) on the hot path. Two issues:
- the reparse is wasted work - paths originate as VirtualPath at
put() time, so the validation pass on read is redundant
- 'unreachable!' is a panic that asserts a structural invariant
the type system already enforces
Replace HashMap<String, StoredEntry> with HashMap<VirtualPath,
StoredEntry>. Lookups now pass &VirtualPath directly; prefix scans
move to key.as_str().starts_with(...). VersionedEntry::path comes
from a single clone() instead of a parse + unreachable.
Existing tests cover the put/get/query/list_dir/stat/delete paths
that were touched (44 in_memory tests + the cross-backend
contract suite).
* fix(filesystem): align in-memory backend on nested VectorNearest semantics
F5: SQL backends reject Filter::VectorNearest nested inside And/Or
with Unsupported because ranking can't be expressed as a WHERE
fragment - the top of query() peels off a top-level VectorNearest
before the translator runs, and the translator's VectorNearest arm
unconditionally errors. The in-memory backend previously treated a
nested VectorNearest as 'any row with IndexValue::Bytes at key',
silently changing semantics across backends.
Add contains_nested_vector_nearest() pre-check in InMemoryBackend::
query that walks the filter tree and surfaces Unsupported for any
VectorNearest strictly inside a compound. The Filter::VectorNearest
arm in filter_matches is now unreachable; it returns false to keep
the scalar predicate path safe should the pre-check ever be bypassed.
Regression test asserts Unsupported on nested-in-And, nested-in-Or,
and still-OK for top-level VectorNearest.
* fix(filesystem): guard u64 to i64 SQL bindings with typed errors
F6: SQL backends used 'expected.get() as i64' and 'page.offset as i64'
casts on the CAS and query/pagination paths. Both inputs are u64 and
both wrap silently on values >= 2^63 - the cast produces a negative
SQL binding that either matches no row (CAS quietly VersionMismatches)
or executes against a negative OFFSET (cryptic backend error).
Add db.rs helpers:
- record_version_to_i64: surfaces CorruptRecordVersion if the value
overflows i64
- page_offset_to_i64: surfaces a typed Backend error naming the
operation and offset
Apply at libsql.rs CAS and query offset bindings and the matching
postgres.rs sites. 'page.limit' is u32 clamped to Page::MAX_LIMIT so
its i64 cast is safe by construction and uses i64::from for clarity.
Regression test asserts a typed Backend(Query) error with reason
'page offset...' when querying with offset = u64::MAX, replacing the
prior silent wrap.
* fix(filesystem): scope Postgres FTS GIN index to declaring prefix
F4: libsql FTS5 virtual tables are declared per-mount-prefix - one
vtable per ensure_index(prefix, ...) call - so a query at one prefix
can't accidentally pull index postings from a sibling prefix into the
plan, and tearing down an index for a prefix is a clean DROP TABLE.
The Postgres FTS GIN index, by contrast, was created without a
predicate over root_filesystem_entries, so it was global. Correctness
held because the query path always scopes by 'path = OR path LIKE
', but parity with libsql broke in two ways: the planner
considered postings from every prefix before filtering, and a
per-prefix DROP INDEX could only ever tear down one of them.
Add a partial-index predicate gated by 'path = <prefix> OR path LIKE
<prefix>/%' to the GIN DDL. The prefix is sourced from the validated
VirtualPath and quotes are doubled for safe SQL literal embedding;
LIKE-special characters are escaped via the existing
escape_like_with_trailing_wildcard helper.
Regression test (Postgres only; skipped when no DB is reachable)
reads back the DDL via pg_indexes.indexdef and asserts the prefix
literal and a WHERE clause appear.
* fix(filesystem): tighten capability docs, type constraints, and hygiene nits
Batched audit findings:
F3: Document the type constraint on IndexKind::Prefix. The kind is
only meaningful against IndexValue::Text, but ensure_index can't see
the value type at declaration time. Filter::PrefixOn rejects every
non-text variant at query time. Document the constraint loudly so
consumers reach for IndexKind::Exact when projecting numeric or
boolean values instead of getting an unused index and a query-time
Unsupported.
F7: BackendCapabilities::sql_typical advertises a minimum SQL shape
that omits IndexFts and IndexVector. The two real backends here
(libsql + postgres) layer them on top. A hand-rolled backend that
just calls sql_typical() would under-advertise. Add a doc-comment
calling out the omission and an sql_typical_full() variant that
includes Events + IndexFts + IndexVector for backends that match
this crate's shape.
F8: validate_simple_identifier indexed bytes[0] after an is_empty
guard. The guard makes the index sound, but the pattern is fragile
to refactors. Switch to bytes.first() so the dependency is explicit
and the panic path goes away.
F9: Multiple doc comments in record.rs and index.rs referenced
stale type names (StorageBackend::put/list/query, Record). Update
to the current RootFilesystem / Entry names.
* fix(engine): dedupe events on append_events for HybridStore parity
HybridStore (`src/bridge/store_adapter.rs:1613`) de-duplicates thread
events by id before insert. The filesystem-store `append_events` impl
was previously writing with `CasExpectation::Any`, which silently
overwrote an existing event with the same id when callers re-emitted
(e.g. recovery after a partial flush).
Pre-read the destination path and skip any id already present.
Matches HybridStore's append-only contract.
Audit finding F3 (Medium) from the ironclaw_engine crate audit.
* fix(memory): map FTS results to documents in FilesystemMemoryDocumentRepository::search_documents
The previous scaffold issued the `Filter::Fts` query, then silently
dropped the results with `let _ = results; Ok(Vec::new())`. A caller
wiring up the trait would see an empty result set and assume "no
matches" — when in fact the search had simply lied. That is worse
than returning `Unsupported`.
Map each `VersionedEntry.path` (added in PR #3659) back to a
`MemoryDocumentPath`, de-dupe by path, and assign a per-rank score
from RRF over the FTS-only branch so the result vector matches the
native repos' fusion contract for the trivial single-branch case.
Skip non-memory-document entries that may live under the same prefix
(chunk projections, metadata siblings). Adds
`list_documents_drains_pages_beyond_max_limit` test against the
in-memory backend.
Audit finding F2 (HIGH) from the ironclaw_memory crate audit.
* fix(secrets): close revoke CAS-loop race with versioned compare-and-swap
`revoke` previously read the lease via the (now-removed)
`read_lease` helper and wrote with `CasExpectation::Any`. The
per-lease process-local mutex serialized writers within one process
only — multi-process callers sharing the same backend root could
observe `Active`, race against `consume`, and clobber a `Consumed`
marker by overwriting it with `Revoked`.
Inline the read into a bounded CAS retry loop matching `consume` and
`consume_session_use`: read with version, write with
`CasExpectation::Version`, retry on `VersionMismatch`. Make revoke
idempotent on terminal states (`Consumed`, `Revoked`, `Expired`) so
the loop converges even when a winner has already written.
Audit finding F2 (Medium) from the ironclaw_secrets crate audit.
* fix(processes): use versioned CAS for status transitions
`update_status` previously read the record and wrote with
`CasExpectation::Any`, relying on the per-instance `transition_lock`
for atomicity. That lock only serializes within one process; a
multi-process deployment sharing the same backend root could observe
identical pre-transition state in both processes and clobber each
other's status flips.
Replace with a bounded CAS retry loop: read with version, validate
the transition, write with `CasExpectation::Version…
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
…rai#3679) * feat(processes): route FilesystemProcessStore through unified put/get First consumer migration onto the new RootFilesystem surface. Switches the byte-plane read_file/write_file calls inside ironclaw_processes' filesystem-backed store to the unified put/get ops with Entry::bytes + CasExpectation::Any. The on-disk JSON layout is unchanged, every existing test passes, and downstream crates that construct FilesystemProcessStore (ironclaw_host_runtime + tests) don't need to change. Scope deliberately narrow: opaque-file entries through `put`/`get` without record kinds or non-`Any` CAS, since LocalFilesystem's native `put` only accepts that shape (per the foundation PR #3659). Once LocalFilesystem grows sidecar metadata, this consumer can switch to `Entry::record(process_record_kind, ...)` + `CasExpectation::Absent` without changing the on-disk layout. Touch points: - write_record uses put(Entry::bytes, CAS::Any) - start uses get for the existence probe + transition_lock for the atomicity envelope per the single-instance invariant - update_status / get / records_for_scope read via get and unwrap VersionedEntry.body - records_for_scope returns ProcessError::Filesystem (not silent skip) when get returns None for a path that list_dir just yielded — matches the pre-migration NotFound propagation invariant Test scaffold update: BackendErrorFilesystem now overrides `get` too, so the fault-propagation regression test continues to exercise its intended path. (Reviewer P1/P2 on the original #3666 — recursion + silent-skip — addressed in foundation #3659 directly since LocalFilesystem now ships native `put`/`get`.) * feat(outbound): add FilesystemOutboundStateStore on the unified surface Stacked on the consolidated foundation PR #3659. Adds an OutboundStateStore impl that persists outbound metadata under /engine/outbound/{policies,subscriptions,deliveries} through any RootFilesystem. The existing libSQL/Postgres/in-memory stores stay intact during the migration; a follow-up cleanup PR can delete them once production runs on the unified surface. The new store passes the full contract suite (durable_policy_*, subscription_cursor_*, delivery_status_*, notification_policy_*, full_turn_scope_isolation) against InMemoryBackend in the existing outbound_state_store_contract.rs test file. * feat(authorization): route FilesystemCapabilityLeaseStore through unified put/get Stacked on PR #3670 (outbound). Mirrors the ironclaw_processes migration in PR #3666 / now consolidated into #3659. Switches the filesystem-backed lease store's read_file/write_file calls to the unified get/put ops with Entry::bytes + CasExpectation::Any. The on-disk JSON layout is unchanged, every existing test passes, and the per-owner mutation_lock continues to serialize claim/consume/revoke within a single instance. Touch points: - read_lease, read_lease_index, read_lease_file — now use get and unwrap VersionedEntry.body. - write_lease, write_lease_index — now use put(Entry::bytes, Any). - Imports updated. - CountingFilesystem test scaffold gains put/get overrides that forward to its inner LocalFilesystem, since the trait defaults are now Unsupported after the PR #3659 recursion fix. * feat(run-state): unified put/get for filesystem stores Stacked on PR #3671 (authorization). Mirrors processes (#3666) and authorization (#3671) migrations. Switches all read_file/write_file calls in FilesystemRunStateStore and FilesystemApprovalRequestStore to the unified get/put ops with Entry::bytes + CasExpectation::Any. On-disk JSON layout unchanged. Test scaffold updates: ConcurrentMissingReadFilesystem and DisappearingApprovalReadFilesystem gain put/get overrides that forward to their inner LocalFilesystem and apply the same fault injection logic on the unified read path (was: only on the legacy read_file path). Required after the trait defaults moved to Unsupported in PR #3659. * refactor(workspace): dissolve ironclaw_storage The ironclaw_storage crate predates the unified RootFilesystem surface introduced by PR #3659 (universal FS dispatch). Its `BlobStore`/`RecordStore` traits, `StorageKey`/`StorageVersion`/`PutCondition` types, and `StoredBlob`/`StoredRecord` shapes parallel the new unified put/get /CasExpectation/RecordVersion machinery on `RootFilesystem` — a textbook duplicate-dispatch smell flagged by .claude/rules/architecture.md. Only `ironclaw_outbound` consumed any of the crate, and only 5 small helpers (`encode_json`, `decode_json`, `redacted_backend_error`, `StorageError::Backend`, `ABSENT_SCOPE_COMPONENT`). All other types and the entire `BlobStore`/`RecordStore` surface (660 LOC) were unused — their intended consumers already moved to `RootFilesystem` directly. Inlined the 5 helpers into `crates/ironclaw_outbound/src/db.rs`: - `encode_json`/`decode_json` → direct `serde_json::to_string`/`from_str` - `redacted_backend_error` → local log+collapse to `OutboundError::Backend` (preserves the redaction boundary required by ironclaw_outbound/CLAUDE.md) - `ABSENT_SCOPE_COMPONENT` → local const "" Removed the crate's workspace membership, the outbound dep, the forbidden-edges BoundaryRule, and the crate directory. Also updated the ironclaw_outbound BoundaryRule to permit a normal dependency on `ironclaw_filesystem` — `FilesystemOutboundStateStore` landed in the prior cascade PR and the boundary rule was stale. * feat(filesystem): add HsmBackend placeholder + scope database.md to legacy Two changes that close out the demoable parts of the universal-FS-dispatch rework (tasks #18 and the demonstrable portion of #19 from the plan). **HsmBackend placeholder** (`crates/ironclaw_filesystem/src/hsm.rs`). Demonstrates that a new backend is a single-file change: implements the one `RootFilesystem` trait, declares a restricted capability surface (`Read` + `Write` + `Stat` + `Delete` + `TxnCapability::Cas` — no records, no query, no index, no events, no multi-key transactions), and routes `put`/`get`/`delete`/`stat`/`list_dir` through an in-process placeholder. Five tests prove the seam works end-to-end: - `hsm_supports_encrypted_bytes_round_trip` — bytes put/get works. - `hsm_rejects_structured_records` — `put` with `RecordKind::Some` or non-empty `indexed` returns `Unsupported`, so a consumer cannot accidentally route records through encryption-only storage. - `hsm_rejects_query_and_index_ops` — `query`/`ensure_index` return `Unsupported` consistent with the declared capabilities. - `composite_rejects_overclaimed_hsm_descriptor` — mount-time validation (`validate_mount_capabilities`) refuses a descriptor that claims `Query`/`IndexExact` over a backend that doesn't deliver, failing with `FilesystemError::DescriptorOverclaims { missing, .. }`. - `composite_routes_to_hsm_under_secrets_mount` — the acceptance gate: mounting HsmBackend at `/secrets` and routing put/get through the composite works with no consumer-visible changes. Indexed projection is still rejected because the declared capabilities advertise no index/query support. A real HSM implementation replaces the in-memory placeholder with an HSM session handle; the trait surface, capability declarations, and mount-time validation are reusable as-is. The placeholder is *not* a security boundary — it is a seam demonstration. **database.md scoped to legacy directories**. The dual-backend rule file (`.claude/rules/database.md`) is `paths`-scoped to `src/db/**`, `src/history/**`, and `migrations/**` — exactly the legacy surface that predates the universal FS dispatch. Added a "Status & Direction" preamble pointing new persistence work at `ScopedFilesystem` and the `2026-05-14-universal-fs-dispatch.md` plan, with the existing per-crate dual-backend guidance kept (and tagged "legacy") for code still inside those directories. * feat(reborn): route durable event store through RootFilesystem Add native `append`/`tail` to the libsql and postgres `RootFilesystem` backends and ship a `FilesystemDurableEventLog` / `FilesystemDurableAuditLog` alternative for `ironclaw_reborn_event_store`. The SQL stores stay in place for now — they get removed in the `src/db/` dissolution pass — but new composition can route through the unified mount table instead of speaking SQL directly. - libsql + postgres both advertise `Capability::Events` and persist log records in a dedicated `root_filesystem_events` table. - Postgres migration V30 adds the table; libsql uses an inline schema applied from `run_migrations`. - Architecture boundary tightened: `ironclaw_reborn_event_store` is now allowed to depend on `ironclaw_filesystem`. * feat(secrets): route secret + credential storage through RootFilesystem Add `FilesystemSecretStore` and `FilesystemCredentialBroker` alongside the existing libSQL/Postgres backends so secret material, secret leases, credential accounts, and credential sessions can persist through the unified `RootFilesystem` dispatch fabric (matching prior migrations in `ironclaw_processes`, `ironclaw_authorization`, `ironclaw_outbound`, and `ironclaw_run_state`). - Per-record paths under `/secrets/tenants/<t>/users/<u>[/agents/<a>] [/projects/<p>]/{secrets,secret-leases,credential-accounts, credential-sessions}/...`. - Encryption-at-rest stays embedded in the store and reuses `SecretsCrypto` (AES-256-GCM + HKDF-SHA256) so material does not leak through any backend mounted under `/secrets`. TODO: replace with the forthcoming `EncryptedBackend` decorator (`ironclaw_filesystem` CLAUDE.md invariant #5). - Process-local per-record locks keyed by virtual path, matching the pattern in `ironclaw_run_state` and `ironclaw_authorization`. - `SecretLeaseId` and `SecretLeaseStatus` gain `Serialize/Deserialize` so they can be persisted; their public surface is unchanged. - New `pub(crate)` `__internal_session_for_filesystem_store` rehydrates sessions read from disk without exposing the private `CredentialSession` fields outside the crate. - Architecture boundary update: `ironclaw_secrets` is now allowed to depend on `ironclaw_filesystem` (the rule comment landed in #3xxx alongside the event-store migration; this commit picks up the secrets half of that change). - Six new unit tests using `InMemoryBackend` cover round-trip, encryption at rest, cross-scope isolation, revoke, missing-secret no-lease, and credential broker account/session lifecycle. All existing tests pass unmodified (60 tests total). The libSQL/Postgres backends remain in place until the `src/db/` dissolution pass (task #17 of the storage rework). * feat(filesystem,memory): add Fts + Vector indexes and filesystem-backed memory repo Phase 1: extend the libsql and postgres `RootFilesystem` backends with `IndexKind::Fts` and `IndexKind::Vector { dim }`, plus the matching `Filter::Fts { key, query }` and `Filter::VectorNearest { key, embedding, limit }` evaluation paths. - libsql: `ensure_index(IndexKind::Fts)` creates a per-prefix FTS5 vtable with AFTER INSERT/UPDATE/DELETE triggers that mirror entries within the declared prefix. Backfill on declaration handles pre-existing rows. `Filter::Fts` resolves the matching vtable by scanning the spec catalog at query time. Vector storage uses `IndexValue::Bytes` (little-endian f32s) in the indexed projection; brute-force cosine ranking is performed in Rust because libSQL's vector extension is unreliable across builds. - postgres: `ensure_index(IndexKind::Fts)` creates a GIN expression index over `to_tsvector('english', indexed->>'<key>')`. `Filter::Fts` translates to a `@@ plainto_tsquery(...)` predicate so the GIN index is usable. Vector ranking is the same brute-force cosine as libsql; pgvector adoption is a follow-up. - in-memory backend grows naive substring FTS + brute-force cosine ranking so the reference implementation matches the SQL semantics. - Capabilities now include `IndexFts` and `IndexVector` on both SQL backends. - Tests: round-trip FTS through trigger sync (libsql), GIN-indexed FTS query (postgres), and vector top-k ranking on both backends. Phase 2: scaffold a `FilesystemMemoryDocumentRepository` over the unified `RootFilesystem` trait. Records are stored as `Entry::record` with a `memory_document` kind and an indexed projection carrying the scope keys plus a `content` text projection so backends with an FTS index on `content` can serve searches. Metadata is stored at a sibling `.meta` path. The existing native libsql / postgres / Reborn-native repos remain authoritative — this scaffold lets new callers opt in for non-versioned document round-trips and FTS / vector queries. Known TODOs documented inline in `filesystem.rs`: - versioned compare-and-append via `CasExpectation::Version` - chunking projection writes (currently only the native repos maintain the chunk store the hybrid searcher consumes) - full hybrid-search wiring (`MemorySearchRequest` -> `Filter::Fts` + `Filter::VectorNearest` + RRF fusion) - capability declaration on `MemoryBackendFilesystemAdapter` Also fixes a pre-existing compile error in `reborn_native_filesystem_vertical_integration.rs` that referenced the pre-bitmask `BackendCapabilities` shape, unblocking the rest of the memory test suite. Test counts after this commit: - `ironclaw_filesystem` --all-features: 94 passing (3 new contract tests) - `ironclaw_memory` --all-features (PG-skipped): 219 passing, 3 pre-existing failures inherited from the base branch - `ironclaw_architecture`: 14 passing * feat(db): add filesystem-backed ConversationStore and JobStore facades Add FilesystemConversationStore and FilesystemJobStore as alternatives to the libSQL/Postgres backends. Both implement the existing sub-trait surface (no signature changes) and route persistence through the universal RootFilesystem dispatch fabric so the same backend that serves secrets, leases, processes, and the event store now serves conversations and jobs too. Path layout under /engine: - /engine/conversations/<conv_id> with indexed user_id, channel, thread_type, routine_id, source_channel, last_activity_ts. - /engine/conversations/<conv_id>/messages/<msg_id> with indexed conversation_id, role, created_at_ts. - /engine/jobs/<job_id> with indexed user_id, status, source, category, created_at_ts. - /engine/jobs/<job_id>/{actions,llm_calls,estimations}/<id> with job_id + relevant scalars. Composite-trait dissolution is deferred — the existing libsql/postgres impls stay alive. 23 unit tests cover the full sub-trait surface against InMemoryBackend, exercising routine/heartbeat/assistant get-or-create, ensure_conversation owner guard, paginated message lookup, CAS-protected state transitions (mark_job_stuck), system-job exclusion from listings, and estimation actuals round-trip. * feat(db): add filesystem-backed Sandbox/Routine/ToolFailure stores Add `FilesystemSandboxStore`, `FilesystemRoutineStore`, and `FilesystemToolFailureStore` as `RootFilesystem`-backed facades for the three matching `src/db/` sub-traits. Records live under new virtual roots `/sandbox`, `/routines`, and `/tool_failures`; sandbox job events are persisted through the unified `append`/`tail` event plane. Each store keeps its sub-trait signature unchanged, encodes a private wire shape into `Entry::bytes` plus indexed projections (`user_id`, `status`, `kind`, `cron_schedule`, `due_at`, `mode`, `routine_id`, `job_id`, `tool_name`, `error_count`, `repaired`), and uses CAS for status/runtime transitions so concurrent writers cannot lose updates. Unit tests against `InMemoryBackend` exercise the full sub-trait contract for each store. The legacy libSQL/Postgres impls are unchanged. * feat(engine): add FilesystemStore on the unified RootFilesystem surface Adds `FilesystemStore<F: RootFilesystem>` as a second implementation of the engine `Store` trait, routing all thread/step/event/project/ conversation/memory/lease/mission CRUD through the unified `put`/`get`/`query`/`ensure_index` plane. Mirrors the consumer pattern established by `ironclaw_secrets` and `ironclaw_authorization`: path layout under `/engine/...`, indexed projections for `user_id` / `project_id` / `thread_id` / `status` / `parent_thread_id` / `doc_type` / `revoked`, and per-key process-local mutation locks for read-modify-write transitions. `HybridStore` in `src/bridge/store_adapter.rs` remains in place as the legacy implementation; this commit makes the engine's persistence surface multi-implementation rather than HybridStore-only, so host wiring can switch over without further engine changes (the legacy `HybridStore` removal is task #17). Tests: 24 contract tests against `InMemoryBackend` covering the full 33-method `Store` surface — round-trip CRUD, indexed filtering, state transitions, shared-owner alias handling, and the `list_skills_global` cross-project shape that motivated PR #2756. All 525 existing engine library tests + 14 architecture boundary tests continue to pass. * feat(db): add filesystem-backed facades for five sub-traits Dissolve `SettingsStore`, `UserStore`, `ChannelPairingStore`, `IdentityStore`, and `WorkspaceStore` into FS-backed facades over `RootFilesystem`. Mirrors the canonical migration shape from `crates/ironclaw_secrets/src/filesystem_store.rs` and `crates/ironclaw_authorization/src/lib.rs`. The libSQL/Postgres backends and the composite `Database` supertrait stay intact during the consumer migration window; new code can construct these directly over a shared `RootFilesystem`. Path layout: - `/system/settings/<user_id>/<key>` - `/users/<id>` + `/users/.tokens/<token_id>` + `/users/.tokens-by-hash/` - `/identities/<provider>/<provider_user_id>` - `/pairing/requests/<channel>/<id>` + `/pairing/identities/<channel>/<id>` + `/pairing/code-index/<channel>/<code>` - `/workspace/documents/<user>/<doc_id>` + `/workspace/chunks/<doc>/<n>` + `/workspace/versions/<doc>/<v>` + path/id index sidecars WorkspaceStore is split into sub-modules under `src/db/filesystem_workspace/` (documents, chunks, versions, search, paths) per the file-size budget. Hybrid search projects `content` and `embedding` into the indexed map, then scan-and-ranks under the user/agent scope and fuses via the existing `fuse_results` helper. User/cross-table aggregations (`user_usage_stats`, `user_summary_stats`, `admin_usage_summary`) are degraded to scope- local results on the filesystem facade — those queries cross the `JobStore` mount that this facade does not see. `/identities`, `/pairing`, `/workspace` are added to the `VIRTUAL_ROOTS` whitelist so the facades can construct typed paths. Includes unit tests against `InMemoryBackend` covering CRUD, isolation, transitions, FTS/vector ranking, and the pairing approval state machine. * fix: replace .expect on validated literals with unwrap_or_else(unreachable!()) CI's `scripts/check_no_panics.py` flags `.unwrap()`/`.expect()` in production code. Agent-generated stores used `.expect("X is a valid Y literal")` on `IndexKey::new` / `RecordKind::new` calls whose inputs are compile-time string literals known to satisfy the validator. Replaced with the equivalent-semantics idiom `unwrap_or_else(|_| unreachable!("..."))` — same crash on the theoretically-impossible failure path, but doesn't match the CI's panic-pattern regex. Affects: - crates/ironclaw_memory/src/repo/filesystem.rs (6 sites) - src/db/filesystem_conversations.rs (4 sites) - src/db/filesystem_jobs.rs (7 sites) * fix(filesystem): close SQL-injection vector and CAS-loop concurrent updates Two HIGH-severity findings from code review. Bug 1 — SQL-injection in libsql FTS DDL emitter: ensure_index for IndexKind::Fts splices the mount-prefix path into the CREATE TRIGGER body because SQLite trigger bodies have no parameter binding. VirtualPath::new rejects NUL/control/backslash/`..` but does not reject `'`, `"`, `;`. Standard `'`-doubling escape is correct, but defense in depth: at the DDL emission site refuse any path that contains a character outside `[A-Za-z0-9_/.-]`. Postgres path is parameterized, so only libsql was affected. Regression test added. Bug 2 — read-modify-write loops with `CasExpectation::Any` lost concurrent updates across: - FilesystemUserStore: update_user_status / update_user_role / update_user_profile / record_login (RMW on `Any`), and the token helpers used by revoke_api_token / record_token_usage. - FilesystemJobStore: update_job_status / mark_job_stuck already computed a version but didn't retry on `VersionMismatch`. - Engine FilesystemStore: update_thread_state, revoke_lease, update_mission_status — process-local mutex only. Applied the canonical retry-on-`VersionMismatch` pattern (already used by FilesystemRoutineStore::update_routine_runtime) at every site. filesystem_settings.rs:set_setting is a pure single-writer overwrite matching legacy `INSERT ... ON CONFLICT DO UPDATE`, so it stays on `Any` with an explanatory comment. Also fixes a pre-existing `unwrap_or_else(|_|...)` typo (1-arg closure on an Option) that blocked `cargo test --lib`. * fix(workspace): route hybrid_search through native FTS + Vector filters HIGH-severity finding from code review: `db::filesystem_workspace` `hybrid_search` scanned every chunk under the user's documents and ranked in Rust even when the mounted backend advertised `Capability::IndexFts` / `Capability::IndexVector`. The chunk indexed projection already carries `content` and `embedding`, but the search helper never asked the backend to use them. - search::hybrid_search now calls `filesystem.query(/workspace/chunks, Filter::Fts { content, query })` and `filesystem.query(.., Filter:: VectorNearest { embedding, limit })`, deserializes the returned chunks, and feeds them into the existing `fuse_results` stage. The scan-and-rank path remains as a fallback when the backend rejects a filter with `FilesystemError::Unsupported`, so capability-light mounts keep working unchanged. - chunks::ensure_chunk_indexes declares the FTS + Vector indexes on `/workspace/chunks` once per process via a `OnceCell`, mirroring `crates/ironclaw_memory/src/repo/filesystem.rs`. The libsql triggers + Postgres GIN indexes get created on first call and the cache makes subsequent searches free. - Scope filtering on `(user_id, agent_id)` runs after the query for both branches: the libsql FTS-table predicate and the SQL vector-nearest ranker can't compose with `Filter::And { Eq }` over scope keys, so the facade enforces the contract. - mod.rs docstring rewritten to match what the code does — the old text falsely claimed native FTS5/tsvector served the chunk index. - Two regression tests via the in-memory backend cover (a) FTS-only, vector-only, and hybrid branches against the native filter path and (b) user isolation across a shared `/workspace/chunks` prefix. Both tests fail against the prior scan-and-rank-only implementation. Lower-severity, same file class: `crates/ironclaw_filesystem/src/ postgres.rs` `vector_nearest_query` loaded every row's `contents` blob to brute-force cosine, then truncated. Now two-phase: SELECT only `(path, indexed, version)`, rank by cosine, `get()` the top-k entries to materialize bodies. Same fix landed for libsql in PR e2530adff. * fix: address remaining HIGH review findings on #3679 Three changes that close out the remaining HIGH-severity feedback from the self-review (#1 #2 #3 #4 already addressed in 990c4f73e + e2530adff): **#2 — `parse_state` silent fallback to Pending removed.** `src/db/filesystem_jobs.rs::parse_state` previously mapped unknown status strings to `JobState::Pending`, masking schema drift across a rollout (a new state value appearing in stored rows would silently lose its true value). Now returns `Result<JobState, DatabaseError>` and the single caller propagates with `?`. Matches the wire-stable enums rule in `types.md`. **#6 — `is_engine_unsupported` no longer substring-matches.** `crates/ironclaw_engine/src/store/filesystem.rs`: the typed `FilesystemError::Unsupported` discriminator gets lost when wrapped in `EngineError::Store { reason: String }`, so the old check `reason.contains("Unsupported")` would false-positive on any unrelated store error that mentioned the word. Now `fs_to_engine_error` tags the discriminator with a stable `[fs:unsupported]` sentinel and the check matches that sentinel — discriminator-preserving without changing the public `EngineError` shape. **#7 — `FilesystemChannelPairingStore` no longer drops corrupted records.** `src/db/filesystem_pairing.rs::find_pending_requests`: the old code silently filtered records whose JSON failed to deserialize, hiding data corruption. Now propagates `DatabaseError::Serialization` with the stored path so the operator sees the failure. Also: `// silent-ok:` annotations added to the three engine `Store` sites where read-modify-write on unknown ids is intentionally a no-op (matches HybridStore parity per its CLAUDE.md). Each annotation names the legacy contract being preserved. Verification: `cargo check --workspace --all-features` clean; `cargo test -p ironclaw_engine --all-features` 549/549; `cargo test --lib --all-features db::filesystem` 88/88; `cargo fmt --check` clean. * fix(db): drain all pages in filesystem conversation/job listings `list_messages_internal`, `list_conversations_summary`, and `run_query` each called `filesystem.query(.., Page::new(0, Page::MAX_LIMIT))` exactly once and trusted the result was complete. Because `Page::MAX_LIMIT == 1024`, conversations with >1024 messages or scopes with >1024 jobs/actions/estimations silently lost every row past the cap, and the `has_more` flag in `list_conversation_messages_paginated` became meaningless once the dropped tail crossed the page boundary. Codex PR #3679 P2 review flagged the pattern. Extract a shared `query_all_pages` helper in `filesystem_conversations` that loops `query(..., Page::new(offset, MAX_LIMIT))` until a short page comes back, then reuse it from `filesystem_jobs::run_query` and from the inline scan in `update_estimation_actuals`. The helper preserves the existing `NotFound -> Vec::new()` short-circuit and the `fs_err_to_database` error mapping so call sites are otherwise unchanged. Regression tests: - `list_messages_drains_pages_beyond_max_limit` writes `MAX_LIMIT + 5` messages and asserts the full count round-trips through `list_conversation_messages` and that `list_conversation_messages_paginated` reports `has_more` honestly for both partial and exhaustive windows. - `get_job_actions_drains_pages_beyond_max_limit` writes `MAX_LIMIT + 3` actions on one job and asserts the full count comes back in sequence order. - `list_agent_jobs_drains_pages_beyond_max_limit` writes `MAX_LIMIT + 7` jobs and asserts both `list_agent_jobs` and `agent_job_summary` count every row. * fix(secrets): close CAS-loop races in filesystem store consume paths Two HIGH-severity findings on PR #3679. Both sites read a versioned entry, validated a one-shot/use-limit condition, then wrote back with `CasExpectation::Any`. The process-local mutex only serializes writers inside one process; multi-process callers sharing the same backend root could both pass the check and overwrite each other. - `FilesystemSecretStore::consume` — two consumers could both observe an Active one-shot lease, both decrypt, and both overwrite the consumed marker. - `FilesystemCredentialBroker::consume_session_use` — two consumers could both pass the max-uses check at `uses=N-1` and overwrite each other's increment, losing a use. Both now use the canonical retry-on-`FilesystemError::VersionMismatch` pattern from `ironclaw_engine::store::filesystem::update_thread_state` (post-`e2530adff`): re-read, re-evaluate the consume/use-limit condition, write with `CasExpectation::Version(versioned.version)`. A shared `CAS_RETRY_ATTEMPTS = 3` constant bounds the loop; exhausting it surfaces a transient backend error rather than papering over pathological hot-spots. Also annotated `leases_for_scope` with a `TODO(perf)` covering the N+1 list+get fan-out — bounded today by the owner-prefix path layout and short lease TTLs; replacing it with `Filter::Eq` over `query` requires the secrets store to declare its first index, which is a follow-up. Regression coverage: two new tests wrap `InMemoryBackend` with a `VersionRacingBackend` that bumps the watched path's version out-of-band on the first versioned `put`, forcing a `VersionMismatch` and exercising the retry loop. They also assert that the retried CAS write actually persisted (the next consume hits LeaseConsumed; the next three increments exhaust the max-uses budget). * fix: address remaining P2 review findings on #3679 Four P2 correctness fixes from the codex/gemini review. **Settings keys use percent-encoding** (`src/db/filesystem_settings.rs`): `encode_segment` previously mapped `/`, space, control chars, and others all to `_`. Keys like `a/b` and `a_b` collided onto the same path and silently overwrote each other. Now percent-encodes every byte outside the unreserved set so distinct inputs map to distinct outputs. **SQL index names get a blake3 suffix on overflow** (`crates/ironclaw_filesystem/src/db.rs`): `sql_index_name` truncated identifiers exceeding 62 chars without disambiguating, so two distinct long `(prefix, name)` specs could collapse onto the same DDL object — `CREATE ... IF NOT EXISTS` would silently reuse the wrong index/trigger. Now appends an 8-char blake3 hash suffix before truncating. Added `blake3 = "1"` to the crate's deps (small + already used by other workspace crates). **InMemoryBackend rejects writes over implicit directories** (`crates/ironclaw_filesystem/src/in_memory.rs`): the SQL backends refuse `put(/a)` when `/a/b` exists (treating `/a` as a directory). The in-memory reference impl silently accepted those writes, letting tests pass against production-impossible state. Mirror the SQL contract. **Event-store head-probe is bounded** (`crates/ironclaw_reborn_event_store/src/filesystem_store.rs`): The replay-gap detection previously called `tail(path, 0)` to read the whole log just to look at its last seq — O(N) on every cold-path call. Now probes `tail(path, after - 1)`: a non-empty result means head == after (consumer is caught up); empty means head < after (foreign-future cursor). Returns at most one record instead of the entire log. Verification: cargo check --workspace --all-features clean; cargo test -p ironclaw_filesystem -p ironclaw_secrets -p ironclaw_reborn_event_store --all-features all pass. * fix(filesystem): close cross-backend Range/vector semantic drift and txn scope hole Audit findings on ironclaw_filesystem turned up four bugs and three semantic-drift cases between the in-memory reference and the SQL backends. Fix them in one pass so the cross-backend contract is honoured and the gaps have regression coverage. Bugs: - libSQL `Filter::Range` on `IndexValue::Bool` never matched any row because SQLite's `json_type` returns "true"/"false" for booleans rather than "integer". Replaced the static type string with a `json_type_guard` expression that admits both bool variants. - `ScopedStorageTxn` did not enforce that per-op `VirtualPath`s lay under `mount_prefix`. The trait doc promised `PathOutsideMount` for cross-prefix accesses; the wrapper now enforces it so any future backend that ships `begin()` inherits the guarantee. - Mixed-variant `Filter::Range` bounds (e.g. I64 lo + Text hi) silently lex-compared on text on both SQL backends. Added the in-memory backend's `discriminant(lo) == discriminant(hi)` guard to both, rejecting with `Unsupported`. - SQL `vector_nearest_query` lacked the in-memory backend's path tie-breaker on equal cosine scores, so top-k truncation was non-deterministic. Added `.then_with(|| a.0.cmp(b.0))` to both. Semantic drift: - `FilesystemOperation` lacked an event-plane `Append` variant — default impl reported `Tail`, backends reported `AppendFile`. Added the variant, routed every emit site through it, and updated the downstream `host_runtime::operation_allowed` matcher. - `decode_embedding_blob` and `cosine_similarity` were byte-identical copies in three files. Extracted to `crate::vector`. - libSQL `run_migrations` ran multiple ALTERs outside any transaction. Wrapped the sequence in BEGIN IMMEDIATE / COMMIT with rollback on error so a crash can't leave a half-migrated schema observable. Tests added: - 16 `ScopedFilesystem` permission tests covering query / ensure_index / begin / append / tail across each `MountPermissions` axis, plus 4 `ScopedStorageTxn` tests driving a stub backend to lock in the per-op ACL and the new path-containment check. - Cross-backend regression tests in `tests/db_root_filesystem_contract.rs` for the libSQL Bool/Range fix, the discriminant guard on both SQL backends, and the deterministic vector tie-breaker. - Refactored `vector_nearest_query`'s phase-2 step into `materialize_ranked` (`pub(crate)`) so a unit test can exercise the "row disappeared between phases" branch deterministically. 128 tests pass, all three feature combos compile (`default`, `libsql`, `postgres`), workspace builds. * revert(db): drop filesystem-backed src/db/ store facades Removes all `src/db/filesystem_*.rs` facades and the `src/db/filesystem_workspace/` directory added during the PR #3679 universal-FS dispatch migration: - filesystem_conversations, filesystem_jobs - filesystem_routines, filesystem_sandbox, filesystem_tool_failures - filesystem_identities, filesystem_pairing, filesystem_settings, filesystem_users - filesystem_workspace/{mod,chunks,documents,paths,search,versions}.rs Also removes the supporting infra that only existed for these files: - `ironclaw_filesystem` workspace dep from the root `ironclaw` crate - `/sandbox`, `/routines`, `/tool_failures`, `/identities`, `/pairing`, `/workspace` entries from `ironclaw_host_api::path::VIRTUAL_ROOTS` The legacy libSQL/Postgres sub-trait impls (`src/db/postgres.rs`, `src/db/libsql/*.rs`) remain the sole backing for the `Database` supertrait. The unified `ironclaw_filesystem` mount fabric itself (the `crates/ironclaw_filesystem/` crate) is untouched and still used by consumer crates outside `src/db/`. Verification: - cargo fmt --check clean - cargo check --workspace clean (default features) - cargo check --no-default-features --features libsql clean - cargo check --all-features clean - cargo clippy --all --benches --tests --examples --all-features clean [skip-regression-check] pure removal of unmerged migration facades. * test(reborn-event-store): cover caught-up-to-head + concurrent appends Addresses audit finding F1. (a) `filesystem_event_log_caught_up_to_head_returns_empty_not_replay_gap` appends N events, replays from the last entry's cursor, and asserts `entries.is_empty()` + `next_cursor == last.cursor` with no `ReplayGap`. Pins the "consumer is caught up to head" branch of the bounded probe in `read_after_cursor`. (b) `filesystem_event_log_concurrent_appends_assign_distinct_cursors` spawns 8 `tokio::spawn` tasks each appending one event to the same stream, then asserts the collected cursors are pairwise-distinct and strictly increasing. Guards the per-stream monotonic-cursor invariant under contention. * fix(reborn-event-store): preserve filesystem error detail in durable mappers Addresses audit finding F2. `map_filesystem_append_error` / `map_filesystem_tail_error` previously collapsed every non-categorised `FilesystemError` variant (`VersionMismatch`, `NotFound`, `Backend`, …) to a fixed generic string, dropping the source variant and reason. Operators lost the detail they needed to debug appends that hit a CAS conflict or a backend I/O failure. Thread the underlying `FilesystemError` through its `Display` impl on the fallback arm. `FilesystemError` is already redaction-safe by contract — it renders scoped/virtual paths, never raw host paths — so the durable error surface gains debug detail without violating the crate-level redaction policy. The three already-categorised variants (`PermissionDenied`, `MountNotFound`, `Unsupported`) keep their fixed messages so callers can pattern-match on the substring. * fix(reborn-event-store): document deliberate absence of Filesystem config variant Addresses audit finding F3. `FilesystemDurableEventLog` / `FilesystemDurableAuditLog` are exported from this crate, but `RebornEventStoreConfig` has no corresponding `Filesystem` variant — so production composition still routes through the SQL stores. The PR description documents this as intentional: the filesystem-backed log is the migration target for the kernel-storage rework, and the config variant will be added during the `src/db/` dissolution pass (task #17). Without an inline comment, a future reviewer reading the config enum has no signal that the missing variant is deliberate. Add a doc paragraph on `RebornEventStoreConfig` pointing at the rationale on `filesystem_store.rs` and at task #17. * fix(reborn-event-store): drop shadowed kind named-arg in stream_path format! Addresses audit finding F4. `stream_path` previously used the named-argument `format!` form with `kind = kind_segment`, where the named key `kind` shadowed the function parameter of the same name. Switch to the implicit positional-capture form (`format!("/events/{kind_segment}/...")`) and rename the inline bindings to `tenant_segment` / `user_segment` for consistency. Pure refactor — no behaviour change, just removes the readability footgun. * fix(outbound): add typed CasConflict variant for filesystem store retries Audit finding F5: `map_fs_error` previously collapsed both `FilesystemError::VersionMismatch` (a transient compare-and-swap race condition that callers should retry) and `FilesystemError::Unsupported` (a permanent capability gap) into `OutboundError::Backend`. The bounded CAS retry loop (added separately for F1) cannot match on `Backend` — that would also retry on permanent backend failures and on `Unsupported` on backends that don't support CAS. Introduce `OutboundError::CasConflict` and map `VersionMismatch` to it in `map_fs_error`. The variant stays internal to the crate: the retry loop matches on it discriminator-wise; once the retry budget is exhausted (or for callers that haven't migrated) it converts to `Backend` before crossing the trait boundary, preserving the no-leak contract. Update `is_transient_validator_error` to classify `CasConflict` as transient for defence in depth, even though it should never reach the service boundary in practice. * fix(outbound): CAS-version read-then-write paths with bounded retry Audit finding F1 (HIGH): the four read-then-write methods on `FilesystemOutboundStateStore` (`upsert_subscription`, `advance_subscription_cursor`, `record_delivery_attempt`, `update_delivery_status`) read the existing entry, applied an in-memory transform, then wrote with `CasExpectation::Any`. Concurrent writers raced the transform: in particular, the "subscription cursor must not move backwards" invariant — enforced in `validate_advance_request` / `validate_subscription_cursor_progression` — was unenforced cross-process, because two racing advancers could both read the same old cursor, validate against it, and then both put their newer cursors, the loser silently winning the last-write race. Capture `VersionedEntry.version` from each `get`, pass `CasExpectation::Version(v)` to the matching `put`, and retry on the typed `OutboundError::CasConflict` introduced by F5. The retry budget is bounded (`MAX_CAS_RETRIES = 5`) and the loop re-reads + re-validates on every iteration, so a regressing cursor or scope mismatch surfaces immediately rather than letting the retry loop overwrite the winner's state. `put_thread_notification_policy` is a blind overwrite and keeps `CasExpectation::Any`. `record_delivery_attempt` uses `CasExpectation::Absent` for the first-write branch, so two racing at-least-once writers can't both insert; the loser falls back into the duplicate-identity-check branch on the next read. * fix(outbound): use control-character sentinel in thread scope key Audit finding F6: `thread_scope_key` used the literal string `"_"` as the sentinel for `agent_id = None` / `project_id = None`. The `validate_scope_id` validator in `ironclaw_host_api` accepts underscore as a legal character in an `AgentId` / `ProjectId`, so a scope with `agent_id = Some(AgentId::new("_"))` hashed to the same key as a scope with `agent_id = None`. Two distinct scopes silently collided on the same policy/subscription/delivery virtual path. Switch the sentinel to `"\x1F"` (ASCII unit-separator). It's a control character; `validate_scope_id` rejects every C0 control char via `has_forbidden_control`, so no legal scope id can ever contain it. Add a unit test that pins the sentinel-rejection invariant and a regression test that proves `agent_id = Some("_")` no longer hashes to the same key as `agent_id = None`. * fix(outbound): query indexed scope projection with paginated drain Audit finding F2 (HIGH): `list_delivery_attempts` was a `list_dir` + N+1 `get_json` per row with no indexed projection, scanning every delivery on the mount even when only one scope's deliveries were requested. Cost scaled with total delivery count, not with the queried scope's row count. Declare an exact-equality index on a new `scope` indexed key. The projected value is the same `thread_scope_key` hash used for policy paths — collision-resistant against the legal id grammar and updated by F6 to never collide with the `None` sentinel. `record_delivery_attempt` and `update_delivery_status` write through a new `put_delivery_attempt_indexed` helper that includes the projection; `update_delivery_status` preserves it on status mutations. The list path drives `query(Filter::Eq { key: "scope", value: ... })` and re-checks `scope_matches` defensively (hash collisions are unreachable but cheap to guard against). Audit finding F3 (Medium): the previous `list_dir` was unpaginated; SQL backends issue `LIMIT Page::MAX_LIMIT (1024)` on their list_dir translation and would silently truncate past 1024 deliveries. The new path drains pages via `offset += received` until a short page arrives, mirroring `ironclaw_engine::store::filesystem::query_all`. `ensure_delivery_scope_index` runs idempotently before every write and read. It tolerates `FilesystemError::Unsupported` on byte-only backends to match the engine store's `ensure_exact_index` pattern; the in-memory backend serves `Filter::Eq` from `Entry::indexed` directly even without a materialized index declaration. * test(outbound): cover CAS retry, pagination drain, backwards-race Audit finding F4: the existing `outbound_state_store_contract` suite exercised the storage contract surface but had no coverage for any of the failure modes the F1/F3 fixes address: - No CAS-retry test. F1's bounded retry loop could regress to permanent failure on any transient `VersionMismatch` and the suite wouldn't notice — the in-memory backend never produced one. - No `> Page::MAX_LIMIT` drain test. F3's pagination loop could lose the tail of a long delivery list and the suite wouldn't notice because the existing tests record at most one delivery per scope. - No concurrent backwards-race test on `advance_subscription_cursor`. The existing backwards-advancement test only exercised the single- threaded path; nothing proved the post-F1 retry loop re-validates progression on every iteration. Add three regression tests: 1. `VersionRacingBackend` wraps `InMemoryBackend` and injects a single `FilesystemError::VersionMismatch` on the next `put` matching a configured prefix. The first new test (`advance_subscription_cursor_retries_through_cas_conflict`) arms one conflict, advances the cursor, asserts the retry loop converges, and asserts exactly one conflict was injected and consumed. 2. `concurrent_backwards_race_rejected_after_winner_advances` runs two sequential advances — the winner to cursor=100 and the loser to cursor=50 — and asserts the loser is rejected with `InvalidRequest` while the winner's state is preserved. Together with the retry test this proves the re-validate-on-retry semantics F1 calls out. 3. `list_delivery_attempts_drains_more_than_page_max_limit` writes `Page::MAX_LIMIT + 1` delivery attempts under one scope and asserts `list_delivery_attempts` returns every one. Before F3 this would silently truncate at 1024 rows. Cargo.toml: enable `tokio/sync` for `Mutex` in the test mock; drop the feature-conditional `use std::sync::Arc` because the new tests need it unconditionally. * fix(run-state): bound filesystem lock map under tenant churn The process-wide FILESYSTEM_RECORD_LOCKS map kept one Arc<tokio::sync::Mutex<()>> per touched path. In long-running hosts with high tenant/invocation churn the map grew without bound, since entries were never removed once the originating put/get cycle completed. Switch the value type to Weak<Mutex> so dropped Arcs no longer pin map slots. Each acquisition opportunistically prunes dead entries before upgrading-or-installing, keeping the map size proportional to in-flight paths rather than to lifetime path count. Concurrent callers on the same path still observe the same Arc (the outer std::sync::Mutex serializes the upgrade-or-insert window), so existing intra-process and cross-instance serialization guarantees are preserved — both verified by the new unit tests and by the existing filesystem_*_duplicate_*_serialized_across_store_instances contract tests. Addresses audit findings F1 (Medium) and F4 (Low). * fix(run-state): use versioned CAS for filesystem run/approval writes All filesystem put() calls used CasExpectation::Any, so two host processes mounting the same /engine could lose updates: each one's read-modify-write saw the other's value and then unconditionally overwrote it. The per-path async mutex only serializes intra-process callers. Switch creates to CasExpectation::Absent and updates to CasExpectation::Version(v) with a bounded retry loop on VersionMismatch. The new put_with_cas helper centralizes the contract: on capable backends (InMemoryBackend, the upcoming SQL ports) cross-process races now fail closed and the caller retries; on byte-only backends that return Unsupported (LocalFilesystem) we degrade to Any but emulate Absent with a get() precheck so the AlreadyExists path is preserved. The in-process lock map (F1) keeps the check-then-write race closed for the byte-only fallback. Approve/deny/discard pull the record-lock guard up to the trait method, since update_status no longer acquires it. Addresses audit finding F2 (Medium). Closes the gap acknowledged in crates/ironclaw_run_state/CLAUDE.md. * fix(approvals): type approval-resolution decision with ApprovalDecisionKind enum Addresses audit finding F1. Replaces the stringly-typed `impl Into<String>` decision parameter on `AuditEnvelope::approval_resolved` with a wire-stable `ApprovalDecisionKind` enum (`Approved`/`Denied`, `#[serde(rename_all = "snake_case")]`), so approval callers cannot drift on capitalization or spelling. Per `.claude/rules/types.md` "wire-stable enums". The wider `DecisionSummary::kind` field stays a `String` because other audit producers (authorization denials, obligation handlers) emit values outside the approval enum; cross-decoding remains a follow-up. Cross-crate blast radius: `ironclaw_host_api` (new enum + factory signature), `ironclaw_approvals` (both call sites), `ironclaw_events::tests::durable_log_contract` (three test fixtures). * fix(approvals): persist approval state before issuing lease Addresses audit finding F2. Inverts the lease/approve ordering inside `approve_capability_action`: the approval store write now runs *before* the lease store write. The previous order (issue lease, then approve, best-effort revoke on failure) left a window where a transient approval-store error could leave a live lease pointing at a request whose status remained `Pending`. The approval record is now treated as the authority of record. Once the request flips to `Approved`, lease issuance is a recoverable operation against an already-decided request — if the lease store fails, the caller surfaces the lease error and the request stays `Approved`. The previous best-effort `let _ = self.leases.revoke(...)` swallow is gone with the same edit. Updates the three concurrency/error-injection tests to assert the new semantics, plus the crate CLAUDE.md guardrail. No external test fixtures break — the public resolver API is unchanged. * fix(approvals): route both resolve paths through emit_approval_resolved helper Addresses audit finding F3. Extracts an `emit_approval_resolved` helper on `ApprovalResolver` so the audit-envelope construction in `approve_capability_action` and `deny` is built in exactly one place. Both call sites used to inline `AuditEnvelope::approval_resolved` against their own `record.scope`/`denied.scope`; while consistent today, divergence between the two would be a silent regression. Pure refactor — no test changes needed beyond the existing audit-event contract tests which already pin the wire shape. * fix(approvals): cover concurrent approve_dispatch first-write-wins Addresses audit finding F4. Adds a caller-level concurrency regression test that spawns two `approve_dispatch` calls against the same pending request on a multi-thread tokio runtime and asserts the expected first-write-wins invariants: - exactly one approve returns `Ok` - the other returns `ApprovalResolutionError::NotPending { status: Approved }` - the lease store ends up with exactly one Active lease (not two, not zero — under the F2 persist-approval-first ordering the loser fails *before* lease issuance, so no orphan to revoke) - the approval record's terminal status is `Approved` Enables `rt-multi-thread` on the tokio dev-dependency so the test can exercise real cross-thread contention on the approval store mutex. * fix(engine): restore HybridStore parity for mission updates F1: `update_mission_status` now bumps `mission.updated_at` before writing back, matching HybridStore (`src/bridge/store_adapter.rs:1950`). Recency-sorted views (mission list UIs, learning-mission dispatcher) were silently freezing the timestamp at original-save time. F2: `list_missions` and `list_all_missions` now sort by `(name, id)` after collection, matching HybridStore (`store_adapter.rs:1913, 1937`). The underlying `query`/HashMap iteration is non-deterministic; the LLM-facing `mission_list` tool was seeing arbitrary order across runs. Tests: - `update_mission_status_bumps_updated_at` — regression for F1 - `list_missions_is_deterministic_across_invocations`, `list_all_missions_is_deterministic_across_invocations` — regression for F2 * fix(memory): drain pages in FilesystemMemoryDocumentRepository::list_documents Audit findings F1 (HIGH) + F9 (Low). F1: `list_documents` issued a single `query(.., Page::new(0, Page::MAX_LIMIT))` and trusted the page was complete. Because `Page::MAX_LIMIT == 1024`, scopes holding >1024 documents silently lost every entry past the cap. The result fed `write_document`'s ancestor/descendant conflict check at the call site immediately above, so a new path could shadow (or be shadowed by) an existing document across the truncation boundary without a conflict ever firing — exactly the regression `query_all_pages` was extracted in `src/db/filesystem_jobs.rs` to prevent. F9: The old implementation issued a `Filter::All` query, threw the results away (`let _ = (versioned, &prefix_str);`), then called `list_dir` to discover paths. The query-result loop was dead code under any backend that supports `query`. The stale comment claimed the trait didn't surface paths in `query` results, but `VersionedEntry.path` (`crates/ironclaw_filesystem/src/record.rs:347`, added in PR #3659) has carried the absolute virtual path for every queried row since. Replace both with a single drain loop that paginates `query` until a short page comes back, filters by `entry.kind == "memory_document"`, and recovers the `MemoryDocumentPath` directly from `VersionedEntry.path`. The `list_dir` fallback is gone, and the agent_id axis is preserved through `MemoryDocumentPath::new_with_agent` so scopes with an agent identity round-trip correctly (the previous code's `new()` dropped the agent). Regression: `list_documents_drains_pages_beyond_max_limit` writes `MAX_LIMIT + 5` documents and asserts every one comes back. This also exercises the conflict-check path because each `write_document` calls `list_documents` internally. * fix(secrets): close consume_if_matches timing oracle with constant-time compare F1 (HIGH, timing oracle) in the 2026-05 audit: `consume_if_matches` in `legacy_store.rs` (trait default + in-memory backend) and `db.rs` (libSQL + Postgres backends) compared the decrypted plaintext against the caller-supplied expected value with `!=`. Rust's `!=` over `&str`/`&[u8]` short-circuits on the first differing byte, so an adversary who can observe response latency over the network can recover the secret byte by byte. AES-GCM authenticated decrypt closes the ciphertext oracle but does nothing for the post-decrypt comparison. Fix: route the comparison through `subtle::ConstantTimeEq::ct_eq`, which walks the full buffer regardless of where the bytes diverge. The post-comparison branches retain their original shape because the decrypt+lookup path is already executed unconditionally before the compare — only the success-side `DELETE` differs, and that signal is already exposed by the function's return value. Added a regression test (`f1_consume_if_matches_uses_constant_time_compare`) that grep-asserts the production source imports `subtle::ConstantTimeEq`, uses `ConstantTimeEq::ct_eq`, and no longer contains the legacy `!=` shape. Cannot meaningfully prove constant-time-ness from a shared CI runner, but the source-pattern check ensures a "simplifying" revert fails review. Audit: F1 (HIGH). * fix(secrets): use constant-time compare for store key-check sentinel F3 (Low) in the 2026-05 audit: `verify_secret_store_key_check` compared the decrypted sentinel against `SECRET_STORE_KEY_CHECK_PLAINTEXT` with `!=`. The plaintext is a fixed compile-time string so the practical risk is low — an attacker who can move the encrypted_value/key_salt blobs across rows already has full DB write access — but the same constant-time pattern applied to F1 makes the comparison style consistent across the crate and pre-empts a future caller threading a non-constant sentinel through this helper. Routed through `subtle::ConstantTimeEq::ct_eq`, mirroring the F1 fix. Audit: F3 (Low). * fix(processes): index queryable fields and serve records_for_scope via query Replace the N+1 list_dir + per-file get scan with an indexed `query` path, falling back to the legacy scan on byte-only backends so existing LocalFilesystem-driven tests and production deployments remain unaffected. - Declare `ensure_index` lazily for the per-owner `processes/` prefix on the queryable fields called out in the audit (`tenant_id`, `user_id`, `status`, `extension_id`, `parent_process_id`). Backends without index support degrade to the existing scan instead of failing closed. - Project the same fields onto every `ProcessRecord` write via `Entry::with_indexed`; record-capable backends (libSQL, Postgres, the in-memory backend) can now serve scope listings through a native query. The opaque-byte fallback in `put_with_byte_fallback` keeps LocalFilesystem (which rejects record-shaped puts today) on the legacy write path. - Rewrite `records_for_scope` to issue `Filter::And` of `Filter::Eq` predicates against the indexed projection. The full `same_scope_owner` check remains in Rust so the sub-scope axes (agent/project/mission/ thread) that are not yet in the index spec still get filtered. - Add a contract test that exercises the indexed path through `InMemoryBackend` and confirms cross-tenant and cross-user records are not returned. Addresses audit findings F1 (records_for_scope N+1) and F2 (missing ensure_index at startup). * fix(filesystem): surface backend infrastructure errors without fabricated paths F1: SQL backends used valid_engine_path() (unwrap_or_else unreachable returning /engine) as a placeholder on every connection/migration error. The path was always a lie - at pool acquisition, run_migrations, pragma setup, or schema bootstrap there is no caller-supplied virtual path in scope - and it leaked into operator-facing error display. Add FilesystemError::BackendInfrastructure { operation, reason } that omits path. Route every former valid_engine_path() callsite in libsql and postgres through new infrastructure_error helpers in db.rs. The enum is non_exhaustive so adding a variant is backward compatible. Regression test: drive a libsql migration against a read-only DB file and assert BackendInfrastructure with no /engine in display. * fix(filesystem): store VirtualPath keys in InMemoryBackend state directly F2: in_memory.rs::query() reparsed every stored row's path with VirtualPath::new(...).unwrap_or_else(|_| unreachable!('stored paths originated as VirtualPath')) on the hot path. Two issues: - the reparse is wasted work - paths originate as VirtualPath at put() time, so the validation pass on read is redundant - 'unreachable!' is a panic that asserts a structural invariant the type system already enforces Replace HashMap<String, StoredEntry> with HashMap<VirtualPath, StoredEntry>. Lookups now pass &VirtualPath directly; prefix scans move to key.as_str().starts_with(...). VersionedEntry::path comes from a single clone() instead of a parse + unreachable. Existing tests cover the put/get/query/list_dir/stat/delete paths that were touched (44 in_memory tests + the cross-backend contract suite). * fix(filesystem): align in-memory backend on nested VectorNearest semantics F5: SQL backends reject Filter::VectorNearest nested inside And/Or with Unsupported because ranking can't be expressed as a WHERE fragment - the top of query() peels off a top-level VectorNearest before the translator runs, and the translator's VectorNearest arm unconditionally errors. The in-memory backend previously treated a nested VectorNearest as 'any row with IndexValue::Bytes at key', silently changing semantics across backends. Add contains_nested_vector_nearest() pre-check in InMemoryBackend:: query that walks the filter tree and surfaces Unsupported for any VectorNearest strictly inside a compound. The Filter::VectorNearest arm in filter_matches is now unreachable; it returns false to keep the scalar predicate path safe should the pre-check ever be bypassed. Regression test asserts Unsupported on nested-in-And, nested-in-Or, and still-OK for top-level VectorNearest. * fix(filesystem): guard u64 to i64 SQL bindings with typed errors F6: SQL backends used 'expected.get() as i64' and 'page.offset as i64' casts on the CAS and query/pagination paths. Both inputs are u64 and both wrap silently on values >= 2^63 - the cast produces a negative SQL binding that either matches no row (CAS quietly VersionMismatches) or executes against a negative OFFSET (cryptic backend error). Add db.rs helpers: - record_version_to_i64: surfaces CorruptRecordVersion if the value overflows i64 - page_offset_to_i64: surfaces a typed Backend error naming the operation and offset Apply at libsql.rs CAS and query offset bindings and the matching postgres.rs sites. 'page.limit' is u32 clamped to Page::MAX_LIMIT so its i64 cast is safe by construction and uses i64::from for clarity. Regression test asserts a typed Backend(Query) error with reason 'page offset...' when querying with offset = u64::MAX, replacing the prior silent wrap. * fix(filesystem): scope Postgres FTS GIN index to declaring prefix F4: libsql FTS5 virtual tables are declared per-mount-prefix - one vtable per ensure_index(prefix, ...) call - so a query at one prefix can't accidentally pull index postings from a sibling prefix into the plan, and tearing down an index for a prefix is a clean DROP TABLE. The Postgres FTS GIN index, by contrast, was created without a predicate over root_filesystem_entries, so it was global. Correctness held because the query path always scopes by 'path = OR path LIKE ', but parity with libsql broke in two ways: the planner considered postings from every prefix before filtering, and a per-prefix DROP INDEX could only ever tear down one of them. Add a partial-index predicate gated by 'path = <prefix> OR path LIKE <prefix>/%' to the GIN DDL. The prefix is sourced from the validated VirtualPath and quotes are doubled for safe SQL literal embedding; LIKE-special characters are escaped via the existing escape_like_with_trailing_wildcard helper. Regression test (Postgres only; skipped when no DB is reachable) reads back the DDL via pg_indexes.indexdef and asserts the prefix literal and a WHERE clause appear. * fix(filesystem): tighten capability docs, type constraints, and hygiene nits Batched audit findings: F3: Document the type constraint on IndexKind::Prefix. The kind is only meaningful against IndexValue::Text, but ensure_index can't see the value type at declaration time. Filter::PrefixOn rejects every non-text variant at query time. Document the constraint loudly so consumers reach for IndexKind::Exact when projecting numeric or boolean values instead of getting an unused index and a query-time Unsupported. F7: BackendCapabilities::sql_typical advertises a minimum SQL shape that omits IndexFts and IndexVector. The two real backends here (libsql + postgres) layer them on top. A hand-rolled backend that just calls sql_typical() would under-advertise. Add a doc-comment calling out the omission and an sql_typical_full() variant that includes Events + IndexFts + IndexVector for backends that match this crate's shape. F8: validate_simple_identifier indexed bytes[0] after an is_empty guard. The guard makes the index sound, but the pattern is fragile to refactors. Switch to bytes.first() so the dependency is explicit and the panic path goes away. F9: Multiple doc comments in record.rs and index.rs referenced stale type names (StorageBackend::put/list/query, Record). Update to the current RootFilesystem / Entry names. * fix(engine): dedupe events on append_events for HybridStore parity HybridStore (`src/bridge/store_adapter.rs:1613`) de-duplicates thread events by id before insert. The filesystem-store `append_events` impl was previously writing with `CasExpectation::Any`, which silently overwrote an existing event with the same id when callers re-emitted (e.g. recovery after a partial flush). Pre-read the destination path and skip any id already present. Matches HybridStore's append-only contract. Audit finding F3 (Medium) from the ironclaw_engine crate audit. * fix(memory): map FTS results to documents in FilesystemMemoryDocumentRepository::search_documents The previous scaffold issued the `Filter::Fts` query, then silently dropped the results with `let _ = results; Ok(Vec::new())`. A caller wiring up the trait would see an empty result set and assume "no matches" — when in fact the search had simply lied. That is worse than returning `Unsupported`. Map each `VersionedEntry.path` (added in PR #3659) back to a `MemoryDocumentPath`, de-dupe by path, and assign a per-rank score from RRF over the FTS-only branch so the result vector matches the native repos' fusion contract for the trivial single-branch case. Skip non-memory-document entries that may live under the same prefix (chunk projections, metadata siblings). Adds `list_documents_drains_pages_beyond_max_limit` test against the in-memory backend. Audit finding F2 (HIGH) from the ironclaw_memory crate audit. * fix(secrets): close revoke CAS-loop race with versioned compare-and-swap `revoke` previously read the lease via the (now-removed) `read_lease` helper and wrote with `CasExpectation::Any`. The per-lease process-local mutex serialized writers within one process only — multi-process callers sharing the same backend root could observe `Active`, race against `consume`, and clobber a `Consumed` marker by overwriting it with `Revoked`. Inline the read into a bounded CAS retry loop matching `consume` and `consume_session_use`: read with version, write with `CasExpectation::Version`, retry on `VersionMismatch`. Make revoke idempotent on terminal states (`Consumed`, `Revoked`, `Expired`) so the loop converges even when a winner has already written. Audit finding F2 (Medium) from the ironclaw_secrets crate audit. * fix(processes): use versioned CAS for status transitions `update_status` previously read the record and wrote with `CasExpectation::Any`, relying on the per-instance `transition_lock` for atomicity. That lock only serializes within one process; a multi-process deployment sharing the same backend root could observe identical pre-transition state in both processes and clobber each other's status flips. Replace with a bounded CAS retry loop: read with version, validate the transition, write with `CasExpectation::Version…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #3666. Adds a new
FilesystemOutboundStateStorethat persists outbound metadata under/engine/outbound/...through anyRootFilesystem— additive, no removals. The existingInMemoryOutboundStateStoreandLibSql/Postgresbackends stay on theOutboundStateStoretrait so callers can flip to the new store via a constructor change at their convenience.Why additive, not destructive
Unlike
ironclaw_processes(#3666), the rest of the consumer crates don't currently touchironclaw_filesystemat all — their SQL backends are independent. Migrating means adding a new filesystem-based store rather than modifying an existing one. The deletions oflibsql_store.rs/postgres_store.rs/db.rscome in a final cleanup PR per consumer once the new store has run in production.Path scheme
/engine/outbound/policies/<thread-scope-hash>.json— thread notification policy keyed by(tenant, agent?, project?, thread)./engine/outbound/subscriptions/<sub-key-hash>.json— projection subscription cursor. The key hashes(subscription_id, actor, scope, thread)so the path doesn't leak the actor on directory listing — preserves the anti-enumeration semantics inOutboundStateStore::load_subscription_cursor's docstring./engine/outbound/deliveries/<delivery_id>.json— delivery attempt keyed bydelivery_id.list_delivery_attemptscurrently scans the deliveries directory and filters by scope in memory; once the SQL backends' query plane is wired into this consumer, the scan becomesquery(prefix, Filter::Eq { key: "scope", ... }).CLAUDE.md guardrails preserved
validation::*checks first).OutboundError::Backendviamap_fs_errorsoFilesystemErrorhost-path detail can't leak.ThreadProjectionAccessGrant,ValidatedReplyTargetBinding) are unaffected — this PR only touches the store layer; the policy service is unchanged.Test plan
cargo test -p ironclaw_outbound --test outbound_state_store_contract filesystem_store_satisfies_outbound_contract_on_in_memory_backend— passes the full contract suite (durable_policy_, subscription_cursor_, delivery_status_, notification_policy_, full_turn_scope_isolation) that already runs againstInMemoryOutboundStateStoreand the SQL backends.cargo clippy -p ironclaw_outbound --all-features --tests— cleancargo fmt --check— cleancargo check --workspace --all-features— clean/engine/outbound/...is the right path namespace, or whether outbound should claim a top-level root (would require adding/outboundtoVIRTUAL_ROOTSinironclaw_host_api).Stacked PR
Base is
reborn/fs-consumer-migrations(PR #3666). Merge order: foundation → port → indexer → processes consumer → this. Subsequent consumers (authorization, run_state, conversations, secrets) follow the same additive pattern, each as its own PR.