fix(manifest): preserve declared boolean fields
tracker-policy-pr / check-pr (pull_request) Successful in 11s
test / integration-docker (pull_request) Successful in 18s
test / unit (pull_request) Successful in 42s
test / integration-firecracker (pull_request) Successful in 3m35s
test / coverage (pull_request) Successful in 26s
test / publish-infra (pull_request) Has been skipped
prd-number / assign-numbers (push) Failing after 29s
test / integration-docker (push) Successful in 44s
lint / lint (push) Successful in 1m0s
test / unit (push) Successful in 56s
Update Quality Badges / update-badges (push) Successful in 1m45s
test / integration-firecracker (push) Successful in 6m54s
test / coverage (push) Successful in 26s
test / publish-infra (push) Successful in 2m22s
tracker-policy-pr / check-pr (pull_request) Successful in 11s
test / integration-docker (pull_request) Successful in 18s
test / unit (pull_request) Successful in 42s
test / integration-firecracker (pull_request) Successful in 3m35s
test / coverage (pull_request) Successful in 26s
test / publish-infra (pull_request) Has been skipped
prd-number / assign-numbers (push) Failing after 29s
test / integration-docker (push) Successful in 44s
lint / lint (push) Successful in 1m0s
test / unit (push) Successful in 56s
Update Quality Badges / update-badges (push) Successful in 1m45s
test / integration-firecracker (push) Successful in 6m54s
test / coverage (push) Successful in 26s
test / publish-infra (push) Successful in 2m22s
This commit was merged in pull request #456.
This commit is contained in:
@@ -49,12 +49,10 @@ class ManifestBottle:
|
|||||||
# costs image weight, a resident service, and relaxed guest device modes
|
# costs image weight, a resident service, and relaxed guest device modes
|
||||||
# that the majority of bottles never need.
|
# that the majority of bottles never need.
|
||||||
nested_containers: bool = False
|
nested_containers: bool = False
|
||||||
# True when nested_containers was explicitly present in the source dict.
|
# Source fields retained across extends/runtime composition. Boolean
|
||||||
# Used by the runtime merge path to distinguish "omitted → default False"
|
# defaults otherwise erase the distinction between "omitted" and an
|
||||||
# from "explicitly set to False", so an explicit false in a later bottle
|
# explicitly declared value (especially False).
|
||||||
# can override an earlier true while a bottle that never mentions the key
|
declared_fields: frozenset[str] = frozenset()
|
||||||
# does not silently drop the capability.
|
|
||||||
nested_containers_explicit: bool = False
|
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def from_dict(cls, name: str, raw: object) -> "ManifestBottle":
|
def from_dict(cls, name: str, raw: object) -> "ManifestBottle":
|
||||||
@@ -145,5 +143,5 @@ class ManifestBottle:
|
|||||||
env=env, agent_provider=agent_provider, git=git,
|
env=env, agent_provider=agent_provider, git=git,
|
||||||
git_user=git_user, egress=egress, supervise=supervise_raw,
|
git_user=git_user, egress=egress, supervise=supervise_raw,
|
||||||
nested_containers=nested_raw,
|
nested_containers=nested_raw,
|
||||||
nested_containers_explicit="nested_containers" in d,
|
declared_fields=frozenset(d),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -8,6 +8,17 @@ from .manifest_git import ManifestGitUser, parse_git_gate_config
|
|||||||
from .manifest_util import ManifestError, as_json_object
|
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":
|
def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle":
|
||||||
"""Merge an ordered list of pre-resolved ManifestBottle objects.
|
"""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:
|
routes: concatenate; agent_provider, supervise, nested_containers:
|
||||||
later replaces (presence-aware).
|
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
|
was loaded from a source that explicitly declared the key, its value
|
||||||
wins (so an explicit `false` can override an earlier `true`). If the
|
wins (so an explicit `false` can override an earlier `true`). If the
|
||||||
later bottle never mentioned the key, the earlier value is preserved
|
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:
|
if not bottles:
|
||||||
raise ValueError("merge_bottles_runtime requires at least one bottle")
|
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_routes = base.egress.routes + override.egress.routes
|
||||||
merged_egress = ManifestEgressConfig(routes=merged_routes, Log=override.egress.Log)
|
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(
|
return ManifestBottle(
|
||||||
env=merged_env,
|
env=merged_env,
|
||||||
agent_provider=override.agent_provider,
|
agent_provider=override.agent_provider,
|
||||||
git=merged_git,
|
git=merged_git,
|
||||||
git_user=merged_git_user,
|
git_user=merged_git_user,
|
||||||
egress=merged_egress,
|
egress=merged_egress,
|
||||||
supervise=override.supervise,
|
supervise=_overlay_declared_bool(base, override, "supervise"),
|
||||||
nested_containers=merged_nc,
|
nested_containers=_overlay_declared_bool(
|
||||||
nested_containers_explicit=base.nested_containers_explicit or override.nested_containers_explicit,
|
base, override, "nested_containers"
|
||||||
|
),
|
||||||
|
declared_fields=base.declared_fields | override.declared_fields,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -213,20 +220,17 @@ def _fold_two_bottles(
|
|||||||
Log=later.egress.Log,
|
Log=later.egress.Log,
|
||||||
)
|
)
|
||||||
|
|
||||||
folded_nc = (
|
|
||||||
later.nested_containers
|
|
||||||
if later.nested_containers_explicit
|
|
||||||
else earlier.nested_containers
|
|
||||||
)
|
|
||||||
return ManifestBottle(
|
return ManifestBottle(
|
||||||
env=merged_env,
|
env=merged_env,
|
||||||
agent_provider=later.agent_provider,
|
agent_provider=later.agent_provider,
|
||||||
git=merged_git,
|
git=merged_git,
|
||||||
git_user=merged_git_user,
|
git_user=merged_git_user,
|
||||||
egress=merged_egress,
|
egress=merged_egress,
|
||||||
supervise=later.supervise,
|
supervise=_overlay_declared_bool(earlier, later, "supervise"),
|
||||||
nested_containers=folded_nc,
|
nested_containers=_overlay_declared_bool(
|
||||||
nested_containers_explicit=earlier.nested_containers_explicit or later.nested_containers_explicit,
|
earlier, later, "nested_containers"
|
||||||
|
),
|
||||||
|
declared_fields=earlier.declared_fields | later.declared_fields,
|
||||||
), merged_repos_raw
|
), merged_repos_raw
|
||||||
|
|
||||||
|
|
||||||
@@ -284,13 +288,9 @@ def _merge_bottles(
|
|||||||
if "agent_provider" in child_raw
|
if "agent_provider" in child_raw
|
||||||
else parent.agent_provider
|
else parent.agent_provider
|
||||||
)
|
)
|
||||||
merged_supervise = (
|
merged_supervise = _overlay_declared_bool(parent, child, "supervise")
|
||||||
child.supervise if "supervise" in child_raw else parent.supervise
|
merged_nested_containers = _overlay_declared_bool(
|
||||||
)
|
parent, child, "nested_containers"
|
||||||
merged_nested_containers = (
|
|
||||||
child.nested_containers
|
|
||||||
if "nested_containers" in child_raw
|
|
||||||
else parent.nested_containers
|
|
||||||
)
|
)
|
||||||
validate_egress_routes(name, merged_egress.routes)
|
validate_egress_routes(name, merged_egress.routes)
|
||||||
|
|
||||||
@@ -302,6 +302,7 @@ def _merge_bottles(
|
|||||||
egress=merged_egress,
|
egress=merged_egress,
|
||||||
supervise=merged_supervise,
|
supervise=merged_supervise,
|
||||||
nested_containers=merged_nested_containers,
|
nested_containers=merged_nested_containers,
|
||||||
|
declared_fields=parent.declared_fields | child.declared_fields,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -56,6 +56,16 @@ class TestMergeBottlesRuntime(unittest.TestCase):
|
|||||||
result = merge_bottles_runtime([base, override])
|
result = merge_bottles_runtime([base, override])
|
||||||
self.assertFalse(result.supervise)
|
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):
|
def test_nested_containers_survives_a_later_bottle_that_omits_the_key(self):
|
||||||
"""A bottle that never mentions nested_containers must not silently
|
"""A bottle that never mentions nested_containers must not silently
|
||||||
drop the capability: `--bottle with-docker --bottle claude-dev`."""
|
drop the capability: `--bottle with-docker --bottle claude-dev`."""
|
||||||
@@ -77,6 +87,25 @@ class TestMergeBottlesRuntime(unittest.TestCase):
|
|||||||
disabled = _bottle(nested_containers=False)
|
disabled = _bottle(nested_containers=False)
|
||||||
self.assertTrue(merge_bottles_runtime([disabled, enabled]).nested_containers)
|
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):
|
def test_three_bottles_merged_left_to_right(self):
|
||||||
b1 = _bottle(env={"A": "1", "B": "1", "C": "1"})
|
b1 = _bottle(env={"A": "1", "B": "1", "C": "1"})
|
||||||
b2 = _bottle(env={"B": "2", "C": "2"})
|
b2 = _bottle(env={"B": "2", "C": "2"})
|
||||||
|
|||||||
@@ -68,6 +68,23 @@ class TestExtendsBasic(unittest.TestCase):
|
|||||||
self.assertTrue(m.bottles["base"].supervise)
|
self.assertTrue(m.bottles["base"].supervise)
|
||||||
self.assertFalse(m.bottles["off"].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):
|
def test_parent_resolved_once_for_multiple_children(self):
|
||||||
# Two children sharing one parent: both inherit; the parent
|
# Two children sharing one parent: both inherit; the parent
|
||||||
# is resolved once + cached. (Cache behavior is internal; we
|
# is resolved once + cached. (Cache behavior is internal; we
|
||||||
@@ -477,6 +494,26 @@ class TestExtendsMultiParent(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertTrue(m.bottles["child"].supervise)
|
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):
|
def test_child_supervise_overrides_all_parents(self):
|
||||||
m = _build(
|
m = _build(
|
||||||
p1={"supervise": True},
|
p1={"supervise": True},
|
||||||
|
|||||||
Reference in New Issue
Block a user