Skip to content

Commit f41bc79

Browse files
vaibhav-patelxuanyang15
authored andcommitted
ADK changes
Co-authored-by: Xuan Yang <xygoogle@google.com> COPYBARA_INTEGRATE_REVIEW=google#6187 from vaibhav-patel:fix/5871-a2a-hitl-sample 028cee9 PiperOrigin-RevId: 945223172
1 parent 3c0fb65 commit f41bc79

9 files changed

Lines changed: 181 additions & 15 deletions

File tree

.github/workflows/pr-triage.yml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,14 @@
1515
name: ADK Pull Request Triaging Agent
1616

1717
on:
18+
# React within seconds of a PR opening/updating so the owner is assigned
19+
# promptly. pull_request_target (not pull_request) is required so the run has
20+
# the base-repo token needed to assign on community fork PRs; this workflow
21+
# only reads PR metadata via the API and never checks out untrusted PR code.
22+
pull_request_target:
23+
types: [opened, reopened, synchronize, ready_for_review]
1824
schedule:
19-
# Run every 6 hours
25+
# Backfill every 6 hours in case an event was missed.
2026
- cron: '0 */6 * * *'
2127
workflow_dispatch:
2228
inputs:
@@ -30,6 +36,12 @@ on:
3036
default: '10'
3137
type: 'string'
3238

39+
# Never let two runs triage the same PR at once (e.g. rapid pushes); the
40+
# scheduled backfill still runs under its own group.
41+
concurrency:
42+
group: pr-triage-${{ github.event.pull_request.number || github.run_id }}
43+
cancel-in-progress: false
44+
3345
jobs:
3446
agent-triage-pull-request:
3547
if: github.repository == 'google/adk-python'

contributing/samples/a2a/a2a_human_in_loop/README.md

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,14 @@ The A2A Human-in-the-Loop sample consists of:
5454

5555
```bash
5656
# Start the remote a2a server that serves the human-in-the-loop approval agent on port 8001
57-
adk api_server --a2a --port 8001 contributing/samples/a2a_human_in_loop/remote_a2a
57+
adk api_server --a2a --port 8001 contributing/samples/a2a/a2a_human_in_loop/remote_a2a
5858
```
5959

6060
1. **Run the Main Agent**:
6161

6262
```bash
6363
# In a separate terminal, run the adk web server
64-
adk web contributing/samples/
64+
adk web contributing/samples/a2a
6565
```
6666

