diff --git a/bot_bottle/manifest_bottle.py b/bot_bottle/manifest_bottle.py index e67303e..b4b2015 100644 --- a/bot_bottle/manifest_bottle.py +++ b/bot_bottle/manifest_bottle.py @@ -49,12 +49,10 @@ class ManifestBottle: # costs image weight, a resident service, and relaxed guest device modes # that the majority of bottles never need. nested_containers: bool = False - # True when nested_containers was explicitly present in the source dict. - # Used by the runtime merge path to distinguish "omitted → default False" - # from "explicitly set to False", so an explicit false in a later bottle - # can override an earlier true while a bottle that never mentions the key - # does not silently drop the capability. - nested_containers_explicit: bool = False + # Source fields retained across extends/runtime composition. Boolean + # defaults otherwise erase the distinction between "omitted" and an + # explicitly declared value (especially False). + declared_fields: frozenset[str] = frozenset() @classmethod def from_dict(cls, name: str, raw: object) -> "ManifestBottle": @@ -145,5 +143,5 @@ class ManifestBottle: env=env, agent_provider=agent_provider, git=git, git_user=git_user, egress=egress, supervise=supervise_raw, nested_containers=nested_raw, - nested_containers_explicit="nested_containers" in d, + declared_fields=frozenset(d), ) diff --git a/bot_bottle/manifest_extends.py b/bot_bottle/manifest_extends.py index f57ec6b..7ebed87 100644 --- a/bot_bottle/manifest_extends.py +++ b/bot_bottle/manifest_extends.py @@ -8,6 +8,17 @@ from .manifest_git import ManifestGitUser, parse_git_gate_config from .manifest_util import ManifestError, as_json_object +def _overlay_declared_bool( + base: ManifestBottle, + override: ManifestBottle, + field: str, +) -> bool: + """Overlay a defaulted boolean only when override declared it.""" + value = getattr(override if field in override.declared_fields else base, field) + assert isinstance(value, bool) + return value + + def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle": """Merge an ordered list of pre-resolved ManifestBottle objects. @@ -18,11 +29,10 @@ def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle": routes: concatenate; agent_provider, supervise, nested_containers: later replaces (presence-aware). - nested_containers uses presence-aware replacement: if the later bottle + Defaulted booleans use presence-aware replacement: if the later bottle was loaded from a source that explicitly declared the key, its value wins (so an explicit `false` can override an earlier `true`). If the - later bottle never mentioned the key, the earlier value is preserved - (so a bottle that only sets env does not silently drop the capability). + later bottle never mentioned the key, the earlier value is preserved. """ if not bottles: raise ValueError("merge_bottles_runtime requires at least one bottle") @@ -54,20 +64,17 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle merged_routes = base.egress.routes + override.egress.routes merged_egress = ManifestEgressConfig(routes=merged_routes, Log=override.egress.Log) - merged_nc = ( - override.nested_containers - if override.nested_containers_explicit - else base.nested_containers - ) return ManifestBottle( env=merged_env, agent_provider=override.agent_provider, git=merged_git, git_user=merged_git_user, egress=merged_egress, - supervise=override.supervise, - nested_containers=merged_nc, - nested_containers_explicit=base.nested_containers_explicit or override.nested_containers_explicit, + supervise=_overlay_declared_bool(base, override, "supervise"), + nested_containers=_overlay_declared_bool( + base, override, "nested_containers" + ), + declared_fields=base.declared_fields | override.declared_fields, ) @@ -213,20 +220,17 @@ def _fold_two_bottles( Log=later.egress.Log, ) - folded_nc = ( - later.nested_containers - if later.nested_containers_explicit - else earlier.nested_containers - ) return ManifestBottle( env=merged_env, agent_provider=later.agent_provider, git=merged_git, git_user=merged_git_user, egress=merged_egress, - supervise=later.supervise, - nested_containers=folded_nc, - nested_containers_explicit=earlier.nested_containers_explicit or later.nested_containers_explicit, + supervise=_overlay_declared_bool(earlier, later, "supervise"), + nested_containers=_overlay_declared_bool( + earlier, later, "nested_containers" + ), + declared_fields=earlier.declared_fields | later.declared_fields, ), merged_repos_raw @@ -284,13 +288,9 @@ def _merge_bottles( if "agent_provider" in child_raw else parent.agent_provider ) - merged_supervise = ( - child.supervise if "supervise" in child_raw else parent.supervise - ) - merged_nested_containers = ( - child.nested_containers - if "nested_containers" in child_raw - else parent.nested_containers + merged_supervise = _overlay_declared_bool(parent, child, "supervise") + merged_nested_containers = _overlay_declared_bool( + parent, child, "nested_containers" ) validate_egress_routes(name, merged_egress.routes) @@ -302,6 +302,7 @@ def _merge_bottles( egress=merged_egress, supervise=merged_supervise, nested_containers=merged_nested_containers, + declared_fields=parent.declared_fields | child.declared_fields, ) diff --git a/tests/unit/test_manifest_bottle_merge.py b/tests/unit/test_manifest_bottle_merge.py index 4a656bc..b501985 100644 --- a/tests/unit/test_manifest_bottle_merge.py +++ b/tests/unit/test_manifest_bottle_merge.py @@ -56,6 +56,16 @@ class TestMergeBottlesRuntime(unittest.TestCase): result = merge_bottles_runtime([base, override]) self.assertFalse(result.supervise) + def test_supervise_survives_a_later_bottle_that_omits_the_key(self): + disabled = _bottle(supervise=False) + quiet = _bottle(env={"X": "1"}) + self.assertFalse(merge_bottles_runtime([disabled, quiet]).supervise) + + def test_supervise_explicit_true_overrides_earlier_false(self): + disabled = _bottle(supervise=False) + enabled = _bottle(supervise=True) + self.assertTrue(merge_bottles_runtime([disabled, enabled]).supervise) + def test_nested_containers_survives_a_later_bottle_that_omits_the_key(self): """A bottle that never mentions nested_containers must not silently drop the capability: `--bottle with-docker --bottle claude-dev`.""" @@ -77,6 +87,25 @@ class TestMergeBottlesRuntime(unittest.TestCase): disabled = _bottle(nested_containers=False) self.assertTrue(merge_bottles_runtime([disabled, enabled]).nested_containers) + def test_extended_boolean_values_remain_explicit_at_runtime(self): + idx = _index( + bottles={ + "enabled": {"nested_containers": True, "supervise": True}, + "parent": {}, + "disabled_child": { + "extends": "parent", + "nested_containers": False, + "supervise": False, + }, + }, + agents={"impl": {"bottle": "enabled", "skills": [], "prompt": ""}}, + ) + result = idx.load_for_agent( + "impl", ("enabled", "disabled_child") + ).bottle + self.assertFalse(result.nested_containers) + self.assertFalse(result.supervise) + def test_three_bottles_merged_left_to_right(self): b1 = _bottle(env={"A": "1", "B": "1", "C": "1"}) b2 = _bottle(env={"B": "2", "C": "2"}) diff --git a/tests/unit/test_manifest_extends.py b/tests/unit/test_manifest_extends.py index 2083e77..92b4588 100644 --- a/tests/unit/test_manifest_extends.py +++ b/tests/unit/test_manifest_extends.py @@ -68,6 +68,23 @@ class TestExtendsBasic(unittest.TestCase): self.assertTrue(m.bottles["base"].supervise) self.assertFalse(m.bottles["off"].supervise) + def test_child_overrides_nested_containers_scalar(self): + m = _build( + base={"nested_containers": True}, + off={"extends": "base", "nested_containers": False}, + ) + self.assertTrue(m.bottles["base"].nested_containers) + self.assertFalse(m.bottles["off"].nested_containers) + + def test_inherited_boolean_declaration_is_preserved(self): + m = _build( + base={"nested_containers": True, "supervise": False}, + child={"extends": "base"}, + ) + child = m.bottles["child"] + self.assertIn("nested_containers", child.declared_fields) + self.assertIn("supervise", child.declared_fields) + def test_parent_resolved_once_for_multiple_children(self): # Two children sharing one parent: both inherit; the parent # is resolved once + cached. (Cache behavior is internal; we @@ -477,6 +494,26 @@ class TestExtendsMultiParent(unittest.TestCase): ) self.assertTrue(m.bottles["child"].supervise) + def test_later_parent_omitting_boole_preserves_earlier_values(self): + m = _build( + p1={"nested_containers": True, "supervise": False}, + p2={"env": {"FROM_P2": "1"}}, + child={"extends": ["p1", "p2"]}, + ) + child = m.bottles["child"] + self.assertTrue(child.nested_containers) + self.assertFalse(child.supervise) + + def test_later_parent_explicit_boole_override_earlier_values(self): + m = _build( + p1={"nested_containers": True, "supervise": False}, + p2={"nested_containers": False, "supervise": True}, + child={"extends": ["p1", "p2"]}, + ) + child = m.bottles["child"] + self.assertFalse(child.nested_containers) + self.assertTrue(child.supervise) + def test_child_supervise_overrides_all_parents(self): m = _build( p1={"supervise": True},