refactor(telemetry)!: relocate module, add MCP snapshots, HITL handling, and TUI tracking

[lavaman131]

Summary

Major refactoring of the telemetry system with new capabilities for TUI session tracking, MCP runtime introspection, and HITL response handling. Includes breaking changes to telemetry module location and init command behavior.

Key Changes

Telemetry System Restructuring

• Module relocation: Moved telemetry from src/utils/telemetry/ to src/telemetry/ with updated import paths across the codebase
• New TUI session tracking (telemetry-tui.ts): Tracks chat UI lifecycle including session start/end, message submissions, tool lifecycle, slash command execution, and user interrupts
• Workflow integration (graph-integration.ts): Added fail-safe callback-based workflow telemetry tracker for graph executor with sampling support

MCP Runtime Introspection

• Session API extension: Added getMcpSnapshot() method to Session interface for runtime server introspection
• Client implementations:
  • Claude client: Uses query.mcpServerStatus() to fetch auth status and available tools
  • OpenCode client: Full implementation with tools, resources, resource templates, and auth status detection
• UI integration: New /mcp command with Codex-style rendering and enhanced MCP server list component showing transport details, auth status, tools, and resources
• Deepwiki restriction: Updated .mcp.json to whitelist only ask_question tool for deepwiki server

HITL Response Handling

• New utilities (hitl-response.ts): Structured handling for AskUserQuestion tool responses with normalization and display text formatting
• Response modes: Support for option, custom_input, chat_about_this, and declined modes
• Transcript integration: Added HITL response display in transcript formatter with comprehensive test coverage

Init Command Refactoring (Breaking Change)

• Variant reconciliation: Replaced SCM template copying with variant reconciliation approach
• Removed --scm flag behavior: No longer copies templates; instead removes unselected SCM variants (gh-* or sl-*) from target directories
• Preserved user content: User-defined or unmanaged files remain untouched during reconciliation
• Windows compatibility: Eliminated need for separate sapling-phabricator-windows template variant

Agent Configuration Updates

• Recency-based sorting: All research/analysis agents now sort candidate files by date prefix (newest first) before reading
• Enhanced coverage: Added specs/ directory to research locator search patterns
• Prioritization: Newest documents in research/docs/, research/tickets/, and specs/ are read first

Breaking Changes

1. Telemetry import paths: All imports of telemetry modules must update from src/utils/telemetry/ to src/telemetry/
2. Init command behavior: The init command no longer copies SCM template files; it reconciles variants by removing unselected ones
3. Session interface: Added required getMcpSnapshot() method to Session interface (returns null for unsupported clients)

Test Plan

• Unit tests added for init reconciliation (src/commands/init.test.ts)
• Unit tests added for OpenCode MCP snapshots (src/sdk/opencode-client.mcp-snapshot.test.ts)
• Unit tests added for HITL responses (src/ui/utils/hitl-response.test.ts)
• Unit tests added for MCP output formatting (src/ui/utils/mcp-output.test.ts)
• Unit tests added for transcript formatter HITL support (src/ui/utils/transcript-formatter.hitl.test.ts)
• Run pnpm typecheck && pnpm lint && pnpm test to verify no regressions
• Verify /mcp command renders server snapshots correctly in TUI
• Verify telemetry events are emitted for tool start/complete and message submit
• Test init command with both GitHub and Sapling SCM selections
• Verify MCP server introspection works with multiple servers

[claude]

PR Review: refactor(telemetry): relocate module, add MCP snapshots, HITL handling, and TUI tracking

This is a substantial PR with multiple interconnected changes. Overall, the code is well-structured and follows good practices. Here's my detailed review:

---

✅ Strengths

1. Good test coverage - Unit tests added for init reconciliation, OpenCode MCP snapshots, HITL responses, MCP output formatting, and transcript formatter HITL support.

2. Clean module reorganization - Moving telemetry from src/utils/telemetry/ to src/telemetry/ improves discoverability and clarifies that telemetry is a core module.

