Embed TeamIdentifier in macOS CodeDirectory - #392
Merged
Conversation
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
![cubic-dev-ai[bot]](https://cdn.github.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/release.yml">
<violation number="1" location=".github/workflows/release.yml:339">
P2: The notarization verification step only warns on failure, so the workflow can pass without notarization being accepted.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
This PR updates the macOS release pipeline to ensure signed Mach-O binaries include a populated TeamIdentifier in the CodeDirectory by switching from GoReleaser’s embedded goreleaser/quill fork to direct anchore/quill CLI usage.
Changes:
- Add a GoReleaser build hook to sign darwin binaries via
scripts/sign-darwin.shbefore archiving. - Disable GoReleaser’s built-in macOS notarization and run
quill notarizeas a separate post-publish workflow step. - Add a post-release
macos-verifyjob and document signing/notarization tradeoffs inRELEASING.md.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
scripts/sign-darwin.sh |
New GoReleaser build hook script to sign darwin binaries using quill sign. |
RELEASING.md |
Documents the updated macOS signing/notarization flow and tradeoffs. |
.goreleaser.yaml |
Adds the signing hook and disables GoReleaser notarization. |
.github/workflows/release.yml |
Installs quill, prepares signing creds, notarizes post-publish, and adds a macOS verification job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://cdn.github.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: a0b99db3bd
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremy
force-pushed
the
macos-codesign-teamidentifier
branch
from
March 27, 2026 20:12
43fc75d to
8738e15
Compare
The goreleaser/quill fork (embedded in GoReleaser v2.14.x) never populates the TeamIdentifier field in the CodeDirectory during signing (anchore/quill#147). Replace the built-in notarize block with direct anchore/quill v0.7.1 CLI calls: - Build hook (scripts/sign-darwin.sh) signs darwin binaries before archiving so archives, checksums, and tap manifests are correct. Fails closed in CI when QUILL_SIGN_P12 is set; skips in local dev. - Notarization runs as a separate workflow step after GoReleaser publishes using --notary-* flags. Password via QUILL_SIGN_PASSWORD env var. - Post-release macos-verify job asserts TeamIdentifier and hardened runtime for both darwin/amd64 and darwin/arm64 on a macOS runner. Notarization status is best-effort telemetry (ticket propagation can lag). - Credentials written to $RUNNER_TEMP with umask 077, cleaned up via if: always(). Quill added to $GITHUB_PATH explicitly. Revert path: #393 tracks reverting when goreleaser/quill syncs the fix.
jeremy
force-pushed
the
macos-codesign-teamidentifier
branch
from
March 27, 2026 20:43
8738e15 to
0015482
Compare
5 tasks
jeremy
added a commit
that referenced
this pull request
May 1, 2026
…460) * Revert macOS signing workaround now that GoReleaser ships TeamID fix GoReleaser v2.15.4 bumps goreleaser/quill to a commit synced with anchore/quill upstream, which includes the TeamIdentifier fix from anchore/quill v0.7.0. The workaround from #392 (parallel signing via scripts/sign-darwin.sh plus a separate post-publish notarize step) is no longer needed. - Drop the scripts/sign-darwin.sh build hook from .goreleaser.yaml and restore the native notarize.macos block with an env-gated `enabled` template, so local dev with no secrets stays signing-free. - Remove the quill install, credential prep, separate notarize, and credential cleanup workflow steps. Restore MACOS_* env vars on the GoReleaser step and drop --skip=notarize. - Bump the GoReleaser action from v2.14.1 to v2.15.4 and pin the same version in .mise.toml so make test-release exercises the same binary CI uses. Both pins carry sync comments cross-referencing each other. - Drop the "macOS signing tradeoffs" section from RELEASING.md — the publish/notarize race window it described no longer exists, since signing and notarization run as one wait-blocking call again. The macos-verify post-release job is signing-method-agnostic and stays in place as ongoing CI coverage of TeamIdentifier, hardened runtime, and notarization. Closes #393 * Clarify mise sync comment refers to GoReleaser CLI version The pinned value is the GoReleaser CLI version, not the goreleaser-action version (which is pinned by SHA at v7.0.0). Reword the cross-reference comment to make that distinction explicit.
jeremy
added a commit
that referenced
this pull request
Jul 24, 2026
Adds `basecamp todolists position <id|url>...` to reorder a to-do list
within its todoset — the one repositioning gap in the to-do family
(`todos` and `todolistgroups` already had it).
Single-list mode sets one explicit position (`--to N`, 1-based). Bulk
mode accepts several incomplete lists from the same todoset and sets
their visible order top-to-bottom; it is restricted to position 1
because reverse-iterating onto any other slot cannot reconstruct the
typed order once loose to-dos or hidden completed lists shift the base.
Bulk preflight enforces a shared todoset/bucket and rejects completed
lists before issuing any PUT, and application stops at the first failure
(a half-applied reorder is a wrong order), reporting applied-vs-attempted.
Uses the dedicated `PUT /todosets/todolists/{id}/position.json` route,
not the generic `/recordings/{id}/position.json` the issue's workaround
used — the generic one skips the to-do-list position math and mis-places
lists when loose to-dos or hidden completed lists exist.
Naming follows the to-do family (`position`, aliases `move`/`reorder`,
`--to` leading with `--position` accepted); a to-do list has no
destination container, so `--to N` is unambiguously an index.
SDK bumped to 93ace8d5 (basecamp-sdk#398), which also absorbs #396
(Templates.CreateProject envelope fix, signature unchanged) and #392
(SDK tests only).
Closes #484
jeremy
added a commit
that referenced
this pull request
Jul 25, 2026
* Add `todolists position` to reorder to-do lists (#484) Adds `basecamp todolists position <id|url>...` to reorder a to-do list within its todoset — the one repositioning gap in the to-do family (`todos` and `todolistgroups` already had it). Single-list mode sets one explicit position (`--to N`, 1-based). Bulk mode accepts several incomplete lists from the same todoset and sets their visible order top-to-bottom; it is restricted to position 1 because reverse-iterating onto any other slot cannot reconstruct the typed order once loose to-dos or hidden completed lists shift the base. Bulk preflight enforces a shared todoset/bucket and rejects completed lists before issuing any PUT, and application stops at the first failure (a half-applied reorder is a wrong order), reporting applied-vs-attempted. Uses the dedicated `PUT /todosets/todolists/{id}/position.json` route, not the generic `/recordings/{id}/position.json` the issue's workaround used — the generic one skips the to-do-list position math and mis-places lists when loose to-dos or hidden completed lists exist. Naming follows the to-do family (`position`, aliases `move`/`reorder`, `--to` leading with `--position` accepted); a to-do list has no destination container, so `--to N` is unambiguously an index. SDK bumped to 93ace8d5 (basecamp-sdk#398), which also absorbs #396 (Templates.CreateProject envelope fix, signature unchanged) and #392 (SDK tests only). Closes #484 * Preserve error classification on partial reposition failure The bulk-apply failure path reclassified every non-*output.Error (raw transport/runtime failures, context cancellation) as code=usage via ErrUsageHint, overwrote the SDK's structured hint with the rerun hint, and claimed the todoset was left in an intermediate order even when the failing PUT was the first/only one and nothing had been mutated. Now: when zero PUTs have landed (always true in single-list mode, and when a bulk run's first PUT fails), surface the underlying error unchanged — its code, HTTP status, and hint intact, no false intermediate-order claim. Only once a partial reorder has actually landed does it add the applied-count accounting and rerun hint, and even then it preserves the underlying error's classification (appending the rerun text to the original hint rather than replacing it) and wraps raw errors with %w instead of reclassifying them as usage. Adds regression coverage for Code/HTTPStatus preservation, original-hint preservation, and the zero-applied single-list and bulk-first-PUT paths. * Reject wrong-type URL args to todolists position A pasted Basecamp URL for a different recording type (a todo, a card, or a todolists *collection* URL) still yields a trailing numeric ID via ExtractIDs, which — because this command mutates — would silently reposition an unrelated todolist that happens to share that ID. Validate any URL argument up front: it must be a non-collection `todolists` URL, mirroring the guard todos position already applies to its --list destination URL. Bare numeric IDs are unaffected. * Warn of partial reorder when a transport failure interrupts a bulk run The raw-error branch of the partial-failure path wrapped the cause with %w and the applied-count prefix, but dropped the recovery hint entirely. After one successful PUT followed by a transport failure (a dropped connection, a canceled context), the operator saw the count but was never told the todoset was left partially reordered and needs a rerun. Return a non-usage structured error that carries the applied-count message, the rerun/intermediate-order hint, and the raw cause (via Cause, so errors.Is/As still unwrap). Code is api_error, never usage. Adds a regression test that drops the connection on the second applied PUT and asserts the non-usage code, the accounting message, the rerun hint, the preserved cause, and that the third list is never touched. * Reconcile SDK pin references and surface after rebase onto e2c1abea Rebasing onto main (#554) advanced the SDK pin to e2c1abea, five commits past the 93ace8d5 this branch first pinned. Refresh the API-COVERAGE SDK line to the merged pin (which additionally carries create-time visible_to_clients support) and regenerate the CLI surface so it reflects both todolists position and main's todolists create --visible-to-clients.
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
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.
Summary
goreleaser/quillfork) with directanchore/quillv0.7.1 CLI calls that correctly populate theTeamIdentifierfield in the CodeDirectorymacos-verifyjob on a macOS runner that asserts TeamIdentifier and hardened runtime for both darwin/amd64 and darwin/arm64; notarization status is best-effort telemetry$RUNNER_TEMPwithumask 077; quill added to$GITHUB_PATHexplicitlyQUILL_SIGN_PASSWORDenv var (quill's native config, not a CLI flag); notarize uses--notary-*flag namesFixes Help Scout #3270572064. Root cause: anchore/quill#147.
Follow-up: #393 tracks reverting when goreleaser/quill syncs the fix.
Test plan
make test-releasepasses locally (hook no-ops without quill/certs)goreleaser checkvalidates configactionlintpasses on release workflowbash -n scripts/sign-darwin.shpassescodesign -dv --verbose=4 basecamp 2>&1 | grep TeamIdentifierreturns2WNYUYRS7Gmacos-verifyjob passes in release workflowspctl -a -vvv -t install basecamp