Skip to content

fix: register mention_resolver before session.initialize() so @-mention skill sources resolve at mount - #347

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/eager-mention-resolver
Sep 3, 2026
Merged

fix: register mention_resolver before session.initialize() so @-mention skill sources resolve at mount#347
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/eager-mention-resolver

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

Problem

Modules that receive @namespace:... sources in mount config (tool-skills' config.skills: ['@wayfinder:skills']) call get_capability("mention_resolver") at mount and get None — the capability is only registered after session.initialize() in _prepared.py. They must defer resolution to the first provider:request, so any consumer that snapshots module state between mount and first prompt sees an incomplete catalog.

Measured impact (live session, amplifier CLI): 19 skill slash-commands from lazy sources (wayfinder-pack, the design-council family, seam-test, …) were absent from cold /skills, absent from /help, and returned Unknown command forever — the CLI's shortcut registry freezes at startup. After one prompt the live capability had them, proving the catalog was late, not lost.

Fix

  • create_session(): build + register mention_resolver/mention_deduplicator before initialize(), unconditionally (both inputs — resolver bundles and base path — are compose-time known; verified, not assumed). The later guarded system-prompt block reuses the registered instance instead of constructing a second one.
  • spawn(): child sessions previously never got the capability registered at all (only session.working_dir was copied). Now registered before child_session.initialize(), mirroring create_session().

Compatibility

  • register_capability has replace semantics: amplifier-app-cli's wrapper re-registration (session_runner.register_mention_handling) is unaffected — it now wraps a real resolver instead of None for previously-empty bundles.
  • Grepped all get_capability("mention_resolver") consumers; subprocess_runner.py registers/reads post-init for prompt-text expansion only (not mount-time) — untouched, out of scope.

Validation

  • 5 new tests (tests/test_eager_mention_resolver_capability.py): capability visible to a module during initialize() for both create_session and spawn; unconditional registration; explicit call-order proof. Suite: 1696 passed (+5, 0 regressions); ruff + pyright clean on touched files.
  • End-to-end in a DTU with the CLI built from this branch: cold /skills lists the @-mention-sourced skill, cold /help lists its shortcut, cold /wayfinder-pack dispatches to a real load_skill — all three red on the unfixed host baseline.

Companion (defense-in-depth, independent): amplifier-app-cli PR adding a live-capability fallback to slash dispatch — covers skills registered at runtime via load_skill(source=…), which this fix doesn't.

🤖 Generated with Amplifier

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

@namespace: skill sources (e.g. tool-skills' '@wayfinder:skills') could not
resolve at module mount because the mention_resolver capability was only
registered AFTER session.initialize() — modules deferred resolution to the
first provider:request. Anything snapshotting module state between mount and
first prompt saw an incomplete catalog: measured live, 19 skill slash-commands
were unavailable cold in the CLI and never recovered (its registry freezes at
startup).

- create_session(): build + register mention_resolver/mention_deduplicator
  BEFORE initialize(), unconditionally (inputs are compose-time known). The
  later guarded system-prompt block reuses the registered instance.
- spawn(): child sessions previously never got the capability at all — now
  registered before child initialize(), mirroring create_session().
- 5 new tests (capability visible during mount for both paths; unconditional;
  explicit call-order proof). Suite: 1696 passed (+5), ruff + pyright clean.

Verified end-to-end in a DTU (CLI built from this branch): cold /skills lists
the @-mention-sourced skill, cold /help lists its shortcut, cold dispatch
works — all three red on the unfixed baseline.

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

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

Copy link
Copy Markdown
Collaborator Author

Merge-queue verification — PASS, merging with --admin

Fresh scratch clone at scratch/tidy/amplifier-foundation (base main@edecb2c, the PR's own stated base — main has since advanced to 2a3cb19 via #348/#349, neither of which touches this PR's files; git diff --stat between the two bases on amplifier_foundation/bundle/_prepared.py, tests/test_cost_bridge_foundation.py, tests/test_eager_mention_resolver_capability.py is empty, and a scratch git merge origin/main --no-commit --no-ff on the PR branch completes with zero conflicts).

Gate Method Result
Diff scoped to the title git diff edecb2c pr-347 --stat → only amplifier_foundation/bundle/_prepared.py (+94/-21) and two test files (test_eager_mention_resolver_capability.py new, test_cost_bridge_foundation.py +16 mock-fixture fields only) PASS
Fail-before: new tests fail on pre-fix source Reverted only _prepared.py to edecb2c, kept the 5 new tests → 5 failed, all real behavioral AssertionErrors (observed_resolver_during_init is None, call_order shows initialize before register_mention_resolver), not import/collection errors PASS
Pass-after Restored _prepared.py to PR state → 5/5 passed PASS
Full suite green uv run pytest tests/ -q1696 passed, 1 skipped (matches PR's claimed count exactly) PASS
Lint / types clean ruff check on both touched files → clean; pyright on _prepared.py → 0 errors/warnings PASS
CI (all legs) gh pr checks 347 → ubuntu×3 + windows×3 (py3.11/3.12/3.13) + license/cla, all pass PASS
Default behavior otherwise unchanged create_session()'s registration is moved earlier and made unconditional, but the values registered are byte-identical (bundles_for_resolver/resolver_base computed from the same already-known-at-compose-time inputs); the post-initialize() block that used to build them now just adds a comment and reuses the same capability. register_capability has replace semantics (confirmed by reading _prepared.py's existing docstring convention), so downstream consumers (e.g. session_runner.register_mention_handling) are unaffected — covered by unmodified pre-existing tests, all still green PASS
No unvalidated performance claim PR cites its own measured "19 skill slash-commands" figure as motivation from a live session, not as a number this verification re-measured; makes no other quantitative claim PASS — honest

Diff scope note: the tests/test_cost_bridge_foundation.py change is in-scope, not drift — spawn()'s eager registration now calls _build_bundles_for_resolver(effective_bundle) unconditionally, which reads bundle.source_base_paths/bundle.name; the two pre-existing mock-bundle fixtures needed those attributes set to behave like a real Bundle instead of tripping on a bare MagicMock. No assertions changed, only fixture setup.

Concurrency check: main moved from edecb2c to 2a3cb19 (#348 cross-provider-leak fix, #349 routing-matrix provenance telemetry) during this verification pass — neither overlaps this PR's files (confirmed above); no rebase needed before merge.

All gates pass. Merging via gh pr merge --squash --admin (required-review ruleset bypass disclosed here — I am the PR author and no other human reviewer is available in this workflow).

@bkrabach
Brian Krabach (bkrabach) merged commit 0d7b3f6 into main Sep 3, 2026
7 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.

2 participants