Hardware validation, CI hardening and a critical arm64 crash fix for the Apple Silicon build - #1
Open
hyperbotsx wants to merge 13 commits into
Conversation
go build runs from inside the temp clone, so a relative dest (as passed by after_success.sh) would drop the binaries in the clone instead of the app bundle and fail the [ -f ] check. Absolutize dest on entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The bundled Go CLI tools were cloned at floating master, so a push to either repo would silently change what ships in the bundle. Pin to the current master tips (mc-agent 2022-12-26, mc-cli 2023-08-21 - both repos are dormant) and make the refs overridable via environment. Dependency integrity below the pins comes from each repo's go.sum. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
make_version runs git describe --tags --abbrev=0, which fails hard in the default shallow checkout (depth 1, no tags), killing the build before compilation starts. The release workflow already fetches full history; do the same here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- workflow_dispatch so any branch can be built on demand - widen the push trigger to feature/apple-silicon-* branches - build with --package so every run produces a complete bundle (daemon, CLI tools, Qt frameworks via macdeployqt) - upload the zipped .app so PR reviewers can download and try a build without cutting a release Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The verify steps only printed file/lipo output, so an accidental x86_64 binary would pass CI unnoticed. Fail the run unless moolticute, moolticuted, mc-agent and mc-cli are all exactly arm64. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Floating 'brew install qt' drifts to whatever Qt is current (untested against this codebase) and its single-arch bottles cannot produce a universal binary later. CI now installs the official Qt 6.2.4 archives (exactly what the upstream Travis pipeline builds against; universal x86_64+arm64) via aqtinstall, with qtwebsockets as the only addon module needed. Homebrew Qt remains fully supported for local builds through detect_qtdir, which now also checks the qt@6 keg paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces floating 'brew install go' so the compiler that builds the bundled mc-agent/mc-cli is deterministic across runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ad-hoc signing step ended in '2>/dev/null || true', so a signing failure would silently ship a DMG whose app macOS kills at launch (arm64 requires at least an ad-hoc signature). Sign moolticuted and the CLI tools explicitly before the outer bundle, drop the error suppression, and verify the result with codesign --verify --strict. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hdiutil intermittently fails with 'Resource busy' on GitHub-hosted macOS runners; retry up to 3 times before giving up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The template Info.plist shipped without CFBundleShortVersionString, CFBundleVersion or LSMinimumSystemVersion, so the packaged app showed no version in Finder/Get Info and made no OS requirement claim. Stamp the git-derived version at package time and declare macOS 11.0 (the first arm64 release) as the minimum. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FilesCache::setCardCPZ and MPDevice::getUInt64EncryptionKeyOld derive a SimpleCrypt key by shifting a 32-bit value by (i*8) for i in 0..7. A shift >= 32 is undefined behaviour: x86 silently wrapped the count, but for arm64 clang compiles the provably-undefined iterations into a brk trap, so the daemon crashes (EXC_BREAKPOINT) the moment a card CPZ is read - i.e. on every device connect/unlock on Apple Silicon. Verified on hardware (Mooltipass Mini BLE, macOS 26.5.2, Mac14,6): deterministic SIGTRAP at FilesCache::setCardCPZ+1376 in every crash report, and the shipped binary contains brk mooltipass#1 (0xd4200020) at exactly that offset. Mask the shift count ((i*8) & 31) to make the historical x86 wrapping explicit and well-defined: derived keys stay byte-identical with existing Intel installs, so cached/exported data continues to decrypt. The V2 path (getUInt64EncryptionKey) already shifts in 64-bit and is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
IOKit invokes the input-report callback with a non-success result (and meaningless buffer/length) when the device drops mid-read, such as during BLE device interface re-enumeration. The callback ignored the result code and parsed whatever was in the buffer. Drop such reports instead of feeding them to the message protocol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Real-device testing on Apple Silicon surfaced two upgrade traps: a stale shared-memory daemon record from the previous install blocks the new daemon from starting, and the auto-updater offers the Intel v1.04.0 DMG to native builds. Document both with workarounds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 25, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df5894c5b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+63
to
+64
| BUNDLE_VERSION="$(get_version "$REPO_ROOT" 2>/dev/null || echo v0.0.0)" | ||
| BUNDLE_VERSION="${BUNDLE_VERSION#v}" |
There was a problem hiding this comment.
When macos-release.yml is started via workflow_dispatch, the requested version is passed only to package-release.sh, while this new metadata stamping still reads the nearest existing Git tag. For example, dispatching v1.05.0-arm64 from a commit whose latest tag is v1.04.0 publishes v1.05-named assets containing an app whose CFBundleShortVersionString and CFBundleVersion both report 1.04.0. Pass the workflow's release version into the packaging/stamping step so the bundle and published release agree.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of all — thank you for mooltipass#1255, it was an excellent base. I took your branch, hardened the pipeline, and validated the result end-to-end on real hardware (Mooltipass Mini BLE, macOS 26.5.2, M-series Mac). This PR contributes those changes back so they can be reviewed together with your work. Everything is small, separable commits — feel free to cherry-pick.
The critical finding: native arm64 builds crash on card read
Testing with a real device immediately hit a daemon crash-loop on every connect/unlock:
EXC_BREAKPOINT (SIGTRAP)atFilesCache::setCardCPZ+1376, reached from the IOHID input-report path viaMPDevice::getCurrentCardCPZ().Root cause (
aae9bf58):FilesCache::setCardCPZandMPDevice::getUInt64EncryptionKeyOldderive a SimpleCrypt key with(32-bit value) << (i*8)for i = 0..7. Shifting a 32-bit type by ≥ 32 is undefined behaviour — x86 silently wraps the shift count, but clang for arm64 compiles the provably-undefined iterations into abrktrap. I confirmed the shipped binary literally containsbrk #1(0xd4200020) at the crash offset. So every native Apple Silicon build of master crashes the moment a card CPZ is read — unit tests can't see it (none exercise that path).The fix masks the shift count (
(i*8) & 31), making the historical x86 wrapping explicit and well-defined. Derived keys stay byte-identical with existing Intel installs, so caches and SimpleCrypt-encrypted exports keep decrypting. (TheSIMPLE_CRYPT_V2path already shifts in 64-bit and is untouched.) After this fix, unlock and all device operations work — verified on hardware.A companion hardening (
4454a2ab): the macOS HID input-report callback ignored the IOReturn result, parsing garbage when a device drops mid-read (e.g. BLE interface re-enumeration). Errored reports are now dropped.Pipeline hardening (on top of your workflows/scripts)
143ba5e4fix relative-dest bug inbuild_mc_cli_tools(go build ran from inside the clone, so after_success.sh's relative path would misplace the binaries)02fa9263pin mc-agent/mc-cli to known-good revisions instead of floating master (reproducibility/supply chain; both repos are dormant)c3fe6d67fetch-depth: 0in macos-build.yml —make_versionneedsgit describe --tags, which dies in a shallow clone9f443ec9workflow_dispatch,feature/apple-silicon-*branch trigger, build with--package, upload an installable .app artifact from every build run0c47c67fCI now asserts all four executables (moolticute, moolticuted, mc-agent, mc-cli) are exactlyarm64instead of just printinglipooutputc4a35ca8workflows install pinned official Qt 6.2.4 via aqtinstall (same version upstream Travis uses; official archives are universal, which keeps a future universal-binary build one variable away). Homebrew Qt remains fully supported for local builds;detect_qtdiralso checks theqt@6keg paths nowa15ea400pin Go via actions/setup-go58153ebfcodesign: fail loudly instead of2>/dev/null || true(a silent failure would ship a DMG that arm64 macOS refuses to launch), sign nested binaries explicitly, verify withcodesign --verify --strict0b52b2a2retryhdiutil create(intermittent "Resource busy" on GH runners)a9cf5cc3stampCFBundleShortVersionString/CFBundleVersionat package time and declareLSMinimumSystemVersion11.0df5894c5document two upgrade traps found in testing (stale shared-memory daemon record from a previous install; auto-updater offering the Intel v1.04.0 DMG to native builds)Hardware validation results (macOS 26.5.2, Mac14,6, Mooltipass Mini BLE)
All over USB, on the build produced by these workflows:
file/lipo/Activity Monitor "Apple") for all 4 binariesAlso verified: the codebase builds green (incl. all tests) against Qt 6.8 LTS with zero code changes — available as an optional bump if wanted, since 6.2.4 is EOL.
Known remaining items (maintainer-side / follow-ups)
CI runs: build https://github.com/hyperbotsx/moolticute/actions/runs/30150368234 · release https://github.com/hyperbotsx/moolticute/actions/runs/30153970614
🤖 Generated with Claude Code