[amplifier-app-cli] First session save crashes with FileNotFoundError on minimal bundles — get_metadata() called before session dir exists

[bkrabach]

Component: amplifier-app-cli (Issues disabled on that repo; filed here per ecosystem convention)

Summary

The first session save of a brand-new session raises an uncaught FileNotFoundError, because the save path calls SessionStore.get_metadata() — which requires the session directory to already exist — before anything has created that directory. On bundles that happen to include a hook which pre-creates the session directory, the bug is masked. On a minimal bundle without such a hook, session persistence is lost entirely.

Root cause (verified against main @ 5462f1e)

SessionStore.get_metadata() raises when the session dir does not exist — amplifier_app_cli/session_store.py:353-355:

session_dir = self.base_dir / session_id
if not session_dir.exists():
    raise FileNotFoundError(f"Session '{session_id}' not found")

The session directory is created only inside SessionStore.save() (session_store.py, session_dir.mkdir(parents=True, exist_ok=True)) — i.e. after the metadata read that precedes it.

Two call sites read metadata before that first save(), neither guarded:

Call site	Enclosing function	Code
amplifier_app_cli/main.py:3649	execute_single() (defined at main.py:3356)	existing_metadata = store.get_metadata(actual_session_id) or {}
amplifier_app_cli/main.py:2902	_save_session() — nested helper inside interactive_chat() (main.py:2795), invoked at main.py:2957, 3158, 3176	existing_metadata = store.get_metadata(actual_session_id) or {}

Note the or {} is ineffective: get_metadata() raises rather than returning a falsy value.

Correction to the original report: the second call site belongs to interactive_chat(), not to execute_single(). Both are on the first-save path, so the impact is as described, but the attribution differs.

Impact

First save of every brand-new session crashes unless some other component has already created the session directory. Effect is total loss of session persistence for minimal bundles — and it is easy to miss, because rich bundles with an ambient logging-style hook incidentally create the directory first and never see it.

Verified vs. inferred

• Verified by direct code inspection at main (5462f1e): the raise condition in get_metadata(), both unguarded call sites with their enclosing functions and line numbers, that save() is what creates the directory, and that no try/except wraps either call site.
• Reported by the finder, not independently re-run here: the live crash with full traceback on a minimal bundle, and the observation that rich bundles mask it.
• Suggested but NOT rigorously tested: the fix below. The finder reports it was sufficient locally; please treat it as a starting point and verify independently rather than as tested-to-your-standards.

Suggested fix

Guard both call sites:

try:
    existing_metadata = store.get_metadata(actual_session_id)
except FileNotFoundError:
    existing_metadata = {}

Worth considering instead/in addition, since this is the second-order cause: give SessionStore a non-raising accessor (e.g. get_metadata_or_default()), or have get_metadata() return {} for a not-yet-created session, so future call sites cannot reintroduce this. Whichever is chosen, the "first save of a new session" path deserves a regression test — it is currently only exercised transitively via bundles that mask it.

Possibly related

• [app-cli] Write session identity to disk at session start, not at first prompt:complete #317 — "[app-cli] Write session identity to disk at session start, not at first prompt:complete". If session identity were written at session start, the directory would always exist and this crash would be structurally impossible. These may want to be resolved together.