Skip to content

🤖 Fix keychain race condition in parallel macOS signing #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Oct 13, 2025
Merged

🤖 Fix keychain race condition in parallel macOS signing #234

merged 6 commits into from
Oct 13, 2025

Conversation

@ammar-agent
Copy link
Collaborator

@ammar-agent ammar-agent commented Oct 13, 2025

Problem

PR #227 introduced parallel builds for macOS (x64 + arm64), which caused a race condition during code signing:

SecKeychainCreate: A keychain with the same name already exists.
Exit code: 48

Both electron-builder processes try to create the same keychain simultaneously.

Solution

Pre-create and configure the keychain in setup-macos-signing.sh before running parallel builds.

Changes

  1. Create unique keychain with timestamp to avoid conflicts
  2. Import certificate before parallel builds start
  3. Configure keychain permissions for codesign access
  4. Export CSC_KEYCHAIN so electron-builder uses the pre-configured keychain

Flow

Before (broken):

setup-macos-signing.sh → exports CSC_LINK
  ↓
parallel: electron-builder x64 → tries to create keychain ❌
parallel: electron-builder arm64 → tries to create keychain ❌
  → RACE CONDITION

After (fixed):

setup-macos-signing.sh → creates keychain + imports cert
  ↓
parallel: electron-builder x64 → uses existing keychain ✅
parallel: electron-builder arm64 → uses existing keychain ✅
  → NO CONFLICT

Testing

This will be tested on the next release. The PR workflow doesn't test signing (no secrets), so we can't verify in CI.

Impact

Generated with cmux

@ammar-agent
🤖 Fix keychain race condition in parallel macOS signing
eba94fe 
Problem:
When running parallel electron-builder processes (x64 + arm64),
both try to create the same keychain simultaneously, causing:
  SecKeychainCreate: A keychain with the same name already exists

Solution:
Pre-create and configure keychain in setup-macos-signing.sh before
running parallel builds. This ensures:
- Certificate is imported before electron-builder runs
- Both parallel processes use the same pre-configured keychain
- No race condition on keychain creation

Changes:
- Create unique keychain with timestamp
- Import certificate into keychain before parallel builds
- Configure keychain permissions for codesign
- Export CSC_KEYCHAIN for electron-builder
- Add verification step to confirm signing is enabled
@ammar-agent ammar-agent force-pushed the fix-parallel-signing-race branch from e8b1965 to eba94fe Compare October 13, 2025 20:48
@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Oct 13, 2025

💡 Codex Review

cmux/Makefile

Lines 165 to 169 in 187855e

dist-mac: build ## Build macOS distributables (x64 + arm64 in parallel)
@echo "Building macOS architectures in parallel..."
@bun x electron-builder --mac --x64 --publish never & \
bun x electron-builder --mac --arm64 --publish never & \
wait

P1 Badge Parallel macOS builds drop exit codes

The new dist-mac recipe spawns x64 and arm64 builders in the background and calls wait without tracking individual PIDs. wait with no args returns the status of only the last job that finishes, so if one architecture fails quickly while the other succeeds later the Make target (and release workflow) will still exit 0 and print "Both architectures built successfully" even though one build failed, potentially publishing incomplete artifacts. Consider capturing each PID and failing the recipe if any wait returns non‑zero; the same pattern appears in dist-mac-release below.

ℹ️ 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 👍.

@ammar-agent
Fix shell script formatting
2f44a51
@ammar-agent
Enable code signing for PR builds to test keychain fix
1753a16
@ammar-agent
Only enable signing for PR #234 to prevent accidental merge
b88ca84
@ammar-agent
Fix parallel build exit code handling
ea27db4 
Codex identified that 'wait' without args only returns the exit code
of the last job. Now we:
- Capture PIDs for both background jobs
- Wait on each PID individually with && to fail if either fails
- Ensure both architectures succeed before printing success message

This fixes both dist-mac and dist-mac-release targets.
@ammar-agent
Copy link
Collaborator Author

ammar-agent commented Oct 13, 2025

@codex review

@ammar-agent
Fix keychain race: sequential builds when signing enabled
6c2a715 
electron-builder's internal keychain creation conflicts when run in
parallel, even with pre-created keychains. Simplest fix:
- Detect if CSC_LINK is set (signing enabled)
- If yes: build sequentially to avoid keychain conflicts
- If no: build in parallel for speed

This guarantees no race conditions while preserving parallelism for
unsigned builds (PR tests, local dev).
@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Oct 13, 2025

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ 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 👍.

@ammario ammario enabled auto-merge October 13, 2025 21:03
@ammario ammario added this pull request to the merge queue Oct 13, 2025
Merged via the queue into main with commit d6d8368 Oct 13, 2025
6 of 7 checks passed
@ammario ammario deleted the fix-parallel-signing-race branch October 13, 2025 21:17
@ammar-agent ammar-agent mentioned this pull request Oct 14, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ammar-agent @ammario