mcp fixes for deterministic usage

Author: michaelzixizhou

internal/mcp/server.go

@@ -18,11 +18,17 @@ import (
 	"bufio"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"net"
 	"os"
+	osexec "os/exec"
 	"sync"
 	"time"
+
+	"github.com/codag-megalith/codag-cli/internal/analytics"
+	"github.com/codag-megalith/codag-cli/internal/api"
 )
 
 // time aliases — kept local so the heartbeat code is easy to mock in tests
@@ -292,6 +298,11 @@ func (s *Server) handleToolCall(req jrpcRequest) {
 	for {
 		select {
 		case r := <-done:
+			// One analytics event per tool invocation, covering both the
+			// success and error paths. This is the only place MCP (agent)
+			// usage is observable — cobra-level events don't see individual
+			// tool calls inside a long-lived `codag mcp` session.
+			captureToolCall(p.Name, startTime, r.err)
 			if r.err != nil {
 				s.writeResult(req.ID, map[string]interface{}{
 					"content": []map[string]interface{}{
@@ -316,6 +327,7 @@ func (s *Server) handleToolCall(req jrpcRequest) {
 			// (Some clients ignore this response since they sent the cancel
 			// themselves, but emitting a clean error keeps the protocol
 			// state consistent and avoids the "hang on Calling codag…" UI.)
+			captureToolCall(p.Name, startTime, context.Canceled)
 			s.writeResult(req.ID, map[string]interface{}{
 				"content": []map[string]interface{}{
 					{"type": "text", "text": "tool call cancelled"},
@@ -327,6 +339,60 @@ func (s *Server) handleToolCall(req jrpcRequest) {
 	}
 }
 
+// captureToolCall records one mcp_tool_called analytics event per tool
+// invocation, mirroring the cli_command_completed/failed shape used for cobra
+// commands so MCP (agent) usage shows up alongside direct CLI usage in the same
+// PostHog project. analytics.Capture no-ops under CODAG_TELEMETRY opt-out, and
+// only error_code (never raw error text) is sent.
+func captureToolCall(name string, start time.Time, err error) {
+	props := map[string]any{
+		"tool":        name,
+		"duration_ms": int(timeSinceSeconds(start) * 1000),
+		"ok":          err == nil,
+	}
+	if err != nil {
+		props["error_code"] = mcpErrorCode(err, props)
+	}
+	analytics.Capture("mcp_tool_called", props)
+}
+
+// mcpErrorCode classifies an error into a stable, low-cardinality code and
+// stamps any safe structured detail onto props. Mirrors cmd.addTelemetryError;
+// duplicated here because internal/mcp cannot import the cmd package.
+func mcpErrorCode(err error, props map[string]any) string {
+	var httpErr *api.HTTPError
+	var billingErr *api.BillingError
+	var exitErr *osexec.ExitError
+	var netErr net.Error
+	switch {
+	case errors.Is(err, api.ErrUnauthenticated):
+		return "unauthenticated"
+	case errors.Is(err, api.ErrQuotaExceeded):
+		return "quota_exceeded"
+	case errors.As(err, &httpErr):
+		props["http_status"] = httpErr.Status
+		return "http_error"
+	case errors.As(err, &billingErr):
+		props["has_upgrade_path"] = billingErr.UpgradePath != ""
+		return "billing_required"
+	case errors.Is(err, api.ErrBillingRequired):
+		return "billing_required"
+	case errors.As(err, &exitErr):
+		props["exit_code"] = exitErr.ExitCode()
+		return "child_exit"
+	case errors.Is(err, context.DeadlineExceeded):
+		return "timeout"
+	case errors.Is(err, context.Canceled):
+		return "canceled"
+	case errors.As(err, &netErr):
+		if netErr.Timeout() {
+			return "network_timeout"
+		}
+		return "network_error"
+	}
+	return "unknown"
+}
+
 // notify writes an unsolicited JSON-RPC notification (no id field).
 func (s *Server) notify(method string, params any) {
 	s.mu.Lock()

internal/mcp/tools_test.go

@@ -1,12 +1,15 @@
 package mcp
 
 import (
+	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/codag-megalith/codag-cli/internal/api"
 )
@@ -102,6 +105,41 @@ func TestWrapAPIErrorBillingRequiredGivesAgentNextSteps(t *testing.T) {
 	}
 }
 
+func TestMCPErrorCodeClassifies(t *testing.T) {
+	cases := []struct {
+		err  error
+		want string
+	}{
+		{api.ErrUnauthenticated, "unauthenticated"},
+		{api.ErrQuotaExceeded, "quota_exceeded"},
+		{api.ErrBillingRequired, "billing_required"},
+		{&api.HTTPError{Status: 500}, "http_error"},
+		{&api.BillingError{UpgradePath: "/dashboard/billing"}, "billing_required"},
+		{context.DeadlineExceeded, "timeout"},
+		{context.Canceled, "canceled"},
+		{errors.New("boom"), "unknown"},
+	}
+	for _, c := range cases {
+		props := map[string]any{}
+		if got := mcpErrorCode(c.err, props); got != c.want {
+			t.Fatalf("mcpErrorCode(%v) = %q, want %q", c.err, got, c.want)
+		}
+	}
+	// Structured detail is stamped onto props for the HTTP case.
+	props := map[string]any{}
+	mcpErrorCode(&api.HTTPError{Status: 503}, props)
+	if props["http_status"] != 503 {
+		t.Fatalf("http_status not stamped: %v", props["http_status"])
+	}
+}
+
+func TestCaptureToolCallNoPanicWhenTelemetryDisabled(t *testing.T) {
+	// analytics client is uninitialized in tests, so Capture is a no-op; this
+	// just guards against a nil-deref / panic regression in the chokepoint.
+	captureToolCall("compact", time.Now(), nil)
+	captureToolCall("tail_vercel", time.Now(), api.ErrUnauthenticated)
+}
+
 func TestCompactRawTextUsesDeterministicFreeCompact(t *testing.T) {
 	t.Setenv("XDG_CONFIG_HOME", t.TempDir())