[codex] fix desktop update packaging integrity#96
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.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: 763a748745
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return new Promise((resolve, reject) => { | ||
| const request = NodeHttp.get( | ||
| { | ||
| hostname: "127.0.0.1", |
There was a problem hiding this comment.
In environments where Node resolves localhost to IPv6 (for example Node 24 on this runner binds server.listen(..., "localhost") to ::1), this smoke test never reaches the mock update server: scripts/mock-update-server.ts listens on localhost, but the new readiness probe always dials 127.0.0.1, so waitForUpdateServer times out before the update flow starts. Please make the mock server and probe use the same explicit host, or probe localhost here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8cbcb3544
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| rootPackageNames: [ | ||
| ...Object.keys(resolvedServerDependencies), | ||
| ...Object.keys(platformFffNativeDependencies), | ||
| ...Object.keys(linuxWslFffNativeDependencies), |
There was a problem hiding this comment.
When packaging Windows/macOS builds, stageClerkPasskeyNativeBinaries copies Clerk passkey .node files into the desktop runtime package, but this manifest seed only walks server/fff dependencies. If an update leaves one of those smart-unpacked desktop native files missing from app.asar.unpacked, the new bootstrap integrity check still passes and the app can fail later while initializing createClerkBridge({ passkeys: true }), which defeats the incomplete-install diagnostic this change is adding. Please include the desktop native unpack roots (at least @clerk/electron-passkeys) in the required-file manifest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 473ccf14d1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return { | ||
| ...state, | ||
| status: "installing", | ||
| message: "Installing update. T3 Code will restart when installation is ready.", |
There was a problem hiding this comment.
When installUpdate() succeeds, DesktopUpdates.install returns the current state after setting this installing message, with { accepted: true, completed: false }. Both the sidebar and settings flows interpret any accepted/non-completed action with a non-empty state.message as an error toast, so a normal install handoff can briefly show “Could not install update” with this installing text before the app restarts. Please keep the installing status message out of the action-error path or special-case installing as non-error.
Useful? React with 👍 / 👎.
Summary
Fixes the desktop update corruption path by shrinking the Windows unpacked payload, disabling differential updater downloads, adding a launch-time packaged integrity check, and adding CI coverage for a real install N-1 -> update -> relaunch path.
Root cause
The desktop package unpacked the entire production node_modules tree into app.asar.unpacked. Windows in-app updates then had to install thousands of loose files, making silent install slow and increasing the chance of a partial app.asar.unpacked payload after update.
Changes
Validation
Note: the true Windows update-path proof is the new CI job; this PR should not be considered fixed until that job passes on GitHub.