fix(session-naming): stop the cross-provider leak — session's own provider only, and attributable events

[Brian Krabach (bkrabach)]

DONE-NOTE — model_performance-dgf

hooks-session-naming: the cross-provider leak, root-caused and fixed

Fixes the root cause behind model_performance-egh: the session-naming hook picked
its own provider, silently borrowed an arbitrary one when that choice failed, and
wrote the resulting llm:* events into the ROOT session's stream with no marker.

Draft PR: #348
Branch: lane/dgf-session-naming-leak · two commits, one per defect.
All line numbers below are modules/hooks-session-naming/amplifier_module_hooks_session_naming/__init__.py
at the base commit edecb2c (egh's numbers, cited from a slightly earlier revision,
are given alongside).

---

Defect A — provider selection (commit 1: da45138)

Sites

egh cited	at edecb2c	code
:37	:37	model_role: str | None = "fast"
:482-504	:509-548	if self.config.model_role: … resolved_provider_name = resolved[0].provider … role_had_no_candidates = True
:511-512	:554-556	if provider is None: / fallback_key = next(iter(providers), None) / provider = providers.get(fallback_key) if fallback_key else None

The chain

1. :37 — the hook defaults to model_role="fast", so it never simply uses the
   session's provider.
2. :509-548 — that role is resolved through the routing matrix, whose default
   matrix is openai, and the resolved name is matched against mounted provider keys
   by substring. In an Anthropic-pinned cell either the match fails (nothing openai
   is mounted) or, worse, it succeeds against a foreign mount.
3. :555 — next(iter(providers), None): an order-dependent, silent borrow of
   whichever provider instance happened to be first in the mount dict. That line is
   the leak.

The fix

• _select_session_provider() returns the provider actually answering this session:
  the conversation.provider_pin pin when one is set, otherwise the same priority
  ordering the streaming orchestrator uses (provider.priority, then
  config["priority"], default 100, ties broken by mount order). There is no
  next(iter(providers.values())) anywhere in the module any more.
• A model_role candidate is honoured only if it is mounted here and shares
  the session provider's get_info().id vendor. Same-vendor siblings
  (anthropic-sonnet → anthropic-haiku) stay allowed — that is the intended
  cheap-model routing, and it cannot change the provider field of an event.
• Anything else is REFUSED with a WARNING naming both the refused provider and
  the one actually used (once per session, DEBUG on repeat).
• An unprovable vendor (get_info missing or raising) fails closed — refused,
  never borrowed.
• A pin whose provider is no longer mounted skips naming for that turn rather
  than answering on a provider the user never chose.

---

Defect B — event attribution (commit 2: 8bcbed4)

Sites

egh cited	at edecb2c	code
:518-526	:604-620	request = ChatRequest(...) … response = await provider.complete(request, extended_thinking=False)
(context)	amplifier_core/session.py:88-91	self.coordinator.hooks.set_default_fields(session_id=self.session_id, parent_id=self.parent_id)
noted	:254-256	# Call the provider — hard timeout caps stalled providers / response = await asyncio.wait_for( … timeout=10.0

Why it was invisible

The provider emits llm:request / llm:response through the coordinator it was
mounted with — the root session's — and the kernel stamps session_id/parent_id
defaults onto every event. hooks-logging then copies the payload into
events.jsonl. So a naming call was recorded with parent_id: null and nothing at
all to distinguish it from the root agent's own turns: 321 of 12,882 root responses
(2.49%) across 12 capture roots, and in 20260902-4nd a 3.3 pp move on an Anthropic
re-warm headline against a pre-registered margin of +0.0016.

The fix

Every event a naming call emits now carries:

{"purpose": "session-naming", "origin_module": "hooks-session-naming"}

Excluding session naming from any analysis is one predicate:

select(.data.purpose != "session-naming")

Mechanism: a provider reads self.coordinator inside its own methods, so a
forwarding proxy cannot intercept it — only a copy with its own coordinator
attribute can. _stamped_provider() returns a shallow copy carrying a
_NamingCoordinator (whose hooks stamp every emit/emit_and_collect and
forward everything else), built once per provider per session so a lazily-created
SDK client is not rebuilt every few turns. The shared provider instance is never
mutated — the foreground conversation's own events stay unstamped.

If a provider's events cannot be stamped (frozen/uncopyable instance), the naming
call is skipped with a WARNING. An unattributable call is worse than a missing
session name.

On the 10 s timeout (:254-256)

