fix(claude-code): seed bank missions only when unset; never overwrite existing (#2492)

[nicolas-vivot]

Summary

Fixes #2492 — the Claude Code plugin overwrites per-bank missions authored out-of-band (control plane UI or PATCH /banks/{id}/config) the first time it touches a bank. This is the Claude Code counterpart of the OpenClaw bug #1270 (fixed by #1473), which was never ported here even though scripts/lib/bank.py documents itself as a port of OpenClaw's banksWithMissionSet logic.

The bug

ensure_bank_mission unconditionally PATCHed reflect_mission + retain_mission from the plugin's configured bankMission/retainMission, guarded only by a local state file (bank_missions.json) tracking "have I pushed a mission" — never "does the bank already have one server-side". Consequences:

• A bank configured elsewhere (control plane, API, another machine) is treated as "unset" and gets stomped.
• The clobber re-fires whenever the local state is lost (fresh machine, cleared cache).
• It runs on both recall (UserPromptSubmit) and retain (session end), so it can fire on the very first prompt.
• The server-side PATCH is a blind JSONB merge (config || $1), so whatever the plugin sends wins.

A straight port of #1473 was insufficient: that fix only stops stamping when the config field is empty, but the Claude Code settings.json ships non-empty mission defaults, so it would still clobber a differing per-bank mission.

The fix

ensure_bank_mission is now server-aware: it reads the bank's existing overrides and only fills mission fields that are not already set — user-authored missions always win, the plugin merely seeds gaps.

• client.py
  • New get_bank_config(bank_id) → returns the bank's config; a 404 (bank not created yet — banks are lazily created on first retain) is treated as "empty" so brand-new banks are still seeded.
  • set_bank_mission(...) now takes reflect_mission= / retain_mission= and sends only the fields provided (no-ops if none). This also fixes a pre-existing quirk where retainMission could not be seeded unless bankMission was also configured.
• bank.py — ensure_bank_mission GETs overrides, computes per-field gaps, and PATCHes only the missing ones. The local bank_missions.json is demoted to a perf cache (avoids a GET on every hook); correctness now comes from the server check, so a wiped state file can no longer re-trigger a clobber.
• Transient failures (unreachable server, timeout, non-404 HTTP errors) skip cleanly without marking the bank reconciled, so they retry on the next hook rather than seeding or clobbering.

Behaviour matrix

Server state (bank overrides)	Plugin configured	Result
both missions set	both	no PATCH (missions preserved)
reflect set, retain empty	both	PATCH retain only
both empty / bank 404	both	seed both
both empty	retain only	seed retain only
unreachable / timeout / 5xx	any	skip, retry next hook (no write)

Edge cases intentionally left as-is

• Non-default retain strategies. The "already set?" check inspects only the root overrides.retain_mission. But the server auto-applies retain_default_strategy when a retain call sends no strategy (memory_engine.py), and apply_strategy overlays that strategy's retain_mission over the root (config_resolver.py). So if the root retain mission is empty but a default strategy carries its own, the plugin still PATCHes a root retain mission. Impact is non-functional — the default strategy's mission still wins at retain time, so this only writes an otherwise-masked root value (semantic pollution, not a behaviour clobber). Fully resolving this would couple the client to server strategy-resolution internals (which also have global defaults), so it was deliberately deferred.
• 404 is overloaded. The server returns 404 both when a bank doesn't exist and when the bank-config API is disabled (HINDSIGHT_API_ENABLE_BANK_CONFIG_API=false). In the disabled case the plugin does a harmless GET+PATCH that both 404, never marks reconciled, and retries each hook — no clobber, just mildly chatty at debug level.
• Server-side hardening not included. The PATCH endpoint still has no "set-if-absent" semantics for mission fields; this PR fixes the client behaviour only. A server-side guard could be a separate change.

Scope

This PR is Patch 1 only (the behavioural fix). Blanking the shipped settings.json mission defaults is a separate, independently-adoptable change and will come as its own PR.

Tests

tests/test_bank.py and tests/test_client.py updated/extended: new-bank seeding, existing-mission preservation, partial (gap-only) fill, retain-only config, local fast-path, opt-out, graceful-error-then-retry, plus get_bank_config (200 / 404 / non-404) and independent-field set_bank_mission. Full plugin suite: 201 passing.

[nicoloboschi]

Closing: superseded by Coding Agents, which now does what this PR argued for — shipped in v0.2.1 (#3352), against your issue #2492.

The rule there is seed-once. Before importing the bank template the client reads the bank-scoped overrides; if any of reflect_mission / retain_mission / observations_mission is already set — ours from an earlier pass or the user's own edit — it imports a structure-only manifest that omits them. Omitted fields are untouched server-side, so an edit made in the control plane survives every subsequent seed pass.

Retain strategies and entity labels are still re-applied, because those aren't preferences: the plugin writes documents under git/gitlog/conversation/document and a bank missing one would reject the write.

npx @vectorize-io/hindsight-coding-agents install claude-code (docs). Thanks for tracing it back to #1270 — that the OpenClaw fix was never ported is exactly what made it worth fixing properly rather than patching the symptom.