## Code Review: PR #82
### What Was Reviewed
This PR implements the Researcher agent infrastructure as specified in ticket #134:
- Added `researcher` agent type with system prompt
- Added `create_proposal` and `list_proposals` MCP tools
- Configured permissions in `mcp_permissions.yml`
- Added `has_many :proposals` association to Agent model
### Code Quality: PASS ✓
- Clean implementation following existing patterns
- Proper project-scoped query in `handle_list_proposals` (proposals scoped to agent's project)
- System prompt is well-structured with clear role boundaries and guardrails
- Proper defaults for `confidence` and `priority` parameters
### Security: PASS ✓
- Researcher role is strictly read-only except for `create_proposal`
- Properly blocked from: create_ticket, update_ticket, transition_ticket, ask_for_memory_deletion, send_message_to_agent, mark_busy, mark_idle
- Guardrails in system prompt are comprehensive (0% noise target, clear forbidden actions)
### Test Coverage: FAIL ✗
**Critical Issue:** No tests for the new MCP tools in `spec/requests/api/v1/mcp_spec.rb`
The PR adds two new tools (`create_proposal`, `list_proposals`) but has zero integration tests covering:
- Success cases (creating proposals, listing with filters)
- Error cases (invalid parameters, validation failures)
- Permission checks (non-researcher agents blocked)
- Edge cases (agent without project, empty results)
The only test change was adding `researcher` to `known_roles` in `mcp_synchronization_spec.rb`.
Every other MCP tool in the codebase has comprehensive test coverage (see `create_ticket`, `update_ticket`, `add_comment`, `list_members` tests). Following the same pattern, `create_proposal` and `list_proposals` should have similar coverage.
### Required Changes
Add test coverage in `spec/requests/api/v1/mcp_spec.rb` following the existing pattern:
```ruby
describe "create_proposal" do
let(:researcher) { create(:agent, :researcher, project: project) }
it "creates a proposal via MCP"
it "returns access denied for workers attempting to create proposals"
it "validates required parameters"
end
describe "list_proposals" do
it "lists proposals scoped to agent's project"
it "filters by status, type, confidence, priority"
it "returns error when agent has no project"
end
```
### Test Suite Results
All 629 examples passed, 10 pending (unrelated to this PR).
### Decision: FAIL_AUDIT
The implementation is solid, but missing test coverage for new MCP tools is a blocker. Please add integration tests for `create_proposal` and `list_proposals` before this can be approved.
