refactor(manifest): convert TypedDict to frozen dataclasses
test / run tests/run_tests.py (pull_request) Successful in 14s
test / run tests/run_tests.py (pull_request) Successful in 14s
Replace the TypedDict + 14 manifest_* free functions with frozen dataclasses (SshEntry, BottleEgress, Bottle, Agent, Manifest) carrying their own validators and constructors. Call sites import Manifest and chain attribute access; the manifest_* helpers and manifest_validate are gone. Behavior changes worth flagging: - Agent.bottle is now required (was optional with a "(none)" fallback). Manifest.from_json_obj dies if any agent lacks a 'bottle' field or references an undefined bottle, where previously start.py raised the error lazily for the specific agent being launched. - ssh.py now takes SshEntry instances; Host/IdentityFile shape checks moved upstream into Manifest construction, leaving only the IdentityFile filesystem-existence check in ssh_validate_entries. - pipelock_bottle_allowlist's per-element string check is dropped — the Manifest validator enforces it at load. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+27
-8
@@ -1,4 +1,7 @@
|
||||
"""Manifest fixtures for the test suite."""
|
||||
"""Manifest fixtures for the test suite. Each fixture returns a built
|
||||
Manifest dataclass; callers that need the raw JSON shape (e.g. to write
|
||||
to a file on disk) can build it themselves or call .from_json_obj on
|
||||
a dict literal in the test."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -7,9 +10,11 @@ import tempfile
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from claude_bottle.manifest import Manifest
|
||||
|
||||
def fixture_minimal() -> dict[str, Any]:
|
||||
"""One bottle, one agent, no env / ssh / skills."""
|
||||
|
||||
def fixture_minimal_dict() -> dict[str, Any]:
|
||||
"""One bottle, one agent, no env / ssh / skills. JSON shape."""
|
||||
return {
|
||||
"bottles": {"dev": {}},
|
||||
"agents": {
|
||||
@@ -18,8 +23,8 @@ def fixture_minimal() -> dict[str, Any]:
|
||||
}
|
||||
|
||||
|
||||
def fixture_with_egress() -> dict[str, Any]:
|
||||
"""Bottle declares an egress.allowlist."""
|
||||
def fixture_with_egress_dict() -> dict[str, Any]:
|
||||
"""Bottle declares an egress.allowlist. JSON shape."""
|
||||
return {
|
||||
"bottles": {
|
||||
"dev": {
|
||||
@@ -32,9 +37,9 @@ def fixture_with_egress() -> dict[str, Any]:
|
||||
}
|
||||
|
||||
|
||||
def fixture_with_ssh() -> dict[str, Any]:
|
||||
def fixture_with_ssh_dict() -> dict[str, Any]:
|
||||
"""Bottle has both an IPv4-literal SSH host (CGNAT) and a hostname host,
|
||||
exercising both ssrf.ip_allowlist and trusted_domains code paths."""
|
||||
exercising both ssrf.ip_allowlist and trusted_domains code paths. JSON shape."""
|
||||
return {
|
||||
"bottles": {
|
||||
"dev": {
|
||||
@@ -60,8 +65,22 @@ def fixture_with_ssh() -> dict[str, Any]:
|
||||
}
|
||||
|
||||
|
||||
def fixture_minimal() -> Manifest:
|
||||
return Manifest.from_json_obj(fixture_minimal_dict())
|
||||
|
||||
|
||||
def fixture_with_egress() -> Manifest:
|
||||
return Manifest.from_json_obj(fixture_with_egress_dict())
|
||||
|
||||
|
||||
def fixture_with_ssh() -> Manifest:
|
||||
return Manifest.from_json_obj(fixture_with_ssh_dict())
|
||||
|
||||
|
||||
def write_fixture(fn) -> Path:
|
||||
"""Write fixture dict to a temp file; return the path. Caller must rm."""
|
||||
"""Write fixture JSON to a temp file; return the path. Caller must rm.
|
||||
Accepts a function returning either a dict (JSON shape) or a Manifest;
|
||||
only the dict form is supported here since we need to serialize."""
|
||||
f = tempfile.NamedTemporaryFile(
|
||||
mode="w", suffix=".json", delete=False, encoding="utf-8"
|
||||
)
|
||||
|
||||
@@ -1,16 +1,18 @@
|
||||
"""Unit: bottle runtime — manifest_bottle_runtime returns the configured
|
||||
runtime (defaulting to runc); manifest_validate rejects unknown values,
|
||||
non-strings, and empty strings."""
|
||||
"""Unit: bottle runtime — Manifest.from_json_obj defaults runtime to runc,
|
||||
accepts runsc, and rejects unknown values, non-strings, and empty strings."""
|
||||
|
||||
import unittest
|
||||
|
||||
from claude_bottle.log import Die
|
||||
from claude_bottle.manifest import manifest_bottle_runtime, manifest_validate
|
||||
from claude_bottle.manifest import Manifest
|
||||
|
||||
|
||||
def _bottle(runtime_value: object | None) -> dict:
|
||||
"""Build a minimal manifest with one bottle whose runtime field is
|
||||
set (or absent if `runtime_value is _ABSENT`)."""
|
||||
_ABSENT = object()
|
||||
|
||||
|
||||
def _bottle(runtime_value: object) -> dict:
|
||||
"""Build a minimal manifest JSON shape with one bottle whose runtime
|
||||
field is set (or absent if `runtime_value is _ABSENT`)."""
|
||||
bottle: dict = {}
|
||||
if runtime_value is not _ABSENT:
|
||||
bottle["runtime"] = runtime_value
|
||||
@@ -20,30 +22,30 @@ def _bottle(runtime_value: object | None) -> dict:
|
||||
}
|
||||
|
||||
|
||||
_ABSENT = object()
|
||||
|
||||
|
||||
class TestManifestBottleRuntime(unittest.TestCase):
|
||||
def test_default_runc_when_absent(self):
|
||||
self.assertEqual("runc", manifest_bottle_runtime(_bottle(_ABSENT), "dev"))
|
||||
m = Manifest.from_json_obj(_bottle(_ABSENT))
|
||||
self.assertEqual("runc", m.bottles["dev"].runtime)
|
||||
|
||||
def test_explicit_runc(self):
|
||||
self.assertEqual("runc", manifest_bottle_runtime(_bottle("runc"), "dev"))
|
||||
m = Manifest.from_json_obj(_bottle("runc"))
|
||||
self.assertEqual("runc", m.bottles["dev"].runtime)
|
||||
|
||||
def test_explicit_runsc(self):
|
||||
self.assertEqual("runsc", manifest_bottle_runtime(_bottle("runsc"), "dev"))
|
||||
m = Manifest.from_json_obj(_bottle("runsc"))
|
||||
self.assertEqual("runsc", m.bottles["dev"].runtime)
|
||||
|
||||
def test_rejects_unknown_runtime(self):
|
||||
with self.assertRaises(Die):
|
||||
manifest_validate(_bottle("kata-runtime"))
|
||||
Manifest.from_json_obj(_bottle("kata-runtime"))
|
||||
|
||||
def test_rejects_non_string(self):
|
||||
with self.assertRaises(Die):
|
||||
manifest_validate(_bottle(42))
|
||||
Manifest.from_json_obj(_bottle(42))
|
||||
|
||||
def test_rejects_empty_string(self):
|
||||
with self.assertRaises(Die):
|
||||
manifest_validate(_bottle(""))
|
||||
Manifest.from_json_obj(_bottle(""))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -5,6 +5,7 @@ pipelock_bottle_ssh_trusted_domains, pipelock_effective_allowlist."""
|
||||
import unittest
|
||||
|
||||
from claude_bottle.log import Die
|
||||
from claude_bottle.manifest import Manifest
|
||||
from claude_bottle.pipelock import (
|
||||
pipelock_bottle_allowlist,
|
||||
pipelock_bottle_ssh_hostnames,
|
||||
@@ -32,7 +33,7 @@ class TestBottleAllowlist(unittest.TestCase):
|
||||
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||
}
|
||||
with self.assertRaises(Die):
|
||||
pipelock_bottle_allowlist(bad, "dev")
|
||||
Manifest.from_json_obj(bad)
|
||||
|
||||
|
||||
class TestSSHHostnames(unittest.TestCase):
|
||||
@@ -54,7 +55,7 @@ class TestSSHHostnames(unittest.TestCase):
|
||||
|
||||
class TestEffectiveAllowlist(unittest.TestCase):
|
||||
def test_union_and_dedup(self):
|
||||
manifest = {
|
||||
manifest = Manifest.from_json_obj({
|
||||
"bottles": {
|
||||
"dev": {
|
||||
"egress": {"allowlist": ["registry.npmjs.org"]},
|
||||
@@ -67,7 +68,7 @@ class TestEffectiveAllowlist(unittest.TestCase):
|
||||
}
|
||||
},
|
||||
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||
}
|
||||
})
|
||||
eff = pipelock_effective_allowlist(manifest, "dev")
|
||||
self.assertIn("api.anthropic.com", eff)
|
||||
self.assertIn("registry.npmjs.org", eff)
|
||||
|
||||
@@ -7,6 +7,7 @@ import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
from claude_bottle.manifest import Manifest
|
||||
from claude_bottle.pipelock import pipelock_write_yaml
|
||||
from tests.fixtures import fixture_minimal, fixture_with_ssh
|
||||
|
||||
@@ -50,7 +51,7 @@ class TestPipelockYaml(unittest.TestCase):
|
||||
self.assertIn("100.78.141.42", content)
|
||||
|
||||
def test_secret_hygiene(self):
|
||||
manifest = {
|
||||
manifest = Manifest.from_json_obj({
|
||||
"bottles": {
|
||||
"dev": {
|
||||
"env": {
|
||||
@@ -61,7 +62,7 @@ class TestPipelockYaml(unittest.TestCase):
|
||||
}
|
||||
},
|
||||
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||
}
|
||||
})
|
||||
yaml_path = self.out_dir / "secret.yaml"
|
||||
pipelock_write_yaml(manifest, "dev", yaml_path)
|
||||
content = yaml_path.read_text()
|
||||
|
||||
Reference in New Issue
Block a user