Repository navigation
feat(minimal): T126 — Refuse a named loadout on a sealed box, a loadout that sets loadout_mode, and a contradicted constraint; a sealed box reads no default loadouts - #3301
Conversation
Expansion now refuses three cases with exit 3 before a loadout applies: a type constraint that holds loadout_mode at another value, a loadout named by --loadout for a sealed box, and a loadout on an open box that sets loadout_mode. loadout_gate runs the first two checks without a loadout, so a client can call it before it reads a loadout file. Covers BOX-172, BOX-173 and BOX-181.
LoadoutFile dropped a key it did not know, so a loadout that set loadout_mode looked like one that did not. The loader now keeps the key, and Loadout::sets_loadout_mode lets the expansion refuse it (BOX-172).
min task run and a session start now read the entry's loadout_mode first. A sealed box selects no loadout, so a missing or broken default loadout cannot fail it, and no line prints about the skipped defaults. A --loadout for a sealed box, or a type constraint the entry breaks, fails with exit 3 before a loadout file is read. On an open box, a loadout that sets loadout_mode fails with exit 3. Covers BOX-171, BOX-172, BOX-173, BOX-175 and BOX-181.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Reviewed: no findings; approval withheld, 10 threads unresolved at APPROVE withheld: 10 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs, minimal-arch[bot] crates/minimal/src/loadouts.rs, minimal-arch[bot] crates/minimal/src/loadouts.rs, minimal-arch[bot] crates/minimal/src/loadouts.rs:736, minimal-arch[bot] crates/mfile/src/expand.rs:430, …. 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. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 4 conditions · the full ruling
The gate is in the right owner, and the refusals run in the order that the spec gives. Two client paths fail open. session_loadout_mode drops an undecided session pick and opens the gate on a bare session entry. It then reads loadout files for a start that is refused later. Make the change in C1. entry_loadout_gate drops the type's constraints when a project type does not resolve, and no start path refuses that type at this head. Line 1264 makes such a chain exit 3. Make the change in C2. File issues for the min dash path (C3) and for the duplicate constraint check (C4), and name them in the PR body.
Ruled at a02789c against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| let typed = entry.and_then(|name| entries.iter().find(|e| e.name == name)); | ||
| let picked = match typed { | ||
| Some(entry) => Some(entry.clone()), | ||
| None => pick_session_entry(&entries).ok(), |
There was a problem hiding this comment.
C1 · This .ok() drops an undecided pick. The gate then uses a bare session entry, which is open, and reads the default and --loadout files. session_box_entry refuses the same start later with exit 2. Use pick_session_entry(&entries)? here, as session_box_entry does. Keep the bare entry only when there is no project file.
There was a problem hiding this comment.
Addressed in e4db6f54db44: session_loadout_mode now uses pick_session_entry(&entries)? when a project file exists, so an undecided pick exits 2 before any loadout is read. The bare entry applies only with no project file.
| /// | ||
| /// An entry whose type does not resolve takes the defaults of its type | ||
| /// name when that name is built in, and no type layer otherwise. The start | ||
| /// refuses such an entry elsewhere, so the gate does not refuse it first. |
There was a problem hiding this comment.
C2 · This sentence is not true at this head. No start path resolves Box Types, in crates/minimal or in crates/minimald. Delete it.
There was a problem hiding this comment.
Addressed in e4db6f54db44: Deleted the sentence that says the start refuses such an entry elsewhere.
| match mfile::expand::expandable_type(entry, &types) { | ||
| Ok(resolved) => (resolved.defaults, resolved.constraints, project_types), | ||
| Err(error) => { | ||
| tracing::debug!(%error, "the loadout gate reads the entry without its type"); |
There was a problem hiding this comment.
C2 · For an entry whose type is not built in, this fallback drops the chain's constraints and decides the mode without them. Line 1264 makes a chain with no built-in root exit 3. Return the type error with exit 3 through a new mfile::Error variant. Keep the fallback for a built-in type name only.
There was a problem hiding this comment.
Addressed in e4db6f54db44: A type name that is not built in and does not resolve now returns mfile::Error::BoxType (exit 3) through ProjectFileInvalid. The fallback stays only for a built-in type name.
| let initial_policy = user_policy.clone(); | ||
| let compose_options = compose_options_from_config(cfg); | ||
| let active = resolve_active_loadouts(selection, cfg, global)?; | ||
| // BOX-172: the gate read the mode before any loadout, so a loadout's |
There was a problem hiding this comment.
C3 · The min dash create path also calls this function, but it never calls entry_loadout_gate. A dashboard create of a sealed entry still reads and applies loadouts. File an issue for that path, and name it in the PR body.
There was a problem hiding this comment.
Addressed in e4db6f54db44: Filed as #3303 for the min dash create path. It is named as a residual in the PR body. The code is unchanged in this PR.
| /// The most specific type of the chain whose constraints hold | ||
| /// `loadout_mode`, as a message names it, with the values the constraint | ||
| /// admits. A constraint is one value or a list of values. | ||
| fn holding_constraint(layers: &Layers) -> Option<(String, Vec<String>)> { |
|
Round 1 of 10 on the runtime's review findings: the coder changed nothing. Revise round 1: no code change this round. Both threads were judged against the branch. The dashboard create path (crates/minimal/src/dash/app.rs) still resolves default loadouts without the loadout-mode gate. That file is outside this task's layers, so a follow-up task must apply gate_selection there. Review threads the coder judged:
|
|
/revise Arch conditions (arch_rulings 1173), in full:
C3 is filed as #3303, and C4 as #3304. Name both in the PR body as residuals, and name #3304 in the |
…amed-loadout-on-a-sealed-box-a-loadout
…gate The loadout gate now picks the session entry as session_box_entry does, so an undecided pick ends with exit 2 before any loadout is read. An entry whose type is not built in and does not resolve fails with the new mfile::Error::BoxType (exit 3). ConstraintViolated names #3304.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Each refusal fails before a loadout is applied. This includes an undecided session pick (exit 2) and an entry whose project type does not resolve (exit 3). A sealed task never reads a broken default loadout and prints nothing about it. Notes from the agent: Revise round 2. C1: session_loadout_mode now picks the entry as session_box_entry does. An undecided pick ends with SessionEntryUndecided (exit 2), and the bare entry applies only when there is no project file. C2: entry_loadout_gate returns the new mfile::Error::BoxType (exit 3) for a type name that is not built in and does not resolve. The built-in fallback stays, and the false doc sentence is deleted. C3 and C4 are filed as #3303 and #3304. Name both as residuals in the PR body. The ConstraintViolated doc comment now names #3304. Two new tests in crates/minimal/src/tests.rs pin C1 and C2: undecided_session_pick_refuses_before_loadouts and unresolved_project_type_refuses_loadout_gate_exit3. Both pass. I did not run just clippy-strict. |
|
The runtime will not revise this pull request again on its own: the diff is +954/-49 (1003 lines), past 3x the plan's estimate of 300 lines (est: 140 code, 160 tests, as #3254 reads now), at round 2 (the last trigger was a /revise note). A person decides now. Remove |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The gate now fails closed on an undecided pick and on an unresolved type. That closes the gaps from the last round. Two paths still read loadouts with no gate. min box spec <entry> expands with no constraints and no named loadouts. Thus it does not refuse what min box start refuses, and it reads a sealed box's defaults (§ minimal.toml layout line 1234; fix in C1). The min dash create still applies default loadouts to a sealed entry (§ Policy layers line 1285; fix in C2).
Outside the diff
crates/minimal/src/cmd/box_spec.rs:693: C1 · This call resolves loadouts with no gate. Line 1234 says the render is exactly what the host receives. Callentry_loadout_gate(Some(&bytes), entry, &overrides.loadout)first, and passgate_selection(.., mode). Then refuse a loadout whosesets_loadout_mode()is true.crates/minimal/src/dash/app.rs:1281: C2 · The dashboard create passesDefaultswith no gate. A sealed entry then reads and applies the default loadouts. Line 1285 says loadouts are subject toloadout_mode. Callcrate::session_loadout_mode(&root, None, &[])and passgate_selection(loadouts, mode).
Ruled at e4db6f5 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| held_by: held.map(|(box_type, _)| box_type), | ||
| }); | ||
| } | ||
| // The daemon log names the box's mode at creation; this is the client's |
There was a problem hiding this comment.
This comment says the daemon log names the mode. minimald does not log loadout_mode, and the PR body says so. Correct the comment. File the daemon log line as an issue.
There was a problem hiding this comment.
Addressed in 887f7cec4680: Rewrote the comment: it now says this is the client's record and that minimald does not log the box's loadout mode. I have no gh access, so the issue for the daemon log line is listed in the notes for a person to file.
|
/revise Arch conditions (arch_rulings 1176), in full:
C2 closes the gap that residual #3303 tracks. Replace #3303 in the PR body with "Closes #3303". |
min box spec <entry> now reads the box's loadout mode before any loadout, as min box start does. A sealed entry refuses --loadout and reads no default loadout. A loadout that sets loadout_mode refuses. A min dash create reads the mode of the project's session entry, so a sealed box reads no default loadout.
…amed-loadout-on-a-sealed-box-a-loadout # Conflicts: # crates/minimal/src/tests.rs
|
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:
Each refusal fails with exit 3 before a loadout is applied, on min box start, min task run and now min box spec . A sealed task, a sealed session, a sealed min box spec and a sealed min dash create never read a broken default loadout or print about it. Notes from the agent: Revise round 3. C1: expand_entry in crates/minimal/src/cmd/box_spec.rs calls entry_loadout_gate before resolve_active_loadouts, gates the selection, and refuses a loadout that sets loadout_mode. Test box_spec_entry_gates_loadouts_as_start covers the four cases in the note. A sealed entry renders '# Loadouts: none.' and exits 0. C2: crates/minimal/src/dash/app.rs gets create_loadout_selection, called in the Effect::Create spawn_blocking closure before prepare_activation_inputs_with_config. Test sealed_dash_create_reads_no_default_loadout is in the app.rs tests module. box_spec.rs and dash/app.rs are outside the task's layers. The /revise note named both files, so I changed them. One gap remains: min dash still resolves the default loadouts once at startup (app.rs, the resolve_active_loadouts call in run), before it knows a project. A broken default loadout still fails min dash startup. C2 did not name that call, so I left it. I cannot use gh. The runner or a person must file the issue for a minimald log line that names the box's loadout mode, and must write 'Closes #3303' in place of the #3303 residual in the PR body. just clippy-strict was not run. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The gate is in the right place on every creation path, and it reads no loadout before it decides the mode. One path is still open. min box spec on an open task entry reads no loadout, but min task run on the same entry applies its default loadouts. So a default loadout that sets loadout_mode fails the run and passes the spec. Line 1234 says the spec render is exactly what the host receives, and BOX-172 says both refuse. Run the gate and the loadout check for a task entry as C1 says.
Ruled at 887f7ce against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| // the start. Folding their values into the spec is #3236. | ||
| // The box's loadout mode, read before any loadout as `min box start` | ||
| // reads it. A sealed box selects no loadout (BOX-171, BOX-173). | ||
| let loadout_mode = |
There was a problem hiding this comment.
C1 · This gate runs only for an entry that is not a task. An open task entry takes its default loadouts at min task run, and that run refuses one that sets loadout_mode. Here the task branch reads no loadout. Move the gate, the selection and the BOX-172 check above the if box_type == boxes::TASK_BOX_TYPE split (line 1234 of the design).
There was a problem hiding this comment.
Addressed in 583032c10bf6: Fixed in commit 498e164: entry_loadout_gate, gate_selection and the BOX-172 check now run above the task/session split in box_spec.rs.
| &cfg, | ||
| global, | ||
| )?; | ||
| // BOX-172: a loadout that sets `loadout_mode` fails the start, so it |
There was a problem hiding this comment.
BOX-172 · This check does not run for an open task entry, so min box spec passes what min task run refuses. Run it for every entry, as C1 says.
There was a problem hiding this comment.
Addressed in 583032c10bf6: Fixed in commit 498e164 with C1: the BOX-172 check now runs for every entry, so min box spec refuses an open task entry the same way min task run does.
|
/revise Arch condition (arch_rulings 1183, ruled at 887f7ce), in full:
|
min box spec now reads the loadout mode and selects loadouts for every entry, as min task run does. An open task entry with a default loadout that sets loadout_mode now fails with exit 3.
A constraint that admits open does not hold the box sealed. The refusal now keeps the open remedy for it. Also formats box_spec.rs.
…amed-loadout-on-a-sealed-box-a-loadout # Conflicts: # crates/mfile/src/error.rs # crates/mfile/src/expand.rs
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms to the design. The loadout gate is in mfile::expand, and every creation path calls it before it reads a loadout file. min box spec calls it too, so the start and the spec agree, as architecture.md line 1234 requires. The two fix commits close the task-entry spec path and the BOX-173 remedy wording. The daemon log does not name the box's loadout mode yet. The design does not require it, but the task plan does. File that residual as an issue.
Ruled at ded1d14 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
…amed-loadout-on-a-sealed-box-a-loadout # Conflicts: # crates/mfile/src/expand.rs
Main made BoxTypeError larger. Clippy result_large_err then failed with -D warnings on mfile::Error and on the gate in loadouts.rs.
|
Round 5 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:
With main merged in, each refusal (a loadout that sets loadout_mode, a named loadout on a sealed box, a constrained loadout_mode) still fails with exit 3 before a loadout is applied. A sealed task still never reads a broken default loadout and prints nothing about it. Notes from the agent: Merge: the one conflict in crates/mfile/src/expand.rs held two sets of new functions. I kept both: this branch's loadout gate and the check_constraints function from #3292. box_types.rs and spec.rs merged cleanly. The merge made BoxTypeError larger, so clippy result_large_err failed with -D warnings. I boxed the Error::BoxType variant (error.rs). I also replaced an and_then closure with a match in loadouts.rs. clippy -p mfile and -p minimal --all-targets -- -D warnings now pass. The behaviour arch passed at ded1d14 did not change. Out of layers: crates/minimal/src/cmd/box_spec.rs got a doc comment fix for thread PRRT_kwDOPdfcxM62fNF0. Earlier rounds already touched it. Commitlint: the failed check is about two earlier commits (498e164, 3dab862), where body lines are longer than 100 characters. Fixing them needs a history rewrite, which this round must not do. The new commits keep body lines short. I did not run just clippy-strict. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The merge from main adds built-in constraints. The build and container-build types now hold loadout_mode = "sealed". The fallback in entry_loadout_gate gives the gate built-in defaults with no constraints. A build entry that sets loadout_mode = "open" then passes the gate, and the start reads its loadouts. Lines 1212 and 1219 of the architecture make that entry a validation error, and BOX-181 refuses it before any loadout is read. Make the change in C1. The rest of the PR keeps the PASS from ded1d148.
Ruled at 583032c against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| Err(error) => match builtin_type_layer(entry) { | ||
| Some(layer) => { | ||
| tracing::debug!(%error, "the loadout gate reads the entry with its built-in type"); | ||
| (layer, Vec::new()) |
There was a problem hiding this comment.
C1 · This arm gives the gate no constraints. After the merge, the built-in build type constrains loadout_mode to sealed (architecture line 1219). Pass builtin_constraints(root) here, so the gate refuses an entry that sets open.
There was a problem hiding this comment.
Addressed in 16cd43cada4e: The fallback arm now passes builtin_constraints(root) to Layers::type_constraints, so a build entry that sets open fails with exit 3.
| .box_type | ||
| .as_deref() | ||
| .and_then(mfile::box_types::builtin) | ||
| .map(|root| vec![(root.to_owned(), mfile::box_types::builtin_defaults(root))]) |
There was a problem hiding this comment.
C1 · Return the built-in constraints together with the built-in defaults. mfile::box_types::builtin_constraints(root) gives them.
There was a problem hiding this comment.
Addressed in 16cd43cada4e: builtin_type_layer() now returns the built-in defaults and mfile::box_types::builtin_constraints(root) together.
|
/revise Arch condition (arch_rulings 1188, ruled at 583032c), in full:
The owner accepted this PR's size on 2026-11-12. The extra lines are arch-directed scope. |
An entry of a built-in type that does not resolve as a start, such as build, now keeps its built-in constraints at the gate. A build entry that sets loadout_mode = open fails with exit 3 before a loadout is read.
|
Round 6 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:
Each refusal fails with exit 3 before a loadout is applied. A [boxes.b] entry with type = "build" and loadout_mode = "open" now fails with exit 3 and names the built-in build type, and it reads no loadout. A sealed task never reads a broken default loadout and prints nothing about it. Notes from the agent: C1: builtin_type_layer() now returns the built-in defaults and the built-in constraints. The fallback arm of entry_loadout_gate() passes the constraints to Layers::type_constraints. The fallback comment and the doc comment say this now. The new test builtin_build_entry_setting_open_refused_exit3 in crates/minimal/src/tests.rs pins it. The commitlint failure is on earlier commits: two body lines are longer than 100 characters and one footer has no leading blank line. Those commits are already on the branch, and the rules forbid a rewrite or a force-push, so they still fail. A squash merge with a clean message avoids the failure. The new commit passes the rules. cargo clippy -p minimal --all-targets and cargo fmt --check printed nothing. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head conforms. The gate fallback now reads the built-in constraints of the type name. A build entry that sets open now fails with exit 3 before a loadout file is read. Line 1219 of the architecture says that build holds loadout_mode at sealed. The new test proves this rule with a broken default loadout present. The rest of the change keeps the PASS on ded1d148.
Ruled at 16cd43c against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 11 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/boxes/create.rs, minimal-arch[bot] crates/minimal/src/loadouts.rs, minimal-arch[bot] crates/minimal/src/loadouts.rs, minimal-arch[bot] crates/minimal/src/loadouts.rs:736, minimal-arch[bot] crates/mfile/src/expand.rs:430, …. 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 head is a merge of main at 3edbb8e9. The merge changes only CI workflows and docs/commit-conventions.md. The PR's own diff is identical to the diff that the PASS on 16cd43ca covered. The loadout gate still checks a Box Type constraint before layer 6, and a sealed box still reads no loadout, as the design requires (architecture.md lines 1285 and 1288). The PASS carries forward with no conditions.
Ruled at 0809073 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
T126 — Refuse a named loadout on a sealed box, a loadout that sets loadout_mode, and a contradicted constraint; a sealed box reads no default loadouts
slice: S5
kind: task
covers: BOX-171, BOX-172, BOX-173, BOX-175, BOX-181
size: M
est: 140 code, 160 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: each refusal names the loadout and the layer that set loadout_mode; the daemon log names the box's mode
observability: the daemon log names the box's mode at creation
layers:
- expansion:
- crates/mfile/src/expand.rs: refuse a constrained mode, a named loadout on a sealed box and a loadout-set mode before the loadouts layer
- crates/mfile/src/error.rs: the two loadout refusals
test: cargo nextest run -p mfile loadout_setting_loadout_mode_refused_exit3
test: cargo nextest run -p mfile named_loadout_on_sealed_box_refused_exit3
test: cargo nextest run -p mfile constrained_loadout_mode_refused_before_loadouts
- loadout:
- crates/boxes/src/core/loadout.rs: the loader keeps the raw loadout_mode key
- client:
- crates/minimal/src/loadouts.rs: a sealed box reads no default loadouts
- crates/minimal/src/cmd/boxes/create.rs: reads the mode before resolving loadouts; prints nothing for a skipped default
- crates/minimal/src/task.rs: the same for min task run
- crates/minimal/src/tests.rs: tests
test: cargo nextest run -p minimal sealed_task_run_prints_nothing_about_loadouts
test: cargo nextest run -p minimal sealed_box_ignores_broken_default_loadout
produces: none
consumes:
- crates/mfile/src/expand.rs
proof: Each refusal fails before a loadout is applied, and a sealed task never reads a broken default loadout or prints about it.
depends on:
- blocked by #3247
Proof
Each refusal fails with exit 3 bef 7438 ore a loadout is applied. A type constraint that the entry's loadout_mode breaks fails first. A --loadout for a sealed box fails next, and names the loadout, the layer that set sealed, and the remedy. On an open box, a loadout that sets loadout_mode fails and names the loadout. min task run of a sealed task (the task default) no longer reads the default loadouts. A missing or broken default loadout does not fail it, and no 'Applying loadouts' line prints. A session entry set to sealed behaves the same way.
Notes from the agent: Design: mfile::expand::Layers has three new fields: type_constraints, named_loadouts and loadouts_setting_mode. The new public loadout_gate runs the BOX-174, BOX-181 and BOX-173 checks in that order and reads no loadout. expand runs that gate and then the BOX-172 check. The client does not apply loadouts through mfile::expand today. So the client calls loadout_gate on the start's entry (loadouts::entry_loadout_gate, through session_loadout_mode and task_loadout_mode in create.rs) before it resolves any loadout. A sealed box gets LoadoutSelection::None. The BOX-172 check in the client runs in prepare_activation_inputs_with_config, on the loaded loadouts (Loadout::sets_loadout_mode). That check also covers the min dash create path, which shares that function but does not run the gate.
Three things are outside this task's scope. (1) No BOX-061 constraint check existed before this task. I added Error::ConstraintViolated and use it only for loadout_mode, so the general BOX-061 final check is still owed. The constraint value can be a string or a list of strings. (2) The client logs the box's loadout mode at info (tracing, target minimal::loadouts), and mfile logs it at debug. The daemon is not in this task's layers, so the daemon log does not name the mode yet. (3) An entry whose type does not resolve falls back to the built-in defaults of its type name, so the gate does not refuse it before the start…[truncated]
Tests
cargo nextest run -p mfile loadout_setting_loadout_mode_refused_exit3cargo nextest run -p mfile named_loadout_on_sealed_box_refused_exit3cargo nextest run -p mfile constrained_loadout_mode_refused_before_loadoutscargo nextest run -p minimal sealed_task_run_prints_nothing_about_loadoutscargo nextest run -p minimal sealed_box_ignores_broken_default_loadoutVerify
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.Closes #3254