8000 T32 — Exec into a box, end to end by minimal-foundry[bot] · Pull Request #2986 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content

T32 — Exec into a box, end to end - #2986

Merged
agent-137 merged 5 commits into
mainfrom
agent/2408-exec-into-a-box-end-to-end
Nov 4, 2026
Merged

agent-137 merged 5 commits into
mainfrom
agent/2408-exec-into-a-box-end-to-end

Conversation

@minimal-foundry
@minimal-foundry minimal-foundry Bot commented Nov 4, 2026 •
Copy link
Copy Markdown
Contributor

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_reattach does 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> --print prints. 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

line exit tail
cargo nextest run -p minimald terminal_exec_reattached_by_id_after_client_drop 0 Applying loadouts: default Running task runtime-implement-test-0 in session task-runtime-implement-test-0-585a... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling ot v0.0.1 (/workbench/crates/ot) Compiling args v0.0.1 (/workbench/crates/args) Compiling client-config v0.0.1 (/wo

Verify

min task run agent-verify passed ([tasks.verify]).

Closes #2408

Residuals

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.
@minimal-foundry
minimal-foundry Bot requested a review from a team as a code owner November 4, 2026 04:49
@coderabbitai
coderabbitai Bot commented Nov 4, 2026 •
Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5f1b5ace-0a9a-4b40-a6cc-5caf77a72cef

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@minimal-foundry
minimal-foundry Bot commented Nov 4, 2026 •
Copy link
Copy Markdown
Contributor Author

Approved: every review thread resolved at 80dd297e94ec.

@agent-137 resolved the last thread, which releases the approval this review had withheld.

Nothing to do.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 1 finding, 1 inline.

Panel: 2 of 3 personas ran (correctness, security); did not complete: supply-chain.

Comment thread scripts/session-e2e.sh Resolved
@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread scripts/session-e2e.sh Resolved
Comment thread scripts/session-e2e.sh Resolved
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.
@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 1 of 10 on the runtime's review findings, pushed as d5b5d78969c7.

line exit tail
cargo nextest run -p minimald terminal_exec_reattached_by_id_after_client_drop 0 Applying loadouts: default Running task runtime-implement-test-0 in session task-runtime-implement-test-0-2b64... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling args v0.0.1 (/workbench/crates/args) Compiling ot v0.0.1 (/workbench/crates/ot) Compiling client-config v0.0.1 (/wo

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — scripts/session-e2e.sh: The re-attach output now goes to a file and rc=$? reads the ssh pipeline status, so a non-zero exit fails the proof.

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).

@mitodrummer
Copy link
Copy Markdown
Member

/revise

From the maintainer's pre-merge review at d5b5d789.

  1. Prove the real-box leg of BOX-149 (blocker). The T30 arch review (T30 — Run a command in a box with a terminal or detached, under an exec id #2843, C2) deferred this to here: "That leg stays open until a just e2e run in a real box passes (T32 — Exec into a box, end to end #2408)". This task's layers say "run an exec with argv unshelled in a real box's namespaces and cgroup". exec_reattach (scripts/session-e2e.sh ~1629–1737) runs sh -c … and never checks it. Add an exec with argv unshelled that shows:

    • namespaces: /proc/self/ns/{mnt,pid,net,uts} match the box's own, read from a shell exec in the same box;
    • cgroup: /proc/self/cgroup is the box's leaf;
    • working directory: cwd is /workbench;
    • argv: it passes through literally, for example printf '%s|' "a b" '$HOME' '*' prints a b|$HOME|*|.
  2. Name the issues that own the client verbs (arch C2, open at se.sh ~1634).

  3. Don't wait out the sleep 30 (~1665–1672). wait "$att1_pid" waits for the whole background job, including the sleep 30 subshell, so the first attach always takes ~30 s. Kill the job (or its left subshell) once the client is dropped, or feed stdin from a fifo. Then drop the fixed sleep 4 (~1681).

  4. Make the log check match its comment (~1725–1729). Require, together with the exec id:

    • bridge.rs's "a client attached to the exec's terminal" at each attach;
    • "the attached client went away" at each drop.

    "exec accepted" alone is not enough.

  5. Use a distinctive exit code. Exit the tty exec and the waited exec with 7, as session_exec does (~1545), and assert 7, so the code is shown to be the exec's own and not ssh's.

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.
@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 2 of 10 on a /revise note, pushed as 80dd297e94ec.

line exit tail
cargo nextest run -p minimald terminal_exec_reattached_by_id_after_client_drop 0 Applying loadouts: default Running task runtime-implement-test-0 in session task-runtime-implement-test-0-36b6... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling args v0.0.1 (/workbench/crates/args) Compiling ot v0.0.1 (/workbench/crates/ot) Compiling client-config v0.0.1 (/wo

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — scripts/session-e2e.sh: Commit d5b5d78 fixed this: the re-attach writes to a file, takes rc=$? from the command, and reads out2 from the file. This round keeps that and also asserts the exec's own exit code 7.
  • 👍 valid — scripts/session-e2e.sh:1636: The case comment now names T15 — Run a command in a running box #2905 (min box exec -t|--detach) and T16 — See, read, wait on and stop one exec #2913 (min box attach|wait|logs|stop --exec), and says the case moves to those commands when they close.

In a running box, just e2e (or just e2e-native) runs the exec_reattach case. A terminal exec is attached, its client is dropped, and a second attach by the exec id alone replays the output, takes input and ends with the exec's exit code 7. A detached exec is waited on by its id (exit 7), and its output is read twice by its id. Unshelled argv execs show literal argv (a b|$HOME|*|), the box's mnt/pid/net/uts namespaces, the box's cgroup leaf and the /workbench working directory. On native lanes, the daemon log must name the exec id at both attaches and at the drop. This proof is not delivered on this host: the e2e case did not reach its first exec here, because no box stays alive on this host.

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 min session exec readlink, the cwd (/proc/self/cwd = /workbench), and the cgroup against the box init's /proc/1/cgroup and a shell exec's /proc/self/cgroup. (2) The comment names #2905 (BCLI T15, min box exec -t|--detach) and #2913 (BCLI T16, min box attach|wait|logs|stop --exec), says the case moves to those commands, and names #2978 for the one span over the drop and the re-attach. PR body: the observability item 'one span covers drop and re-attach' is owned by #2978. (3) The first attach reads stdin from a fifo that the script holds on fd 9, so wait returns when the client is killed. The fixed sleep 4 is gone, and the exec's own sleep is now 1 s. (4) The log check counts lines that hold the exec id and "a client attached to the exec's terminal" (2 or more) and "the attached client went away" (1 or more). (5) The tty exec and the waited detached exec exit 7, and the case asserts 7. Verification limits: bash -n and shellcheck give no new findings. A native e2e run on …[truncated]

@minimal-foundry
Copy link
Copy Markdown
Contributor Author

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 exit 1 because the tests job failed. The tests job excerpt does not name a failed test. It shows passing decode tests and the post-job cleanup. I was not able to reproduce a failure from this branch. The Rust diff against origin/main (194b68e) touches only crates/minimald/src/exec/tests.rs. The full minimald suite passes here, and the T32 test passes in 1.5s. The other change is scripts/session-e2e.sh. No Rust test reads that script, and the only gates on it (common shell_lint with shellcheck, and common shell_harnesses) pass here. So the failure is most probably a flaky test in another crate, or a failure that the excerpt does not show. Next step: read the full log of job 126415542568 for the FAIL line, or re-run the check. A re-run is the cheapest step if it is a flake. Earlier answered threads keep their verdicts.

@mitodrummer mitodrummer left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 30 and sleep 4 waits 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.

@agent-137 agent-137 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@minimal-foundry minimal-foundry Bot added the review:approved minimal-review approved the current head label Nov 4, 2026
@agent-137
agent-137 added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 1d49da5 Nov 4, 2026
39 of 41 checks passed
@agent-137
agent-137 deleted the agent/2408-exec-into-a-box-end-to-end branch November 4, 2026 16:24
@bryan-minimal
Copy link
Copy Markdown
Member

On a stock Mac (macOS 27, nothing from Homebrew in the job user's PATH), scripts/session-e2e.sh now stops in the "exec by id" proof group:

scripts/session-e2e.sh: line 1711: timeout: command not found
::error::'min://exec/wait 01a1ab42-…' (argv) exited 127 (expected the exec's 0)

timeout is GNU coreutils; macOS does not ship it. The three calls came in with #2986 (timeout 30 "${er_ssh[@]}" … at lines 1756, 1849, 1875, and ${be_ssh[@]} at 2037 on today's main). Everything before that group passes (cold activate 5.1 s, session exec proof OK) on 0.8.0-dev.381.g3c2b2a73, job j-99797a2b4d8bb390 on the acceptance farm.

A ssh -o ConnectTimeout plus a bash-side wait, or a small with_timeout shell function, would keep it portable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:approved minimal-review approved the current head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

T32 — Exec into a box, end to end

3 participants

0