[P1] Add evidence blocks to Madar MCP responses#348
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR implements the MCP response evidence shape from issue ChangesMCP Response Evidence Shape and Agent Directives
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
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)
src/runtime/stdio/tools.ts (1)
1206-1211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
evidencetocontext_packreview/impact responses.Line 1211 and Line 1241 return
context_packpayloads without top-levelevidence. That leaves strict clients withoutevidence.agent_directivefor these two task modes.Suggested patch
- const payload = { + const payload = { ...contextPackBasePayload(task, prompt, resolvedBudget, graphPath, plan), pack: compactPack, ...reviewMetadata, + evidence: buildMadarResponseEvidence({ + coverage: prResult.review_bundle.coverage, + missingPhases: missingPhasesFromPayload(prResult.review_bundle), + coveredWorkflowOwners: collectWorkflowOwners(prResult.changed_files), + }), } return helpers.ok(id, helpers.textToolResult(JSON.stringify(payload))) @@ return helpers.ok(id, helpers.textToolResult(JSON.stringify({ ...contextPackBasePayload(task, prompt, resolvedBudget, graphPath, initialPlan), target: impactTarget, pack: impactPack, ...metadata, + evidence: buildMadarResponseEvidence({ + coverage: metadata.coverage, + coveredWorkflowOwners: collectWorkflowOwners( + impactResult.target_file ? [impactResult.target_file] : [], + impactResult.affected_files, + impactResult.direct_dependents.map((entry) => entry.source_file), + impactResult.transitive_dependents.map((entry) => entry.source_file), + ), + }), })))Also applies to: 1236-1241
🤖 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 `@src/runtime/stdio/tools.ts` around lines 1206 - 1211, The payload returned for the context_pack review/impact responses (the local variable payload built from contextPackBasePayload, compactPack, and reviewMetadata and then returned via helpers.ok(id, helpers.textToolResult(JSON.stringify(payload)))) must include a top-level evidence field; update those return sites (the one that builds payload with compactPack and the other similar return around lines 1236-1241) to merge in an evidence object that at minimum contains agent_directive (e.g., evidence: { agent_directive: <appropriate directive string> }) so strict clients receive evidence.agent_directive; keep using the same payload variable and ensure the modified payload is JSON-stringified in the helpers.textToolResult call.
🧹 Nitpick comments (1)
tests/unit/stdio-server.test.ts (1)
827-913: ⚡ Quick winExpand this contract test to include
pr_impactandcontext_promptevidence.Nice coverage for most tools, but this shared evidence assertion currently skips two evidence-enabled surfaces from the PR contract, so regressions there can slip through.
Suggested patch
const calls = { @@ impact: await Promise.resolve(handleStdioRequest(graphPath, { id: 107, method: 'tools/call', params: { name: 'impact', arguments: { label: 'AuthService', depth: 3, }, }, })), + context_prompt: await Promise.resolve(handleStdioRequest(graphPath, { + id: 108, + method: 'tools/call', + params: { + name: 'context_prompt', + arguments: { + prompt: 'How does auth reach transport?', + provider: 'gemini', + }, + }, + })), } as constThen add the same
evidenceassertions for apr_impactresponse in the existingpr_impacttest block (that fixture already sets up git state).🤖 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 `@tests/unit/stdio-server.test.ts` around lines 827 - 913, Add two missing tool calls to the test's calls fixture and assert their evidence fields: update the calls object (where handleStdioRequest is invoked for context_pack/retrieve/...) to include entries for 'context_prompt' and 'pr_impact' (use the same pattern: id, method 'tools/call', params.name set to 'context_prompt' or 'pr_impact' and appropriate params.arguments); then, in the existing pr_impact test block (the test that already sets up git state), add the same evidence assertions used for other evidence-enabled tools to validate the 'evidence' structure on the pr_impact response. Ensure you reference the same response shapes used for 'context_pack'/'retrieve' assertions so coverage matches the other tools.
🤖 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 `@src/runtime/stdio/tools.ts`:
- Around line 1206-1211: The payload returned for the context_pack review/impact
responses (the local variable payload built from contextPackBasePayload,
compactPack, and reviewMetadata and then returned via helpers.ok(id,
helpers.textToolResult(JSON.stringify(payload)))) must include a top-level
evidence field; update those return sites (the one that builds payload with
compactPack and the other similar return around lines 1236-1241) to merge in an
evidence object that at minimum contains agent_directive (e.g., evidence: {
agent_directive: <appropriate directive string> }) so strict clients receive
evidence.agent_directive; keep using the same payload variable and ensure the
modified payload is JSON-stringified in the helpers.textToolResult call.
---
Nitpick comments:
In `@tests/unit/stdio-server.test.ts`:
- Around line 827-913: Add two missing tool calls to the test's calls fixture
and assert their evidence fields: update the calls object (where
handleStdioRequest is invoked for context_pack/retrieve/...) to include entries
for 'context_prompt' and 'pr_impact' (use the same pattern: id, method
'tools/call', params.name set to 'context_prompt' or 'pr_impact' and appropriate
params.arguments); then, in the existing pr_impact test block (the test that
already sets up git state), add the same evidence assertions used for other
evidence-enabled tools to validate the 'evidence' structure on the pr_impact
response. Ensure you reference the same response shapes used for
'context_pack'/'retrieve' assertions so coverage matches the other tools.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d5a0d69b-ba15-4b10-b401-99b52c6f8624
📒 Files selected for processing (13)
docs/mcp-response-shape.mdsrc/infrastructure/compare.tssrc/infrastructure/install-skill-templates.tssrc/infrastructure/install.tssrc/runtime/implementation-checklist.tssrc/runtime/mcp-response-evidence.tssrc/runtime/risk-map.tssrc/runtime/stdio/tools.tstests/unit/compare.test.tstests/unit/install-templates.test.tstests/unit/install.test.tstests/unit/mcp-response-shape-doc.test.tstests/unit/stdio-server.test.ts
Closes #339
Summary
evidenceblocks across the relevant Madar MCP response surfacesevidence.agent_directiveand its three literal branchesagent_directive_seenin compare traces and document the response-shape mappingTesting
Summary by CodeRabbit
New Features
answer_from_pack,verify_one_targeted_file,explore_with_caution) to guide exploration.Documentation
Tests