Skip to content

ci(desktop): keep unsigned verification out of updater signing - #5338

Merged
lidge-jun merged 9 commits into
devfrom
codex/260920-desktop-ci-signing
Sep 20, 2026
Merged

lidge-jun merged 9 commits into
devfrom
codex/260920-desktop-ci-signing

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Summary

The macOS macos widget + bundle verification job successfully builds the app, widget extension, and sidecar, then Tauri also tries to create an updater archive and refuses because the job intentionally has no TAURI_SIGNING_PRIVATE_KEY.

This PR passes createUpdaterArtifacts: false only to that unsigned CI invocation. The committed Tauri release configuration still enables updater artifacts, and the release workflow still requires the signing secrets before publication.

Scope

The final PR diff against current dev is exactly one file: .github/workflows/ci.yml.

Unchanged:

  • workflow triggers and permissions
  • action pins and checkout credential behavior
  • repository secrets and key material
  • desktop/src-tauri/tauri.conf.json
  • .github/workflows/release.yml
  • release eligibility and updater publication checks
  • post-build assertions for the app, embedded WidgetKit appex, executable sidecar, and code signatures

The temporary dangling-role test repair was removed after #5344 landed the focused fix directly on dev. This branch now includes current dev (ab90bea6b1e53ba5c8f8c1eda538e88ec70682f3) and contains no overlapping test diff.

Validation

  • git diff --check origin/dev...HEAD — clean
  • final file list — .github/workflows/ci.yml only
  • exact-head hosted CI is the required execution evidence, especially macos widget + bundle

Security and release review

The command uses a repository-authored literal JSON override, not event-controlled input. It does not add release credentials to the verification job or relax release signing. Explicit workflow/security review has been completed; merge remains gated on replacement exact-head hosted CI.

Integration note

No Go counterpart exists for this GitHub Actions-only change. The remote repository currently has no dev2-go branch despite stale transition wording in MAINTAINERS.md.

Checklist

  • Targets dev; main is untouched.
  • Final diff is workflow-only and minimal.
  • Explicit workflow/security review completed.
  • Replacement exact-head hosted CI is fully green.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 20, 2026 12:33
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-20T12:36:06.144036Z 66cf989 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4156d2fb-9295-4d11-a3b9-48101b0e92b5

📥 Commits

Reviewing files that changed from the base of the PR and between 66cf989 and d356cb1.

📒 Files selected for processing (3)
  • src/lab/conformance/fixture-provider.ts
  • src/lab/conformance/fixtures/protocol-v1-cases.json
  • tests/lab/lab-conformance-harness.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The widget CI build now disables Tauri updater artifact creation. The conformance fixture documentation, expected role mapping, and fixture configuration test now reflect native OpenAI behavior.

Changes

Widget CI build

Layer / File(s) Summary
Unsigned verification build
.github/workflows/ci.yml
The widget job passes createUpdaterArtifacts: false to bunx tauri build --ci --bundles app, so the verification build does not require TAURI_SIGNING_PRIVATE_KEY.

Protocol conformance

Layer / File(s) Summary
Role mapping expectations
src/lab/conformance/fixture-provider.ts, src/lab/conformance/fixtures/protocol-v1-cases.json, tests/lab/lab-conformance-harness.test.ts
The Chat fixture documentation no longer claims developer-role support. The request-mapping fixture expects developer content to map to the system role. A test verifies the native OpenAI URL and confirms that foldDeveloperRoleToSystem is not declared.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the desktop CI change that disables updater signing requirements for unsigned verification builds. It is concise and directly related to a primary change in the pull reques…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved exact head 66cf9892a6a6601dd153f0633fdb46a766981b3d after explicit workflow/security review.

