feat(state): clean up per-bottle state on session end (except capability-block)
Previously every bottle launch left ~/.claude-bottle/state/<identity>/ behind forever — metadata.json on every run, plus per-bottle Dockerfile + transcript snapshot on capability-block rebuilds. The metadata accumulated debris across launches; the only state worth keeping was the capability-block rebuild bundle. Make cleanup the default; preserve only on capability-block. - bottle_state.py: .preserve marker helpers (mark_preserved, is_preserved, clear_preserve_marker, preserve_marker_path) + cleanup_state(identity) that rm -rf's the per-bottle dir. - capability_apply.apply_capability_change writes mark_preserved before teardown so cli.py's session-end cleanup keeps the dir. - prepare.py clears any leftover marker at launch (start or resume), so a marker from a prior capability-block doesn't keep state alive past a subsequent normal session-end. - cli/start.py runs the cleanup decision AFTER the launch context closes: if is_preserved → print resume hint; else cleanup_state. The resume hint moves out of the launch with-block (was previously printed unconditionally — would have misled the operator about whether state was actually kept). Future-proof: cli.py never persists state speculatively. If the agent wants to be resumable, it has to go through capability-block. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -45,6 +45,10 @@ _STATE_SUBDIR = "state"
|
||||
_PER_BOTTLE_DOCKERFILE_NAME = "Dockerfile"
|
||||
_TRANSCRIPT_SUBDIR = "transcript"
|
||||
_METADATA_NAME = "metadata.json"
|
||||
# Empty marker file. capability_apply writes it before teardown so
|
||||
# cli.py's session-end cleanup knows to preserve the state dir for
|
||||
# `cli.py resume <identity>`. Absent = clean up.
|
||||
_PRESERVE_MARKER = ".preserve"
|
||||
|
||||
# 5 chars of base36 alphabet ≈ 60M combinations. Plenty for human
|
||||
# operators starting bottles by hand; collision-free in practice.
|
||||
@@ -155,14 +159,61 @@ def transcript_snapshot_dir(identity: str) -> Path:
|
||||
return bottle_state_dir(identity) / _TRANSCRIPT_SUBDIR
|
||||
|
||||
|
||||
# --- Preserve-on-close marker ----------------------------------------------
|
||||
|
||||
|
||||
def preserve_marker_path(identity: str) -> Path:
|
||||
return bottle_state_dir(identity) / _PRESERVE_MARKER
|
||||
|
||||
|
||||
def mark_preserved(identity: str) -> Path:
|
||||
"""Mark this bottle's state for preservation across session
|
||||
teardown. Written by capability_apply.apply_capability_change so
|
||||
cli.py's session-end cleanup leaves the state dir intact for a
|
||||
subsequent `cli.py resume`."""
|
||||
path = preserve_marker_path(identity)
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.touch()
|
||||
return path
|
||||
|
||||
|
||||
def is_preserved(identity: str) -> bool:
|
||||
return preserve_marker_path(identity).exists()
|
||||
|
||||
|
||||
def clear_preserve_marker(identity: str) -> None:
|
||||
"""Idempotent removal. Called at fresh launch (start or resume)
|
||||
so a marker left from a prior capability-block doesn't keep
|
||||
state alive past the next normal session-end."""
|
||||
try:
|
||||
preserve_marker_path(identity).unlink()
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
|
||||
|
||||
def cleanup_state(identity: str) -> None:
|
||||
"""Remove the per-bottle state dir entirely. Called by cli.py
|
||||
when a bottle session ends and is_preserved(identity) is False.
|
||||
Idempotent — missing dir is success."""
|
||||
import shutil
|
||||
state_dir = bottle_state_dir(identity)
|
||||
if state_dir.is_dir():
|
||||
shutil.rmtree(state_dir, ignore_errors=True)
|
||||
|
||||
|
||||
__all__ = [
|
||||
"BottleMetadata",
|
||||
"bottle_identity",
|
||||
"bottle_state_dir",
|
||||
"cleanup_state",
|
||||
"clear_preserve_marker",
|
||||
"is_preserved",
|
||||
"mark_preserved",
|
||||
"metadata_path",
|
||||
"per_bottle_dockerfile",
|
||||
"per_bottle_dockerfile_path",
|
||||
"per_bottle_image_tag",
|
||||
"preserve_marker_path",
|
||||
"read_metadata",
|
||||
"transcript_snapshot_dir",
|
||||
"write_metadata",
|
||||
|
||||
@@ -37,6 +37,7 @@ from pathlib import Path
|
||||
|
||||
from ...log import info, warn
|
||||
from .bottle_state import (
|
||||
mark_preserved,
|
||||
per_bottle_dockerfile,
|
||||
per_bottle_dockerfile_path,
|
||||
transcript_snapshot_dir,
|
||||
@@ -117,6 +118,11 @@ def apply_capability_change(slug: str, new_dockerfile: str) -> tuple[str, str]:
|
||||
_snapshot_transcript(slug)
|
||||
_push_working_tree(slug)
|
||||
write_per_bottle_dockerfile(slug, new_dockerfile)
|
||||
# Set the preserve marker BEFORE teardown so cli.py's session-end
|
||||
# cleanup sees it and keeps the state dir intact for the
|
||||
# operator's `cli.py resume <identity>`. Without the marker the
|
||||
# state dir would be deleted as part of normal session end.
|
||||
mark_preserved(slug)
|
||||
_teardown_bottle(slug)
|
||||
|
||||
return before, new_dockerfile
|
||||
|
||||
@@ -30,6 +30,7 @@ from .git_gate import DockerGitGate, git_gate_container_name
|
||||
from .bottle_state import (
|
||||
BottleMetadata,
|
||||
bottle_identity,
|
||||
clear_preserve_marker,
|
||||
per_bottle_dockerfile,
|
||||
per_bottle_dockerfile_path,
|
||||
per_bottle_image_tag,
|
||||
@@ -73,6 +74,10 @@ def resolve_plan(
|
||||
copy_cwd=spec.copy_cwd,
|
||||
started_at=datetime.now(timezone.utc).isoformat(),
|
||||
))
|
||||
# Clear any leftover preserve marker from a prior capability-block
|
||||
# so this fresh launch can be cleaned up at session-end unless
|
||||
# the agent triggers another capability-block.
|
||||
clear_preserve_marker(slug)
|
||||
|
||||
# PRD 0016 capability-block: if a per-bottle Dockerfile has been
|
||||
# written (via apply_capability_change), the base image becomes
|
||||
|
||||
@@ -17,6 +17,7 @@ import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from ..backend import BottleSpec, get_bottle_backend
|
||||
from ..backend.docker.bottle_state import cleanup_state, is_preserved
|
||||
from ..log import die, info
|
||||
from ..manifest import Manifest
|
||||
from ._common import PROG, USER_CWD, read_tty_line
|
||||
@@ -99,13 +100,29 @@ def _launch_bottle(
|
||||
claude_args.append("--remote-control")
|
||||
bottle.exec_claude(claude_args, tty=True)
|
||||
info(f"session ended; container {bottle.name} will be removed")
|
||||
if identity:
|
||||
info(f"to resume this bottle: ./cli.py resume {identity}")
|
||||
return 0
|
||||
# Context exited → containers + networks gone. Now decide
|
||||
# what to do with the per-bottle state dir on the host:
|
||||
# capability-block apply sets the preserve marker before it
|
||||
# tears the bottle down, so the operator can resume from the
|
||||
# new Dockerfile + transcript snapshot. Any other session
|
||||
# end (normal exit, agent crash, Ctrl-C) leaves no marker,
|
||||
# and the state dir gets cleaned up so ~/.claude-bottle/state/
|
||||
# doesn't accumulate per-launch debris.
|
||||
_settle_state(identity)
|
||||
return 0
|
||||
finally:
|
||||
shutil.rmtree(stage_dir, ignore_errors=True)
|
||||
|
||||
|
||||
def _settle_state(identity: str) -> None:
|
||||
if not identity:
|
||||
return
|
||||
if is_preserved(identity):
|
||||
info(f"to resume this bottle: ./cli.py resume {identity}")
|
||||
return
|
||||
cleanup_state(identity)
|
||||
|
||||
|
||||
def _identity_from_plan(plan: object) -> str:
|
||||
"""Backend-specific: the docker plan exposes the identity as
|
||||
`.slug`. Other backends in the future would expose their own
|
||||
|
||||
@@ -114,6 +114,60 @@ class TestBottleIdentity(unittest.TestCase):
|
||||
self.assertTrue(identity.startswith("my-agent-"))
|
||||
|
||||
|
||||
class TestPreserveMarker(_FakeHomeMixin, unittest.TestCase):
|
||||
"""The .preserve marker is how capability_apply tells cli.py's
|
||||
session-end cleanup to keep the state dir instead of removing it."""
|
||||
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
|
||||
def tearDown(self):
|
||||
self._teardown_fake_home()
|
||||
|
||||
def test_default_is_unpreserved(self):
|
||||
self.assertFalse(bottle_state.is_preserved("dev-x"))
|
||||
|
||||
def test_mark_then_is_preserved(self):
|
||||
bottle_state.mark_preserved("dev-x")
|
||||
self.assertTrue(bottle_state.is_preserved("dev-x"))
|
||||
|
||||
def test_clear_removes_marker(self):
|
||||
bottle_state.mark_preserved("dev-x")
|
||||
bottle_state.clear_preserve_marker("dev-x")
|
||||
self.assertFalse(bottle_state.is_preserved("dev-x"))
|
||||
|
||||
def test_clear_is_idempotent(self):
|
||||
# No marker present — should not raise.
|
||||
bottle_state.clear_preserve_marker("never-existed")
|
||||
self.assertFalse(bottle_state.is_preserved("never-existed"))
|
||||
|
||||
def test_marker_path_under_state_dir(self):
|
||||
path = bottle_state.preserve_marker_path("dev-x")
|
||||
self.assertTrue(str(path).endswith("/.claude-bottle/state/dev-x/.preserve"))
|
||||
|
||||
|
||||
class TestCleanupState(_FakeHomeMixin, unittest.TestCase):
|
||||
"""cleanup_state removes the entire per-bottle state dir.
|
||||
Called by cli.py when a session ends without the preserve marker."""
|
||||
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
|
||||
def tearDown(self):
|
||||
self._teardown_fake_home()
|
||||
|
||||
def test_removes_state_dir_and_contents(self):
|
||||
bottle_state.write_per_bottle_dockerfile("dev-x", "FROM x\n")
|
||||
d = bottle_state.bottle_state_dir("dev-x")
|
||||
self.assertTrue(d.is_dir())
|
||||
bottle_state.cleanup_state("dev-x")
|
||||
self.assertFalse(d.exists())
|
||||
|
||||
def test_idempotent_when_dir_missing(self):
|
||||
# Never created — should not raise.
|
||||
bottle_state.cleanup_state("never-existed")
|
||||
|
||||
|
||||
class TestBottleMetadata(_FakeHomeMixin, unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
|
||||
@@ -107,6 +107,14 @@ class TestApplyCapabilityChange(_FakeHomeMixin, unittest.TestCase):
|
||||
self._calls,
|
||||
)
|
||||
|
||||
def test_marks_preserved_before_teardown(self):
|
||||
# cli.py's session-end cleanup reads the marker after the
|
||||
# bottle is torn down. The marker must therefore be written
|
||||
# before teardown — otherwise the cleanup would see no
|
||||
# marker and rm the state dir we just populated.
|
||||
apply_capability_change("dev", "FROM new\n")
|
||||
self.assertTrue(bottle_state.is_preserved("dev"))
|
||||
|
||||
def test_first_change_falls_back_to_repo_dockerfile_for_before(self):
|
||||
# No per-bottle override yet — before-diff comes from the
|
||||
# repo's Dockerfile.
|
||||
|
||||
Reference in New Issue
Block a user