docs: audit feature verification catalog - #1345
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe feature catalog now maps expanded CLI, MCP, SDK, and plugin surfaces to executable verification procedures. Contract tests validate catalog coverage, while critical paths and guardian messaging support the updated manifest model. ChangesFeature verification catalog
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request comprehensively updates the feature verification catalog for Agent Relay. It refactors the feature manifest, verification procedures, and critical paths to document exact CLI and MCP surfaces, adds previously unlisted SDK and plugin integrations, and maps every category to an end-to-end procedure. Additionally, a contract test suite is introduced to prevent CLI and MCP drift. The reviewer feedback suggests adding a nullish coalescing operator in the contract test to robustly handle cases where a category features array might be missing or undefined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const proceduresPath = new URL('../../features/verify/procedures.md', import.meta.url); | ||
| const manifest = parse(readFileSync(manifestPath, 'utf8')) as Manifest; | ||
| const procedures = readFileSync(proceduresPath, 'utf8'); | ||
| const features = Object.values(manifest.categories).flatMap((category) => category.features); |
There was a problem hiding this comment.
If any category in the manifest does not define a features array (or if it is temporarily set to null/undefined during development), calling flatMap directly on it will result in undefined elements in the features array, which will subsequently cause a TypeError when mapping over feature.id in the tests. Using the nullish coalescing operator (?? []) ensures the test suite is robust against missing or malformed category features.
| const features = Object.values(manifest.categories).flatMap((category) => category.features); | |
| const features = Object.values(manifest.categories).flatMap((category) => category.features ?? []); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
.agentworkforce/agents/relay-feature-guardian/agent.ts (1)
562-570: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrompt wording is CLI-centric but
cliis now optional. For MCP-only featuressurfaceresolves to"CLI: (MCP-only surface)", yet the instructions still tell the LLM to "show the CLI command" and confirm "a specific CLI feature." This can produce messages that reference or invent a nonexistent CLI command. Consider generalizing the wording to "command or MCP tool/prompt."♻️ Suggested wording generalization
- 'Write a brief, conversational Slack message (3-5 sentences, no markdown headers) asking the team to confirm whether a specific CLI feature is working as intended.', - 'Be specific: name the feature, describe what it should do, show the CLI command, and ask if it behaves this way or if anything has drifted.', + 'Write a brief, conversational Slack message (3-5 sentences, no markdown headers) asking the team to confirm whether a specific feature is working as intended.', + 'Be specific: name the feature, describe what it should do, show the CLI command or MCP tool/prompt, and ask if it behaves this way or if anything has drifted.',🤖 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/agents/relay-feature-guardian/agent.ts around lines 562 - 570, Generalize the prompt wording in the prompt array so it supports both CLI and MCP-only features: replace CLI-specific references to the feature, command, and expected behavior with language covering a command or MCP tool/prompt. Preserve the existing specificity, conversational tone, feature interpolation, and required reaction ending.
🤖 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/features/critical-paths.md:
- Around line 23-26: Bind TOKEN_B explicitly to the critical-b agent before
using it: update .agentworkforce/features/critical-paths.md lines 23-26 so the
registration name matches the invite and list commands, and apply the same
explicit TOKEN_B-to-agent identity binding in lines 80-88 for DM send, list, and
read-receipt verification.
In @.agentworkforce/features/verify/procedures.md:
- Line 155: Remove the `|| true` fallback from the `relay node agent release
"$AGENT"` verification command so release failures propagate and fail the
procedure. Keep tolerant error handling limited to any separate cleanup trap.
- Line 151: Initialize every external fixture required by the verification
procedures: at .agentworkforce/features/verify/procedures.md:151-151, define and
export PROVIDER before the relay spawn command; at
.agentworkforce/features/verify/procedures.md:199-199, provide WORKER_TOKEN; at
.agentworkforce/features/verify/procedures.md:260-260, obtain CAPTURE_URL from a
controlled receiver fixture; and at
.agentworkforce/features/critical-paths.md:38-38, reuse the same explicit
provider fixture.
- Around line 131-140: Update the reaction verification flow around the
add/remove commands to read the target message after each mutation and assert
that thumbsup is present after add and absent after remove. Preserve the
existing token identities and message identifiers while making both
reaction-state assertions executable before continuing to inbox and archive
checks.
- Around line 74-82: Update the verification procedure’s jq assertion after the
channel invitation to validate the channel name, updated topic, and membership
of the invited audit agent, rather than checking only .name. Keep the existing
cleanup sequence that leaves and archives the channel, and ensure test
identities are removed as requested.
- Around line 54-58: Update the capability flow after the deletion commands to
verify both resources are absent: list capabilities and assert no entry matches
CAP, then list agents and assert no entry matches audit-extra-$RUN. Keep these
checks immediately after the corresponding relay capabilities delete and relay
agent remove operations, before completing verification.
- Around line 116-120: Update the group-DM procedure’s send_group command to
include RELAY_AGENT_TOKEN="$TOKEN_A", ensuring it uses the intended identity.
Move removal of audit-c-$RUN until after list_dms verification confirms the same
conversation is visible to both group recipients.
In @.claude/skills/verify-features.md:
- Around line 24-26: Update the manifest lookup command in the “View all
features in a category” section of verify-features.md to match the actual
four-space indentation of the nested messaging-messages key under
verification.categories, or replace the text search with a structural YAML
query. Ensure the command correctly locates the category.
---
Nitpick comments:
In @.agentworkforce/agents/relay-feature-guardian/agent.ts:
- Around line 562-570: Generalize the prompt wording in the prompt array so it
supports both CLI and MCP-only features: replace CLI-specific references to the
feature, command, and expected behavior with language covering a command or MCP
tool/prompt. Preserve the existing specificity, conversational tone, feature
interpolation, and required reaction ending.
🪄 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: f5ac7d8c-6283-42f6-9114-d00512b0f6c9
📒 Files selected for processing (8)
.agentworkforce/agents/relay-feature-guardian/agent.ts.agentworkforce/agents/relay-feature-guardian/manifest-contract.test.ts.agentworkforce/features/critical-paths.md.agentworkforce/features/manifest.yaml.agentworkforce/features/verify/procedures.md.agentworkforce/trajectories/active/traj_rtyte8r4g07t/trajectory.json.claude/skills/verify-features.mdCHANGELOG.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
.agentworkforce/features/verify/procedures.md (6)
12-14: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse collision-resistant run identifiers.
RUNcontains only second-level timestamps. Concurrent verification runs in the same hosted workspace can reuse agent/resource names, causing cross-run assertions or cleanup to target another run’s fixtures. Include a CI run identifier or a cryptographically/randomly unique suffix.Also applies to: 20-25
🤖 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/features/verify/procedures.md around lines 12 - 14, Update the RUN identifier setup in the verification procedure to append a CI-provided run identifier or cryptographically/randomly generated suffix to the timestamp, ensuring concurrent runs receive unique agent and resource names. Apply the same collision-resistant identifier pattern to the additional RUN usage around the referenced setup and cleanup steps.
238-252: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake cloud-worker cleanup executable.
The procedure starts a daemon but only describes PID termination in prose; no command captures the PID or terminates it. A successful run therefore leaves a worker daemon—and potentially its registration—behind.
🤖 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/features/verify/procedures.md around lines 238 - 252, Update the cloud-workers procedure around the daemon start commands to capture the launched worker PID, record its log path, and explicitly terminate that PID during cleanup. Ensure cleanup also removes the temporary worker registration when applicable, while preserving the guidance to use --once for bounded assignment fixtures.
187-204: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert the spawned task’s result before release.
The flow verifies that the agent exists and accepts hold/auto/release commands, but never confirms that the bounded task produced
relay-e2e-okor reached its expected terminal state. A spawn that fails immediately could therefore pass this procedure.🤖 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/features/verify/procedures.md around lines 187 - 204, Update the local-agent-lifecycle procedure after the spawn and before release to poll the spawned agent’s status or output until the bounded task reaches its expected terminal state, asserting that it produced “relay-e2e-ok”. Keep the existing hold/auto checks, and only run relay node agent release after the task-result assertion succeeds.
300-314: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert webhook delivery at the controlled receiver.
The commands create, trigger, list, and delete the webhook, but never read the receiver’s captured request. The procedure can pass even when delivery, payload content, or signature verification fails. Add a receiver-side wait/query and assert the exact payload and signature before deletion.
🤖 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/features/verify/procedures.md around lines 300 - 314, Update the integrations-and-webhooks procedure to wait for and query the controlled receiver after triggering the webhook, then assert the captured request’s exact payload and signature before deleting the webhook. Preserve the existing create, list, and cleanup flow, and ensure deletion occurs only after receiver-side delivery validation.
23-26: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInstall cleanup for hosted fixtures.
cleanup_agentsis defined but never registered as an exit trap. Any failed assertion after registration leaks hosted identities, and the function does not cover dynamically created identities such as C oraudit-extra-$RUN.🤖 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/features/verify/procedures.md around lines 23 - 26, Update cleanup_agents in the fixture setup to register it with an exit trap immediately after defining it, and include every hosted identity created by the procedure, including dynamically created C and audit-extra-$RUN agents. Preserve best-effort removal and ensure cleanup runs when assertions or later commands fail.
12-15: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake verification failures fail the procedure.
The shared shell setup does not enable
set -e/pipefail, so a failedjq -eassertion can be ignored and later commands can still make the procedure appear successful.Suggested fix
RUN="feature-$(date +%s)"; TMP="$(mktemp -d)"; cd "$TMP" +set -Eeuo pipefail relay node up --background --no-spawn🤖 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/features/verify/procedures.md around lines 12 - 15, Update the shared shell setup in the verification procedure to enable fail-fast behavior and pipeline error propagation before running commands, including `jq -e` assertions. Ensure any failed command causes the procedure to exit nonzero while preserving the existing temporary-directory cleanup trap.
🤖 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.
Outside diff comments:
In @.agentworkforce/features/verify/procedures.md:
- Around line 12-14: Update the RUN identifier setup in the verification
procedure to append a CI-provided run identifier or cryptographically/randomly
generated suffix to the timestamp, ensuring concurrent runs receive unique agent
and resource names. Apply the same collision-resistant identifier pattern to the
additional RUN usage around the referenced setup and cleanup steps.
- Around line 238-252: Update the cloud-workers procedure around the daemon
start commands to capture the launched worker PID, record its log path, and
explicitly terminate that PID during cleanup. Ensure cleanup also removes the
temporary worker registration when applicable, while preserving the guidance to
use --once for bounded assignment fixtures.
- Around line 187-204: Update the local-agent-lifecycle procedure after the
spawn and before release to poll the spawned agent’s status or output until the
bounded task reaches its expected terminal state, asserting that it produced
“relay-e2e-ok”. Keep the existing hold/auto checks, and only run relay node
agent release after the task-result assertion succeeds.
- Around line 300-314: Update the integrations-and-webhooks procedure to wait
for and query the controlled receiver after triggering the webhook, then assert
the captured request’s exact payload and signature before deleting the webhook.
Preserve the existing create, list, and cleanup flow, and ensure deletion occurs
only after receiver-side delivery validation.
- Around line 23-26: Update cleanup_agents in the fixture setup to register it
with an exit trap immediately after defining it, and include every hosted
identity created by the procedure, including dynamically created C and
audit-extra-$RUN agents. Preserve best-effort removal and ensure cleanup runs
when assertions or later commands fail.
- Around line 12-15: Update the shared shell setup in the verification procedure
to enable fail-fast behavior and pipeline error propagation before running
commands, including `jq -e` assertions. Ensure any failed command causes the
procedure to exit nonzero while preserving the existing temporary-directory
cleanup trap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c468d7bd-21b8-4eff-8267-0978577285be
📒 Files selected for processing (3)
.agentworkforce/features/critical-paths.md.agentworkforce/features/verify/procedures.md.claude/skills/verify-features.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/skills/verify-features.md
- .agentworkforce/features/critical-paths.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agentworkforce/agents/relay-feature-guardian/agent.ts (1)
584-593: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winFix the fallback branch
mcpNoteandmcpPromptNoteare undefined here, so thecatchpath throws aReferenceErrorinstead of returning the backup Slack message. Also, this fallback only shows the first available surface; reuse the existingsurfacelist so CLI/MCP features aren’t truncated.🤖 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/agents/relay-feature-guardian/agent.ts around lines 584 - 593, Fix the fallback Slack message construction in the catch path so it does not reference the undefined mcpNote or mcpPromptNote variables. Reuse the existing surface list when rendering the feature surface, preserving all available CLI, MCP, and MCP prompt entries instead of selecting only the first one.
🤖 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.
Outside diff comments:
In @.agentworkforce/agents/relay-feature-guardian/agent.ts:
- Around line 584-593: Fix the fallback Slack message construction in the catch
path so it does not reference the undefined mcpNote or mcpPromptNote variables.
Reuse the existing surface list when rendering the feature surface, preserving
all available CLI, MCP, and MCP prompt entries instead of selecting only the
first one.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d533681-d095-4780-9d01-9ac62f0d3156
📒 Files selected for processing (3)
.agentworkforce/agents/relay-feature-guardian/agent.ts.agentworkforce/features/verify/procedures.md.agentworkforce/trajectories/active/traj_rtyte8r4g07t/trajectory.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .agentworkforce/features/verify/procedures.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Summary
Validation
npm run typechecknpm test— 1,297 passed, 20 skippedNote:
npm testin the existing checkout has a pre-existing integration test failure because ignored.agentworkforce/relay/workspace state is intentionally present; the same built suite passes in a clean worktree.