fix(gateway): a steer/interrupt-triggered queued terminal turn gets its own ledger id - #106358
Open
nftpoetrist wants to merge 1 commit into
Open
nftpoetrist wants to merge 1 commit into
nftpoetrist wants to merge 1 commit into
Conversation
…ts own ledger id The queued-lane ledger saga (NousResearch#106312/NousResearch#106316, merged earlier today) fixed _run_agent_queued_followup's next_inbound_id so a chained queued turn no longer collides on the SAME delivery-ledger obligation id as an earlier reply in the chain -- but only for the pending_event branch. The sibling branch, reached when the follow-up has no originating event (a leftover /steer drained via result["pending_steer"], or a bare control interrupt_message), left next_inbound_id at its None default. That None flows into queued_terminal_inbound_id, and the call site that reads it back (gateway/run.py's `if _terminal_inbound:`) is a truthiness check, so a None there is treated exactly like "absent": event.ledger_message_id is never overridden, and the outer final send falls back to the id of the event that OPENED the chain. If this steer/interrupt-triggered terminal reply carries the same text as an earlier reply in the chain, both compute the same obligation id -- the earlier (possibly flood-refused) row is silently overwritten and marked delivered, and is never redelivered. This is the exact bug the saga just fixed, reached from the one branch it didn't cover. Fix: when pending_event is None, mint a synthetic id (uuid4, prefixed "steer:") distinct from any real transport id -- there is no natural identity to derive one from for a synthesized continuation, matching the existing keyless-turn-owner fallback shape already used elsewhere in this file (`_run_agent_prepare_turn`'s `owner` id). Verified empirically: read the consumer (`if _terminal_inbound:`) to confirm None is treated as absent, not as "no override needed" -- it's a real collision, not a defensive no-op. Added a regression test mirroring the saga's own `test_a_deeper_chain_keeps_the_innermost_inbound_id` pattern, asserting the inbound id is truthy, feeds into queued_terminal_inbound_id, and differs across two separate occurrences (not deterministic/reused). Mutation-verified: reverting run_turn.py drops exactly the new test (9 pass, 1 fail: `assert None` reproducing the bug exactly); reapplying restores green (10 passed). Also ran the sibling queued-native-image and multiplex-busy-input suites (32 passed) for interaction regressions -- none.
Contributor
SummaryIn FindingsNo blocking issues.
VerdictLooks good. No concerns. |
This branch has not been deployed
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.
What does this PR do?
Closes the one branch the queued-lane ledger saga (#106312/#106316, merged earlier today) didn't reach: a queued follow-up triggered by a leftover
/steeror a bare controlinterrupt_message(no originating queued event) still collides on the same delivery-ledger obligation id as an earlier reply in the chain.Problem
gateway/run_turn.py::_run_agent_queued_followupderivesnext_inbound_id— the id that keys the follow-up's own delivery-ledger obligation — only inside theif pending_event is not None:branch. When the follow-up instead comes fromresult["pending_steer"]or a bareinterrupt_message(pending_event is None),next_inbound_idstays at itsNonedefault.That
Noneflows intoqueued_terminal_inbound_id, which the caller ingateway/run.pyreads back with a truthiness check:Noneis indistinguishable from "absent" here, soevent.ledger_message_idis never overridden and the outer final send falls back to the id of the event that opened the chain. If this steer/interrupt-triggered terminal reply happens to carry the same text as an earlier reply in the same chain, both compute the identical obligation id — the earlier (possibly flood-refused) row is silently overwritten and marked delivered, and is never redelivered. This is exactly the bug the saga just fixed, reached from the one branch it didn't cover.Fix
When
pending_event is None, mint a synthetic id (uuid4, prefixedsteer:) distinct from any real transport id. There's no natural identity to derive one from for a synthesized continuation — this mirrors the existing keyless-turn-owner fallback already used elsewhere in the same file (_run_agent_prepare_turn'sownerid, which falls back touuid.uuid4()whenevent.message_idis absent).Testing
test_a_steer_triggered_terminal_turn_gets_its_own_inbound_id, mirroring the saga's owntest_a_deeper_chain_keeps_the_innermost_inbound_idpattern: asserts the inbound id is truthy, that it feeds intoqueued_terminal_inbound_id, and that two separate occurrences get different ids (not a deterministic/reusable value).gateway/run_turn.pydrops exactly the new test (9 pass, 1 fail —assert None, reproducing the bug directly); reapplying restores green (10 passed).uv run --frozen python -m pytest tests/gateway/test_queued_final_ledger.py tests/gateway/test_queued_native_image_session_key.py tests/gateway/test_multiplex_busy_input_mode.py -q→ 32 passed (checked sibling queued-lane suites for interaction regressions — none).ruff checkclean on both changed files.Type of Change
Checklist
fix(scope):,feat(scope):, etc.)