Bound evaOS notarization polling - #284
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/evaosBetaReleaseGate.js`:
- Around line 533-541: collectReleaseConfigIssues currently only audits
scripts/afterSign.js for notarization helpers but misses the DMG path; update
the function to call readText(rootDir, 'scripts/evaosFinalizeMacDmg.js') and add
requireText checks for the DMG-specific helpers and env markers analogous to the
app checks—specifically add requireText(afterDmg, 'buildNotarytoolSubmitArgs',
'scripts/evaosFinalizeMacDmg.js', issues), requireText(afterDmg,
'buildNotarytoolInfoArgs', 'scripts/evaosFinalizeMacDmg.js', issues),
requireText(afterDmg, 'EVAOS_DMG_NOTARY_PROCESS_TIMEOUT_MS',
'scripts/evaosFinalizeMacDmg.js', issues), requireText(afterDmg,
'EVAOS_DMG_NOTARY_POLL_INTERVAL_MS', 'scripts/evaosFinalizeMacDmg.js', issues)
and a notarytool info polling evidence check like requireText(afterDmg,
'notarytool info', 'scripts/evaosFinalizeMacDmg.js', issues, 'dmg notarization
status polling') so DMG submit+poll regressions are audited (refer to
collectReleaseConfigIssues, buildNotarytoolInfoArgs, buildNotarytoolSubmitArgs).
- Around line 517-523: The current requireText call (requireText(reusableBuild,
'timeout-minutes: 45', '.github/workflows/_build-reusable.yml', issues, 'bounded
macOS release packaging step')) only checks global text and can false-pass;
change the assertion to scope the check to the specific job/step named "Build
with electron-builder (macOS)" by matching the job block or step label and
verifying 'timeout-minutes: 45' appears inside that block (e.g., use a regex
that anchors to the 'Build with electron-builder (macOS)' section or add a
helper like requireTextWithinJob that takes reusableBuild and the job name and
asserts the timeout string is inside that job) so the check fails if the macOS
packaging job remains unbounded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 42977725-5c62-4b45-8147-a3403a261817
📒 Files selected for processing (5)
.github/workflows/_build-reusable.ymlscripts/afterSign.jsscripts/evaosBetaReleaseGate.jsscripts/evaosFinalizeMacDmg.jstests/unit/process/evaosBetaReleaseGate.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Test (macos-arm64)
- GitHub Check: Build Test (macos-x64)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions
Files:
scripts/evaosBetaReleaseGate.jstests/unit/process/evaosBetaReleaseGate.test.tsscripts/evaosFinalizeMacDmg.jsscripts/afterSign.js
**/*[!.module].ts
📄 CodeRabbit inference engine (AGENTS.md)
Utility files must use camelCase naming (e.g., formatDate.ts)
Files:
tests/unit/process/evaosBetaReleaseGate.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use@icon-park/reactfor icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases:@/,@process/,@renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)
Files:
tests/unit/process/evaosBetaReleaseGate.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for testing with a coverage target of at least 80%
Files:
tests/unit/process/evaosBetaReleaseGate.test.ts
🪛 Betterleaks (1.3.1)
tests/unit/process/evaosBetaReleaseGate.test.ts
[high] 233-233: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (21)
.github/workflows/_build-reusable.yml (1)
596-596: LGTM!scripts/afterSign.js (8)
22-23: LGTM!Also applies to: 140-150
182-209: LGTM!
211-216: LGTM!
266-312: LGTM!
314-326: LGTM!
328-363: LGTM!
389-393: LGTM!
463-474: LGTM!scripts/evaosFinalizeMacDmg.js (7)
43-44: LGTM!
58-103: LGTM!
118-142: LGTM!
151-196: LGTM!
198-243: LGTM!
268-270: LGTM!
318-328: LGTM!tests/unit/process/evaosBetaReleaseGate.test.ts (5)
21-62: LGTM!
229-289: LGTM!
291-361: LGTM!
453-466: LGTM!
520-525: LGTM!
| requireText( | ||
| reusableBuild, | ||
| 'timeout-minutes: 45', | ||
| '.github/workflows/_build-reusable.yml', | ||
| issues, | ||
| 'bounded macOS release packaging step' | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
timeout-minutes: 45 check is too loose and can false-pass
The assertion at Line 519 checks only global text presence. This can pass if the same string appears in another job/step or comment, while Build with electron-builder (macOS) is unbounded.
Suggested hardening
- requireText(
- reusableBuild,
- 'timeout-minutes: 45',
- '.github/workflows/_build-reusable.yml',
- issues,
- 'bounded macOS release packaging step'
- );
+ if (
+ !/name:\s*Build with electron-builder \(macOS\)[\s\S]*?id:\s*macos-build[\s\S]*?timeout-minutes:\s*45/m.test(
+ reusableBuild
+ )
+ ) {
+ issues.push(
+ '.github/workflows/_build-reusable.yml: missing timeout-minutes: 45 on Build with electron-builder (macOS) step'
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| requireText( | |
| reusableBuild, | |
| 'timeout-minutes: 45', | |
| '.github/workflows/_build-reusable.yml', | |
| issues, | |
| 'bounded macOS release packaging step' | |
| ); | |
| if ( | |
| !/name:\s*Build with electron-builder \(macOS\)[\s\S]*?id:\s*macos-build[\s\S]*?timeout-minutes:\s*45/m.test( | |
| reusableBuild | |
| ) | |
| ) { | |
| issues.push( | |
| '.github/workflows/_build-reusable.yml: missing timeout-minutes: 45 on Build with electron-builder (macOS) step' | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/evaosBetaReleaseGate.js` around lines 517 - 523, The current
requireText call (requireText(reusableBuild, 'timeout-minutes: 45',
'.github/workflows/_build-reusable.yml', issues, 'bounded macOS release
packaging step')) only checks global text and can false-pass; change the
assertion to scope the check to the specific job/step named "Build with
electron-builder (macOS)" by matching the job block or step label and verifying
'timeout-minutes: 45' appears inside that block (e.g., use a regex that anchors
to the 'Build with electron-builder (macOS)' section or add a helper like
requireTextWithinJob that takes reusableBuild and the job name and asserts the
timeout string is inside that job) so the check fails if the macOS packaging job
remains unbounded.
| requireText(afterSign, 'assertPublicBetaNotarizationEnv', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'buildAppNotarytoolSubmitArgs', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'buildAppNotarytoolInfoArgs', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'EVAOS_APP_NOTARY_PROCESS_TIMEOUT_MS', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'EVAOS_APP_NOTARY_COMMAND_PROCESS_TIMEOUT_MS', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'EVAOS_APP_NOTARY_POLL_INTERVAL_MS', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'EVAOS_APP_TRUST_PROCESS_TIMEOUT_MS', 'scripts/afterSign.js', issues); | ||
| requireText(afterSign, 'notarytool info', 'scripts/afterSign.js', issues, 'app notarization status polling'); | ||
| requireText(afterSign, 'getNotarizationOptions', 'scripts/afterSign.js', issues); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Release-gate coverage is incomplete: DMG notarization polling path is not audited
The new assertions at Line 535, Line 537, Line 538, and Line 540 only verify scripts/afterSign.js (app path). collectReleaseConfigIssues still does not audit scripts/evaosFinalizeMacDmg.js, so DMG submit+poll regressions can pass audit-config.
- Add
readText(rootDir, 'scripts/evaosFinalizeMacDmg.js'). - Add required markers for DMG submit+poll helpers/env keys (
buildNotarytoolInfoArgs, DMG timeout/poll env vars, andnotarytool infopolling evidence).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/evaosBetaReleaseGate.js` around lines 533 - 541,
collectReleaseConfigIssues currently only audits scripts/afterSign.js for
notarization helpers but misses the DMG path; update the function to call
readText(rootDir, 'scripts/evaosFinalizeMacDmg.js') and add requireText checks
for the DMG-specific helpers and env markers analogous to the app
checks—specifically add requireText(afterDmg, 'buildNotarytoolSubmitArgs',
'scripts/evaosFinalizeMacDmg.js', issues), requireText(afterDmg,
'buildNotarytoolInfoArgs', 'scripts/evaosFinalizeMacDmg.js', issues),
requireText(afterDmg, 'EVAOS_DMG_NOTARY_PROCESS_TIMEOUT_MS',
'scripts/evaosFinalizeMacDmg.js', issues), requireText(afterDmg,
'EVAOS_DMG_NOTARY_POLL_INTERVAL_MS', 'scripts/evaosFinalizeMacDmg.js', issues)
and a notarytool info polling evidence check like requireText(afterDmg,
'notarytool info', 'scripts/evaosFinalizeMacDmg.js', issues, 'dmg notarization
status polling') so DMG submit+poll regressions are audited (refer to
collectReleaseConfigIssues, buildNotarytoolInfoArgs, buildNotarytoolSubmitArgs).
1 participant
Summary
notarytool submit --waitwith explicit submit-id capture plus boundednotarytool infopolling.Evidence
27443987790reached exactly two fresh accepted app zip submissions, then remained inBuild with electron-builder (macOS)with no fresh DMG submissions or mounted-app validation.submit --waitdependency that can hide after Apple has already accepted the submission.Validation
npm test -- tests/unit/process/evaosBetaReleaseGate.test.ts --maxWorkers=1node scripts/evaosBetaReleaseGate.js audit-configprek run --files .github/workflows/_build-reusable.yml scripts/afterSign.js scripts/evaosFinalizeMacDmg.js scripts/evaosBetaReleaseGate.js tests/unit/process/evaosBetaReleaseGate.test.tsgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Chores