Skip to content

fix(bridge): register resolver for resynced asks so answers resume the parked run - #11

Merged
bigduu merged 1 commit into
mainfrom
fix/9-resync-ask-resolver
Jul 15, 2026
Merged

fix(bridge): register resolver for resynced asks so answers resume the parked run#11
bigduu merged 1 commit into
mainfrom
fix/9-resync-ask-resolver

Conversation

@bigduu

@bigduu bigduu commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Closes #9.

Root cause

resync_pending_asks (post-restart ask recovery) re-parks a pending question as state.pending_ask = Some(parked) but never sets state.ask_resolution — there's no waiting render_until_settled task after a restart to own that sender. When the user then answers, try_resolve_pending_ask matched the ask but required ask_resolution to also be Some, so the matched answer fell through to the normal busy/queue routing and process_one started a brand-new run on a session that was still suspended server-side waiting for that exact answer.

Fix

try_resolve_pending_ask now returns a PendingAskMatch enum distinguishing:

  • Live(sender, answer) — the ordinary case, a render_until_settled task is waiting; unchanged behavior.
  • Resynced(ParkedAsk, answer) — no live task (resync case). The caller resolves inline via a new resolve_resynced_ask, which reuses the exact subscribe-before-respond logic render_until_settled's live pause branch already used (extracted into respond_and_resubscribe), then keeps watching the resumed run through render_until_settled — using the answering message's own ReplyCtx, since resync itself never recovers one (documented on resync_pending_asks).

There's no ask_ref to edit in the resync case (resync never re-renders the ask as a message, per its existing doc comment), so the ✅-edit step from #8 is skipped there but stays unchanged on the live path.

busy is now set to true atomically, in the same lock as the match, when a resync answer is resolved — mirroring handle_inbound's own busy-then-spawn dance for a fresh message — so a message racing in immediately behind the answer queues instead of racing a second concurrent run onto the same session. drain_chat's queue-draining tail is extracted into drain_queue and reused by resolve_resynced_ask so busy is correctly cleared (and anything that queued up gets processed) once the resumed run settles.

Both handle_inbound (text answers) and handle_callback (button presses) were updated to handle the new PendingAskMatch variants.

Expired-ask semantics

No new "this ask expired" messaging was added. The bug was the silent fallthrough itself — once a resync-matched answer resolves inline instead of falling through, there's no more silent "start a new run" path left to guard against. Every remaining failure mode already has an explicit reply:

  • respond/subscribe failures on the resync path → the existing "Failed to record your answer" / "Answer recorded, but failed to resume watching the run" replies (shared with the live path via respond_and_resubscribe).
  • A stale pre-restart button nonce (resync always mints a fresh nonce, so an old visible button can never match post-restart) → the existing callback-path "This action has expired." ack, unchanged and still exercised in the resync scenario (see the new resync_pre_restart_button_nonce_is_stale_and_reports_expired test).
  • Ordinary non-matching text that isn't an answer at all → still falls through to normal message routing, per the intentional, already-tested match_text_answer contract (not changed here).

Tests

5 new tests in src/bridge.rs:

  • resync_parks_the_ask_with_no_live_resolver — confirms the gap resync_pending_asks leaves (pending_ask set, ask_resolution None).
  • resync_text_answer_resumes_the_parked_run_instead_of_starting_a_new_one — the core regression test: asserts api.chat_calls/execute_calls stay empty after a matching text answer (i.e. no new run), and that subscribe happens before respond, exactly like the live path. I sanity-checked this test fails (times out) against the pre-fix fallthrough behavior.
  • resync_callback_answer_resumes_the_parked_run_instead_of_starting_a_new_one — same regression, via a button press.
  • resync_pre_restart_button_nonce_is_stale_and_reports_expired — a stale pre-restart nonce still gets the "expired" ack, not a silent new run.
  • resync_answer_marks_the_chat_busy_and_drains_a_message_queued_behind_it — busy/queue semantics around a resync resolution.

Test plan

  • cargo fmt --check
  • cargo clippy --all-targets (no new warnings)
  • cargo build --tests
  • cargo test — 182 passed, 0 failed (177 lib + 5 main)

…e parked run

resync_pending_asks re-parks a pending question after a restart with no
live render_until_settled task waiting, so ChatState::ask_resolution is
never set for it. try_resolve_pending_ask required BOTH a matching
pending_ask AND a live ask_resolution sender, so a matched answer to a
resynced ask fell through to the normal busy/queue routing and started
a new run on a session that was still suspended server-side (#9).

try_resolve_pending_ask now distinguishes PendingAskMatch::Live (the
ordinary case: hand the answer to the waiting render task) from
::Resynced (no live task — the caller resolves inline). Resolving
inline reuses the exact subscribe-before-respond logic render_until_settled's
live pause branch already used (extracted into respond_and_resubscribe),
then keeps watching the resumed run through render_until_settled using
the answering message's own ReplyCtx, since resync itself never
recovers one. There's no ask_ref to edit in the resync case (resync
never re-renders the ask as a message), so the ✅-edit step is skipped
there but unchanged on the live path. busy is set atomically with the
match so a message racing in right behind the answer queues instead of
racing a second run onto the same session; drain_queue (the queue tail
extracted from drain_chat) clears busy once the resumed run settles, so
the queued message still gets processed afterward.

Expired-ask semantics: no new "this ask expired" messaging was added.
The core bug was the fallthrough itself — once fixed, a resolvable
answer never falls through, and existing explicit-failure replies
(subscribe/respond errors, callback "This action has expired.") already
cover every case where resolution genuinely can't proceed.

Adds 5 tests covering: the resync gap itself (parked with no resolver),
text-answer and callback-answer resolution resuming the same parked
run instead of starting a new one, a stale pre-restart button nonce
still reporting "expired" without starting a new run, and busy/queue
draining around a resync resolution.
@bigduu
bigduu merged commit 4f7d85a into main Jul 15, 2026
3 checks passed
@bigduu
bigduu deleted the fix/9-resync-ask-resolver branch July 15, 2026 20:17
bigduu added a commit that referenced this pull request Jul 16, 2026
resync_pending_asks re-parks an ask after a restart but never sets
busy=true (no live render task exists yet to own that invariant) — so
an unrelated inbound message arriving between park and answer used to
fall through to normal busy/queue routing and start a second
concurrent run on a session that's still suspended server-side.

handle_inbound now queues any message that doesn't match a chat's
pending_ask, regardless of busy, reusing the existing FIFO queue +
drain_queue machinery from PR #11. invalidate_pending_ask now reports
whether the invalidated ask had no live resolver (the resync case) so
/stop and /new drain the backlog they leave behind too — via a
detached task so handle_inbound still returns immediately, and (for
/new) only after the session map rotation so drained messages land on
the fresh session. Also logs any parked ask / queued backlog still
outstanding at graceful shutdown, since ChatState is in-memory only.

Closes #12.

Co-authored-by: bigduu <mugeng.du@gmail.com>
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.

bridge: answers to resynced asks fall through to a new run — resync parks the ask without a resolver

1 participant