Skip to content

feat: resume-time provider-mismatch guard + session repair/diagnose commands - #233

Open
Michael J. Jabbour (michaeljabbour) wants to merge 1 commit into
microsoft:mainfrom
michaeljabbour:feat/208-resume-provider-guard
Open

feat: resume-time provider-mismatch guard + session repair/diagnose commands#233
Michael J. Jabbour (michaeljabbour) wants to merge 1 commit into
microsoft:mainfrom
michaeljabbour:feat/208-resume-provider-guard

Conversation

@michaeljabbour

Copy link
Copy Markdown

Summary

Implements Part of microsoft-amplifier/amplifier-support#208 (Option A + recovery path).

Cross-provider resume can brick sessions due to provider/model mismatches (#206, #207, #208). Wave 1 landed provider-edge fixes; this PR is the resume-layer guard and recovery path.


Features

Feature 1: Resume-time provider-mismatch warning

  • Wired at the single resume choke point (_prepare_resume_context) — all four resume paths funnel through it.
  • Provider/model derivation from existing artifacts: last-writing provider/model derived from session's events.jsonl via memory-safe line-by-line tail-read (gets last llm:response event). No core changes needed — uses existing llm:response events that have always recorded provider/model.
  • On mismatch:
    • Interactive TTY: warns + confirms. User can abort cleanly (exit code before any LLM call) or proceed.
    • Non-TTY: logs the mismatch and proceeds (automation-safe).
    • Model-only mismatch: stays silent (within-provider variation is normal).
  • Proven on real incident sessions: tested on openai-written sessions resumed with anthropic config.

Feature 2: Session repair and diagnostic commands

  • New amplifier session repair <session> and amplifier session diagnose <session> subcommands.
  • Wraps amplifier_foundation.session (diagnose → backup → repair → write → re-verify).
  • Repairs structural tool-call damage:
    • Missing tool_results (orphaned tool_use IDs)
    • Ordering violations (tool_results that arrived out of order)
    • Incomplete assistant turns (no response after tool results)
  • Honest no-op when session is healthy; used by pre-turn auto-repair and available as CLI for user recovery.

Technical Details

New Files

File Purpose Lines
amplifier_app_cli/provider_guard.py Feature 1: stdlib-only provider check/warn logic 181
tests/test_provider_guard.py Feature 1: decision-table tests (16 cases) 286
tests/test_resume_provider_guard_wiring.py Feature 1: choke-point wiring integration 77
tests/test_session_repair_cli.py Feature 2: CLI helpers for repair/diagnose 184

Modified Files

File Changes
amplifier_app_cli/commands/session.py Guard call at resume choke point; Feature 2 subcommands + helpers (+119 LOC)
uv.lock Foundation pin bump (REQUIRED)

Foundation Lock Bump: A Critical Discovery

Shipped auto-repair was silently dead.

The old pin (91dc9dc0 → v1.0.0) predates the foundation's session-library restructure. The installed wheel lacked diagnosis.py, store.py, and finder.py entirely. Consequently:

  • Tests masked this: conftest.py shadows the wheel with a local foundation checkout on sys.path, so tests never saw the gap.
  • Production was broken: the existing pre-turn auto-repair at main.py:2762 was a silent ImportError no-op in shipped builds.

The lock bump (37a257ad) revives both the new Feature 2 and the pre-existing-but-broken pre-turn repair independently. Wheel-level API probe passes post-bump — all repair functions are present.


Testing

Metric Result
Baseline (clean tree) 1137 passing, 15 failing, 13 deselected, 1 xfailed
Post lock-bump +18 tests from Feature 1 → 1155 passing
Final (all features) +25 total new tests → 1162 passing
Net-new failures 0 (the 15 pre-existing failures are byte-identical before and after)
Ruff Clean on all touched files
  • Feature 1 tests: 16 decision-table cases (TTY/non-TTY, match/mismatch, model-only, no events) + 2 wiring integration tests.
  • Feature 2 tests: 7 CLI helper tests (diagnose healthy, repair missing_tool_results, repair ordering_violation, repair incomplete_turn, re-verify, etc.).
  • End-to-end proven via real CLI on incident sessions.

End-to-End Proof

(a) Provider mismatch warning + abort

PTY-driven resume with openai-written session + anthropic config:

$ amplifier session resume <id>
⚠️  Provider mismatch: session last used anthropic/claude-fable-5, but active resolved provider is openai/gpt-5-turbo-preview
Continue anyway? [y/N]: n
Aborted.

✓ User can abort before any LLM call. events.jsonl untouched.

(b) Session diagnosis and repair

$ amplifier session diagnose <session>
broken: missing_tool_results ([toolu_abc123])

$ amplifier session repair <session>
Backup: <session>.backup.2026-07-15_09-17-28.jsonl
Repaired: <session> (1/1 orphaned tool_use_ids injected synthetic tool_result)
Verified: healthy

$ amplifier session diagnose <session>
healthy ✓

Scope & Deferred Work

What's included:

  • Resume-time provider-mismatch guard (Feature 1)
  • Session repair/diagnose CLI (Feature 2)
  • Foundation lock bump (critical bugfix)

What's out of scope (per design):

  • Thinking-block damage (Option C): handled at the provider edge (microsoft-amplifier/amplifier-support#207)
  • Tool_call ID format unification (Option B): requires incremental per-provider work and contract negotiation
  • Producer tagging (Option D): kernel-level contract decision deferred

Remaining for this Issue

Per the maintainer-triaged A+B direction, Wave 2 Phase 2:

  • Option B generalization: tool_call ID formats, tool_result shapes, attachments per the compatibility table — incremental per-provider work
  • Option D: producer tagging — kernel-contract discussion

The subtraction check (documented in the issue thread) proved no core kernel changes are needed for Wave 1 or Wave 2/Phase 1.

…ommands

Implements microsoft-amplifier/amplifier-support#208 (Option A + recovery path).

FEATURE 1: Resume-time provider-mismatch warning
- New stdlib-only provider_guard.py module wired at _prepare_resume_context, the single
  choke point all four resume paths funnel through.
- Derives last-writing provider/model from session's events.jsonl via memory-safe line-by-line
  tail-read (gets last llm:response event).
- Compares against active resolved provider; on mismatch, interactive TTY warns + confirms
  (abort exits cleanly BEFORE any LLM call).
- Non-TTY proceeds with logging (automation-safe); model-only mismatch stays silent.
- Tested on real incident sessions (openai-written sessions with anthropic config).

FEATURE 2: Session repair and diagnostic commands
- New 'amplifier session repair <session>' and 'amplifier session diagnose <session>'
  subcommands wrapping amplifier_foundation.session.
- Repair: backup → diagnose → fix structural tool-call damage → write → re-verify.
- Honest no-op when healthy; repairs missing tool_results, ordering violations,
  incomplete assistant turns.
- Used by pre-turn repair (main.py:2762) and available as CLI for user recovery.

FOUNDATION LOCK BUMP: 91dc9dc0 → 37a257ad (REQUIRED)
- Old pin predated session-library restructure; installed wheel lacked diagnosis.py,
  store.py, finder.py.
- Tests masked this via conftest path shadowing; production runs were silent ImportError
  no-ops.
- Bump revives pre-turn auto-repair independently valuable.
- Wheel-level API probe passes post-bump.

Testing: 1137 → 1162 passing, zero net-new failures. Ruff clean on all touched files.
End-to-end proven via real CLI on incident sessions.

Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Salil Das (sadlilas) added a commit that referenced this pull request Aug 18, 2026
…ted from #259) (#266)

* test: repair all 15 pre-existing test failures

All 15 failures are stale test fixtures — unrelated to the Windows fixes on this branch.
Verified identical failures on `origin/main` via `git stash` baseline A/B.
No product bugs discovered; all causes are fixture-maintenance issues:

- 11: stale mock target (commit 5b8e995 renamed process_runtime_mentions)
- 2: stale prompt assertions (fork-skill load_skill form changed)
- 1: unanswered provider-add credential-collision prompt
- 1: stale session config dict assertion (agents key now present)

Before: 15 failed, 1271 passed
After: 1286 passed, 0 failed

Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>

* fix: correct stale resolution-order docstring in FoundationSettingsResolver

The class docstring claimed a 6-layer resolution strategy copied from
StandardModuleSourceResolver's comment, with step 4 listed as a removed
legacy module pattern and step 5 skipped entirely, jumping to step 6
(installed package). The actual code here only ever walks 5 steps, and
step 4 is the source hint pulled from bundle config -- not a legacy
pattern at all. Fixed the docstring to describe what the resolver
actually does instead of a stale description carried over from a
different resolver's comment.

* test: restore exact-equality assertion in subprocess routing test

6885b2c weakened test_subprocess_param_routes_to_subprocess from an
exact dict-equality assertion to two looser ones, citing an "agents"
key that "now" appeared in the forwarded config. That was a mocking
artifact: the fixture's coordinator.config was correctly stubbed as a
plain dict even before that commit, but the default fixture value
carries no "agents" key, so spawn_sub_session's issue #233
live-registry propagation has nothing to add here. Verified empirically
on this branch: with the exact-equality assertion restored, the full
test_session_spawner_subprocess.py suite (including the sibling test
that exercises a populated live registry) still passes.

Restores assert call_kwargs.kwargs["config"] == {"session": {}} and
documents why exact-equality is the correct, intentional assertion for
this specific (empty-registry) parent fixture.

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>

---------

Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Co-authored-by: sadlilas <11658960+sadlilas@users.noreply.github.com>
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