mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
307521f155
* fix(processes): resume runs whose lease expired at a safe checkpoint A hosted run that lost its lease died as a user-visible failure, even when it had committed nothing and was sitting idle waiting on the model. Lease recovery could only tell "has a checkpoint" from "has none", so every checkpointed run was treated as possibly-mid-side-effect and failed terminally with `lease_expired`. Record what the checkpoint actually was. `ProcessCheckpointKind` (`BeforeModel` / `BeforeSideEffect` / `BeforeBlock`) now rides on the process snapshot beside `checkpoint_ref`, because a recovery sweep reads process rows without loading checkpoint rows. Recovery requeues a run whose latest checkpoint replays no external effect, under the same bounded crash-reclaim budget; `BeforeSideEffect` and unknown kinds stay terminal, since no durable idempotency exists for a replayed capability call. The requeue waits one full lease TTL past expiry before acting. A worker starved of heartbeats and a dead worker look identical from the journal; a worker still running would have renewed its lease inside that window, so anything still expired afterwards is genuinely gone. Nothing else changes timing: cancellation and the checkpointless requeue stay immediate. Old journals deserialize with no kind, and an unrecognized kind degrades to "unknown" rather than failing the whole snapshot — both read as side-effecting, so the fail-closed path is the default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): give the process journal its own Postgres pool The journal heartbeat is the liveness signal a run's lease depends on, and it was sharing one max-size-2 connection pool with every other Postgres consumer — the event store, triggers, result reads. One turn's read burst could saturate that pool and starve the run's own heartbeat until the lease expired underneath it. Two changes, both about not putting the heartbeat behind other traffic: - A Postgres deployment opens a small second pool (2 connections) and mounts the journal's filesystem over it. The mount set is byte-identical to the data plane's, so the journal addresses the same rows over a different connection — only the pool differs. libSQL and in-memory arms are untouched; libSQL is single-writer by design. - The default data-plane pool goes 2 -> 8. Two was small enough that one turn queued behind itself. Operators sizing connections should budget `pool_max_size + 2` per instance; docs, the shipped Docker config, and its smoke assertion follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(turn_runner): let a turn survive one slow store checkout Hosted turn runs heartbeat every 5 seconds, and the supervisor uses that interval as each heartbeat's timeout, so the generic budget of 3 consecutive failures tolerated about 30 seconds of stall — no more than a single Postgres connection-checkout timeout. One slow checkout was enough to abandon a healthy run. Turn runs now get 8, which clears a checkout stall while still giving up inside the 90-second lease TTL. That bound is the point: a worker that stops on its own leaves a live lease behind, whereas one still retrying past the TTL gets its run reclaimed out from under it. The budget is therefore derived from the configured heartbeat interval rather than fixed, so widening the interval shrinks the budget instead of producing an abandon window that outlives the lease. The generic `ProcessSupervisorConfig` default stays at 3 — the capability path heartbeats every 30 seconds, where 8 would run far past the TTL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): route the shipped Docker configs to the test that pins them `Detect Reborn test scope` failed closed on `docker/reborn/config.production.toml`, skipping every downstream Reborn lane. The planner's comment claimed the runtime configs have no owning lane, but `crates/app/ironclaw_cli/tests/smoke.rs` parses both `config.toml` and `config.production.toml` and pins their boot profile, storage backend, pool sizing and runtime policy — so a lane does read them, and static control would have skipped exactly the assertions such an edit can break. Add `ROOT_FIXTURE_TEST_OWNERS`, the read-at-test-time counterpart of `EMBEDDED_ASSET_OWNERS`, mapping each config to the `smoke` test target that asserts it. The two hosted-single-tenant configs stay unclassified: no test parses them, so they must keep failing closed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(processes): pin the serde and grace-window edges review flagged Three review findings on the recovery path, none behavioural: - `lease_duration_millis` now converts the TTL once instead of three verbatim copies in claim, heartbeat and expiry recovery, so the bound and its rejection message cannot drift between them. - The in-grace sweep now runs strictly after expiry. Expiry is inclusive (`lease_expires_at <= now`), so the old instant did select the process and the assertion was not vacuous — but pinning the grace hold on a boundary instant made that a property of the comparison rather than of the grace window. - A persisted snapshot carrying an unrecognized `checkpoint_kind`, or none at all, now has coverage at the serde boundary: it degrades to `None` rather than failing the whole read, and `None` already recovers as side-effecting, so an older host fails closed. The retry-projection fixture also derives `kind` the way `put_loop_checkpoint` does instead of claiming `BeforeModel` in metadata while storing `None`, and now asserts the retried snapshot inherits the kind — the propagation was previously unasserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(turn_runner): cap the heartbeat interval the lease cannot afford Both reviewers found the same hole. `heartbeat_failure_budget_within_lease` computed how many failures the lease TTL could pay for and then clamped the result up to 1 — but at an interval past half the TTL, even one failure costs more than the lease. An operator setting `heartbeat_interval_secs = 120` against the 90s default got a budget of 1 whose abandon window was 240s: the journal would declare the lease expired while the worker was still waiting on its first heartbeat, which is exactly the "worker is provably gone" premise recovery relies on. Cap the interval at half the TTL instead of clamping the budget, so `budget >= 1` is honest for every configurable value and the scheduler errs toward heartbeating more often than asked. The explicit budget setter is capped by the same lease-derived ceiling — it was the other way to construct a config past the TTL. The test's `|| budget == 1` escape hatch was the hole itself; it is gone, and the 120s case plus a 10x-TTL case are now asserted for real. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(composition): prove the journal pool reaches the data plane's rows The only coverage of the journal's separate Postgres pool built two independent `InMemoryBackend`s and wrote through each, which cannot prove the two pool-backed handles reach the same rows — a wrong connection config or a shared-pool fallback passed it unchanged. Drive `postgres_from_config_and_env` (the only public constructor that resolves a connection config, and so the only one that opens the second pool at all), submit a turn through the turn coordinator so the journal writes its process row over its own pool, and read that row back over a connection neither build pool owns. Docker-gated through the existing `postgres_pool_or_skip` harness; the in-memory test stays as the cheap mount-set parity check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(contracts): describe the expired-lease machine that ships Both contracts said an expired lease transitions to `RecoveryRequired`. Recovery converges directly to a settled state instead: cancellation to `Cancelled`; no checkpoint or a replay-safe one (`BeforeModel`, `BeforeBlock`) back to `Queued`, the checkpointed case only after a full lease TTL of grace; a side-effecting or unrecognized checkpoint, or an exhausted reclaim budget, to `Failed`. Code is the deliberately-reviewed behavior here, so the docs follow it. `turn-runner.md` §3 carries the full transition table and notes that the legacy `RecoveryRequired` status still exists in the vocabulary but is no longer produced by expiry; `turn-persistence.md` §6 gets the summary and points at it, so the two cannot drift into two half-descriptions again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(planner): classify the shipped docker/reborn configs by their parsing test `Detect Reborn test scope` failed closed on `docker/reborn/config.production.toml` (`unclassified pull-request path`), cascading into the whole `Tests (Reborn)` roll-up on #7471. The planner's comment asserted these configs have no owning lane. They do: `crates/app/ironclaw_cli/tests/smoke.rs` parses both `config.toml` and `config.production.toml` through `RebornConfigFile::parse_text` and asserts on the profile, storage backend and policy. So they are not static control (whose membership rule is "no Reborn test lane reads the file") and not prose — either would silently under-select the one lane that catches a broken production config. `DOCKER_RUNTIME_CONFIG_OWNERS` routes each to that test target instead. The two `config.hosted-single-tenant*.toml` siblings stay fail-closed: their reader is `tests/dockerfile_runtime_home.rs`, which `_root_test_partitions()` does not inventory, so no lane can be selected for them. Verified against #7471's real diff: the planner exited 1 before and exits 0 after, naming the owner in its reasons; all 75 planner self-tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(processes): fence stale executors on lease reclaim; address review comments - supervisor: never start a replacement executor while the reclaimed process's prior executor is still running; a definitive lease-lost heartbeat (InvalidLease/InvalidTransition) now cancels the stale task at the next tick instead of waiting out the failure budget - turn scheduler: clamp heartbeat intervals past half the lease TTL (budget-of-one no longer masks an unaffordable interval); CLI config rejects them with a clear error - extract lease_duration_millis helper shared by claim/heartbeat/recovery - tests: fence regression (no executor overlap), unknown checkpoint-kind wire degradation, in-grace sweep strictly past expiry, retry kind propagation, stale-worker reply assertions, Postgres pool isolation, planner owner pin, smoke test for the interval bound - docs: turn-runner expired-lease contract aligned with the state machine * chore(deps): bump lru 0.18.1 -> 0.18.2 (RUSTSEC-2026-0253) cargo-deny fails fast-checks on the lru 0.18.1 panic-safety advisory (use-after-free in LruCache::pop, patched in 0.18.2, issued 2026-08-11). All four dependents (composition, webui, extension_host, hooks) already require `lru = "0.18"`, so this is a lock-only patch bump. * fix(loop): lease-fence transcript writes so a reclaimed worker cannot ghost-reply Lease recovery requeuing a safe checkpoint opened a window the journal alone cannot close: run transitions are lease-fenced (ensure_lease), but transcript writes were not, so a worker whose lease recovery already reclaimed — starved of heartbeats while blocked in a model call, or suspended past the grace window — could wake and append a second assistant answer beside the replacement worker's. Time-based fencing (abandon window + one TTL of grace) bounds only a worker whose runtime is live to observe its heartbeat failures; it can never be total. The other half of the guarantee: ThreadBackedLoopTranscriptPort now carries the lease the run was claimed under and asks the journal — the only authority on ownership — before every transcript write (draft begin/update, finalize, capability-result append). A stale or unverifiable lease refuses the write as an explicit transcript-write failure; the zombie's loop exit then fails through its own lease-fenced claim, so nothing it produces can land on the run the replacement completed. The turn runner's host factory installs the fence for every claimed run. The recovery branch comment in the process journal now states the two-part guarantee honestly instead of over-claiming that the old executor has "provably given up". Regression coverage: the lease_wedge integration test releases the stale worker after the recovered run completes and asserts its output never reaches the transcript; a seam test pins that the production host build installs the fence at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(processes): drop duplicated lease_duration_millis from the merge Both reconciled lines extracted the same helper; the merge kept both copies and E0592'd. One definition remains, with the fuller doc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(runtime): address lease recovery review feedback (#7471) * ci: reseed composition budget for merged tree (#7471) * docs(turns): clarify legacy recovery lock release (#7471) * fix(architecture-tests): move the absolute-mass record with the reseeded ceiling The 2026-08-11 budget reseed (41582 -> 41810 for #7471's merged tree) updated the manifest ceiling but not the paired record constant this ratchet compares it against, leaving 228 LOC of apparent headroom — past the 200-LOC nudge window, so the merge-queue run failed reborn_restructure_baseline_ratchets_stay_armed. Re-measured on this tree: 41731 (`check-composition-budget.sh`). The 41810 ceiling was seeded from the merge-queue commit, where concurrent mainline growth adds ~79 LOC on top of this branch — inside the nudge window of the corrected record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>