fix(manifest): OR nested_containers across composed bottles
tracker-policy-pr / check-pr (pull_request) Successful in 13s
test / integration-docker (pull_request) Successful in 38s
test / unit (pull_request) Successful in 40s
lint / lint (push) Successful in 50s
test / integration-firecracker (pull_request) Successful in 3m22s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Successful in 13s
test / integration-docker (pull_request) Successful in 38s
test / unit (pull_request) Successful in 40s
lint / lint (push) Successful in 50s
test / integration-firecracker (pull_request) Successful in 3m22s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
merge_bottles_runtime sees resolved bottles, where a bottle that never mentioned the key is indistinguishable from one that set it false. With "later replaces" semantics, `--bottle with-containers --bottle claude-dev` silently dropped the capability — the second bottle's default clobbered it. OR instead: composing bottles adds capabilities, it does not remove them. The file-based extends path still sees raw keys, so an explicit `nested_containers: false` in a child continues to turn it back off. Also surface the flag in the launch summary, where its absence was the only reason the clobber went unnoticed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -524,6 +524,8 @@ def _manifest_to_yaml(manifest: Manifest) -> str:
|
||||
lines.append(f" scheme: {r.AuthScheme}")
|
||||
|
||||
lines.append(f" supervise: {'true' if bottle.supervise else 'false'}")
|
||||
if bottle.nested_containers:
|
||||
lines.append(" nested_containers: true")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ Bottle schema (frontmatter):
|
||||
egress: { routes: [ <egress-route>, ... ] }
|
||||
# route keys: host, matches, auth, role, dlp
|
||||
supervise: <bool> # optional (default true)
|
||||
nested_containers: <bool> # optional (default false)
|
||||
|
||||
Agent schema (frontmatter):
|
||||
bottle: <bottle-name> # required
|
||||
|
||||
@@ -15,8 +15,16 @@ 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,
|
||||
nested_containers: later replaces.
|
||||
routes: concatenate; agent_provider, supervise: later
|
||||
replaces; nested_containers: OR (see below).
|
||||
|
||||
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.
|
||||
"""
|
||||
if not bottles:
|
||||
raise ValueError("merge_bottles_runtime requires at least one bottle")
|
||||
@@ -55,7 +63,7 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle
|
||||
git_user=merged_git_user,
|
||||
egress=merged_egress,
|
||||
supervise=override.supervise,
|
||||
nested_containers=override.nested_containers,
|
||||
nested_containers=base.nested_containers or override.nested_containers,
|
||||
)
|
||||
|
||||
|
||||
@@ -208,7 +216,7 @@ def _fold_two_bottles(
|
||||
git_user=merged_git_user,
|
||||
egress=merged_egress,
|
||||
supervise=later.supervise,
|
||||
nested_containers=later.nested_containers,
|
||||
nested_containers=earlier.nested_containers or later.nested_containers,
|
||||
), merged_repos_raw
|
||||
|
||||
|
||||
|
||||
@@ -56,11 +56,15 @@ class TestMergeBottlesRuntime(unittest.TestCase):
|
||||
result = merge_bottles_runtime([base, override])
|
||||
self.assertFalse(result.supervise)
|
||||
|
||||
def test_nested_containers_later_wins(self):
|
||||
base = _bottle(nested_containers=True)
|
||||
override = _bottle(nested_containers=False)
|
||||
self.assertFalse(merge_bottles_runtime([base, override]).nested_containers)
|
||||
self.assertTrue(merge_bottles_runtime([override, base]).nested_containers)
|
||||
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."""
|
||||
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_three_bottles_merged_left_to_right(self):
|
||||
b1 = _bottle(env={"A": "1", "B": "1", "C": "1"})
|
||||
|
||||
Reference in New Issue
Block a user