feat: native A2A protocol support (Phase 8)#4
Conversation
A2A client for agent card discovery and full task lifecycle (submit, poll, stream, cancel). A2A server to expose evalanche skills as A2A-compliant endpoints. Economy adapters bridging A2A concepts to discovery, negotiation, and settlement. 6 new MCP tools: fetch_agent_card, a2a_list_skills, a2a_submit_task, a2a_get_task, a2a_cancel_task, a2a_serve. 49 new tests across 3 test files (421 total passing).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc24a3e9df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| parts: [{ type: 'text', text: result.text ?? 'Task completed' }], | ||
| }); | ||
| task.status = 'completed'; |
There was a problem hiding this comment.
After /tasks/{id}/cancel marks a task as canceled, the async _executeSkill path still unconditionally sets task.status = 'completed' when the handler resolves. This means long-running tasks can flip from canceled back to completed and publish artifacts/messages, so cancellation is not actually reliable for callers that depend on terminal state.
Useful? React with 👍 / 👎.
src/interop/a2a-server.ts
Outdated
| this._handleRequest(req, res).catch(() => { | ||
| if (!res.headersSent) { | ||
| res.writeHead(500, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ error: 'Internal server error' })); |
There was a problem hiding this comment.
The server wraps all _handleRequest errors into HTTP 500 responses, including client-side validation errors such as missing skill_id or unknown skills thrown by _handleSubmitTask. In those common bad-input scenarios, clients get a misleading internal-server-error response instead of a 4xx, which can trigger incorrect retry behavior and obscure actionable feedback.
Useful? React with 👍 / 👎.
… validation errors Address Codex review: - P1: _executeSkill now checks for terminal state before overwriting status - P2: _handleSubmitTask validation errors (missing/unknown skill_id) return 400 not 500
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94309c45b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/interop/a2a-server.ts
Outdated
| skills, | ||
| defaultInputModes: ['text'], | ||
| defaultOutputModes: ['text'], | ||
| supportsStreaming: this._options.supportsStreaming ?? false, |
There was a problem hiding this comment.
When supportsStreaming is set in A2AServerOptions, getAgentCard() advertises supportsStreaming: true, but _handleRequest does not implement GET /tasks/{id}/stream; the server returns 404 for that route. In deployments that enable this flag, clients that trust the card (including A2AClient.streamTask) will reliably fail at runtime despite the capability being advertised.
Useful? React with 👍 / 👎.
src/interop/a2a-server.ts
Outdated
| const raw = Buffer.concat(chunks).toString('utf-8'); | ||
| resolve(raw ? JSON.parse(raw) : {}); | ||
| } catch { | ||
| reject(new Error('Invalid JSON body')); |
There was a problem hiding this comment.
_parseBody rejects malformed JSON with a generic Error, but the top-level request handler only maps EvalancheErrorCode.A2A_ERROR to HTTP 400, so malformed client payloads currently fall through as HTTP 500. This misclassifies bad input as a server fault and can trigger incorrect retry behavior for callers sending invalid JSON.
Useful? React with 👍 / 👎.
src/interop/a2a-server.ts
Outdated
| }); | ||
| }); | ||
|
|
||
| this._server.listen(port); |
There was a problem hiding this comment.
listen() starts the HTTP server without attaching an 'error' listener, so startup failures like EADDRINUSE/EACCES become unhandled server errors. In practice (for example when another process already owns the port), this can crash the process instead of returning a controlled failure to the caller.
Useful? React with 👍 / 👎.
- P1: never advertise supportsStreaming (no stream endpoint implemented) - P2: malformed JSON bodies now return 400 instead of 500 - P2: add error listener on server.listen() for EADDRINUSE/EACCES - Remove misleading supportsStreaming option from A2AServerOptions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f912186ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (url === '/tasks' && method === 'POST') { | ||
| const body = await this._parseBody(req); | ||
| const task = await this._handleSubmitTask(body); | ||
| res.writeHead(201, { ...cors, 'Content-Type': 'application/json' }); |
There was a problem hiding this comment.
The server advertises authentication in the agent card, but the task submission path still executes _handleSubmitTask without validating request credentials. In deployments that rely on A2AServerOptions.authentication for access control, unauthenticated clients can still invoke paid or sensitive skills, so the advertised auth requirement is not actually enforced.
Useful? React with 👍 / 👎.
| artifacts: task.artifacts.map((a) => ({ | ||
| name: a.name, | ||
| mimeType: a.mimeType, | ||
| text: a.text, | ||
| uri: a.uri, |
There was a problem hiding this comment.
mapTaskCompletion strips each artifact down to name, mimeType, text, and uri, dropping the data payload from A2AArtifact. Any task that returns binary/base64 artifacts will lose its output content at this adapter boundary, which can break downstream settlement/result consumers that need the full artifact.
Useful? React with 👍 / 👎.
src/interop/a2a-server.ts
Outdated
| this._server.on('error', (err) => { | ||
| throw new EvalancheError( | ||
| `A2A server failed to start: ${err.message}`, |
There was a problem hiding this comment.
Fresh evidence relative to the prior thread: an 'error' listener is now attached, but it immediately throws inside the callback. When Node emits startup/runtime server errors like EADDRINUSE, that throw is uncaught in the event loop and can still terminate the process instead of surfacing a controlled failure path to callers of listen().
Useful? React with 👍 / 👎.
…listen() Address Codex review round 3: - P1: reject unauthenticated requests on /tasks when authentication configured - P1: listen() returns Promise, rejects on EADDRINUSE/EACCES (no uncaught throw) - P2: mapTaskCompletion now preserves binary artifact data field - Agent card still served without auth (public discovery)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1e56c7426
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // Submit task | ||
| if (url === '/tasks' && method === 'POST') { |
There was a problem hiding this comment.
Task routing compares req.url directly (exact '/tasks' and regexes), but req.url includes the query string. When authentication.in is 'query', authenticated calls like POST /tasks?api_key=... pass _checkAuth and then miss every task route, returning 404 instead of executing; the same mismatch affects /tasks/{id} and /tasks/{id}/cancel with query params.
Useful? React with 👍 / 👎.
src/interop/a2a.ts
Outdated
| Accept: 'application/json', | ||
| }; | ||
| if (options.auth) { | ||
| headers['Authorization'] = options.auth; |
There was a problem hiding this comment.
The client always sends credentials as the Authorization header, but agent cards can declare auth in query params or under a different header name (authentication.in / authentication.name). This makes submitTask/getTask/streamTask/cancelTask fail against valid A2A agents that require non-Authorization credentials, even when callers provide auth.
Useful? React with 👍 / 👎.
src/interop/a2a.ts
Outdated
| buffer = lines.pop() ?? ''; | ||
|
|
||
| for (const line of lines) { | ||
| if (line.startsWith('data: ')) { |
There was a problem hiding this comment.
The stream parser only accepts lines starting with data: (with a space), but SSE allows data: without the extra space. For compliant servers emitting data:{...}, onUpdate is never called and task streaming appears broken even though the server is sending valid events.
Useful? React with 👍 / 👎.
…ata: without space
Address Codex review round 4:
- P1: strip query string from req.url before route matching (fixes query-based auth)
- P2: client _applyAuth honors authPlacement (header name, query param) from agent card
- P2: SSE parser accepts both 'data: {...}' and 'data:{...}' formats
mc-review summaryVerdict: close — Closed as spam TestsTests found: ✅ yes Note: package.json defines QualityIssues (medium): Recommendation🚫 Close this PR |
mc-review summaryVerdict: close — Closed as spam TestsTests found: ✅ yes Note: The repo already had relevant tests, so no new stubs were needed; I had to install dependencies first and rerun npm test with write access because sandboxed Vitest could not create its temp/cache directories. QualityIssues (high): src/interop/a2a-server.ts:350-366 treats authentication as "header/query param exists" instead of validating a configured credential value, so any non-empty token bypasses protection on Recommendation🚫 Close this PR |
2 participants
Summary
src/interop/a2a.ts): Fetch agent cards, resolve via ERC-8004, submit/poll/stream/cancel taskssrc/interop/a2a-server.ts): Expose evalanche skills as A2A-compliant endpoints with auto-generated agent cardssrc/interop/a2a-adapters.ts): Bridge A2A skills → discovery, agent cards → registrations, tasks → negotiation proposals, failures → escrow refundsfetch_agent_card,a2a_list_skills,a2a_submit_task,a2a_get_task,a2a_cancel_task,a2a_serveSDK primitives only — no contracts, no deployment, no infrastructure changes.
Test plan
npx vitest run— 421 tests passing (27 files, 0 failures)npx tsc --noEmit— clean type-checka2a_serveand submit tasks from another agent