microsoft / microsoft/amplifier
[amplifier-bundle-skills] tool-skills: prefix-placement fallback logs a misleading WARNING (wrong branch described) once per delegate child — downgrade unless `placement` was explicitly configured
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 3.1k
- Forks
- 261
- Avg merge
- 3h 28m
- Merged PRs (30d)
- 13
Description
Summary
modules/tool-skills/amplifier_module_tool_skills/hooks.py (as of @main cache 2026-08-01):
placementdefaults to'prefix'(hooks.py:80:config.get("placement", "prefix"))._ensure_prefix_placement()returns a bare bool with two distinct failure modes: context module lacksset_system_prompt_factory(~line 214), or the surface exists but no factory was ever registered (_system_prompt_factory is None, ~lines 217-222 — the hook rightly refuses to wrap, since wrapping would drop a static system prompt).- The single warn-once message (~lines 153-162) describes only the first branch ("the context module offers no system-prompt factory surface"), but the second branch is the one real sessions actually hit — e.g. every in-process delegate child spawned by amplifier-app-cli's
session_spawner.py, which injects personas via staticadd_messageand never registers a factory. Each child mounts a fresh hook instance, so the "once" guard is per child: delegation-heavy sessions and multi-step recipes produce a warning storm on the user's terminal, with text that misdiagnoses the cause. - The hook's own comment (~lines 147-151) acknowledges sessions hit this path "without any user misconfiguration".
Suggested fix
- Have
_ensure_prefix_placement()return a reason ("ok" | "no_surface" | "no_factory") instead of a bool. - Log two distinct, accurate messages.
- Severity: WARNING only when
placement: prefixwas explicitly configured ("placement" in config); INFO otherwise (the default-config case). The fallback is graceful and skills remain fully visible, so default-config noise is not actionable for users. - Update
tests/test_prefix_placement.pyfor the new branches.
Context: this warning was investigated after a user saw it across many sessions and suspected crashes (unrelated — external SIGTERMs from their terminal host). The root cause of the fallback itself is amplifier-app-cli's spawner never registering a factory for delegate children; a companion issue is filed there. This issue covers only the log level/wording, which ships independently and helps users on any CLI version.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in modules/tool-skills/amplifier_module_tool_skills/hooks.py, especially _ensure_prefix_placement() and the warn-once logic around lines 147-162. Then read tests/test_prefix_placement.py and update coverage for no_surface, no_factory, and explicit versus default placement configuration. Done means the fallback logs an accurate message at the appropriate level without changing skill visibility.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100