Phase 1 (MVP): Memory and ticket pattern analysis
Description
{
"assigned_to": 5,
"blocked_reason": "Waiting for dependencies: 134"
}
Recent Comments
## Code Review: PASS ✅ (Post-Merge) PR #83 already merged. All MCP tests passing (92/92). ### Summary This PR implements Phase 1 (MVP) for Researcher memory and ticket pattern analysis: - Added `researcher_reviewed_at` columns to tickets and agent_memories tables - Created `list_memories` MCP tool with filtering and sorting - Created `mark_as_reviewed` MCP tool for bulk marking - Updated `list_tickets` with `reviewed_before` filter - Added comprehensive tests (19 new test cases) - Updated permissions for researcher role ### Test Results - 92/92 MCP tests passing - All new tools tested with success/error cases - Permission checks verified ### Code Quality - Clean implementation following project patterns - Proper use of ActiveRecord scopes - Good test coverage including edge cases - API properly documented ### Recommendation **APPROVE** - Post-merge review confirms quality implementation. Note: Cannot transition ticket automatically - reviewer tool lacks `transition_ticket` capability. Please manually move ticket #135 to done.
PR #83 already merged. Passing audit post-merge.
## Code Review: PR #83 ### ✅ Test Suite Results - **68 MCP spec tests: PASSING** - 3 pre-existing failures unrelated to this PR (dashboard/kanban feature specs) ### ✅ Code Quality Assessment **Implementation Quality:** - Clean, well-structured MCP tool handlers following existing patterns - Proper project scoping on all queries prevents cross-project data access - Good error handling with descriptive error messages - Indexed columns (`researcher_reviewed_at`) for query performance - Consistent timestamp handling using ISO 8601 format **New MCP Tools:** 1. `list_memories` - Supports memory_type, reviewed_before, sort_by, direction, limit filters 2. `mark_as_reviewed` - Bulk operation for tickets/memories/comments with project validation 3. Updated `list_tickets` and `list_comments` with `reviewed_before` filter ### ✅ Security Review - All database queries properly scoped to `current_agent.project_id` - Project ownership validation in `mark_as_reviewed` prevents cross-project modifications - No SQL injection vectors (parameterized queries) - Permission guards properly configured in mcp_permissions.yml ### ✅ Test Coverage - **19 new test cases** covering success, error, permission, and edge cases - Tests for reviewed_before filters with "null" and ISO 8601 timestamps - Permission tests verify workers are blocked, researchers allowed - All new handlers comprehensively tested ### 📝 Minor Observations (Non-blocking) 1. **Code duplication**: `reviewed_before` filter logic repeated in handlers (could be extracted to private method) 2. **Unreachable branch**: `handle_list_memories` has an else branch that appears unreachable due to early return These are minor refactoring opportunities that don't affect functionality or security. ### Acceptance Criteria Status - [x] Schema migration applied (`researcher_reviewed_at` on tickets, memories, comments) - [x] `list_memories` tool functional with unreviewed filter - [x] `mark_as_reviewed` tool functional - [x] Permissions configured correctly for researcher role - [x] Bonus: Comment support added to reviewed tracking **Recommendation: PASS**
Ticket Stats
Comments
3 commentsAdd a Comment
No Subtasks Yet
Break down this ticket into smaller, manageable subtasks