P0: bundle self-contained Mac connector runtime - #675
Conversation
📝 WalkthroughWalkthroughThe macOS desktop bridge now prepares, embeds, signs, and verifies a pinned CPython 3.12 runtime with PyObjC dependencies. Workflows, packaging, release gates, smoke checks, retry handling, and tests validate provenance, architecture, licensing, symlinks, signatures, and execution. ChangesBundled Python Runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BuildWorkflow
participant RuntimePreparation
participant BridgeResource
participant PackageVerification
participant ReleaseGate
BuildWorkflow->>RuntimePreparation: prepare pinned Python runtime
RuntimePreparation->>BridgeResource: provide runtime and provenance metadata
BridgeResource->>PackageVerification: package bundled runtime
PackageVerification->>PackageVerification: verify architecture, provenance, symlinks, and signatures
ReleaseGate->>PackageVerification: inspect ZIP payload and runtime artifacts
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
evaOS review status: stale headPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #675 Details: Superseded by a newer PR head. |
|
@coderabbitai review Please review exact current head |
evaOS review status: stale headPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #675 Details: Superseded by a newer PR head. |
|
✅ Action performedReview finished.
|
coderabbitai
Bot
added
area:native-companion
![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: a8302ad451
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 @.github/workflows/_build-reusable.yml:
- Around line 673-679: Update the “Install pinned evaOS desktop bridge Python
runtime” workflow step to pass matrix.arch through the step’s env configuration,
then reference the environment variable in the
scripts/prepareEvaosDesktopBridgePythonRuntime.sh invocation instead of inline
GitHub template expansion. Preserve the existing macOS condition, shell, and
GITHUB_ENV argument.
In `@scripts/afterPack.js`:
- Around line 254-256: Update the validation around versionedPythonPath in the
after-pack checks to require executable access using X_OK, not only filesystem
existence. In strict mode, also validate that versionedPythonPath itself is a
Mach-O binary, while preserving the existing missing-entry behavior and applying
the same requirement to the related checks around the referenced lines.
In `@scripts/create-mock-release-artifacts.sh`:
- Line 74: Update the mock ZIP creation flow in
scripts/create-mock-release-artifacts.sh to preserve symlinks such as
python_bin/python3 instead of dereferencing them. Use ZipInfo entries with
symlink metadata and the link target as entry content for symlinks, while
continuing to use archive.write() for regular files.
In `@scripts/evaosBetaReleaseGate.js`:
- Line 1651: Update the Python validation logic around pythonManifestValid and
pythonLicenseDigestValid to use a hard-coded, verified CPython license SHA-256
rather than the ZIP manifest’s licenseSha256 value. Require the manifest license
field to equal Python-2.0, and apply the same checks at all corresponding
validation blocks.
- Around line 1664-1665: Update the PyObjC validation that assigns
result["pythonObjcMachO"] to verify the extension’s architecture, not just
whether its header is any Mach-O magic. Accept a thin binary only when it
matches the expected runtime architecture, or accept a fat binary only when it
contains a matching slice; apply the same validation at both occurrences and
assert the resulting architecture check.
In `@scripts/prepareEvaosDesktopBridgePythonRuntime.sh`:
- Around line 45-48: Derive the Python minor-version component from
PYTHON_RUNTIME_VERSION and reuse it wherever the script currently references
python3.12, including the runtime LICENSE path, lipo validation path, and the
additional occurrence near the script’s later runtime handling. Preserve the
existing checks while ensuring future version bumps update all paths
automatically.
In `@tests/unit/bootstrap/afterPackBundledResources.test.ts`:
- Around line 283-290: Update the symlink setup in the test around
verifyEvaosDesktopBridgeResource to use an absolute target that exists during
the test, while still representing a relocatable launcher symlink scenario.
Ensure the target is created or points to an available Python executable before
invoking the verifier, so the assertion exercises the symlink-relocatability
check rather than the missing-runtime validation.
In `@tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts`:
- Line 27: Update the installPythonRuntime signature in the test mock to use the
concrete structured metadata type returned by the implementation, including its
packages array, instead of Record<string, string> | undefined. Reuse the
existing return type or metadata interface if available, and keep the optional
parameters and undefined result behavior unchanged.
🪄 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: 37379dc7-bd5f-4bbd-b129-b53aaa9f758a
📒 Files selected for processing (13)
.github/workflows/_build-reusable.yml.github/workflows/workbench-functional-smoke.ymlCHANGELOG.mdscripts/afterPack.jsscripts/afterSign.jsscripts/create-mock-release-artifacts.shscripts/evaosBetaReleaseGate.jsscripts/prepareEvaosDesktopBridgePythonRuntime.shscripts/prepareEvaosDesktopBridgeResource.jsscripts/verify-release-assets.shtests/unit/bootstrap/afterPackBundledResources.test.tstests/unit/process/evaosBetaReleaseGate.test.tstests/unit/process/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Name utility files using camelCase, such as formatDate.ts.
Prefix unused parameters with_.
Format code with Oxfmt using Prettier-compatible rules: inline single-element arrays that fit on one line, require trailing commas in multiline arrays and objects, and use single quotes for strings.
Files:
scripts/afterSign.jstests/unit/bootstrap/afterPackBundledResources.test.tsscripts/prepareEvaosDesktopBridgeResource.jsscripts/evaosBetaReleaseGate.jstests/unit/process/prepareEvaosDesktopBridgeResource.test.tstests/unit/process/evaosBetaReleaseGate.test.tsscripts/afterPack.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript; do not useanyand do not leave implicit returns.
Use the path aliases@/*,@process/*, and@renderer/*.
Prefertypeoverinterfaceaccording to the Oxlint configuration.
Write code comments in English and use JSDoc for public functions.
Files:
tests/unit/bootstrap/afterPackBundledResources.test.tstests/unit/process/prepareEvaosDesktopBridgeResource.test.tstests/unit/process/evaosBetaReleaseGate.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for tests and maintain at least 80% coverage.
Files:
tests/unit/bootstrap/afterPackBundledResources.test.tstests/unit/process/prepareEvaosDesktopBridgeResource.test.tstests/unit/process/evaosBetaReleaseGate.test.ts
🪛 ast-grep (0.44.1)
scripts/afterSign.js
[warning] 164-164: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, { encoding: null, flag: 'r' })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[error] 248-248: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(directory, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
tests/unit/process/evaosBetaReleaseGate.test.ts
[warning] 540-540: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(pythonDylibPath, Buffer.from('cffaedfe0c000001', 'hex'))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
scripts/afterPack.js
[warning] 197-197: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🪛 zizmor (1.26.1)
.github/workflows/_build-reusable.yml
[warning] 678-678: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (18)
tests/unit/bootstrap/afterPackBundledResources.test.ts (1)
2-11: LGTM!Also applies to: 40-106, 221-230, 258-277
tests/unit/process/evaosBetaReleaseGate.test.ts (2)
6-6: LGTM!Also applies to: 138-171, 436-468
539-542: 🎯 Functional CorrectnessNo issue:
writeMachOFixturealready creates thepython/bindirectory.
writeMachOFixture(pythonPath)callsfs.mkdirSync(path.dirname(filePath), { recursive: true }), soBridge/python/binexists beforepython3.12is written. The extramkdirSyncbefore line 539 is unnecessary.> Likely an incorrect or invalid review comment.tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts (1)
1-1: LGTM!Also applies to: 52-100
.github/workflows/_build-reusable.yml (1)
47-50: LGTM!.github/workflows/workbench-functional-smoke.yml (1)
44-47: LGTM!Also applies to: 156-161, 201-212, 226-227, 257-264
scripts/prepareEvaosDesktopBridgePythonRuntime.sh (1)
1-43: LGTM!Also applies to: 49-83
scripts/prepareEvaosDesktopBridgeResource.js (1)
15-15: LGTM!Also applies to: 171-179, 268-300, 302-385, 566-566, 587-588, 610-610
scripts/afterPack.js (2)
181-203: LGTM!Also applies to: 204-213
299-302: LGTM!Also applies to: 374-374, 529-529
scripts/afterSign.js (3)
162-180: LGTM!
207-216: LGTM!
240-264: LGTM!scripts/create-mock-release-artifacts.sh (2)
45-73: LGTM!Also applies to: 75-86
97-107: LGTM!scripts/evaosBetaReleaseGate.js (1)
93-109: LGTM!Also applies to: 478-478, 1037-1092
scripts/verify-release-assets.sh (1)
230-250: LGTM!CHANGELOG.md (1)
20-25: LGTM!
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
evaOS review status: stale headPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #675 Details: Superseded by a newer PR head. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
evaOS review status: completedPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #675 Review URL: #675 (review) |
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #675 - P0: bundle self-contained Mac connector runtime
Head: 263ce4426d72fe0b66d3094fffdf8f7f6986aab9 into evaos/beta-rc-20260612. Review event: REQUEST_CHANGES.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~70 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/_build-reusable.yml |
modified | +13/-0 | Changed file | Low |
.github/workflows/workbench-functional-smoke.yml |
modified | +24/-0 | Changed file | Low |
CHANGELOG.md |
modified | +6/-0 | Documentation | Low |
scripts/afterPack.js |
modified | +89/-2 | Changed file | Elevated: validated P1 finding |
scripts/afterSign.js |
modified | +43/-4 | Changed file | Moderate: validated P2 finding |
scripts/build-with-builder.js |
modified | +33/-2 | Changed file | Low |
scripts/create-mock-release-artifacts.sh |
modified | +51/-3 | Changed file | Moderate: validated P3 finding |
scripts/dmgRetryEligibility.js |
added | +27/-0 | Changed file | Low |
scripts/evaosBetaReleaseGate.js |
modified | +154/-3 | Changed file | Low |
scripts/prepareEvaosDesktopBridgePythonRuntime.sh |
added | +84/-0 | Changed file | Moderate: validated P2 finding |
scripts/prepareEvaosDesktopBridgeResource.js |
modified | +95/-16 | Changed file | Low |
scripts/verify-release-assets.sh |
modified | +21/-0 | Changed file | Low |
tests/fixtures/licenses/CPython-3.12.13-LICENSE.txt |
added | +279/-0 | Changed file | Elevated: large change |
tests/unit/bootstrap/afterPackBundledResources.test.ts |
modified | +101/-5 | Test coverage | Low |
tests/unit/bootstrap/buildWithBuilder.test.ts |
modified | +1/-0 | Test coverage | Low |
tests/unit/bootstrap/dmgRetryEligibility.test.ts |
added | +30/-0 | Test coverage | Low |
tests/unit/process/evaosBetaReleaseGate.test.ts |
modified | +77/-3 | Test coverage | Low |
tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts |
modified | +54/-2 | Test coverage | Low |
Review Signal
Validated inline findings: 5 (P0: 0, P1: 1, P2: 3, P3: 1).
Dropped findings before posting: 0. High-severity findings: 1.
Risk Taxonomy
- Dependency: 1
- Release regression: 4
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Do not ask for broad local suites when remote CI or fast-smoke proof is the right gate.
Profile proof expectations: Look for focused app smoke, packaged resource checks, or CI artifact proof when relevant.
Related Context
Related issues/PRs: #655, #653, #654, #656, #480, #623.
Suggested labels: bug, docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
apps/eva-desktop-mac/**- Check macOS identity, helper path, TCC identity, and packaged resource shape risk. - Path instructions:
scripts/**- Treat release, packaging, and artifact-shape changes as high risk. - Label suggestions: workbench, macos, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review Please review exact current head d379f29 after the architecture-closure fixes. Focused validation is 82/82, and all prior review threads have terminal dispositions. |
|
✅ Action performedReview finished.
|
coderabbitai
Bot
removed
the
public-beta
evaOS review status: completedPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #675 Review URL: #675 (review) |
|
evaOS Code Review Bot queued a current-head review for Command: Safety boundary: command handling cannot approve, merge, repair, push branches, or expand repo permissions. |
There was a problem hiding this comment.
Walkthrough
PR: #675 - P0: bundle self-contained Mac connector runtime
Head: c37a837a69a47cf9e63c7c8d21f8236caa0129ef into evaos/beta-rc-20260612. Review event: REQUEST_CHANGES.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~70 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/_build-reusable.yml |
modified | +13/-0 | Changed file | Low |
.github/workflows/workbench-functional-smoke.yml |
modified | +33/-0 | Changed file | Moderate: validated P2 finding |
CHANGELOG.md |
modified | +6/-0 | Documentation | Low |
scripts/afterPack.js |
modified | +190/-3 | Changed file | Moderate: validated P2 finding |
scripts/afterSign.js |
modified | +67/-11 | Changed file | Moderate: validated P2 finding |
scripts/build-with-builder.js |
modified | +49/-3 | Changed file | Elevated: validated P1 finding |
scripts/create-mock-release-artifacts.sh |
modified | +52/-3 | Changed file | Low |
scripts/dmgRetryEligibility.js |
added | +63/-0 | Changed file | Moderate: validated P2 finding |
scripts/evaosBetaReleaseGate.js |
modified | +188/-6 | Changed file | Low |
scripts/prepareEvaosDesktopBridgePythonRuntime.sh |
added | +114/-0 | Changed file | Moderate: validated P2 finding |
scripts/prepareEvaosDesktopBridgeResource.js |
modified | +105/-17 | Changed file | Low |
scripts/verify-release-assets.sh |
modified | +23/-0 | Changed file | Low |
tests/fixtures/licenses/CPython-3.12.13-LICENSE.txt |
added | +279/-0 | Changed file | Elevated: large change |
tests/unit/bootstrap/afterPackBundledResources.test.ts |
modified | +183/-6 | Test coverage | Low |
tests/unit/bootstrap/buildWithBuilder.test.ts |
modified | +102/-2 | Test coverage | Low |
tests/unit/bootstrap/dmgRetryEligibility.test.ts |
added | +79/-0 | Test coverage | Low |
tests/unit/process/evaosBetaReleaseGate.test.ts |
modified | +185/-4 | Test coverage | Low |
tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts |
modified | +60/-6 | Test coverage | Low |
Review Signal
Validated inline findings: 6 (P0: 0, P1: 1, P2: 5, P3: 0).
Dropped findings before posting: 1. High-severity findings: 1.
Risk Taxonomy
- Flaky test risk: 2
- Release regression: 1
- Runtime correctness: 3
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Do not ask for broad local suites when remote CI or fast-smoke proof is the right gate.
Profile proof expectations: Look for focused app smoke, packaged resource checks, or CI artifact proof when relevant.
Related Context
Related issues/PRs: #655, #653, #654, #656, #480, #623.
Suggested labels: bug, docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
apps/eva-desktop-mac/**- Check macOS identity, helper path, TCC identity, and packaged resource shape risk. - Path instructions:
scripts/**- Treat release, packaging, and artifact-shape changes as high risk. - Label suggestions: workbench, macos, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@evaos-code-review-bot review Please review exact current head |
|
@coderabbitai review Please review exact current head |
evaOS review status: stale headPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #675 |
|
✅ Action performedReview finished.
|
|
Exact-head reviewer-set record for |
|
evaOS Code Review Bot queued a current-head review for Command: Safety boundary: command handling cannot approve, merge, repair, push branches, or expand repo permissions. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/workbench-functional-smoke.yml (1)
210-221: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHash-verify the packaged CPython license.
The smoke only checks that
CPython-LICENSE.txtexists. Verify its bytes against the exported pin so packaging-time license corruption cannot pass this end-to-end proof.Proposed fix
test -f "$BRIDGE_PYTHON_LICENSE" +printf '%s %s\n' "$PYTHON_RUNTIME_LICENSE_SHA256" "$BRIDGE_PYTHON_LICENSE" | + shasum -a 256 -c -Also applies to: 266-273
🤖 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 @.github/workflows/workbench-functional-smoke.yml around lines 210 - 221, Update the packaged-artifact checks in the workflow around BRIDGE_PYTHON_LICENSE to verify the file’s SHA-256 bytes against the exported CPython license hash pin, while retaining the existing existence check. Apply the same hash verification in the additional corresponding check block, using the workflow’s established hash-verification mechanism and pin symbol.
🤖 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 1767-1771: Update the pythonLauncherValid assignment in the
generated launcher validation to accept only a symbolic link whose target bytes
are exactly b"python3.12". Remove the regular-file permission and macho_has_arch
acceptance so release validation matches package-time verification and preserves
relocatability.
In `@scripts/prepareEvaosDesktopBridgeResource.js`:
- Around line 208-213: Update the runtime inventory traversal around entryPath
and metadata so directories are recorded as inventory entries as well as
traversed, while preserving recursive processing of their contents. Adjust the
inventory test and expected entry count to include directory entries, including
the corresponding logic at the second affected location.
In `@tests/unit/bootstrap/afterPackBundledResources.test.ts`:
- Around line 14-20: Replace the deep relative script imports with the
configured `@/`* aliases in both tests: update the import used by bridgeResource
in tests/unit/bootstrap/afterPackBundledResources.test.ts (lines 14-20) and the
dmgRetryEligibility script import in
tests/unit/bootstrap/dmgRetryEligibility.test.ts (lines 12-21), preserving the
existing imported symbols and behavior.
In `@tests/unit/process/evaosBetaReleaseGate.test.ts`:
- Line 256: Update the Python fixture string in the test case so the f-string’s
nested path access uses single quotes or escaped double quotes, keeping the
generated expression valid on Python versions before 3.12.
---
Outside diff comments:
In @.github/workflows/workbench-functional-smoke.yml:
- Around line 210-221: Update the packaged-artifact checks in the workflow
around BRIDGE_PYTHON_LICENSE to verify the file’s SHA-256 bytes against the
exported CPython license hash pin, while retaining the existing existence check.
Apply the same hash verification in the additional corresponding check block,
using the workflow’s established hash-verification mechanism and pin symbol.
🪄 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: a7fd6a94-c4d0-4486-b2cb-192684e60323
📒 Files selected for processing (16)
.github/workflows/workbench-functional-smoke.yml.gitignorepackages/desktop/electron-builder.ymlscripts/afterPack.jsscripts/afterSign.jsscripts/build-with-builder.jsscripts/create-mock-release-artifacts.shscripts/dmgRetryEligibility.jsscripts/evaosBetaReleaseGate.jsscripts/prepareEvaosDesktopBridgePythonRuntime.shscripts/prepareEvaosDesktopBridgeResource.jstests/unit/bootstrap/afterPackBundledResources.test.tstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/dmgRetryEligibility.test.tstests/unit/process/evaosBetaReleaseGate.test.tstests/unit/process/prepareEvaosDesktopBridgeResource.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Name utility files using camelCase, such as formatDate.ts.
Prefix unused parameters with_.
Format code with Oxfmt using Prettier-compatible rules: inline single-element arrays that fit on one line, require trailing commas in multiline arrays and objects, and use single quotes for strings.
Files:
scripts/dmgRetryEligibility.jstests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/dmgRetryEligibility.test.tsscripts/evaosBetaReleaseGate.jsscripts/afterSign.jstests/unit/process/prepareEvaosDesktopBridgeResource.test.tstests/unit/process/evaosBetaReleaseGate.test.tsscripts/afterPack.jsscripts/prepareEvaosDesktopBridgeResource.jsscripts/build-with-builder.jstests/unit/bootstrap/afterPackBundledResources.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript; do not useanyand do not leave implicit returns.
Use the path aliases@/*,@process/*, and@renderer/*.
Prefertypeoverinterfaceaccording to the Oxlint configuration.
Write code comments in English and use JSDoc for public functions.
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/dmgRetryEligibility.test.tstests/unit/process/prepareEvaosDesktopBridgeResource.test.tstests/unit/process/evaosBetaReleaseGate.test.tstests/unit/bootstrap/afterPackBundledResources.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest 4 for tests and maintain at least 80% coverage.
Files:
tests/unit/bootstrap/buildWithBuilder.test.tstests/unit/bootstrap/dmgRetryEligibility.test.tstests/unit/process/prepareEvaosDesktopBridgeResource.test.tstests/unit/process/evaosBetaReleaseGate.test.tstests/unit/bootstrap/afterPackBundledResources.test.ts
🪛 ast-grep (0.44.1)
scripts/dmgRetryEligibility.js
[error] 44-44: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(outDir, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 15-15: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(markerPath(appOutDir, AFTER_PACK_MARKER), 'verified\n', { encoding: 'utf8', mode: 0o600 })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 27-27: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(markerPath(appOutDir, AFTER_SIGN_MARKER), 'verified\n', { encoding: 'utf8', mode: 0o600 })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/prepareEvaosDesktopBridgeResource.js
[error] 201-201: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(directory, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 224-224: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(entryPath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 259-259: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(inventoryPath, inventoryBytes)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 281-281: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(inventoryPath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
scripts/build-with-builder.js
[warning] 340-340: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(envFile, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🔇 Additional comments (16)
.github/workflows/workbench-functional-smoke.yml (1)
73-81: LGTM!Also applies to: 165-170, 235-236
scripts/prepareEvaosDesktopBridgePythonRuntime.sh (1)
1-114: LGTM!tests/unit/bootstrap/buildWithBuilder.test.ts (1)
10-135: LGTM!Also applies to: 219-226, 245-342
scripts/prepareEvaosDesktopBridgeResource.js (1)
9-19: LGTM!Also applies to: 190-193, 214-247, 268-306, 400-425, 429-514, 695-695, 716-717, 739-748
packages/desktop/electron-builder.yml (1)
117-118: LGTM!.gitignore (1)
206-208: LGTM!tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts (1)
1-51: LGTM!Also applies to: 76-181
scripts/afterPack.js (1)
13-14: LGTM!Also applies to: 157-165, 202-315, 347-417, 457-457, 490-496, 641-648
scripts/build-with-builder.js (1)
18-18: LGTM!Also applies to: 303-304, 339-405, 629-664, 751-751, 782-782
scripts/afterSign.js (1)
10-10: LGTM!Also applies to: 27-36, 172-190, 217-226, 249-274, 522-544, 585-593
scripts/dmgRetryEligibility.js (1)
5-74: LGTM!tests/unit/bootstrap/afterPackBundledResources.test.ts (1)
2-13: LGTM!Also applies to: 49-130, 236-405
scripts/evaosBetaReleaseGate.js (1)
93-115: LGTM!Also applies to: 463-470, 488-488, 1047-1123, 1635-1766, 1772-1902
scripts/create-mock-release-artifacts.sh (1)
45-176: LGTM!tests/unit/bootstrap/dmgRetryEligibility.test.ts (1)
23-114: LGTM!tests/unit/process/evaosBetaReleaseGate.test.ts (1)
6-6: LGTM!Also applies to: 124-255, 257-293, 539-551, 553-629, 652-699, 1633-1763
There was a problem hiding this comment.
Walkthrough
PR: #675 - P0: bundle self-contained Mac connector runtime
Head: 898e229ec3023351ef96ca026e343b719b1f6ae7 into evaos/beta-rc-20260612. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~70 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/_build-reusable.yml |
modified | +13/-0 | Changed file | Low |
.github/workflows/workbench-functional-smoke.yml |
modified | +33/-0 | Changed file | Low |
.gitignore |
modified | +1/-0 | Changed file | Low |
CHANGELOG.md |
modified | +6/-0 | Documentation | Low |
scripts/afterPack.js |
modified | +192/-3 | Changed file | Moderate: validated P3 finding |
scripts/afterSign.js |
modified | +67/-11 | Changed file | Moderate: validated P2 finding |
scripts/build-with-builder.js |
modified | +95/-15 | Changed file | Moderate: validated P3 finding |
scripts/create-mock-release-artifacts.sh |
modified | +96/-3 | Changed file | Low |
scripts/dmgRetryEligibility.js |
added | +75/-0 | Changed file | Moderate: validated P3 finding |
scripts/evaosBetaReleaseGate.js |
modified | +298/-17 | Changed file | Elevated: large change |
scripts/prepareEvaosDesktopBridgePythonRuntime.sh |
added | +114/-0 | Changed file | Moderate: validated P3 finding |
scripts/prepareEvaosDesktopBridgeResource.js |
modified | +228/-18 | Changed file | Elevated: large change |
scripts/verify-release-assets.sh |
modified | +23/-0 | Changed file | Low |
tests/fixtures/licenses/CPython-3.12.13-LICENSE.txt |
added | +279/-0 | Changed file | Elevated: large change |
tests/unit/bootstrap/afterPackBundledResources.test.ts |
modified | +213/-6 | Test coverage | Elevated: large change |
tests/unit/bootstrap/buildWithBuilder.test.ts |
modified | +222/-2 | Test coverage | Elevated: large change |
tests/unit/bootstrap/dmgRetryEligibility.test.ts |
added | +114/-0 | Test coverage | Low |
tests/unit/process/evaosBetaReleaseGate.test.ts |
modified | +292/-10 | Test coverage | Elevated: large change |
tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts |
modified | +129/-6 | Test coverage | Low |
Review Signal
Validated inline findings: 5 (P0: 0, P1: 0, P2: 1, P3: 4).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Release regression: 2
- Runtime correctness: 2
- Security boundary: 1
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Do not ask for broad local suites when remote CI or fast-smoke proof is the right gate.
Profile proof expectations: Look for focused app smoke, packaged resource checks, or CI artifact proof when relevant.
Related Context
Related issues/PRs: #655, #653, #654, #656, #480, #623.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
apps/eva-desktop-mac/**- Check macOS identity, helper path, TCC identity, and packaged resource shape risk. - Path instructions:
scripts/**- Treat release, packaging, and artifact-shape changes as high risk. - Label suggestions: workbench, macos, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@evaos-code-review-bot review Please review exact current head |
evaOS review status: completedPR: #675 - P0: bundle self-contained Mac connector runtime evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #675 Review URL: #675 (review) |
|
evaOS Code Review Bot queued a current-head review for Command: Safety boundary: command handling cannot approve, merge, repair, push branches, or expand repo permissions. |
There was a problem hiding this comment.
Walkthrough
PR: #675 - P0: bundle self-contained Mac connector runtime
Head: 81cf9306677c7135c4816630759de88dec5f0a50 into evaos/beta-rc-20260612. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~70 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/_build-reusable.yml |
modified | +13/-0 | Changed file | Low |
.github/workflows/workbench-functional-smoke.yml |
modified | +33/-0 | Changed file | Moderate: validated P3 finding |
.gitignore |
modified | +1/-0 | Changed file | Low |
CHANGELOG.md |
modified | +6/-0 | Documentation | Low |
scripts/afterPack.js |
modified | +192/-3 | Changed file | Moderate: validated P2 finding |
scripts/afterSign.js |
modified | +67/-11 | Changed file | Low |
scripts/build-with-builder.js |
modified | +115/-15 | Changed file | Moderate: validated P3 finding |
scripts/create-mock-release-artifacts.sh |
modified | +102/-3 | Changed file | Low |
scripts/dmgRetryEligibility.js |
added | +75/-0 | Changed file | Low |
scripts/evaosBetaReleaseGate.js |
modified | +320/-17 | Changed file | Elevated: large change |
scripts/prepareEvaosDesktopBridgePythonRuntime.sh |
added | +117/-0 | Changed file | Low |
scripts/prepareEvaosDesktopBridgeResource.js |
modified | +245/-18 | Changed file | Elevated: large change |
scripts/verify-release-assets.sh |
modified | +23/-0 | Changed file | Low |
tests/fixtures/licenses/CPython-3.12.13-LICENSE.txt |
added | +279/-0 | Changed file | Elevated: large change |
tests/unit/bootstrap/afterPackBundledResources.test.ts |
modified | +213/-6 | Test coverage | Elevated: large change |
tests/unit/bootstrap/buildWithBuilder.test.ts |
modified | +304/-2 | Test coverage | Elevated: large change |
tests/unit/bootstrap/dmgRetryEligibility.test.ts |
added | +114/-0 | Test coverage | Low |
tests/unit/process/evaosBetaReleaseGate.test.ts |
modified | +341/-10 | Test coverage | Elevated: large change |
tests/unit/process/prepareEvaosDesktopBridgeResource.test.ts |
modified | +196/-6 | Test coverage | Elevated: large change |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 1, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- CI/build: 1
- Release regression: 2
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Do not ask for broad local suites when remote CI or fast-smoke proof is the right gate.
Profile proof expectations: Look for focused app smoke, packaged resource checks, or CI artifact proof when relevant.
Related Context
Related issues/PRs: #655, #653, #654, #656, #480, #623.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
apps/eva-desktop-mac/**- Check macOS identity, helper path, TCC identity, and packaged resource shape risk. - Path instructions:
scripts/**- Treat release, packaging, and artifact-shape changes as high risk. - Label suggestions: workbench, macos, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
|
Exact-head reviewer-set record for |
2 participants
Summary
Linked Work
Type Of Change
Root Cause
Prepared Macs masked a packaging dependency: the connector launcher searched external Python paths. A pristine Mac could report setup progress while lacking the runtime needed for end-to-end control. The release pipeline also had false-green edges: multi-architecture packaging could reuse one Bridge tree, stale afterPack markers could survive between attempts, and the single-app DMG retry could repackage an incomplete bundle. The release ZIP verifier did not bind the full bundled Python closure.
The PR removes those assumptions. Each Mac architecture receives its own bundled Bridge/Python runtime, afterPack verifies the exact pre-sign inventory, afterSign verifies signed Mach-O identity, and the release gate binds one app root plus the complete runtime path/type/mode/digest or signed-Mach-O architecture closure.
Validation
Exact head:
81cf9306677c7135c4816630759de88dec5f0a504d26e163f86908b5545eddc11cf8a9bb749872360423fe6491aa2df823849b2c; all directories are owner-readable/executable and no dangling symlink was acceptedPreviewPanel.domimport tests hit their 10-second timeout under full-suite load, and that exact file passed 3/3 alonegit diff --checkpassed9e3b7332a88fbdea22291923bfd10dd37494d92d: https://github.com/100yenadmin/evaOS-GUI/actions/runs/29236681600Release Notes And Proof Boundary
Workbench now carries the Python/PyObjC runtime needed by the Mac connector, so pristine customer setup no longer depends on Homebrew or a separately installed Python stack. This does not redesign Mac Control: CUA remains primary and Peekaboo remains fallback.
This PR proves source and packaging contracts only. Customer readiness still requires the strict signed/notarized release artifact, updater ZIP proof, installed-app functional smoke, authenticated dashboard launch, live broker and direct Mac-control proof, revoke/kill-switch proof, and pristine-Mac onboarding proof on the same candidate.
Next Release Action
After merge, dispatch the strict macOS arm64 Build and Release workflow from the exact beta merge SHA. Reject any run whose head SHA differs. Do not publish customer-ready claims until signed installed-app and live broker/Mac-control gates pass.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation