feat(update): refuse updates that would delete existing configuration

[chuckmeyer]

Phase 1 of #14.

Problem

PATCH /agents/{id} replaces object fields rather than merging them, so anything absent from the payload is destroyed. agent-config.json cannot express a second tool, tool-level fields like mode, or per-index searchControls — so update deletes them, and until #13 it did so silently.

#13 made the loss visible in --dry-run. This makes it impossible to do by accident.

Behaviour

$ algolia-agent update <id> --config agent-config.json
ERROR: this update would remove configuration the agent currently has:
  - the 'algolia_display_results' tool would be deleted
  - algolia_search_index.allowUnlistedIndices would revert to its default (now True)
  - algolia_search_index.mode would revert to its default (now 'dynamic')
  - searchControls on 'tcg_cards_msft-build-2026' would be wiped
  - config key 'enableAlgoliaMcp' would be removed
  - config key 'max_iterations' would be removed
  - config key 'memory' would be removed

The Agent Studio API replaces these fields instead of merging them, so
anything missing from the payload is lost. agent-config.json cannot express
them, which is why they are absent.

Either add the values to your config, or pass --force to accept the removals.
Run the same command with --dry-run to see the full diff first.

What counts as a removal

The guard fires on losses the config model cannot ask for, since those can only be accidental:

• a tool type present on the agent but absent from the payload
• a tool field whose current value differs from its API default (mode, allowUnlistedIndices)
• per-index searchControls that would be wiped
• config keys the payload omits

Deliberately not flagged:

• index membership — the config names its indices, so dropping one is an explicit choice
• fields already at their default — omitting mode: "static" changes nothing, verified against the API

--dry-run is never blocked: it writes nothing, and refusing to report would defeat its purpose.

Refactor

_tool_key() and _TOOL_IDENTITY move from locals inside _diff() to module scope, so the guard and the diff share one definition of tool identity rather than duplicating the rules.

Verification

130 tests pass (8 new), CI on 3.10 / 3.11 / 3.13.

Live end-to-end against a throwaway agent carrying all four categories (created and deleted; nothing existing touched):

	Result
without --force	refused, exit 1 — agent afterwards byte-for-byte intact: 2 tools, mode: dynamic, allowUnlisted: True, searchControls present, 4 config keys
with --force	proceeded — tools 2→1, dynamic→static, True→False, searchControls wiped, config 4 keys→1

The --force half matters as much as the refusal: it confirms every warned-about loss is real, so the guard is accurate rather than over-cautious.

Next

Phase 2 is the snapshot command, after which the destructive path is only reachable from a hand-written partial config plus --force. The error message will then recommend snapshot as the remedy.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds a safety guard to algolia-agent update to prevent accidental destructive updates caused by the Agent Studio API’s replace-not-merge PATCH behavior, requiring explicit --force to proceed when the payload would drop unmodelable existing agent configuration.

Changes:

• Introduces _removals() to detect configuration losses that the local config model can’t intentionally express, and blocks update unless --force is provided.
• Refactors tool identity helpers (_tool_key, _TOOL_IDENTITY) to module scope so _diff() and the new guard share the same identity rules.
• Adds unit tests covering _removals() behavior and cmd_update refusal/--force/--dry-run interactions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/algolia_agent/cli.py	Adds destructive-update detection + --force override and refactors shared tool-identity logic.
tests/test_cli.py	Adds tests validating removal detection and update refusal/force/dry-run behavior.

---

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

[chuckmeyer]

Both findings applied in the latest commit.

Hard-coded agent-config.json — correct, and worse than cosmetic. update takes --config with any path and can run with no config file at all (just --index/--name flags), so the message was sometimes naming a file that had nothing to do with the invocation. It now interpolates the path in use, or says "the fields you supplied" when there is no config file. Verified live with a custom filename.

would revert to its default (now 'dynamic') — also correct. Putting "default" adjacent to the current value reads as though dynamic were the default. Since both defaults are known from probing, they are now named outright:

  - algolia_search_index.allowUnlistedIndices: True would revert to its default of False
  - algolia_search_index.mode: 'dynamic' would revert to its default of 'static'

For a field with no known default the message says it "would be dropped from the payload" rather than implying knowledge we lack. That matters going forward: the dashboard is shipping fields faster than we can probe them, so the honest fallback is the one that stays correct.

One deliberate extension beyond the review's scope: the matching line in _diff() carried the same ambiguity from the same source (→ default (not sent)). Two different wordings for one phenomenon is worse than either, so it now reads 'dynamic' → 'static' (not sent), or 42 → dropped (not sent) when the default is unknown. That updated one existing test's assertion.

133 tests pass (3 new, covering the custom-path message and both known/unknown-default branches).