Skip to content

fix: filter spawned sub-sessions in session list and resume - #52

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/session-filter-spawned-subsessions
Jan 14, 2026
Merged

fix: filter spawned sub-sessions in session list and resume#52
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/session-filter-spawned-subsessions

Conversation

@bkrabach

@bkrabach Brian Krabach (bkrabach) commented Jan 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes an issue where amplifier resume with partial session IDs would fail with "ambiguous session ID" errors when spawned sub-sessions existed, and where amplifier session list was showing spawned sub-sessions.

Root Cause

Spawned sub-sessions have IDs like {parent_id}_{agent_name} (e.g., a5c32e0d-..._foundation:explorer). When searching for partial ID a5c32e0d, both the parent and spawned sessions matched.

Solution (DRY fix in SessionStore)

  1. Added is_top_level_session() helper function to identify main sessions (those without _ in ID)
  2. Added top_level_only=True parameter to list_sessions() - filters out spawned sessions by default
  3. Added top_level_only=True parameter to find_session() - only matches top-level sessions by default
  4. Removed redundant manual filtering in _interactive_resume_impl()

Commands Fixed

  • amplifier resume (interactive and with partial ID)
  • amplifier session list
  • amplifier session resume
  • amplifier run --resume
  • amplifier continue

Files Changed

  • amplifier_app_cli/session_store.py - Added helper function and parameters
  • amplifier_app_cli/commands/session.py - Removed redundant filtering (now handled by SessionStore)

@bkrabach
fix: filter spawned sub-sessions in session list and resume
a005a05 
Fixes an issue where 'amplifier resume' with partial session IDs would
fail with 'ambiguous session ID' errors when spawned sub-sessions
existed, and where 'amplifier session list' was showing spawned
sub-sessions.

Root cause: Spawned sub-sessions have IDs like {parent_id}_{agent_name}
(e.g., a5c32e0d-..._foundation:explorer). When searching for partial ID
'a5c32e0d', both the parent and spawned sessions matched.

Solution (DRY fix in SessionStore):
- Added is_top_level_session() helper to identify main sessions (those
  without '_' in ID)
- Added top_level_only=True parameter to list_sessions() - filters out
  spawned sessions by default
- Added top_level_only=True parameter to find_session() - only matches
  top-level sessions by default
- Removed redundant manual filtering in _interactive_resume_impl()

Commands fixed:
- amplifier resume (interactive and with partial ID)
- amplifier session list
- amplifier session resume
- amplifier run --resume
- amplifier continue
@bkrabach
Brian Krabach (bkrabach) merged commit c03d2fe into main Jan 14, 2026
1 check passed
@bkrabach
Brian Krabach (bkrabach) deleted the fix/session-filter-spawned-subsessions branch January 14, 2026 15:05
@bkrabach Brian Krabach (bkrabach) mentioned this pull request Sep 3, 2026
Merged
Brian Krabach (bkrabach) pushed a commit that referenced this pull request Sep 3, 2026
Amplifier Lane adq @microsoft-amplifier
fix(routing): mark a matrix that shadows a same-named matrix in `rout…
8548b71 
…ing list`/`show`

`amplifier routing list` showed a user matrix and a bundle matrix of the
same name as peers. Only one is ever loaded: hooks-routing's mount()
searches `[*custom_routing_dirs, bundle routing/]` and takes the first
hit, so a file in ~/.amplifier/routing/ silently makes the shipped bundle
matrix dead -- and nothing in the CLI said so. That is why a matrix change
shipped in the bundle can be completely inert on a host.

The precedence rule is NOT re-derived here. It is consumed from
hooks-routing's own `resolve_matrix_source()` (routing-matrix PR #52),
loaded by file path out of the same cached bundle directory the CLI
already globs -- app-cli does not depend on hooks-routing as a
distribution, and `routing list` never mounts a bundle, so neither an
import nor the session-time `model_role_resolver` capability is reachable
from this process.

When the cached bundle predates PR #52 (no `resolve_matrix_source`), the
CLI draws no marker at all rather than guessing the search order: a wrong
shadowing claim is worse than none. Unshadowed output is byte-identical
to before, in both text and JSON.

- amplifier_app_cli/lib/routing_provenance.py: locate + load the bundle's
  matrix_loader, classify custom vs bundle routing dirs, resolve one
  MatrixSource per matrix name.
- commands/routing.py: row marker (`⚠ shadows bundle`), a footer naming
  the file in use and each file it suppresses, the same note on
  `routing show`, and MatrixSource.to_dict() in `--format json`.
- tests/test_routing_shadowing.py: 15 tests -- shadowed marks the winner,
  unshadowed output byte-identical, no user routing dir, and the
  old-bundle degradation path.
