Split code signing from build - #8587
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf157bb111
ℹ️ 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".
| name: Goose-darwin-arm64 | ||
| path: ui/desktop/out/Goose-darwin-arm64/Goose.zip | ||
| name: Goose-darwin-arm64-unsigned | ||
| path: ui/desktop/out/Goose-darwin-arm64/ |
There was a problem hiding this comment.
Prevent unsigned arm64 zip from colliding with release artifact
Uploading the entire Goose-darwin-arm64/ directory here also includes the unsigned Goose.zip, but sign-desktop later uploads a signed Goose.zip under a different artifact name. In signing-enabled runs, downstream workflows (for example release.yml with download-artifact + merge-multiple: true) merge artifact contents into one directory, so the duplicate Goose.zip names can overwrite/race and cause the unsigned binary to be released instead of the signed one. Keep the intermediate artifact to Goose.app (or otherwise avoid publishing a same-named zip before signing).
Useful? React with 👍 / 👎.
| name: Goose-darwin-x64 | ||
| path: ui/desktop/out/Goose-darwin-x64/Goose_intel_mac.zip | ||
| name: Goose-darwin-x64-unsigned | ||
| path: ui/desktop/out/Goose-darwin-x64/ |
There was a problem hiding this comment.
Prevent unsigned Intel zip from colliding with release artifact
This intermediate upload now includes Goose_intel_mac.zip (unsigned) because it archives the whole output directory, while sign-desktop-intel later uploads a signed Goose_intel_mac.zip. When release jobs merge all artifacts into one directory (download-artifact with merge-multiple: true), those same-named files conflict, so the final published Intel zip can be the unsigned one depending on extraction order. Exclude/rename the unsigned zip in the build artifact so only the signed zip is eligible for publication.
Useful? React with 👍 / 👎.
|
This pull request has been automatically marked as stale because it has not had recent activity for 23 days. What happens next?
Thank you for your contribution! 🚀 |
DOsinga
left a comment
There was a problem hiding this comment.
Looks good — one thing to double-check: Codex flagged that the unsigned intermediate artifacts (which upload the entire output directory including the .zip) could collide with the signed .zip when the release job downloads everything with merge-multiple: true. That seems like a real concern worth verifying before merge.
* main: (66 commits) Switch GH pages deploy to actions/artifact workflow (#9025) fix(summon): re-apply canonical limits when delegate overrides model (#9183) Split code signing from build (#8587) refactor(logging): consolidate logging setup into shared helper in goose crate (#8817) fix(cli): report cumulative total_tokens in stream-json/json output (#8910) plugins: add open plugins (just skills for now) (#9063) fix(providers): refresh GCP metadata server token on expiration (#8929) chore(deps): bump the cargo-minor-and-patch group across 1 directory with 14 updates (#9178) chore(deps): bump bzip2 from 0.5.2 to 0.6.1 (#8964) chore(deps): bump tauri from 2.10.3 to 2.11.1 in /ui/goose2/src-tauri (#9066) chore(deps): bump hono from 4.12.14 to 4.12.18 in /evals/open-model-gym/mcp-harness (#9073) localize hardcoded strings in provider settings UI (#8931) chore(deps): bump @babel/plugin-transform-modules-systemjs from 7.28.5 to 7.29.4 in /documentation (#9122) move settings into app shell (#9047) Add Location column to CLI skills table (#8785) (feat): add routstr as a declarative provider (#9175) Add FuturMix provider (#8840) fix: convert quoted numeric config values to numbers if needed (#8844) fix(ui): keep SSE reconnect loop alive on long disconnects (#8717) (#8846) fix(openai): apply request_params to outgoing API payload (#9151) ...
* main: (102 commits) Dynamically refresh skill instructions each turn (#9217) Build non-vulkan linux variants using ubuntu 22.04 (#9211) fix(ui): show tool name in approval prompt (#9216) feat: add Atomic Chat as declarative OpenAI-compatible provider (#9210) chore: bump package.json versions from 0.19.1 to 0.20.0 (#9218) feat: support GOOSE_OAUTH_CALLBACK_PORT for stable OAuth redirect_uri (#9209) [RFC] feat(oauth): proactive token refresh to avoid re-auth on every session (#8386) fix: resolve Azure CLI on Windows by using az.cmd (#9215) fix: handle non-interactive terminal in goose configure on Windows (#9214) Better parsing of pasted html as markdown so agents understand (#9190) fix: persist accumulated cost in session DB to survive reload (#9191) fix(publish-npm): build binary from current SHA + add compat check (#9212) feat(desktop): add goose://new-session deep link to open fresh chat (#9196) Add PR previews using cloudflare pages (#9208) fix: prevent tool-use marker leakage in toolshim output (#8310) Prompt injection mitigation: update pattern-based detection (#9198) remove goose2 related skills (#9189) Switch GH pages deploy to actions/artifact workflow (#9025) fix(summon): re-apply canonical limits when delegate overrides model (#9183) Split code signing from build (#8587) ...
Code signing can fail for all sorts of reasons. This lets us retry without rebuilding.