refactor: simplify database client operations - #2774

…2742)

Rebased onto main at `214b5568`, including #2774’s atomic
`RecordRuntimeRevision` operation and #2780’s generic database error
categories. This PR keeps that operation atomic and acquires pair locks
before revision writes to preserve GC lock ordering.

Deleting or replacing a prepared AgentTemplate left a retired pair
pointing at its last successful revision. GC deleted compute and then
hit a RESTRICT foreign key; it also missed checkpoint references and
raced with new instance references. A single failed deletion could fail
unrelated pair reconciles.

Run GC as a separate leader-elected manager runnable. Controller leader
election defaults to enabled even with one replica because rolling
updates overlap controllers; `LEADER_ELECT=false` is available for local
testing. Helm relies on that default and always grants lease
permissions. GC sweeps on startup and every minute, logs each failed
candidate, continues collecting other revisions, and retries durable
deletions without template events. Database discovery and each candidate
have one-minute deadlines.

Pair reconciliation only prepares runtimes and manages references. It
exclusively owns KRT readiness observations, keyed by pair and tagged
with their revision; changed inputs ignore stale observations, and a
fresh reconciler can clean them up without remembering previous calls.
When a desired digest is being deleted, clearing its observation makes
KRT derive a pending pair. The existing pending-template poll retries
that state outside the error retry budget. There are no private retry or
observation maps, and GC has no KRT dependency or callback.

Begin deletion durably before touching Substrate. Explicit row locks in
the store transactions serialize pair, instance, and checkpoint
acquisition with deletion: an acquisition that wins prevents collection;
deletion that wins rejects new references with ErrObjectDeleting. Pair
writes check both the desired and retained last-successful revision
before committing. Ordinary and scheduled instance creation acquire the
revision lock in their shared insertion helper. There are no reference
triggers or custom SQL exceptions. Lock the revision first, then check
the shared eligibility view in a separate statement so references
committed during the lock wait are visible. No database lock spans a
network call.

Retain revision and ActorTemplate identity until runtime cleanup
succeeds. Finalization atomically releases retired success pointers and
deletes the row, checks the ActorTemplate UID against stale collectors,
and preserves pointers when skipped. Operations touching both pairs and
revisions lock pairs first to avoid a finalization/reactivation
deadlock. Preserve the current UID pair's last-good revision across
invalid edits, retire replaced identities, and publish Ready only after
the successful revision is persisted.

Queues use ten attempts with exponential backoff from one to thirty
seconds. Pending preparation still polls, but a removed pair whose
retirement exhausts its retries is no longer discoverable from the
graph; a restart can also lose that event. Operator repair is required
until the system doctor in #2768 compares persisted identities with the
synced Kubernetes graph. Leader election reduces overlapping collectors,
but full stale-request fencing still requires Substrate to support UID
preconditions on deletion.

The schema is unreleased, so the requested changes are folded into
migration 1. Existing RESTRICT foreign keys remain. The store uses
main's inline pgx queries and transaction helpers; no sqlc queries or
generated accessors are restored. The Migration Immutability workflow
rejects this baseline rewrite; no workflow bypass is included.

Validation:

- PostgreSQL coverage: retirement, instance/checkpoint retention, both
concurrent acquisition/deletion orderings for instances, new pairs, and
reactivated pairs, both finalization/reactivation and
finalization/recording lock orderings, skipped finalization, restart
recovery, and stale collector UID protection.
- Controller coverage: startup and periodic sweeps, list/delete
failures, healthy candidates behind failed or blocked cleanup,
cancellation, real KRT Ready-to-pending transitions, fresh-reconciler
observation replacement and cleanup, bounded backoff, preparation
retries during deletion, readiness recovery, and fresh-collector
recovery after failed compute deletion, a lost response, or successful
compute deletion followed by failed database finalization.
- Database/controller/app/env tests, database/controller/migration race
checks, and full Go lint pass. PostgreSQL coverage includes preparation
of every inline SQL statement against the migrated schema. Baseline
Up/Down, related service/Substrate/gRPC checks, and E2E compilation also
pass.
- All 279 Helm tests pass, including single-replica lease RBAC. App
tests cover default-enabled election and the local opt-out.
- Real Substrate lifecycle E2E validates immediate instance creation
after Ready, last-good invocation, checkpoint retention, eventual
ActorTemplate and golden-actor cleanup after the final checkpoint is
deleted with no subsequent template change, and recreation/invocation
under a new template UID. Before the KRT-ownership follow-up, the
strengthened test passed in 85 seconds while the preserved `runsc state`
exit-128 deletion failed on each independent sweep; unrelated pair
reconciles no longer reported that GC error.

Live Substrate E2E was not rerun after the KRT changes or this rebase:
the dedicated test cluster was removed before rollout.

GC metrics remain in #2744; failed or abandoned checkpoint recovery
remains in #2746. Sweeps are serial; bounded workers can be added if
slow deletions cause a material backlog.

---------

Signed-off-by: Eitan Yarmush <[email protected]>

…f the catalog

main honors the label nowhere: kagent-dev#2565 removed
predicates.DiscoveryDisabledPredicate together with the old MCPServer
tool controller, and the catalog reconciler restored in kagent-dev#2644 watches
every kmcp MCPServer unfiltered, so the opt-out the docs describe for
the agentgateway setup was a no-op on main.

The MCPServer catalog reconciler now skips discovery for a labeled
server and drops its catalog projection. That is what the 0.10.x
predicate achieved, minus two stale-row cases: a label added after
discovery left the row in place, and the deletion of a labeled server
never removed it. The controller filters no events, so a label change
re-enters Reconcile at once.

A RemoteMCPServer stays in the catalog with the DiscoveryDisabled
status on purpose: it is kagent's own CR that agents reference. An
MCPServer is kmcp's resource that kagent only projects, has no kagent
status to explain an empty row, and AgentTemplates cannot reference it
directly.

Rebased onto main after kagent-dev#2774 (simplify database client operations):
the reconciler's deleteCatalog is gone, the catalog store's
DeleteToolServer with the MCPServer group kind is what the not-found
path calls now, so the opt-out path calls the same.

Signed-off-by: Timo Derstappen <[email protected]>
(cherry picked from commit bf0b26e)

| 2026-09-10 20:51 | `3dcfc28` | `4a91c27` (2026-09-10) | `5dd243a` → `c231bd6` | 9 | `c0125c5` Use `apk upgrade` on harness images (kagent-dev#2756) — merged upstream as `8e1ff48` | [run](https://github.com/giantswarm/kagent-upstream/actions/runs/34526022137) — checks red on the first candidate (carried MCPServer patch vs upstream kagent-dev#2774), fixed and landed by hand |