fix(samples): make the a2a_human_in_loop approval threshold config-derived, not caller-bypassable - #6613
Open
Kapustin2000 wants to merge 1 commit into
Open
Conversation
…rived, not caller-bypassable contributing/samples/a2a/a2a_human_in_loop/ demonstrates human-in-the-loop approval for large reimbursements, but the "$100 auto-approve, otherwise ask a human" rule lived only in the agent's instruction text. Nothing in code stopped a direct call to reimburse() -- from a model that ignores its own instructions, or a caller crafting tool-call arguments -- from reimbursing any amount with no approval at all. Adds approval_config.py (one copy per agent directory, matching this sample's existing pattern of duplicating reimburse() itself): a pure module exposing get_approval_threshold_usd() and requires_manager_approval(amount). No tool-call argument can influence this value -- the only input is an environment variable read at call time. reimburse() in both agent.py and remote_a2a/human_in_loop/agent.py now takes a ToolContext and uses ADK's existing request_confirmation() / tool_confirmation.confirmed mechanism (the same one already demonstrated in contributing/samples/hitl/tool_confirmation/) to park/refuse execution for amounts at or above the threshold, rather than relying solely on the prompt. No sample behavior changes for amounts under the threshold. For amounts at/above it, a direct call to reimburse() now returns pending_confirmation/rejected instead of ok until confirmed.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
contributing/samples/a2a/a2a_human_in_loop/demonstrates human-in-the-loop approval for large reimbursements, but the "$100 auto-approve, otherwise ask a human" rule lives only in the agent's instruction text. Nothing in code stops a direct call toreimburse()— from a model that ignores its own instructions, or a caller crafting tool-call arguments — from reimbursing any amount with no approval at all. This is the same shape of bug as a real, previously-observed production incident where a caller-controlled value defeated an approval gate that existed only as a convention rather than as an enforced check.Solution:
Adds a small, additive, config-derived guard, kept idiomatic to this sample and to the rest of this repo's HITL samples:
approval_config.py(new, one copy per agent directory, matching this sample's existing pattern of duplicatingreimburse()itself): a pure module exposingget_approval_threshold_usd()(readsREIMBURSEMENT_APPROVAL_THRESHOLD_USDfrom the environment, default $100) andrequires_manager_approval(amount). No tool-call argument can influence this value.reimburse()in bothagent.pyandremote_a2a/human_in_loop/agent.pynow takes atool_context: ToolContextand uses ADK's existingrequest_confirmation()/tool_confirmation.confirmedmechanism (the same one already demonstrated incontributing/samples/hitl/tool_confirmation/) to park/refuse execution for amounts at or above the threshold, rather than relying solely on the prompt.No sample behavior changes for amounts under the threshold. For amounts at/above it, a direct call to
reimburse()now returnspending_confirmation/rejectedinstead ofokuntil confirmed — closing the gap the sample previously left open.Testing Plan
Unit Tests:
tests/unittests/test_a2a_human_in_loop_approval_guard.py, 13 cases).I additionally verified the new test is not vacuous by temporarily reverting the two
agent.pychanges: all 13 new test cases fail against the pre-fix code (AttributeError: ... has no attribute 'get_approval_threshold_usd'), then pass again once the fix is restored.Manual E2E: Not run for this PR (no network calls, no
adk web/adk api_serverinstance started) — the change is scoped to tool-function logic covered by the unit tests above; happy to add manual E2E notes (approve/reject via the Web UI) if a maintainer wants them before merge.Checklist
(No issue number linked — none exists yet for this specific gap; happy to open one first if a maintainer prefers that over describing the change inline.)