fix(acp): improve claude acp tool messages and model selector flow (#442)

apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go

@@ -813,10 +813,18 @@ func (a *Adapter) convertNotification(n acp.SessionNotification) *AgentEvent {
 	case u.ConfigOptionUpdate != nil:
 		configOptions := convertACPConfigOptions(u.ConfigOptionUpdate.ConfigOptions)
 		if len(configOptions) > 0 {
+			// Include cached available models so this event doesn't overwrite
+			// the model list that was set during session initialization.
+			a.mu.RLock()
+			cachedModels := a.availableModels
+			a.mu.RUnlock()
+			currentModelID := resolveCurrentModelFromConfig(configOptions)
 			return &AgentEvent{
-				Type:          streams.EventTypeSessionModels,
-				SessionID:     sessionID,
-				ConfigOptions: configOptions,
+				Type:           streams.EventTypeSessionModels,
+				SessionID:      sessionID,
+				CurrentModelID: currentModelID,
+				SessionModels:  convertSessionModels(cachedModels),
+				ConfigOptions:  configOptions,
 			}
 		}
 	}
@@ -1273,6 +1281,21 @@ func (a *Adapter) convertToolCallResultUpdate(sessionID string, tcu *acp.Session
 		a.normalizer.NormalizeToolResult(payload, tcu.RawOutput)
 	}
 
+	// Enrich modify_file payload from tool_call_contents.
+	// Claude ACP sends path and content in tool_call_update, not in the initial tool_call.
+	if payload != nil && payload.Kind() == streams.ToolKindModifyFile {
+		if mf := payload.ModifyFile(); mf != nil {
+			enrichModifyFileFromContents(mf, tcu.Content)
+		}
+	}
+
+	// Enrich read_file payload path from title if still empty.
+	if payload != nil && payload.Kind() == streams.ToolKindReadFile {
+		if rf := payload.ReadFile(); rf != nil && rf.FilePath == "" && tcu.Title != nil {
+			rf.FilePath = extractPathFromTitle(*tcu.Title)
+		}
+	}
+
 	if isTerminal {
 		delete(a.activeToolCalls, toolCallID)
 	}
@@ -1309,6 +1332,48 @@ func (a *Adapter) convertToolCallResultUpdate(sessionID string, tcu *acp.Session
 	}
 }
 
+// enrichModifyFileFromContents updates a ModifyFilePayload with data from
+// tool_call_contents. Claude ACP sends file path and content in tool_call_update
+// events rather than in the initial tool_call rawInput.
+func enrichModifyFileFromContents(mf *streams.ModifyFilePayload, contents []acp.ToolCallContent) {
+	for _, c := range contents {
+		if c.Diff == nil {
+			continue
+		}
+		if mf.FilePath == "" && c.Diff.Path != "" {
+			mf.FilePath = c.Diff.Path
+		}
+		if len(mf.Mutations) == 0 {
+			continue
+		}
+		mut := &mf.Mutations[0]
+		if mut.Diff != "" {
+			continue // Already has diff, don't overwrite
+		}
+		if c.Diff.OldText != nil {
+			diffPath := c.Diff.Path
+			if diffPath == "" {
+				diffPath = mf.FilePath
+			}
+			mut.Diff = shared.GenerateUnifiedDiff(*c.Diff.OldText, c.Diff.NewText, diffPath, mut.StartLine)
+		} else if c.Diff.NewText != "" {
+			mut.Type = streams.MutationCreate
+			mut.Content = c.Diff.NewText
+		}
+		break
+	}
+}
+
+// extractPathFromTitle extracts a file path from tool titles like "Read /path/to/file".
+func extractPathFromTitle(title string) string {
+	for _, prefix := range []string{"Read ", "Write ", "Edit "} {
+		if strings.HasPrefix(title, prefix) {
+			return strings.TrimPrefix(title, prefix)
+		}
+	}
+	return ""
+}
+
 // handlePermissionRequest handles permission requests from the agent.
 // Since both acpclient and adapter now use the shared types package,
 // no conversion is needed - we just forward to the handler.

apps/backend/internal/agentctl/server/adapter/transport/acp/normalize.go

@@ -251,6 +251,18 @@ func (n *Normalizer) UpdatePayloadInput(payload *streams.NormalizedPayload, rawI
 			shellExec.Description = desc
 		}
 	}
+
+	// Claude ACP sends file_path in incremental rawInput updates
+	if mf := payload.ModifyFile(); mf != nil {
+		if path := shared.GetString(inputMap, "file_path"); path != "" && mf.FilePath == "" {
+			mf.FilePath = path
+		}
+	}
+	if rf := payload.ReadFile(); rf != nil {
+		if path := shared.GetString(inputMap, "file_path"); path != "" && rf.FilePath == "" {
+			rf.FilePath = path
+		}
+	}
 }
 
 // normalizeEdit converts ACP edit tool data.

apps/backend/internal/agentctl/server/adapter/transport/acp/normalize_test.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/coder/acp-go-sdk"
 	"github.com/kandev/kandev/internal/agentctl/server/adapter/transport/shared"
 	"github.com/kandev/kandev/internal/agentctl/types/streams"
 )
@@ -654,6 +655,200 @@ func TestUpdatePayloadInput(t *testing.T) {
 	})
 }
 
+// TestEnrichModifyFileFromContents tests enriching modify_file payloads from tool_call_contents.
+func TestEnrichModifyFileFromContents(t *testing.T) {
+	oldText := "old content"
+
+	t.Run("enriches file path from diff content", func(t *testing.T) {
+		mf := &streams.ModifyFilePayload{
+			FilePath:  "",
+			Mutations: []streams.FileMutation{{Type: streams.MutationPatch}},
+		}
+		contents := []acp.ToolCallContent{
+			{Diff: &acp.ToolCallContentDiff{Path: "/workspace/README.md", NewText: "new content"}},
+		}
+		enrichModifyFileFromContents(mf, contents)
+		if mf.FilePath != "/workspace/README.md" {
+			t.Errorf("expected FilePath '/workspace/README.md', got %q", mf.FilePath)
+		}
+	})
+
+	t.Run("generates unified diff when oldText is provided", func(t *testing.T) {
+		mf := &streams.ModifyFilePayload{
+			FilePath:  "",
+			Mutations: []streams.FileMutation{{Type: streams.MutationPatch}},
+		}
+		contents := []acp.ToolCallContent{
+			{Diff: &acp.ToolCallContentDiff{Path: "file.ts", OldText: &oldText, NewText: "new content"}},
+		}
+		enrichModifyFileFromContents(mf, contents)
+		if mf.Mutations[0].Diff == "" {
+			t.Error("expected Diff to be generated")
+		}
+		if mf.Mutations[0].Type != streams.MutationPatch {
+			t.Errorf("expected type to remain patch, got %q", mf.Mutations[0].Type)
+		}
+		if !strings.Contains(mf.Mutations[0].Diff, "-old content") {
+			t.Error("expected diff to contain removed old content")
+		}
+		if !strings.Contains(mf.Mutations[0].Diff, "+new content") {
+			t.Error("expected diff to contain added new content")
+		}
+	})
+
+	t.Run("sets create mutation when only newText", func(t *testing.T) {
+		mf := &streams.ModifyFilePayload{
+			FilePath:  "",
+			Mutations: []streams.FileMutation{{Type: streams.MutationPatch}},
+		}
+		contents := []acp.ToolCallContent{
+			{Diff: &acp.ToolCallContentDiff{Path: "file.ts", NewText: "new file content"}},
+		}
+		enrichModifyFileFromContents(mf, contents)
+		if mf.Mutations[0].Type != streams.MutationCreate {
+			t.Errorf("expected type 'create', got %q", mf.Mutations[0].Type)
+		}
+		if mf.Mutations[0].Content != "new file content" {
+			t.Errorf("expected content 'new file content', got %q", mf.Mutations[0].Content)
+		}
+	})
+
+	t.Run("does not overwrite existing file path", func(t *testing.T) {
+		mf := &streams.ModifyFilePayload{
+			FilePath:  "existing.ts",
+			Mutations: []streams.FileMutation{{Type: streams.MutationPatch}},
+		}
+		contents := []acp.ToolCallContent{
+			{Diff: &acp.ToolCallContentDiff{Path: "other.ts", NewText: "content"}},
+		}
+		enrichModifyFileFromContents(mf, contents)
+		if mf.FilePath != "existing.ts" {
+			t.Errorf("expected FilePath to remain 'existing.ts', got %q", mf.FilePath)
+		}
+	})
+
+	t.Run("does not overwrite existing diff", func(t *testing.T) {
+		mf := &streams.ModifyFilePayload{
+			FilePath:  "file.ts",
+			Mutations: []streams.FileMutation{{Type: streams.MutationPatch, Diff: "existing diff"}},
+		}
+		contents := []acp.ToolCallContent{
+			{Diff: &acp.ToolCallContentDiff{Path: "file.ts", OldText: &oldText, NewText: "new"}},
+		}
+		enrichModifyFileFromContents(mf, contents)
+		if mf.Mutations[0].Diff != "existing diff" {
+			t.Errorf("expected Diff to remain 'existing diff', got %q", mf.Mutations[0].Diff)
+		}
+	})
+
+	t.Run("handles empty content gracefully", func(t *testing.T) {
+		mf := &streams.ModifyFilePayload{
+			FilePath:  "",
+			Mutations: []streams.FileMutation{{Type: streams.MutationPatch}},
+		}
+		enrichModifyFileFromContents(mf, nil)
+		enrichModifyFileFromContents(mf, []acp.ToolCallContent{})
+		if mf.FilePath != "" {
+			t.Errorf("expected FilePath to remain empty, got %q", mf.FilePath)
+		}
+	})
+}
+
+// TestExtractPathFromTitle tests file path extraction from tool titles.
+func TestExtractPathFromTitle(t *testing.T) {
+	tests := []struct {
+		title string
+		want  string
+	}{
+		{"Read /path/to/file.ts", "/path/to/file.ts"},
+		{"Write /path/to/file.ts", "/path/to/file.ts"},
+		{"Edit /path/to/file.ts", "/path/to/file.ts"},
+		{"Unknown /path/to/file.ts", ""},
+		{"", ""},
+		{"Read", ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.title, func(t *testing.T) {
+			got := extractPathFromTitle(tt.title)
+			if got != tt.want {
+				t.Errorf("extractPathFromTitle(%q) = %q, want %q", tt.title, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestUpdatePayloadInput_ModifyFile tests incremental rawInput updates for modify_file payloads.
+func TestUpdatePayloadInput_ModifyFile(t *testing.T) {
+	normalizer := NewNormalizer()
+
+	t.Run("updates empty file path from rawInput", func(t *testing.T) {
+		payload := normalizer.NormalizeToolCall("edit", map[string]any{
+			"kind":      "edit",
+			"raw_input": map[string]any{},
+		})
+		if payload.ModifyFile().FilePath != "" {
+			t.Errorf("expected empty initial FilePath, got %q", payload.ModifyFile().FilePath)
+		}
+		normalizer.UpdatePayloadInput(payload, map[string]any{
+			"file_path": "/workspace/file.ts",
+		})
+		if payload.ModifyFile().FilePath != "/workspace/file.ts" {
+			t.Errorf("expected FilePath '/workspace/file.ts', got %q", payload.ModifyFile().FilePath)
+		}
+	})
+
+	t.Run("does not overwrite existing file path", func(t *testing.T) {
+		payload := normalizer.NormalizeToolCall("edit", map[string]any{
+			"kind": "edit",
+			"raw_input": map[string]any{
+				"path": "existing.ts",
+			},
+		})
+		normalizer.UpdatePayloadInput(payload, map[string]any{
+			"file_path": "other.ts",
+		})
+		if payload.ModifyFile().FilePath != "existing.ts" {
+			t.Errorf("expected FilePath to remain 'existing.ts', got %q", payload.ModifyFile().FilePath)
+		}
+	})
+}
+
+// TestUpdatePayloadInput_ReadFile tests incremental rawInput updates for read_file payloads.
+func TestUpdatePayloadInput_ReadFile(t *testing.T) {
+	normalizer := NewNormalizer()
+
+	t.Run("updates empty file path from rawInput", func(t *testing.T) {
+		payload := normalizer.NormalizeToolCall("read", map[string]any{
+			"kind":      "read",
+			"raw_input": map[string]any{},
+		})
+		if payload.ReadFile().FilePath != "" {
+			t.Errorf("expected empty initial FilePath, got %q", payload.ReadFile().FilePath)
+		}
+		normalizer.UpdatePayloadInput(payload, map[string]any{
+			"file_path": "/workspace/README.md",
+		})
+		if payload.ReadFile().FilePath != "/workspace/README.md" {
+			t.Errorf("expected FilePath '/workspace/README.md', got %q", payload.ReadFile().FilePath)
+		}
+	})
+
+	t.Run("does not overwrite existing file path", func(t *testing.T) {
+		payload := normalizer.NormalizeToolCall("read", map[string]any{
+			"kind": "read",
+			"raw_input": map[string]any{
+				"path": "existing.md",
+			},
+		})
+		normalizer.UpdatePayloadInput(payload, map[string]any{
+			"file_path": "other.md",
+		})
+		if payload.ReadFile().FilePath != "existing.md" {
+			t.Errorf("expected FilePath to remain 'existing.md', got %q", payload.ReadFile().FilePath)
+		}
+	})
+}
+
 // TestSplitLines tests the line splitting utility.
 func TestSplitLines(t *testing.T) {
 	tests := []struct {

apps/backend/internal/agentctl/server/config/config.go

@@ -304,11 +304,13 @@ func ParseCommand(cmd string) []string {
 func CollectAgentEnv(additional map[string]string) []string {
 	envMap := make(map[string]string)
 
-	// Start with current environment, excluding AGENTCTL_* vars
+	// Start with current environment, excluding AGENTCTL_* and npm_config_* vars.
+	// npm_config_* vars from the parent pnpm process cause "Unknown env config"
+	// warnings when the agent is launched via npx.
 	for _, e := range os.Environ() {
 		if idx := strings.Index(e, "="); idx > 0 {
 			key := e[:idx]
-			if !strings.HasPrefix(key, "AGENTCTL_") {
+			if !strings.HasPrefix(key, "AGENTCTL_") && !isNpmEnvVar(key) {
 				envMap[key] = e[idx+1:]
 			}
 		}
@@ -327,6 +329,16 @@ func CollectAgentEnv(additional map[string]string) []string {
 	return result
 }
 
+// isNpmEnvVar returns true if the key is an npm-related environment variable
+// that should be filtered out to prevent warnings in npx commands.
+func isNpmEnvVar(key string) bool {
+	return strings.HasPrefix(key, "npm_config_") ||
+		strings.HasPrefix(key, "npm_package_") ||
+		strings.HasPrefix(key, "npm_lifecycle_") ||
+		strings.HasPrefix(key, "npm_execpath") ||
+		strings.HasPrefix(key, "npm_node_execpath")
+}
+
 func getEnv(key, defaultValue string) string {
 	if value := os.Getenv(key); value != "" {
 		return value

apps/web/components/task/model-selector.tsx

@@ -96,6 +96,43 @@ function resolveCurrentModel(
   return activeModel || acpCurrentModel || snapshotModel || profileModel;
 }
 
+function logModelSelectorState(
+  sessionId: string | null,
+  state: {
+    usingAcpModels: boolean;
+    sessionModelsData:
+      | { models?: { modelId: string }[]; currentModelId?: string; configOptions?: unknown[] }
+      | undefined;
+    activeModel: string | null;
+    acpCurrentModel: string | null;
+    snapshotModel: string | null;
+    profileModel: string | null;
+    currentModel: string | null;
+    modelOptions: ModelOption[];
+  },
+) {
+  if (!sessionId) return;
+  console.log("[model-selector] state resolved", {
+    sessionId,
+    usingAcpModels: state.usingAcpModels,
+    sessionModelsData: state.sessionModelsData
+      ? {
+          modelsCount: state.sessionModelsData.models?.length ?? 0,
+          models: state.sessionModelsData.models?.map((m) => m.modelId),
+          currentModelId: state.sessionModelsData.currentModelId,
+          configOptionsCount: state.sessionModelsData.configOptions?.length ?? 0,
+        }
+      : null,
+    activeModel: state.activeModel,
+    acpCurrentModel: state.acpCurrentModel,
+    snapshotModel: state.snapshotModel,
+    profileModel: state.profileModel,
+    resolvedCurrentModel: state.currentModel,
+    modelOptionsCount: state.modelOptions.length,
+    modelOptions: state.modelOptions.map((m) => ({ id: m.id, name: m.name })),
+  });
+}
+
 /** Resolves available models and current model from store state. */
 function useModelSelectorState(sessionId: string | null) {
   useSettingsData(true);
@@ -116,7 +153,8 @@ function useModelSelectorState(sessionId: string | null) {
     [session?.agent_profile_id, settingsAgents],
   );
 
-  const availableModels = sessionModelsData?.models?.length
+  const usingAcpModels = !!sessionModelsData?.models?.length;
+  const availableModels = usingAcpModels
     ? sessionModelsToOptions(sessionModelsData.models)
     : resolveStaticModels(settingsAgents as Agent[], session?.agent_profile_id, availableAgents);
 
@@ -130,6 +168,17 @@ function useModelSelectorState(sessionId: string | null) {
   );
   const modelOptions = buildModelOptions(availableModels, currentModel);
 
+  logModelSelectorState(sessionId, {
+    usingAcpModels,
+    sessionModelsData,
+    activeModel,
+    acpCurrentModel,
+    snapshotModel,
+    profileModel,
+    currentModel,
+    modelOptions,
+  });
+
   const handleModelChange = useCallback(
     (sid: string, modelId: string) => {
       setActiveModel(sid, modelId);

apps/web/lib/diff/adapter.ts

@@ -62,16 +62,42 @@ export function normalizeDiffString(diff: string, filePath: string): string {
  * The DiffViewer component handles diff generation from content using the library.
  */
 export function transformFileMutation(filePath: string, mutation: FileMutation): FileDiffData {
+  const resolvedPath = mutation.new_path || filePath;
   return {
-    filePath: mutation.new_path || filePath,
+    filePath: resolvedPath,
     oldContent: mutation.old_content || "",
     newContent: mutation.new_content || mutation.content || "",
-    diff: mutation.diff ? normalizeDiffString(mutation.diff, filePath) : undefined,
+    diff: resolveMutationDiff(mutation, resolvedPath),
     additions: 0,
     deletions: 0,
   };
 }
 
+/** Resolve the diff string for a mutation: explicit diff > create diff > undefined. */
+function resolveMutationDiff(mutation: FileMutation, filePath: string): string | undefined {
+  if (mutation.diff) return normalizeDiffString(mutation.diff, filePath);
+  if (mutation.type === "create" && mutation.content)
+    return generateCreateDiff(mutation.content, filePath);
+  return undefined;
+}
+
+/**
+ * Generate an all-additions diff for new file creation.
+ * Shows the entire content as green (added) lines in the diff viewer.
+ */
+function generateCreateDiff(content: string, filePath: string): string {
+  const lines = content.split("\n");
+  const header = [
+    `diff --git a/${filePath} b/${filePath}`,
+    `new file mode 100644`,
+    `--- /dev/null`,
+    `+++ b/${filePath}`,
+    `@@ -0,0 +1,${lines.length} @@`,
+  ];
+  const addedLines = lines.map((l) => `+${l}`);
+  return [...header, ...addedLines].join("\n");
+}
+
 /**
  * Transform a git status diff string to FileDiffData.
  * Language detection is handled automatically by the library.

apps/web/lib/state/slices/session-runtime/session-runtime-slice.ts

@@ -250,6 +250,14 @@ export const createSessionRuntimeSlice: StateCreator<
     }),
   setSessionModels: (sessionId, data) =>
     set((draft) => {
+      console.log("[store] setSessionModels", {
+        sessionId,
+        modelsCount: data.models?.length ?? 0,
+        models: data.models?.map((m) => m.modelId),
+        currentModelId: data.currentModelId,
+        configOptionsCount: data.configOptions?.length ?? 0,
+        configOptions: data.configOptions?.map((o) => ({ id: o.id, category: o.category })),
+      });
       draft.sessionModels.bySessionId[sessionId] = data;
     }),
   setPromptUsage: (sessionId, usage) =>

apps/web/lib/ws/handlers/session-models.ts

@@ -22,11 +22,19 @@ export function registerSessionModelsHandlers(store: StoreApi<AppState>): WsHand
           currentModelId = modelOpt.current_value;
         }
       }
-      console.debug("[session-models] received session.models_updated", {
+      console.log("[session-models] received session.models_updated", {
         sessionId: payload.session_id,
         rawCurrentModelId: payload.current_model_id,
         resolvedCurrentModelId: currentModelId,
-        models: acpModels.map((m) => m.model_id),
+        modelsCount: acpModels.length,
+        models: acpModels.map((m) => ({ id: m.model_id, name: m.name })),
+        configOptions: (payload.config_options ?? []).map((o) => ({
+          id: o.id,
+          category: o.category,
+          currentValue: o.current_value,
+          options: o.options?.map((opt: { value: string; name: string }) => opt.value),
+        })),
+        rawPayload: payload,
       });
       store.getState().setSessionModels(payload.session_id, {
         currentModelId,
@@ -53,7 +61,7 @@ export function registerSessionModelsHandlers(store: StoreApi<AppState>): WsHand
         const state = store.getState();
         const currentActive = state.activeModel.bySessionId[payload.session_id];
         if (currentActive && !acpModels.some((m) => m.model_id === currentActive)) {
-          console.debug("[session-models] clearing stale activeModel", {
+          console.log("[session-models] clearing stale activeModel", {
             sessionId: payload.session_id,
             staleModel: currentActive,
           });