fix(ui): send approvals past busy chat queue#101532
Conversation
Co-authored-by: Vincent Koc <[email protected]>
|
Land-ready proof refreshed for exact head
The replay remains intentionally narrow: only busy Control UI |
…bchat-approve # Conflicts: # CHANGELOG.md
|
Codex review: stale review; fresh review needed. Summary Next step |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 712b6f44bb
ℹ️ 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 backend resolves /approve before active-run admission. Send it now so | ||
| // the approval command cannot queue behind the run that is waiting for it. | ||
| const shouldSendDetachedCommand = | ||
| isBtwCommand(message) || (parsed?.command.key === "approve" && isChatBusy(host)); |
There was a problem hiding this comment.
Keep detached approvals out of the transcript
When an operator types /approve ... while a run is busy, this branch sends it as a detached chat.send, but the Gateway still treats that text as a normal main/text-slash turn (turnKind only special-cases BTW in src/gateway/server-methods/chat.ts:3673-3675). The non-agent command branch then persists the user turn and appends an assistant transcript message (src/gateway/server-methods/chat.ts:4852-5260), unlike the BTW side-result path, so approving an exec during an active run pollutes the same session history/context with the approval command and success notice while the original run is still in progress. Please route these approvals through a non-transcript backend path or mark them as side/non-persistent before detaching them.
Useful? React with 👍 / 👎.
|
|
||
| ### Fixes | ||
|
|
||
| - **Control UI typed approvals:** send `/approve` commands immediately through the authorized Gateway command path while an agent run is blocked instead of queueing the command behind that run. (#77672) Thanks @vincentkoc. |
There was a problem hiding this comment.
Remove the normal-fix changelog entry
This adds an Unreleased entry for a normal fix, but the repo’s root AGENTS.md says CHANGELOG.md is release-only and that normal fixes should keep release-note context in the PR body, squash message, or direct commit. Leaving this line in a regular fix makes the release generator inherit prewritten content that can be duplicated or contradicted during release-note generation; please drop the changelog edit from this commit.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
Co-authored-by: Vincent Koc <[email protected]>
Co-authored-by: Vincent Koc <[email protected]>
What Problem This Solves
When a Control UI chat run is waiting for exec approval, typed
/approve ...commands follow the ordinary busy-chat path and are queued behind the run that cannot finish until that command is handled.This is a narrow current-main replay of the useful WebChat portion of #77672. It does not carry that stale draft's requester-limited approval-list CLI, duplicated reconnect loader, already-landed sessions alias, or unrelated docs.
Why This Change Was Made
The Gateway already resolves authorized
/approvecommands before active-run queue admission. Control UI now sends a recognized/approvecommand through the existing detachedchat.sendpath only while the selected chat is busy. Idle command behavior remains unchanged, the active run keeps ownership, and Gateway scope and command authorization checks remain authoritative.Contributor credit is preserved through the commit co-author and changelog attribution to @vincentkoc.
User Impact
Operators can type an approval decision in Control UI while an exec is waiting without deadlocking the decision behind the blocked run.
Evidence
tbx_01kwxy6jtj8x9fzarrt507zpy9:pnpm test ui/src/pages/chat/chat-send.test.ts— 99 passed.pnpm test:ui:e2e ui/src/e2e/approval-flow.e2e.test.ts— 2 passed, including a browser composer/WebSocket scenario that keeps the first run active and proves the approval becomes an immediate secondchat.sendwith no queue entry or run replacement.pnpm check:changed— core/core-tests/docs lanes passed, including typechecks, lint, changelog/dependency/storage/media/runtime guards, and import-cycle scan.autoreview --mode local— clean, no accepted/actionable findings (overall correctness 0.91).git diff --check— clean.