fix(bottle): identity-key all per-bottle resources by (agent, cwd)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -17,60 +17,90 @@ the same way the original does.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import hashlib
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from ... import supervise as _supervise
|
from ... import supervise as _supervise
|
||||||
|
from . import util as docker_mod
|
||||||
|
|
||||||
|
|
||||||
# Directory layout: ~/.claude-bottle/state/<slug>/...
|
# Directory layout: ~/.claude-bottle/state/<identity>/...
|
||||||
_STATE_SUBDIR = "state"
|
_STATE_SUBDIR = "state"
|
||||||
_PER_BOTTLE_DOCKERFILE_NAME = "Dockerfile"
|
_PER_BOTTLE_DOCKERFILE_NAME = "Dockerfile"
|
||||||
_TRANSCRIPT_SUBDIR = "transcript"
|
_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)-<sha256(resolved-cwd)[:N]>`
|
||||||
|
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
|
"""Per-bottle state directory on the host. Created lazily by the
|
||||||
write helpers; readers tolerate its absence."""
|
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:
|
def per_bottle_dockerfile_path(identity: str) -> Path:
|
||||||
return bottle_state_dir(slug) / _PER_BOTTLE_DOCKERFILE_NAME
|
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
|
"""Return the per-bottle Dockerfile content if present, else
|
||||||
None. None means: use the repo's Dockerfile (the original
|
None. None means: use the repo's Dockerfile (the original
|
||||||
pre-capability-block behavior)."""
|
pre-capability-block behavior)."""
|
||||||
p = per_bottle_dockerfile_path(slug)
|
p = per_bottle_dockerfile_path(identity)
|
||||||
if p.is_file():
|
if p.is_file():
|
||||||
return p.read_text()
|
return p.read_text()
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
def write_per_bottle_dockerfile(slug: str, content: str) -> Path:
|
def write_per_bottle_dockerfile(identity: str, content: str) -> Path:
|
||||||
p = per_bottle_dockerfile_path(slug)
|
p = per_bottle_dockerfile_path(identity)
|
||||||
p.parent.mkdir(parents=True, exist_ok=True)
|
p.parent.mkdir(parents=True, exist_ok=True)
|
||||||
p.write_text(content)
|
p.write_text(content)
|
||||||
p.chmod(0o644)
|
p.chmod(0o644)
|
||||||
return p
|
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
|
"""Image tag for a rebuilt bottle. Distinct from the base
|
||||||
claude-bottle:latest so per-bottle rebuilds don't collide in
|
claude-bottle:latest so per-bottle rebuilds don't collide in
|
||||||
the docker image cache."""
|
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
|
"""Where capability_apply stashes the agent's transcript before
|
||||||
teardown, so the next `cli.py start <agent>` can offer to
|
teardown, so the next `cli.py start <agent>` can offer to
|
||||||
resume from it."""
|
resume from it."""
|
||||||
return bottle_state_dir(slug) / _TRANSCRIPT_SUBDIR
|
return bottle_state_dir(identity) / _TRANSCRIPT_SUBDIR
|
||||||
|
|
||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
|
"bottle_identity",
|
||||||
"bottle_state_dir",
|
"bottle_state_dir",
|
||||||
"per_bottle_dockerfile",
|
"per_bottle_dockerfile",
|
||||||
"per_bottle_dockerfile_path",
|
"per_bottle_dockerfile_path",
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ from .cred_proxy import (
|
|||||||
)
|
)
|
||||||
from .git_gate import DockerGitGate, git_gate_container_name
|
from .git_gate import DockerGitGate, git_gate_container_name
|
||||||
from .bottle_state import (
|
from .bottle_state import (
|
||||||
|
bottle_identity,
|
||||||
per_bottle_dockerfile,
|
per_bottle_dockerfile,
|
||||||
per_bottle_dockerfile_path,
|
per_bottle_dockerfile_path,
|
||||||
per_bottle_image_tag,
|
per_bottle_image_tag,
|
||||||
@@ -53,7 +54,16 @@ def resolve_plan(
|
|||||||
agent = manifest.agents[spec.agent_name]
|
agent = manifest.agents[spec.agent_name]
|
||||||
bottle = manifest.bottle_for(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
|
# PRD 0016 capability-block: if a per-bottle Dockerfile has been
|
||||||
# written (via apply_capability_change), the base image becomes
|
# written (via apply_capability_change), the base image becomes
|
||||||
|
|||||||
@@ -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 tempfile
|
||||||
import unittest
|
import unittest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -68,5 +69,48 @@ class TestPerBottleDockerfile(_FakeHomeMixin, unittest.TestCase):
|
|||||||
self.assertTrue(str(path).endswith("/.claude-bottle/state/dev/transcript"))
|
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__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user