Skip to content

fix(macOS): restore native toolbar bubbles in release builds - #1742

Merged
kshivang merged 1 commit into
mainfrom
fix/macos-release-toolbar-appearance
Sep 27, 2026
Merged

kshivang merged 1 commit into
mainfrom
fix/macos-release-toolbar-appearance

Conversation

@kshivang

Copy link
Copy Markdown
Contributor

Description

The released BOSS launcher declares macOS SDK 14.2, while the local development Java launcher declares SDK 26.5. AppKit therefore gives the packaged app older toolbar styling even though both paths create the same bordered native toolbar items. This explains the missing icon bubbles in release.

Port BossTerm's launcher preparation step: use Apple's vtool to opt each older Mach-O slice into SDK 26.0, preserving its deployment target and executable instructions, then re-sign the bundle. Run this after native trimming, PTY signing and CLI extraction, before DMG packaging. Windows/Linux packaging is unchanged.

Apple describes SDK adoption and native toolbar styling in Build an AppKit app with the new design. This adapts the existing BossTerm approach to jpackage's prebuilt launcher rather than changing the Java runtime or copying debug preferences.

Validation

  • Applied to a temporary copy of the actual released BOSS.app: SDK 14.2 → 26.0, minimum macOS remains 11.0; strict deep signature verification passed. Installed app untouched.
  • Ran the Gradle preparation task against a copied release bundle with ad-hoc signing, including successful configuration-cache store/reuse and repeat invocation.
  • Four regression tests pass: universal slice handling/preservation of newer SDKs and per-slice deployment targets, executable permission preservation, signing options, and read-only release verification.
  • ktlint passed; packageDmg dry-run confirms preparation runs after all bundle modifications and before packaging.
  • CI now runs the script tests. Release CI verifies SDK metadata and bundle signatures before publication.
  • Full DMG creation, notarization, and interactive appearance/older-macOS smoke tests were not run locally. SDK opt-in affects AppKit's linked-SDK behavior, not just one toolbar item.

Type and version impact

  • Bug fix and macOS packaging configuration; patch impact managed by release automation.

Separate sidebar resize follow-up: #1741. This PR has no dependency on that change.

@supabase

supabase Bot commented Sep 26, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project pcnwqamqdnsadranufjv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@github-actions

Copy link
Copy Markdown
Contributor

Claude diff review of 8749703 (no tests executed).

Review: PR #1742 @ 8749703

Scope: 2 workflow edits, 1 Gradle task, 1 new Python script, 1 new test file. Review is diff-only; I could not verify surrounding Gradle code, the rest of the release workflow, or actual vtool/codesign behaviour on a real bundle.


Confirmed defects / inconsistencies

1. scripts/prepare-macos-appearance.py:71-72 — comment contradicts the ordering this PR itself establishes

# Signing the bundle signs its main executable and rebuilds the resource seal.
# Nested libraries are unchanged; the existing native-signing finalizer follows.

But composeApp/build.gradle.kts explicitly forces this task to run after the native signer:

mustRunAfter("createDistributable", "stripForeignPlatformNatives", "signPty4jBinaries", "extractCLIToAppResources")

The finalizer precedes, it does not follow. The chosen order is the correct one (re-sealing after nested signing), but the stale comment invites a future maintainer to reorder and silently break the seal. Fix the comment.

2. prepare-macos-appearance.py:16 vs :60 — target SDK is duplicated as a literal

MIN_SDK = (26, 0)
...
run("xcrun", "vtool", "-arch", arch, "-set-build-version",
    "macos", minimum, "26.0", "-replace", ...)

Bumping MIN_SDK to e.g. (27, 0) would leave the writer stamping 26.0, and the immediately following check version(actual_sdk) < MIN_SDK (line 63) would then fail every build. Derive the literal from MIN_SDK.

3. prepare-macos-appearance.py:51 vs :74 — asymmetric verification strictness
The mutate path validates with codesign --verify --strict, the release gate validates with codesign --verify --deep --strict. A bundle that passes the build-time check can still fail the release-time check (nested code signed with a different/absent identity, or unsigned Mach-O added by extractCLIToAppResources). That failure surfaces only in release.yml, i.e. at the most expensive point. Use the same flags in both paths.

4. prepare-macos-appearance.py:23-24, 48 — version("26") < (26, 0) is True
vtool printing an SDK as 26 (no minor) is treated as below target, so an already-conforming slice gets rewritten and re-signed. Harmless but wasteful/confusing; normalize to a fixed-width tuple.

5. .github/workflows/release.yml:631-636 — the gate validates the staging app image, not the shipped artifact
It inspects composeApp/build/compose/binaries/main/app/BOSS.app, not the DMG payload produced by the preceding step. If the DMG were ever produced from a different app image (matrix variant, custom --dest, a packageReleaseDmg path), the gate would pass while the shipped DMG lacks the SDK bump. Verifying the mounted/extracted DMG would close this.

6. prepare-macos-appearance.py:37-38 — unguarded plist access
plistlib.load(stream)["CFBundleExecutable"] raises a bare KeyError/FileNotFoundError traceback rather than an actionable message. Minor, but this runs in CI where the message is the only diagnostic.

Positive note: the Path(executable).name != executable guard (lines 39-40) correctly blocks path traversal via a malicious CFBundleExecutable, and all subprocess calls use argv lists (no shell), so the identity/path values are not injectable.


Uncertain observations (need verification against code not in the diff)

7. Only the non-release Compose task variants are wired. The diff hooks createDistributable and packageDmg (build.gradle.kts, finalizedBy / mustRunAfter blocks). Compose Desktop also exposes createReleaseDistributable / packageReleaseDmg. If the release workflow uses the Release variants, prepareMacOSAppearance never runs and the new release gate fails the pipeline. Please confirm which task the macOS DMG step invokes.

8. build.gradle.kts — mustRunAfter(...) configured unconditionally.

tasks.named("prepareMacOSAppearance") {
    mustRunAfter("createDistributable", "stripForeignPlatformNatives", "signPty4jBinaries", "extractCLIToAppResources")
}

Unlike the adjacent packageDmg block, this has no if (isMacOS) guard and uses hard task-name strings rather than findByName. If any of those tasks are registered only on macOS, a non-macOS build that realizes and schedules this task can hit UnknownTaskException. Risk looks low (the task is only scheduled via the mac-only finalizedBy), but the asymmetry with the existing guarded block is worth resolving.

9. developerId type. if (signingDisabled.get()) "-" else developerId — if macOSDeveloperId is a Provider/nullable rather than a String, commandLine will stringify the wrapper or pass null/empty to codesign --sign, producing a confusing signing failure. Cannot confirm from the diff.

10. Release gate is unconditional. The new step in release.yml has no if:; if there is a signing-disabled release path, codesign --verify --deep --strict on an unsigned bundle will fail the job. The Gradle side explicitly handles the signing-disabled case ("-"), the workflow side does not.

11. vtool -arch <a> -set-build-version ... -output on a fat binary. The loop re-reads staged for the next arch (line 62), which assumes vtool writes back all slices. If it emits only the selected slice, the next build_versions call errors out — loud, not silent, so not a shipping risk, but it means the universal-binary path has never been exercised by anything in this PR (see gap 12).

12. --entitlements is required=True but unused in --verify-only mode (lines 46-52, 85). The release step is forced to pass a path that is only resolve(strict=True)-checked. Cosmetic, but it couples the gate to a file it doesn't read.


Test gaps

scripts/test/test-prepare-macos-appearance.py patches appearance.run wholesale, so no real lipo/vtool/codesign invocation is exercised anywhere in CI — the only real execution is the release job, where failure is late and expensive. Specific uncovered behaviour:

  • Argument syntax of the vtool call. The mock parses positionally (args.index('-set-build-version'), +2, +3, args[-1]), so it would accept a wrong flag order, a missing -replace, or a swapped minos/sdk pair. The one thing the script must get exactly right is untested.
  • The traversal guard (lines 39-40). A security-relevant check with zero coverage; no test feeds a CFBundleExecutable of ../../evil or "".
  • build_versions regex (lines 29-33). The fixture emits only the minos/sdk shape. The version … (LC_VERSION_MIN_MACOSX) alternative in the regex and the minimum is None or sdk is None error branch are both uncovered.
  • Empty-arch branch (lines 43-44) — no test where lipo returns nothing.
  • Post-sign re-verification loop (lines 75-78) — the mock can never change minos after signing, so the "signing corrupted the launcher" detection is dead code under test.
  • Failure propagation — no test where codesign exits non-zero (CalledProcessError), i.e. nothing asserts the script fails the build rather than swallowing it.
  • Fat-slice preservation — the mock's shutil.copyfile trivially preserves everything, so gap 11 above is structurally untestable with the current harness.
  • Gradle wiring — nothing asserts prepareMacOSAppearance is ordered after signPty4jBinaries/extractCLIToAppResources; given defect 1's misleading comment, a TaskExecutionGraph assertion would be cheap insurance.

Also worth noting: .github/workflows/build.yml:53-54 adds the test step with no if: / OS qualifier. That's fine today because everything is mocked, but it means the new script is never smoke-tested against a real Mach-O before a release run.

@kshivang
kshivang merged commit 95a9595 into main Sep 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant