From f943e1489191d7aeb315c8bc282ff58027b10de9 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 11 May 2026 16:50:22 -0400 Subject: [PATCH] refactor(pipelock): take stage_dir, derive yaml_path internally PipelockProxy.prepare now accepts (bottle, slug, stage_dir) and derives the yaml_path itself, so callers don't need to know the filename. DockerBottleBackend.prepare_proxy becomes a one-line wrapper whose only caller already has bottle and slug in scope, so it's inlined and deleted. --- claude_bottle/backend/docker/backend.py | 12 +----------- claude_bottle/pipelock.py | 5 +++-- tests/integration/test_pipelock_sidecar_smoke.py | 3 +-- tests/unit/test_pipelock_yaml.py | 14 +++++++------- 4 files changed, 12 insertions(+), 22 deletions(-) diff --git a/claude_bottle/backend/docker/backend.py b/claude_bottle/backend/docker/backend.py index 70a880a..3f97db3 100644 --- a/claude_bottle/backend/docker/backend.py +++ b/claude_bottle/backend/docker/backend.py @@ -104,7 +104,7 @@ class DockerBottleBackend(BottleBackend): prompt_file.write_text("") prompt_file.chmod(0o600) - proxy_plan = self.prepare_proxy(spec, stage_dir) + proxy_plan = self._proxy.prepare(bottle, slug, stage_dir) resolved = resolve_env(manifest, spec.agent_name) self._write_env_files(resolved, env_file, args_file) prompt_file.write_text(agent.prompt) @@ -151,16 +151,6 @@ class DockerBottleBackend(BottleBackend): args_lines = [f"-e\n{name}" for name in resolved.forwarded] args_file.write_text("\n".join(args_lines) + ("\n" if args_lines else "")) - def prepare_proxy(self, spec: BottleSpec, stage_dir: Path) -> pipelock.PipelockProxyPlan: - """Decide where the pipelock yaml lives in `stage_dir`, delegate - to PipelockProxy to write it, and return the resolved - PipelockProxyPlan for the launch step to consume. Stage-only: - no Docker resources created yet.""" - yaml_path = stage_dir / "pipelock.yaml" - bottle = spec.manifest.bottle_for(spec.agent_name) - slug = docker_mod.slugify(spec.agent_name) - return self._proxy.prepare(bottle, slug, yaml_path) - @contextmanager def launch(self, plan: BottlePlan) -> Iterator[DockerBottle]: """Build, launch, and provision a Docker bottle. Teardown on exit.""" diff --git a/claude_bottle/pipelock.py b/claude_bottle/pipelock.py index 9661c93..2a28041 100644 --- a/claude_bottle/pipelock.py +++ b/claude_bottle/pipelock.py @@ -177,9 +177,9 @@ class PipelockProxy(ABC): and lives on concrete subclasses.""" def prepare( - self, bottle: Bottle, slug: str, yaml_path: Path + self, bottle: Bottle, slug: str, stage_dir: Path ) -> PipelockProxyPlan: - """Write the pipelock yaml config (mode 600) to `yaml_path` + """Write the pipelock yaml config (mode 600) under `stage_dir` and return the plan for `.start`. `slug` is the agent-derived identifier (lowercased, @@ -188,6 +188,7 @@ class PipelockProxy(ABC): (`claude-bottle-pipelock-`), the internal/egress networks. It's stored on the returned plan so the backend's start step can derive the sidecar's container name.""" + yaml_path = stage_dir / "pipelock.yaml" self._build_pipelock_yaml(bottle, yaml_path) return PipelockProxyPlan(yaml_path=yaml_path, slug=slug) diff --git a/tests/integration/test_pipelock_sidecar_smoke.py b/tests/integration/test_pipelock_sidecar_smoke.py index 0ed030b..028a71b 100644 --- a/tests/integration/test_pipelock_sidecar_smoke.py +++ b/tests/integration/test_pipelock_sidecar_smoke.py @@ -68,8 +68,7 @@ class TestPipelockSidecarSmoke(unittest.TestCase): def test_prepare_and_start_yield_healthy_sidecar(self): proxy = DockerPipelockProxy() - yaml_path = self.work_dir / "pipelock.yaml" - prep = proxy.prepare(fixture_minimal().bottles["dev"], self.slug, yaml_path) + prep = proxy.prepare(fixture_minimal().bottles["dev"], self.slug, self.work_dir) self.internal_net = network_create_internal(self.slug) self.egress_net = network_create_egress(self.slug) diff --git a/tests/unit/test_pipelock_yaml.py b/tests/unit/test_pipelock_yaml.py index 2ed6aa7..4618e62 100644 --- a/tests/unit/test_pipelock_yaml.py +++ b/tests/unit/test_pipelock_yaml.py @@ -66,11 +66,10 @@ class TestRenderAndWrite(unittest.TestCase): self.assertIn(required, text) def test_prepare_writes_file_at_mode_600(self): - yaml_path = self.out_dir / "min.yaml" - DockerPipelockProxy().prepare( - fixture_minimal().bottles["dev"], "demo", yaml_path + plan = DockerPipelockProxy().prepare( + fixture_minimal().bottles["dev"], "demo", self.out_dir ) - self.assertEqual(0o600, os.stat(yaml_path).st_mode & 0o777) + self.assertEqual(0o600, os.stat(plan.yaml_path).st_mode & 0o777) def test_prepare_does_not_leak_env_names_or_values(self): manifest = Manifest.from_json_obj({ @@ -85,9 +84,10 @@ class TestRenderAndWrite(unittest.TestCase): }, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) - yaml_path = self.out_dir / "secret.yaml" - DockerPipelockProxy().prepare(manifest.bottles["dev"], "demo", yaml_path) - content = yaml_path.read_text() + plan = DockerPipelockProxy().prepare( + manifest.bottles["dev"], "demo", self.out_dir + ) + content = plan.yaml_path.read_text() self.assertNotIn("literal-value-should-not-appear", content) self.assertNotIn("MY_SECRET", content) self.assertNotIn("prompt-message", content)