Repository navigation
test(host-agent): wait for finalize-record deletion — don't race the job's post-persist steps - #629
Merged
Merged
Conversation
…job's post-persist steps snapshot_begin_completes_and_produces_an_eviction_final_checkpoint waited only for the CheckpointRecord file to appear, then immediately asserted the EvictionFinalizeRecord was deleted and pending_finalizes cleared. The checkpoint file becomes VISIBLE at rename time, but the finalize job deletes the record only after persist() returns — which, since the durable-record engine grew a parent-directory fsync (an F_FULLFSYNC on macOS, tens of ms), happens measurably AFTER the rename. The test's poll can now win that window: main went red on exactly this assertion (run 29059847354) the first time the fsync landed. The production ordering is correct and unchanged (checkpoint durable → delete finalize record → clear pending_finalizes → destroy sandbox); the test was asserting a point-in-time snapshot of a still-running job. Wait for the deletion and the map-clear the same way the test already waits for the destroy. Fixed test passes 10/10 repeated runs locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nikhilunni
added a commit
that referenced
this pull request
Jul 18, 2026
… oracle, the gap-family seeds TTL clock -> now_mono: MigrationExport carries the injected clock; created_at/last_activity are now_mono readings and expired() subtracts against the injected clock — expiry DECIDES destroy/abort, so it is decision-feeding time (D1), off the metrics_now carve-out it previously rode. The prod constructors bind PooledBackend.clock; the Linux-only peer page server (PeerExport, which shares the same Arc anchor — #216 Gap 2) converts with it, carrying the same injected clock. The paused sim clock now drives the REAL expired() deterministically. (The musl cross-check caught the Linux-only PeerExport half — the macOS sweep can't see migrate_peer.rs.) Sim (engram-dst-host): steps MigrationBegin (a REAL MigrationExport in the REAL MigrationRegistry; deterministic entropy-minted export id — the prod OsRng nonce must not launder into the replayable id stream; the guest freezes exactly as the export's held capture lock excludes writes/flushes/captures — and the Flow F interleaving steps now gate on it, which also fixes a hang the swarm found: a flush step on a frozen slot armed a seam an empty pipeline never reached), MigrationServeState (the split-brain flag), MigrationTouch, MigrationTtlSweep (REAL expired() + ttl_verdict over the scriptable coordinator's ownership answer, applied as lib.rs does), MigrationCommit, MigrationAbort — in both swarm profiles (pick roll widened 0..112; old seeds re-explore, fine per the seed contract). Oracle #7 (the #216 decision table): state_served => never abort-unpause — a split-brain un-pause is structurally recorded by the sweep/abort appliers, so a ttl_verdict regression or bypassing caller fires it. Oracle #2 (no-plane-leak): migrating <=> an open registry export, with a live backend — no frozen guest ever leaks without an export to end it. Seeds: ttl_expired_unshipped_export_aborts_in_place_zero_loss, state_served_export_never_unpauses_then_destroys_on_ownership_flip, actively_serving_export_never_expires_mid_transfer (#216 Gap 1), unreachable_coordinator_stays_paused_never_guesses, reattached_source_verdict_never_destroys_on_a_transient_binding (Gap 3). Scope notes (recorded in the ADR row): #582/#598/#629 turned out to be FC-lane/test-hygiene issues whose portable content P5/P7 already absorbed — no hollow seeds manufactured. HostEffects::production consolidation deliberately retired rather than done: every seam reaches its flow through its own field; the bundle ctor remains the sim's assembly point (the pooled TODO now says so). ADR: P8 row -> Landed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
What broke
Main went red on the macOS lane after #615 merged (run 29059847354, 1527/1528 passed):
snapshot_begin_completes_and_produces_an_eviction_final_checkpointpanicked on "the in-flight finalize record must be deleted on completion".Root cause — a test race that #615's dir-fsync widened, not a product bug
The production ordering is correct and unchanged (
eviction_finalize.rs): persist the durableCheckpointRecord→ delete theEvictionFinalizeRecord→ clearpending_finalizes→ best-effort destroy. But the test waited only for the checkpoint file to appear, then immediately asserted the finalize record was gone. The checkpoint file becomes visible at rename time, while the deletion happens only afterpersist()returns — and #615 added a parent-directory fsync (correctly — the classic durable-rename gap) after the rename, which on macOS is anF_FULLFSYNCcosting tens of ms. The test's poll now wins that window with real probability; the very first main run carrying the fsync tripped it.Fix
Wait for the deletion and the
pending_finalizesclear the same way the test already waits for the destroy call, instead of asserting a point-in-time snapshot of a still-running background job. Test-only change.Validation
cargo nextest run -p engram-host-agent: 343 passed.clippy -D warnings+fmt --checkclean.🤖 Generated with Claude Code