feat(a2a-server): Add detached/background task execution mode #15674
Conversation
Foundation for google-gemini#4168 - adds background/detached task execution capability. Changes: - Add isBackground flag to AgentSettings type - Add SpawnWorkerCommand - fire-and-forget background task spawning - Add ListWorkersCommand, GetWorkerCommand, CancelWorkerCommand - Add background_event_bus.ts for minimal event logging - Add comprehensive unit tests Limitations (intentionally out of scope): - No lifecycle manager (auto-start/embedded mode) - No cross-restart task persistence (in-memory only) - No native CLI DeclarativeTool surface (deferred until lifecycle resolved)
Summary of ChangesHello @muff-c, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes the foundational infrastructure within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](/s/com/githubusercontent/avatars/G.https/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a solid foundation for background task execution in the a2a-server with new commands for spawning, listing, getting, and canceling workers, utilizing a fire-and-forget pattern for spawn-worker. A security audit identified two high-severity vulnerabilities: a Prompt Injection vulnerability due to direct user input in an LLM prompt, and a Broken Access Control (IDOR) vulnerability in the get-worker and cancel-worker commands lacking task ownership verification. Furthermore, a high-severity issue was noted regarding the lack of input validation for the timeout parameter, which could lead to unexpected behavior.
| parts: [ | ||
| { | ||
| kind: 'text', | ||
| text: `${taskDescription}\n\nWhen you complete this task, output a clear summary of what you did.`, |
There was a problem hiding this comment.
User-provided input from the --task argument is directly concatenated into the prompt sent to the LLM without any sanitization. An attacker can craft the task description to manipulate the agent's behavior, causing it to ignore its original instructions and execute unintended actions. This could allow an attacker to make the agent perform malicious operations, constrained only by the agent's available tools and permissions.
| } | ||
|
|
||
| const agentExecutor = context.agentExecutor as CoderAgentExecutor; | ||
| const wrapper = agentExecutor.getTask(workerId); |
There was a problem hiding this comment.
The get-worker command uses a user-provided workerId to fetch task details without verifying if the requesting user owns that task. In a multi-user environment, this allows any user to retrieve details of any other user's running tasks if they can obtain the workerId (e.g., via the list-workers command). This constitutes an Insecure Direct Object Reference (IDOR) vulnerability, leading to information disclosure.
| const cancelEventBus = createBackgroundEventBus(workerId); | ||
|
|
||
| logger.info(`[cancel-worker] Cancelling worker ${workerId}`); | ||
| await agentExecutor.cancelTask(workerId, cancelEventBus); |
There was a problem hiding this comment.
The cancel-worker command uses a user-provided workerId to cancel a task without verifying if the requesting user owns that task. In a multi-user environment, this allows any user to cancel any other user's running tasks if they can obtain the workerId. This is an Insecure Direct Object Reference (IDOR) vulnerability that leads to a denial of service.
| const timeoutMinutes = parseInt(parsedArgs.get('timeout') || '30', 10); | ||
|
|
There was a problem hiding this comment.
The timeout argument is parsed using parseInt but there is no validation to check if the result is NaN. If a non-numeric string is provided for timeout (e.g., --timeout abc), parseInt will return NaN. This NaN value propagates to timeoutMs, and setTimeout(..., NaN) behaves like setTimeout(..., 0), causing the worker task to be cancelled almost immediately after it's spawned. This is likely not the intended behavior and can be confusing for the user.
I recommend adding validation to ensure the timeout is a valid number and throwing an error if it's not. This will provide clearer feedback to the user for invalid input.
You should also consider adding a unit test case for this scenario to prevent regressions.
const timeoutMinutesRaw = parsedArgs.get('timeout') || '30';
const timeoutMinutes = parseInt(timeoutMinutesRaw, 10);
if (Number.isNaN(timeoutMinutes)) {
throw new Error(
`Invalid timeout value: "${timeoutMinutesRaw}". Timeout must be a number.`,
);
}- Add timeout NaN validation with clear error message - Add max task length (10k chars) to limit prompt injection surface - Add prompt injection mitigation with delimiters and untrusted warning - Bind server to 127.0.0.1 only (localhost) for single-user access - Add unit tests for timeout validation, task length, and prompt delimiters Addresses Gemini Code Assist review feedback on PR google-gemini#15674
Addressing Gemini Code Assist Review FindingsThank you for the thorough security review! I've pushed a follow-up commit ( ✅ A) Timeout NaN Validation
✅ B) IDOR / Access Control MitigationSince no session/auth infrastructure exists in a2a-server's HTTP context, I implemented Option 2 (localhost-only binding):
✅ C) Prompt Injection Mitigation
Testing: All 105 unit tests pass (3 new tests added for these validations). Could a maintainer please approve the workflow runs so CI/E2E can validate the changes? The checks are currently "awaiting approval" for first-time contributors. |
- Add EventTypes constants for standardized event type names - Add TaskEvent interface with phase/agent/tool attribution fields - Add TaskMetrics and AgentMetrics interfaces for TUI dashboard - Add appendTaskEvent, tailEvents, getEventsAfter, subscribeToEvents - Add inferPhase and computeMetrics utilities
- Add SSE types (SSEInitEvent, SSEHeartbeatEvent, SSETaskEvent) - Add createEventStreamClient utility with typed callbacks - Add parseSSEMessage helper for manual SSE parsing
Merge main's vi.hoisted() pattern with spawn-worker command mocks. Use debugLogger.warn instead of console.warn.
Use main's vi.hoisted pattern and test structure while adding spawn-worker command mocks for background task feature.
1 participant
Summary
Foundation for #4168 - adds background/detached task execution capability to a2a-server.
Changes
isBackgroundflag toAgentSettingstypeSpawnWorkerCommand- fire-and-forget background task spawning with timeoutListWorkersCommand- list active background tasksGetWorkerCommand- get detailed status of a specific taskCancelWorkerCommand- cancel a running background taskbackground_event_bus.tsfor minimal event loggingLimitations (intentionally out of scope)
Testing
npm run buildpassesnpm test --workspace=packages/a2a-serverpasses (102/102 tests)npm run preflightpartial on Windows:Related
Foundation for #4168 (does not fix - future work needed for full native background agents feature)