Ask separately for confirmation

[DOsinga]

Summary

Fixes: #2371

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[zanesq]

Ran it locally it works

Couple of things goose picked up that seem worth looking into

---

Missing deny_once handling - The Permission enum includes deny_once but the UI only offers "Deny" which maps to always_deny. Is this intentional? The old code had DENY = 'deny' which is different from always_deny.

isStreaming prop change - Changed from isStreaming?: boolean with default false to required isStreaming: boolean. Need to verify all call sites pass this prop.

findConfirmationForToolAcrossMessages defined inside render - This function is recreated on every render. Could be memoized or moved outside the component.

cancel permission - The enum includes cancel but it's not used in the UI. Is this for programmatic cancellation only?

No error handling UI - handleAction in ToolApprovalButtons calls the API but doesn't show errors to users if the call fails.

The PR removes the "Change" button that let users modify permissions after approval. Was this intentional? Users can no longer change their decision after the fact from this UI.

[DOsinga]

Missing deny_once handling - The Permission enum includes deny_once but the UI only offers "Deny" which maps to always_deny. Is this intentional? The old code had DENY = 'deny' which is different from always_deny.

that's not true. we have allow, allow once & deny (which is deny once)

isStreaming prop change - Changed from isStreaming?: boolean with default false to required isStreaming: boolean. Need to verify all call sites pass this prop.

the compiler would have picked up on that.

findConfirmationForToolAcrossMessages defined inside render - This function is recreated on every render. Could be memoized or moved outside the component.

why does it matter that it gets recreated every time

cancel permission - The enum includes cancel but it's not used in the UI. Is this for programmatic cancellation only?

correct, but there's no change here. the enum always had cancel in there, before we were using custom strings with
all the risks involved, now we use the enum in typescript too.

No error handling UI - handleAction in ToolApprovalButtons calls the API but doesn't show errors to users if the call fails.

same as before.

The PR removes the "Change" button that let users modify permissions after approval. Was this intentional? Users can no longer change their decision after the fact from this UI.

the Change button never allowed the user to modify their permission. it was a short cut to settings. just like this confused you, I think it confuses all users, so yeah removed

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.