Feat/additional metrics remy

Makefile

@@ -28,7 +28,7 @@ LDFLAGS_DEV := "-X 'github.com/snyk/studio-mcp/application/config.Development=tr
 
 TOOLS_BIN := $(shell pwd)/.bin
 
-OVERRIDE_GOCI_LINT_V := v2.6.1
+OVERRIDE_GOCI_LINT_V := v2.10.1
 GOLICENSES_V := v1.6.0
 PACT_V := 2.4.2
 

internal/analytics/analytics.go

@@ -47,7 +47,14 @@ type EventParam struct {
 	InteractionUUID string         `json:"interactionId"`
 }
 
-func NewAnalyticsEventParam(interactionType string, err error, path types.FilePath) EventParam {
+// NewAnalyticsEventParam builds a new analytics event.
+// correlationID, when non-empty, is stamped on the event as its
+// InteractionUUID so that events sharing a correlationID (e.g. a
+// session-scoped ID minted once per MCP process) can be joined downstream,
+// instead of each call minting its own fresh random ID.
+// Pass an empty string to preserve the previous behavior of letting
+// PayloadForAnalyticsEventParam mint a fresh UUID for this event.
+func NewAnalyticsEventParam(interactionType string, err error, path types.FilePath, correlationID string) EventParam {
 	status := string(analytics.Success)
 	if err != nil {
 		status = string(analytics.Failure)
@@ -71,6 +78,7 @@ func NewAnalyticsEventParam(interactionType string, err error, path types.FilePa
 		Status:          status,
 		TimestampMs:     time.Now().UnixMilli(),
 		TargetId:        targetId,
+		InteractionUUID: correlationID,
 	}
 }
 

internal/analytics/analytics_test.go

@@ -0,0 +1,127 @@
+/*
+ * © 2025 Snyk Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package analytics
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/snyk/go-application-framework/pkg/analytics"
+	"github.com/snyk/go-application-framework/pkg/configuration"
+	"github.com/snyk/go-application-framework/pkg/mocks"
+	"github.com/snyk/go-application-framework/pkg/runtimeinfo"
+	"github.com/snyk/studio-mcp/internal/types"
+	"github.com/stretchr/testify/require"
+)
+
+// These tests cover the "Session-scoped correlation ID on feedback events"
+// requirement (specs/snyk-fix-feedback-verification): studio-mcp mints one
+// correlation ID per process and stamps it on every snyk_send_feedback
+// analytics event, replacing the previous behavior of minting a fresh random
+// ID on every call.
+
+func TestNewAnalyticsEventParam_CorrelationID(t *testing.T) {
+	t.Run("stamps the provided correlation ID as InteractionUUID", func(t *testing.T) {
+		event := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp"), "session-correlation-id")
+		require.Equal(t, "session-correlation-id", event.InteractionUUID)
+	})
+
+	t.Run("two calls with the same correlation ID (same session) share it", func(t *testing.T) {
+		correlationID := "shared-session-id"
+		first := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp/a"), correlationID)
+		second := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp/b"), correlationID)
+
+		require.Equal(t, correlationID, first.InteractionUUID)
+		require.Equal(t, first.InteractionUUID, second.InteractionUUID)
+	})
+
+	t.Run("different correlation IDs (different sessions/processes) differ", func(t *testing.T) {
+		first := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp"), "session-a")
+		second := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp"), "session-b")
+
+		require.NotEqual(t, first.InteractionUUID, second.InteractionUUID)
+	})
+
+	t.Run("empty correlation ID leaves InteractionUUID unset", func(t *testing.T) {
+		event := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp"), "")
+		require.Empty(t, event.InteractionUUID)
+	})
+}
+
+// setupMockEngine builds a minimal mock workflow.Engine sufficient for
+// PayloadForAnalyticsEventParam, which only reads GetConfiguration()/GetRuntimeInfo().
+func setupMockEngine(t *testing.T) *mocks.MockEngine {
+	t.Helper()
+	ctrl := gomock.NewController(t)
+	engine := mocks.NewMockEngine(ctrl)
+	engineConfig := configuration.NewWithOpts(configuration.WithAutomaticEnv())
+	engine.EXPECT().GetConfiguration().Return(engineConfig).AnyTimes()
+	engine.EXPECT().GetRuntimeInfo().Return(runtimeinfo.New(runtimeinfo.WithName("test"), runtimeinfo.WithVersion("1.0.0"))).AnyTimes()
+	return engine
+}
+
+// interactionURN renders the instrumentation collector's payload to JSON and
+// returns it as a string, so tests can assert on the embedded
+// "urn:snyk:interaction:<id>" value without reaching into go-application-framework's
+// internal API types.
+func interactionURNPayload(t *testing.T, engine *mocks.MockEngine, event EventParam) string {
+	t.Helper()
+	ic := PayloadForAnalyticsEventParam(engine, "", event)
+	obj, err := analytics.GetV2InstrumentationObject(ic)
+	require.NoError(t, err)
+	b, err := json.Marshal(obj)
+	require.NoError(t, err)
+	return string(b)
+}
+
+func TestPayloadForAnalyticsEventParam_CorrelationID(t *testing.T) {
+	t.Run("preserves a pre-set session correlation ID instead of minting a fresh one", func(t *testing.T) {
+		engine := setupMockEngine(t)
+		correlationID := "session-correlation-id"
+		event := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp"), correlationID)
+
+		payload := interactionURNPayload(t, engine, event)
+
+		require.Contains(t, payload, "urn:snyk:interaction:"+correlationID)
+	})
+
+	t.Run("two events built from the same session correlation ID resolve to the same interaction URN", func(t *testing.T) {
+		engine := setupMockEngine(t)
+		correlationID := "shared-session-id"
+
+		eventOne := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp/a"), correlationID)
+		eventTwo := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp/b"), correlationID)
+
+		payloadOne := interactionURNPayload(t, engine, eventOne)
+		payloadTwo := interactionURNPayload(t, engine, eventTwo)
+
+		expected := "urn:snyk:interaction:" + correlationID
+		require.Contains(t, payloadOne, expected)
+		require.Contains(t, payloadTwo, expected)
+	})
+
+	t.Run("mints a fresh interaction ID when InteractionUUID is unset", func(t *testing.T) {
+		engine := setupMockEngine(t)
+		event := NewAnalyticsEventParam("Send feedback", nil, types.FilePath("/tmp"), "")
+		require.Empty(t, event.InteractionUUID, "sanity check: no correlation ID was provided")
+
+		payload := interactionURNPayload(t, engine, event)
+
+		require.Contains(t, payload, "urn:snyk:interaction:")
+	})
+}

internal/mcp/llm_binding.go

@@ -62,6 +62,27 @@ type McpLLMBinding struct {
 	started         bool
 	cliPath         string
 	openBrowserFunc types.OpenBrowserFunc
+
+	// correlationID is a session-scoped identifier minted once per process in
+	// Start() and stamped on every snyk_send_feedback analytics event, so
+	// events from the same MCP session can be correlated.
+	// It is set once, before the server starts accepting tool calls, and is
+	// thereafter only read, so it needs no dedicated lock.
+	correlationID string
+
+	// scanCacheMu guards scanCache.
+	// It is intentionally separate from mutex above, which only guards
+	// Start/Started lifecycle state.
+	scanCacheMu sync.Mutex
+	// scanCache is an in-process cache of the freshest scan findings observed
+	// per file path, populated by defaultHandler after successful
+	// snyk_code_scan/snyk_sca_scan calls and consulted by snykSendFeedback to
+	// verify fixedIssueIds/preventedIssueIds claims.
+	// Keyed by file path extracted from each scan's own results, not the tool
+	// call's own path argument.
+	// Bounded to maxScanCacheEntries, evicting the least-recently-updated
+	// entry first.
+	scanCache map[string]*scanCacheEntry
 }
 
 func NewMcpLLMBinding(opts ...Option) *McpLLMBinding {
@@ -78,8 +99,22 @@ func NewMcpLLMBinding(opts ...Option) *McpLLMBinding {
 	return mcpServerImpl
 }
 
+// mintCorrelationID mints the session-scoped correlation ID once per process,
+// if it hasn't been minted already.
+// Every shipped MCP config launches studio-mcp as a fresh process per IDE
+// connection, so minting once here (before any tool call can occur) is
+// sufficient to correlate every snyk_send_feedback event emitted during this
+// session's lifetime.
+func (m *McpLLMBinding) mintCorrelationID() {
+	if m.correlationID == "" {
+		m.correlationID = uuid.New().String()
+	}
+}
+
 // Start starts the MCP server. It blocks until the server is stopped via Shutdown.
 func (m *McpLLMBinding) Start(invocationContext workflow.InvocationContext) error {
+	m.mintCorrelationID()
+
 	runTimeInfo := invocationContext.GetRuntimeInfo()
 	version := ""
 	if runTimeInfo != nil {

internal/mcp/llm_binding_test.go

@@ -402,3 +402,45 @@ func TestStart(t *testing.T) {
 		})
 	})
 }
+
+// TestMintCorrelationID covers the "Session-scoped correlation ID on feedback
+// events" requirement (specs/snyk-fix-feedback-verification): one correlation
+// ID is minted per process and reused for the rest of that process's
+// lifetime, while a different process (a different McpLLMBinding here) gets
+// its own distinct ID.
+func TestMintCorrelationID(t *testing.T) {
+	t.Run("mints a non-empty ID", func(t *testing.T) {
+		binding := NewMcpLLMBinding()
+		require.Empty(t, binding.correlationID, "correlationID must be unset before minting")
+
+		binding.mintCorrelationID()
+
+		require.NotEmpty(t, binding.correlationID)
+	})
+
+	t.Run("two feedback calls in the same process (same binding) share one correlation ID", func(t *testing.T) {
+		binding := NewMcpLLMBinding()
+
+		binding.mintCorrelationID()
+		first := binding.correlationID
+
+		// Simulates a second call within the same session/process reading the
+		// same session-scoped ID; minting must be idempotent.
+		binding.mintCorrelationID()
+		second := binding.correlationID
+
+		require.Equal(t, first, second)
+	})
+
+	t.Run("two different sessions (different bindings) get different correlation IDs", func(t *testing.T) {
+		bindingA := NewMcpLLMBinding()
+		bindingB := NewMcpLLMBinding()
+
+		bindingA.mintCorrelationID()
+		bindingB.mintCorrelationID()
+
+		require.NotEmpty(t, bindingA.correlationID)
+		require.NotEmpty(t, bindingB.correlationID)
+		require.NotEqual(t, bindingA.correlationID, bindingB.correlationID)
+	})
+}