Repository navigation
Conversation
The listing and the read of a box now say whether its stored spec sets io.pty_enabled, so min box attach can tell a terminal box from one with none. A daemon that predates the field reads as no terminal.
min box attach <box> hands the terminal to a box that has one. With no box named it picks the project's running terminal box, else its only retained one. Refusals exit 2, 4 or 7. An attach the host ends prints one line that names why and exits 254.
…a-box-s-terminal-with-or-without-an-ad
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
…a-box-s-terminal-with-or-without-an-ad # Conflicts: # crates/minimal/src/tests.rs
|
A merge of main before round 1, not counted against the cap (1 of 3), pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review. With main merged in, Notes from the agent: Conflict round. In crates/minimal/src/tests.rs, both sides added tests at the end of the file. The branch added the attach tests, and main added hold_record and none_box_hold_is_released_by_destroy_and_moved_by_rename. The resolution keeps both, with no other edits. The other files in the merge commit come from main without conflicts. cargo clippy -p minimal --all-targets is clean. main's merged test none_box_hold_is_released_by_destroy_and_moved_by_rename also passes. |
|
/revise Review at head 56f99b7. The four files outside the layers (cmd/boxes.rs, cmd/boxes/list.rs, cmd/mod.rs, minimald-rpc/src/tests.rs) are accepted as wiring. Fix these:
Change nothing else. |
…a-box-s-terminal-with-or-without-an-ad
The resume's PTY check and the listing's record_has_pty were two copies of one rule. The resume now calls record_has_pty.
The daemon closes the attach's channel before it writes the stopped record, so one read after ssh exits could see the box still running. The read now polls under a bound until the box leaves running. A stopped box ends with 254 whatever exit code its entrypoint recorded. The host and exited lines now say what can happen in plain words.
|
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.
Notes from the agent: Revise round 1. Item 1 (blocking): I used the fallback that the note allows. The cause cannot travel on the current wire. The attach runs through an OpenSSH subprocess, so the client sees only ssh's exit status. The daemon's binding also does not know whether a stop or a removal ended the host. After ssh exits with 0 or 254, the client now polls ShowBox every 50 ms while the box reads Item 2: NOT done. The daemon's binding cannot tell a |
…a-box-s-terminal-with-or-without-an-ad # Conflicts: # crates/minimal/src/tests.rs
|
A merge of main before round 2, not counted against the cap (2 of 3), pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review.
Notes from the agent: Conflict round. The only conflict was in crates/minimal/src/tests.rs: this branch added the attach tests and helpers, and main added run_address_step and the address end-to-end test, at the same place. Both sides are independent, so the resolution keeps both blocks unchanged. The merge commit is e6ac801. The merge also brought in main's changes to crates/minimald/src/box_runtime/enforcement.rs and launcher.rs, which are not this task's edits. The 10 minimal test lines ran in one nextest invocation with their filters combined, not one by one. |
…a-box-s-terminal-with-or-without-an-ad # Conflicts: # crates/minimal/src/cli.rs # crates/minimal/src/cmd/mod.rs
|
A merge of main before round 2, not counted against the cap (3 of 3), pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review.
Notes from the agent: Conflict round only. Both conflicts were the BoxCommand enum in cli.rs and its two match arms in cmd/mod.rs (verb name and dispatch). main added Rename and Prune where this branch added Attach. I kept all three variants and arms. The other merged files came from main without conflict. cargo clippy -p minimal --all-targets is clean, and all 11 test lines pass at the merge commit. |
|
/revise Two follow-ups at head bc5aac7. The poll fix is right for the common case.
The duplicate cause line is residual #3035 now (named in the PR body), so leave binding.rs alone. Change nothing else. |
…over After a 254 the read now polls for STOP_GRACE plus HOST_PROBE_TIMEOUT, shared from minimald-rpc, so a stop that waits on a process ignoring SIGTERM still prints the stop line. An OOM record carries exit code 137, so its reason is matched before the exit code.
|
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.
Notes from the agent: Round 2. Item 1: DAEMON_END_SETTLE is now STOP_GRACE + HOST_PROBE_TIMEOUT (12 s). The poll still stops as soon as the state leaves running. The 500 ms bound after a 0 is unchanged. Trade-off for the PR body: a real takeover (another terminal attached) now waits the full 12 s before its line prints, and a detach that the daemon reports as 0 still waits up to 500 ms. A stop whose record lands later than 500 ms after a 0 still reads as a plain exit 0. To share the constants, I moved STOP_GRACE and HOST_PROBE_TIMEOUT to pub consts in crates/minimald-rpc/src/lib.rs. crates/minimald/src/box_actor/host.rs is outside the layers: its two pub(crate) consts now alias the minimald-rpc values (two lines, same values). The client does not depend on minimald at runtime, so this was the only way to use one definition. New test: a 254 case in attach_ended_from_other_side_notice_exit254_before_the_record where the stop lands 2.5 s after the channel closes. Item 2: ended_box_line matches reason oom before the exit-code arm. attach_ended_from_other_side_notice_exit254 now pins the OOM line for a record with exit_code 137. binding.rs is untouched. I did not run just clippy-strict. |
…a-box-s-terminal-with-or-without-an-ad # Conflicts: # crates/minimal/src/cli.rs # crates/minimal/src/cmd/mod.rs # crates/minimal/src/main.rs
|
Round 3 of 10 on a conflict with the base branch, pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review.
Notes from the agent: This round resolved the merge conflict with main and changed nothing else. The three conflicts in cli.rs, cmd/mod.rs and main.rs kept both sides. The Attach variant, its name and its dispatch sit beside the new Events and Cp variants from main. failure_exit_code checks BoxAttachRefusal and then BoxFileRefusal. cargo clippy -p minimal --all-targets is clean after the merge. |
There was a problem hiding this comment.
Reviewed at abf760a against #2904 (BCLI-068), with the daemon layer. The listing and show replies carry the stored spec's pty_enabled (serde default false). min box attach reattaches a terminal box, refuses a running non-PTY box with exit 2 naming min box logs, and in the bare form picks the project's terminal boxes. After the attach ends it polls the record while it reads running, up to 500 ms after a 0 and STOP_GRACE + HOST_PROBE_TIMEOUT after a 254 (shared constants), then names one cause; an OOM end is read before its 137. The daemon's duplicate PTY line is residual #3035.
|
/review |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 4 conditions · the full ruling
The lookup, the refusals and the placement conform to architecture.md line 491. The exit codes do not. Line 491 and line 376 give an attach only 0, 7 and 254, and 254 only for a stop, a removal or a takeover. attach_ending lets ssh's 255 out as the exit code (C1). It exits 254 for a box that ended by itself (C2). It exits 254 with no stderr line when the read after the attach fails (C3). The --exec legs of BCLI-068 are not built, and the PR names no issue that owns them (C4).
Ruled at abf760a against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
A 254 against a box that still runs waits the stop's grace period to read why. After one quiet second, the terminal now says the attach ended and how long the check can take.
…a-box-s-terminal-with-or-without-an-ad # Conflicts: # crates/minimal/src/cli.rs # crates/minimal/src/cmd/mod.rs # crates/minimal/src/main.rs
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 5 unresolved threads — minimal-foundry[bot] crates/minimal/src/cmd/boxes/attach.rs:1022, minimal-arch[bot] crates/minimal/src/cmd/boxes/attach.rs:1119, minimal-arch[bot] crates/minimal/src/cmd/boxes/attach.rs:1104, minimal-arch[bot] crates/minimal/src/cmd/boxes/attach.rs:1119, minimal-arch[bot] crates/minimal/src/cli.rs:621. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
Commit 1385327c adds a second stderr line before a 254 exit. A takeover, or a slow stop, now prints Detached from <box>. Checking why. ... and then the Detached: ... notice. architecture.md line 491 says attach exits 254 after a one-line notice on stderr. BCLI-068 says the same. Make the change in C1: remove the wait line and keep the one bounded read. The rest of the change conforms, and the merge resolution keeps both attach and exec.
Ruled at 3fb1980 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 4 of 10 on the runtime's review findings, pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
Notes from the agent: Round 4 change: when the daemon ends an attach (254) and the box still reads running after 1 s, stderr now prints "Detached from . Checking why. This can take up to 11 seconds." Then the rest of the settle bound runs. Unit test daemon_end_wait_line_names_the_box_and_the_wait pins the text. The daemon sends no signal that separates a takeover from a stop in progress. So a shorter bound for a takeover needs a daemon change in box_actor, which is outside this task's layers. |
|
/revise Round 4 answered only the foundry LOW (thread B00). Arch's conditions at abf760a and its new ruling at 3fb1980 are open. Fix them, and decline B00 in its thread (arch's line 491 rules it out). New C1 (threads z7WyC, z7WyF; architecture.md line 491, "a one-line notice"): revert 1385327's wait line. Remove the Arch C1-C3 at abf760a (crates/minimal/src/cmd/boxes/attach.rs ~1092-1119):
C4 (z7ElX) is done: the PR body names #2913 for |
…a-box-s-terminal-with-or-without-an-ad
The design allows one notice line before exit 254. This reverts 1385327 and restores the single DAEMON_END_SETTLE read.
Every ssh 255 is now a lost connection, exit 7. A 254 against a box that reads exited ends with 0 and keeps its line. A 254 with an unread record prints one line that names the host.
…a-box-s-terminal-with-or-without-an-ad
|
Round 5 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:
Notes from the agent: Reverted 1385327 (wait line, DAEMON_END_QUIET, daemon_end_wait_line and its test). The single DAEMON_END_SETTLE read after a 254 is back. A 255 no longer reads the box afterwards, because every 255 now maps to ConnectionLost. C2 follows the ruling literally: every 254 whose record reads |
There was a problem hiding this comment.
Re-reviewed at 9827817. 4d9fc70 drops the wait line, so a 254 ends with one notice line after one bounded settle read (architecture.md line 491). bc036d2 ends min box attach with 0, 7 or 254 only: every 255 is a lost connection (exit 7), a 254 whose record reads exited ends with 0 and its line, and a 254 with an unread record prints 'Detached: host ended the attach to .'. The tests that pinned the old endings are flipped. The minimal-review takeover-wait thread is declined under line 491.
|
/review |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head conforms to architecture.md line 491 and line 376. The wait line before a 254 is gone, so a 254 prints one line on stderr. Every ssh 255 is now exit 7. A box that reads exited returns 0, as line 376 says for the entrypoint's own exit. The --exec legs stay with #2913, and the daemon's Disconnecting line stays with #3035.
Ruled at 9827817 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
A spec read error listed the box with no terminal, so a bare min box attach could skip a real session. The listing now falls back to the Box Type, as for a box with no stored spec, and logs a warning.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare changes one thing. A box with an unreadable stored spec now lists by its Box Type, the same as a box with no spec. A session is a terminal box by architecture.md line 1184, so a bare min box attach finds it as line 491 requires. The resume and launch gates still refuse an unreadable spec, so the daemon fails closed. The rest of the baseline PASS on 98278174 stands.
Ruled at 3d4c26b against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 6 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
Notes from the agent: Revise round 6. The only change is the Listing handler in minimald: a spec read error now falls back to the Box Type, as for a box with no stored spec, and logs at warn, not debug. box_listing_and_show_carry_pty_enabled gains an unreadable-spec session case (a directory at spec.json). The finding also mentions one spec read per box in the manager loop. I left that as it is: it is one small file read per retained box, the same cost as the record read beside it. cargo clippy -p minimald --all-targets is clean. The client files did not change this round. |
T12 — Attach to a box's terminal, with or without an address
slice: S3
kind: task
covers: BCLI-068
size: L
est: 140 code, 640 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: when the daemon ends an attach, the terminal shows one line such as "Detached: was stopped." and the command exits 254; with no candidate box it prints "No box with a terminal in this project. Start one with min box start." and exits 4, pinned by attach_ended_from_other_side_notice_exit254 and box_attach_without_address_none_exit4_hints_start
observability: the command's existing tracing span records the verb, the box id it resolved and the outcome
layers:
- daemon:
- crates/minimald-rpc/src/lib.rs:
BoxListingEntryandShowBoxResponsecarry the stored spec'spty_enabled(serde default false, so an older daemon reads as no terminal)- crates/minimald/src/box_manager.rs:
box_listing_entryand the show reply fillpty_enabledfrom the box's stored spec- crates/minimald/src/box_manager/tests.rs: the listing and show tests for the field
test: cargo nextest run -p minimald box_listing_and_show_carry_pty_enabled
- client:
- crates/minimal/src/cli.rs: the verb's arguments and help in the one command model
- crates/minimal/src/cmd/boxes/attach.rs:
min box attach [<box>]: the bare form, and the 0, 2, 4, 7 and 254 endings- crates/minimal/src/attach.rs: the terminal attach reports why the daemon ended it
- crates/minimal/src/main.rs:
failure_exit_codemaps this verb's refusals to their exit codes- crates/minimal/src/tests.rs: the in-process minimald test harness gains this layer's tests
test: cargo nextest run -p minimal box_attach_reattaches_pty_box
test: cargo nextest run -p minimal box_attach_without_tty_exit2
test: cargo nextest run -p minimal box_attach_running_non_pty_box_exit2_names_logs
test: cargo nextest run -p minimal box_attach_without_address_picks_running_else_sole_retained
test: cargo nextest run -p minimal box_attach_without_address_ambiguous_exit2_lists
test: cargo nextest run -p minimal box_attach_without_address_none_exit4_hints_start
test: cargo nextest run -p minimal box_attach_without_address_unreachable_host_exit7
test: cargo nextest run -p minimal attach_detach_or_entrypoint_exit_returns_0
test: cargo nextest run -p minimal attach_ended_from_other_side_notice_exit254
test: cargo nextest run -p minimal attach_connection_failure_exit7
produces: none
consumes:
- crates/minimal/src/cmd/boxes/address.rs
proof:
min box attachreattaches a terminal box, finds the project's box with no address, and exits with a code a script can read.depends on:
- blocked by #2895
Proof
A person can run
min box attach <box>to hand their terminal to a box that has one. They can run a baremin box attachin a project to reach its running terminal box, or its only stopped or exited one. A script can read the exit code: 0 for a detach or an entrypoint exit, 2 for no terminal (client or box) or two candidates, 4 for no candidate, 7 for an unreachable host or a lost connection, and 254 after one line such as "Detached: web was stopped." when the host ends the attach.Notes from the agent: Files outside the layers, each a small change: crates/minimal/src/cmd/mod.rs (dispatch and verb name for
box attach), crates/minimal/src/cmd/boxes.rs (pub use attach::BoxAttachRefusalso main.rs can downcast it), and crates/minimald-rpc/src/tests.rs plus crates/minimal/src/cmd/boxes/list.rs (test struct literals gainpty_enabled). crates/minimal/src/attach.rs needed no change: the code that reads why the attach ended is in cmd/boxes/attach.rs (attach_ending,box_after_attach).ShowBoxResponsecarriespty_enabledthrough itsrecord: BoxListingEntryand gains no field of its own. A box filed with no stored spec reads as a terminal box when its type is session, the same as the resume check in box_actor. A reaped entry reportspty_enabled: false.The tests run the command up to the ssh hand-off against in-process harness daemons, and the hand-off is a closure that returns ssh's exit code. No test drives a real ssh or PTY attach. The daemon does not say why it ended an attach, so the client works out the cause: after 0, 254 or 255 it reads the box once from its host. 254 + stopped is 'was stopped'. 254 + gone is 'was removed'. 254 + running is 'another terminal attached'. 254 + silent host is 'host shut down'. 0 + stopped with no exit code is also 254 'was stopped', because a stop from another client can reach the client as 0. 255 + silent host is exit 7. A 255 from a host that still answers (the daemon's slow-terminal shed) stays 255. 0 …[truncated]
Tests
cargo nextest run -p minimald box_listing_and_show_carry_pty_enabledcargo nextest run -p minimal box_attach_reattaches_pty_boxcargo nextest run -p minimal box_attach_without_tty_exit2cargo nextest run -p minimal box_attach_running_non_pty_box_exit2_names_logscargo nextest run -p minimal box_attach_without_address_picks_running_else_sole_retainedcargo nextest run -p minimal box_attach_without_address_ambiguous_exit2_listscargo nextest run -p minimal box_attach_without_address_none_exit4_hints_startcargo nextest run -p minimal box_attach_without_address_unreachable_host_exit7cargo nextest run -p minimal attach_detach_or_entrypoint_exit_returns_0cargo nextest run -p minimal attach_ended_from_other_side_notice_exit254cargo nextest run -p minimal attach_connection_failure_exit7Verify
min task run agent-verifytimed out after 1800s, so this PR is a draft.Base
The base branch moved while the coder worked; it was merged in before the tests ran.
Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimal/src/cmd/boxes.rscrates/minimal/src/cmd/boxes/list.rscrates/minimal/src/cmd/mod.rscrates/minimald-rpc/src/tests.rsResiduals
min box attach --exec <id>and the fivebox_attach_exec_*legs of BCLI-068. This PR has no--exec.min box attachprints its "Detached: …" line. The binding cannot tell this client frommin session attachormin dash, so dropping the line for this client alone needs the client to tell the daemon it prints its own line.Closes #2904