fix: skip unchanged MCP tool writes to Postgres

[supreme-gg-gg]

Summary

• Cache the last persisted MCP tool list "snapshot" in the reconciler and skip StoreToolServer / RefreshToolsForServer when nothing changed.
• Controllers still poll on the existing refresh interval (which is still configurable through feat: configurable refresh interval for tool server controllers #2480), but idle Postgres is no longer written every cycle.

Test plan

• Unit tests for snapshot equality, description/tool changes, and cache evict
• On a cluster: confirm periodic successfully registered remote MCP server logs, and that tool / toolserver.updated_at and n_tup_upd stay at the first write

[Copilot]

🟡 Changes recommended

The new skip/write logic introduces a couple of correctness risks (notably around initial DB visibility on discovery failure and snapshot determinism) that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR reduces unnecessary Postgres churn from MCP tool discovery by caching the last persisted tool list “snapshot” in the controller reconciler and skipping StoreToolServer / RefreshToolsForServer when the discovered tool set and description are unchanged across polling cycles.

Changes:

• Added an in-memory per-(server, groupKind) tool snapshot cache to avoid repeated no-op writes during periodic tool discovery.
• Evicted cached snapshots when ToolServer resources are deleted.
• Added unit tests covering snapshot equality, change detection, and cache eviction.

File summaries

File	Description
go/core/internal/controller/reconciler/reconciler.go	Adds snapshot caching + eviction to skip unchanged Postgres writes during MCP tool polling.
go/core/internal/controller/reconciler/reconciler_test.go	Adds unit tests for snapshot determinism, change detection, and cache eviction behavior.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Snapshot hashing currently ignores json.Marshal errors (risking incorrect “unchanged” detection) and there are unresolved cache/DB-healing edge cases that should be addressed or explicitly accepted.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

go/core/internal/controller/reconciler/reconciler.go:1181

• The early-return cache is purely in-memory. If the ToolServer/tools rows are deleted or the DB is reset while this controller process keeps running, subsequent reconciles will keep skipping the StoreToolServer/RefreshToolsForServer writes until the tool list changes (or the controller restarts), leaving DB-backed APIs empty/stale. Consider adding a TTL-based forced refresh (e.g., re-write at least once per N intervals) or validating row existence before skipping writes.
  go/core/internal/controller/reconciler/reconciler.go:1211
• json.Marshal errors are currently ignored when computing the snapshot. If marshaling ever fails (e.g., a future field type becomes non-marshalable), b becomes nil and the hash degenerates to a constant, which can incorrectly treat different tool lists as "unchanged" and skip required DB updates. Handle the error with a deterministic fallback that still depends on the tool data.

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[EItanya]

🤖 AI-generated review.

1. Invalidate the cached snapshot before attempting changed writes — reconciler.go:1184–1189. StoreToolServer commits separately from the tool refresh. If snapshot A is cached, writing B updates the server row, and the refresh fails, A remains cached. Rolling back to A then skips persistence indefinitely, leaving B’s metadata in the database. An uncertain commit outcome can also leave stale tools. Evict the snapshot before attempting the writes; remember it only after both succeed.

Reproduced through real MCP discovery with injected database failures. Existing reconciler tests pass; both regression cases and the full package pass with the one-line fix applied through a scratch overlay. No live PostgreSQL testing was performed.