fix(dag): mark wake reported at admit time - #326
Merged
Conversation
wake_reported landed only after the whole wake-driven turn completed, so a restart or mid-turn interruption left the synthetic part durable but the mark absent, and the startup sweep re-injected a byte-identical wake. Mark the batch (and unregister terminal workflows) immediately after admitIfIdle, before awaiting the turn; drop the turn-failure-redelivery semantics. Mark failures degrade to retry-later while the admitted turn still runs. Flips the two dag-wake-integration failure-redelivery assertions and reworks the two timeout-escalation Q2-gate tests (their "held turn => wakeReported=false" window no longer exists; they now hold the wake via a busy parent session). Adds regression tests for failed-turn no-redelivery and a simulated restart.
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 freeto 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.
Closes#321.
Incident chain (from #321)
After the terminal wake of
dag_fe5ff13ada6fUONA2rH3Pypdf4was injected at 17:25:29, the parent turn ran ~10 min when the process restarted at 17:35:22. Becausewake_reportedwas persisted only AFTERyield* delivered.value(the whole wake-driven turn completes) — while the transcript part is written at admit time — the mark never landed. The new process's startup sweep re-injected the byte-identical 4391-char wake (diff-verified): the in-memory dedup mapdeliveredWakeSummariesdoes not survive restarts. This is the hole the old GOAL-FP-01-14 rationale inloop.tsadmitted.Semantic change
Admit success = delivered.
markWakeBatchReported(and the terminal-workflowautomation.unregistercalls) now run immediately afteradmitIfIdleadmits the part, BEFORE awaiting the wake turn — dropping the "turn failure triggers redelivery" semantics. The synthetic part is durable in transcript history either way; redelivery added duplicates, never information. The restart window shrinks from minutes (turn duration) to microseconds (admit→mark).Mark failures (leases lost / generation raced / store write dying) degrade to retry-later instead of propagating: rows stay unreported, a later trigger re-marks, and the admitted turn still runs (its end-of-turn idle re-arms the retry). The in-memory dedup map is kept only for that narrow same-process mark-retry path.
automation-lease.tsunregister stays safe moved earlier: it gates its goal re-trigger on session status and relies on the turn re-emitting idle when it ends.What flipped in tests
dag-wake-integration— the two failure-redelivery tests flipped to the new contract: the batch is reported at admit even when the turn then fails, and no duplicate prompt is injected on a re-trigger. Added regression tests for both plus a simulated restart (fresh loop over the same store after an admitted-but-unfinished turn delivers nothing).dag-timeout-escalation— the two Q2-gate tests held their wake via an in-flight turn to keepwake_reported=false. That window no longer exists (the mark lands at admit), so they now hold the wake via a busy parent session (the idle-gate never admits it), preserving theescalationPending ∧ ¬wakeReported ⇒ skip re-timecontract. This coupling was not anticipated by fix(dag): mark wake reported at admit time (restart-safe wake delivery) #321 but is sanctioned by the mark-at-admit semantics; fulltest/dag/acceptance requires them green.Acceptance
wake_reported=1, no re-injection on next trigger.bun test test/dag/full suite green (551 pass, 0 fail).bun typecheckclean; repo lint under the--max-warningsratchet.