refactor(start): extract prepare_with_preflight + attach_claude
PRD 0020 chunk 1. `cli/start.py`'s `_launch_bottle` did three
things in one function: prepare + preflight, attach claude, and
settle state on teardown. Split them so the dashboard (PRD 0020
chunk 2+) can reuse the prepare + attach pieces piecewise
without going through the CLI's one-shot orchestrator:
- `prepare_with_preflight(spec, *, stage_dir, render_preflight,
prompt_yes, dry_run)` — injects render + prompt callables so
the CLI binds them to stderr/stdin while the dashboard binds
them to a curses modal. Returns `(plan, identity)`; identity
is set after `backend.prepare` returns so callers can reap
the prepare-time state dir on abort via `settle_state` in
their finally — preserving today's preflight-N cleanup.
- `attach_claude(bottle, *, remote_control)` — runs claude
inside the bottle and returns its exit code. The dashboard
calls this from inside a `curses.endwin` → … →
`stdscr.refresh()` handoff.
- `capture_session_state` / `settle_state` lose their leading
underscore; the dashboard will call them on
session-end + explicit-stop respectively.
`_launch_bottle` becomes a thin orchestrator over those helpers.
No behavior change; all 453 unit tests pass and `./cli.py start
implementer --dry-run` produces identical preflight output.
This commit is contained in:
+116
-49
@@ -2,8 +2,10 @@
|
|||||||
interactive claude-code session. The container is torn down when the
|
interactive claude-code session. The container is torn down when the
|
||||||
session ends.
|
session ends.
|
||||||
|
|
||||||
The launch core is shared with `cli.py resume <identity>`: see
|
The launch core is shared with `cli.py resume <identity>` and (PRD
|
||||||
_launch_bottle below.
|
0020 chunk 1+) the dashboard's in-process start flow: see the
|
||||||
|
public helpers `prepare_with_preflight`, `attach_claude`, and the
|
||||||
|
private orchestrator `_launch_bottle`.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
@@ -14,8 +16,10 @@ import shutil
|
|||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from typing import Callable
|
||||||
|
|
||||||
from ..backend import BottleSpec, get_bottle_backend
|
from ..backend import Bottle, BottleSpec, get_bottle_backend
|
||||||
|
from ..backend.docker.bottle_plan import DockerBottlePlan
|
||||||
from ..backend.docker.bottle_state import (
|
from ..backend.docker.bottle_state import (
|
||||||
cleanup_state,
|
cleanup_state,
|
||||||
is_preserved,
|
is_preserved,
|
||||||
@@ -51,36 +55,52 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _launch_bottle(
|
# --- Public helpers shared with the dashboard (PRD 0020) -----------------
|
||||||
|
|
||||||
|
|
||||||
|
def prepare_with_preflight(
|
||||||
spec: BottleSpec,
|
spec: BottleSpec,
|
||||||
*,
|
*,
|
||||||
dry_run: bool,
|
stage_dir: Path,
|
||||||
remote_control: bool,
|
render_preflight: Callable[[DockerBottlePlan], None],
|
||||||
) -> int:
|
prompt_yes: Callable[[], bool],
|
||||||
"""Shared launch core for `start` and `resume`. Builds the plan,
|
dry_run: bool = False,
|
||||||
prints / dry-runs / prompts as appropriate, brings the bottle up,
|
) -> tuple[DockerBottlePlan | None, str]:
|
||||||
attaches claude, and prints the resume hint on session end."""
|
"""Run `backend.prepare`, render the preflight summary via the
|
||||||
stage_dir = Path(tempfile.mkdtemp(prefix="claude-bottle-stage."))
|
injected callable, prompt y/N via the injected callable. The CLI
|
||||||
identity = ""
|
binds these to stderr/stdin; the dashboard binds them to a
|
||||||
try:
|
curses modal.
|
||||||
|
|
||||||
|
Returns `(plan, identity)`. `plan` is None on dry-run or
|
||||||
|
operator-N, but `identity` is set as soon as `backend.prepare`
|
||||||
|
returns so callers can reap the prepare-time state dir via
|
||||||
|
`settle_state(identity)` in their finally — exactly the existing
|
||||||
|
semantics."""
|
||||||
backend = get_bottle_backend()
|
backend = get_bottle_backend()
|
||||||
plan = backend.prepare(spec, stage_dir=stage_dir)
|
plan = backend.prepare(spec, stage_dir=stage_dir)
|
||||||
identity = _identity_from_plan(plan)
|
identity = _identity_from_plan(plan)
|
||||||
|
|
||||||
plan.print(remote_control=remote_control)
|
render_preflight(plan)
|
||||||
|
|
||||||
if dry_run:
|
if dry_run:
|
||||||
info("dry-run requested; not starting container.")
|
info("dry-run requested; not starting container.")
|
||||||
return 0
|
return None, identity
|
||||||
|
if not prompt_yes():
|
||||||
sys.stderr.write("claude-bottle: launch this agent? [y/N] ")
|
|
||||||
sys.stderr.flush()
|
|
||||||
reply = read_tty_line()
|
|
||||||
if reply not in ("y", "Y", "yes", "YES"):
|
|
||||||
info("aborted by user")
|
info("aborted by user")
|
||||||
return 0
|
return None, identity
|
||||||
|
return plan, identity
|
||||||
|
|
||||||
with backend.launch(plan) as bottle:
|
|
||||||
|
def attach_claude(
|
||||||
|
bottle: Bottle, *, remote_control: bool = False,
|
||||||
|
) -> int:
|
||||||
|
"""Run claude inside `bottle` as an interactive session. Blocks
|
||||||
|
until the session ends; returns the claude process's exit code.
|
||||||
|
|
||||||
|
Used as the inner step of `./cli.py start` (one-shot) and by the
|
||||||
|
dashboard (PRD 0020), which calls it from inside a `curses.endwin
|
||||||
|
→ … → stdscr.refresh()` handoff so the curses surface gets out
|
||||||
|
of the terminal's way while claude has it."""
|
||||||
info(
|
info(
|
||||||
"attaching interactive claude session "
|
"attaching interactive claude session "
|
||||||
"(Ctrl-D or 'exit' to leave; container will be removed)"
|
"(Ctrl-D or 'exit' to leave; container will be removed)"
|
||||||
@@ -88,36 +108,14 @@ def _launch_bottle(
|
|||||||
claude_args = ["--dangerously-skip-permissions"]
|
claude_args = ["--dangerously-skip-permissions"]
|
||||||
if remote_control:
|
if remote_control:
|
||||||
claude_args.append("--remote-control")
|
claude_args.append("--remote-control")
|
||||||
exit_code = bottle.exec_claude(claude_args, tty=True)
|
return bottle.exec_claude(claude_args, tty=True)
|
||||||
info(
|
|
||||||
f"session ended (exit {exit_code}); "
|
|
||||||
f"container {bottle.name} will be removed"
|
|
||||||
)
|
|
||||||
# While the container is still alive: always snapshot the
|
|
||||||
# transcript and — if the agent exited non-zero — mark
|
|
||||||
# the state for preservation. Capability-block already
|
|
||||||
# did both before triggering teardown from the dashboard;
|
|
||||||
# this picks up crashes / Ctrl-Cs / OOM kills the same
|
|
||||||
# way. snapshot_transcript is best-effort so the
|
|
||||||
# capability-block path's prior snapshot isn't clobbered
|
|
||||||
# when the container is already gone.
|
|
||||||
_capture_session_state(identity, exit_code)
|
|
||||||
return 0
|
|
||||||
finally:
|
|
||||||
# PRD 0018 chunk 2: prepare now writes the bottle's bind-mount
|
|
||||||
# sources under state/<slug>/. If we never reached the
|
|
||||||
# launch context (dry-run, preflight-N, prepare exception), or
|
|
||||||
# we did but nothing requested preservation, reap them along
|
|
||||||
# with everything else. _settle_state subsumes the prior
|
|
||||||
# post-launch settlement and the new pre-launch cleanup.
|
|
||||||
_settle_state(identity)
|
|
||||||
shutil.rmtree(stage_dir, ignore_errors=True)
|
|
||||||
|
|
||||||
|
|
||||||
def _capture_session_state(identity: str, exit_code: int) -> None:
|
def capture_session_state(identity: str, exit_code: int) -> None:
|
||||||
"""Inside the launch context, while the container is still
|
"""Inside the launch context, while the container is still
|
||||||
alive: snapshot the transcript and mark for preservation if
|
alive: snapshot the transcript and mark for preservation if
|
||||||
claude crashed. Pure-function-ish; tests stub the helpers."""
|
claude crashed. Public for the dashboard's death-handling path
|
||||||
|
(PRD 0020 open question 3)."""
|
||||||
if not identity:
|
if not identity:
|
||||||
return
|
return
|
||||||
snapshot_transcript(identity)
|
snapshot_transcript(identity)
|
||||||
@@ -125,7 +123,11 @@ def _capture_session_state(identity: str, exit_code: int) -> None:
|
|||||||
mark_preserved(identity)
|
mark_preserved(identity)
|
||||||
|
|
||||||
|
|
||||||
def _settle_state(identity: str) -> None:
|
def settle_state(identity: str) -> None:
|
||||||
|
"""Post-teardown housekeeping: print the resume hint if the
|
||||||
|
state was preserved, otherwise reap the per-bottle state dir.
|
||||||
|
Public so the dashboard's explicit-stop path calls the same
|
||||||
|
settlement the CLI uses on context exit."""
|
||||||
if not identity:
|
if not identity:
|
||||||
return
|
return
|
||||||
if is_preserved(identity):
|
if is_preserved(identity):
|
||||||
@@ -140,3 +142,68 @@ def _identity_from_plan(plan: object) -> str:
|
|||||||
identity attribute; for now we duck-type to keep this layer
|
identity attribute; for now we duck-type to keep this layer
|
||||||
backend-agnostic."""
|
backend-agnostic."""
|
||||||
return getattr(plan, "slug", "")
|
return getattr(plan, "slug", "")
|
||||||
|
|
||||||
|
|
||||||
|
def _text_prompt_yes() -> bool:
|
||||||
|
"""Default `prompt_yes` for CLI use: reads y/N from the
|
||||||
|
controlling tty via stderr prompt + tty-line read."""
|
||||||
|
sys.stderr.write("claude-bottle: launch this agent? [y/N] ")
|
||||||
|
sys.stderr.flush()
|
||||||
|
reply = read_tty_line()
|
||||||
|
return reply in ("y", "Y", "yes", "YES")
|
||||||
|
|
||||||
|
|
||||||
|
def _text_render_preflight(*, remote_control: bool):
|
||||||
|
def _render(plan: DockerBottlePlan) -> None:
|
||||||
|
plan.print(remote_control=remote_control)
|
||||||
|
return _render
|
||||||
|
|
||||||
|
|
||||||
|
def _launch_bottle(
|
||||||
|
spec: BottleSpec,
|
||||||
|
*,
|
||||||
|
dry_run: bool,
|
||||||
|
remote_control: bool,
|
||||||
|
) -> int:
|
||||||
|
"""Shared launch core for `start` and `resume`. Builds the plan,
|
||||||
|
prints / dry-runs / prompts as appropriate, brings the bottle up,
|
||||||
|
attaches claude, and prints the resume hint on session end."""
|
||||||
|
stage_dir = Path(tempfile.mkdtemp(prefix="claude-bottle-stage."))
|
||||||
|
identity = ""
|
||||||
|
try:
|
||||||
|
plan, identity = prepare_with_preflight(
|
||||||
|
spec,
|
||||||
|
stage_dir=stage_dir,
|
||||||
|
render_preflight=_text_render_preflight(remote_control=remote_control),
|
||||||
|
prompt_yes=_text_prompt_yes,
|
||||||
|
dry_run=dry_run,
|
||||||
|
)
|
||||||
|
if plan is None:
|
||||||
|
return 0
|
||||||
|
|
||||||
|
backend = get_bottle_backend()
|
||||||
|
with backend.launch(plan) as bottle:
|
||||||
|
exit_code = attach_claude(bottle, remote_control=remote_control)
|
||||||
|
info(
|
||||||
|
f"session ended (exit {exit_code}); "
|
||||||
|
f"container {bottle.name} will be removed"
|
||||||
|
)
|
||||||
|
# While the container is still alive: always snapshot the
|
||||||
|
# transcript and — if the agent exited non-zero — mark
|
||||||
|
# the state for preservation. Capability-block already
|
||||||
|
# did both before triggering teardown from the dashboard;
|
||||||
|
# this picks up crashes / Ctrl-Cs / OOM kills the same
|
||||||
|
# way. snapshot_transcript is best-effort so the
|
||||||
|
# capability-block path's prior snapshot isn't clobbered
|
||||||
|
# when the container is already gone.
|
||||||
|
capture_session_state(identity, exit_code)
|
||||||
|
return 0
|
||||||
|
finally:
|
||||||
|
# PRD 0018 chunk 2: prepare now writes the bottle's bind-mount
|
||||||
|
# sources under state/<slug>/. If we never reached the
|
||||||
|
# launch context (dry-run, preflight-N, prepare exception), or
|
||||||
|
# we did but nothing requested preservation, reap them along
|
||||||
|
# with everything else. `settle_state` subsumes the prior
|
||||||
|
# post-launch settlement and the new pre-launch cleanup.
|
||||||
|
settle_state(identity)
|
||||||
|
shutil.rmtree(stage_dir, ignore_errors=True)
|
||||||
|
|||||||
@@ -45,25 +45,25 @@ class TestCaptureSessionState(_FakeHomeMixin, unittest.TestCase):
|
|||||||
self._teardown_fake_home()
|
self._teardown_fake_home()
|
||||||
|
|
||||||
def test_clean_exit_snapshots_but_does_not_mark(self):
|
def test_clean_exit_snapshots_but_does_not_mark(self):
|
||||||
start_mod._capture_session_state("dev-abc", exit_code=0)
|
start_mod.capture_session_state("dev-abc", exit_code=0)
|
||||||
self.assertEqual(["dev-abc"], self._snap_calls)
|
self.assertEqual(["dev-abc"], self._snap_calls)
|
||||||
self.assertFalse(bottle_state.is_preserved("dev-abc"))
|
self.assertFalse(bottle_state.is_preserved("dev-abc"))
|
||||||
|
|
||||||
def test_crash_snapshots_and_marks(self):
|
def test_crash_snapshots_and_marks(self):
|
||||||
start_mod._capture_session_state("dev-abc", exit_code=137)
|
start_mod.capture_session_state("dev-abc", exit_code=137)
|
||||||
self.assertEqual(["dev-abc"], self._snap_calls)
|
self.assertEqual(["dev-abc"], self._snap_calls)
|
||||||
self.assertTrue(bottle_state.is_preserved("dev-abc"))
|
self.assertTrue(bottle_state.is_preserved("dev-abc"))
|
||||||
|
|
||||||
def test_ctrl_c_treated_as_crash(self):
|
def test_ctrl_c_treated_as_crash(self):
|
||||||
# SIGINT delivers exit 130; the operator may have Ctrl-C'd
|
# SIGINT delivers exit 130; the operator may have Ctrl-C'd
|
||||||
# because something went wrong, so we preserve.
|
# because something went wrong, so we preserve.
|
||||||
start_mod._capture_session_state("dev-abc", exit_code=130)
|
start_mod.capture_session_state("dev-abc", exit_code=130)
|
||||||
self.assertTrue(bottle_state.is_preserved("dev-abc"))
|
self.assertTrue(bottle_state.is_preserved("dev-abc"))
|
||||||
|
|
||||||
def test_empty_identity_is_noop(self):
|
def test_empty_identity_is_noop(self):
|
||||||
# Backends without an identity field shouldn't crash this
|
# Backends without an identity field shouldn't crash this
|
||||||
# path (the _identity_from_plan helper falls back to "").
|
# path (the _identity_from_plan helper falls back to "").
|
||||||
start_mod._capture_session_state("", exit_code=137)
|
start_mod.capture_session_state("", exit_code=137)
|
||||||
self.assertEqual([], self._snap_calls)
|
self.assertEqual([], self._snap_calls)
|
||||||
|
|
||||||
|
|
||||||
@@ -77,16 +77,16 @@ class TestSettleState(_FakeHomeMixin, unittest.TestCase):
|
|||||||
def test_preserved_state_survives(self):
|
def test_preserved_state_survives(self):
|
||||||
bottle_state.write_per_bottle_dockerfile("dev-abc", "FROM x\n")
|
bottle_state.write_per_bottle_dockerfile("dev-abc", "FROM x\n")
|
||||||
bottle_state.mark_preserved("dev-abc")
|
bottle_state.mark_preserved("dev-abc")
|
||||||
start_mod._settle_state("dev-abc")
|
start_mod.settle_state("dev-abc")
|
||||||
self.assertTrue(bottle_state.bottle_state_dir("dev-abc").is_dir())
|
self.assertTrue(bottle_state.bottle_state_dir("dev-abc").is_dir())
|
||||||
|
|
||||||
def test_unpreserved_state_is_cleaned(self):
|
def test_unpreserved_state_is_cleaned(self):
|
||||||
bottle_state.write_per_bottle_dockerfile("dev-abc", "FROM x\n")
|
bottle_state.write_per_bottle_dockerfile("dev-abc", "FROM x\n")
|
||||||
start_mod._settle_state("dev-abc")
|
start_mod.settle_state("dev-abc")
|
||||||
self.assertFalse(bottle_state.bottle_state_dir("dev-abc").exists())
|
self.assertFalse(bottle_state.bottle_state_dir("dev-abc").exists())
|
||||||
|
|
||||||
def test_empty_identity_is_noop(self):
|
def test_empty_identity_is_noop(self):
|
||||||
start_mod._settle_state("") # should not raise
|
start_mod.settle_state("") # should not raise
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user