From bf8ff91b3100f3fca77e019df5873e809eaa011b Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 22 Jul 2026 17:58:00 +0000 Subject: [PATCH] fix(manifest): presence-aware runtime merge for nested_containers The runtime merge (merge_bottles_runtime / _fold_two_bottles) was using OR semantics for nested_containers, which meant `nested_containers: false` in a later bottle could never override an earlier `true`. Fix: add `nested_containers_explicit: bool` to ManifestBottle, set to True only when the key is present in the source dict. The runtime merge functions now use the override's value only when it was explicitly supplied; otherwise the base's value is preserved. This matches the documented last-wins / presence-aware semantics used by supervise and agent_provider, and aligns with _merge_bottles (the file-based extends path) which already did presence checks via `"nested_containers" in child_raw`. Tests: rename existing test to clarify the preserved-when-omitted case, add explicit-false-overrides-true and explicit-true-overrides-false cases. Co-Authored-By: Claude Sonnet 4.6 --- bot_bottle/manifest_bottle.py | 7 ++++++ bot_bottle/manifest_extends.py | 32 ++++++++++++++++-------- tests/unit/test_manifest_bottle_merge.py | 19 +++++++++++--- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/bot_bottle/manifest_bottle.py b/bot_bottle/manifest_bottle.py index 455a926..e67303e 100644 --- a/bot_bottle/manifest_bottle.py +++ b/bot_bottle/manifest_bottle.py @@ -49,6 +49,12 @@ 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 @classmethod def from_dict(cls, name: str, raw: object) -> "ManifestBottle": @@ -139,4 +145,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, ) diff --git a/bot_bottle/manifest_extends.py b/bot_bottle/manifest_extends.py index 36fdd40..f57ec6b 100644 --- a/bot_bottle/manifest_extends.py +++ b/bot_bottle/manifest_extends.py @@ -15,16 +15,14 @@ def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle": the same field-merge rules as the file-based extends machinery: env: dict merge, later wins; git_user: per-field overlay, later wins on non-empty; git (repos): union by name, later wins; egress - routes: concatenate; agent_provider, supervise: later - replaces; nested_containers: OR (see below). + routes: concatenate; agent_provider, supervise, nested_containers: + later replaces (presence-aware). - nested_containers is OR'd rather than replaced because these objects - are already resolved: a bottle that never mentions the key is - indistinguishable from one that sets it false, so "later replaces" - would let any bottle composed after a container-enabled one silently - drop the capability. The file-based `extends:` path still sees the - raw keys, so there an explicit `nested_containers: false` in a child - turns it back off. + nested_containers uses 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). """ if not bottles: raise ValueError("merge_bottles_runtime requires at least one bottle") @@ -56,6 +54,11 @@ 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, @@ -63,7 +66,8 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle git_user=merged_git_user, egress=merged_egress, supervise=override.supervise, - nested_containers=base.nested_containers or override.nested_containers, + nested_containers=merged_nc, + nested_containers_explicit=base.nested_containers_explicit or override.nested_containers_explicit, ) @@ -209,6 +213,11 @@ 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, @@ -216,7 +225,8 @@ def _fold_two_bottles( git_user=merged_git_user, egress=merged_egress, supervise=later.supervise, - nested_containers=earlier.nested_containers or later.nested_containers, + nested_containers=folded_nc, + nested_containers_explicit=earlier.nested_containers_explicit or later.nested_containers_explicit, ), merged_repos_raw diff --git a/tests/unit/test_manifest_bottle_merge.py b/tests/unit/test_manifest_bottle_merge.py index 9aad8cb..4a656bc 100644 --- a/tests/unit/test_manifest_bottle_merge.py +++ b/tests/unit/test_manifest_bottle_merge.py @@ -56,16 +56,27 @@ class TestMergeBottlesRuntime(unittest.TestCase): result = merge_bottles_runtime([base, override]) self.assertFalse(result.supervise) - def test_nested_containers_survives_a_later_bottle(self): - """OR, not replace: a resolved bottle that never mentioned the key is - indistinguishable from one that set it false, so `--bottle with-docker - --bottle claude-dev` must not silently drop the capability.""" + 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`.""" enabled = _bottle(nested_containers=True) quiet = _bottle(env={"X": "1"}) self.assertTrue(merge_bottles_runtime([enabled, quiet]).nested_containers) self.assertTrue(merge_bottles_runtime([quiet, enabled]).nested_containers) self.assertFalse(merge_bottles_runtime([quiet, quiet]).nested_containers) + def test_nested_containers_explicit_false_overrides_earlier_true(self): + """An explicit nested_containers: false in a later bottle must win + over an earlier true (last-wins, presence-aware).""" + enabled = _bottle(nested_containers=True) + disabled = _bottle(nested_containers=False) + self.assertFalse(merge_bottles_runtime([enabled, disabled]).nested_containers) + + def test_nested_containers_explicit_true_overrides_earlier_false(self): + enabled = _bottle(nested_containers=True) + disabled = _bottle(nested_containers=False) + self.assertTrue(merge_bottles_runtime([disabled, enabled]).nested_containers) + 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"})