diff --git a/amplifier_foundation/bundle/_dataclass.py b/amplifier_foundation/bundle/_dataclass.py index f4eba7a..c5d9e97 100644 --- a/amplifier_foundation/bundle/_dataclass.py +++ b/amplifier_foundation/bundle/_dataclass.py @@ -4,6 +4,7 @@ import logging from collections.abc import Callable +from copy import deepcopy from dataclasses import dataclass, field from pathlib import Path from typing import TYPE_CHECKING, Any @@ -320,7 +321,9 @@ def to_mount_plan(self) -> dict[str, Any]: if self.spawn: mount_plan["spawn"] = dict(self.spawn) - return mount_plan + # Modules may add runtime defaults to nested config during mount. Keep + # those changes out of this reusable bundle and future child plans. + return deepcopy(mount_plan) async def prepare( self, diff --git a/amplifier_foundation/bundle/_prepared.py b/amplifier_foundation/bundle/_prepared.py index beb09c4..feade02 100644 --- a/amplifier_foundation/bundle/_prepared.py +++ b/amplifier_foundation/bundle/_prepared.py @@ -4,6 +4,7 @@ import asyncio import logging +from copy import deepcopy from dataclasses import dataclass from dataclasses import field from decimal import Decimal @@ -585,12 +586,15 @@ async def create_session( inject_additional_events, ) - inject_additional_events(self.mount_plan, FOUNDATION_OBSERVABILITY_EVENTS) + # A prepared bundle can create sessions with different runtime defaults. + # Event injection and module mounts must only mutate this session's plan. + mount_plan = deepcopy(self.mount_plan) + inject_additional_events(mount_plan, FOUNDATION_OBSERVABILITY_EVENTS) from amplifier_core import AmplifierSession session = AmplifierSession( - self.mount_plan, + mount_plan, session_id=session_id, parent_id=parent_id, approval_system=approval_system, diff --git a/tests/test_observability_injection.py b/tests/test_observability_injection.py index 1218eb3..c086da5 100644 --- a/tests/test_observability_injection.py +++ b/tests/test_observability_injection.py @@ -253,7 +253,7 @@ class TestCreateSessionInjectsFoundationEvents: @pytest.mark.asyncio async def test_create_session_calls_inject_with_foundation_events(self): - """create_session() calls inject_additional_events(self.mount_plan, FOUNDATION_OBSERVABILITY_EVENTS).""" + """create_session() injects foundation events into the session's own plan.""" from amplifier_foundation.bundle._prepared import PreparedBundle # noqa: PLC0415 from amplifier_foundation.bundle._observability import ( FOUNDATION_OBSERVABILITY_EVENTS, @@ -307,9 +307,10 @@ def _spy(mp, events, **kwargs): ) # Verify called with the right mount_plan and events mp_used, events_used = call_record[0] - assert mp_used is mount_plan, ( - "inject_additional_events was not called with self.mount_plan" - ) + assert mp_used is not mount_plan + assert MockSession.call_args.args[0] is mp_used + assert mount_plan["hooks"][0]["config"] == {} + assert "session:config" in mp_used["hooks"][0]["config"]["additional_events"] for ev in FOUNDATION_OBSERVABILITY_EVENTS: assert ev in events_used, f"{ev!r} not passed to inject_additional_events" diff --git a/tests/test_session_config_isolation.py b/tests/test_session_config_isolation.py new file mode 100644 index 0000000..b15c058 --- /dev/null +++ b/tests/test_session_config_isolation.py @@ -0,0 +1,87 @@ +"""Runtime defaults must not contaminate reusable bundle configuration.""" + +from copy import deepcopy + +import pytest +from amplifier_core import AmplifierSession + +from amplifier_foundation.bundle import Bundle +from amplifier_foundation.bundle._prepared import BundleModuleResolver, PreparedBundle + + +@pytest.mark.parametrize( + "section", ["session", "providers", "tools", "hooks", "agents", "spawn"] +) +def test_mount_plan_mutation_cannot_change_future_child_config(section): + config = {"paths": ["declared"], "options": {"mode": "declared"}} + value = {"config": config} + if section in ("providers", "tools", "hooks"): + value = [{"module": "example", "config": config}] + bundle = Bundle(name="parent", **{section: value}) + before = deepcopy(value) + + plan = bundle.to_mount_plan() + runtime = plan[section][0] if isinstance(plan[section], list) else plan[section] + runtime["config"]["working_dir"] = "/parent" + runtime["config"]["paths"].append("runtime") + runtime["config"]["options"]["mode"] = "runtime" + + assert getattr(bundle, section) == before + child_plan = bundle.compose(Bundle(name="child")).to_mount_plan() + assert child_plan[section] == before + + +@pytest.mark.asyncio +async def test_reused_prepared_bundle_isolates_mount_defaults_and_runtime_mutations( + tmp_path, monkeypatch +): + bundle = Bundle( + name="shared", + session={ + "orchestrator": {"module": "loop-test"}, + "context": {"module": "context-test"}, + }, + tools=[{"module": "tool-test", "config": {"paths": ["declared"]}}], + hooks=[ + { + "module": "hooks-logging", + "config": {"additional_events": ["custom:event"]}, + } + ], + ) + prepared = PreparedBundle( + bundle=bundle, + mount_plan=bundle.to_mount_plan(), + resolver=BundleModuleResolver(module_paths={}), + ) + before = deepcopy(prepared.mount_plan) + + async def initialize_with_mutating_module(session): + # Use the real Core session/coordinator, with a simulated module mount + # to exercise Foundation's ownership boundary without loading providers. + config = session.config["tools"][0]["config"] + config.setdefault( + "working_dir", session.coordinator.get_capability("session.working_dir") + ) + config["paths"].append(session.session_id) + + monkeypatch.setattr(AmplifierSession, "initialize", initialize_with_mutating_module) + parent = await prepared.create_session( + session_id="parent", session_cwd=tmp_path / "parent" + ) + child = await prepared.create_session( + session_id="child", session_cwd=tmp_path / "child" + ) + + for session, name in [(parent, "parent"), (child, "child")]: + config = session.config["tools"][0]["config"] + assert config["working_dir"] == str((tmp_path / name).resolve()) + assert config["paths"] == ["declared", name] + events = session.config["hooks"][0]["config"]["additional_events"] + assert events[0] == "custom:event" + assert events.count("session:config") == 1 + + parent.config["hooks"][0]["config"]["additional_events"].append("parent:only") + assert "parent:only" not in child.config["hooks"][0]["config"]["additional_events"] + assert prepared.mount_plan == before + assert bundle.to_mount_plan() == before