Repository navigation
T32 — Exec into a box, end to end - #2986
Conversation
A terminal exec and a detached exec lose their client. A lookup by the exec id alone attaches to the terminal again and replays what the exec wrote with no client. The detached exec's output reads by its id.
The exec_reattach case starts a terminal and a detached min://exec in the shared box over plain ssh, drops each client, and brings each back by its exec id alone: a re-attach with the replay and new input, and a wait and two equal output reads.
|
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 · 2 conditions · the full ruling
The daemon test proves re-attach by exec id as architecture.md line 497 describes it. The e2e leg cannot pass. scripts/session-e2e.sh line 1683 reads ${PIPESTATUS[1]} after a $(...) assignment. Under the script's set -u, that line aborts the whole lane with an unbound-variable error. C1 takes the status from $? and keeps the exit-code check hard. The raw min://exec ssh route and the undelivered span are interims with no linked issue. C2 links them in the script and the PR body.
Ruled at b61f82a against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
PIPESTATUS after a command substitution reads the assignment, so rc was empty and a non-zero exit never failed the proof. Write the output to a file and take the ssh status directly.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-foundry[bot] scripts/session-e2e.sh, minimal-arch[bot] scripts/session-e2e.sh, minimal-arch[bot] scripts/session-e2e.sh:1634. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This change adds tests only, and they prove architecture.md line 497 (min box exec). A -t exec survives the loss of its client and re-attaches by its exec id with its transcript. A --detach exec ke
8000
eps its output and its exit code for a read by id. Commit d5b5d78 makes the e2e re-attach fail on a non-zero exit, so the proof is now hard on the exit code. No condition is open.
Ruled at d5b5d78 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
In a running box, a person can start a terminal exec or a detached exec, drop the client, and then attach to the terminal exec again or read the detached exec's output again by its exec id. The e2e proof now also fails when the re-attach does not end with the exec's exit 0. Notes from the agent: Revise round 1: proof_exec_reattach now writes the re-attach output to $WORK/exec-reattach-attach2.out and takes rc from the ssh pipeline itself. bash -n and shellcheck pass. I did not run scripts/session-e2e.sh on this box, because it needs a real box and daemon (just e2e). |
|
/revise From the maintainer's pre-merge review at
|
Check argv, namespaces, cgroup leaf and cwd of an unshelled exec against a shell exec. Hold the first attach's stdin on a fifo, so the case no longer waits out a sleep. Require the attach and drop log records for the exec id. Name the issues that own the client commands and the span.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This push makes the exec proof stricter and changes no product code. The new checks prove what architecture.md line 497 states: an unshelled argv, the box's namespaces, cgroup and working directory, and a propagated non-zero exit code. The interim ssh route now names the issues that own the min box commands and the span. The ruling stays PASS, the same as on d5b5d789.
Ruled at 80dd297 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-foundry[bot] scripts/session-e2e.sh, minimal-arch[bot] scripts/session-e2e.sh, minimal-arch[bot] scripts/session-e2e.sh:1636. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
In a running box, Notes from the agent: Revise round 2, all five note items addressed in scripts/session-e2e.sh proof_exec_reattach. (1) New unshelled detached execs check argv literally (printf '%s|' "a b" '$HOME' '*'), the mnt/pid/net/uts namespaces against |
|
Round 3 of 10 on a failed check suite: the coder changed nothing. noop: this round changes nothing. The directive lists two failed checks. ci-linux-native-success is the lane aggregator, and its log shows only |
There was a problem hiding this comment.
Reviewed at 80dd297e. Round 2 lands the requested proof of BOX-149's real-box leg.
- Box identity: an unshelled exec reads the same
/proc/self/ns/{mnt,pid,net,uts}and cgroup leaf as a shell exec in the box, with cwd/workbench. - Argv: it passes through literally (
a b|$HOME|*|). - Exit codes: the tty and detached execs exit 7, and both are asserted.
- Daemon log: the check requires the attach and drop lines together with the exec id.
- Timing: the fixed
sleep 30andsleep 4waits are gone.
The re-attach and re-read by exec id were already shown not vacuous at d5b5d789. The tests failure on this head was the known minvmd concurrent_boots_never_draw_the_same_proxy_port flake, and it passed on rerun. Residuals: #2905, #2913 and #2978.
There was a problem hiding this comment.
Reviewed at 80dd297 against #2408's plan. Unit test terminal_exec_reattached_by_id_after_client_drop and e2e proof_exec_reattach cover drop + re-attach by id, replay, input, exit code, and detached output read twice; CI shows "exec by id proof OK" on native and macOS. The three open threads are fixed on this head: the re-attach output goes to a file and rc=$? (no PIPESTATUS under set -u), and the header names #2905/#2913. The plan's single drop/re-attach span is deferred to #2978.
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 80dd297e94ec 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.
|
On a stock Mac (macOS 27, nothing from Homebrew in the job user's PATH),
A |
T32 — Exec into a box, end to end
slice: S7
kind: integration
covers: none
size: S
est: 40 code, 120 tests
executor: agent
difficulty: routine
model: light
diagnostics: daemon log names exec id throughout
observability: one span covers drop and re-attach
layers:
- slice:
- crates/minimald/src/exec/tests.rs: tests
test: cargo nextest run -p minimald terminal_exec_reattached_by_id_after_client_drop
- e2e:
- scripts/session-e2e.sh: exec in a real box, terminal and detached
produces: none
consumes: none
proof: run a command in a running box with a terminal or detached, drop the client, and attach to it or read its output again by its exec id.
depends on:
- blocked by #2404
- blocked by #2382
- blocked by #2406
Proof
A person can run a command in a running box with a terminal or detached, drop the client, and get it back by its exec id. The daemon test terminal_exec_reattached_by_id_after_client_drop shows this. A re-attach by the id replays what the terminal wrote while no client was there, takes new input and ends with the exit code. The detached exec's output reads by its id, and reads again the same. The e2e case
scripts/session-e2e.sh exec_reattachdoes the same against a real box over min://exec, min://exec/attach, min://exec/wait and min://exec/output.Notes from the agent: The task adds tests only. The product code from T31 already supports this flow. The e2e case did not run on this box: jq is not installed and no min or minimald binary is built. The KVM, native and macOS lanes run it. It is also in the whole-lane order after session_exec. min has no command for terminal or detached execs or for attach by exec id. The e2e case sends the min://exec requests over a plain ssh. It builds that ssh from the transport that
min session setup-zed <box> --printprints. When a min command for this lands, the e2e case can switch to it. The daemon-log check (the log names the exec id) runs only where the daemon log is on the host, and it is reported as not run on VM lanes. Observability: the task asks for one span over the drop and the re-attach. Each SSH channel request (exec, exec/attach) gets its own span in crates/minimald/src/exec.rs. That file is outside this task's layers, so one span over both is not delivered. The exec id is in every exec log line, so the log ties the two together.Tests
cargo nextest run -p minimald terminal_exec_reattached_by_id_after_client_dropVerify
min task run agent-verifypassed ([tasks.verify]).Closes #2408
Residuals
min box exec -t|--detach) and T16 T16 — See, read, wait on and stop one exec #2913 (min box attach|wait|logs|stop --exec) deliver the client verbs.exec_reattachmoves to them when they land (arch C2).