From e996f72532b56156713fa682e18c5d0afe620e2f Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 25 May 2026 05:46:26 -0400 Subject: [PATCH] fix(bottle): identity-key all per-bottle resources by (agent, cwd) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single point that computed `slug = slugify(agent_name)` in prepare.py is now `slug = bottle_identity(agent_name, cwd)`. With --cwd the identity has a sha256(resolved-cwd)[:12] suffix, so the same agent against different projects gets distinct container names, network names, queue dir, audit log paths, and per-bottle state (Dockerfile + transcript). Without --cwd the identity is just slugify(agent_name), unchanged from before — no-cwd bottles look the same as today. The downstream `slug` field on DockerBottlePlan keeps its name — every module already threads it under "slug" and the value flowing through is now the bottle's full identity. A comment in prepare.py flags the change. Fixes the bug surfaced in PR #22 review: running the same agent against project-A's cwd then project-B's would silently share project-A's per-bottle Dockerfile + transcript snapshot, container name (forcing serialized runs), and queue/audit history. Co-Authored-By: Claude Opus 4.7 --- claude_bottle/backend/docker/bottle_state.py | 56 +++++++++++++++----- claude_bottle/backend/docker/prepare.py | 12 ++++- tests/unit/test_bottle_state.py | 46 +++++++++++++++- 3 files changed, 99 insertions(+), 15 deletions(-) diff --git a/claude_bottle/backend/docker/bottle_state.py b/claude_bottle/backend/docker/bottle_state.py index 55f635c..e168828 100644 --- a/claude_bottle/backend/docker/bottle_state.py +++ b/claude_bottle/backend/docker/bottle_state.py @@ -17,60 +17,90 @@ the same way the original does. from __future__ import annotations +import hashlib from pathlib import Path from ... import supervise as _supervise +from . import util as docker_mod -# Directory layout: ~/.claude-bottle/state//... +# Directory layout: ~/.claude-bottle/state//... _STATE_SUBDIR = "state" _PER_BOTTLE_DOCKERFILE_NAME = "Dockerfile" _TRANSCRIPT_SUBDIR = "transcript" +# How many hex chars of the cwd hash to fold into the identity. 12 +# hex chars = 48 bits of entropy; the cost of a collision is two +# unrelated cwds sharing the same state — annoying but not security- +# relevant. 12 keeps the identity short enough to stay readable in +# container names and `ls` output. +_CWD_HASH_LEN = 12 -def bottle_state_dir(slug: str) -> Path: + +def bottle_identity(agent_name: str, cwd: Path | None) -> str: + """Stable, unique identifier for a bottle. Used as the key for + every persistent and runtime resource: container names, network + names, queue dir, audit log, per-bottle Dockerfile state. + + Without --cwd, the identity is just `slugify(agent_name)` — the + same value the codebase used to compute as `slug`. With --cwd + the identity is `slugify(agent_name)-` + so the same agent against different projects gets distinct + state. Same agent against the same cwd is stable across launches. + + `cwd` should be the path the agent will see, *resolved* by the + caller, or None when no cwd was passed (no --cwd flag).""" + slug = docker_mod.slugify(agent_name) + if cwd is None: + return slug + h = hashlib.sha256(str(cwd).encode("utf-8")).hexdigest() + return f"{slug}-{h[:_CWD_HASH_LEN]}" + + +def bottle_state_dir(identity: str) -> Path: """Per-bottle state directory on the host. Created lazily by the write helpers; readers tolerate its absence.""" - return _supervise.claude_bottle_root() / _STATE_SUBDIR / slug + return _supervise.claude_bottle_root() / _STATE_SUBDIR / identity -def per_bottle_dockerfile_path(slug: str) -> Path: - return bottle_state_dir(slug) / _PER_BOTTLE_DOCKERFILE_NAME +def per_bottle_dockerfile_path(identity: str) -> Path: + return bottle_state_dir(identity) / _PER_BOTTLE_DOCKERFILE_NAME -def per_bottle_dockerfile(slug: str) -> str | None: +def per_bottle_dockerfile(identity: str) -> str | None: """Return the per-bottle Dockerfile content if present, else None. None means: use the repo's Dockerfile (the original pre-capability-block behavior).""" - p = per_bottle_dockerfile_path(slug) + p = per_bottle_dockerfile_path(identity) if p.is_file(): return p.read_text() return None -def write_per_bottle_dockerfile(slug: str, content: str) -> Path: - p = per_bottle_dockerfile_path(slug) +def write_per_bottle_dockerfile(identity: str, content: str) -> Path: + p = per_bottle_dockerfile_path(identity) p.parent.mkdir(parents=True, exist_ok=True) p.write_text(content) p.chmod(0o644) return p -def per_bottle_image_tag(slug: str) -> str: +def per_bottle_image_tag(identity: str) -> str: """Image tag for a rebuilt bottle. Distinct from the base claude-bottle:latest so per-bottle rebuilds don't collide in the docker image cache.""" - return f"claude-bottle-rebuilt-{slug}:latest" + return f"claude-bottle-rebuilt-{identity}:latest" -def transcript_snapshot_dir(slug: str) -> Path: +def transcript_snapshot_dir(identity: str) -> Path: """Where capability_apply stashes the agent's transcript before teardown, so the next `cli.py start ` can offer to resume from it.""" - return bottle_state_dir(slug) / _TRANSCRIPT_SUBDIR + return bottle_state_dir(identity) / _TRANSCRIPT_SUBDIR __all__ = [ + "bottle_identity", "bottle_state_dir", "per_bottle_dockerfile", "per_bottle_dockerfile_path", diff --git a/claude_bottle/backend/docker/prepare.py b/claude_bottle/backend/docker/prepare.py index e879960..8f74c89 100644 --- a/claude_bottle/backend/docker/prepare.py +++ b/claude_bottle/backend/docker/prepare.py @@ -27,6 +27,7 @@ from .cred_proxy import ( ) from .git_gate import DockerGitGate, git_gate_container_name from .bottle_state import ( + bottle_identity, per_bottle_dockerfile, per_bottle_dockerfile_path, per_bottle_image_tag, @@ -53,7 +54,16 @@ def resolve_plan( agent = manifest.agents[spec.agent_name] bottle = manifest.bottle_for(spec.agent_name) - slug = docker_mod.slugify(spec.agent_name) + # PRD 0016 follow-up: identity, not bare slug. With --cwd, the + # identity carries a sha256(cwd) suffix so the same agent against + # different projects gets distinct container names, networks, + # queue + audit + state dirs. Without --cwd, identity == + # slugify(agent_name) — same value the old `slug` produced — so + # no-cwd bottles look unchanged. We keep the variable named `slug` + # because every downstream module already threads it under that + # name; the value is now the bottle's full identity. + cwd_for_identity = Path(spec.user_cwd).resolve() if spec.copy_cwd else None + slug = bottle_identity(spec.agent_name, cwd_for_identity) # PRD 0016 capability-block: if a per-bottle Dockerfile has been # written (via apply_capability_change), the base image becomes diff --git a/tests/unit/test_bottle_state.py b/tests/unit/test_bottle_state.py index 59f6ee2..b3a31cb 100644 --- a/tests/unit/test_bottle_state.py +++ b/tests/unit/test_bottle_state.py @@ -1,5 +1,6 @@ -"""Unit: per-bottle state helpers (PRD 0016 Phase 1).""" +"""Unit: per-bottle state helpers (PRD 0016 Phase 1) + identity.""" +import re import tempfile import unittest from pathlib import Path @@ -68,5 +69,48 @@ class TestPerBottleDockerfile(_FakeHomeMixin, unittest.TestCase): self.assertTrue(str(path).endswith("/.claude-bottle/state/dev/transcript")) +class TestBottleIdentity(unittest.TestCase): + """bottle_identity(agent_name, cwd) — PRD 0016 follow-up. + + Without --cwd, identity == slugify(agent_name) so existing + no-cwd bottles look unchanged. With --cwd, identity has a + cwd-hash suffix so the same agent against different projects + gets distinct container / queue / audit / state dirs.""" + + def test_no_cwd_returns_slug(self): + self.assertEqual("dev", bottle_state.bottle_identity("dev", None)) + self.assertEqual("api-foo", bottle_state.bottle_identity("Api Foo", None)) + + def test_cwd_appends_hash_suffix(self): + identity = bottle_state.bottle_identity("dev", Path("/proj/A")) + self.assertTrue(identity.startswith("dev-")) + suffix = identity[len("dev-"):] + self.assertEqual(12, len(suffix)) + self.assertTrue(re.fullmatch(r"[0-9a-f]+", suffix), suffix) + + def test_same_cwd_same_identity(self): + a = bottle_state.bottle_identity("dev", Path("/proj/A")) + b = bottle_state.bottle_identity("dev", Path("/proj/A")) + self.assertEqual(a, b) + + def test_different_cwds_differ(self): + a = bottle_state.bottle_identity("dev", Path("/proj/A")) + b = bottle_state.bottle_identity("dev", Path("/proj/B")) + self.assertNotEqual(a, b) + + def test_different_agents_same_cwd_differ(self): + a = bottle_state.bottle_identity("dev", Path("/proj/A")) + b = bottle_state.bottle_identity("api", Path("/proj/A")) + self.assertNotEqual(a, b) + + def test_agent_name_slugified(self): + # Identity's agent-name prefix is slugify(name), not the raw + # name — same rule the rest of the codebase has always used. + self.assertEqual( + "my-agent", + bottle_state.bottle_identity("My Agent", None), + ) + + if __name__ == "__main__": unittest.main()