🤖 fix: skip agent_report reminder when follow-up message is incoming#2200
🤖 fix: skip agent_report reminder when follow-up message is incoming#2200ethanndickson wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3a0ac877a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
AgentSession's stream-end handler drains the queue synchronously while TaskService's isCompactionStreamWithFollowUp() yields. Capture hasQueuedMessages() before the await so the guard sees the true state.
6e1bcb8 to
249c499
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e1bcb8c43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
After compaction, history is cleared and replaced with a single assistant summary message. The old code searched for the last user message (which no longer exists) and called getCompactionFollowUpContent on it, always returning undefined. Now checks the last message for compaction-summary + pendingFollowUp, mirroring dispatchPendingFollowUp.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e73e0009e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dispatchPendingFollowUp() may append a follow-up user message before TaskService reads history, making the summary no longer the last message. Scan all messages (at most 2 after compaction) instead of only the last. pendingFollowUp is never cleared from the summary, so this check is stable regardless of ordering.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e849bff07e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed P1: Added regression tests covering:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3cdaa4056
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the summary with output parts. | ||
| for (let i = summaryIndex + 1; i < historyResult.data.length; i++) { | ||
| const msg = historyResult.data[i]; | ||
| if (msg.role === "assistant" && msg.parts.length > 0) { |
There was a problem hiding this comment.
Treat empty follow-up completion as no longer pending
The compaction guard only stops suppressing reminders once it finds an assistant message after the summary with parts.length > 0, but a follow-up stream can legitimately end with zero parts; in that case StreamManager does not rewrite the placeholder in history, so this check keeps returning true and handleStreamEnd() keeps exiting early. When that happens, the task never reaches the agent_report reminder/fallback path and can remain stuck in a non-reported state, which can block parent waits for that subtask.
Useful? React with 👍 / 👎.
Summary
Skip the "call agent_report" reminder in
TaskService.handleStreamEnd()when a follow-up message is about to be sent — either from compaction with a pending follow-up, or from a queued user message.Background
When a sub-agent task's stream ends without calling
agent_report,TaskServicesends a reminder forcing the agent to report. However, there are two legitimate cases where the stream ends withoutagent_reportand a follow-up is about to be dispatched automatically:agentSession.dispatchPendingFollowUp()re-sends the original user message. The reminder interferes with this.agentSession.sendQueuedMessages()dispatches it. The reminder races with this.In both cases, the agent_report reminder is incorrect — the conversation will continue naturally.
Implementation
AgentSession.hasQueuedMessages()— new public method exposing whether the message queue is non-empty.WorkspaceService.hasQueuedMessages(workspaceId)— delegates to the session (usessessions.get()to avoid side effects fromgetOrCreateSession).TaskService.isCompactionStreamWithFollowUp()— reads history and checks if the last user message is a compaction request withfollowUpContent(reusesgetCompactionFollowUpContentfrommessage.ts, same pattern asCompactionHandler).handleStreamEnd()— inserted before the reminder logic, returning early if either condition is true.Note: compactions without follow-ups (e.g. idle compaction) still trigger the reminder as expected.
Risks
Low. The guards add two early-return paths before existing reminder logic. False positives (incorrectly skipping the reminder) would only occur if the queue or compaction state is stale, but both are checked synchronously/within the same event loop tick as the stream-end handler.
Generated with
mux• Model:anthropic:claude-opus-4-6• Thinking:medium• Cost:$11.05