3. Well-designed MCP snapshot architecture - The McpRuntimeSnapshot type and buildMcpSnapshotView() function provide a clean abstraction for MCP server introspection.

4. Good separation of concerns - HITL response handling is properly extracted into hitl-response.ts with clear interfaces.

5. Comprehensive telemetry types - The new TUI telemetry events (tui_session_start, tui_session_end, etc.) provide granular tracking without being intrusive.

---

🔍 Code Quality Issues

1. Inconsistent indentation in .mcp.json (lines 348-362 in diff)

The JSON formatting changed from 2-space to 4-space indentation. This should be consistent with the project's style.

2. Potential duplicate tool normalization (src/ui/utils/mcp-output.ts:72-75)

function normalizeToolNames(serverName: string, toolNames: string[] | undefined): string[] {
  if (!toolNames || toolNames.length === 0) return [];
  return [...new Set(toolNames.map((name) => normalizeToolName(serverName, name)).filter((name) => name.length > 0))]
    .sort((a, b) => a.localeCompare(b));
}

This is good defensive code, but the long line could be split for readability.

3. Magic string duplication (multiple files)

The HITL tool name check appears in multiple places:

const isHitlTool = tc.toolName === "AskUserQuestion"
  || tc.toolName === "question"
  || tc.toolName === "ask_user";

Consider extracting this to a utility function like isHitlToolName(name: string): boolean.

---

🐛 Potential Bugs

1. Possible race condition in MCP snapshot fetch (src/ui/commands/builtin-commands.ts:436-443)

let runtimeSnapshot = null;
if (context.session?.getMcpSnapshot) {
  try {
    runtimeSnapshot = await context.session.getMcpSnapshot();
  } catch {
    runtimeSnapshot = null;
  }
}

Silent catch is fine here, but logging the error in debug mode would help troubleshooting.

2. MCP server toggle state not persisted (src/ui/chat.tsx:1790)

const [mcpServerToggles, setMcpServerToggles] = useState<McpServerToggleMap>({});

Toggle state is lost on TUI restart. The message says "Changes apply to the next session" but this only means within the current process. Consider clarifying in the user-facing message or documenting this limitation.

---

🔒 Security Considerations

1. Good: Header masking - maskPairValues() properly masks sensitive header values in MCP output.

2. Good: Environment variable masking - Env vars are masked as API_TOKEN=***** format.

3. Note: Ensure telemetry events don't accidentally include sensitive data from tool inputs/outputs.

---

⚡ Performance Considerations

1. MCP snapshot fetching - getMcpSnapshot() creates a temporary query with maxTurns: 0. This is reasonable but adds API overhead for /mcp command. The caching/memoization approach could be considered for frequent calls.

2. Telemetry events - Events are written synchronously via appendFileSync. This is fine for low-frequency events but could impact performance if called in hot paths.

---

📝 Suggestions

1. Consider adding disabledReason to config files - Currently only set programmatically. Allowing users to document why a server is disabled could be useful.

2. The research doc is excellent - The research/docs/2026-02-14-mcp-tool-discovery-startup-bugs.md provides thorough documentation of the MCP tool discovery flow and identified bugs. This is valuable for future maintenance.

3. Init command refactor - The change from copyScmCommands() to reconcileScmVariants() is cleaner. Removing the --scm CLI flag simplifies the API.

---

📋 Test Plan Verification

Per the PR description:

• Unit tests for init reconciliation
• Unit tests for OpenCode MCP snapshots
• Unit tests for HITL responses
• Unit tests for MCP output formatting
• Unit tests for transcript formatter HITL support
• pnpm typecheck && pnpm lint && pnpm test - Should be verified in CI
• Manual verification of /mcp command rendering
• Manual verification of telemetry events

---

Summary

This is a well-executed refactoring PR that:

• Improves code organization (telemetry module location)
• Adds valuable MCP introspection capabilities
• Improves HITL response handling with proper normalization
• Adds comprehensive TUI telemetry without being invasive

The code quality is high with good test coverage. The minor issues noted above are suggestions for polish rather than blockers.

Recommendation: ✅ Approve with minor suggestions