📬 fix: Show Background Results Still Waiting for the Agent - #16363
danny-avila wants to merge 1 commit into
Conversation
The chat header listed a background task as completed as soon as its tool finished, while its result could still be queued behind a busy turn and arrive minutes later as a new agent turn. The header also read only the process-local registry, so tasks on another replica were invisible. The task index now reads the durable delivery store the agent's check_background_task already uses: each finished task carries delivery pending, delivered or failed, and finished results known only to the store are listed too. The header marks results still on their way, keeps them ahead of delivered ones, and polls them every 10 s.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb7243f874
ℹ️ 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".
| (row) => | ||
| isActive(row) || row.settledAt == null || now - row.settledAt <= RECENT_SUBAGENT_WINDOW_MS, | ||
| isActive(row) || | ||
| isAwaitingDelivery(row) || |
There was a problem hiding this comment.
When a result remains pending for more than the one-hour retention window and its wake-up then dead-letters, this predicate immediately removes the row: isAwaitingDelivery only recognizes pending, while settledAt is the original tool settlement time rather than the later delivery-failure time. Long-running parent turns can defer delivery indefinitely without consuming attempts, so users in this scenario never see the newly added “Result not delivered” state. Retain failed deliveries long enough to expose the failure, or base their expiry on the delivery transition time.
AGENTS.md reference: AGENTS.md:L42-L45
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
| const tasks = deps.registry.list(userId, conversationId); | ||
| const durable = await readDurable(deps, { userId, conversationId }, tasks); |
There was a problem hiding this comment.
This adds a MongoDB read to every background-task request, including the quiet 60-second header poll, but listPendingAgentBackgroundToolCompletions filters by user, nested conversation/source fields, capability, and status and sorts by createdAt; triggerDeliverySchema has no compound index matching that lookup (the closest is { user, status, availableAt }). For users who accumulate persistent dead letters across many conversations, every open conversation repeatedly scans and sorts that user's unrelated delivery rows. Add an index matching the conversation-scoped query or avoid polling this durable lookup when it is unnecessary.
AGENTS.md reference: AGENTS.md:L51-L54
Useful? React with 👍 / 👎.
| tasks: deps.registry.list(userId, conversationId).map(toBackgroundTaskSummary), | ||
| tasks: [ | ||
| ...tasks.map((task) => toBackgroundTaskSummary(task, resolveTaskDelivery(task, durable))), | ||
| ...(durable?.pending ?? []) |
There was a problem hiding this comment.
When a conversation has more than 200 durable rows, this route consumes only the first page and silently omits remote completions beyond it. The underlying lookup sorts oldest-first and applies a shared 200-row limit before separating pending and dead entries, while dead letters intentionally never expire; after a restart or across replicas, 200 old dead rows can therefore occupy the page permanently and prevent every newer remote result from appearing in the header. Handle the incomplete result by paging, or query the visible categories separately so persistent dead rows cannot starve newer pending work.
AGENTS.md reference: AGENTS.md:L42-L45
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
The chat header's background task list marks a task Completed as soon as its tool finishes. The agent may not have its result yet. A result that finishes while its conversation is busy waits for the running turn to end, then arrives as its own new agent turn. So the header can read "everything completed" while results keep arriving for minutes.
On the demo, one long turn started 12 background checks it never polled. They finished within seconds to about a minute, and the header showed them as completed. Their results then arrived 18 to 35 minutes later, as 12 separate wake-up turns in 7 minutes, mixed in with the user's own messages. Each result was delivered exactly once, so nothing was duplicated or lost. The header just had no way to say "finished, but not yet with the agent".
The header also read only the process-local task registry. With more than one replica, it never showed tasks owned by another process.
This PR makes the task index read the durable delivery store that the agent's
check_background_taskhas used since #16343:delivery:pending(the result will still arrive as a new turn),delivered, orfailed(automatic delivery dead-lettered).check_background_task(readDurableCompletionsandresolveTaskDelivery), so the header and the agent always agree on delivery state.In the header:
Follow-ups for
canary, not in this PR: #16352 coalesces a conversation's ready results into one wake-up turn, and a busy turn could receive finished results at its next tool step instead of after it ends.Type of change
Testing
The motivating case is from the demo (
devat7b2362d): background task deliveries and Langfuse tool calls for one conversation, matched by task id. That confirmed the late results were never polled in-turn and were each delivered once.Automated tests:
tasks.spec.ts, new cases for the index route:background.spec.tsandbackgroundCompletionWakeup.spec.ts: existingcheck_background_taskdelivery cases, unchanged behavior on the shared helper. Fixtures now carrytoolCallId.rows.test.ts: ordering and retention of pending rows.queries.polling.test.tsx: poll intervals.convos.spec.js,convos-duplicate-ratelimit.spec.js: the route module with the new dependency mocked.npx tsc --noEmitinpackages/apiandclient.Screenshots / recordings
The finished task row gains a "Result pending" label next to "Completed".
Risk / compatibility
BackgroundTaskSummary.deliveryis optional and additive.PendingBackgroundCompletionnow carriestoolCallId, which the data layer already returned.Checklist