fix(kernel): stop the claim repair from stealing an in-flight claim (#160)

[kjgbot]

Closes #160. Unblocks #168. Do not merge without an independent signoff at the exact head — this is the exactly-once core.

The bug

CI's left: 2 was two_racing_deliveries_of_one_event_produce_exactly_one_run. It is a real exactly-once violation, not a flake: two racing deliveries of one event each started a run.

claim_event repairs a claim whose run never materialised, testing for it with "no row in runs for the claimed run_id". That predicate is true of two situations it could not distinguish:

• a run that crashed before registering — nobody will ever spawn it, so the claim must be taken over;
• a run that is in flight and has not registered yet — taking its claim over starts a second run for one event.

wake.rs makes the window explicit — the claim is written, and only several statements later does the run come into existence:

if self.registry()?.claim_event(&flow_key, &trigger.id, &event_key, &run_id)?.is_some() { ... }
let path = self.run_path(&run_id);
let mut journal = SqliteJournal::create(&path, &run_id, now_ms)?;   // ← run appears only here

So: A claims and proceeds; B's insert is ignored, B reads A's run_id, B finds no runs row because A is mid-window, B judges the claim abandoned, takes it over, and spawns. left: 2.

This explains why #168's sequential restart test passes (A is registered by then), why busy_timeout was necessary but not sufficient (lock contention, not this window), and why one commit both hung and passed.

The fix

The claim now carries the engine's boot id. A claim from this boot is in flight → dedupe. A claim from a previous boot with no registered run is wreckage → repair, exactly as before.

That deliberately preserves the crash recovery the original comment defends — it was guarding a real exactly-once bug of its own, where a claim pointing at a run that could never be resumed made every retry answer "deduped" with no run.

The same-boot rule creates one obligation: a claim this boot takes but cannot turn into a run would strand the event, because every retry inside the process would be told "duplicate" with no run to carry it. So the span from claim to register now releases the claim on its failure path, scoped to the claiming run_id so a release cannot steal a claim another delivery has legitimately taken over.

Existing databases get the column via a guarded ALTER TABLE (CREATE TABLE IF NOT EXISTS is inert against a table that already exists). Pre-existing rows carry '', which matches no live boot id and is therefore treated as a previous boot — which is right: the process that wrote them is gone.

Rejected: a time-based abandonment threshold. It swaps a correctness bound for a timing guess — the same class of mistake as the tick-count window in the allSettled repair.

Evidence

The racing integration test is probabilistic by nature: it only catches the bug when a loser reads inside the winner's window. Measured against a deliberately broken guard (if false && existing_boot == boot_id):

topology	runs caught
2 bare threads	5 / 10
2 threads + Barrier	8 / 10
8 threads + Barrier	7 / 10

More racers does not help — SQLite serialises the writes, so the miss is always "winner registered before any loser read". I kept the barrier and 2 racers.

A gate that misses a fifth of its regressions cannot be the only witness, so the rule is also asserted directly in relayflowd-journal, where no scheduling is involved: same-boot dedupes, previous-boot repairs, registered-run dedupes across boots, release frees the event, release is scoped to its run. Two of those fail deterministically under the mutation:

assertion `left == right` failed: a claim held by this boot is in flight, not abandoned
  left: None
 right: Some("run-a")
failures:
    registry::tests::a_same_boot_claim_with_no_run_yet_is_a_duplicate_not_wreckage
    registry::tests::releasing_is_scoped_to_the_claiming_run

Mutation applied and restored with hashes checked at each step (8829889b → 5e9b1a95 → restored), so the gate is verified against a failing witness rather than assumed.

Kernel workspace after: 148 passed, 0 failed, no warnings (143 before, 5 added).

Review notes

Worth an adversarial look at two things specifically:

1. Is per-Engine the right granularity for the boot id? Two Engines in one process are two "boots" here. That is what makes a drop-and-reopen restart test behave like a real restart, but it does mean two engines sharing a data dir in one process can repair each other's in-flight claims. No code path does that today outside tests.
2. The release-on-failure path. It fires only before register succeeds; after that the run is discoverable and a later failure is a run to resume, not a claim to hand back. Check I have the boundary in the right place.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR

[coderabbitai]

Warning

Review limit reached

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 0652e4e9-4e30-465a-ae4d-0c86c4c42344

📥 Commits

Reviewing files that changed from the base of the PR and between 350dc5a and 128a6ef.

📒 Files selected for processing (2)

• kernel/relayflowd/src/engine.rs
• kernel/relayflowd/tests/event_wake.rs

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: b03e8d11-f9ba-4aaa-ad43-ff7322513cbe

📥 Commits

Reviewing files that changed from the base of the PR and between dfc638e and 350dc5a.

📒 Files selected for processing (6)

• kernel/relayflowd-journal/src/lib.rs
• kernel/relayflowd-journal/src/registry.rs
• kernel/relayflowd-journal/src/subscriptions.rs
• kernel/relayflowd/src/engine.rs
• kernel/relayflowd/src/engine/wake.rs
• kernel/relayflowd/tests/event_wake.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The registry now enforces WAL mode and tracks engine boot IDs for event claims. Same-boot unregistered claims remain duplicates, while prior-boot claims can be repaired. Failed run registration releases claims. Concurrency tests verify exactly-once delivery.

Changes

Event deduplication lifecycle

Layer / File(s)	Summary
Registry storage and claim lifecycle kernel/relayflowd-journal/src/lib.rs, kernel/relayflowd-journal/src/registry.rs	The registry retries WAL activation, reports non-WAL mode, migrates boot_id, distinguishes same-boot and prior-boot claims, and adds scoped release_claim.
Registry migration and claim validation kernel/relayflowd-journal/src/registry.rs, kernel/relayflowd-journal/src/subscriptions.rs	Tests cover migration, boot-aware deduplication, claim repair, registered-run retention, scoped release, and updated claim_event calls.
Engine boot identity kernel/relayflowd/src/engine.rs	Each Engine constructor creates a fresh ULID boot identifier and exposes it within the parent module.
Wake claim and recovery flow kernel/relayflowd/src/engine/wake.rs, kernel/relayflowd/tests/event_wake.rs	Event wake passes the boot ID, releases claims after pre-registration failures, and tests concurrent delivery with one registered run.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EngineWake
  participant Registry
  participant RunRegistration
  EngineWake->>Registry: claim_event with boot_id
  Registry-->>EngineWake: claim or deduplicated result
  EngineWake->>RunRegistration: create and register run
  RunRegistration-->>EngineWake: success or failure
  EngineWake->>Registry: release_claim after failure

Loading

Poem

A rabbit records each boot
Claims follow the proper route
Old claims yield when boots renew
Failed runs release claims too
One event starts; one waits true

---

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing.

_{Comment @coderabbitai help to get the list of available commands.}

[kjgbot]

Correction to the evidence table above, and a flake I shipped

Two things in the opening description are now wrong. Both were caught by stress-testing my own change rather than trusting one green run.

1. The racing test was flaky, and my "148 passed, 0 failed" was one lucky run

Run 25 times, it failed 15 times with open run registry: database is locked. The test introduces the first genuinely concurrent access in the suite, and Registry::open could not survive it — every registry() call opens its own connection.

Two wrong fixes before the right one:

1. PRAGMA busy_timeout at the end of the pragma batch → still 34/50 failures.
2. Moved to the front, theorising the WAL switch ran first and was uncovered → still 36/50.

Instrumenting the batch statement-by-statement named journal_mode every time. SQLite takes an exclusive lock to change journal mode and does not invoke the busy handler for it, so busy_timeout cannot cover that statement however early it is set. I should have probed before the second guess.

The WAL switch now retries on a bounded loop and then verifies the mode, returning a new RegistryNotWal error otherwise — swallowing it would leave the registry in rollback-journal mode silently, which is the silent fallback this codebase refuses. busy_timeout is kept for ordinary statements, where it does apply.

50 of 50 racing runs pass now, 0 lock failures.

2. The kill-rate table was contaminated — the real number is much worse

With the lock noise gone I re-measured, separating assertion failures from lock failures:

	caught by assertion	lock noise	missed
previously reported	"8 / 10"	—	—
actual, 30 runs	3	0	27

Most of what I counted as "the test caught the bug" was database is locked. The racing test catches the seeded bug about 10% of the time, not 80%.

That does not change the fix, but it changes what the evidence is worth. The racing test stays because it exercises the real path and cannot false-positive — but it is not the gate, and its comment no longer claims to be. The deterministic registry::tests are the gate: they assert the same rule with no scheduling involved and fail under the mutation every time.

The boot_id fix itself is unchanged and still verified by those deterministic witnesses.

Kernel workspace: 148 passed, 0 failed, no warnings.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR

[kjgbot]

Status: signoff obtained, merge blocked on #174 — not on anything in this PR.

Condition 1 — independent signoff: met. The cloud review-swarm gate cannot run at all (it invokes agent-relay and no step installs it; exit 127 — a defect common to every flows PR), so I ran the repo's own local 3-lens preswarm review against this head instead:

PRESWARM_maintainability: REVIEW_PASSED
PRESWARM_history:         REVIEW_PASSED
PRESWARM_structure:       REVIEW_PASSED

It earned it. Across iterations the lenses caught, and I fixed:

• this branch reverting fix(gate): make the review-swarm scripts executable #172's executable bit on all three gate scripts (cut before fix(gate): make the review-swarm scripts executable #172 landed — a clean merge that undoes someone else's fix);
• an unindented IIFE around the release guard, now the named spawn_claimed_run whose doc states that register is deliberately last;
• a comment still asserting the registry repairs any unregistered claim, which stopped being true in this very PR;
• a surviving 8 in 10 measurement in registry.rs after I had corrected the same figure in event_wake.rs — the branch carried both numbers at once;
• a false scope claim about which files consume sdk/dist.

Condition 2 — green CI: not met, and not because of this change. Three runs were cancelled at ~25-30 minutes with the kernel step hung:

test agent::rung_c_sigkill_boundaries_resume_only_unfinished_steps_via_real_cli has been running for over 60 seconds
##[error]The operation was canceled.

The same test hangs on feat/gate2-wake-context (#168), which changes registry.rs by 0 lines and shares no code with this PR. Filed as #174.

I also chased and rejected one hypothesis honestly: my first read was that busy_timeout armed before an attempt-bounded WAL retry could stall an open for 50 × 5s. That is a real hazard and the reorder is kept on its own merits (crash_resume now runs 37.9s locally against main's 38.8s in CI), but it is not the cause — the hang survived it.

Not merging. The signoff is real, but this is the exactly-once core and I have no green CI run. Earlier tonight I reported this branch green off a single lucky local run and it turned out to ship a flaky test; merging on a weaker basis than that would be the same mistake twice. #174 is the gate.

Local evidence at this head: kernel workspace 149 passed, 0 failed, no warnings; racing test 30/30; crash_resume 34/34.