Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| pub fn status(code: i32) -> std::process::ExitCode { | ||
| debug!("exiting with code: {code}"); | ||
| std::process::exit(code) | ||
| std::process::ExitCode::from(code as u8) |
There was a problem hiding this comment.
When a Windows child returns a status outside 0..=255, casting the requested i32 status to u8 changes it before returning it to the OS, causing values such as 256 to be reported as success instead of preserving the child process failure.
Knowledge Base Used: CLI Dispatch Flow
Greptile SummaryThis refactor removes targeted Clippy exceptions while preserving command cleanup through returned exit codes.
Confidence Score: 4/5This PR is not yet safe to merge because both the main executable and standalone shim can misreport native Windows failure statuses. The main exit helper still casts requested i32 statuses to u8, and the newly refactored shim independently applies the same narrowing to its child status; values such as 256 therefore become successful status 0. Files Needing Attention: src/exit.rs and crates/mise-shim/src/main.rs Important Files Changed
Reviews (2): Last reviewed commit: "refactor: remove targeted clippy excepti..." | Re-trigger Greptile |
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
69471d6 to
479c2ea
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 479c2ea. Configure here.
| pub fn status(code: i32) -> std::process::ExitCode { | ||
| debug!("exiting with code: {code}"); | ||
| std::process::exit(code) | ||
| std::process::ExitCode::from(code as u8) |
There was a problem hiding this comment.
Windows child exit codes truncated
Medium Severity
Replacing std::process::exit with ExitCode::from(code as u8) narrows propagated statuses to eight bits. On Windows, std::process::exit previously forwarded the full i32 from mise exec and shims; values above 255 (or wrapped negatives) can now report the wrong process status, including mapping some failures to 0.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 479c2ea. Configure here.
| }); | ||
| fn main() -> ExitCode { | ||
| match run() { | ||
| Ok(code) => ExitCode::from(code as u8), |
There was a problem hiding this comment.
When a Windows tool launched through an executable mise shim returns a status outside 0..=255, casting the status to u8 changes it before forwarding it. A status of 256 therefore becomes 0, causing the failed shimmed command to be reported as successful.
Knowledge Base Used: Shims
## Summary - group aqua minisign inputs in a typed verification context - group GitHub attestation inputs in a public request type - encapsulate Homebrew dependency traversal state - remove four backend and sigstore `clippy::too_many_arguments` exclusions ## Validation - `cargo clippy --workspace --all-targets --all-features -- -D warnings` - `cargo test -p mise-sigstore` (26 passed) - `cargo test --all-features backend::aqua::tests` (52 passed) - `cargo test --all-features system::packages::brew` (139 passed) This is a prerequisite refactor in stack #11523; it targets #11524. *AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Mechanical signature refactors with existing test coverage; no intended changes to verification or install resolution logic. > > **Overview** > Refactors several **long-argument** call sites into typed context structs so Clippy’s `too_many_arguments` allow-list can be dropped without changing behavior. > > **mise-sigstore** introduces public **`GithubAttestationRequest`** and routes GitHub attestation verification (including the base-URL + digest path) through it; **`src/github/sigstore.rs`** is updated to build that struct at the digest-aware call site. > > **Aqua** minisign verification now takes a **`MinisignCheck`** bundle (artifact, package, config, optional checksum, download dir, progress) instead of eight separate parameters at each **`run_minisign_check`** call. > > **Homebrew formula resolution** replaces the recursive **`visit`** helper’s many parameters with a **`VisitContext`** that holds traversal state for dependency sorting. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1dd6171. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->


Summary
ExitCodeinstead of calling the disallowedstd::process::exitawaitmise-shimwarningsValidation
mise run lintcargo clippy --workspace --all-targets --all-features -- -D warningscargo test -p mise-shim -p vfox --all-features(91 passed, 2 ignored)cargo test --all-features test_manager_repairs_linked_record_without_repouringThis is a prerequisite refactor in stack #11523; it targets #11522.
AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Low Risk
Behavioral change is mainly exit path ordering (destructors run before exit); exit codes and shell activation semantics are preserved.
Overview
This PR clears several Clippy suppressions by adjusting how binaries exit, renaming a few enums, and tightening test synchronization.
Process exit:
mise,mise-shim, andvfoxno longer callstd::process::exit. They returnExitCodefrommain(withexit::statusreplacingexit::terminate), so teardown can run before the process ends.Naming:
ActivatePreludevariants drop theEnvsuffix (Set,Prepend,MovePrepend), and the completion CLI shell enum usesPowershellinstead ofPowerShell, removingenum_variant_namesallowances.Other: Seccomp rule keys use a small
syscall_numberhelper instead of a conversion lint allow. Brew pour tests switch fromstd::sync::Mutextotokio::sync::Mutexwithblocking_lock/.awaitso the async test can hold the env lock acrossawaitwithout a clippy exception.Reviewed by Cursor Bugbot for commit 479c2ea. Bugbot is set up for automated code reviews on this repo. Configure here.