Left deliberately unchanged. It is why one arm logged 15 naming requests and only 13
responses: a timed-out call leaves an llm:request with no matching llm:response.
The stamp makes that orphan identifiable rather than mysterious, which is what the
scorers need; changing the timeout would be a behaviour change the acceptance
criteria explicitly did not ask for ("default behaviour otherwise unchanged").

---

Deliverables

Deliverable	Status
DRAFT PR on origin, branch lane/dgf-session-naming-leak, two commits, tests green	DONE
Test: a provider-pinned session cannot emit a naming call on another provider	DONE — TestProviderPurity::test_pinned_session_never_calls_foreign_vendor
Test: naming events are attributable	DONE — TestNamingEventAttribution::test_naming_llm_events_carry_purpose_marker
DONE-NOTE.md in the PR body quoting egh's file:line sites	DONE — this file

Tests

modules/hooks-session-naming: 38 passed (26 pre-existing, unmodified; 12 new).

New, and each verified to FAIL against the pre-fix module:

TestProviderPurity (7; 6 fail pre-fix — the 7th pins behaviour that must NOT regress)

• test_pinned_session_never_calls_foreign_vendor — anthropic-pinned session, openai
  listed first in the mount dict, resolver resolving to openai: openai is never
  called, no stray model override, warning names both providers.
• test_same_vendor_sibling_is_allowed_with_model_override — the deliberate
  non-regression: cheap-model routing within a vendor still works.
• test_unknown_vendor_candidate_is_refused — fail-closed on unprovable vendor.
• test_resolved_provider_not_mounted_is_refused
• test_unpinned_session_uses_priority_not_dict_order
• test_stale_pin_refuses_instead_of_borrowing
• test_cross_provider_refusal_warns_once_per_session

TestNamingEventAttribution (5; 3 fail pre-fix)

• test_naming_llm_events_carry_purpose_marker
• test_original_provider_is_not_mutated — the inverted bug: the root agent's own
  events must never start claiming to be session naming.
• test_stamped_view_is_built_once_per_provider
• test_unstampable_provider_skips_rather_than_leaks
• test_provider_without_coordinator_still_names

Repo suite (pytest tests/ -q, the command CI runs): 1690 passed, 1 skipped, 1
failed locally. That single local failure is
tests/test_grpc_adapter_main.py::TestVerifyModuleType::test_non_isinstance_object_with_mount_passes
(TypeError: Instance and class checks can only be used with @runtime_checkable protocols) — pre-existing and environment-specific: it reproduces on the
untouched base commit edecb2c, and it PASSES in CI. CI on this PR is green on all
six legs (ubuntu + windows × Python 3.11/3.12/3.13), plus license/cla. Other in-repo module suites: hooks-deprecation 72,
hooks-process-guard 22, tool-delegate 150 — all pass. ruff check modules/hooks-session-naming: clean.

Also changed

• Module version 0.1.2 → 0.2.0 (behaviour change: a deployment can tell whether
  it has the fix).
• README.md: rewrote Provider Selection, added Event Attribution with the
  jq exclusion predicate and the reason the vendor check exists.

Decisions taken without escalation

• model_role="fast" default kept. The acceptance criteria require the fallback
  to be safe, not the role to be removed, and the tests require default behaviour to
  be otherwise unchanged. The vendor check makes the default safe.
• Vendor-level, not mount-level, purity. Two mounts sharing a get_info().id
  cannot produce a foreign provider field in an event, so same-vendor routing stays
  allowed. Mount-level purity would have broken intended cheap-model routing for zero
  additional protection.
• Fail closed on unknown vendor. An unprovable sameness is exactly how this leak
  got in.
• Skip rather than emit unstamped. For a best-effort background chore, a missing
  session name is strictly cheaper than a contaminated event stream.
• 10 s timeout left alone (see above).
• This file lives in the lane directory, not in the repo tree. It is lane
  bookkeeping; committing it into microsoft/amplifier-foundation would put a lane
  artifact in the PR tree. It is used verbatim as the PR body, which is what the
  deliverable asks for.

Spend

$0.00. No API calls, no DTU, no infrastructure created — every test is offline
(mocks and fakes only). The $0 cap was not approached. Had budget been available, the
one thing it would have bought is an end-to-end DTU run of an Anthropic-pinned cell
confirming zero provider: "openai" events in events.jsonl; the unit tests pin the
same invariant at the seam where the leak actually lived.

What remains open

