Repository navigation
T56 — Store each box's stopped and exited ending on its record - #2676
mitodrummer merged 9 commits into
Conversation
BoxState gains stopped and exited. The record stores the reason, exit_code and signal of how the box ended, and Record::state reads them back. A reason this build does not know loads and saves as written, and the store logs it. Refs BOX-011.
Callers match the stopped and exited states. min ls prints running for an active record, an ended box renders hollow in the attach picker, and minimald refuses to finalize or host an ended box. Record literals in other crates set the new ending fields to none. Refs BOX-011.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: no findings at Nothing to do. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The record now stores stopped and exited with reason, exit_code and signal, as architecture.md line 495 gives them. The restart reap keeps a record whose ending it cannot read, so it fails closed. One gap stays open. BoxActor::run refuses an ended record "until" a later task, and no issue owns that task. Line 495 requires that resume restarts a stopped PTY box, so this work needs an owner. C1 files that issue and links it from the comment and the PR body.
Ruled at 301ca00 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings: the coder changed nothing. Revise round 1: the one thread is judged invalid, so this round adds no commit. The three test lines pass at the branch head, which includes the merge of origin/main. A rename of the wire name of BoxState::Active to running is a separate change. It needs a compatibility plan for older clients and its own pull request. Review threads the coder judged:
|
|
/revise One comment change on 301ca00, from arch-review C1. Change no code. In Run the plan's test lines. |
…-box-s-stopped-and-exited-ending-on-it
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head conforms to architecture.md line 495. The record holds the five states and stores reason, exit_code and signal by the table. The compare only names #2393 at the refusal in BoxActor::run and merges main, and that meets the earlier condition. Two residuals stay open with their issues: hosting an ended box (#2393) and the active spelling in -o json (#1991).
Ruled at 13b77c4 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
/revise One fix on 13b77c4, from minimal-review (crates/minimald/src/box_actor.rs, The arm keeps a record only when Run the plan's test lines plus |
|
That Nothing to type again: the command gets a rocket when its round starts, or a refusal that names the reason. No round was counted for it yet. |
|
Round 2 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:
A stored record holds stopped or exited with its reason, exit code and signal. This round, the comment on the Stopped | Exited arm in BoxActor::run names #2393 (T6, BOX-025) as the work where resume brings a stopped PTY box back up and the actor learns to host an ended box. Notes from the agent: This round changed one comment and no code, as the /revise note asked. crates/minimald/src/box_actor.rs is not in the task's layers. The note named that file explicitly. The PR body is unchanged. |
…-box-s-stopped-and-exited-ending-on-it
DestroyIfUnfinalized kept a record only when it read Active, so a Stopped or Exited record was destroyed. Destroy only Pending and Materializing records, through an exhaustive match.
|
Round 3 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:
A stored record holds stopped or exited with its reason, exit code and signal. A cleanup destroy (DestroyIfUnfinalized) now keeps such an ended record and destroys only a Pending or Materializing one. Notes from the agent: Revise round 3. crates/minimald/src/box_actor.rs is outside the task's layers. The directive asked for this fix, and earlier rounds already touched the file. The keep-or-destroy decision is now the function survives_conditional_destroy, with an exhaustive match on BoxState, so the compiler flags a new state. An unreadable record is still destroyed. The test checks the predicate as a function, as the directive allowed. The harness has no simple way to put a live actor's record into Stopped. The test sits in an inline module in box_actor.rs and not in box_actor/tests.rs, so no further file changes. The first commit had an import order that rustfmt rejects. A second commit fixes it. cargo clippy -p minimald --all-targets is clean. |
There was a problem hiding this comment.
Approved at 3224694. BoxState holds stopped and exited. Record stores reason, exit_code and signal as optional fields, so old records load unchanged. StoredReason keeps a reason this build doesn't know as written, so oom and later reasons need no migration. set_state writes a state with its ending in one step, and a cleanup destroy keeps ended records. Hosting an ended box is T6 (#2393), listed under Residuals.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare since the PASS on 13b77c4a is a clean merge of main and one fix in box_actor.rs. A cleanup destroy now removes only a pending or materializing box. It keeps a running, stopped or exited box. Line 21 of architecture.md says that an ended box keeps its record until rm or prune. The fix conforms, and BOX-011 still conforms.
Ruled at 3224694 against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The merge from main does not change the tree of this PR. The head has the same tree as 32246949, and that head got a PASS. BOX-011 conforms to architecture.md line 495. The record stores stopped and exited with reason, exit_code and signal as that line gives them. The residuals #2393 and #1991 stay open.
Ruled at beb2f3b against gominimal/arch@109541a48c31 by claude-opus-5-5.
Policy and destroy resolve a box's VM from its name, as attach, exec, run and net forward do. A box no running VM holds names --vm, a box on a stopped VM names the command that starts it, and a name any two VMs hold, running or stopped, is refused with every VM listed. A relay failure writes the unwind before the attach error prints, so the error stays on screen. Backport of #2811. Adapted to the line: cmd/boxes/attach.rs and policy.rs hunks land in cmd/session.rs. Dropped, because the line has no stopped session status (#2676) and keeps box ids apart from session ids (#2485): min ls rows for a stopped VM's boxes, vm_running in its JSON, and id-prefix matching against a stopped VM's records, which match by name only here. (cherry picked from commit 2e3962f)
Policy and destroy resolve a box's VM from its name, as attach, exec, run and net forward do. A box no running VM holds names --vm, a box on a stopped VM names the command that starts it, and a name any two VMs hold, running or stopped, is refused with every VM listed. A relay failure writes the unwind before the attach error prints, so the error stays on screen. Backport of #2811. Adapted to the line: cmd/boxes/attach.rs and policy.rs hunks land in cmd/session.rs. Dropped, because the line has no stopped session status (#2676) and keeps box ids apart from session ids (#2485): min ls rows for a stopped VM's boxes, vm_running in its JSON, and id-prefix matching against a stopped VM's records, which match by name only here. (cherry picked from commit 2e3962f)
T56 — Store each box's stopped and exited ending on its record
slice: S0
kind: task
covers: BOX-011
size: M
est: 250 code, 150 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: the store logs a record it loads with an unknown reason, naming it
observability: none: no new runtime signal; the ending is read by T3's stop span
layers:
- record:
- crates/boxes/src/lib.rs: BoxState holds stopped and exited; Record stores reason, exit_code, signal
- crates/boxes/src/core/record.rs: Record::state() reads the stored ending; state_is_exactly_one over Record
- crates/boxes/src/store.rs: persist and load the ending; an unknown reason loads as written
test: cargo nextest run -p boxes record_stores_stopped_and_exited_with_their_ending
test: cargo nextest run -p boxes unknown_reason_loads_as_written
- callers:
- crates/minimal/src/cmd/list.rs: min ls label active -> running
- crates/minimal/src/attach.rs: state matches
- crates/minimald/src/box_actor/setup.rs: state matches
test: cargo nextest run -p minimal list_labels_a_running_box_running
produces: none
consumes:
- crates/boxes/src/events.rs
proof: A stored record holds stopped or exited with its reason, exit code and signal.
depends on:
- blocked by #2387
Proof
A box record saved as stopped or exited now loads from disk in that state. Its record.json holds status, reason, exit_code and signal as the BOX-011 table gives them, and Record::state() reads them back as the same RecordState. A reason this build does not know loads, is logged, reads as an UnreadableEnding, and saves unchanged. min ls labels a running box running.
Notes from the agent: Files touched outside the layers. Each change is mechanical and needed to compile. (1) The new Record fields reason, exit_code and signal break every Record struct literal, so these files set them to None: crates/minimal/src/dash/render.rs, crates/minimal/src/dash/rpc.rs, crates/minimal/src/cmd/boxes/vm_host.rs, crates/minimal/tests/dash_snapshots.rs, crates/minimal-client/src/box_registration.rs, crates/minimald/src/box_actor/host.rs, crates/minimald/src/box_manager.rs, crates/minimald/src/box_runtime.rs, crates/minimald/src/exec/tests.rs and crates/minimald/src/store.rs. (2) crates/minimald/src/box_actor.rs has an exhaustive match on BoxState. It now refuses to spawn an actor for a stopped or exited record, the same as for materializing. No path writes these states yet. The task that first writes them must decide how the actor holds an ended box. (3) Record::state() now returns Result<RecordState, UnreadableEnding>, so the restart reap in crates/minimald/src/box_manager.rs keeps a record whose ending it cannot read. (4) Two tests pinned the old label 'active' and now expect 'running': crates/minimal/src/tests.rs (bare_status_renders_a_cwd_session_verbatim, duplicate_session_warning_waits_before_attach_for_materializing) and crates/minimal/tests/cli.rs (ls_table_exposes_project_path_and_status). Printed text: the min ls STATUS column and the bare min status row print 'running' in place of 'active'. The duplicate-session warning now says 'until its status is running'. For a stopped or exited duplicate, the warning prints only its first sentence, because attach does not a…[truncated]
Tests
cargo nextest run -p boxes record_stores_stopped_and_exited_with_their_endingcargo nextest run -p boxes unknown_reason_loads_as_writtencargo nextest run -p minimal list_labels_a_running_box_runningVerify
min task run agent-verifytimed out after 1800s, so this PR is a draft.Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimal-client/src/box_registration.rscrates/minimal/src/cmd/boxes/vm_host.rscrates/minimal/src/dash/render.rscrates/minimal/src/dash/rpc.rscrates/minimal/src/tests.rscrates/minimal/tests/cli.rscrates/minimal/tests/dash_snapshots.rscrates/minimald/src/box_actor.rscrates/minimald/src/box_actor/host.rscrates/minimald/src/box_manager.rscrates/minimald/src/box_runtime.rscrates/minimald/src/exec/tests.rscrates/minimald/src/store.rsResiduals
BoxActor::runrefuses an actor for astoppedorexitedrecord until resume brings a stopped PTY box back up (arch-review C1).-o json/ wire spelling ofrunning(stillactive, read either way since T52): the box CLI spec's output schema (BCLI-001, docs(specs): add the min box CLI spec (BCLI) for inbox#731 #1991).Closes #2634
Maintainer accepted the out-of-layer edits, 2026-10-28: the
reason/exit_code/signal: Nonefields where aRecordis built, theactive→runningtest updates,box_actor.rsrefusing an actor for an ended record, and the restart reap keeping a record whose ending is unreadable. Undrafted.