Conversation
🦋 Changeset detectedLatest commit: bc87f2c The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
Contributor
📝 WalkthroughWalkthroughAgent-card URL generation was changed to advertise the JSON-RPC endpoint path Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/server-hono/src/routes/a2a.routes.spec.ts (1)
31-49: Reduceanytype casts in test mocks to maintain TypeScript type safety.Lines 37, 44, 48, and 56 use
as any, which bypasses compile-time checks in a critical test file. Replace with narrow test interfaces orPartial<T>to keep mocks properly typed. This aligns with the repo's TypeScript-first approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-hono/src/routes/a2a.routes.spec.ts` around lines 31 - 49, The tests currently cast mockDeps and mockLogger to any (and cast app in registerA2ARoutes) which removes TypeScript safety; replace these with narrow mock types using Partial<T> or small test interfaces: type MockDeps = Partial<Pick<YourA2ADepsType, "a2a">> (or Partial<typeof realDeps>) for mockDeps, and type MockLogger = Partial<Logger> for mockLogger, then remove the as any casts and pass app as the concrete OpenAPIHono instance to registerA2ARoutes; update the beforeEach to use these typed mocks so vi.fn() implementations satisfy the declared interfaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/with-a2a-server/README.md`:
- Around line 82-90: The README example uses inconsistent endpoint IDs: the
agent card path /.well-known/supportagent/agent-card.json and its advertised
JSON-RPC url "http://localhost:3141/a2a/supportagent" but a subsequent curl
example still targets "/a2a/support", causing a 404; update the later curl
request (and any references to the JSON-RPC endpoint) to use "/a2a/supportagent"
so the documented flow matches the advertised "url" from the agent card and the
paths (e.g., adjust the curl that currently calls "/a2a/support" to
"/a2a/supportagent").
In `@packages/a2a-server/src/server.ts`:
- Around line 36-42: The sanitizeSegment function currently strips internal
whitespace and characters which mangles IDs (e.g., "my agent" -> "myagent") and
fails to escape reserved URL characters; update sanitizeSegment (used by
buildA2AEndpointPath and DEFAULT_A2A_ROUTE_PREFIX) to only trim leading/trailing
slashes and then return an encoded segment via encodeURIComponent so internal
spaces and reserved chars are preserved/escaped rather than removed; ensure
buildA2AEndpointPath uses this new behavior to build safe, unambiguous route
paths.
In `@website/docs/agents/a2a/a2a-server.md`:
- Around line 74-75: Update the troubleshooting text to consistently use the
`{serverId}` terminology used by the endpoints (`GET
/.well-known/{serverId}/agent-card.json`, `POST /a2a/{serverId}`): replace
occurrences of “agent ID” with “serverId” and change any references to the
`agents` key to point to the correct A2A routing key (or configuration field)
that holds the serverId mapping (e.g., reference `serverId` or the servers
routing key used by A2A), and make the same fixes in the other affected block
(lines referenced around 81-84).
---
Nitpick comments:
In `@packages/server-hono/src/routes/a2a.routes.spec.ts`:
- Around line 31-49: The tests currently cast mockDeps and mockLogger to any
(and cast app in registerA2ARoutes) which removes TypeScript safety; replace
these with narrow mock types using Partial<T> or small test interfaces: type
MockDeps = Partial<Pick<YourA2ADepsType, "a2a">> (or Partial<typeof realDeps>)
for mockDeps, and type MockLogger = Partial<Logger> for mockLogger, then remove
the as any casts and pass app as the concrete OpenAPIHono instance to
registerA2ARoutes; update the beforeEach to use these typed mocks so vi.fn()
implementations satisfy the declared interfaces.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3801e0b8-4b96-4850-808c-9e89ef03768c
📒 Files selected for processing (12)
.changeset/bright-badgers-confess.mdexamples/with-a2a-server/README.mdexamples/with-a2a-server/scripts/smoke-test.mjspackages/a2a-server/src/server.spec.tspackages/a2a-server/src/server.tspackages/a2a-server/src/types.tspackages/server-core/src/a2a/types.tspackages/server-elysia/src/routes/a2a.routes.spec.tspackages/server-elysia/src/routes/a2a.routes.tspackages/server-hono/src/routes/a2a.routes.spec.tspackages/server-hono/src/routes/a2a.routes.tswebsite/docs/agents/a2a/a2a-server.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="examples/with-a2a-server/README.md">
<violation number="1" location="examples/with-a2a-server/README.md:89">
P2: The updated endpoint examples use `supportagent`, but the subsequent JSON-RPC curl command still targets `/a2a/support`, which makes the README flow inconsistent and likely to fail for users following it step-by-step.</violation>
</file>
<file name="packages/a2a-server/src/server.ts">
<violation number="1" location="packages/a2a-server/src/server.ts:37">
P2: The new segment sanitizer removes internal whitespace from agent IDs, which can advertise a different endpoint than the actual agent identifier.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Deploying voltagent with
|
| Latest commit: |
bc87f2c
|
| Status: | ⚡️ Build in progress... |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server-hono/src/routes/a2a.routes.spec.ts (1)
8-29: Type the mock factories to keep strict TypeScript guarantees.The mock setup currently relies on untyped
vi.importActual(...)and untyped callback params, which weakens compile-time checks in this TypeScript-first codebase.♻️ Proposed typed mock refinement
vi.mock("@voltagent/server-core", async () => { - const actual = await vi.importActual("@voltagent/server-core"); + const actual = await vi.importActual<typeof import("@voltagent/server-core")>( + "@voltagent/server-core", + ); return { ...actual, executeA2ARequest: vi.fn(), resolveAgentCard: vi.fn(), A2A_ROUTES: actual.A2A_ROUTES, }; }); vi.mock("@voltagent/a2a-server", async () => { return { - normalizeError: vi.fn().mockImplementation((id, error) => ({ + normalizeError: vi + .fn() + .mockImplementation((id: string | number | null, error: { code?: number; message?: string }) => ({ jsonrpc: "2.0", id, error: { code: error.code || -32603, - message: error.message, + message: error.message ?? "Internal error", }, })), }; });As per coding guidelines, "Maintain type safety in TypeScript-first codebase".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-hono/src/routes/a2a.routes.spec.ts` around lines 8 - 29, The mocks are untyped which weakens TS guarantees; update the mock factories to return properly typed mocks by importing the original module types and annotating the factory return types and mocked functions. For the first vi.mock use const actual = await vi.importActual<typeof import("@voltagent/server-core")>(...) and type the factory return as Partial<typeof import("@voltagent/server-core")> (or vi.Mocked<typeof import(...)) so executeA2ARequest and resolveAgentCard are typed (e.g. vi.fn() as vi.MockedFunction<typeof actual.executeA2ARequest>), and for the second vi.mock annotate the factory with typeof import("@voltagent/a2a-server") so normalizeError is typed (e.g. vi.fn().mockImplementation(...) as vi.MockedFunction<typeof import("@voltagent/a2a-server").normalizeError>). This will preserve strict TypeScript checks while keeping the existing mock behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/server-hono/src/routes/a2a.routes.spec.ts`:
- Around line 8-29: The mocks are untyped which weakens TS guarantees; update
the mock factories to return properly typed mocks by importing the original
module types and annotating the factory return types and mocked functions. For
the first vi.mock use const actual = await vi.importActual<typeof
import("@voltagent/server-core")>(...) and type the factory return as
Partial<typeof import("@voltagent/server-core")> (or vi.Mocked<typeof
import(...)) so executeA2ARequest and resolveAgentCard are typed (e.g. vi.fn()
as vi.MockedFunction<typeof actual.executeA2ARequest>), and for the second
vi.mock annotate the factory with typeof import("@voltagent/a2a-server") so
normalizeError is typed (e.g. vi.fn().mockImplementation(...) as
vi.MockedFunction<typeof import("@voltagent/a2a-server").normalizeError>). This
will preserve strict TypeScript checks while keeping the existing mock behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3302e016-2b46-4a98-a2e4-dee617019756
📒 Files selected for processing (7)
examples/with-a2a-server/README.mdpackages/a2a-server/src/server.spec.tspackages/a2a-server/src/server.tspackages/server-core/src/a2a/routes.spec.tspackages/server-core/src/a2a/routes.tspackages/server-hono/src/routes/a2a.routes.spec.tswebsite/docs/agents/a2a/a2a-server.md
✅ Files skipped from review due to trivial changes (1)
- packages/server-core/src/a2a/routes.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/with-a2a-server/README.md
- packages/a2a-server/src/server.ts
- packages/a2a-server/src/server.spec.ts
- website/docs/agents/a2a/a2a-server.md
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
1 participant
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.
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
A2A agent cards advertise the discovery document path (
/.well-known/{serverId}/agent-card.json) in theirurlfield instead of the JSON-RPC endpoint. The returned URL is also relative, which breaks third-party clients that expect an absolute URL.What is the new behavior?
A2A agent cards now advertise
/a2a/{serverId}. When the card is served through the Hono or Elysia integrations, VoltAgent resolves that endpoint to an absolute URL using the incoming request URL.fixes #1198
Notes for reviewers
@voltagent/a2a-serverfor relative and absolute card URLs.with-a2a-serverexample smoke test to assert the advertised endpoint.pnpm --filter @voltagent/a2a-server exec vitest run src/server.spec.tspnpm --filter @voltagent/server-hono exec vitest run src/routes/a2a.routes.spec.tspnpm --filter @voltagent/server-elysia exec vitest run src/routes/a2a.routes.spec.tspnpm exec biome check packages/a2a-server/src/server.ts packages/a2a-server/src/server.spec.ts packages/a2a-server/src/types.ts packages/server-core/src/a2a/types.ts packages/server-hono/src/routes/a2a.routes.ts packages/server-hono/src/routes/a2a.routes.spec.ts packages/server-elysia/src/routes/a2a.routes.ts packages/server-elysia/src/routes/a2a.routes.spec.ts examples/with-a2a-server/scripts/smoke-test.mjs examples/with-a2a-server/README.md website/docs/agents/a2a/a2a-server.mdhttp://localhost:3141/.well-known/supportagent/agent-card.jsonreturns"url": "http://localhost:3141/a2a/supportagent"inexamples/with-a2a-serverexamples/with-a2a-serversmoke test was not run becauseOPENAI_API_KEYis not configured in this environment.Summary by cubic
Fix A2A agent cards to advertise
/a2a/{serverId}and return absolute URLs when served via@voltagent/server-honoor@voltagent/server-elysia. Also correct URL encoding for server IDs so reserved characters are encoded and spaces are preserved. Fixes #1198.@voltagent/a2a-server, the cardurlnow points to/a2a/{serverId}and resolves to an absolute URL when arequestUrlis provided.@voltagent/server-honoand@voltagent/server-elysiapass the incoming request URL toresolveAgentCard; added route tests to assert this.@voltagent/server-core, route helpers now percent‑encode server IDs for both/a2a/{serverId}and/.well-known/{serverId}/agent-card.jsonpaths (spaces encoded as%20, nothing stripped).with-a2a-serverexample to validate absolute/a2aURLs.Written for commit bc87f2c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests