Repository navigation
T24 — Copy files out of a box and read its events - #3016
Conversation
Client::open_box_subsystem opens a channel for one box with MINIMAL_SESSION_ID set. stream_box_subsystem reads its reply one line at a time, for the ReadEvents read. read_box_file reads one file over the box's sftp subsystem, which serves a stopped box's retained files.
min box events replays a box's events as the daemon reads them, with -o jsonl for one min/v1/event object a line. --follow replays and then tails until the box is removed. --parent merges the streams of every child of a box in time order. min box cp <box>:<path> <local> copies one file out of a box over its sftp subsystem. A stopped or exited box is served from its kept files.
…-out-of-a-box-and-read-its-events # Conflicts: # crates/minimal/src/tests.rs
|
Important Review skippedBot user detected. To trigger a single review, invoke the
|
|
/revise Review at head 7d7bcf3. The design is sound: the follow lifecycle, the bounded mpsc, the env replies, path safety and the JSON schema are all correct. Fix these:
Cargo.toml (russh-sftp, already a workspace dependency) and the matching Cargo.lock line are accepted as wiring. Change nothing else. |
|
That What the platform saidcould not start the coder: Error: there is no container instance that can be provided to this durable object 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. |
min box cp streams the file into a temporary file beside the destination and renames it into place, so a large file is not held in memory and a failed copy leaves no file. The library opens the file as a stream and says why an open failed. The errors of min box cp and min box events name the box by the address the person typed and give one next step. The internal detail goes to the log. New signatures say BoxId, and the doc of append_daemon_error is back on its fn.
…-out-of-a-box-and-read-its-events
|
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. (1) SubsystemLines now sits above the doc block of append_daemon_error. (2) The library replaces read_box_file with open_box_file, which returns a streaming sftp file or a BoxFileError (Unavailable, NotFound, PermissionDenied, Refused). The CLI logs the internal detail with tracing::warn and prints new errors that are not wrapped around the library chain, so the path prints once. Examples: |
|
/revise The six items are done. The streaming rewrite brought in three defects. Fix them at head d9242f8: A. A wrong remedy for a directory source (crates/minimal/src/cmd/boxes/events.rs ~365, and the B. The temp file name can collide and follows symlinks (events.rs ~356, ~405). C. The cleanup path is not tested (tests.rs ~7394). The only failure case errors before the temp file exists. Add a case that fails after the temp file was created, for example a directory source (A). Check that the destination directory holds no temp file afterwards. In the PR body, add one line on the behaviour change at an existing destination. The rename replaces a symlink or an existing file with a new file at the default mode, where |
…-out-of-a-box-and-read-its-events
…file A directory source now says the path is a directory. Open and read failures no longer suggest the box is gone. The temporary file is created with O_EXCL under a random name, and a test covers its cleanup.
…-out-of-a-box-and-read-its-events
|
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. A person can run Notes from the agent: A: After the open, copy_box_file reads the file metadata. A directory gives " in box is a directory, and min box cp copies one file. Run |
There was a problem hiding this comment.
Reviewed at ba2cc25 against #2908 (BCLI-062, BCLI-016), with the library layer. min box events streams READ_EVENTS_SUBSYSTEM lines (follow and --parent) as min/v1/event objects; min box cp reads a retained box's file over sftp in 64 KiB chunks into an O_EXCL temp file beside the destination (tempfile, already a dependency), flushes, and renames it into place, removing the temp file on every error path. A directory source is named as one before any read. Errors name the box by the address the user typed and keep subsystem/uuid detail in the log.
|
/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 · 5 conditions · the full ruling
This change copies a file out of a stopped box, and it replays and follows box events, as lines 501 and 503 give. The placement and the transport conform. Four outputs do not match the design. A failure under -o jsonl is not the min/v1/error object of line 383 (C1). ts is integer milliseconds, but line 381 gives RFC 3339 UTC (C2). --follow has no -f, which line 482 lists (C3). A missing file in min box cp ends with exit 1, not 4 (C4). The fixed set of children under --parent --follow needs a filed issue and an interim comment (C5).
Outside the diff
crates/minimal/src/main.rs:517: C1 · Add an arm forBoxCommand::Eventswith-o jsonl, beside theAuditarm. Line 383 says that a machine-mode failure is onemin/v1/errorobject on stderr.
Ruled at ba2cc25 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
/revise Arch fails at ba2cc25 with conditions C2-C5. All are checked against the code. Fix C2-C4 and the jsonl error object. C5 is residual #3024. C2 (threads z2zM4, z2zM-): machine-mode timestamps are RFC 3339 UTC (architecture C3 (thread z2zNC): C4 (thread z2zNK): Also: C5 (thread z2zNO): residual #3024. Add an Change nothing else. |
…-out-of-a-box-and-read-its-events
Write ts in min box events -o jsonl as an RFC 3339 UTC string, give --follow its -f short form, and end min box cp with 4 for a missing file and 5 for a denied read. A min box events -o jsonl failure is now one min/v1/error object. Mark the one-time child read under --parent as interim, ended by #3024.
…-out-of-a-box-and-read-its-events # Conflicts: # crates/minimal/src/tests.rs
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare fixes each point of the last ruling. Event ts is now RFC 3339 UTC, as Output conventions line 381 requires. --follow has -f (line 482). A -o jsonl failure is one min/v1/error object (line 383). min box cp ends with 4 or 5 per the exit-code table. The one-time child read under --parent --follow stays a residual on #3024.
Ruled at 8d3c001 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 3 of 10 on a /revise note, 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: crates/minimal/src/main.rs is outside the task's layers. The directive asked for it by name: failure_exit_code downcasts the new BoxFileRefusal, and machine_output_mode lists min box events -o jsonl. Both have unit tests there. The exit 5 path has only a unit test in main.rs, because the in-process harness cannot easily make a file that the box cannot read. The exit 4 path is also checked in box_cp_reads_stopped_box_as_box_017. The PR body must name #3024 for C5. The runner writes the body, so I could not add it. |
There was a problem hiding this comment.
Re-reviewed at 8d3c001. abe9acb answers arch C2-C5: ts is RFC 3339 UTC with milliseconds (tests parse it), --follow takes -f, min box cp exits 4/5 through BoxFileRefusal, min box events -o jsonl is a machine mode so failures print a min/v1/error object, and the --parent follow gap is residual #3024 with an Interim comment. Arch passes this head. commitlint's subject-case failure is not a required check.
…-out-of-a-box-and-read-its-events
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head adds one merge of main and no new branch commit. The merge conflicts in cli.rs and cmd/mod.rs, and each resolution keeps the arms from both sides. The branch's own changes are the same as on 8d3c001f, so that PASS stands. min box events uses the shared event type, so it also prints the renamed event that #3008 adds (architecture.md line 382). Issue #3024 owns the --parent --follow residual.
Ruled at e500688 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
A merge of main before round 4, 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.
Notes from the agent: Conflict round only. Both conflicts were additive: main added the Rename and Prune variants of BoxCommand, and this branch added Events and Cp. The merge keeps all four variants in cli.rs and all four arms in both matches in cmd/mod.rs. cargo clippy -p minimal --all-targets is clean, and cargo fmt --check passes. The merge commit is e500688. |
T24 — Copy files out of a box and read its events
slice: S6
kind: task
covers: BCLI-062, BCLI-016
size: L
est: 320 code, 280 tests
executor: agent
difficulty: routine
model: standard
diagnostics:
min box events <box> -o jsonlprints one event per line withschema,ts,box,parent,typeanddata, pinned by events_jsonl_replay_schema_fieldsobservability: the command's existing tracing span records the verb, the box id it resolved and the outcome
layers:
- library:
- crates/minimal-client/src/lib.rs: a public
Clientmethod opens a session subsystem channel for a box (MINIMAL_SESSION_ID set) and streams its reply line by line, formin box events(READ_EVENTS_SUBSYSTEM, with follow) and the sftp read of a retained box formin box cptest: cargo nextest run -p minimal-client session_subsystem_channel_streams_reply_lines
- client:
- crates/minimal/src/cli.rs: the verb's arguments and help in the one command model
- crates/minimal/src/cmd/mod.rs: dispatches the verb
- crates/minimal/src/cmd/boxes/events.rs (new):
min box events [--follow] [--parent]andmin box cpfrom a retained filesystem- crates/minimal/src/cmd/boxes.rs: declares the events module
- crates/minimal/src/tests.rs: the in-process minimald test harness gains this layer's tests
test: cargo nextest run -p minimal box_cp_reads_stopped_box_as_box_017
test: cargo nextest run -p minimal events_jsonl_replay_schema_fields
test: cargo nextest run -p minimal events_follow_replays_then_tails
test: cargo nextest run -p minimal events_parent_merges_children
produces:
- crates/minimal/src/cmd/boxes/events.rs (entry point)
consumes:
- crates/minimal/src/cmd/boxes/address.rs
proof:
min box cpcopies a file out of a stopped box, andmin box eventsreplays and follows its events.depends on:
- blocked by #2895
Proof
A person can run
min box cp web:notes.txt .against a stopped box and get notes.txt from the files the box kept.min box events web -o jsonlprints one min/v1/event line for each event (schema, ts, box, parent, type, data), oldest first.--followprints the kept events, then each new event, and ends when the box is removed.--parent <box>merges the events of every child box in time order.Notes from the agent: Outside the layers: crates/minimal-client/Cargo.toml gains
russh-sftp.workspace = true, and Cargo.lock gains that one edge. The sftp read of a retained box needs an SFTP client, and russh-sftp is already a workspace dependency (minimald, minvmd). The alternative was a hand-written SFTP client in minimal-client.Not done, because the file is outside the layers: main.rs
machine_output_modedoes not listmin box events -o jsonl. So a failure under-o jsonlprints a plain line, not a min/v1/error object. A one-arm change in crates/minimal/src/main.rs adds it, as formin box audit.Behaviour to know: under
--parent --followthe events print in arrival order across the children, not sorted by time. Without--followthey are sorted by ts. The set of children is read once at the start; following children created later is residual #3024.min box cpcopies one file out of a box. It does not copy directories or copy into a box.The daemon never sets a box's parent today. The --parent test therefore writes
parentinto the children's record.json on disk, as the minimald box_manager tests do.One finding outside this task: Client::exec_channel sends MINIMAL_SESSION_ID with want_reply=true and then reads the first Success as the exec ack. That Success is the env ack, so a refused exec can look accepted. The new open_box_subsystem sends the env without a reply for this reason.
Tests
cargo nextest run -p minimal-client session_subsystem_channel_streams_reply_linescargo nextest run -p minimal box_cp_reads_stopped_box_as_box_017cargo nextest run -p minimal events_jsonl_replay_schema_fieldscargo nextest run -p minimal events_follow_replays_then_tailscargo nextest run -p minimal events_parent_merges_childrenVerify
min task run agent-verifyexited 100 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/minimal/src/tests.rsOut of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimal-client/Cargo.tomlCloses #2908