• The eval harness's G-PROVIDER-PURITY gate (shipped by egh) stays useful as
  defence in depth: it catches any future leak source, not just this one.
• Other utility callers on the same pattern were not audited in this lane.
  examples/modules/router-orchestrator/…:66 still contains
  return next(iter(providers.values())), and the /goal loop's
  _resolve_goal_model fallback in amplifier-module-loop-streaming (a different
  repo) warns but still uses next(iter(providers.items())). Neither is in this
  lane's scope; both are the same family as 74w/l1/rc0.
• Providers do not echo ChatRequest.metadata into their llm:* payloads, so
  attribution had to be done on the caller's side. A kernel-level "utility call"
  concept — a child event scope for hook-issued LLM calls — would make this
  unnecessary for every future hook. Worth filing upstream; not attempted here.

[Brian Krabach (bkrabach)]

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

Fresh scratch clone at scratch/merge8/amplifier-foundation (base main@edecb2c, identical to the PR's stated base — no drift, no conflict with concurrent lane wok).

Gate	Method	Result
Defect A fixed (no silent next(iter(providers...)) fallback)	grep -n "next(iter(" __init__.py — only 2 hits, both inside docstrings/comments describing the old bug, zero in executable code	PASS
Defect A fixed (provider-pinned session never borrows a foreign vendor)	Read _select_session_provider/_same_vendor/_match_resolved_provider: pin → priority order, fail-closed on unknown vendor, refuses+warns rather than substituting	PASS
Test: pinned session cannot emit naming call on a different provider	Ran TestProviderPurity::test_pinned_session_never_calls_foreign_vendor against (a) PR code → pass, (b) same test re-applied over pre-fix __init__.py (checked out from main) → fails with AssertionError: A session pinned to anthropic must NEVER emit a naming call on openai	PASS (fail-before/pass-after confirmed)
Defect B fixed (events attributable)	Read _NamingHooks/_NamingCoordinator/_stamped_provider: stamps purpose="session-naming", origin_module on every naming-issued llm:* event via a per-provider-per-session shallow-copy coordinator view; skips (never emits unstamped) if stamping is impossible	PASS
Test: naming events are attributable	Ran TestNamingEventAttribution::test_naming_llm_events_carry_purpose_marker against (a) PR code → pass, (b) pre-fix __init__.py → fails, event dict has no purpose key	PASS (fail-before/pass-after confirmed)
Default behavior otherwise unchanged	26 pre-existing module tests unmodified and still pass; test_same_vendor_sibling_is_allowed_with_model_override pins the cheap-model-routing non-regression explicitly; model_role="fast" default untouched	PASS
Diff touches only what the title says	git diff main...pr-348 --stat → only modules/hooks-session-naming/{README.md, __init__.py, pyproject.toml, tests/test_session_naming.py} (904+/64-)	PASS
Module suite green	PYTHONPATH=modules/hooks-session-naming uv run pytest modules/hooks-session-naming/tests -q → 38 passed (matches PR claim exactly)	PASS
Full repo suite green	uv run pytest tests/ -q --tb=short (the exact command CI runs) → 1691 passed, 1 skipped, 0 failed (PR reported 1690 passed + 1 pre-existing env-specific failure that reproduces on unmodified main too — not reproduced in this sandbox, consistent with "environment-specific")	PASS
Other in-repo module suites unaffected	hooks-deprecation 72 passed, hooks-process-guard 22 passed, tool-delegate 150 passed — all match PR's numbers exactly	PASS
Lint clean	uv run ruff check modules/hooks-session-naming → All checks passed	PASS
CI green	gh pr checks 348 → 6/6 OS×Python legs pass + license/cla	PASS
No unvalidated performance claim	PR cites egh's prior measured numbers as background/motivation only, makes no new performance claim itself, explicitly states the one thing NOT measured (an end-to-end DTU confirmation) and why, and reports spend as $0.00 (no API calls — verified: every test here is offline, mocks/fakes only)	PASS — honest, no unvalidated claim stated as measured

Concurrency check: lane wok is reported live on this repo. main at verification time is still edecb2c (the PR's own base) — no divergence, no conflict to reconcile.

Surprising/notable: none of substance. The PR's own "What remains open" section proactively flags two out-of-scope sibling instances of the same next(iter(...)) pattern (examples/modules/router-orchestrator, and _resolve_goal_model in amplifier-module-loop-streaming) — worth follow-up items, not blockers here.

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