Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
bf75799
feat(filesystem): add shared cas_update read-modify-write helper
henrypark133 Jun 25, 2026
356d945
refactor(resources): route CAS snapshot store through shared cas_upda…
henrypark133 Jun 25, 2026
8ab1219
refactor(run_state): route CAS through shared cas_update; drop redund…
henrypark133 Jun 25, 2026
fb8d2a2
refactor(threads): route ensure_thread through shared cas_update; dro…
henrypark133 Jun 25, 2026
f73361e
refactor(secrets): route CAS through shared cas_update; delete per-re…
henrypark133 Jun 25, 2026
aba904c
refactor(turns): re-home local CAS loop onto shared cas_update helper
henrypark133 Jun 25, 2026
3223540
docs(filesystem): guardrail — RMW must use cas_update, never a per-re…
henrypark133 Jun 25, 2026
0eaf69d
fix(reborn): address CAS migration review feedback
henrypark133 Jun 25, 2026
2097c6d
Merge origin/main into fix/reborn-p1-cas-helper
henrypark133 Jun 25, 2026
578e971
fix(reborn): address second-round CAS review feedback
henrypark133 Jun 26, 2026
4b061dd
fix(reborn): address third-round CAS review feedback
henrypark133 Jun 26, 2026
0d4bc1c
fix(reborn): close byte-only first-write fail-closed gap + error-path…
henrypark133 Jun 26, 2026
878a44c
fix(reborn): tighten byte-only CAS test + ensure_thread identity parity
henrypark133 Jun 26, 2026
e3e1558
test(reborn): back CAS-store test harnesses with InMemoryBackend, not…
henrypark133 Jun 26, 2026
805bd15
test(secrets) + docs(resources): already-expired-lease coverage; fix …
henrypark133 Jun 26, 2026
23be54b
Merge remote-tracking branch 'origin/main' into fix/reborn-p1-cas-helper
henrypark133 Jun 26, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .claude/rules/database.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ Don't just translate `CREATE TABLE`. Also check:

Multi-step operations (INSERT+INSERT, UPDATE+DELETE, read-modify-write) MUST be wrapped in a transaction. Ask: "If this crashes between step N and N+1, is the database consistent?" If not, wrap in a transaction. Applies to both backends.

For the `RootFilesystem`/`ScopedFilesystem` mount plane, a filesystem read-modify-write MUST go through `ironclaw_filesystem::cas_update` (the one shared bounded-CAS-retry helper). Never wrap it in a per-record `tokio::sync::Mutex` held across the backend `.await` — it is a redundant serializer over a backend that already does versioned CAS and convoys/wedges the runtime under burst. See `crates/ironclaw_filesystem/CLAUDE.md` invariant 2 and `docs/plans/2026-06-25-cas-migration.md`.

## libSQL Connection Model

`LibSqlBackend::connect()` creates a fresh connection per operation with `PRAGMA busy_timeout = 5000`. This is intentional -- no pool exists. Never hold connections open across `await` points. Satellite stores (`LibSqlSecretsStore`, `LibSqlWasmToolStore`) receive `Arc<LibSqlDatabase>` via `shared_db()` and call `.connect()` themselves -- never pass a live `Connection`.
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions crates/ironclaw_capabilities/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ pub(crate) fn approval_not_approved_error_kind(status: ApprovalStatus) -> &'stat
ApprovalStatus::Approved => "ApprovalApproved",
ApprovalStatus::Denied => "ApprovalDenied",
ApprovalStatus::Expired => "ApprovalExpired",
ApprovalStatus::Discarded => "ApprovalDiscarded",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — Discarded approval status is untested.

The new ApprovalStatus::Discarded arm has no direct regression test. Nothing asserts that discarded approvals map to ApprovalDiscarded, so this one-line branch could drift silently.

Fix: Add tests::helpers::approval_not_approved_error_kind_maps_discarded_status covering ApprovalStatus::Discarded -> "ApprovalDiscarded".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4b061dd — added approval_not_approved_error_kind_maps_discarded_status asserting ApprovalStatus::Discarded -> "ApprovalDiscarded".

}
}

Expand Down Expand Up @@ -409,4 +410,15 @@ mod tests {
CapabilityRunStateTransition::BlockAuth { .. }
));
}

/// Regression for the `ApprovalStatus::Discarded` arm added to
/// `approval_not_approved_error_kind`: ensures the arm maps to the
/// correct string constant and does not silently drift to another value.
#[test]
fn approval_not_approved_error_kind_maps_discarded_status() {
assert_eq!(
approval_not_approved_error_kind(ApprovalStatus::Discarded),
"ApprovalDiscarded",
);
}
}
28 changes: 28 additions & 0 deletions crates/ironclaw_filesystem/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,34 @@ codified in `docs/reborn/2026-05-14-universal-fs-dispatch.md` (the new ADR).
retry on `FilesystemError::VersionMismatch`. Consumers must never assume
`begin`/`StorageTxn` is available; backends that don't expose it return
`Unsupported` and that must be a working path.

**Use the shared `cas_update` helper — never a per-record mutex.** Every
filesystem read-modify-write MUST go through
[`ironclaw_filesystem::cas_update`](src/cas.rs) (bounded CAS-retry +
jittered backoff + overall timeout + fail-closed capability gate). Do NOT
wrap a filesystem RMW in a per-record `tokio::sync::Mutex`
(`FILESYSTEM_RECORD_LOCKS`-style) held across the backend `get`/`put`
`.await`: it is a redundant in-process serializer over a backend that
already does versioned CAS, and under burst it convoys every same-scope
writer behind one stalled writer (the 2026-06-24 runtime wedge). It also
tends to leak (a strong-`Arc` map that is never pruned). There is exactly
ONE CAS implementation — `cas_update`; do not re-copy the retry/backoff
loop into a store. `cas_update` fails **closed** on a non-CAS backend
(`CasUpdateError::CasUnsupported`) rather than falling back to a blind
`CasExpectation::Any` overwrite; all production store mounts resolve to
CAS-capable db/in-memory backends (`LocalFilesystem` is byte-only and is
structurally unreachable from those mounts), so fail-closed is correct.
See `docs/plans/2026-06-25-cas-migration.md`.

**Known scoped exception (tracked):** the `ironclaw_turns` runner-lease
sidecar (`filesystem_store/runner_lease.rs`, landed independently in
#5232) still drives its per-run lease records through a local
`put_with_cas` + `cas_retry_backoff` retry loop rather than
`cas_update`. This predates the migration on this branch and is not yet
re-homed; the main turn-state snapshot RMW *does* go through
`cas_update`. Do not copy the sidecar's loop as a precedent — migrate
it onto `cas_update` per follow-up #5274 (runner-lease CAS
consolidation). New stores must still use `cas_update` directly.
3. **Capabilities are declared, not discovered.** A backend that cannot
serve an `IndexKind::Vector` or a `Filter::Range` declares so up front
via `BackendCapabilities`; mount-time validation refuses the attachment.
Expand Down
2 changes: 1 addition & 1 deletion crates/ironclaw_filesystem/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ tracing = "0.1"

[dev-dependencies]
tempfile = "3"
tokio = { version = "1", features = ["macros", "rt"] }
tokio = { version = "1", features = ["macros", "rt", "test-util"] }
tracing-test = { version = "0.2", features = ["no-env-filter"] }
uuid = { version = "1", features = ["v4"] }
Loading
Loading