fix(delivery): bound cursor-verify retries, give up instead of infinite re-inject#83
Open
zane668 wants to merge 1 commit into
Open
fix(delivery): bound cursor-verify retries, give up instead of infinite re-inject#83zane668 wants to merge 1 commit into
zane668 wants to merge 1 commit into
Conversation
…te re-inject The native PTY delivery loop verified delivery by watching for the target's cursor to advance. On a target whose cursor never advanced (busy panel / hook not writing back), the VerifyCursor timeout branch reset to Pending and re-injected the <hcom> wake prompt every ~VERIFY_TIMEOUT (10s) forever: the failure path reset `attempt` but never `inject_attempt` and had no give-up exit. Observed in production: 13 re-injects over ~90s against one target. Introduce a total inject-attempt cap (MAX_INJECT_ATTEMPTS). The timeout decision is factored into a pure `verify_timeout_decision` fn (Retry / SuccessNoCursor / FailReset / GiveUp) for unit testing. At the cap the loop parks in a new `State::GiveUp` that stops injecting and surfaces a "delivery stalled" gate status, re-arming only on genuine new activity (a fresh notify — send.rs wake_all fires per message — or the target cursor advancing). No silent message loss: give-up never deletes messages. `has_pending` is purely cursor-based over the persistent `events` table, so a parked message stays retrievable via `hcom` and re-delivers on the next activity. A drained queue (has_pending==false) always resolves as delivered and is never given up on. Adds 5 regression tests asserting injections terminate at the cap instead of looping unbounded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Session: 1422c55e-ec89-44cf-ad66-5e5538a19fb9 Producer: ai-session Tool: cc
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
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.
Problem
The native PTY delivery loop verifies delivery by watching for the target's cursor to advance. On a target whose cursor never advances (a busy panel, or a hook that isn't writing back), the
VerifyCursortimeout branch reset toPendingand re-injected the<hcom>wake prompt every~VERIFY_TIMEOUT(10s) forever: the failure path resetattemptbut neverinject_attempt, and there was no give-up exit. Observed in production as 13 re-injects over ~90s against a single target.Fix
Introduce a total inject-attempt cap (
MAX_INJECT_ATTEMPTS). The timeout decision is factored into a pureverify_timeout_decisionfn (Retry/SuccessNoCursor/FailReset/GiveUp) so it can be unit-tested directly. At the cap the loop parks in a newState::GiveUpthat stops injecting and surfaces a "delivery stalled" gate status, re-arming only on genuine new activity (a fresh notify —send.rswake_allfires per message — or the target cursor advancing).No silent message loss: give-up never deletes messages.
has_pendingis purely cursor-based over the persistenteventstable, so a parked message stays retrievable viahcomand re-delivers on the next activity. A drained queue (has_pending == false) always resolves as delivered and is never given up on.Tests
Adds 5 regression tests asserting that injections terminate at the cap instead of looping unbounded, plus the retry/fail-reset/success-when-drained branches of the decision function.
Verified locally with
cargo fmt --all -- --check,cargo clippy --all-targets --locked -- -D warnings, andcargo test --locked(all green).Only
src/delivery.rsis touched.🤖 Generated with Claude Code