Skip to content

fix(chat): deliver background job notifications at step boundaries - #1932

Open
zhanba wants to merge 2 commits into
mainfrom
feat/background-job-completion-notifications-20260901-203907
Open

fix(chat): deliver background job notifications at step boundaries#1932
zhanba wants to merge 2 commits into
mainfrom
feat/background-job-completion-notifications-20260901-203907

Conversation

@zhanba

@zhanba zhanba commented Sep 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Background job completion notifications (from executeCommand with background: true) were only handed to the model once the whole turn had ended. Mid-loop they sat in the queue, so they arrived late and often after the model had already moved past the point where they were actionable.
  • Deliver them at the agent loop step boundary instead, riding along with the continuation request that is issued anyway — no extra request, latency is now at most one step.
  • packages/vscode-webui: the auto-dequeue effect in chat-toolbar.tsx gated delivery on taskStatus being idle. Delivery is now also attempted from the auto-continuation decision chain (page.tsx -> continueAutomatically), so it inherits every existing safety gate: an intentional pause (autoApproveGuard === "stop", paused todo loop) is never resumed by a notification, and sendQueuedMessage(..., { keepAutoApproveGuard: true }) makes sure a one-shot manual approval is not upgraded to auto. Only a background-job notification sitting at the queue head is eligible; user-typed queued messages keep their previous behaviour. Dedup is keyed on message identity (deliveredNotificationsRef) rather than an in-flight boolean, so an aborted send cannot stall the queue.
  • packages/cli: TaskRunner had the same gate — it only drained pendingBackgroundJobNotifications on a "finished" step. The drain now also happens right before the continuation chat.sendMessage() when the step result is "next". The decision lives in a new pure helper, src/lib/background-job-notification-delivery.ts, so it is unit-testable without the full TaskRunner harness. "retry" steps never inject (the retry must re-send the same state), and the end-of-task waitForAsyncWork() drain is untouched.

Not in scope: packages/cli/src/running-task-adaptor.ts creates per-task BackgroundJobManagers but never subscribes to onDidFinish, so that path surfaces no background job notifications at all. Pre-existing, worth a follow-up.

Test plan

  • cd packages/cli && bun run test — 16 files / 132 tests pass, including 6 new tests for the delivery helper (drains all pending events into one message; "next" delivers and empties the queue; "retry" / "finished" / empty queue do not).
  • cd packages/vscode-webui && bun run test — 77 files / 532 tests pass, including new coverage for the queue helpers, sendQueuedMessage (guard preservation, single-entry removal) and the toolbar delivery callback (head notification delivers once; user message ahead blocks it).
  • bun tsc clean in both packages; bun fix / bun check clean at the repo root.
  • Manual: start a 25s background job, then run a sequence of short tool calls — the notification now arrives mid-loop instead of after attemptCompletion; two jobs finishing near the same boundary are batched into one notification message; a job started immediately before attemptCompletion is still awaited and delivered by the existing end-of-task drain.

Note: the local pre-push hook could not run packages/vscode's extension tests in this worktree (IPC handle ... is longer than 103 chars — the checkout path exceeds the macOS unix socket limit). Every other hook step passed; this change does not touch packages/vscode.

🤖 Generated with Pochi | Task

Background job completion notifications waited for the whole turn to end
before reaching the model, so they arrived late and often after the model
had already moved past the point where they mattered. Deliver them at the
agent loop step boundary instead, riding along with the continuation
request that is sent anyway, in both the webui and the CLI runner.

🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-98432c863b7443779e3b27f5d90db811)

Co-Authored-By: Pochi <noreply@getpochi.com>
@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.07407% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.38%. Comparing base (ab3dc4d) to head (1db7386).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/vscode-webui/src/features/chat/page.tsx 0.00% 19 Missing ⚠️
packages/cli/src/task-runner.ts 0.00% 9 Missing ⚠️

❌ Your patch status has failed because the patch coverage (74.07%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1932      +/-   ##
==========================================
+ Coverage   33.32%   33.38%   +0.06%     
==========================================
  Files         778      779       +1     
  Lines       83120    83210      +90     
  Branches     4897     4757     -140     
==========================================
+ Hits        27696    27780      +84     
- Misses      55304    55310       +6     
  Partials      120      120              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants