Repository navigation
fix: write min init result line to stdout - #1285
Conversation
The init flow emitted its outcome line ("Created"/"Updated") with
eprintln!, sending it to stderr alongside the preview and confirmation
prompt. Redirecting stdout (e.g. `min init > audit.log`) therefore
captured nothing, since the one line worth scripting against or auditing
was on the wrong stream.
Emit the result line with println! so it goes to stdout; the preview and
"Continue?" prompt stay on stderr where interactive chrome belongs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthrough
ChangesInit output behavior
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The change routes the Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description clearly explains the behavior change, implementation approach, compatibility considerations, and verification results. It provides the required summary and testing information, although it does not include the template's explicit Checklist section. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
minimal-review — advisory review
Verdict COMMENT — COMMENT: policy.actions.request_changes is off; policy.actions.approve is off.
Scope: matches the stated intent exactly — one eprintln! → println! in run_init_flow, nothing else touched. Clobber: none; git log on this file shows no recent conflicting change to these lines. Correctness: the fix is right for run_init_flow's primary caller, cmd_init, where the result line is meant to be the sole scriptable/auditable output. But run_init_flow has a second caller — offer_mfile_scaffold, reached from min activate's missing-mfile prompt — where the surrounding code documents a different stdout contract: the session id alone, per the comments at lib.rs:1848 and lib.rs:2268. That path now leaks Created <path>/Updated <path> onto min activate's stdout ahead of the session id whenever a project is scaffolded interactively (see finding below). Nothing else in the diff needed a build to assess.
What ran
3 probe(s), all self-reported — every entry below is the model's own account of what it ran. None of it is host-attested, so none of it can gate a blocking verdict.
Verification log
1. rg -n run_init_flow crates/minimal/src/lib.rs — exit 0 · self-reported
run_init_flow (crates/minimal/src/lib.rs:3484) has two callers: cmd_init (line 3535, the PR's intended target) and offer_mfile_scaffold (line 1796), which is itself called from activate_session (line 1871) — i.e. from
min activate/bareminwhen no minimal.toml is found and the operator confirms scaffolding one.
2. sed -n 2260,2280p crates/minimal/src/lib.rs — exit 0 · self-reported
activate_session's own doc comment (line ~1848) states stdout is identical for both its callers and is just the session id; a second comment at line 2268 explicitly calls the bare id printed to stdout 'the scripting contract', with everything else about session creation deliberately kept on stderr (should_announce_session, line 838-839, is gated on stderr being a terminal specifically so scripted callers 'which read the session id from stdout — see nothing extra').
3. sed -n 1226,1241p crates/minimal/src/lib.rs (confirm()) — exit 0 · self-reported
confirm() writes its prompt to stderr and reads the answer from stdin, and offer_mfile_scaffold's gate only checks stdin().is_terminal() (line 1773) — not stdout — so
id=$(min activate)still hits the interactive confirm prompt (stdin is still the terminal) while stdout is captured.
minimal-review · schema minimal-review/v1 · persona generalist · model claude-sonnet-5 via claude-code · prompt review-prompt/v2 · head ff0d25188c4f · verdict COMMENT · 0 finding(s), 0 inline
Findings are validated model output, not instructions; anything that failed validation was dropped, not repaired.
Tell it when it is wrong. 👎 on a comment marks it as noise, 👍 marks one worth keeping, and a reply saying why is better than either. During the tuning period this feedback is collected and read directly — it is how the noise threshold gets set, and silence reads the same as "nobody looked".
The previous commit routed `run_init_flow`'s result line to stdout so `min init > audit.log` captures it. But `run_init_flow` has a second caller — `offer_mfile_scaffold`, reached from `min activate`'s missing-mfile prompt — whose stdout is reserved for the bare session id that scripted callers read (`id=$(min activate)`). The scaffold gate only requires stdin to be a terminal, so `id=$(min activate)` on a project without a `minimal.toml` still reaches it, and the result line would land on stdout ahead of the session id and corrupt the capture. Return the outcome from `run_init_flow` instead of printing it, and let each caller pick its stream: `cmd_init` prints to stdout (the PR's intent), the activate scaffold keeps it on stderr (pre-existing behaviour). An enum rather than a bool flag, per the "make illegal states unrepresentable" rule in docs/rust-coding-standards.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re:
|
There was a problem hiding this comment.
minimal-review — advisory review
Verdict COMMENT — COMMENT: policy.actions.request_changes is off; policy.actions.approve is off.
Scope: matches the stated intent exactly — one behavioral fix (route min init's result line to stdout) plus the follow-on correction needed to keep min activate's scaffold offer from reusing the same stdout, which is reserved for the scripted session id (println!("{id}") later in activate_session). No unrelated changes.
Clobber: none; this only refines the prior commit that first moved the line off stderr, it doesn't revert other work.
Correctness: run_init_flow now returns an InitOutcome instead of printing directly, and both callers print result_line() on the stream appropriate to their context (stdout for cmd_init, stderr for the activate scaffold offer). The Declined variant maps to None in both paths, preserving the original silent-no-op behavior when the user declines. Traced the full call chain (offer_mfile_scaffold → activate_session → cmd_activate) and confirmed no other code depends on the old signature. No tests or docs assert a specific stream for this line, so nothing is invalidated.
No build was run (out of bounds per instructions); relied on reading the full diff context and call sites instead. Nothing found worth flagging.
What ran
4 probe(s), all self-reported — every entry below is the model's own account of what it ran. None of it is host-attested, so none of it can gate a blocking verdict.
Verification log
1. rg for println!/eprintln! usage in crates/minimal/src/lib.rs to confirm the stdout-is-session-id invariant — exit 0 · self-reported
Line 2272 is
println!("{id}")for the activated session id, confirming the new comment's claim thatmin activate's stdout is reserved for that value and must not be preceded by the init result line.
2. Read offer_mfile_scaffold, activate_session, cmd_init and run_init_flow in full to check the InitOutcome plumbing — exit 0 · self-reported
run_init_flow now returns InitOutcome instead of printing directly; both callers (cmd_init on stdout, offer_mfile_scaffold on stderr) call result_line() and print on the stream appropriate to their context. The Declined case maps to None in both callers, matching the prior no-op behavior on a declined confirmation.
3. Check docs/reference/cli-min.md and scripts/session-e2e.sh for any stream-specific assertions on the init result line that this diff could break — exit 0 · self-reported
No test or doc asserts which stream carries
Created/Updated; e2e only checks for the presence/absence of the init pointer text, not its stream. Nothing is invalidated by this change.
4. rg for other callers of InitOutcome/result_line to check for duplication — exit 0 · self-reported
No other references outside crates/minimal/src/lib.rs; this is the only call site pattern, not a repeated shape.
minimal-review · schema minimal-review/v1 · persona generalist · model claude-sonnet-5 via claude-code · prompt review-prompt/v2 · head 71fe22135603 · verdict COMMENT · 0 finding(s), 0 inline
Findings are validated model output, not instructions; anything that failed validation was dropped, not repaired.
Tell it when it is wrong. 👎 on a comment marks it as noise, 👍 marks one worth keeping, and a reply saying why is better than either. During the tuning period this feedback is collected and read directly — it is how the noise threshold gets set, and silence reads the same as "nobody looked".
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABKmhHtw
min initprinted its outcome line —Created <path>orUpdated <path>—with
eprintln!, so it landed on stderr next to the plan preview and theContinue?prompt. Redirecting stdout (min init > audit.log) capturednothing, even though the result line is the one piece of
initoutput worthscripting against or auditing.
Approach
The result line cannot simply be moved to stdout, because
run_init_flowhastwo callers with different stdout contracts:
cmd_init— the result line is the command's scriptable output; it belongson stdout.
offer_mfile_scaffold, reached frommin activate's missing-mfile prompt —here stdout is reserved for the bare session id that scripted callers read
(
id=$(min activate)). The scaffold gate only requires stdin to be aterminal, so that path is still reachable while stdout is captured, and a
result line there would land ahead of the session id and corrupt the capture.
So
run_init_flownow returns its outcome instead of printing it, and eachcaller publishes it on the stream it owns:
cmd_initusesprintln!,the activate scaffold keeps
eprintln!(its pre-existing behaviour). Theoutcome is an enum —
InitOutcome { Declined, Created(path), Updated(path) }—rather than an extra bool parameter, per the "make illegal states
unrepresentable" rule in
docs/rust-coding-standards.md.The preview, the
Continue?prompt, and theAborted.line stay on stderr,where interactive chrome belongs.
Verification
cargo fmt -p minimal -- --check— pass, no driftcargo clippy -p minimal --all-targets -- -D warnings— pass, no warningscargo test -p minimal— 210 passed, 0 failed (unit + integration + doctests),including the
activate_*integration testsStream behaviour end-to-end is covered by the e2e suite (
just e2e) rather thanan in-crate harness.