fix(release): select notarization credential keychains

[steipete]

Allow macOS release automation to read notarization credentials from a configured keychain when the login keychain is locked. NOTARYTOOL_KEYCHAIN_PATH passes the selected path to notarytool --keychain for both desktop and server notarization.

Five focused signing tests pass, including a subprocess regression test for a credential-keychain path containing spaces. A Foundation-signed CLI was accepted by Apple using the explicit keychain, and its online notarized requirement was verified. Independent P0–P2 review is clean.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed October 1, 2026, 12:38 AM ET / 04:38 UTC.

ClawSweeper review

What this changes

Adds an optional credential-keychain setting for macOS desktop and server notarization, with documentation and argument-forwarding regression coverage.

Merge readiness

✅ Ready for maintainer review

This remains useful: current main and v0.6.0 lack explicit credential-keychain selection. No concrete patch defect was found, and GitHub confirms the author has repository admin access.

Priority: P2
Reviewed head: e3d47bb82f62684c01e2ccb362a5ad35d6d93eda

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, compatible implementation with useful regression coverage and no concrete blocking findings; the trusted-author proof exemption applies.
Proof confidence	🌊 off-meta tidepool	Not applicable: Verified repository-admin authorship exempts this PR from the external-contributor proof gate. The body reports explicit-keychain Apple acceptance, but does not independently demonstrate the changed helper’s execution; the subprocess test uses fake xcrun. No stored-data contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: Verified repository-admin authorship exempts this PR from the external-contributor proof gate. The body reports explicit-keychain Apple acceptance, but does not independently demonstrate the changed helper’s execution; the subprocess test uses fake xcrun. No stored-data contract changes.
Evidence reviewed	9 items	Introduced change and compatibility: The pinned delta adds an optional keychain path and passes it as a separate execFile argument. When unset, the original notarization argument list is preserved; credential-profile requirements and acceptance validation remain unchanged. Both production callers use the shared helper: Server packaging calls notarizeArchive directly, while desktop packaging reaches it through notarizeApp. Both inherit the optional environment setting; desktop also forwards an explicit options value. Current-main and release necessity check: The fetched main helper has no explicit keychain argument. The v0.6.0 desktop submission path likewise supplies only the credential profile, so the requested capability is not already present in either inspected revision.
Findings	None	None.
Security	None	None.

How this fits together

ClickClack’s macOS release scripts submit signed desktop and server archives to Apple for notarization. The shared submission helper selects credentials and checks Apple’s acceptance before artifact verification continues.

flowchart TD
  A[Signed desktop or server archive] --> C[Shared notarization helper]
  B[Credential profile and optional keychain] --> C
  C --> D[Apple notarization submission]
  D --> E{Submission accepted}
  E -->|Yes| F[Artifact verification]
  E -->|No| G[Release stops]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test LOC	production +7/-1; tests +42/-1	The small production addition supports an optional release setting, with focused argument-forwarding coverage.

Technical review

Best possible solution:

Keep explicit keychain selection optional in the shared submission helper so headless release hosts can use managed credentials while existing release commands retain their defaults.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this adds explicit credential-keychain selection. Source inspection establishes the missing setting on main; the reported Apple run was not independently reproduced.

Is this the best way to solve the issue?

Yes: extending the existing shared submission helper is the narrowest approach and preserves the current command when the setting is absent.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning medium; reviewed against bc909123584d.

Labels

Label changes:

• add P2: This is a bounded release-operator improvement with no established urgent user-facing regression.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Verified repository-admin authorship exempts this PR from the external-contributor proof gate. The body reports explicit-keychain Apple acceptance, but does not independently demonstrate the changed helper’s execution; the subprocess test uses fake xcrun. No stored-data contract changes.

Label justifications:

• P2: This is a bounded release-operator improvement with no established urgent user-facing regression.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Verified repository-admin authorship exempts this PR from the external-contributor proof gate. The body reports explicit-keychain Apple acceptance, but does not independently demonstrate the changed helper’s execution; the subprocess test uses fake xcrun. No stored-data contract changes.

Evidence

What I checked:

• Introduced change and compatibility: The pinned delta adds an optional keychain path and passes it as a separate execFile argument. When unset, the original notarization argument list is preserved; credential-profile requirements and acceptance validation remain unchanged. (apps/desktop/scripts/after-sign.mjs:66, e3d47bb82f62)
• Both production callers use the shared helper: Server packaging calls notarizeArchive directly, while desktop packaging reaches it through notarizeApp. Both inherit the optional environment setting; desktop also forwards an explicit options value. (apps/desktop/scripts/package-macos-server.mjs:93, e3d47bb82f62)
• Current-main and release necessity check: The fetched main helper has no explicit keychain argument. The v0.6.0 desktop submission path likewise supplies only the credential profile, so the requested capability is not already present in either inspected revision. (apps/desktop/scripts/after-sign.mjs:64, bc909123584d)
• Latest release inspection: The v0.6.0 notarization command does not select a credential keychain explicitly. (apps/desktop/scripts/after-sign.mjs, 31d299eee271)
• Regression coverage: The added subprocess test invokes the production helper with a fake xcrun executable and checks that a keychain path containing spaces remains one argument. This is supplemental mocked coverage; tests were inspected but not executed. (apps/desktop/scripts/macos-signing.test.mjs:69, e3d47bb82f62)
• Author role verified: The collaborator-permission endpoint reports admin permission for steipete, resolving the differing author-association values in the supplied context and confirming the trusted-author proof exemption.

Likely related people:

• Peter Steinberger: Raw commit bc90912 adds apps/desktop/scripts/after-sign.mjs:64 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bc909123584d; files: apps/desktop/scripts/after-sign.mjs)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.