ci(desktop): keep unsigned verification out of updater signing - #5338
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe 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. ChangesWidget CI build
Protocol conformance
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
Ingwannu
left a comment
There was a problem hiding this comment.
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.
Ingwannu
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
리뷰 · 우선순위 42 / 80
첫 번째예요. macOS 데스크톱 확인 빌드는 위젯·번들·사이드카까지 잘 만든 뒤, 두 번째예요. #5334 이후 destination에 기록이 없으면 라인 - 라인 - 라인 - 한 PR에 CI 서명 우회와 Lab 픽스처 수정이 같이 있어요. 원인은 독립이고, 작성자도 “한 CI 사이클”을 이유로 묶었어요. 범위가 작은 편이라 위험은 낮아요. 메인테이너의 판단이 필요한 지점 두 고침을 이대로 한 PR에 둘지, 나중에 되짚기 쉽게 나눠 머지할지만 정해 주세요. #5327은 같은 업데이터 키 문제를 로컬에서 풀지만 파일이 겹치지 않아요. 무효/중복으로 닫지 마세요. types.ts/config.ts 분할이나 미리보기 배포 이야기는 해당 없어요. 너의 추천 이 head로 hosted CI가 초록인지 확인한 뒤 머지하세요. 릴리즈 서명 비밀을 CI 확인 작업에 넣지 마세요. #5327은 로컬 빌드 쪽이 정리될 때까지 열어 두세요. 픽스처·워크플로 변경은 작아서, CI만 통과하면 합쳐도 됩니다. 이 댓글은 grok-bot이 작성했습니다 |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
Ingwannu
left a comment
There was a problem hiding this comment.
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.
|
The replacement-head run exposed one unrelated current- |
…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.
…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.
…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.
…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.
…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.
…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.
…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.
* 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
Summary
The macOS
macos widget + bundleverification 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 noTAURI_SIGNING_PRIVATE_KEY.This PR passes
createUpdaterArtifacts: falseonly 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
devis exactly one file:.github/workflows/ci.yml.Unchanged:
desktop/src-tauri/tauri.conf.json.github/workflows/release.ymlThe temporary dangling-role test repair was removed after #5344 landed the focused fix directly on
dev. This branch now includes currentdev(ab90bea6b1e53ba5c8f8c1eda538e88ec70682f3) and contains no overlapping test diff.Validation
git diff --check origin/dev...HEAD— clean.github/workflows/ci.ymlonlymacos widget + bundleSecurity 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-gobranch despite stale transition wording inMAINTAINERS.md.Checklist
dev;mainis untouched.