feat(web): composer stays usable during approvals, and sending declines and redirects#21503

🟠 High threads/ThreadDetailScreen.tsx:906

Saving a queued-message edit declines the pending approval even though the edit sends no redirect, so the agent’s active command is rejected just to update a queued message. onSendMessage handles the edit and returns null only after this block runs; skip the decline when props.queuedRunEdit is non-null.

Fixed in fb6bd388c3. Saving a queued-message edit no longer declines the approval: web skips the decline while isEditingQueuedMessage, and mobile skips it while queuedRunEdit is set.

🟠 High chat/ChatComposer.tsx:4005

A pasted attachment can be omitted from the approval follow-up send and then appear in the next message: pendingDraftWork is checked before declinePendingApproval is awaited, but not after it, so an import started during a slow decline is not included in the send snapshot. Recheck pendingDraftWork after the await and abort the send if work is still pending.

No longer relevant as of fb6bd388c3357b120591156f22fba919b4b82c32

Fixed in fb6bd388c3. After the decline resolves, the send re-checks image compressions and pendingDraftWork against attachmentTargetKeyRef.current, and stops with the same toast if anything is still pending. The draft stays, so the next send goes through normally.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the try/finally compiler bailout.

The web React Compiler coverage test fails on this new try/finally block. Reset approvalSubmissionInFlightRef with control flow the compiler can analyze, including the rejection path, so the test passes without leaving the guard set. The applicable rule says, “Every file under apps/web/src must compile,” and specifically identifies try with finally as a missed bailout.

Sources: Coding guidelines, Path instructions, Pipeline failures

✅ Addressed in commits fb6bd38 to 4869512

Already fixed in fb6bd388c3: the try/finally is gone. The decline now runs with .catch(() => false) and the in-flight ref resets right after the await. reactCompilerCoverage.test.ts passes on the current head (1027 tests in that run).

@maria-rcks Thanks for the fix. The supplied ChatComposer.tsx confirms that .catch(() => false) handles rejection and approvalSubmissionInFlightRef resets immediately after the await. This addresses the try/finally finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High chat/ChatComposer.tsx:4003

With a pending approval, submitting /usage-limits declines the approval before onSend opens the usage dialog and returns without sending a redirect, so viewing usage limits unexpectedly rejects the agent's outstanding approval. Handle this local-only command before declining approvals.

Fixed in a25ebec883. The composer now skips the decline for local commands that send nothing to the agent: isUsageLimitsCommand, parseStandaloneComposerSlashCommand (/plan, /default), and parseCodexFeedbackCommand. Mobile skips it for /feedback too, and already handles usage limits before onSendMessage.

🟠 High threads/ThreadDetailScreen.tsx:922

An empty iOS hardware-keyboard submit declines the pending approval without sending a redirect message, so the approval is lost even though the draft was rejected. handleSendMessage calls declinePendingApprovals before props.onSendMessage validates the draft; validate that the message is sendable—including context, upload, and model checks—before declining approvals.

Fixed in 70fe2007b9. The decline now runs inside onSendMessage after it has validated the draft (empty, too much context, unavailable model, queued edit, feedback), so an empty hardware-keyboard submit returns before anything is declined.

🟠 High threads/ThreadDetailScreen.tsx:918

When /feedback ... is sent to a non-Codex provider or with an attachment, this condition skips declinePendingApprovals even though the send handler treats it as a normal agent message. The approval stays pending and the message does not redirect the agent; apply the same provider and attachment checks used by the send handler.

No longer relevant as of a5042ce62412453af1466af43fbc6d526583bc29

Fixed in 70fe2007b9 by moving the decline. Mobile no longer classifies /feedback itself: onSendMessage calls the decline only after its own feedback, queued-edit, empty-draft, and model checks have passed, right before it enqueues the message. A /feedback that goes to the agent as a normal message (non-Codex, or with attachments) now declines like any other redirect.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Compare attachment IDs before dispatching after approval decline.

stillSendable compares only attachment count. The mobile composer keeps attachment removal and picker controls enabled during the approval-decline wait. Removing one attachment and adding another preserves the count, so the changed draft can be enqueued.

✅ Addressed in commit 70fe200

Fixed in 70fe2007b9: stillSendable now also compares attachment ids in order, so swapping one attachment for another during the decline stops the send.

@maria-rcks Thanks for the fix. stillSendable now compares attachment IDs in order, so replacing an attachment during the approval-decline wait stops the send.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Check approvalSubmissionInFlightRef for every submission, not only inside the approval branch.

Only the pending-approval branch reads approvalSubmissionInFlightRef. During the await declinePendingApprovals(...), the server can resolve the approval. The component then re-renders with an empty pendingApprovals list. If the user presses Enter again in that window, the new submitComposer closure skips the approval branch. It then calls submitComposerDraft and sends the message.

When the first call resumes, the draft has already been cleared by that send. The draft-identity check therefore fails, and the user sees the toast "Approval declined. Your message changed, so it wasn't sent." That message is false, because the message was sent. Move the in-flight check above the branch so that a second submit waits for the decline to finish.

✅ Confirmed as addressed by @maria-rcks

Superseded in 70fe2007b9: the decline moved out of the composer into ChatView.onSend, which holds sendInFlightRef for the whole decline. A second submit returns early from that guard, so it can't send behind the first one.

@maria-rcks Confirmed in the current code. ChatView.onSend checks sendInFlightRef.current at Line 5999 and sets it to true at Line 6436, before awaiting declinePendingApprovals at Line 6526.

The guard stays active throughout the decline. A second submission returns early even if pendingApprovals becomes empty. This fixes the reported race without a composer-level guard.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High components/ChatView.tsx:6526

A message send is aborted if an approval in pendingApprovals is resolved during upload or docking, even though no approval remains to decline. This uses the earlier captured list, and declinePendingApprovals treats undefined from onRespondToApprovalStable for the resolved request as failure; re-read the pending requests before declining or treat already-resolved requests as success.

Fixed in 83526b3267. The decline now reads pendingApprovalsRef.current, kept current with a layout effect like currentRouteThreadKeyRef, so it declines the approvals still pending when the message is about to go out. If the only approval was resolved during the upload, the list is empty and the send goes through with no decline.

🟠 High components/ChatView.tsx:6524

A send started in thread A can decline thread B’s pending approvals, even though the send is later aborted without delivering a redirect. After the preceding awaits, this reads the current approvals and onRespondToApprovalStable forwards to the latest handler, so navigation to B redirects B’s commands; bind the approvals and responder to the submitting thread, or reject a changed route before declining.

Fixed in 472671e6bb. approvalsToDecline is pendingApprovalsRef.current only while currentRouteThreadKeyRef.current === routeThreadKey. After navigating to another thread during an upload it is empty, so thread A's send never touches thread B's approvals.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Re-check the full draft after beforeDispatch.

onSendMessage snapshots draft before the asynchronous beforeDispatch callback. If a context import completes during that callback, the current draft can contain new context while the captured draft does not. The existing checks do not detect this completed import. The dispatch can then send the stale context, and clearComposerDraftContent can remove the newly imported context.

Read the current draft after beforeDispatch and abort when its text, context, or attachment IDs differ from the captured draft.

✅ Confirmed as addressed by @maria-rcks

Fixed in 6be924c5a9. After beforeDispatch resolves, onSendMessage re-reads the draft and returns without sending if its text, context, or attachment ids differ from the snapshot it is about to enqueue, in addition to the pending-import check.

@maria-rcks Thanks for the fix. The supplied code re-reads the draft after beforeDispatch and aborts if the text, context, or attachment IDs changed. It also retains the pending-import check. This addresses the stale-draft concern.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High components/ChatView.tsx:6552

Sending a screenshot annotation while approvals are pending is rejected as a changed draft, so the annotation is neither sent nor used to decline the approval. composerAttachmentsSnapshot includes the direct annotation image, but getSendContext() can still omit it because ChatComposer updates its image ref in an effect; the ID comparison therefore reports a mismatch. Compare against the original send-context attachment IDs or normalize the live context to include the direct annotation image.

Fixed in a6a3541504. A direct annotation send writes its image and reference into the draft as it sends, so the live draft trailing the message is expected there. draftChangedSinceSend() now returns false for direct annotation sends. They still decline approvals and still get the new-approval check.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Read the in-flight approval state from a ref.

This check can miss an Approve that starts after the send render. The stable callback does not prevent a second response. Thread commands are serialized per thread, so the queued decline cannot override a successful Approve. It can still dispatch a redundant decline after the Approve settles. If that response fails, declinePendingApprovals returns false, the send aborts, and this path shows no informational toast.

✅ Confirmed as addressed by @maria-rcks

Fixed in 24243e97ce. The send now reads in-flight approvals from a ref kept in sync with respondingRequestIds, so an Approve clicked while uploads finish skips the decline. When that happens the draft stays and a toast says an approval is already being answered.

@maria-rcks Thanks for the fix. The supplied code uses respondingRequestIdsRef to check in-flight approvals. When an approval is already being answered, it skips the decline, preserves the draft, and shows the informational toast.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High state/use-thread-composer-state.ts:649

Changing the model or runtime/interaction mode while beforeDispatch() waits for approval still sends the message with the settings captured before the wait, even though the composer displays the new settings. The unchanged-draft check compares only text, context, and attachments; include the send-affecting settings in that check so the stale snapshot is rejected.

Fixed in e8a63f403d. After the decline, the unchanged-draft check also compares the draft's model selection, runtime mode, and interaction mode, so a change made while approvals are declined keeps the message in the draft.

Confirmed—the unchanged-draft check now covers all three settings from the original concern.

🟠 High chat/ChatComposer.tsx:2833

When useComposerTypingGuard holds an approval, activePendingApproval is null, so followUpBehavior can remain queue. ChatView.onSend still declines the approval but queues the redirect behind the running turn, and alternate-send behavior is also wrong during the hold. Choose the redirect default from pendingApprovals instead.

Fixed in 5cc67ce4c3. The steer default now comes from pendingApprovals, the same list ChatView.onSend declines, so a redirect steers (and the alternate shortcut queues) even while the typing guard holds the approval.

Revised in 8c3461f52e. Declining approvals the typing guard holds meant Enter acted on a request the user hadn't seen, which the guard exists to prevent. Now the decline and the steer default both use only the approvals the composer shows, so they still agree, and held approvals stay pending.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Decline only the approvals that the composer shows.

approvalsToDecline reads pendingApprovalsRef.current. That list comes from usePendingThreadRequests and is not filtered. ChatComposer filters the same approval through the typing guard. If heldRequestIds contains the approval, activePendingApproval is null. The approval card and the "Send a message to decline…" placeholder are then hidden.

Here is the trigger. The user types a message, and an approval arrives during typing. The typing guard holds that approval, and the user presses Enter.

Here is the consequence:

• onSend declines an approval that the user never saw.
• followUpBehavior falls back to settings.followUpBehavior, because isComposerApprovalState is false. With the "queue" setting, the message queues behind the run instead of steering it.

The typing guard exists to stop keystrokes from acting on a request the user has not seen. This path makes Enter reject that request.

Expose the declinable approval IDs from the composer, for example through getSendContext. Filter approvalsToDecline with those IDs. A held approval then keeps the behavior it had before this change.

✅ Confirmed as addressed by @maria-rcks

Fixed in 8c3461f52e. getSendContext now returns shownPendingApprovalIds (pending approvals the typing guard isn't holding), captured before the send awaits anything. onSend declines only those, and the steer default uses the same set. A held approval stays pending, and a send with only held approvals goes out with the user's follow-up setting, as on main.

@maria-rcks Thanks for the fix. The captured shownPendingApprovalIds addresses this finding. Held approvals remain pending, and using the same IDs for the steer default preserves the user's follow-up setting when all approvals are held.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.