[PR #572] Add 5 new client metrics event types: ToolCall, McpInvocation, NewMessage, SkillUsed, SubagentEvent #580

New issue

Closed

opened 2026-03-02 04:14:01 +03:00 by kerem · 0 comments

kerem commented

2026-03-02 04:14:01 +03:00

Owner

Original Pull Request: https://github.com/git-ai-project/git-ai/pull/572

State: closed
Merged: No

Add 5 new client metrics event types for agent telemetry

Summary

Adds client-side support for 5 new metrics event types: ToolCall (5), McpInvocation (6), NewMessage (7), SkillUsed (8), and SubagentEvent (9). These are emitted from the checkpoint command based on hook context passed through a new HookMetadata struct on AgentRunResult.

Key changes:

metrics/types.rs / metrics/events.rs: New MetricEventId variants and corresponding value structs (ToolCallValues, McpInvocationValues, NewMessageValues, SkillUsedValues, SubagentEventValues) with full PosEncoded/EventValues trait implementations.
checkpoint.rs: Event emission logic keyed on hook_event_name strings. Also moves build_checkpoint_attrs outside the !entries.is_empty() guard so hook-only events (no file changes) can still emit metrics. AgentUsage pings now fire on message hooks too (gated by should_emit_agent_usage).
agent_presets.rs: New HookMetadata { hook_event_name, tool_name } struct. Claude preset extracts subagent_id/subagent_model into agent_metadata. Cursor preset extracts MCP server/tool names on beforeMCPExecution.
Agent hook installers updated: Claude Code (+UserPromptSubmit, Stop, SubagentStart, SubagentStop), Cursor (+beforeMCPExecution), Gemini (+BeforeModel, AfterModel, BeforeMCPExecution, AfterMCPExecution).
parse_mcp_tool_name helper parses mcp__server__tool convention.
Unit tests for all new event structs (builder, sparse, roundtrip). Integration tests via TmpRepo verify checkpoint execution doesn't panic with various hook metadata.

Note: Server-side work (ClickHouse migrations, metrics_parser, worker-metrics) is not included in this PR. Events will buffer client-side but be rejected server-side until that lands.

Review & Testing Checklist for Human

Verify build_checkpoint_attrs move is safe. Previously only called when !entries.is_empty(). Now runs unconditionally so hook-only events can use attrs. Check if this causes issues when repo state is unusual (e.g., detached HEAD, no commits).
Check for duplicate MCP events. MCP invocations can fire from both PostToolUse (if tool name has mcp__ prefix) AND explicit beforeMCPExecution hooks. If both trigger for the same call, you'll get two McpInvocationValues events. Confirm this is acceptable or needs deduplication.
Confirm should_emit_agent_usage gating is intentional. Function currently always returns false, so AgentUsage pings on messages won't fire until rate-limiting is enabled. Is this expected?
Review hook event name string matching. Logic uses hardcoded strings like "UserPromptSubmit", "PostToolUse", "beforeMCPExecution". Handles multiple casing variants but may miss some. Verify coverage for all agents.
Test end-to-end with real agent hooks. Integration tests are shallow (just verify no panic). Manually trigger hooks from Claude Code/Cursor/Gemini and verify events appear in observability log (~/.git-ai/internal/logs/{PID}.log) and metrics DB.

Notes

Link to Devin run: https://app.devin.ai/sessions/ed389a486416455b9dc36faf30f7c6c3
Requested by: @svarlamov
All 1092 existing tests pass + 13 new integration tests added
parse_mcp_tool_name only handles mcp__server__tool format (2 underscores). Extra segments would be misparsed.
Cursor test fix: Changed .get(*hook_name).unwrap() to .and_then(...).unwrap_or_default() to gracefully handle missing hook keys (changes production behavior from panic to silent empty).

**Original Pull Request:** https://github.com/git-ai-project/git-ai/pull/572 **State:** closed **Merged:** No --- # Add 5 new client metrics event types for agent telemetry ## Summary Adds client-side support for 5 new metrics event types: **ToolCall** (5), **McpInvocation** (6), **NewMessage** (7), **SkillUsed** (8), and **SubagentEvent** (9). These are emitted from the checkpoint command based on hook context passed through a new `HookMetadata` struct on `AgentRunResult`. **Key changes:** - `metrics/types.rs` / `metrics/events.rs`: New `MetricEventId` variants and corresponding value structs (`ToolCallValues`, `McpInvocationValues`, `NewMessageValues`, `SkillUsedValues`, `SubagentEventValues`) with full `PosEncoded`/`EventValues` trait implementations. - `checkpoint.rs`: Event emission logic keyed on `hook_event_name` strings. Also moves `build_checkpoint_attrs` outside the `!entries.is_empty()` guard so hook-only events (no file changes) can still emit metrics. AgentUsage pings now fire on message hooks too (gated by `should_emit_agent_usage`). - `agent_presets.rs`: New `HookMetadata { hook_event_name, tool_name }` struct. Claude preset extracts subagent_id/subagent_model into `agent_metadata`. Cursor preset extracts MCP server/tool names on `beforeMCPExecution`. - Agent hook installers updated: Claude Code (+UserPromptSubmit, Stop, SubagentStart, SubagentStop), Cursor (+beforeMCPExecution), Gemini (+BeforeModel, AfterModel, BeforeMCPExecution, AfterMCPExecution). - `parse_mcp_tool_name` helper parses `mcp__server__tool` convention. - Unit tests for all new event structs (builder, sparse, roundtrip). Integration tests via TmpRepo verify checkpoint execution doesn't panic with various hook metadata. **Note:** Server-side work (ClickHouse migrations, metrics_parser, worker-metrics) is **not included** in this PR. Events will buffer client-side but be rejected server-side until that lands. ## Review & Testing Checklist for Human - [ ] **Verify `build_checkpoint_attrs` move is safe.** Previously only called when `!entries.is_empty()`. Now runs unconditionally so hook-only events can use attrs. Check if this causes issues when repo state is unusual (e.g., detached HEAD, no commits). - [ ] **Check for duplicate MCP events.** MCP invocations can fire from both `PostToolUse` (if tool name has `mcp__` prefix) AND explicit `beforeMCPExecution` hooks. If both trigger for the same call, you'll get two `McpInvocationValues` events. Confirm this is acceptable or needs deduplication. - [ ] **Confirm `should_emit_agent_usage` gating is intentional.** Function currently always returns `false`, so AgentUsage pings on messages won't fire until rate-limiting is enabled. Is this expected? - [ ] **Review hook event name string matching.** Logic uses hardcoded strings like `"UserPromptSubmit"`, `"PostToolUse"`, `"beforeMCPExecution"`. Handles multiple casing variants but may miss some. Verify coverage for all agents. - [ ] **Test end-to-end with real agent hooks.** Integration tests are shallow (just verify no panic). Manually trigger hooks from Claude Code/Cursor/Gemini and verify events appear in observability log (`~/.git-ai/internal/logs/{PID}.log`) and metrics DB. ### Notes - Link to Devin run: https://app.devin.ai/sessions/ed389a486416455b9dc36faf30f7c6c3 - Requested by: @svarlamov - All 1092 existing tests pass + 13 new integration tests added - `parse_mcp_tool_name` only handles `mcp__server__tool` format (2 underscores). Extra segments would be misparsed. - Cursor test fix: Changed `.get(*hook_name).unwrap()` to `.and_then(...).unwrap_or_default()` to gracefully handle missing hook keys (changes production behavior from panic to silent empty).  --- <a href="https://app.devin.ai/review/git-ai-project/git-ai/pull/572" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a>