Repository navigation
T67 — Restart an until_stopped box from its stored spec, under the same id and name - #3228
Conversation
Add the RestartBox RPC with a typed RestartRefusal reply. The box's actor checks the lifetime, the name, the stored spec and the volumes before it stops anything, stops a running box as a stop does, and starts it again from its stored spec with no client attached, under the same id and name. A start that fails leaves the box stopped. The restart records the stop's exited event and a started event, and runs only the hooks a resume runs (BOX-176, BOX-177, BOX-178).
Cover BOX-176, BOX-177 and BOX-178: a restart from the stored spec under the same id and name, its refusals before any stop, a failed start that leaves the box stopped, the hooks it runs, and the events it records.
…-until-stopped-box-from-its-stored-spe
|
Important Review skippedBot user detected. To trigger a single review, invoke the
|
|
Approved: every review thread resolved at @agent-137 resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The restart can report success for a service whose entrypoint did not start. In restart_launch(), an argv box that holds a host-side row keeps the on_activate shell and returns Ok(()). The restart then answers Restarted and writes started. Line 471 of the design says a restart starts the box's processes again. min task run refuses this state with EntrypointBehindHookShell (#3042). Make the change in C1, so the restart takes its start-failure branch and leaves the box stopped. The rest of the change conforms.
Ruled at 282fabd against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
|
/revise Arch condition C1 on head 282fabd (arch_rulings 1096), quoted in full from the arch-review check run below. Apply it exactly, with the test it names. Change nothing else.
The residual the ruling names is filed as #3231: such a box cannot restart until #3042 lands. Do not work on #3042 here. |
…-until-stopped-box-from-its-stored-spe
An argv box that holds a host-side row and keeps its hook-launched shell never starts its entrypoint. The restart now fails as ensure_host does, so the box stays stopped and no started event is written.
There was a problem hiding this comment.
APPROVE withheld: 2 unresolved threads — minimal-arch[bot] crates/minimald/src/box_actor/host.rs, minimal-arch[bot] crates/minimald/src/box_actor.rs:2000.
minimal-review — advisory review
Verdict COMMENT — No findings.
Panel: 2 of 3 personas ran (correctness, security); did not complete: supply-chain.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The restart conforms to the design. It refuses on lifetime first, runs the resume checks before any stop, and starts the box again under the same record (architecture.md line 471). The new fix fails a restart when an argv box's entrypoint sits behind its hook shell. That keeps the started event honest. This refusal is a known gap, but only a test comment names its issue, #3231. Make the change in C1: name #3231 and its flip condition at the refusal in restart_launch().
Ruled at d312de8 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
|
Round 1 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
An until_stopped box restarts from its stored spec under the same id and name. Every other box, and every refused restart, is left as it was. An argv box that holds a host-side row and sets hooks_on_resume now fails its restart: it stays stopped with its exited event, and no started event follows. Notes from the agent: C1 applied in restart_launch(). When a live Hooks host is in the slot, holds_host_row and starts_argv() are both true, the function now returns Err(AttachError::SpawnFailed(EntrypointBehindHookShell)), as ensure_host() does. The Ok(()) keep-branch now applies only to a box that does not start an argv. restart_named() takes its existing start-failure branch. New test: restart_argv_box_behind_hook_shell_with_host_row_fails_stopped. cargo-nextest is not installed here, so I did not run the test: lines as written. I ran the same tests with cargo test -p minimald --lib. The residual case is #3231, and I did no work on #3042. |
|
The runtime will not revise this pull request again on its own: the diff is +1490/-118 (1608 lines), past 3x the plan's estimate of 420 lines (est: 170 code, 250 tests, as #3225 reads now), at round 1 (the last trigger was a /revise note). A person decides now. Remove |
|
/revise Arch condition C1 on head d312de8 (arch_rulings 1101), quoted in full from the arch-review check run below. Apply it exactly. It changes only a comment. Change nothing else. The owner lifted the size cap for this round.
|
|
Round 1 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
An until_stopped box restarts from its stored spec under the same id and name. Every other box, and every refused restart, is left as it was. This round adds only a comment: the hook-shell refusal in restart_launch now names its known gap, issue #3231, and the condition that lifts it (#3042). Notes from the agent: Revise round 1: comment-only change for arch condition C1 in crates/minimald/src/box_actor/host.rs. No code changed. cargo-nextest is not installed here, so each test: line exited 101 before it ran any test. I ran |
|
The runtime will not revise this pull request again on its own: the diff is +1494/-118 (1612 lines), past 3x the plan's estimate of 420 lines (est: 170 code, 250 tests, as #3225 reads now), at round 1 (the last trigger was a /revise note). A person decides now. Remove |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare adds only the interim note at the hook-shell refusal in restart_launch. The note names #3231 and the flip condition #3042, and the refusal fails closed. The restart follows lifetime = "until_stopped" and keeps the same record, as architecture.md lines 338 and 471 require. The hook-shell gap stays open as a filed residual on #3231.
Ruled at bd2fc99 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-arch[bot] crates/minimald/src/box_actor/host.rs, minimal-arch[bot] crates/minimald/src/box_actor.rs:2000, minimal-arch[bot] crates/minimald/src/box_actor/host.rs:1791. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @agent-137 closed the last one. The review of bd2fc9981750 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
T67 — Restart an until_stopped box from its stored spec, under the same id and name
slice: S0
kind: task
covers: BOX-176, BOX-177, BOX-178
size: M
est: 170 code, 250 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: daemon log names the box and the refusal or the outcome; the reply carries a typed RestartRefusal
observability: box.restart span with box_id, was_running, refusal kind and outcome; exited, exec_exited and started events as BOX-178 states
layers:
- rpc:
- crates/minimald-rpc/src/lib.rs: RestartBox RPC, RestartBoxRequest, typed RestartRefusal reply
- daemon:
- crates/minimald/src/rpc.rs: serve_restart_box, registered under RestartBox::NAME
- crates/minimald/src/box_manager.rs: restart_box resolves the 8000 box and hands the restart to its actor
- crates/minimald/src/box_actor.rs: restart: lifetime, name, spec and volume checks before any stop; stop as BOX-013; start from stored spec; start-failure leaves stopped
- crates/minimald/src/box_actor/host.rs: start through launch_host with no attach; resume's hooks only
- crates/minimald/src/box_actor/tests.rs: tests
test: cargo nextest run -p minimald restart_until_stopped_box_from_stored_spec_same_id_and_name
test: cargo nextest run -p minimald restart_refuses_box_not_until_stopped_keeps_state
test: cargo nextest run -p minimald restart_takes_resume_refusals_before_it_stops_anything
test: cargo nextest run -p minimald restart_start_failure_leaves_box_stopped_with_its_stop_recorded
test: cargo nextest run -p minimald restart_runs_only_resume_hooks
test: cargo nextest run -p minimald restart_records_exited_then_started_only
produces:
- crates/minimald-rpc/src/lib.rs
consumes: none
proof: An until_stopped box restarts from its stored spec under the same id and name. Every other box, and every refused restart, is left as it was.
depends on:
Proof
A RestartBox RPC now restarts a box whose spec sets lifetime = "until_stopped". A running box is stopped and started again from the spec stored at its creation. A stopped box is started again. In both cases the box keeps its id, its name and its files. Every other box is refused with a typed RestartRefusal (exit 2) and stays as it was. A refusal for the name (exit 2) or for the stored spec (exit 3) also comes before anything stops, so a running box keeps running. A start that fails leaves the box stopped, with only the stop's exited event recorded.
Notes from the agent: Outside the layers: crates/minimald/src/box_actor/kind.rs got one line, the name of the new BoxMessage::RestartBox variant. Its match is exhaustive, so the crate does not compile without it.
cargo-nextest is not installed on this box. I ran each test: line as
cargo test -p minimald --lib <same test name>. All six passed. I also ran the 35 existingresumetests, because the resume name check, the stored-spec check and the hostname marking moved into shared helpers (check_start_name, checked_stored_spec, mark_name_running). They pass, and the resume refusal texts are unchanged. clippy -p minimald -p minimald-rpc --all-targets is clean.Design: the actor runs the restart in its message loop, as it runs a stop. The order is lifetime, then name, then stored spec, then an upload in flight, then volumes. The volume check is a no-op, as in resume. RestartRefusalKind has an
uploadkind that the spec does not name. A running box is stopped through the existing stop_box (graceful, STOP_GRACE), which records exited, and the exec bridges record exec_exited. Then restart_launch (host.rs) starts the box through launch_host with no attach, and the restart writes astartedevent. No other daemon path writesstartedyet (BOX-040 is not covered by this task). A host left from a hooks_on_resume on_activate run is kept for a shell box and replaced for an argv box. When a restart leaves the box st…[truncated]Tests
cargo nextest run -p minimald restart_until_stopped_box_from_stored_spec_same_id_and_namecargo nextest run -p minimald restart_refuses_box_not_until_stopped_keeps_statecargo nextest run -p minimald restart_takes_resume_refusals_before_it_stops_anythingcargo nextest run -p minimald restart_start_failure_leaves_box_stopped_with_its_stop_recordedcargo nextest run -p minimald restart_runs_only_resume_hookscargo nextest run -p minimald restart_records_exited_then_started_onlyVerify
min task run agent-verifyexited 101 under rustc 1.99.0, which the repository does not pin (1.99.1 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Base
The base branch moved while the coder worked and was merged in before the tests ran. These files changed on BOTH sides since the branch point — read them first, because a rewrite from a stale reading can undo a merge without a conflict:
crates/minimald/src/box_actor/tests.rscrates/minimald/src/box_manager.rsOut of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimald/src/box_actor/kind.rsCloses #3225