fix(agent): stale replies after queued user turns#90759
Conversation
|
Codex review: stale review; fresh review needed. Summary Next step Review history (5 earlier review cycles)
|
|
After comparing this with #96533 against the exact #76888 failure shape, I think #90759 is the stronger landing candidate. The main reason is that #76888 is not only about how an orphaned user leaf is merged once found. The reported root cause is that the orphaned user message can be hidden behind later non-message session metadata, so the runner never repairs the right leaf in the first place. #90759 fixes that at the embedded-runner/session boundary by walking back to the trailing message entry, replaying skipped state entries, and removing the repaired orphan from provider history. It also includes stronger end-to-end proof: the PR body has redacted production If maintainers prefer the stale-internal-drop semantics from #96533, that is a valid product decision. But for the exact #76888 reproduction, #90759 looks closer to the root cause and more complete as a fix. |
3d218c6 to
5b3fa1b
Compare
a173364 to
014239a
Compare
|
Maintainer follow-up pushed in 014239a.
Exact-head CI: 43 jobs green, including build, prod/test types, lint, guards, and all Node test shards. The two QA smoke failures are unrelated packaging failures: this older branch lacks the CHANGELOG.md release section for 2026.7.2. |
014239a to
492dd34
Compare
|
Landed via squash onto main.
The two QA smoke failures were unrelated release packaging checks: the branch lacked the release-owned CHANGELOG.md section for 2026.7.2. |
…0759) * fix(agent): keep orphaned user turns contextual * test(agent): prove orphan repair targets active prompt * fix(agent): repair orphaned user behind session metadata * fix(agent): satisfy orphan repair CI types * fix(agent): preserve orphan repair state entries * fix(agent): filter repaired orphan from assembled history * fix(embedded-agent): repair orphan session entry types * fix(embedded-agent): remap replayed metadata labels * fix(embedded-agent): skip dangling replay labels * fix(agents): repair orphan history before assembly --------- Co-authored-by: 张贵萍0668001030 <[email protected]> Co-authored-by: Ayaan Zaidi <[email protected]>
What Problem This Solves
Fixes an issue where users in channel-backed embedded-agent sessions could receive a visible reply to an older queued user message after sending a newer active prompt.
Closes #76888
The failure shape is that orphan repair only checked the current session leaf. Real sessions can append non-message metadata entries such as thinking/model snapshots after the orphaned user message before prompt submission, which hid the older user message from repair and left it eligible as an active instruction.
Why This Change Was Made
The embedded runner now looks past trailing non-message metadata that does not contribute LLM content, repairs the older user turn as context-only prompt material, branches away from that orphaned user entry, replays the skipped state entries, and keeps the already-sanitized active session messages for provider submission.
This keeps the fix scoped to the embedded-agent/session/provider boundary. It does not change Discord delivery, queue semantics, provider routing, model selection, public config/schema/protocol, API keys/auth, storage format migrations, or session persistence format. The remaining maintainer-owned decision is whether this preserve-as-context strategy should be canonical versus the alternative candidate in #96533, which uses different stale-prompt semantics.
User Impact
When an active channel prompt follows an orphaned queued user turn, the older turn is preserved only as prior-turn context and the latest active prompt remains the answer target. Users should no longer see the assistant answer the stale queued turn in this session-state shape.
The highest-risk area is embedded-runner session-state/prompt-shape compatibility. The mitigation is focused coverage for the metadata-skipping repair, state replay, label target remap, provider-seam prompt order, and the shared queued-user marker contract. Exact marker wording and final cross-PR session-state semantics remain maintainer review decisions.
Evidence
Current HEAD validation:
The current focused regressions prove the issue-specific provider-boundary behavior:
Reviewer-visible live behavior proof previously recorded for this PR:
Additional validation already recorded on this PR:
What was not tested: I did not send an actual Discord message through the live Discord transport, and I did not reproduce the original stochastic production stale reply end to end. The live proof used production
runEmbeddedAgentwith Discord channel metadata, a real JSONL session fixture, and a configured provider; the post-sync current-head follow-up reran the focused embedded-runner/provider-seam regressions and the plugin SDK declaration shard after rebasing onto current main and after the dangling-label replay guard.