Skip to content

test(daemon): replay a settled hook failure once, not per poll - #4181

Merged
ScriptedAlchemy merged 1 commit into
masterfrom
fix/4172-replayed-admission
Oct 17, 2026
Merged

ScriptedAlchemy merged 1 commit into
masterfrom
fix/4172-replayed-admission

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Fixes #4172.

Root cause

The test was wrong. Production was not. Neither #4144's fix nor #4147's rework regressed.

The replay loop re-admitted the same envelope every 5 ms until the acknowledgement fired. While the replayed cycle ran, each pass joined it. After 32 joins, the next pass hit MAX_COALESCED_HOOK_COMPLETIONS and got Backpressured. The loop asserted Enqueued on every pass. A full unit_test binary loads the shared native runtime. Under that load, the replayed cycle stays in flight past 32 polls, about 160 ms, so the test failed.

Production behaves as designed. The coalescing bound is deliberate. The host spool treats Backpressured as replay-later (tracedecay-agent-hosts/src/hooks/dispatch.rs).

I checked the paths named in the issue against the current settle_operation:

  • The superseded successor that never got a permit passes None. It owns nothing.
  • Every path that holds a permit passes Some(permit). settle_operation drops it under the in-flight lock, right after it removes the entry.
  • The permit comes from the orchestrator's own permits semaphore.

#4147's atomicity guarantee is unchanged.

Change

The test now waits for the failed cycle to settle. It replays once and awaits that replay's terminal, so it no longer polls. It asserts exactly one acknowledgement and exactly two attempts.

Verification

  • Reproduced the failure by making the successful cycle take 400 ms. The old test failed with the reported left: Backpressured, right: Enqueued. The new test passes with the same delay.
  • A full //crates/tracedecay:unit_test run on master 46092b64d4 without this fix failed at types_tests.rs:570 with the same assertion.
  • The new test still catches the defects it exists for. Each check broke production temporarily:
    • The failed cycle leaks its permit (mem::forget on the retryable path). The test fails with Backpressured.
    • The failed cycle stays in the events map. The test fails with "a replayed admission must complete a fresh cycle after a retryable failure".
  • Load recipe: 30/30 (--runs_per_test=30 --local_test_jobs=20 --run_under=taskset -c 0-7).
  • I ran the gates on this commit combined with the test(daemon): concurrent re-enable scheduler test flakes 2/10 on master #4165 fix. //crates/tracedecay:unit_test passed in two full runs (901/901). //crates/tracedecay:daemon_suite passed. That suite does not compile these test files. Clippy (--config=clippy) and rustfmt_check_2024 passed.

Devin Review

The replay loop re-admitted the same envelope every 5 ms while the
replayed cycle ran. Each pass joined that cycle, and once 32 joins
accumulated the next one hit MAX_COALESCED_HOOK_COMPLETIONS and was
backpressured, which the loop asserted could never happen. A cycle on a
loaded process runtime outlives 32 polls, so the test failed under load
while production behaved as designed: the spool consumer treats
Backpressured as replay-later.

The test now waits for the failed cycle to settle, replays once, and
awaits that replay's terminal. It still fails if the settled failure
keeps its permit or stays in flight.

Fixes #4172
@changeset-bot

changeset-bot Bot commented Oct 17, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0e19056

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@ScriptedAlchemy
ScriptedAlchemy merged commit 90f69ae into master Oct 17, 2026
15 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(daemon): replayed hook admission test sees Backpressured

1 participant