Skip to content

fix: allow editing tool confirmation payloads - #540

Open
jabrailkhalil wants to merge 2 commits into
google:mainfrom
jabrailkhalil:fix/edit-tool-confirmation-payload
Open

jabrailkhalil wants to merge 2 commits into
google:mainfrom
jabrailkhalil:fix/edit-tool-confirmation-payload

Conversation

@jabrailkhalil

@jabrailkhalil jabrailkhalil commented Oct 4, 2026 •

Copy link
Copy Markdown

Advanced tool confirmation currently displays a read-only payload, so users cannot enter the structured response requested by toolConfirmation.payload. For example, the time-off flow keeps submitting approved_days: 0 because there is no input for changing it.

I added a labeled JSON editor for supplied confirmation payloads. Invalid or empty JSON stays visible with an accessible error and blocks both Approve and Reject, including the submit handler itself; correcting it enables submission again. The response preserves the edited JSON and the selected confirmation value, including confirmed: false, scalar/array values and an explicitly entered JSON null. Simple confirmations with an absent/null supplied payload retain the existing original-arguments fallback.

The October 10 update integrates main dfac466d959a8f486b5683e8d27860f1a89cd2c1. Main moved confirmation into the dedicated ToolConfirmation card, so I ported the editor and regression coverage there instead of restoring the removed long-running-response UI. The old component now matches main; the PR is limited to four confirmation-component files. The upstream shell/file-edit displays and Approve/Reject layout remain intact.

Fixes #463.

Validation on 18a5aac0d0c53a52b874f711ee2630b8aa509593 (Windows, Chrome Headless 154):

  • Added 21 DOM regression cases for editing, both decisions, invalid input and recovery, simple confirmation, scalar/array/null values and input reset on a different request. Against unmodified current-main production code, 17 fail and the four simple-confirmation cases pass.
  • Full ChromeHeadlessCI suite after the fix: 846 passed, 37 skipped, 0 failed. No timeouts or skip conditions were changed. The existing read-only-content test now checks the complete JSON value in the editor.
  • Application and test TypeScript checks passed; production build passed with existing dependency optimization warnings.
  • Authored diff whitespace check passed. The merge imports existing main whitespace separately; no unrelated formatting cleanup is included.

This is browser-component validation, not a live ADK/backend round trip. Running only the component spec hits an existing CustomJsonViewer/JsonTooltip initialization cycle before tests start; the full suite above executes the component cases successfully. Upstream CI/review on the new head is still required.

@jabrailkhalil

Copy link
Copy Markdown
Author

Updated to 18a5aac and resolved the conflict with current main. The confirmation UI moved to its own component upstream, so I ported the payload editor there and kept the new shell/file-edit rendering and Approve/Reject layout; the removed long-running-response form is not brought back.

The 21 regression cases reproduce the missing editor on main (17 fail before the fix; four simple-confirmation checks already pass). After the fix, the full browser suite passes: 846 passed / 37 skipped / 0 failed. App/test TypeScript checks and the production build also pass. Invalid or empty JSON cannot submit either decision, and entered JSON null stays null rather than falling back to the original arguments.

The merge-conflict demon is out of the way; the structured payload now actually makes the trip out of the form. Could you review the new head and approve upstream CI if required? The description has the exact validation scope and remaining live-backend limitation.

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.

Advanced Tool Confirmation UI cannot collect editable payload input

1 participant