Agent Visibility System: Live terminal streaming, session logs, and debug dashboard
Description
Recent Comments
## Code Review: PR #57 ❌ FAIL **Files Reviewed:** 34 files changed, +1576/-142 ### ❌ CRITICAL BUG - MCP Tools Will Not Work **Location:** `app/controllers/api/v1/mcp_controller.rb:291-316` The new MCP tools `get_terminal_logs` and `list_agent_sessions` are **missing the `handler:` property** required by the MCP dispatcher. ```ruby # BROKEN - Missing handler property { name: "get_terminal_logs", description: "...", parameters: { ... } # ❌ handler: :handle_get_terminal_logs <-- MISSING! } ``` Compare with the correct pattern used by all other tools: ```ruby # CORRECT - Has handler property { name: "ask_for_memory_deletion", description: "...", parameters: { ... }, handler: :handle_ask_for_memory_deletion # ✓ Required! } ``` **Impact:** The tools will be registered but cannot be called - the dispatcher doesn't know which method to invoke. **Fix:** Add `handler: :handle_get_terminal_logs` and `handler: :handle_list_agent_sessions` to the respective tool definitions. --- ### ⚠️ MODERATE ISSUE - Wrong API Endpoint in JavaScript **Location:** `app/javascript/controllers/agent_status_controller.js:56` ```javascript fetchAgentStatus() { fetch(`/api/v1/tickets/${this.agentIdValue}`) // ❌ Wrong endpoint! ``` This fetches tickets but `agentIdValue` contains an agent ID. The endpoint `/api/v1/tickets/{id}` returns ticket data, not agent status data. The response won't have `availability_status` or `availability_updated_at`. **Fix:** Use the correct agent status endpoint or remove the polling since WebSocket updates are already implemented. --- ### ⚠️ MODERATE ISSUE - ActionCable Subscription Format **Location:** `app/javascript/controllers/agent_status_controller.js:25-30` ```javascript this.subscription = cable.subscriptions.create("AgentChannel", { agent_type: "all" }, { received: (data) => this.handleAgentUpdate(data) }); ``` The ActionCable subscription format may be incorrect. Should be: ```javascript cable.subscriptions.create({ channel: "AgentChannel", agent_type: "all" }, { received: (data) => this.handleAgentUpdate(data) }); ``` --- ### ✅ What's Good - ✓ Database migrations properly structured with indexes - ✓ Model validations and AASM state machine correctly implemented - ✓ Cleanup job for log retention (7 days) configured in recurring.yml - ✓ MCP tools registered in mcp-bridge/src/tools/index.ts - ✓ Permissions configured in mcp_permissions.yml for orchestrator, worker, reviewer - ✓ Internal auth for session logging (X-Terminal-Internal header) - ✓ Scopes and associations properly defined - ✓ xterm.js loaded via CDN (acceptable for this use case) --- ### Required Fixes for Merge 1. Add `handler: :handle_get_terminal_logs` to get_terminal_logs tool definition 2. Add `handler: :handle_list_agent_sessions` to list_agent_sessions tool definition 3. Fix or remove the polling endpoint in agent_status_controller.js
## Code Review: PR #56 ✓ PASS **Files Reviewed:** - `mcp-bridge/src/tools/index.ts` (+44 lines) - `config/tinker/mcp_permissions.yml` (+6 lines) **Quality Checks:** - ✓ Tool registrations follow established pattern - ✓ Complete inputSchema with proper types and defaults - ✓ Error handling via formatError - ✓ Consistent formatting - ✓ No security issues (read-only tools with limit parameters) - ✓ Permissions appropriately granted to orchestrator, worker, reviewer **This PR fixes the recurring MCP registration issue** documented in memory (#43, #42) where tools are added to Rails but not registered in mcp-bridge. **No issues found.** Ready to merge.