6767
### Example Interactions
@@ -82,10 +82,24 @@ Agent: ✅ Reimbursement approved and processed: $50 for meals
8282
User: Please reimburse $200 for conference travel
8383
Agent: I'll process your reimbursement request for $200 for conference travel. Since this amount exceeds $100, I need to get manager approval.
8484
Agent: 🔄 Request submitted for approval (Ticket: reimbursement-ticket-001). Please wait for manager review.
85-
[Human manager interacts with root agent to approve the request]
85+
[Human manager approves the pending request from the ADK Web UI]
8686
Agent: ✅ Great news! Your reimbursement has been approved by the manager. Processing $200 for conference travel.
8787
```
8888

89+
> **Approving from the ADK Web UI:** The approval is a *long-running tool* call
90+
> that runs on the remote approval agent. The pending call is surfaced in the
91+
> Web UI as a function call awaiting a response. To approve (or reject), hover
92+
> over the pending `ask_for_approval` function response in the UI and use
93+
> **"Send another response"** to send back an updated response such as
94+
> `{"status": "approved", "ticketId": "reimbursement-ticket-001"}`. Simply
95+
> typing "I approve" as a chat message will **not** resume the pending request,
96+
> because the framework needs a `FunctionResponse` that carries the same call
97+
> `id` to resume the long-running tool.
98+
>
99+
> For this resume to be routed back to the remote approval agent (rather than
100+
> restarting at the root agent), the sample is exposed as an `App` with
101+
> `ResumabilityConfig(is_resumable=True)` in `agent.py`.
102+
89103
## Code Structure
90104

91105
### Main Agent (`agent.py`)

contributing/samples/a2a/a2a_human_in_loop/agent.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
from google.adk.agents.llm_agent import Agent
1717
from google.adk.agents.remote_a2a_agent import AGENT_CARD_WELL_KNOWN_PATH
1818
from google.adk.agents.remote_a2a_agent import RemoteA2aAgent
19+
from google.adk.apps import App
20+
from google.adk.apps import ResumabilityConfig
1921
from google.genai import types
2022

2123

@@ -49,3 +51,18 @@ def reimburse(purpose: str, amount: float) -> str:
4951
sub_agents=[approval_agent],
5052
generate_content_config=types.GenerateContentConfig(temperature=0.1),
5153
)
54+
55+
# The human-in-the-loop approval runs as a long-running tool on the remote
56+
# approval_agent. When the manager approves (or rejects) the request, the ADK
57+
# Web UI sends back a FunctionResponse for that pending long-running call. For
58+
# the next turn to be routed back to the (remote) approval_agent so it can
59+
# resume the paused tool instead of restarting at the root reimbursement_agent,
60+
# the app must be resumable. Without this, the confirmation is delivered to the
61+
# root agent, which has no pending call, and nothing happens (see issue #5871).
62+
app = App(
63+
name='a2a_human_in_loop',
64+
root_agent=root_agent,
65+
resumability_config=ResumabilityConfig(
66+
is_resumable=True,
67+
),
68+
)

contributing/samples/adk_team/adk_pr_triaging_agent/agent.py

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from adk_pr_triaging_agent.utils import error_response
2323
from adk_pr_triaging_agent.utils import get_diff
2424
from adk_pr_triaging_agent.utils import get_request
25+
from adk_pr_triaging_agent.utils import is_assignable
2526
from adk_pr_triaging_agent.utils import post_request
2627
from adk_pr_triaging_agent.utils import read_file
2728
from adk_pr_triaging_agent.utils import run_graphql_query
@@ -41,17 +42,34 @@
4142
"web",
4243
]
4344

45+
# Component label -> GitHub login of the owner who shepherds that component.
46+
# The owner becomes the PR's assignee so the contributor can see who is
47+
# handling their PR. github login != corp ldap, so this is the login form. Keep
48+
# in sync with the OWNERS file (the authority) and adk_triaging_agent's map.
49+
LABEL_TO_OWNER = {
50+
"documentation": "joefernandez",
51+
"services": "DeanChensj",
52+
"tools": "xuanyang15",
53+
"mcp": "wukath",
54+
"eval": "ankursharmas",
55+
"live": "wuliang229",
56+
"models": "xuanyang15",
57+
"tracing": "jawoszek",
58+
"core": "DeanChensj",
59+
"web": "wyf7107",
60+
}
61+
4462
CONTRIBUTING_MD = read_file(
4563
Path(__file__).resolve().parents[4] / "CONTRIBUTING.md"
4664
)
4765

4866
APPROVAL_INSTRUCTION = (
49-
"Do not ask for user approval for labeling or commenting! If you can't find"
50-
" appropriate labels for the PR, do not label it."
67+
"Do not ask for user approval for labeling, commenting, or assigning!"
68+
" If you can't find appropriate labels for the PR, do not label it."
5169
)
5270
if IS_INTERACTIVE:
5371
APPROVAL_INSTRUCTION = (
54-
"Only label or comment when the user approves the labeling or commenting!"
72+
"Only label, comment, or assign when the user approves the action!"
5573
)
5674

5775

@@ -82,6 +100,11 @@ def get_pull_request_details(pr_number: int) -> str:
82100
name
83101
}
84102
}
103+
assignees(first: 10) {
104+
nodes {
105+
login
106+
}
107+
}
85108
files(last: 50) {
86109
nodes {
87110
path
@@ -194,6 +217,48 @@ def add_label_to_pr(pr_number: int, label: str) -> dict[str, Any]:
194217
}
195218

196219

220+
def assign_owner_to_pr(pr_number: int, label: str) -> dict[str, Any]:
221+
"""Assign the component owner (the shepherd) to a PR based on its label.
222+
223+
The owner is looked up from `LABEL_TO_OWNER` so the contributor can see who is
224+
shepherding their PR. GitHub only allows assigning users with
225+
repo write/triage access, so a non-assignable owner is reported as skipped
226+
rather than silently dropped.
227+
228+
Args:
229+
pr_number: the number of the GitHub pull request
230+
label: the component label the PR was triaged into
231+
232+
Returns:
233+
The status of this request, with the assigned owner when successful.
234+
"""
235+
owner = LABEL_TO_OWNER.get(label)
236+
if not owner:
237+
return error_response(f"Error: no owner mapped for label '{label}'.")
238+
print(f"Attempting to assign owner '{owner}' to PR #{pr_number}")
239+
if not is_assignable(owner):
240+
return {
241+
"status": "skipped",
242+
"reason": f"'{owner}' is not assignable (needs repo access)",
243+
"owner": owner,
244+
}
245+
246+
# Pull Request is a special issue in GitHub, so we can use the issue url.
247+
assignee_url = (
248+
f"{GITHUB_BASE_URL}/repos/{OWNER}/{REPO}/issues/{pr_number}/assignees"
249+
)
250+
try:
251+
response = post_request(assignee_url, {"assignees": [owner]})
252+
except requests.exceptions.RequestException as e:
253+
return error_response(f"Error: {e}")
254+
255+
return {
256+
"status": "success",
257+
"assigned_owner": owner,
258+
"response": response,
259+
}
260+
261+
197262
def add_comment_to_pr(pr_number: int, comment: str) -> dict[str, Any]:
198263
"""Add the specified comment to the given PR number.
199264
@@ -282,6 +347,7 @@ def list_untriaged_pull_requests(pr_count: int) -> dict[str, Any]:
282347
Your core responsibility includes:
283348
- Get the pull request details.
284349
- Add a label to the pull request.
350+
- Assign the component owner (the shepherd) to the pull request.
285351
- Check if the pull request is following the contribution guidelines.
286352
- Add a comment to the pull request if it's not following the guidelines.
287353
@@ -337,17 +403,23 @@ def list_untriaged_pull_requests(pr_count: int) -> dict[str, Any]:
337403
- Check if the PR is following the contribution guidelines.
338404
- If it's not following the guidelines, recommend or add a comment to the PR that points to the contribution guidelines (https://github.com/google/adk-python/blob/main/CONTRIBUTING.md).
339405
- If it's following the guidelines, recommend or add a label to the PR.
406+
- After you add a component label, assign the component owner (the shepherd) to the PR:
407+
- Call `assign_owner_to_pr` with the same label you applied.
408+
- Skip assignment if the PR already has an assignee.
409+
- If the tool reports the owner is not assignable, just note it; do not comment about it.
340410
341411
# 5. Output
342412
Present the following in an easy to read format highlighting PR number and your label.
343413
- The PR summary in a few sentence
344414
- The label you recommended or added with the justification
415+
- The owner you assigned (or why you did not)
345416
- The comment you recommended or added to the PR with the justification
346417
""",
347418
tools=[
348419
list_untriaged_pull_requests,
349420
get_pull_request_details,
350421
add_label_to_pr,
422+
assign_owner_to_pr,
351423
add_comment_to_pr,
352424
],
353425
)

contributing/samples/adk_team/adk_pr_triaging_agent/utils.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@
1515
import sys
1616
from typing import Any
1717

18+
from adk_pr_triaging_agent.settings import GITHUB_BASE_URL
1819
from adk_pr_triaging_agent.settings import GITHUB_GRAPHQL_URL
1920
from adk_pr_triaging_agent.settings import GITHUB_TOKEN
21+
from adk_pr_triaging_agent.settings import OWNER
22+
from adk_pr_triaging_agent.settings import REPO
2023
from google.adk.agents.run_config import RunConfig
2124
from google.adk.runners import Runner
2225
from google.genai import types
@@ -66,6 +69,16 @@ def post_request(url: str, payload: Any) -> dict[str, Any]:
6669
return response.json()
6770

6871

72+
def is_assignable(login: str) -> bool:
73+
"""Whether a GitHub user can be assigned to an issue/PR in this repo."""
74+
# GitHub only allows assignees with repo write/triage access and silently
75+
# drops others from an assignee POST; check first so callers can report the
76+
# skip instead of a silent no-op.
77+
url = f"{GITHUB_BASE_URL}/repos/{OWNER}/{REPO}/assignees/{login}"
78+
response = requests.get(url, headers=headers, timeout=60)
79+
return response.status_code == 204
80+
81+
6982
def error_response(error_message: str) -> dict[str, Any]:
7083
"""Returns an error response."""
7184
return {"status": "error", "error_message": error_message}

src/google/adk/environment/_local_environment.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,16 @@ async def write_file(self, path: str | Path, content: str | bytes) -> None:
142142
return await asyncio.to_thread(self._sync_write, resolved, content)
143143

144144
def _resolve_path(self, path: str | Path) -> Path:
145-
"""Resolve a relative path against the working directory."""
146-
path_obj = Path(path)
147-
if path_obj.is_absolute():
148-
return path_obj
149-
return self.working_dir / path_obj
145+
"""Resolve a file path inside the working directory."""
146+
candidate = Path(path)
147+
working_dir = self.working_dir.resolve()
148+
if not candidate.is_absolute():
149+
candidate = working_dir / candidate
150+
151+
resolved = candidate.resolve()
152+
if not resolved.is_relative_to(working_dir):
153+
raise ValueError(f'Path escapes working directory: {path}')
154+
return resolved
150155

151156
@staticmethod
152157
def _sync_read(path: Path) -> bytes:

src/google/adk/tools/transfer_to_agent_tool.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
# function directly. TransferToAgentTool provides additional enum constraints
2929
# that prevent LLMs from hallucinating invalid agent names.
3030
def transfer_to_agent(agent_name: str, tool_context: ToolContext) -> None:
31-
"""Transfer the question to another agent.
31+
"""Transfer the query to another agent.
3232
3333
Use this tool to hand off control to another agent that is more suitable to
34-
answer the user's question according to the agent's description.
34+
answer the user's query according to the agent's description.
3535
3636
Args:
3737
agent_name: the agent name to transfer to.

tests/unittests/tools/test_local_environment.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,39 @@ async def test_write_creates_parent_dirs(self, env: LocalEnvironment):
7676
data = await env.read_file("sub/dir/file.txt")
7777
assert data == b"nested"
7878

79+
@pytest.mark.asyncio
80+
async def test_absolute_path_inside_working_dir(self, env: LocalEnvironment):
81+
"""Absolute paths are accepted when they stay inside the workspace."""
82+
path = env.working_dir / "absolute.txt"
83+
await env.write_file(path, "absolute")
84+
data = await env.read_file(path)
85+
assert data == b"absolute"
86+
87+
@pytest.mark.asyncio
88+
async def test_rejects_relative_path_escape(self, env: LocalEnvironment):
89+
"""Parent traversal cannot escape the workspace."""
90+
outside = env.working_dir.parent / "outside.txt"
91+
outside.write_text("secret", encoding="utf-8")
92+
93+
with pytest.raises(ValueError, match="escapes working directory"):
94+
await env.read_file(Path("..") / outside.name)
95+
96+
with pytest.raises(ValueError, match="escapes working directory"):
97+
await env.write_file(Path("..") / "write-outside.txt", "nope")
98+
99+
assert not (env.working_dir.parent / "write-outside.txt").exists()
100+
101+
@pytest.mark.asyncio
102+
async def test_rejects_absolute_path_outside_working_dir(
103+
self, env: LocalEnvironment
104+
):
105+
"""Absolute paths outside the workspace are rejected."""
106+
outside = env.working_dir.parent / "outside-absolute.txt"
107+
outside.write_text("secret", encoding="utf-8")
108+
109+
with pytest.raises(ValueError, match="escapes working directory"):
110+
await env.read_file(outside)
111+
79112
@pytest.mark.asyncio
80113
async def test_read_nonexistent_raises(self, env: LocalEnvironment):
81114
"""Reading a missing file raises FileNotFoundError."""

tests/unittests/tools/test_transfer_to_agent_tool.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ def test_transfer_to_agent_tool_preserves_description():
120120

121121
assert decl is not None
122122
assert decl.description is not None
123-
assert 'Transfer the question to another agent' in decl.description
123+
assert 'Transfer the query to another agent' in decl.description
124124

125125

126126
def test_transfer_to_agent_tool_maintains_inheritance():

0 commit comments

Comments
 (0)