Skip to content

fix: snapshot-based remapping preserves default provider instance when second mounts#42

Merged
bkrabach merged 1 commit intomainfrom
fix/multi-instance-snapshot
Mar 8, 2026
Merged

fix: snapshot-based remapping preserves default provider instance when second mounts#42
bkrabach merged 1 commit intomainfrom
fix/multi-instance-snapshot

Conversation

@bkrabach
Copy link
Collaborator

@bkrabach bkrabach commented Mar 8, 2026

Problem

When a user has an existing provider entry (no id) and adds a second instance via the CLI (with id), both entries for the same module cause the first instance to be silently lost.

Root cause: Provider modules always self-mount under their default name (e.g. 'anthropic'). When the second entry loaded:

  1. First entry: no instance_id -> mounts as 'anthropic' -> no remap needed
  2. Second entry: instance_id = 'anthropic-sonnet' -> mounts as 'anthropic' -> overwrites the first instance!
  3. Remap: moves second instance from 'anthropic' to 'anthropic-sonnet', unmounts 'anthropic'
  4. Result: 'anthropic-sonnet' exists, but 'anthropic' (the first instance) is gone

Symptom: Partial provider failure: 4/5 loaded. Missing: {'anthropic': 1}

Fix

Snapshot approach: Before each provider mount, snapshot any existing occupant of the default mount name. After the remap, if the snapshot was overwritten, restore it.

This correctly handles the mixed scenario:

  • Entry without instance_id -> mounts under default name, stays there
  • Entry with instance_id -> self-mounts, gets remapped, previous occupant restored

Validation relaxed: Previously ALL entries in a multi-instance scenario required instance_id. Now at most ONE entry per module may omit it (the 'default' instance). Additional entries still require it.

Both _session_init.py (Rust bridge path) and session.py (Python path) are updated.

Tests

  • New: test_mixed_instance_id_preserves_default_entry -- reproduces the exact bug; verifies both 'anthropic' and 'anthropic-sonnet' are accessible after init
  • Updated: test_duplicate_module_one_missing_instance_id_allowed -- reflects the new relaxed validation (one default entry is now allowed)

Companion PR

amplifier-app-cli: revert auto-assign Pass 2 in _map_id_to_instance_id() (removes the workaround that caused this bug)

…mounts

When a multi-instance provider config has one entry without instance_id
(the 'default' instance, e.g. original anthropic entry) and a second entry
with an explicit instance_id (e.g. 'anthropic-sonnet'), the second mount
was silently overwriting the first under the default name.

Root cause: provider modules always self-mount under their default name
(e.g. 'anthropic'). When the second entry loaded, it replaced the first
at that key. The remap then moved the second to 'anthropic-sonnet' and
unmounted 'anthropic' — the first instance was gone.

Fix: snapshot the current occupant of the default mount name before each
provider mount. After the remap, if the snapshot was overwritten, restore
it to the default name.

Also relaxes the multi-instance validation: instead of requiring ALL
entries to have instance_id, now allows exactly ONE entry per module to
omit it (the 'default' instance). Additional entries still require it.

This enables the real-world migration pattern where an existing provider
entry (no 'id') coexists with a newly-added named instance (with 'id').
@bkrabach bkrabach merged commit b18595b into main Mar 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant