fix(build): support unsigned macOS source builds

[esteve]

Unsigned source builds intentionally disable macOS notifications, but build.rs still unconditionally invokes /usr/bin/codesign and macos.rs unconditionally embeds its generated CodeResources.

This prevents source builds in restricted environments where ad-hoc codesigning is unavailable, even though the resulting binary will not use notifications.

Add an explicit MISE_NOTIFICATION_SIGNING=disabled mode. It:

• skips codesigning
• omits signature resources from the embedded helper
• keeps MISE_NOTIFICATION_RELEASE_SIGNED=0, so notifications remain disabled

The default ad-hoc and Developer ID signing paths are unchanged.

Summary by CodeRabbit

• New Features

  • macOS notification helpers now support both signed and unsigned builds.
  • Signing behavior is automatically selected based on the configured build mode.
  • Unsigned builds can be installed and used without signature resources.

• Bug Fixes

  • Signature resources are now included, validated, and verified only when available, improving reliability across different macOS build configurations.

[greptile-apps]

Confidence Score: 5/5

The implementation appears safe to merge, although unsigned-mode build coverage remains a worthwhile non-blocking improvement.

Findings

1.  Unsigned Mode Lacks Coverage ▶
2.  Test Bypasses Task Interface ▶

Summary

This PR adds an explicit unsigned mode for macOS notification-helper builds.

• MISE_NOTIFICATION_SIGNING=disabled skips the codesign invocation and signature-resource embedding.
• Unsigned and ad-hoc builds remain marked as not release-signed, keeping notifications disabled.
• Signing-mode selection is isolated and unit-tested while signed and unsigned bundle layouts are handled through conditional compilation.

_{Reviews (13) · Last reviewed commit: "test(build): cover helper signing mode"}

[greptile-apps]

Unsigned Mode Lacks Coverage

No macOS CI job sets MISE_NOTIFICATION_SIGNING=disabled, so the new build-script branch and the corresponding unsigned implementations in macos.rs are never compiled or tested. This is non-blocking, but a later change could restore the codesign dependency or break unsigned resource handling without CI catching it. Please add a macOS build or test invocation with this environment variable set.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[esteve]

@jdx this is done now in 3c95ec2

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: d8a3c2bc-7866-4456-8c67-c171f311b742

📥 Commits

Reviewing files that changed from the base of the PR and between 8d8abec and 735d2cd.

📒 Files selected for processing (4)

• build.rs
• build/helper_signing_mode.rs
• src/system/history/notify/macos.rs
• tests/helper_signing_mode.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/system/history/notify/macos.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The macOS notification helper now selects Disabled, AdHoc, or Release signing. Build metadata, signature resources, bundle fingerprints, completion checks, installation, and verification follow the selected mode.

Changes

macOS notification signing

Layer / File(s)	Summary
Signing mode contract build/helper_signing_mode.rs, tests/helper_signing_mode.rs	The new selector maps signing settings and identities to Disabled, AdHoc, or Release. Tests load the selector and cover the supported combinations.
Signing configuration build.rs	The build script tracks the signing environment, sets release-signing metadata, and enables signature resources and codesign for non-disabled modes.
Bundle resource validation src/system/history/notify/macos.rs	The notification bundle conditionally includes signature resources. Fingerprinting, completion checks, installation, and codesign --verify follow resource availability.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant build_rs
  participant helper_signing_mode
  participant macos_notification_helper
  build_rs->>helper_signing_mode: select signing setting and identity
  helper_signing_mode-->>build_rs: return Disabled, AdHoc, or Release
  build_rs->>macos_notification_helper: configure metadata and signature resources
  build_rs->>macos_notification_helper: run codesign unless signing is Disabled

Loading

Suggested reviewers: jdx

Merge Risk: ⚪ Minimal · up to 735d2

The disabled, ad-hoc, and release signing paths remain consistent, with no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding support for unsigned macOS source builds.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 4 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Test Bypasses Task Interface

The added CI step runs cargo test through mise x, but the repository testing guide requires testing and linting commands to run through mise run. This repository requirement must be satisfied before merging by exposing the unsigned-notification test through an appropriate task and invoking it here.

Context Used: how to test the mise codebase (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[esteve]

@jdx it seems to me the bot incorrectly detects this as an error, but the other CI tasks also use mise x -- cargo test. Let me know if this test should be a separate mise run task.

[jdx]

The additional unsigned-notification test forces a second macOS compilation. In the current CI run, that step took about 22 minutes, which is too expensive for exercising a small build-script branch and makes unit-macos substantially slower.

Please remove the additional workflow invocation. The signing-mode decision should instead be extracted into logic that can be covered by a cheap unit test in the existing test run. The existing macOS job already covers construction and execution of the notification helper; duplicating the mise build is not proportionate coverage for this change.

AI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.

[esteve]

@jdx it makes sense to remove the duplicate test, done in 3f74c84 I've extracted the logic for deciding whether to enable/disable signing and added a test in 735d2cd, though perhaps I didn't understand your feedback, but IMO it doesn't seem to me that such test makes the most sense. I've done it in separate commits so it's easier to drop 735d2cd in case you'd rather not keep that test.

[esteve]

Back to draft to resolve the conflicts with main.

[github-actions]

This PR currently has failing checks. If this continues for 7 days, it will be closed automatically.

This is warning day 1 of 7.

Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it.

This comment was generated by an automated workflow.

[github-actions]

This PR currently has failing checks. If this continues for 7 days, it will be closed automatically.

This is warning day 1 of 7.

Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it.

This comment was generated by an automated workflow.

[github-actions]

This PR currently has failing checks. If this continues for 7 days, it will be closed automatically.

This is warning day 1 of 7.

Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it.

This comment was generated by an automated workflow.

[esteve]

@jdx I've addressed your feedback, let me know if there's anything else to fix, thanks.

[jdx]

Thanks for reworking this — extracting the decision into a unit-testable selector and dropping the extra macOS build addresses my earlier review.

I'd like to take the design in a different direction, though: I don't want contributors to have to set an env var to build mise. That's the part I'm not sold on. Nothing advertises MISE_NOTIFICATION_SIGNING — it isn't in docs/contributing.md or settings.toml, and the build failure doesn't mention it — so the discovery path is "hit an opaque codesign error, go read build.rs, find the escape hatch." The build script already has enough information to handle this itself.

Detect instead of opting in

The asymmetry that makes this safe:

• Ad-hoc (identity -, the contributor default) emits MISE_NOTIFICATION_RELEASE_SIGNED=0, so notification() bails at src/system/history/notify/macos.rs:50 and the bundle is never launched. The ad-hoc seal is load-bearing for nothing at runtime in that build. The helper still executes under test, but that comes from ld's automatic ad-hoc Mach-O signature, not from /usr/bin/codesign sealing the bundle.
• Developer ID must fail hard. Silently shipping a release with notifications disabled is exactly the bug worth protecting against.

So it's error handling on a command that already runs, rather than a new mode:

let signed = codesign.arg(&app).output()?;
if !signed.status.success() {
    if identity != "-" {
        return Err(eyre!(
            "failed to sign the macOS notification helper: {}",
            String::from_utf8_lossy(&signed.stderr).trim()
        ));
    }
    println!("cargo:warning=codesign unavailable; building the notification helper unsigned (desktop notifications disabled)");
} else {
    println!("cargo:rustc-cfg=mise_notification_has_signature_resources");
}

Contributors in restricted environments just build, and nobody sets anything. MISE_NOTIFICATION_SIGNING, build/helper_signing_mode.rs, and tests/helper_signing_mode.rs all go away with it.

The cost is that a genuine codesign regression on a normal dev Mac degrades to a warning instead of an error. cargo:warning= keeps it visible, CI runners have codesign so the codesign --verify assertion in the existing test stays live there, and release builds still abort.

Releases must still fail loudly

scripts/build-tarball.sh:94 already exports the Developer ID identity unconditionally for macOS, so under the above every real release build takes the identity != "-" branch and aborts on a signing failure. That's the important half.

I'd like the weaker half closed too: if that export is ever dropped or empty, the build quietly falls back to ad-hoc and ships a release with notifications disabled. That hole exists on main today — it needs no codesign failure at all, just a missing variable — so it's arguably separate from this PR, but this is the natural place to fix it. Exporting MISE_NOTIFICATION_SIGNING=required alongside the identity in build-tarball.sh, with build.rs erroring when required doesn't end in a Developer ID signature, makes the release contract explicit instead of inferred. That also inverts the polarity here: the env var lands on the one release script that must care, not on every contributor.

Narrow the cfg

Separately, the cfg is scoped wider than it needs to be. Since notification() bails on !release_signed() and that's the only non-test caller of ensure_app, the fingerprint, complete(), the _CodeSignature mkdir, and the write are all unreachable in an unsigned build. The only thing that genuinely cannot compile without the file is include_bytes!:

#[cfg(mise_notification_has_signature_resources)]
const CODE_RESOURCES: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/mise-notify.app/Contents/_CodeSignature/CodeResources"));
#[cfg(not(mise_notification_has_signature_resources))]
const CODE_RESOURCES: &[u8] = &[];

That plus gating the codesign --verify assertion is the whole change — one branch point instead of five, and most of the macos.rs diff disappears.

Packaging bug worth knowing about regardless

build.rs declares #[path = "build/helper_signing_mode.rs"] mod helper_signing_mode; unconditionally, but the file isn't added to include in Cargo.toml — which is exactly why /build/lockfile_rollout.rs is listed there. cargo package --list on this branch ships only build.rs and build/lockfile_rollout.rs, so the published crate would be missing the module and fail at build-script compile time on every platform. Since xtasks/release-plz publishes with --no-verify, nothing would catch it before cargo install mise broke.

The direction above deletes that file, so this resolves itself — but flagging it in case any part of the module survives.

Smaller

• The docs-only commits add /// to a lot of pre-existing functions (notification_command, executable, release_signed, ensure_app, main). macos.rs on main has no doc comments on private functions, so this widens the diff without matching surrounding style. I'd drop those.
• If a build-script module ever does need tests, the existing pattern is src/main.rs:22 — #[cfg(test)] #[path = "../build/lockfile_rollout.rs"] — which runs them inside cargo test --bin mise rather than adding a test binary.

AI-assisted — Tool: Claude Code; model: Anthropic/claude-opus-5; version: 2.1.236.