Repository navigation
T20 — Scaffold and append entries with min init - #2962
minimal-foundry[bot] wants to merge 12 commits into
Conversation
min init now writes minimal.toml in the entry shape: [defaults.session], [defaults.task] and a [boxes.dev] session entry. min init <entry> --type <type> appends one [boxes.<entry>] entry. --session, --task, --agent and --service are aliases for --type with that noun. More than one of them, or one with --type, ends with exit 2 and the file unchanged. A type that is, or extends, agent, service, build or container-build ends with exit 3. The activate scaffold offer keeps the legacy shape, because this tree does not read entries at activation yet.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Reading This comment updates when the review finishes. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The scaffold shape and the append conform to architecture.md line 448 and lines 1173–1176. A refusal ends the process with std::process::exit inside the root span. The spool records a span only when it ends, so D9 (line 793) loses the refused outcome. C1 routes the exit code through failure_exit_code and adds cmd/mod.rs and main.rs to the layers. init_request accepts self and names that are not DNS-safe, against line 1176. C2 refuses them with exit 2. C3 links the legacy-shape interim to an issue with a flip condition. The escalation asks whether BCLI-017 can merge before the BOX reader of [defaults.session].
Outside the diff
crates/minimal/src/cmd/mod.rs:294: C1 · This arm formats the error to a string, and theInitRefusaltype is lost. Mapmctx::Error::Other(e)toe, and format only the other variants.crates/minimal/src/main.rs:208: C1 · Add a branch: whene.downcast_ref::<minimal::InitRefusal>()matches, returnrefusal.codeasu8. Add a test for exit 2 and exit 3.
Ruled at 5c90f9d against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Held by the maintainer: this waits on BOX T15 (#2409), which reads |
|
/revise Head 1. C1: a refusal must end through Delete 2. C2: entry names (architecture line 1176, Addressing line 390). In 3. C3: the two-shape interim is no longer needed. Remove it. C3 asked for an issue for the BOX reader of |
…nd-append-entries-with-min-init
cmd_init returns an InitRefusal as its error, so the cmd span closes and its outcome record reaches the spool. failure_exit_code in main.rs gives the refusal's code (2 or 3). exit_refused is gone. min init refuses the entry name self, and a name that is not a DNS label, with exit 2 before it reads any file. The activate scaffold stays in the legacy shape: session start does not read [defaults.session] yet. The comment names #2410 as the flip.
…nd-append-entries-with-min-init
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The compare fixes the exit path. Refusals now end through failure_exit_code, and the cmd span closes. Three exit codes still do not agree with the design. Line 1176 makes self, a name that is not DNS-safe, and a duplicate name invalid minimal.toml, and line 364 gives exit 3 for that. mfile::entry already uses 3. C1 and C2 change check_entry_name and the duplicate check to code 3, and C2 also replaces ENTRY_TABLES with EntryTable::ALL. C3 makes a [box_types] entry with no extends exit 3, as line 1232 states.
Ruled at af34c8a against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
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:
Notes from the agent: The branch already contained origin/main (merge 1a65936 includes #3091), so no new merge was necessary. C1 and C2 are done as the directive asks. cmd/mod.rs and main.rs are outside layers:, and the directive names them. InitRefusal.code is now u8, because failure_exit_code returns u8. INIT_ONE_TYPE_FLAG is now pub so that the main.rs test can use it. C3 is NOT done as the directive asks, on purpose. The premise is that the reader of [defaults.session] is on main. It is not. #3091 adds mfile::entry::read_entries, which nothing outside mfile calls. It reads only the entry tables. mfile::File still has no [defaults.session]: that key lands in Defaults.extra and logs 'unknown fields in [defaults]'. Session start reads packages only from the legacy [session]. If the activate scaffold flips to the entry shape, a plain |
|
/revise Head 1. C1: an invalid entry name is a minimal.toml error, exit 3. In 2. C2: a duplicate entry name exits 3, and the table list has one owner. In 3. C3: a In Also add one line to the PR body, before the notes: |
|
Marked ready by the BCLI babysitter, with the owner's approval. The runtime drafted this PR because |
An entry name that is self or not DNS-safe, a duplicate entry name, and a [box_types] entry with no extends make minimal.toml invalid, so min init refuses them with exit 3. The duplicate check reads the entry tables from mfile::entry::EntryTable::ALL.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 9 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/project.rs, minimal-arch[bot] crates/minimal/src/cmd/project.rs, minimal-arch[bot] crates/minimal/src/cmd/project.rs:532, minimal-foundry[bot] crates/minimal/src/cmd/project.rs:839, minimal-arch[bot] crates/minimal/src/cmd/project.rs, …. 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
The compare resolves the open conditions. An invalid entry name, self, a duplicate name and a [box_types] entry with no extends now exit 3. This agrees with design lines 1176 and 1232 and the exit-code table. The flag conflicts stay exit 2, as line 448 states. The duplicate check now uses mfile::entry::EntryTable::ALL, so mfile owns the table list. Two residuals stay open: #2410 for reading [defaults.session], and gominimal/inbox#579 for organization types.
Ruled at 127ec0e against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
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:
Notes from the agent: Revise round 2 did only the three ruling-1014 items. C1: check_entry_name returns code 3 for |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @agent-137 closed the last one. The review of 127ec0ee8d8f 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.
|
Held by the owner until #2410 (BOX T16, PR #3093) lands. Arch passes this head, and minimal-review found nothing. But |
|
/revise Head
#2410 (BOX T16, #3093) merged, but it only adds
|
…nd-append-entries-with-min-init
Box creation does not read [defaults.<type>] until #2418 (BOX T21), so an entry-shaped scaffold loses its packages at session start. Both init callers pass INIT_ENTRY_SHAPE, which is false until then.
|
Round 3 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: Residual for the PR body, before the notes: BCLI-017's entry-shape scaffold is built and tested, but How I did it: |
|
/revise Head 1. Thread PRRT_kwDOPdfcxM60qlds (LOW): the scaffold span names an entry the file lacks.
2. Thread PRRT_kwDOPdfcxM60qldo (MEDIUM): This is the owner's decision, so do not change it. Reply on the thread: on The PR body still shows the round-0 notes, which describe the removed |
…nd-append-entries-with-min-init
The legacy [session] scaffold has no dev entry, so the span records shape = "legacy" and leaves entry out while INIT_ENTRY_SHAPE is false.
| fn find_init_mfile(dir: &std::path::Path) -> Option<std::path::PathBuf> { | ||
| [ | ||
| dir.join(mfile::MFILE_NAME), | ||
| dir.join(".minimal").join(mfile::MFILE_NAME), | ||
| ] | ||
| .into_iter() | ||
| .find(|path| path.is_file()) | ||
| } | ||
|
|
||
| /// Append the entry to the project's `minimal.toml` in `dir`, under the | ||
| /// file's edit lock, and return the path written. A refusal comes back as | ||
| /// an [`InitRefusal`] inside the error, and leaves the file unchanged. | ||
| pub(crate) fn append_entry_in( | ||
| dir: &std::path::Path, | ||
| entry: &str, | ||
| box_type: &str, | ||
| ) -> Result<std::path::PathBuf, anyhow::Error> { | ||
| let Some(path) = find_init_mfile(dir) else { | ||
| return Err(InitRefusal { | ||
| code: 1, | ||
| message: format!( | ||
| "No minimal.toml in {}. Run min init to create one, then append the entry.", | ||
| dir.display() | ||
| ), | ||
| } | ||
| .into()); | ||
| }; |
There was a problem hiding this comment.
MEDIUM · correctness · confidence 0.60 · in find_init_mfile
find_init_mfile looks only in the given directory. min add and the other commands search the ancestor directories (MFileSearchStrategy::CurrentDirRecursive in crates/mctx/src/lib.rs). So min init worker --type task from a project subdirectory fails with "No minimal.toml", and the hint to run min init would create a nested file. The function also picks the root file silently when both layouts exist. mfile::File::from_dir refuses that case with ConflictingLayouts. Use mfile::File::from_dir_recursive to find the path, or the same search as the other commands.
What the reviewer checked
find_init_mfile: [dir.join(MFILE_NAME), dir.join(".minimal").join(MFILE_NAME)].find(is_file); no ancestor walk, no conflict check
Hand this to an agent
Act on a code-review finding.
File: crates/minimal/src/cmd/project.rs
Lines: 899-925
Category: correctness
Severity: medium (reviewer confidence 0.60)
Finding:
`find_init_mfile` looks only in the given directory. `min add` and the other commands search the ancestor directories (`MFileSearchStrategy::CurrentDirRecursive` in `crates/mctx/src/lib.rs`). So `min init worker --type task` from a project subdirectory fails with "No minimal.toml", and the hint to run `min init` would create a nested file. The function also picks the root file silently when both layouts exist. `mfile::File::from_dir` refuses that case with `ConflictingLayouts`. Use `mfile::File::from_dir_recursive` to find the path, or the same search as the other commands.
First verify this against the code yourself. It is model output and
may be wrong. If it is wrong, or the tradeoff was deliberate, say so
and change nothing.
👍 useful · 👎 noise — react to tell the reviewer. Replying works too, and says more.
There was a problem hiding this comment.
Addressed in 5cf31bab27bf: Fixed in 5cf31ba: find_init_mfile uses mfile::File::from_dir_recursive (from_dir with --repo-dir), refuses both layouts with exit 3 naming both paths, and two tests pin the subdirectory and conflict cases.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
BCLI-018 and the exit codes are unchanged, and the PASS on 127ec0e stands for them. Bare min init now writes the legacy [session] table. The design's minimal.toml layout (lines 1165 and 1173) and its canonical example have no such table. The interim is honest and its reason is true: box creation does not merge [defaults.<type>] yet. C1 is the flip when #2418 lands, and it stays open as that residual. Also update the PR body's Proof section. It still says that min init writes the entry shape.
Ruled at f2efb84 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
| /// Whether `min init` and the `min activate` scaffold write the entry shape. | ||
| /// `false` until #2418 (BOX T21): box creation still reads packages from | ||
| /// `[session]`, so an entry-shaped scaffold loses them at session start. | ||
| pub(crate) const INIT_ENTRY_SHAPE: bool = false; |
There was a problem hiding this comment.
C1 · This const keeps bare min init on the legacy [session] table. The design layout (architecture.md lines 1165 and 1173) has no such table. When #2418 lands, set this to true. Then remove the const and the entry_shape parameter of run_init_flow.
|
Round 4 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: Round 4 changes only the scaffold tracing record in cmd_init (commit f2efb84). It records shape = "legacy" and leaves out entry while INIT_ENTRY_SHAPE is false. The append records keep entry = . No other behaviour changed. BCLI-017's default entry-shaped scaffold is not on by default. That is the owner's decision, and it waits on #2418. |
|
/revise Head
Reply on the thread with the commit. |
…nd-append-entries-with-min-init
min init <entry> --type <type> now locates the project's minimal.toml with mfile::File::from_dir_recursive, the search the other commands use, so it works from a project subdirectory. With --repo-dir it looks in that directory only, as MFileSearchStrategy::Override does. Both layouts in one directory are refused with exit 3, and the message names both paths.
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The new search for minimal.toml conforms to the Discovery line (architecture.md line 1167). But mfile::File::from_dir now parses the file before the append. A minimal.toml that does not parse now exits 1 through the generic error arm in find_init_mfile. The Exit codes table (line 364) gives 3 for invalid configuration, and this head gave 3 for that file before this commit. Make the change in C1. Then the change conforms.
Ruled at 5cf31ba against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
| ), | ||
| } | ||
| .into()), | ||
| Err(e) => Err(anyhow::Error::new(e).context("reading minimal.toml")), |
There was a problem hiding this comment.
C1 · A minimal.toml that does not parse comes here as mfile::Error::Format and exits 1. The Exit codes table (line 364) gives 3 for invalid configuration. Keep mfile::Error::IO on this arm. Return every other mfile::Error as InitRefusal { code: 3, .. } that names the file. Pin it with init_invalid_mfile_exit3_appends_nothing.
|
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: Revise round 5, commit 5cf31ba. find_init_mfile now calls mfile::File::from_dir_recursive without --repo-dir (MFileSearchStrategy::CurrentDirRecursive), and mfile::File::from_dir with --repo-dir (MFileSearchStrategy::Override). mfile::Error::ConflictingLayouts becomes an InitRefusal with code 3 that names both paths. NotFound stays code 1, and its message adds 'or its parent directories' when the search walks up. Other mfile errors (for example a parse error) are now reported before the append, with the context 'reading minimal.toml'. The bare scaffold is unchanged and still writes to the current directory. New tests: init_entry_type_appends_from_subdirectory and init_conflicting_layouts_exit3_appends_nothing. cargo clippy -p minimal --all-targets is clean. just clippy-strict was not run. |
|
/revise Head C1: a In
Add the test |
| match append_entry_in(&dir, search_ancestors, &entry, &box_type) { | ||
| Ok(path) => { | ||
| tracing::info!(verb = "init", entry, box_type, outcome = "appended"); | ||
| println!("Added the {box_type} entry {entry} to {}", path.display()); |
There was a problem hiding this comment.
LOW · correctness · confidence 0.50 · in cmd_init
The command prints "Added the ... entry" as success, but the PR body says nothing reads [boxes.*] yet. Activation ignores the entry, and bare min init still writes the legacy [session] shape. The user sees success for a change that has no effect. Say that the entry takes effect later, or hold the append until box creation reads entries.
What the reviewer checked
println!("Added the {box_type} entry {entry} to {}", path.display()); PR body: 'Nothing in this tree reads [boxes.*] ... mfile logs them as unknown fields'
Hand this to an agent
Act on a code-review finding.
File: crates/minimal/src/cmd/project.rs
Lines: 1020-1020
Category: correctness
Severity: low (reviewer confidence 0.50)
Finding:
The command prints "Added the ... entry" as success, but the PR body says nothing reads [boxes.*] yet. Activation ignores the entry, and bare `min init` still writes the legacy [session] shape. The user sees success for a change that has no effect. Say that the entry takes effect later, or hold the append until box creation reads entries.
First verify this against the code yourself. It is model output and
may be wrong. If it is wrong, or the tradeoff was deliberate, say so
and change nothing.
👍 useful · 👎 noise — react to tell the reviewer. Replying works too, and says more.
A minimal.toml that does not parse or validate is invalid configuration. min init now refuses it with exit 3, names the file, and leaves it unchanged. An I/O error still exits 1.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The compare fixes the open point from 5cf31ba. A minimal.toml that does not parse or validate now refuses min init <entry> --type <type> with exit 3, names the file and changes nothing. That agrees with the exit-code table, line 364. An I/O error stays exit 1. BCLI-018 conforms. The BCLI-017 interim from f2efb84 is still open and filed on #2418. C1 is its flip, and it can stay open at merge.
Ruled at bef69e7 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
| /// Whether `min init` and the `min activate` scaffold write the entry shape. | ||
| /// `false` until #2418 (BOX T21): box creation still reads packages from | ||
| /// `[session]`, so an entry-shaped scaffold loses them at session start. | ||
| pub(crate) const INIT_ENTRY_SHAPE: bool = false; |
T20 — Scaffold and append entries with min init
slice: S5
kind: task
covers: BCLI-017, BCLI-018
size: L
est: 200 code, 300 tests
executor: agent
difficulty: routine
model: standard
diagnostics: when two of
--session,--task,--agent,--serviceand--typeare given,min initprints "Pass one of --session, --task, --agent, --service or --type." and changes nothing, pinned by init_noun_flags_exclusive_exit2_appends_nothingobservability: the command's existing tracing span records the verb, the box id it resolved and the outcome
layers:
- client:
- crates/minimal/src/cli.rs: the verb's arguments and help in the one command model
- crates/minimal/src/cmd/project.rs:
min initscaffolds the entry shape and appends one entry by--typeor a noun flagtest: cargo nextest run -p minimal init_scaffolds_entry_shape
test: cargo nextest run -p minimal init_entry_type_appends_one
test: cargo nextest run -p minimal init_refused_type_exit3_appends_nothing
test: cargo nextest run -p minimal init_noun_flag_is_type_alias
test: cargo nextest run -p minimal init_noun_flags_exclusive_exit2_appends_nothing
produces: none
consumes: none
proof:
min initwrites an entry-shapedminimal.toml, andmin init <entry> --type <type>appends one entry.Residuals
min initwith no flags writes the legacy[session]shape (INIT_ENTRY_SHAPE = false). Onmain, box creation does not read[defaults.<type>]yet, so an entry-shaped scaffold would lose its packages at session start. BOX T21 (T21 — One minimal.toml for every box, end to end #2418) wiresmfile::expandinto box creation. When T21 — One minimal.toml for every box, end to end #2418 lands,min initandoffer_mfile_scaffoldwrite the entry shape, and theentry_shapeparameter is removed. The owner decided this on 2026-11-06.crates/minimal/src/cmd/mod.rsis outside T20'slayers:. Arch ruling 808 C1 required the change so that anInitRefusalreachesfailure_exit_code.min initwrites the entry shape, that is superseded: at this head, baremin initwrites the legacy[session]shape (see the first residual), andinit_no_flags_writes_legacy_shapepins it.main.rsfailure_exit_code(exit 2 for usage, 3 for invalidminimal.toml), andexit_refusedis gone.Proof
min initnow writes minimal.toml in the entry shape: [defaults.session] with the packages, [defaults.task] with state_key, and a [boxes.dev] entry with type = "session".min init <entry> --type <type>appends one [boxes.] entry and keeps the rest of the file.min init --session <n>(also --task, --agent, --service) writes the same file asmin init <n> --type <noun>. If you give more than one of these flags, min init prints "Pass one of --session, --task, --agent, --service or --type.", exits 2 and changes nothing. A type that is, or extends, agent, service, build or container-build is refused with exit 3 and changes nothing.Notes from the agent: 1. Exit codes: crates/minimal/src/cmd/mod.rs turns every cmd_init error into a string, so main.rs can only give exit 1. mod.rs and main.rs are outside the layers. For this reason cmd_init prints its refusal and calls std::process::exit with the code (2 or 3) itself. The tests check the code on the InitRefusal value and do not run the binary. A later task can route this through failure_exit_code. 2. Nothing in this tree reads [boxes.*], [defaults.session] or [defaults.task] yet. mfile logs them as unknown fields. So a project scaffolded by
min initgets no session packages from minimal.toml at activation until the BOX entry expansion lands. For this reason the scaffold offer inmin activate(offer_mfile_scaffold) keeps the legacy [session] shape. run_init_flow takes a new entry_shape flag for this. 3. Choices the spec does not fix: appended entries go under [boxes.] with type. An unknown type, an entry with no type, or --type with no entry exits 2. A duplicate entry name in boxes/sessions/tasks/agents/services exits 1. A missing minimal.toml exits 1 and tells the user to run min init first. Project types resolve through the [box_types.] extends chain to a built-in root. 4. The exit-3 refusal names gominimal/inbox#678 because BOX-063 requires it. That is an issue reference, not a co…[truncated]Tests
cargo nextest run -p minimal init_scaffolds_entry_shapecargo nextest run -p minimal init_entry_type_appends_onecargo nextest run -p minimal init_refused_type_exit3_appends_nothingcargo nextest run -p minimal init_noun_flag_is_type_aliascargo nextest run -p minimal init_noun_flags_exclusive_exit2_appends_nothingVerify
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 #2893