Conversation
📝 WalkthroughWalkthroughConfiguration updates to the AVA agent with nine new skills (chat, sitrep, manage_feature, auto_mode, board_health, bug_triage, provision_discord, plan, plan_resume), environment-aware callback URL protocol selection (https in production, http otherwise), and PUBLIC_CALLBACK_URL environment variable support for external A2A peer routing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ 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)
Comment |
…labs.ai Root cause: Workstacean couldn't reach the live agent card because the card URL advertised http://automaker-server:3008/a2a (Docker internal hostname). It fell back to agents.yaml where Ava only had one skill (onboard_project), so bare "chat" message/sends fell through to protoBot. Three-part fix: 1. agents.yaml — add all 10 DECLARED_SKILLS (chat, sitrep, manage_feature, auto_mode, board_health, bug_triage, onboard_project, provision_discord, plan, plan_resume) to Ava's entry so the fallback table is correct. 2. buildAgentCard() — use protocol:'https' in production so the card URL is externally routable; add logger.warn when resolved URL is localhost. 3. docker-compose.staging.yml — expose PUBLIC_CALLBACK_URL env var so operators can set https://ava.proto-labs.ai without rebuilding the image. Closes #471. Unblocks protoVoice F7 (Ava A2A delegate). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mabry1985
force-pushed
the
fix/a2a-messagesend-on-avaproto-labsai-routes-to-8sg399r
branch
from
e358534 to
8977528
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/server/src/routes/a2a/index.ts (1)
241-242: Replace hardcoded host example in runtime warning.Use a generic placeholder (or config-derived value) instead of embedding
https://ava.proto-labs.aiin business-logic logging text.Proposed tweak
- 'Set PUBLIC_CALLBACK_URL to an externally-routable URL (e.g. https://ava.proto-labs.ai).' + 'Set PUBLIC_CALLBACK_URL to an externally-routable URL (e.g. https://your-public-host).'As per coding guidelines: "Never hardcode workflow-specific values (file paths, branch names, CI check names, channel IDs, hosting providers) in business logic — these must come from settings or configuration files."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/routes/a2a/index.ts` around lines 241 - 242, The runtime warning string contains a hardcoded example URL ("https://ava.proto-labs.ai"); replace that literal with a config-driven value or a generic placeholder and interpolate it into the message instead of embedding it. Read the external callback value (e.g. process.env.PUBLIC_CALLBACK_URL or your app config entry PUBLIC_CALLBACK_URL) and fall back to a neutral placeholder like "https://your-public-callback-url.example" if not set, then update the log/emission that currently contains the "A2A agent card URL resolved to localhost..." text to use the interpolated value in place of the hardcoded URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/routes/a2a/index.ts`:
- Line 238: The code unconditionally passes port into resolveCallbackUrl,
causing resolveCallbackUrl({ port, protocol }) to append a port even when
PUBLIC_CALLBACK_URL is explicitly set; change the call to only include port when
PUBLIC_CALLBACK_URL is not configured (e.g., call resolveCallbackUrl({ protocol
}) or omit the port argument if process.env.PUBLIC_CALLBACK_URL is truthy), and
adjust any logic that builds callbackBase to prefer the explicit
PUBLIC_CALLBACK_URL raw value without modification; also update the related
warning log (the hardcoded example URL) to avoid embedding a provider-specific
URL—use the actual configured PUBLIC_CALLBACK_URL or a generic placeholder in
the log message.
---
Nitpick comments:
In `@apps/server/src/routes/a2a/index.ts`:
- Around line 241-242: The runtime warning string contains a hardcoded example
URL ("https://ava.proto-labs.ai"); replace that literal with a config-driven
value or a generic placeholder and interpolate it into the message instead of
embedding it. Read the external callback value (e.g.
process.env.PUBLIC_CALLBACK_URL or your app config entry PUBLIC_CALLBACK_URL)
and fall back to a neutral placeholder like
"https://your-public-callback-url.example" if not set, then update the
log/emission that currently contains the "A2A agent card URL resolved to
localhost..." text to use the interpolated value in place of the hardcoded URL.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c3d4abc-1102-4b24-b71b-a45e419d549e
📒 Files selected for processing (3)
agents.yamlapps/server/src/routes/a2a/index.tsdocker-compose.staging.yml
| // correct scheme); otherwise we default to https in production. | ||
| const isProduction = process.env['NODE_ENV'] === 'production'; | ||
| const protocol = isProduction ? 'https' : 'http'; | ||
| const callbackBase = resolveCallbackUrl({ port, protocol }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Verify call site in buildAgentCard:"
sed -n '222,246p' apps/server/src/routes/a2a/index.ts
echo
echo "2) Verify resolver behavior for explicitPublicUrl + port:"
fd "callback-url.ts" libs --exec sed -n '161,229p' {}Repository: protoLabsAI/protoMaker
Length of output: 4222
Do not unconditionally pass PORT to resolveCallbackUrl() when PUBLIC_CALLBACK_URL is explicitly configured.
When PUBLIC_CALLBACK_URL is set (e.g., https://ava.proto-labs.ai), the resolver appends the port suffix if the explicit URL has no port. This results in advertising an incorrect external URL like https://ava.proto-labs.ai:3008 instead of the configured public endpoint.
Proposed fix
- const callbackBase = resolveCallbackUrl({ port, protocol });
+ const publicCallbackUrl = process.env['PUBLIC_CALLBACK_URL']?.trim();
+ const callbackBase = publicCallbackUrl
+ ? resolveCallbackUrl({ publicCallbackUrl, protocol })
+ : resolveCallbackUrl({ port, protocol });Additionally, the warning log message hardcodes https://ava.proto-labs.ai as an example—hosting provider URLs should not be hardcoded in runtime messages.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const callbackBase = resolveCallbackUrl({ port, protocol }); | |
| const publicCallbackUrl = process.env['PUBLIC_CALLBACK_URL']?.trim(); | |
| const callbackBase = publicCallbackUrl | |
| ? resolveCallbackUrl({ publicCallbackUrl, protocol }) | |
| : resolveCallbackUrl({ port, protocol }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/routes/a2a/index.ts` at line 238, The code unconditionally
passes port into resolveCallbackUrl, causing resolveCallbackUrl({ port, protocol
}) to append a port even when PUBLIC_CALLBACK_URL is explicitly set; change the
call to only include port when PUBLIC_CALLBACK_URL is not configured (e.g., call
resolveCallbackUrl({ protocol }) or omit the port argument if
process.env.PUBLIC_CALLBACK_URL is truthy), and adjust any logic that builds
callbackBase to prefer the explicit PUBLIC_CALLBACK_URL raw value without
modification; also update the related warning log (the hardcoded example URL) to
avoid embedding a provider-specific URL—use the actual configured
PUBLIC_CALLBACK_URL or a generic placeholder in the log message.
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.
Summary
buildAgentCard()advertisedhttp://automaker-server:3008/a2a(Docker internal hostname) — unreachable by Workstacean externally. Workstacean fell back toagents.yaml, where Ava only had one skill (onboard_project), so baremessage/sendrequests fell through to protoBot.DECLARED_SKILLSto Ava's fallback entry (chat,sitrep,manage_feature,auto_mode,board_health,bug_triage,onboard_project,provision_discord,plan,plan_resume) so the fallback table matches the live agent card.protocolis now'https'in production so Tailscale/HOSTNAME-derived URLs are externally routable. Addedlogger.warnwhen the resolved URL is localhost.PUBLIC_CALLBACK_URLenv var so operators can sethttps://ava.proto-labs.aiwithout rebuilding the image.Deployment note
After deploying, set
PUBLIC_CALLBACK_URL=https://ava.proto-labs.aiin the staging.env(or Docker env). This overrides any Tailscale/hostname heuristic and ensures the agent card always advertises the correct public URL. The localhost warning in logs will confirm if this is missing.Test plan
PUBLIC_CALLBACK_URL=https://ava.proto-labs.aicurl https://ava.proto-labs.ai/.well-known/agent.json— verifyurlfield ishttps://ava.proto-labs.ai/a2acurl -X POST https://ava.proto-labs.ai/a2a -H "Content-Type: application/json" -d '{"jsonrpc":"2.0","method":"message/send","params":{"message":{"parts":[{"text":"ping"}]}}}'— verify response identifies as Ava, not protoBotPUBLIC_CALLBACK_URLsetCloses #471. Unblocks protoVoice F7.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
chat,sitrep,manage_feature,auto_mode,board_health,bug_triage,provision_discord,plan, andplan_resumewith A2A and Discord DM triggering support.Improvements