[BUG] Duplicate tool row permanently bricks tool-server reconciliation (tool_pkey 23505) with a misleading error

[shreyasskasetty]

Note: This issue was originally filed suspecting a concurrency race in RefreshToolsForServer. On closer inspection of the actual row data, the evidence points to PostgreSQL index corruption, not a code-level race — the current code path is concurrency-safe. The report has been rewritten accordingly. The remaining actionable parts are a misleading error message and the lack of self-recovery.

🎯 Affected Service(s)

Controller Service

🚦 Impact/Severity

Minor inconvenience — but note: while it lasts, the affected tool server is fully stuck (Accepted: False, stale tool list). There is a manual workaround, and the trigger appears to be DB-level index corruption rather than a code defect.

🐛 Bug Description

If the tool table's unique index (tool_pkey, PRIMARY KEY (id, server_name, group_kind)) ever becomes inconsistent with the heap — i.e. two physical rows end up sharing one primary key — then every subsequent reconcile of that tool server fails deterministically:

failed to refresh tools for toolServer <ns>/<name>: failed to delete existing tools:
ERROR: duplicate key value violates unique constraint "tool_pkey" (SQLSTATE 23505)

The RemoteMCPServer is pinned at Accepted: False forever, its tool list goes stale, and the controller retries every 60s indefinitely with no self-recovery. The only fix is manual DB surgery (DELETE the duplicate + REINDEX).

Two things make this worse than it needs to be:

1. Misleading error. The failure is wrapped as "failed to delete existing tools" (go/core/internal/database/client_postgres.go:477), but the real error is a unique-constraint violation surfaced by the soft-delete UPDATE while it re-indexes pre-existing duplicate rows. The label points at the wrong operation and made this hard to diagnose.
2. No recovery path. A single bad row bricks the tool server permanently; the controller never heals it and never surfaces a clearer condition telling an operator what to do.

🔎 Root Cause Analysis (what the evidence actually shows)

Observed duplicate rows on an affected cluster:

 ctid   | id                    | deleted_at                    | created_at                    | updated_at
--------+-----------------------+-------------------------------+-------------------------------+------------------------------
 (1,26) | k8s_get_resource_yaml |                               | 2026-07-29 01:33:08.603063+00 | 2026-07-29 01:33:08.603063+00
(11,35) | k8s_get_resource_yaml | 2026-07-29 01:33:08.603063+00 | 2026-07-18 02:07:08.931972+00 | 2026-07-29 01:32:38.754744+00

Three observations point to index corruption, not a concurrency race:

• Same transaction, not two racing ones. Row (1,26)'s created_at and row (11,35)'s deleted_at are byte-identical to the microsecond (01:33:08.603063). NOW() in Postgres is the transaction start time and is constant within a transaction; both the soft-delete (deleted_at = NOW()) and the insert (created_at = NOW()) use NOW(). Identical values ⇒ one RefreshToolsForServer transaction did both the soft-delete and the duplicate insert. A two-transaction race would show different timestamps.
• REINDEX was required to fix it. Deleting the duplicate row alone wasn't the whole cure; the index itself was inconsistent (index-scan count(*) returned 2). REINDEX is the remedy for a corrupted index, not for clean duplicate data.
• The code path is concurrency-safe. RefreshToolsForServer does a soft-delete UPDATE followed by per-tool INSERT ... ON CONFLICT (id, server_name, group_kind) DO UPDATE against the full composite PK. Across all interleavings this cannot create a duplicate — ON CONFLICT always resolves to an update of the existing row. For a single transaction to instead insert a new row, the unique index must have already been missing the existing row's entry (i.e. already corrupt).

Most likely trigger: a Postgres instance that suffered an unclean shutdown / restart (this was seen on a local kind/Docker cluster that had been restarted many times), leaving tool_pkey inconsistent. Not a kagent logic bug.

🔄 Steps To Reproduce

Not reliably reproducible from application code (the refresh path is concurrency-safe). The deterministic failure can be demonstrated by seeding an inconsistent index — once two rows share a tool_pkey, any RefreshToolsForServer for that server fails as above and the server stays Accepted: False.

🤔 Expected Behavior

• The error should name the real failure (a unique-constraint violation on tool_pkey), not "failed to delete existing tools".
• Ideally, a single corrupt/duplicate row should not permanently brick a tool server with no operator-visible guidance or recovery.

📱 Actual Behavior

• RemoteMCPServer stuck Accepted: False indefinitely; tool list frozen.
• Every reconcile fails every 60s with a mislabeled error.

💻 Environment

• Kubernetes provider: kind (local dev)
• Controller version: v0.0.0-3fb5a9d6 (current main)
• Database: PostgreSQL (kagent-postgresql)

🔍 Additional Context

Relevant code:

• go/core/internal/database/client_postgres.go:472 — RefreshToolsForServer (soft-delete + per-tool ON CONFLICT upsert, one txn)
• go/core/internal/database/client_postgres.go:477 — the misleading "failed to delete existing tools" wrap
• go/core/internal/controller/reconciler/reconciler.go:1139 — the only caller, via upsertToolServerForRemoteMCPServer (reached from the RemoteMCPServer, MCPServer, and Service reconcile paths)
• go/core/pkg/migrations/core/000001_initial.up.sql:93 — PRIMARY KEY (id, server_name, group_kind)

Possible small improvements (open to maintainer opinion):

1. Fix the error wrapping so a tool_pkey unique violation is reported as such, not as a delete failure.
2. Optionally, detect a duplicate/corrupt-row condition during refresh and either self-heal or surface a clear Accepted: False reason so operators know to REINDEX.

(Note: adding an application/DB lock to "serialize refreshes" was considered and dropped — the refresh is already concurrency-safe, so a lock would not address the observed index corruption.)

Workaround (if you hit this): delete the duplicate row by ctid and rebuild the index:

-- find the duplicate(s) via a heap scan (bypass the corrupt index):
SET enable_indexscan=off; SET enable_bitmapscan=off;
SELECT ctid, id, server_name, group_kind, deleted_at FROM tool
GROUP BY ctid, id, server_name, group_kind, deleted_at;  -- inspect for repeats
-- then remove the extra physical row and rebuild:
DELETE FROM tool WHERE ctid = '<duplicate-row-ctid>';
REINDEX TABLE tool;

The controller recovers on the next reconcile (~60s).

📋 Logs

{"level":"error","ts":"...","logger":"reconciler","msg":"failed to upsert tool server for remote mcp server","remoteMCPServer":"kagent/kagent-tool-server","error":"failed to refresh tools for toolServer kagent/kagent-tool-server: failed to delete existing tools: ERROR: duplicate key value violates unique constraint \"tool_pkey\" (SQLSTATE 23505)","stacktrace":"...reconciler.go:651..."}

🙋 Are you willing to contribute?

Yes — happy to submit the small error-message improvement (and discuss whether any recovery/detection is wanted).