fix(tool-delegate): thread model_role/provider_preferences through the resume path

[Brian Krabach (bkrabach)]

The defect

A delegation pinned to a model lost that pin the moment it was resumed.
Confirmed on the wire (rc0). Two independent sites had to be fixed; the
app-cli half shipped in microsoft/amplifier-app-cli#292 (31ad917). This PR
is the amplifier-foundation half, which was the one still open.

The two sites (at parent 0d7b3f6)

Both in modules/tool-delegate/amplifier_module_tool_delegate/__init__.py.

Call site, execute() :1533-1539 — execute() has already resolved
the caller's model_role into provider_preferences (resolver block at
:1396-1490); both are live locals. The spawn branch 60 lines below passes
both to _spawn_new_session. This branch passes neither:

            return await self._resume_existing_session(
                session_id,
                instruction,
                hooks,
                tool_call_id=tool_call_id,
                parallel_group_id=parallel_group_id,
            )

Resume path, _resume_existing_session() :2113-2121 — no parameter
exists to receive them even if the call site had sent them. And the wire
call to the app layer, :2199-2202:

            resume_coro = resume_fn(
                sub_session_id=full_session_id,
                instruction=effective_instruction,
            )

session.resume is the app-layer capability — this call is the request
tool-delegate makes of the app layer, and its kwargs are what the app layer
turns into the resumed session's provider config. The app layer already
accepts provider_preferences and model_role (app-cli session_spawner.py
:911-926 → resume_sub_session :1246-1251, #292). This repo simply never
sent them, so the promotion had nothing to promote and the resumed leg fell
back to settings priority.

It failed silently: delegate:agent_spawned carried model_role,
delegate:agent_resumed carried no routing field at all — nothing to compare
it against.

The fix

1. Call site threads provider_preferences + raw_model_role.
2. _resume_existing_session accepts both and sends them to resume_fn.
3. _session_routing records the routing each spawn resolved to, keyed
   by sub_session_id (mirrors the existing _session_agents cache). This is
   what makes the acceptance criterion hold for the shape real callers use:
   routing is pinned once on the spawn call, and resumes are then issued as
   (session_id, instruction) with no routing argument. Caller-explicit
   routing on the resume call still wins.
4. _supported_resume_routing_kwargs() introspects the app-layer
   capability and sends only what it declares. An app layer still taking
   (sub_session_id, instruction) keeps working instead of dying on an
   unexpected kwarg — but cannot drop the routing quietly: withheld kwargs
   are named in a logger.warning.
5. delegate:agent_resumed now carries model_role /
   provider_preferences, matching delegate:agent_spawned. Additive;
   absence in an old capture means UNKNOWN, never "no routing".
6. README documents the capability's optional kwargs and the precedence rule.

Evidence

Fail-before / pass-after — same test file both runs; the module fix was
stashed for the "before" run, so only the module differed.

Before (parent 0d7b3f6, fix stashed):

FAILED test_delegate_resume_routing.py::TestResumeThreadsExplicitRouting::test_resume_sends_model_role_to_resume_capability
FAILED test_delegate_resume_routing.py::TestResumeThreadsExplicitRouting::test_resume_sends_resolved_preferences_to_resume_capability
FAILED test_delegate_resume_routing.py::TestResumeThreadsExplicitRouting::test_resume_sends_explicit_provider_preferences
FAILED test_delegate_resume_routing.py::TestResumeInheritsSpawnRouting::test_resume_reuses_spawn_time_routing_when_caller_omits_it
FAILED test_delegate_resume_routing.py::TestResumeInheritsSpawnRouting::test_explicit_resume_role_overrides_the_spawn_time_role
FAILED test_delegate_resume_routing.py::TestResumedRequestConfig::test_resumed_leg_resolves_to_the_same_provider_and_model_as_spawn
FAILED test_delegate_resume_routing.py::TestResumeCapabilityCompatibility::test_legacy_capability_drop_is_logged_not_silent
FAILED test_delegate_resume_routing.py::TestResumedEventRouting::test_agent_resumed_event_carries_role_and_preferences
8 failed, 2 passed in 0.08s

The 2 that pass before are the backward-compatibility tests — they should
pass before; they pin what the fix must not break.

After: 10 passed.

Full suite (uv run pytest -q, local testpaths = tests +
modules/tool-delegate/tests):

1873 passed, 1 skipped, 1 warning in 19.45s

The one warning is pre-existing (tests/test_subprocess_runner.py).

Honest scope of "the wire"

This repo does not build LLM requests — it defines the capability contract;
the app layer implements it. The strongest assertion available here is at
the session.resume seam, which is exactly where the values were lost.

TestResumedRequestConfig closes as much of the rest as this repo can: it
drives foundation's own apply_provider_preferences — the same function the
app layer calls to build a child's provider config — with what actually
crossed the seam, and asserts the resumed leg's effective provider/model
(provider-anthropic / claude-opus-4) equals the spawn leg's. That is the
config the first request would be issued against.

Spend

$0.00 — no API calls, no DTU, no infrastructure created. Cap for this
item was $0.

Deviations from a literal "thread two arguments"

• _session_routing cache added. Without it the fix only helps a caller
  who restates the role on every resume, which is not how the tool is used,
  and the acceptance criterion is about a session spawned with a role.
  Same lifetime/cold-cache caveat as _session_agents; on a cold cache the
  app layer's own recovery (agent overlay → persisted mount plan) takes over.
• Signature guard added. Sending two new kwargs unconditionally to an
  app-supplied callable would convert a silent downgrade into a hard
  TypeError for any app layer without feat(bundles): compose Context Intelligence by default #292.
• Routing fields on delegate:agent_resumed. Not requested. The drop
  survived this long because it was invisible in telemetry.
• Note: CI runs pytest tests/ only, which excludes
  modules/tool-delegate/tests — so this test would not run in CI as
  configured. Flagged, not changed here; worth its own item.

---

Lane DONE-NOTE (verbatim)

Committed in this PR at docs/lanes/98a-foundation-resume-role/DONE-NOTE.md (commit a8b5e73, present on the remote branch head). Inlined here so the measurements, spend, and deviations are readable in the PR body itself, not only by opening the file.

docs/lanes/98a-foundation-resume-role/DONE-NOTE.md

DONE-NOTE — 98a: tool-delegate resume path drops model_role/provider_preferences

Item: model_performance-98a (project model_performance)
Branch: lane/98a-foundation-resume-role
Parent commit measured against: 0d7b3f6e66953bcdafd4372c16f6694aab942292

Result

DONE. The amplifier-foundation half of the "resume wipes the model role"
defect is fixed, with a test that fails on the parent commit and passes on
the fix. Full suite green. Draft PR opened. Nothing merged from this lane.

The two sites, quoted

Both at modules/tool-delegate/amplifier_module_tool_delegate/__init__.py,
line numbers as of the parent commit 0d7b3f6.

Site 1 — the call site, execute() :1533-1539

            return await self._resume_existing_session(
                session_id,
                instruction,
                hooks,
                tool_call_id=tool_call_id,
                parallel_group_id=parallel_group_id,
            )

Why it drops the role. By the time control reaches this branch,
execute() has already resolved the caller's model_role into
provider_preferences (the resolver block at :1396-1490) — both values are
live local variables. The spawn branch 60 lines below passes both to
_spawn_new_session(... provider_preferences=..., raw_model_role=...). This
branch passes neither. The resolution work is done and then thrown away.

Site 2 — the resume path, _resume_existing_session()

Signature, :2113-2121 — no parameter exists to receive them even if the
call site had sent them:

    async def _resume_existing_session(
        self,
        session_id: str,
        instruction: str,
        hooks,
        *,
        tool_call_id: str = "",
        parallel_group_id: str | None = None,
    ) -> ToolResult:

The wire call to the app layer, :2199-2202:

            # Resume agent session (with optional session-level timeout)
            resume_coro = resume_fn(
                sub_session_id=full_session_id,
                instruction=effective_instruction,
            )

Why it drops the role. session.resume is the app-layer capability —
this call is the request tool-delegate makes of the app layer, and its
kwargs are what the app layer turns into the resumed session's provider
config. Two kwargs and no more. The app-layer half already accepts
provider_preferences and model_role
(amplifier-app-cli session_spawner.py:911-926 → resume_sub_session
:1246-1251, shipped in #292 / 31ad917); this repo simply never sent them,
so the app layer's promotion had nothing to promote and the resumed leg fell
back to settings priority. Silent: no error, no telemetry difference —
delegate:agent_spawned carried model_role, delegate:agent_resumed
carried no routing field at all, so there was nothing to compare it against.

The two sites are one bug: the call site has the values and does not pass
them; the resume path could not accept them if it did.

What changed

1. Call site now threads provider_preferences + raw_model_role.
2. _resume_existing_session accepts both; sends them to resume_fn.
3. _session_routing cache — records the routing each spawn resolved
   to, keyed by sub_session_id, mirroring the existing _session_agents
   cache. This is what makes the acceptance criterion hold for the shape
   every real caller uses: routing is pinned once on the spawn call, and
   resumes are then issued as (session_id, instruction) with no routing
   argument at all. Caller-explicit routing on the resume call still wins.
4. _supported_resume_routing_kwargs() — introspects the app-layer
   capability and sends only what it declares. An older app layer taking
   (sub_session_id, instruction) keeps working instead of dying on an
   unexpected kwarg; what it cannot do is drop the routing quietly — the
   withheld kwargs are named in a logger.warning.
5. delegate:agent_resumed now carries model_role /
   provider_preferences, same shape as delegate:agent_spawned, so the
   two legs of a delegation are comparable in telemetry. Additive.
6. README: documents the resume capability's optional kwargs and the
   precedence rule.

Measured

Fail-before / pass-after. Same test file both times; the fix was stashed
for the "before" run, so only the module changed.

Before (parent 0d7b3f6, fix stashed) — docs/lanes/98a-foundation-resume-role/fail-before.txt:

8 failed, 2 passed

The 2 that passed before are the backward-compatibility tests
(test_legacy_two_argument_resume_capability_still_works,
test_plain_resume_sends_no_routing_kwargs) — they should pass before, and
they pin the behaviour the fix must not break.

After: 10 passed.

Full suite: 1873 passed, 1 skipped, 1 warning in 19.45s (the warning is
pre-existing, in tests/test_subprocess_runner.py).

Honest scope of "the wire"

The acceptance criterion says "the resumed session's first LLM request
carries the same model_role/provider_preferences". This repo does not build
LLM requests — it defines the capability contract and the app layer
implements it. So the strongest assertion available here is at the
session.resume seam, which is where the values were being lost.

TestResumedRequestConfig closes as much of the remaining gap as this repo
can: it drives foundation's own apply_provider_preferences — the same
function the app layer calls to build a child's provider config — with
whatever actually crossed the seam, and asserts the resumed leg's effective
provider/model (provider-anthropic / claude-opus-4) equals the spawn
leg's. That is the config the first request would be issued against. The
end-to-end wire capture that proves it against a live provider is rc0's, and
is what motivated this item; it is not re-run here (no spend authority).

Spend

$0.00. No API calls, no DTU, no infrastructure created, nothing to tear
down. The spend cap for this item was $0 and it was not approached: every
assertion is a local unit test against mocked capabilities. Nothing in the
remaining budget would have bought a stronger result at this layer — an
end-to-end wire re-capture would need a live provider and belongs with the
integration lane, not here.

Deviations

1. Added the _session_routing spawn-time cache. Strictly more than
   "thread the two arguments through". Without it the fix only covers a
   caller who restates the role on every resume call, which is not how the
   tool is used — and the acceptance criterion is phrased about a session
   spawned with a role, not a resume call carrying one. Same lifetime and
   same cold-cache caveat as the existing _session_agents cache; on a cold
   cache the app layer's own recovery (agent overlay → persisted mount plan)
   takes over, so a cold cache is a fallback, not a regression.

2. Added the capability-signature guard. Unconditionally sending two new
   kwargs to an app-layer-provided callable would have turned a silent
   downgrade into a hard TypeError for any app layer that has not taken
   app-cli feat(bundles): compose Context Intelligence by default #292. Guarded + warned instead. A callable that cannot be
   introspected is treated as accepting nothing (preserve the working call
   shape) and is also warned about — chosen deliberately over guessing and
   crashing a resume.

3. Added routing fields to delegate:agent_resumed. Not asked for. The
   reason the drop survived this long is that it was invisible in telemetry;
   leaving that gap open would leave the next regression equally invisible.
   Additive — absence in an old capture means UNKNOWN, never "no routing".

4. CI runs pytest tests/ only, which does not include
   modules/tool-delegate/tests. The local testpaths does include it and
   that is what was run for the green result above. Not changed here — out
   of scope for a bug fix, and worth its own item.

Open

• The app-cli half (feat(bundles): compose Context Intelligence by default #292) is merged; the foundation half is this draft PR.
  Neither alone closes the defect — a deployment needs both.
• routing-matrix fix: use git URL for hooks-progress-monitor source #53 (dca54b5) remains defense-in-depth only; it does not
  substitute for either fix.
• CI's pytest tests/ excludes the tool-delegate module tests, so this
  test would not run in CI as configured. Flagged, not fixed.

[Brian Krabach (bkrabach)]

Manager verification (independent, scratch clone)

Verified in a fresh clone of microsoft/amplifier-foundation at PR head
a8b5e7323bca3164f0748e5850a889993bb434e7, working tree clean, parent
0d7b3f6e66953bcdafd4372c16f6694aab942292 (current origin/main).

Full suite — uv run pytest -q:

1873 passed, 1 skipped, 1 warning in 19.02s

(the one warning is the pre-existing tests/test_subprocess_runner.py one)

Fail-before reproduced — new test file kept, module source reverted to the
parent (git checkout 0d7b3f6 -- modules/tool-delegate/amplifier_module_tool_delegate/__init__.py):

8 failed, 2 passed in 0.08s

…the same 8 named in the PR body. Restoring the module: 10 passed. So the
tests do fail without this change and pass with it.

The 2 that pass before are the right 2. They are the
backward-compatibility pins, and they are exactly what makes "default
behaviour is unchanged" checkable rather than asserted:

• test_plain_resume_sends_no_routing_kwargs — a resume with no routing
  stated and no spawn record calls the capability with
  set(kwargs) == {"sub_session_id", "instruction"}, i.e. the identical call
  shape as before this PR.
• test_legacy_two_argument_resume_capability_still_works — an app layer
  whose resume takes only (sub_session_id, instruction) still succeeds;
  test_legacy_capability_drop_is_logged_not_silent (fails before, passes
  after) pins that the withheld kwargs are named in a warning rather than
  dropped quietly.

The one behavioural change for a caller that passes nothing is the intended
one: a resume of a sub-session this tool spawned with a role now reuses
that spawn-time routing (test_resume_reuses_spawn_time_routing_when_caller_omits_it),
and an explicit resume-time role still wins over it
(test_explicit_resume_role_overrides_the_spawn_time_role).

Diff scope — git diff --stat 0d7b3f6 HEAD: 5 files, 830 insertions, 0
deletions (purely additive):

docs/lanes/98a-foundation-resume-role/DONE-NOTE.md    183 +
docs/lanes/98a-foundation-resume-role/fail-before.txt  13 +
modules/tool-delegate/README.md                        25 +
modules/tool-delegate/.../__init__.py                 167 +
modules/tool-delegate/tests/test_delegate_resume_routing.py 442 +

Nothing outside the tool-delegate resume path, its tests/README, and the lane
artifacts. The delegate:agent_resumed additions are the two routing fields
only.

CI: green on this exact head (6 test jobs, ubuntu+windows × 3.11/3.12/3.13, plus CLA).

Merge policy: bug/correctness fix with a reproduced fail-before test → merging
to main (squash).