fix(agent): stop outgoing-reply notifications polluting the inbox snapshot - #18
Closed
terafin wants to merge 1 commit into
Closed
fix(agent): stop outgoing-reply notifications polluting the inbox snapshot#18terafin wants to merge 1 commit into
terafin wants to merge 1 commit into
Conversation
…pshot Synthetic outgoing-reply notifications (MessageID "reply-<taskID>") are one-shot: delivered in real time via OnIncoming and consumed by the caller. Their TaskID is an *outgoing* task that never exists in s.tasks, so several inbox-cleanup paths missed them and they lingered in the persisted snapshot, re-surfacing on every read/wake. Four fixes: - CompleteTask: drop matching inbox entries *before* the s.tasks lookup, so a synthetic reply (absent from s.tasks) is cleared instead of returning ErrTaskNotFound and leaving the entry behind. - appendSyntheticReply: skip contentless completions (a bare ack with no reply text) — the terminal state is already on the task, so an empty record is pure noise. - evictTerminal: evict aged one-shot reply records by identity (MessageID prefix) past a short delivery TTL, since the by-taskID drop never reaches them. - PeekInbox: consume one-shot reply records on read so a peek (vs a drain) doesn't leave them to re-surface; genuine incoming task messages are untouched, keeping peek non-destructive. Adds table tests covering all four paths and updates the inbox hook to select the live-PID inbox and prune stale-PID snapshot files. Signed-off-by: terafin <jwood@me.com>
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
2 participants
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.
Summary
Synthetic outgoing-reply notifications (
MessageID: "reply-<taskID>") are one-shot — delivered in real time viaOnIncomingand consumed by the caller. TheirTaskIDis an outgoing task that never exists ins.tasks, so several inbox-cleanup paths missed them: they lingered in the persisted inbox snapshot and re-surfaced on every read/wake.Fix (
internal/agent/store.go)CompleteTasks.taskslookup, so a synthetic reply (absent froms.tasks) is cleared instead of returningErrTaskNotFoundand leaving the entry behind.appendSyntheticReplyevictTerminalMessageIDprefix) past a short delivery TTL, since the by-taskID drop never reaches them.PeekInboxPlus
internal/assets/hook/a2a-inbox-hook.sh: select the live-PID inbox and prune stale-PID snapshot files.Tests
Adds table tests in
store_test.gocovering all four paths — each fails on the pre-fix code and passes with the fix:TestCompleteTaskClearsOutgoingReplyNotificationTestIngestOutgoingTerminalSkipsEmptyReplyTestEvictTerminalReapsDeliveredOutgoingRepliesTestPeekConsumesOutgoingReplygo test ./...green on go 1.25.0. No wire-protocol /internal/a2a/files touched.