Brian Krabach (bkrabach) added a commit that referenced this pull request Sep 3, 2026
@bkrabach Amplifier Lane adq
fix(routing): mark a matrix that shadows a same-named matrix in `rout…
90b6b3b 
…ing list`/`show` (#293)

* fix(routing): mark a matrix that shadows a same-named matrix in `routing list`/`show`

`amplifier routing list` showed a user matrix and a bundle matrix of the
same name as peers. Only one is ever loaded: hooks-routing's mount()
searches `[*custom_routing_dirs, bundle routing/]` and takes the first
hit, so a file in ~/.amplifier/routing/ silently makes the shipped bundle
matrix dead -- and nothing in the CLI said so. That is why a matrix change
shipped in the bundle can be completely inert on a host.

The precedence rule is NOT re-derived here. It is consumed from
hooks-routing's own `resolve_matrix_source()` (routing-matrix PR #52),
loaded by file path out of the same cached bundle directory the CLI
already globs -- app-cli does not depend on hooks-routing as a
distribution, and `routing list` never mounts a bundle, so neither an
import nor the session-time `model_role_resolver` capability is reachable
from this process.

When the cached bundle predates PR #52 (no `resolve_matrix_source`), the
CLI draws no marker at all rather than guessing the search order: a wrong
shadowing claim is worse than none. Unshadowed output is byte-identical
to before, in both text and JSON.

- amplifier_app_cli/lib/routing_provenance.py: locate + load the bundle's
  matrix_loader, classify custom vs bundle routing dirs, resolve one
  MatrixSource per matrix name.
- commands/routing.py: row marker (`⚠ shadows bundle`), a footer naming
  the file in use and each file it suppresses, the same note on
  `routing show`, and MatrixSource.to_dict() in `--format json`.
- tests/test_routing_shadowing.py: 15 tests -- shadowed marks the winner,
  unshadowed output byte-identical, no user routing dir, and the
  old-bundle degradation path.

* docs(adq): lane DONE-NOTE — findings, seam rationale, honest test limitation

---------

Co-authored-by: Amplifier Lane adq <lane-adq@localhost>
Brian Krabach (bkrabach) added a commit that referenced this pull request Sep 3, 2026
@bkrabach Amplifier Lane adq @microsoft-amplifier
fix(routing): select the listed matrix by the loader's rule, not last…
8c4ad7a 
…-write-wins (#294)

* fix(routing): mark a matrix that shadows a same-named matrix in `routing list`/`show`

`amplifier routing list` showed a user matrix and a bundle matrix of the
same name as peers. Only one is ever loaded: hooks-routing's mount()
searches `[*custom_routing_dirs, bundle routing/]` and takes the first
hit, so a file in ~/.amplifier/routing/ silently makes the shipped bundle
matrix dead -- and nothing in the CLI said so. That is why a matrix change
shipped in the bundle can be completely inert on a host.

The precedence rule is NOT re-derived here. It is consumed from
hooks-routing's own `resolve_matrix_source()` (routing-matrix PR #52),
loaded by file path out of the same cached bundle directory the CLI
already globs -- app-cli does not depend on hooks-routing as a
distribution, and `routing list` never mounts a bundle, so neither an
import nor the session-time `model_role_resolver` capability is reachable
from this process.

When the cached bundle predates PR #52 (no `resolve_matrix_source`), the
CLI draws no marker at all rather than guessing the search order: a wrong
shadowing claim is worse than none. Unshadowed output is byte-identical
to before, in both text and JSON.

- amplifier_app_cli/lib/routing_provenance.py: locate + load the bundle's
  matrix_loader, classify custom vs bundle routing dirs, resolve one
  MatrixSource per matrix name.
- commands/routing.py: row marker (`⚠ shadows bundle`), a footer naming
  the file in use and each file it suppresses, the same note on
  `routing show`, and MatrixSource.to_dict() in `--format json`.
- tests/test_routing_shadowing.py: 15 tests -- shadowed marks the winner,
  unshadowed output byte-identical, no user routing dir, and the
  old-bundle degradation path.

* docs(adq): lane DONE-NOTE — findings, seam rationale, honest test limitation

* fix(routing): select the listed matrix by the loader's rule, not last-write-wins

`amplifier routing list` built its rows with

    matrices[data["name"]] = (data, path)   # over sorted(discovered files)

which diverges from hooks-routing in two independent ways at once: it keyed on
the `name:` field INSIDE each YAML (the loader resolves by file STEM), and it let
the LAST file in sort order win (the loader takes the FIRST hit in
`[*custom_routing_dirs, bundle routing/]`).

The two rules agreed only by alphabetical accident -- `~/.amplifier/cache/...`
sorts before `~/.amplifier/routing/...` because "c" < "r". When they disagree the
command asserts something false: it names a file as in use that the loader would
never read. A user file `my-fast.yaml` declaring `name: balanced` overwrote the
row for the real `balanced` matrix outright.

Rows are now keyed by file stem, and the file behind each row is
`MatrixSource.path` -- the value hooks-routing's own `resolve_matrix_source()`
assigns to `matrix_path` and loads. Those fields are reachable from the CLI
through the seam PR #293 already built, so precedence is not re-derived a third
time. Only the winning file is parsed, so a shadowed file can no longer supply a
row's description, `updated:` date or compatibility count.

Also: `routing use` now writes the filename the loader resolves (it could write
an unloadable internal name before), a `name:`/stem disagreement is surfaced
rather than silently keyed on the internal name, and every JSON row carries
`matrix_file`.

When the cached bundle predates routing-matrix PR #52 there is no
`resolve_matrix_source` to ask. The shadowing MARKER is still withheld (#293's
rule: a wrong marker is worse than none), but a row must point at some file, so
selection falls back to the first candidate in `[*custom_dirs, *bundle_dirs]` --
the same list hooks-routing builds as `search_dirs`, in one labelled function.

Tests: `tests/test_routing_winner_selection.py` constructs both disagreement
classes explicitly, each with a non-vacuity gate that re-runs the old algorithm
inline and asserts it picks the other file. Full suite green (1605 passed);
existing routing list/show/use tests unmodified.

---------

Co-authored-by: Amplifier Lane adq <lane-adq@localhost>
Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@bkrabach