Repository navigation
T21 — One minimal.toml for every box, end to end - #3244
Conversation
The contribution carries the expanded minimal.toml entry, and the daemon composer applies each list it carries. A list the entry leaves out keeps the daemon's own project items of that kind (BOX-068).
Session and task starts expand their minimal.toml entry over [defaults] and [defaults.<type>], with a legacy [session] table read as [defaults.session], and send it on the contribution. The daemon composes the project's part of each box from it (BOX-068).
…l-toml-for-every-box-end-to-end # Conflicts: # crates/minimal/src/tests.rs
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
…l-toml-for-every-box-end-to-end # Conflicts: # crates/minimal/src/tests.rs
|
Reviewed: no findings; approval withheld, 6 threads unresolved at APPROVE withheld: 6 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs:1513, minimal-arch[bot] crates/boxes/src/daemon/composer.rs:316, minimal-arch[bot] crates/minimal/src/tests.rs:13215, minimal-arch[bot] crates/boxes/src/wire/request.rs:55, minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs:1450, …. Resolving the last thread releases the approval on this head. Resolve the open threads. Resolving the last one releases the approval on this head; no new review is required. Verdict APPROVE, posted as a comment: the App cannot review its own pull request; the check and the label carry it. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The client expands each entry correctly. The daemon still reads the legacy [session] table for every box, so a task box keeps that table's vars, patches and packages. Line 1205 merges [defaults.<type>] into entries of that type only. C1 stops the daemon reading [session] when an entry is present, and adds a test for a task in a legacy project. C2 sends entry packages through the package-wiring walk (line 1204). C3 makes the wire doc state that the entry is not yet the full expanded spec, and links the issue for that work.
Ruled at 8251333 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| if hooks_enabled { | ||
| wire.lifecycle_hooks = list(spec, "lifecycle_hooks").map_err(format_error)?; | ||
| } | ||
| if box_type != boxes::TASK_BOX_TYPE { |
There was a problem hiding this comment.
C1 · A task entry sends no vars or patches. The daemon then keeps the ones that it reads from a legacy [session] table. Line 1205 merges [defaults.session] into session entries only. Make the change in C1: build_composables does not read mfile.session when an entry is present.
There was a problem hiding this comment.
Addressed in d31321f7e6e2: build_composables now starts from Session::default() and leaves [session] packages out when the contribution carries an entry, so a task box gets no legacy [session] vars (C1).
| .retain(|v| !(is_project(v.source()) && names.contains(v.var().name()))); | ||
| contribution.vars.extend(incoming_vars); | ||
| } | ||
| if packages.is_some() { |
There was a problem hiding this comment.
C2 · An entry package joins the box here, but it is not in top_level_names in build_composables. So it gets no env_state_wiring and no fs mappings. A package in [session] gets both. Line 1204 makes a [defaults] package the same as an entry package. Add the entry's packages to top_level_names.
There was a problem hiding this comment.
Addressed in d31321f7e6e2: build_composables adds the entry's packages to top_level_names, so they get env_state_wiring and fs mappings; new test entry_only_package_contributes_its_state_wiring (C2).
| "[session]\nvars = { EDITOR = \"vi\" }\n", | ||
| ) | ||
| .unwrap(); | ||
| let legacy_root = camino::Utf8Path::from_path(legacy.path()).expect("a UTF-8 path"); |
There was a problem hiding this comment.
C1 · The legacy case starts only a session. Also start a task entry from this project, and assert that its vars are empty. The fixture above uses [defaults.session], which the daemon does not read, so it cannot show the leak.
There was a problem hiding this comment.
Addressed in d31321f7e6e2: The legacy [session] project now also declares [tasks.check]; the test starts that task from its entry and asserts its vars are empty.
| /// that predate the field deserialize cleanly. | ||
| #[serde(default)] | ||
| pub orientation: WireOrientation, | ||
| /// The `minimal.toml` entry the box comes from, expanded by the client |
There was a problem hiding this comment.
C3 · This field carries five lists, not the expanded Box Spec that BOX-068 sends and stores. State the gap here and on WireBoxEntry, and name the issue that closes it. Line 1234 says the Box Host receives exactly what min box spec renders.
There was a problem hiding this comment.
Addressed in d31321f7e6e2: WireContribution::entry and WireBoxEntry now state that they carry only the five composed lists, not the expanded Box Spec, that the daemon still reads [stack], and name #3257.
|
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. A person can declare a session entry and a task entry with shared defaults in one minimal.toml and start both boxes from their expanded specs. An old [session] table still works with a hint. The branch now merges cleanly with main, and both proof tests pass. Notes from the agent: Revise round 1 was a merge conflict in crates/minimal/src/tests.rs. Both sides added new code after the same test: this branch added the start_from_entry helper and the expanded-entry test, and main added exec_box_dir and the exec tests. I kept both sides and closed the HEAD function before the main block. The merge commit is 8251333. The other files in the merge diff come from main, not from this task. cargo-nextest is not installed on this box, so I ran each test line with |
|
/revise Arch conditions (arch_rulings 1119, ruled at 8251333), in full:
Owner decision (Karl, 2026-11-10):
Also: |
…l-toml-for-every-box-end-to-end
When the client sends an expanded entry, build_composables no longer reads the project's [session] table, so a task box gets none of it. Entry packages join the closure walk, so their env_state_wiring and fs mappings compose. The wire docs state that the entry carries only the five composed lists, and name #3257 for the full expanded spec.
session_box_entry now returns the SessionEntryUndecided refusal in place of sending no entry, so the start never falls back to [session]. The end-to-end test also starts a task entry in a legacy [session] project and checks that the task gets no vars, and checks the refusal.
…l-toml-for-every-box-end-to-end
|
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. Review threads the coder judged:
One minimal.toml declares a session and a task entry that share [defaults]. Each box starts from its own expanded entry. A legacy [session] table still works with a hint and reaches the session box only, never a task box. An entry package now gets its env_state_wiring. A start that cannot pick a session entry fails with the existing refusal. Notes from the agent: cargo-nextest is not installed on this box, so I ran each test: line through cargo test with the same package and test name. The round-1 failure of expanded_session_and_task_entries_start_both_boxes (exit 101) came from the test helper. It required a Pending ConfigureLoadout response, but a box with nothing to gate gets Materialized. The helper now accepts Materialized, and the test passes. C1 and C2 touch crates/minimald/src/box_manager/composables.rs, which the directive adds to the layers. The new C2 test lives in the tests module of that file. C3: the doc comments on WireContribution::entry and WireBoxEntry name #3257. The PR body must link #3257 as a residual. The runner writes the body. K1: session_box_entry returns SessionEntryUndecided (exit 2) and no longer falls back to [session]. A project with no minimal.toml still sends no entry. The test now also starts a task entry in a legacy [session] project and checks that its vars are empty. It also checks that two session entries with no default give NoDefault. clippy -p minimal -p minimald --all-targets is clean. I did not run just clippy-strict. |
…l-toml-for-every-box-end-to-end # Conflicts: # crates/minimal/src/tests.rs
…l-toml-for-every-box-end-to-end
|
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. A person can declare a session entry and a task entry with shared defaults in one minimal.toml. An old [session] table still works with a hint. Both boxes start from their expanded entries. The branch now merges cleanly with main, and both proof tests pass after the merge. Notes from the agent: This round resolved the merge conflict with main in crates/minimal/src/tests.rs. Both sides only appended tests at the same point. The resolution keeps the branch's entry tests (start_from_entry and the session entry tests) and then main's start and show tests. No function names are duplicated. cargo clippy -p minimal --all-targets reported nothing. The other merged files came from main without conflicts. No thread verdicts changed this round. |
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 4 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs:1508, minimal-arch[bot] crates/boxes/src/daemon/composer.rs:316, minimal-arch[bot] crates/minimal/src/tests.rs:13215, minimal-arch[bot] crates/boxes/src/wire/request.rs:55. 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
The session path conforms to architecture.md lines 1197, 1205 and 1210. The task path does not conform to line 1204: [defaults] merges into every entry. box_entry sends no vars or patches for a task entry, and min task run reads only the [tasks.<name>] table. So those keys from [defaults] and [defaults.task] do not reach a task box. The doc comment on box_entry says the task run applies them, and that is not true. Do C1: correct the comment and file the gap as an issue, or close the gap in this PR.
Ruled at bdb2152 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| /// read as `[defaults.session]` (BOX-050), and takes the lists the daemon | ||
| /// composes from the expanded spec. | ||
| /// | ||
| /// A list the spec has no key for stays `None`. A task entry sends only |
There was a problem hiding this comment.
C1 · This comment says min task run applies a task entry's vars and patches inside the box. min task run reads only the [tasks.<name>] table. A vars or patches key in [defaults] or [defaults.task] does not reach the task box (architecture.md line 1204). Write what is true here and name the issue that tracks the gap.
There was a problem hiding this comment.
Addressed in 03f80cd8f745: Rewrote the box_entry doc comment: a task entry sends no vars, vars_lenient or patches, min task run reads only [tasks.], and #3266 tracks the gap.
| if hooks_enabled { | ||
| wire.lifecycle_hooks = list(spec, "lifecycle_hooks").map_err(format_error)?; | ||
| } | ||
| if box_type != boxes::TASK_BOX_TYPE { |
There was a problem hiding this comment.
C1 · This branch drops the expanded vars, vars_lenient and patches for a task entry, the [defaults] and [defaults.task] layers included. File the gap as an issue and name it here, or send the layered values to the task box.
There was a problem hiding this comment.
Addressed in 03f80cd8f745: Added a comment beside the task branch that says the [defaults] and [defaults.task] vars and patches are dropped, and names #3266.
|
/revise Arch condition (arch_rulings 1127, ruled at bdb2152), in full:
Take the first option: correct the doc comment and name the residual. The gap is filed as #3266. Name it in the PR body: list #3257 and #3266 as residuals. The body does not name #3257 yet, which the C3 ruling on 8251333 asked for. |
…l-toml-for-every-box-end-to-end
The box_entry doc comment claimed that min task run applies a task entry's vars and patches. It reads only the [tasks.<name>] table, so the [defaults] layers are dropped. Name issue #3266 for that gap.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare changes only a comment in crates/minimal/src/cmd/boxes/create.rs. The comment now says that a task entry sends no vars, vars_lenient or patches. It also says that [defaults] and [defaults.task] values for them do not reach a task box. That is true of min task run, and the comment names #3266. This answers the condition on bdb21522. The expansion stays in the client and the composition stays in the daemon, as line 1234 requires. Every entry item still goes to the client gate. The five-list entry (#3257) and the task gap (#3266) stay open as residuals.
Ruled at 03f80cd against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 6 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs:1513, minimal-arch[bot] crates/boxes/src/daemon/composer.rs:316, minimal-arch[bot] crates/minimal/src/tests.rs:13215, minimal-arch[bot] crates/boxes/src/wire/request.rs:55, minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs:1450, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
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 person can declare a session entry and a task entry with shared defaults in one minimal.toml, and both boxes start from their expanded specs. An old [session] table still works and prints a hint. The box_entry doc comment now states that a task entry sends no vars, vars_lenient or patches, and it names #3266 for that gap. Notes from the agent: Revise round 2 took the first option of arch condition C1. The change is comments only: the box_entry doc comment and a comment beside the |
T21 — One minimal.toml for every box, end to end
slice: S4
kind: integration
covers: none
size: L
est: 300 code, 240 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: daemon log names each box's entry
observability: one span from expansion to creation
layers:
- client:
- crates/minimal/src/cmd/boxes/create.rs: sends the expanded entry
- crates/minimal/src/task.rs: sends the expanded entry
- crates/minimal/src/tests.rs: tests
test: cargo nextest run -p minimal expanded_session_and_task_entries_start_both_boxes
- daemon:
- crates/boxes/src/daemon/composer.rs: composes from the spec
- crates/boxes/src/wire/request.rs: carries the expanded entry
test: cargo nextest run -p boxes daemon_composes_from_received_spec_keeping_absent_lists_apart
produces: none
consumes: none
proof: declare a session and a task entry with shared defaults in one minimal.toml, keep an old
[session]table working with a hint, and start both boxes from their expanded specs.depends on:
- blocked by #2409
- blocked by #2410
- blocked by #2411
- blocked by #2412
- blocked by #2416
- blocked by #2413
- blocked by #2414
- blocked by #2415
Proof
A person can write one minimal.toml with a [defaults] table that a [sessions.default] entry and a [tasks.check] entry share.
min session startandmin task runthen expand each entry and send it to the daemon. The daemon composes and starts each box from its own expanded entry: the session gets the shared hook, its own hook and the [defaults.session] vars, and the task gets only the shared hook. A project with only an old [session] table still starts. The daemon reads the table as [defaults.session] of the implicit default entry, and the file load prints the existing legacy hint. The test expanded_session_and_task_entries_start_both_boxes shows all three boxes go through create, compose, verdict and finalize on a real harness daemon.Notes from the agent: Files outside the layers: crates/boxes/src/client/composer.rs and crates/boxes/src/core/compose/tests.rs. Each got one
entry: Noneline in a WireContribution literal that lists every field. The new field does not compile without it. Every other literal in the workspace uses..Default::default().The tests ran with
cargo test, notcargo nextest, because cargo-nextest is not installed here. The test names are the same.Design: WireContribution carries
entry: Option<Box<WireBoxEntry>>(boxed for clippy large_enum_variant). Each list in it is an Option. The daemon composer applies the lists as follows. A presentpatchesorlifecycle_hookslist replaces the project's own items of that kind, so an empty list clears them.varsandvars_lenientoverride by name.packagesadds by name. An absent list leaves the daemon's own project items alone. Vars and packages do not replace, because minimald folds the stack's vars and packages into the project list, and a replace would drop them. Package and loadout items are never touched.A task entry sends only
packagesandlifecycle_hooks. Itsvarsandpatcheshave the task shape, andmin task runapplies them inside the box. With `--…[truncated]Tests
cargo nextest run -p minimal expanded_session_and_task_entries_start_both_boxescargo nextest run -p boxes daemon_composes_from_received_spec_keeping_absent_lists_apartVerify
min task run agent-verifyexited 101 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/boxes/src/client/composer.rscrates/boxes/src/core/compose/tests.rsCloses #2418