From 9c71e9aae562dbf31ad7e43d7be89b2c8ba9f4fa Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Mon, 3 Aug 2026 08:01:17 -0700 Subject: [PATCH] fix(spawn): honor an agent's declared agents: access-control policy An agent can declare agents: -- a Smart Single Value that controls which sub-agents its spawned session may delegate to. merge_configs() honors it. The runtime-registry propagation block added later then silently undid it. This commit: - Applies agent_config declarations to the live registry propagation (not just merge_configs) - Ensures same-name local-wins collision avoidance is preserved - Adds comprehensive spawn-level test coverage for all declaration forms - Fixes PR #178's original complaint about allowlists under-delivering Blast radius: zero. Surveyed all 749 config files under ~/.amplifier/cache/ -- 51 agents: occurrences exist, all are dict-shaped agent rosters. Zero access-control declarations in the installed ecosystem, so nothing changes for deployed systems. Preserves from #253: fresh-dict-and-rebind (no cross-session mutation), deepcopy per agent, local-wins. Follow-up to #178 and #253. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- amplifier_app_cli/session_spawner.py | 39 ++++++ tests/test_session_spawner_issue_233.py | 177 ++++++++++++++++++++++++ 2 files changed, 216 insertions(+) diff --git a/amplifier_app_cli/session_spawner.py b/amplifier_app_cli/session_spawner.py index ac590ef..3180d46 100644 --- a/amplifier_app_cli/session_spawner.py +++ b/amplifier_app_cli/session_spawner.py @@ -319,6 +319,45 @@ async def spawn_sub_session( live_agents = (parent_coord.config or {}).get("agents") or {} except AttributeError: live_agents = {} + + # Reconcile this propagation with the overlay's OWN access-control + # declaration (agent_config["agents"] -- a Smart Single Value: + # "none" | list-of-names | "all"/absent). merge_configs() already + # applied this same declaration once, but only against the STATIC + # parent snapshot it can see -- a mode-contributed live agent named + # in an explicit allowlist isn't in that snapshot, so merge_configs + # resolves it to an empty dict there (this was PR #178/#233's own + # documented under-delivery: "even an explicit agents: [sibling_b] + # declaration wouldn't reach sibling_b -- the source dict is the + # wrong one"). Left unhandled, the block below would then blindly + # union in the FULL live registry regardless of that declaration, + # silently re-opening delegation for an agent that said "none" or + # handing it agents outside its stated allowlist -- defeating the + # sub-agent access-control contract merge_configs() exists to + # enforce (commit d609bb2; documented in AGENT_AUTHORING.md). + # + # So: apply the declaration a second time here, against the live + # registry, before it gets merged in. This reconciles both intents + # in one place -- #233's "mode siblings must be reachable" and the + # original "agents: declares exactly who I can delegate to." + # + # Gate on the OVERLAY (agent_config), NOT on merged_config["agents"]. + # An unrestricted agent (no `agents:` key) whose parent simply has + # no STATIC agents also produces an empty merged_config["agents"] -- + # gating on emptiness there would wrongly suppress propagation for + # that agent too. The overlay's own declared value is the only + # signal that distinguishes "restricted to nothing/some" from + # "unrestricted, parent just has nothing (yet) in the snapshot." + agent_filter = agent_config.get("agents") + if agent_filter == "none": + live_agents = {} + elif isinstance(agent_filter, list): + live_agents = { + name: cfg for name, cfg in live_agents.items() if name in agent_filter + } + # else: "all", None, or absent -- inherit the live registry unchanged + # (current/original #233 behavior). + if live_agents: # Build a FRESH dict and rebind it; never mutate the dict # merged_config already holds. merge_configs() deep-copies the diff --git a/tests/test_session_spawner_issue_233.py b/tests/test_session_spawner_issue_233.py index b3f7253..164e16b 100644 --- a/tests/test_session_spawner_issue_233.py +++ b/tests/test_session_spawner_issue_233.py @@ -23,6 +23,16 @@ caller passing extra context. S5 — skill capability propagation: parent's coordinator has runtime_skill_overlay capability; child coordinator inherits it. + S6 — parent isolation: propagation writes into the child's config only, + never mutates the parent's. + S7 — access-control declaration honored (fix/honor-agents-declaration): + the spawned agent's OWN `agents:` Smart Single Value declaration + ("none" | list-of-names | "all"/absent) must be respected when the + live registry is unioned in, not silently overridden by the #233 + propagation. Covers all five rows of the target-behavior table: + "none" -> {}, ["sibling_b"] -> {sibling_b} (fixes PR #178's + under-delivery), ["explorer"] -> {explorer}, "all" -> full union, + absent -> full union (both unchanged from #233 behavior). """ from __future__ import annotations @@ -479,3 +489,170 @@ async def test_propagation_does_not_mutate_parent_config(self) -> None: assert captured["agents"] is not parent.coordinator.config["agents"], ( "child's agents dict is the parent's LIVE registry object" ) + + +# --------------------------------------------------------------------------- +# S7 — Access-control declaration honored (fix/honor-agents-declaration) +# --------------------------------------------------------------------------- + + +class TestS7AccessControlDeclaration: + """The spawned agent's OWN `agents:` declaration gates propagation. + + Fixture shared across all five rows (matches the issue's target-behavior + table exactly): + + Parent STATIC agents (session.config): {explorer, builder} + Parent LIVE registry (coordinator.config): {explorer, builder, sibling_b} + (sibling_b is mode-contributed + only -- absent from the static + snapshot) + + The agent "explorer" is spawned with a varying `agents:` overlay value. + Each assertion checks the EXACT resulting child agent name set, not mere + membership -- both over-delivery (declared "none"/allowlist but child + gets more) and under-delivery (declared allowlist but child gets less, + PR #178's original bug) are real failure modes here. + """ + + def _make_fixture_parent(self) -> MagicMock: + return _make_parent_session( + session_config_agents={ + "explorer": {"description": "Explorer agent"}, + "builder": {"description": "Builder agent"}, + }, + coordinator_config_agents={ + "explorer": {"description": "Explorer agent"}, + "builder": {"description": "Builder agent"}, + "sibling_b": {"description": "Mode-contributed sibling B"}, + }, + ) + + @pytest.mark.asyncio + async def test_agents_none_disables_all_delegation(self) -> None: + """`agents: "none"` must yield an EMPTY child agents dict. + + Before the fix: the #233 propagation block unconditionally unioned + in the full live registry, reopening delegation the overlay + explicitly disabled. + """ + parent = self._make_fixture_parent() + child = _make_child_session_mock() + captured: dict = {} + + await _run_spawn( + parent, + {"explorer": {"description": "Explorer agent", "agents": "none"}}, + child, + "explorer", + captured, + ) + + assert captured.get("agents", {}) == {}, ( + "agents: 'none' must disable ALL sub-agent delegation. " + f"Got: {sorted(captured.get('agents', {}).keys())}" + ) + + @pytest.mark.asyncio + async def test_agents_list_of_live_only_name_is_honored_from_live_registry( + self, + ) -> None: + """`agents: ["sibling_b"]` must yield EXACTLY {sibling_b}. + + sibling_b is mode-contributed-only (absent from the static parent + snapshot). merge_configs() filters the STATIC dict and resolves this + to {} (PR #178's documented under-delivery). The spawner must apply + the same allowlist against the LIVE registry so sibling_b still + reaches the child -- and nothing else does. + """ + parent = self._make_fixture_parent() + child = _make_child_session_mock() + captured: dict = {} + + await _run_spawn( + parent, + {"explorer": {"description": "Explorer agent", "agents": ["sibling_b"]}}, + child, + "explorer", + captured, + ) + + assert captured.get("agents", {}).keys() == {"sibling_b"}, ( + "agents: ['sibling_b'] must yield EXACTLY {sibling_b} -- honored " + "from the live registry even though sibling_b is absent from " + f"the static snapshot. Got: {sorted(captured.get('agents', {}).keys())}" + ) + + @pytest.mark.asyncio + async def test_agents_list_of_static_name_is_honored_as_allowlist(self) -> None: + """`agents: ["explorer"]` must yield EXACTLY {explorer}. + + Before the fix: the #233 propagation block would blow the allowlist + open by unioning in the full live registry (builder, sibling_b too). + """ + parent = self._make_fixture_parent() + child = _make_child_session_mock() + captured: dict = {} + + await _run_spawn( + parent, + {"explorer": {"description": "Explorer agent", "agents": ["explorer"]}}, + child, + "explorer", + captured, + ) + + assert captured.get("agents", {}).keys() == {"explorer"}, ( + "agents: ['explorer'] must yield EXACTLY {explorer} -- the " + "allowlist must not be blown open by live-registry propagation. " + f"Got: {sorted(captured.get('agents', {}).keys())}" + ) + + @pytest.mark.asyncio + async def test_agents_all_yields_full_union(self) -> None: + """`agents: "all"` must yield the FULL union: {explorer, builder, sibling_b}. + + Unchanged from #233 behavior -- explicit "all" inherits everything, + static and live-registry alike. + """ + parent = self._make_fixture_parent() + child = _make_child_session_mock() + captured: dict = {} + + await _run_spawn( + parent, + {"explorer": {"description": "Explorer agent", "agents": "all"}}, + child, + "explorer", + captured, + ) + + assert captured.get("agents", {}).keys() == {"explorer", "builder", "sibling_b"}, ( + "agents: 'all' must yield the full union of static + live agents. " + f"Got: {sorted(captured.get('agents', {}).keys())}" + ) + + @pytest.mark.asyncio + async def test_agents_absent_yields_full_union(self) -> None: + """No `agents:` key at all must yield the FULL union (unrestricted). + + Unchanged from #233 behavior -- absence of the declaration means + "unrestricted," identical to explicit "all". + """ + parent = self._make_fixture_parent() + child = _make_child_session_mock() + captured: dict = {} + + await _run_spawn( + parent, + {"explorer": {"description": "Explorer agent"}}, # no "agents" key + child, + "explorer", + captured, + ) + + assert captured.get("agents", {}).keys() == {"explorer", "builder", "sibling_b"}, ( + "Absent agents: declaration must be unrestricted (full union), " + "identical to explicit 'all'. " + f"Got: {sorted(captured.get('agents', {}).keys())}" + )