The diff changes only the existing read-only macOS verification command. It does not alter triggers, permissions, action pins, checkout credential persistence, secret exposure, or release eligibility. The JSON override is a repository-authored literal, not event-controlled input. It disables updater artifact creation only for this unsigned CI invocation; desktop/src-tauri/tauri.conf.json remains createUpdaterArtifacts: true, and the release workflow keeps its existing private-key requirement and publication checks. The app, embedded WidgetKit appex, sidecar, executability, and code-signature assertions remain unchanged.

Merge only after the exact-head required CI, especially macos widget + bundle, completes successfully.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 20, 2026
@lidge-jun lidge-jun changed the title ci(desktop): keep the release updater key out of the verification build ci,test: unblock dev — desktop verification build and the developer-role fixture Sep 20, 2026
@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 20, 2026

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved exact head 6809e9fcdfd440c955b6454b3077bc51bd2f3ba4.

The workflow/security conclusion from the prior head is unchanged. The follow-up fixes the independent dev regression introduced by #5334: the CL-01 Chat vector now expects the safe undeclared-destination fold, and the fixture-provider comment no longer falsely claims that the undeclared fixture asserts developer-role support. With isolated temporary HOME, CODEX_HOME, and OPENCODEX_HOME, tests/lab/lab-conformance-harness.test.ts passes 17/17, including all 35 canonical protocol scenarios.

Merge remains gated on the replacement exact-head hosted CI, especially the previously failing macos widget + bundle job.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final exact-head approval for d356cb1ee138177a53479fe1d38cdfadbb2d20d0.

The head now includes a focused regression test for the CL-01 fixture contract, satisfying the hygiene gate: native OpenAI URL selection does not implicitly declare optional developer-role support, while the canonical scenario verifies the safe fold and all 35 protocol scenarios pass. Isolated focused result: 18 pass, 0 fail, 102 assertions. The workflow/security review remains unchanged and approved.

Do not merge until this replacement head completes required hosted CI, including macos widget + bundle.

@github-actions github-actions Bot added documentation Improvements or additions to documentation and removed chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). intake: hygiene-blocked Deterministic PR hygiene checks failed labels Sep 20, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 42 / 80

dev가 빨간 이유가 서로 다른 두 개예요. 이 PR은 그 두 개를 한 번에 풀어서, CI를 한 바퀴 돌리면 둘 다 지나가게 만들어요.

첫 번째예요. macOS 데스크톱 확인 빌드는 위젯·번들·사이드카까지 잘 만든 뒤, tauri build에서 “공개 키는 있는데 비밀 키가 없다”고 멈춰요. createUpdaterArtifacts가 켜져 있고 업데이터 공개 키는 저장소에 들어 있는데, 확인 빌드에는 릴리즈 비밀 키가 없어요. 확인 단계는 앱이 만들어졌는지, 위젯이 들어갔는지, ocx가 있는지, appex 서명이 있는지만 봐요. 업데이터 서명 파일은 필요 없어요. 그래서 이 작업의 tauri build만 --config로 업데이터 산출을 끄고, tauri.conf.json과 release.yml의 서명 길은 그대로 둬요.

두 번째예요. #5334 이후 destination에 기록이 없으면 developer는 system으로 접혀요. Lab의 chat-core.protocol.request-mapping은 아직 전선에 developer가 남는다고 기대해서 shard가 깨졌어요. 픽스처 입력 바이트는 그대로 두고, 기대값만 system으로 바꿨어요. DEV가 두 번째 자리인 것도 그대로예요. fixtureProviderConfig는 foldDeveloperRoleToSystem을 안 적어서 “안전한 기본”을 쓰는지 테스트 하나로 고정했어요. base는 dev예요.

라인 - .github/workflows/ci.yml Build unsigned desktop app — 오버라이드는 이 run 한 줄뿐이고 Verify 단언은 그대로예요. 방향은 맞아요. 다만 같은 뿌리 원인(공개 키 + 비밀 키 없음)을 로컬 build:local에서 다루는 열린 PR이 #5327이에요. 여기는 CI 확인 작업, 저기는 로컬 스크립트라 중복으로 닫을 대상은 아니에요. 둘 다 필요할 수 있어요.

