From 01e4917df1bbfe30fdd5eb110556356d476fce3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Thu, 4 Jun 2026 03:16:39 +0100 Subject: [PATCH] fix(utility): use claude-acp as default inference agent id (#1271) * fix(utility): use claude-acp as default inference agent id The utility-agent SQLite schema, frontend dialog defaults, and edit fallback all pointed at the legacy "claude-code" identifier, but the only registered Claude inference agent is "claude-acp" (since the ACP migration in #566). Opening Settings > Utility Agents and picking Claude resolved to an unknown agent id, so the Model dropdown rendered "no model found" and the agent could not be configured. Switch the defaults to "claude-acp" and migrate existing rows so previously-seeded built-ins (which were saved with agent_id="" and later coerced to "claude-code" by the dialog fallback) now populate their model list correctly. Closes #1269 * fix(utility): cover empty agent_id seeds and add migration test Address PR review feedback on #1271: - Extend the agent_id backfill migration to also rewrite rows seeded with an empty string. seedBuiltinAgents() inserts built-in rows with ON CONFLICT(id) DO NOTHING, so older deployments that landed agent_id="" never got repaired by re-seeding. - Set AgentID = "claude-acp" explicitly in getBuiltinAgents() so future first-run seeds match the schema default without depending on the dialog fallback or the backfill migration. - Add sqlite_migration_test.go covering both the legacy "claude-code" and empty-string backfill paths, plus a seed-contract assertion that every built-in row references "claude-acp" on first run. * fix(utility): clarify migration comment and check rows.Err in test Address PR review feedback (round 2) on #1271: - Reword the agent_id backfill comment as a legacy-data backfill note and point at builtinSeedAgentID as the current seed path, so a future reader doesn't chase the old "seeds insert agent_id=''" behavior that this PR already fixed. - Add rowsxDB.Err() check after the seed iteration loop, before the count guard, so a mid-scan error fails the test instead of silently shrinking the row count. --------- Co-authored-by: Kandev Agent --- .../backend/internal/utility/models/models.go | 4 +- .../internal/utility/service/service.go | 2 +- .../internal/utility/store/builtins.go | 6 + apps/backend/internal/utility/store/sqlite.go | 13 +- .../utility/store/sqlite_migration_test.go | 119 ++++++++++++++++++ .../settings/utility-agent-dialog.tsx | 4 +- 6 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 apps/backend/internal/utility/store/sqlite_migration_test.go diff --git a/apps/backend/internal/utility/models/models.go b/apps/backend/internal/utility/models/models.go index 6d8bc22fb2..5fa4c9e136 100644 --- a/apps/backend/internal/utility/models/models.go +++ b/apps/backend/internal/utility/models/models.go @@ -3,13 +3,13 @@ package models import "time" // UtilityAgent represents a configured utility agent for quick one-shot tasks. -// It references an inference-capable agent (like claude-code, amp) by ID. +// It references an inference-capable agent (like claude-acp, amp) by ID. type UtilityAgent struct { ID string `json:"id" db:"id"` Name string `json:"name" db:"name"` Description string `json:"description" db:"description"` Prompt string `json:"prompt" db:"prompt"` - AgentID string `json:"agent_id" db:"agent_id"` // Reference to inference agent (e.g., "claude-code") + AgentID string `json:"agent_id" db:"agent_id"` // Reference to inference agent (e.g., "claude-acp") Model string `json:"model" db:"model"` // Model to use (e.g., "claude-haiku-4-5") Builtin bool `json:"builtin" db:"builtin"` // Built-in agents cannot be deleted Enabled bool `json:"enabled" db:"enabled"` // Whether agent is enabled diff --git a/apps/backend/internal/utility/service/service.go b/apps/backend/internal/utility/service/service.go index 75ea71bc6a..cb2c2e6853 100644 --- a/apps/backend/internal/utility/service/service.go +++ b/apps/backend/internal/utility/service/service.go @@ -211,7 +211,7 @@ func (s *Service) PreparePromptRequest(ctx context.Context, utilityID string, tm type PromptRequest struct { UtilityID string ResolvedPrompt string - AgentCLI string // The inference agent ID (e.g., "claude-code", "amp") + AgentCLI string // The inference agent ID (e.g., "claude-acp", "amp") Model string // The model to use } diff --git a/apps/backend/internal/utility/store/builtins.go b/apps/backend/internal/utility/store/builtins.go index 5c9070f38d..f632c8fe49 100644 --- a/apps/backend/internal/utility/store/builtins.go +++ b/apps/backend/internal/utility/store/builtins.go @@ -40,6 +40,11 @@ var builtinDefs = []builtinDef{ {"builtin-summarize-session", "summarize-session", "Summarize a session conversation for context handover", "summarize-session"}, } +// builtinSeedAgentID is the inference-agent ID embedded in seeded built-in +// rows. Kept aligned with the schema DEFAULT so a row that survives every +// migration path still maps to a registered inference agent. +const builtinSeedAgentID = "claude-acp" + // getBuiltinAgents returns the predefined built-in utility agents with prompts from embedded files. func getBuiltinAgents() []*models.UtilityAgent { now := time.Now().UTC() @@ -50,6 +55,7 @@ func getBuiltinAgents() []*models.UtilityAgent { Name: d.name, Description: d.description, Prompt: utilcfg.Get(d.promptFile), + AgentID: builtinSeedAgentID, Builtin: true, Enabled: false, CreatedAt: now, diff --git a/apps/backend/internal/utility/store/sqlite.go b/apps/backend/internal/utility/store/sqlite.go index 112950f043..6241cadb91 100644 --- a/apps/backend/internal/utility/store/sqlite.go +++ b/apps/backend/internal/utility/store/sqlite.go @@ -47,7 +47,7 @@ func (r *sqliteRepository) initSchema() error { name TEXT NOT NULL UNIQUE, description TEXT NOT NULL DEFAULT '', prompt TEXT NOT NULL, - agent_id TEXT NOT NULL DEFAULT 'claude-code', + agent_id TEXT NOT NULL DEFAULT 'claude-acp', model TEXT NOT NULL DEFAULT '', builtin INTEGER NOT NULL DEFAULT 0, enabled INTEGER NOT NULL DEFAULT 1, @@ -99,6 +99,17 @@ func (r *sqliteRepository) initSchema() error { AND agent_id <> '' AND model <> '' AND updated_at = created_at`) + // Legacy-data backfill: rewrite the old "claude-code" identifier and any + // pre-fix builtin rows that were persisted with an empty agent_id to the + // registered inference agent ID "claude-acp". The Claude agent was + // renamed during the ACP migration (#566), and seedBuiltinAgents() + // historically inserted rows with agent_id = "" — combined with its + // ON CONFLICT(id) DO NOTHING contract, those stale rows survived + // untouched on every restart. Current seeds now use builtinSeedAgentID + // (see builtins.go), so this migration only repairs existing deployments. + _, _ = r.db.Exec(`UPDATE utility_agents SET agent_id = 'claude-acp' + WHERE agent_id IN ('claude-code', '')`) + // Seed built-in agents if err := r.seedBuiltinAgents(); err != nil { return fmt.Errorf("failed to seed built-in agents: %w", err) diff --git a/apps/backend/internal/utility/store/sqlite_migration_test.go b/apps/backend/internal/utility/store/sqlite_migration_test.go new file mode 100644 index 0000000000..21e8a135a1 --- /dev/null +++ b/apps/backend/internal/utility/store/sqlite_migration_test.go @@ -0,0 +1,119 @@ +package store + +import ( + "context" + "path/filepath" + "testing" + "time" + + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/db" +) + +// openTestDB opens a fresh SQLite file under t.TempDir and registers cleanup. +func openTestDB(t *testing.T) *sqlx.DB { + t.Helper() + tmpDir := t.TempDir() + conn, err := db.OpenSQLite(filepath.Join(tmpDir, "test.db")) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + sqlxDB := sqlx.NewDb(conn, "sqlite3") + t.Cleanup(func() { + _ = sqlxDB.Close() + }) + return sqlxDB +} + +// TestInitSchema_MigratesLegacyClaudeCodeAgentID guards against the #1269 +// regression: a stale "claude-code" agent_id (and the empty agent_id seeded +// into pre-fix built-in rows) must be rewritten to the registered inference +// agent ID "claude-acp" on schema init, so the utility-agent dialog's model +// dropdown can resolve to a valid agent. +func TestInitSchema_MigratesLegacyClaudeCodeAgentID(t *testing.T) { + sqlxDB := openTestDB(t) + + // Bootstrap the schema once so we can seed legacy rows directly. + if _, err := newSQLiteRepositoryWithDB(sqlxDB, sqlxDB); err != nil { + t.Fatalf("initial schema init: %v", err) + } + + now := time.Now().UTC() + rows := []struct { + id, agentID string + }{ + {"legacy-claude-code", "claude-code"}, + {"legacy-empty", ""}, + {"untouched-amp", "amp"}, + } + for _, r := range rows { + if _, err := sqlxDB.Exec(`INSERT OR REPLACE INTO utility_agents + (id, name, description, prompt, agent_id, model, builtin, enabled, created_at, updated_at) + VALUES (?, ?, '', '', ?, '', 0, 1, ?, ?)`, + r.id, r.id, r.agentID, now, now); err != nil { + t.Fatalf("seed %s: %v", r.id, err) + } + } + + // Re-run schema init — this fires the agent_id backfill migration. + if _, err := newSQLiteRepositoryWithDB(sqlxDB, sqlxDB); err != nil { + t.Fatalf("re-init schema: %v", err) + } + + want := map[string]string{ + "legacy-claude-code": "claude-acp", + "legacy-empty": "claude-acp", + "untouched-amp": "amp", + } + for id, expected := range want { + var got string + if err := sqlxDB.QueryRowx(`SELECT agent_id FROM utility_agents WHERE id = ?`, id). + Scan(&got); err != nil { + t.Fatalf("read %s: %v", id, err) + } + if got != expected { + t.Errorf("%s: agent_id = %q, want %q", id, got, expected) + } + } +} + +// TestSeedBuiltinAgents_UsesClaudeACP pins the seed contract: every built-in +// row inserted on first run already references the registered inference +// agent, with no reliance on the dialog fallback or the backfill migration. +func TestSeedBuiltinAgents_UsesClaudeACP(t *testing.T) { + sqlxDB := openTestDB(t) + + if _, err := newSQLiteRepositoryWithDB(sqlxDB, sqlxDB); err != nil { + t.Fatalf("schema init: %v", err) + } + + ctx := context.Background() + rowsxDB, err := sqlxDB.QueryxContext(ctx, `SELECT id, agent_id FROM utility_agents WHERE builtin = 1`) + if err != nil { + t.Fatalf("query builtins: %v", err) + } + t.Cleanup(func() { + if err := rowsxDB.Close(); err != nil { + t.Errorf("close rows: %v", err) + } + }) + + var count int + for rowsxDB.Next() { + var id, agentID string + if err := rowsxDB.Scan(&id, &agentID); err != nil { + t.Fatalf("scan: %v", err) + } + count++ + if agentID != "claude-acp" { + t.Errorf("builtin %s: agent_id = %q, want %q", id, agentID, "claude-acp") + } + } + if err := rowsxDB.Err(); err != nil { + t.Fatalf("rows iteration error: %v", err) + } + if count == 0 { + t.Fatalf("expected at least one built-in row, found 0") + } +} diff --git a/apps/web/components/settings/utility-agent-dialog.tsx b/apps/web/components/settings/utility-agent-dialog.tsx index f2cc4c132f..ae384c692b 100644 --- a/apps/web/components/settings/utility-agent-dialog.tsx +++ b/apps/web/components/settings/utility-agent-dialog.tsx @@ -39,7 +39,7 @@ const defaultFormState: FormState = { name: "", description: "", prompt: "", - agent_id: "claude-code", + agent_id: "claude-acp", model: "", }; @@ -209,7 +209,7 @@ export function UtilityAgentDialog({ open, onOpenChange, agent, onSuccess }: Pro name: agent.name, description: agent.description, prompt: agent.prompt, - agent_id: agent.agent_id || "claude-code", + agent_id: agent.agent_id || "claude-acp", model: agent.model || "", }); } else {