docs(mcp): state what every MCP tool returns - #1420
Conversation
Relay publishes a CLI, a Rust broker, and 17 npm packages that broker credentials and tool invocations between agents, but the repo documented no way to report a vulnerability privately. The only channels on offer were public GitHub issues and Discord, both of which disclose a defect to every workspace running the affected version before a fix exists. Add SECURITY.md pointing at GitHub Security Advisories, with supported versions, scope, and reporting guidance. Scope and out-of-scope reflect what Relay actually owns: the message protocol's trust boundaries and credential handling are in, third-party harness defects are out. The report checklist asks for credentials to be redacted, since workspace keys, broker keys, and observer tokens routinely appear in Relay logs and terminal transcripts. Link it from the README references list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vfi46d3iWyTEm3NAWi31vy
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded ChangesDocumentation updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![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: f61fdaa593
ℹ️ 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".
![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)
SECURITY.md (1)
58-66: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winClarify the supported-version contract.
Lines 58-59 say fixes land on the latest released minor, but lines 63 and 65 mark the full
11.xand3.xranges as supported. State whether older minors are supported but receive no fixes, or narrow the table to the latest minor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SECURITY.md` around lines 58 - 66, Clarify the security-support contract in the version-support table: either explicitly state that older minors within 11.x and 3.x remain supported without receiving fixes, or replace those ranges with the latest supported minor versions. Keep the table entries and introductory policy consistent so “Supported” has one unambiguous meaning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SECURITY.md`:
- Around line 100-103: Update the security-scan coverage paragraph in
SECURITY.md to accurately describe the conditions in
.github/workflows/security.yml: npm audit, CodeQL, and license compliance run
only when node_changed applies, dependency review runs only for pull requests,
and gitleaks coverage should match the workflow. Retain the weekly scheduled-run
statement if applicable.
---
Nitpick comments:
In `@SECURITY.md`:
- Around line 58-66: Clarify the security-support contract in the
version-support table: either explicitly state that older minors within 11.x and
3.x remain supported without receiving fixes, or replace those ranges with the
latest supported minor versions. Keep the table entries and introductory policy
consistent so “Supported” has one unambiguous meaning.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 672b17cc-b54d-44a3-8eb2-e0624e44e893
📒 Files selected for processing (2)
README.mdSECURITY.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The gradable.dev report on the exposed MCP surface found 28 defects, almost all of them no_return_description: a tool description said what the call did but never what came back. An agent reading the tool list could not tell whether post_message hands back a message ID it can reply to, whether a successful spawn means the worker is running or merely queued, or whether an empty list_agents array means "no agents" or "the filter excluded them". Add a return clause to all 30 tool descriptions that lacked one, each grounded in what the handler actually returns — the confirmation string for textContent tools, the named array for list tools, the record shape for jsonContent tools. Where the return is asynchronous (spawn, add_agent, remove_agent, invoke_action) the description now says that acceptance is not completion. register_agent also carried a name_restates_behavior defect: "Register an agent identity..." restated the tool name without adding anything. It now leads with why an agent calls it — claiming an identity is a prerequisite for every messaging tool — and notes that the registered name can differ from the requested one when the session rebinds. invoke_action already documented its return and is unchanged. Tools registered dynamically from action descriptors carry descriptions supplied by whoever registered the action, so they are out of scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vfi46d3iWyTEm3NAWi31vy
willwashburn
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 8-12: Change the changelog heading from “## [Unreleased - Patch]”
to “## [Unreleased]”. Rewrite the single bullet in the Changed section to lead
with the user impact—agents can determine what each agent-relay MCP tool
returns—then retain the existing post_message, spawn, list_agents, and
register_agent examples in that same bullet.
In `@packages/cli/src/cli/agent-relay-mcp.ts`:
- Around line 234-235: Update the submit_result tool description in
agent-relay-mcp.ts to state that timeouts or non-2xx responses leave recording
status unknown, rather than asserting the result was not recorded. Do not imply
retries are safe unless the submission uses a stable idempotency or
deduplication guarantee.
In `@packages/cli/src/cli/mcp/messaging-tools.ts`:
- Around line 302-304: Update the description for the list_dms MCP tool to
remove the claim that conversation IDs can be used to send follow-up messages;
describe the conversations array as identifying one-to-one and group DM
conversations only. Do not add a follow-up tool as part of this change.
- Around line 257-259: Update the get_message_thread input handling and thread
invocation so limit: 0 follows the documented contract: either preserve zero
when constructing the options object or reject zero explicitly in the Zod
schema. Keep positive limits and omitted limits behaving as currently
documented.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 06944b4c-23c7-49cd-be56-696db48c7e69
📒 Files selected for processing (4)
CHANGELOG.mdpackages/cli/src/cli/agent-relay-mcp.tspackages/cli/src/cli/mcp/action-tools.tspackages/cli/src/cli/mcp/messaging-tools.ts
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Three tool descriptions asserted more than the code guarantees:
submit_result said a timeout meant the result was not recorded and the
call could be retried. A timeout only means the client aborted before
reading a response; the server may already have recorded the result, so
a retry can duplicate it. The description now says the outcome is
unknown.
list_dms said conversation IDs are used to send follow-up messages. No
tool accepts a conversation ID — send_dm addresses a recipient by name.
The IDs do address the relay://dm/{conversation_id} resource, so the
description now points there and says how to actually reply.
get_message_thread said results are capped by limit "when supplied", but
the handler tests limit for truthiness, so limit: 0 is dropped and the
full thread comes back. Documented the actual behavior rather than
changing the handler, which would alter runtime behavior in a
description-only change.
SECURITY.md carried three inaccuracies of its own:
- The supported-versions table marked all of 11.x and 3.x supported
while the text said only the latest minor gets fixes. Split into
"reports investigated" and "fix delivered in" so each column means one
thing.
- The local-access exclusion was broad enough to reject reports about
world-readable key files, since exploiting those needs another local
user. Narrowed to same-user access, with cross-user weaknesses named
as in scope.
- The tooling section claimed every push runs the full suite. In fact
CodeQL, npm audit, and license compliance are gated on Node-toolchain
changes, dependency review is PR-only, CodeQL covers JS/TS only (not
the in-scope Rust broker), and several jobs are advisory. Replaced
with the real coverage.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vfi46d3iWyTEm3NAWi31vy
|
Worked through the review feedback in Fixed
I documented the behavior instead of changing the handler. Both suggested fixes — honoring
DeclinedChanging The same review asked the bullet to lead with user impact; it already does — it opens with what an agent can now determine from the tool list, then gives the examples. Verification
All 40 checks were green on the previous head; this push re-runs them. Generated by Claude Code |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vfi46d3iWyTEm3NAWi31vy
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md:
- Around line 1-13: Replace the durable trajectory record as a single
task-specific update: in
.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md
lines 1-13, replace the escape-aware paced injection title and summary with the
current MCP documentation and SECURITY.md work; in trajectory.json lines 11-13,
update completion metadata for the correct task, line 27, update the work end
time, and lines 88-108, retain only decisions and retrospective findings from
this PR.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e718c46-bed3-4fed-be49-9fa11f4f2af3
📒 Files selected for processing (2)
.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/trajectory.json
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md (1)
1-59: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReplace this trajectory record with metadata for the current PR.
This file still records issue
#801escape-aware paced injection work, including unrelated PTY, WebSocket, SIGKILL, and smoke-test decisions. The current PR documents MCP tool results and addsSECURITY.md. Replace the summary, decisions, and work list with task-specific content, and update the companiontrajectory.jsonin the same change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md around lines 1 - 59, Replace the contents of the trajectory summary with metadata describing the current PR’s MCP tool-results documentation and SECURITY.md changes, removing the unrelated issue `#801`, PTY, WebSocket, SIGKILL, and smoke-test details. Update the summary, key decisions, and work list consistently, and modify the companion trajectory.json to match the new task-specific metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md:
- Around line 1-59: Replace the contents of the trajectory summary with metadata
describing the current PR’s MCP tool-results documentation and SECURITY.md
changes, removing the unrelated issue `#801`, PTY, WebSocket, SIGKILL, and
smoke-test details. Update the summary, key decisions, and work list
consistently, and modify the companion trajectory.json to match the new
task-specific metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c90c6a9-70df-4637-945f-809662fad030
📒 Files selected for processing (2)
.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md.agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/trajectory.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/trajectory.json
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
This reverts commit 4242b9c.
…edback-i6ijz8 # Conflicts: # CHANGELOG.md
…z8' into claude/agent-relay-feedback-i6ijz8 # Conflicts: # .agentworkforce/trajectories/completed/2026-08/traj_9835z9cvpl9q/summary.md
2 participants
Summary
Addresses the gradable.dev report on Relay's exposed MCP surface: 28 defects, overwhelmingly
no_return_description, plus aname_restates_behavioronregister_agent.Every tool description said what the call did and none said what came back. An agent reading the tool list couldn't tell whether
post_messagehands back a message ID it can reply to, whether a successfulspawnmeans the worker is running or merely queued, or whether an emptylist_agentsarray means "no agents exist" or "the status filter excluded them".30 tool descriptions across
agent-relay-mcp.ts,mcp/messaging-tools.ts, andmcp/action-tools.tsgain a return clause, each grounded in what the handler actually returns rather than restated from the output schema:textContenttools describe the confirmation string (add_reactionnotes the returned emoji is the resolved one, so:+1:comes back as 👍).jsonContenttools describe the record, and name the field callers need next —post_messagereturns the ID thatreply_to_threadandadd_reactionconsume.spawn,add_agent,remove_agent) now say acceptance is not completion.create_workspaceexplains thatwarningappears only when the workspace exists but its session couldn't be persisted — the case where the key must be kept manually.register_agentalso carriedname_restates_behavior: "Register an agent identity in the current workspace..." restated the tool name. It now leads with why an agent calls it — claiming an identity is a prerequisite for every messaging tool — and notes the registered name can differ from the requested one when the session rebinds.Descriptions state only what the code guarantees. Where a handler is looser than the obvious wording would imply, the description follows the handler:
submit_resultsays a timeout leaves the outcome unknown rather than promising a safe retry, andget_message_threadrecords thatlimit: 0is ignored because the handler tests truthiness.Deliberately unchanged
invoke_actionalready documented its return ("returns an ack with an invocation id"). Left alone rather than churned.action-tools.ts:133) take their description from the action descriptor, supplied by whoever registered the action. Not ours to rewrite — worth a follow-up that validates descriptions at registration time if the score should cover them.SECURITY.md. Two review findings proposed changingget_message_thread's runtime handling oflimit: 0; I documented the behavior instead, since neither fix is verifiable from the client side in a description-only change. See the review reply for the follow-up worth opening —check_inboxandget_message_threadtreat the same parameter differently.Also in this PR
Commit
f61fdaaaddsSECURITY.md— a private vulnerability disclosure channel, which the repo lacked despite publishing 17 npm packages and running CodeQL/gitleaks/npm-audit in CI. It predates my seeing the report and is unrelated to the score. Happy to split it into its own PR — say the word and I'll rebase it out.Two items in it want a human's call: the 3-day/10-day response targets are conventional defaults nobody has committed to, and I deliberately did not invent a
security@address. It also needs private vulnerability reporting enabled in Settings → Code security for the advisory link to work.Test Plan
agent-relay-mcp.startup.test.tspass unchangednpm run typecheck— cleannpx vitest run packages/cli/src/cli/agent-relay-mcp.startup.test.ts packages/cli/src/cli/mcp— 26/26 passnpx prettier --check— cleanrelay://dm/{conversation_id}resource referenced bylist_dmsexists atresources.ts:202, and thesecurity.ymlcoverage described inSECURITY.mdmatches the job-levelif:conditionsScreenshots
n/a