라인 - src/lab/conformance/fixtures/protocol-v1-cases.json roles — 입력 바이트·digest는 안 건드리고 expected만 바꿨어요. #5334의 “기록 없음 → 접기”와 맞아요. 새 단위 테스트는 설정 필드가 undefined인지만 봐요. 실제 전선 매핑은 이 시나리오 assertion이 담당해요. 그 짝은 괜찮아요.

라인 - 한 PR에 CI 서명 우회와 Lab 픽스처 수정이 같이 있어요. 원인은 독립이고, 작성자도 “한 CI 사이클”을 이유로 묶었어요. 범위가 작은 편이라 위험은 낮아요.

메인테이너의 판단이 필요한 지점

두 고침을 이대로 한 PR에 둘지, 나중에 되짚기 쉽게 나눠 머지할지만 정해 주세요. #5327은 같은 업데이터 키 문제를 로컬에서 풀지만 파일이 겹치지 않아요. 무효/중복으로 닫지 마세요. types.ts/config.ts 분할이나 미리보기 배포 이야기는 해당 없어요.

너의 추천

이 head로 hosted CI가 초록인지 확인한 뒤 머지하세요. 릴리즈 서명 비밀을 CI 확인 작업에 넣지 마세요. #5327은 로컬 빌드 쪽이 정리될 때까지 열어 두세요. 픽스처·워크플로 변경은 작아서, CI만 통과하면 합쳐도 됩니다.

이 댓글은 grok-bot이 작성했습니다

@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed.

Hygiene

✅ Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft September 20, 2026 13:01
@github-actions
github-actions Bot marked this pull request as ready for review September 20, 2026 13:01

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head approval for 128b0f2e472cee99d48cad8c75330fc75b3a4449 after synchronizing current dev.

#5341 now owns the CL-01 developer-role fix on dev; the temporary overlapping fixture commits have been neutralized and the branch includes the merged base. GitHub’s final PR file list is again exactly one file: .github/workflows/ci.yml. The prior workflow/security review applies unchanged. Isolated CL-01 verification on the synchronized tree: 17 pass, 0 fail, all 35 canonical scenarios green.

Merge only after this replacement head completes required hosted CI, including the macOS widget bundle.

@Ingwannu

Copy link
Copy Markdown
Owner

The replacement-head run exposed one unrelated current-dev regression in test 3/4: three dangling-toolcall assertions still expect wire role developer after the generic OpenAI Chat destination began folding that role to system. I kept this workflow PR single-file and opened the minimal test-only repair as #5343. Please do not merge #5338 while its exact-head required CI is red; after #5343 is independently reviewed and lands, refresh this branch onto current dev and rerun the macOS widget bundle job.

@Ingwannu Ingwannu changed the title ci,test: unblock dev — desktop verification build and the developer-role fixture ci,test: unblock dev desktop bundle and dangling-role checks Sep 20, 2026
@Ingwannu Ingwannu changed the title ci,test: unblock dev desktop bundle and dangling-role checks ci(desktop): keep unsigned verification out of updater signing Sep 20, 2026
@github-actions github-actions Bot added chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). and removed documentation Improvements or additions to documentation labels Sep 20, 2026
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…not meant to have"

This reverts commit 8b957dd.

#5338 makes the same change - the same flag on the same invocation - and was opened for exactly
this failure. One fix is enough, and a duplicate in a stack that has to rebase repeatedly would
only collide with it. The widget job's repair belongs to that pull request.
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…not meant to have"

This reverts commit 8b957dd.

#5338 makes the same change - the same flag on the same invocation - and was opened for exactly
this failure. One fix is enough, and a duplicate in a stack that has to rebase repeatedly would
only collide with it. The widget job's repair belongs to that pull request.
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…the widget is linked

The widget job's verification step has never run: the build above it stopped on the updater key
every time, so nobody noticed that its first assertion looks for Contents/MacOS/OpenCodex - the
productName - while the bundle carries opencodex-desktop, the crate name. #5338 fixes the build,
which is what exposes this.

It also now asserts that the WidgetBundle is linked into the extension. That is the defect this
stack fixes elsewhere, and the reason it shipped is that nothing observable distinguishes a widget
from a widget with no widget in it: the appex builds, signs and registers with pluginkit exactly
the same way, and only the gallery stays empty.
@lidge-jun
lidge-jun merged commit 1bccc45 into dev Sep 20, 2026
25 of 32 checks passed
@lidge-jun
lidge-jun deleted the codex/260920-desktop-ci-signing branch September 20, 2026 14:17
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…the widget is linked

The widget job's verification step has never run: the build above it stopped on the updater key
every time, so nobody noticed that its first assertion looks for Contents/MacOS/OpenCodex - the
productName - while the bundle carries opencodex-desktop, the crate name. #5338 fixes the build,
which is what exposes this.

It also now asserts that the WidgetBundle is linked into the extension. That is the defect this
stack fixes elsewhere, and the reason it shipped is that nothing observable distinguishes a widget
from a widget with no widget in it: the appex builds, signs and registers with pluginkit exactly
the same way, and only the gallery stays empty.
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…the widget is linked

The widget job's verification step has never run: the build above it stopped on the updater key
every time, so nobody noticed that its first assertion looks for Contents/MacOS/OpenCodex - the
productName - while the bundle carries opencodex-desktop, the crate name. #5338 fixes the build,
which is what exposes this.

It also now asserts that the WidgetBundle is linked into the extension. That is the defect this
stack fixes elsewhere, and the reason it shipped is that nothing observable distinguishes a widget
from a widget with no widget in it: the appex builds, signs and registers with pluginkit exactly
the same way, and only the gallery stays empty.
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…the widget is linked

The widget job's verification step has never run: the build above it stopped on the updater key
every time, so nobody noticed that its first assertion looks for Contents/MacOS/OpenCodex - the
productName - while the bundle carries opencodex-desktop, the crate name. #5338 fixes the build,
which is what exposes this.

It also now asserts that the WidgetBundle is linked into the extension. That is the defect this
stack fixes elsewhere, and the reason it shipped is that nothing observable distinguishes a widget
from a widget with no widget in it: the appex builds, signs and registers with pluginkit exactly
the same way, and only the gallery stays empty.
lidge-jun added a commit that referenced this pull request Sep 20, 2026
…the widget is linked

The widget job's verification step has never run: the build above it stopped on the updater key
every time, so nobody noticed that its first assertion looks for Contents/MacOS/OpenCodex - the
productName - while the bundle carries opencodex-desktop, the crate name. #5338 fixes the build,
which is what exposes this.

It also now asserts that the WidgetBundle is linked into the extension. That is the defect this
stack fixes elsewhere, and the reason it shipped is that nothing observable distinguishes a widget
from a widget with no widget in it: the appex builds, signs and registers with pluginkit exactly
the same way, and only the gallery stays empty.
lidge-jun added a commit that referenced this pull request Sep 20, 2026
* ci(desktop): assert the executable the app bundle declares

The macos widget + bundle job's Verify step asserted
Contents/MacOS/OpenCodex, but Tauri renames the main binary only when
mainBinaryName is set. desktop/src-tauri/tauri.conf.json does not set it,
so the bundled executable keeps the Cargo bin name opencodex-desktop and
that assertion could never pass.

The step had never run before: every earlier attempt died in the build
ahead of it on the missing updater signing key, which #5338 has now
separated out. With the build green the Verify step ran for the first time
and failed on its first line with no output, because test is silent.

Read CFBundleExecutable from the bundle's own Info.plist rather than
restating the name, so the check follows the config instead of drifting
from it, and reject an empty value so a missing key cannot pass by testing
a directory. The other three assertions are unchanged.

* docs(devlog): record the two-layer widget job failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants