fix(evaos): fail closed on incomplete staging signer#709
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🪛 ast-grep (0.44.1)tests/unit/evaos/evaosDesktopBridgeReceipt.test.ts[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🔇 Additional comments (2)
📝 WalkthroughWalkthroughThe bridge now rejects incomplete Mac-control canary configuration, including empty environment sets. Tests cover the mode-only staging failure, absence of plist and ChangesCanary configuration validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@codex review exact head f92d45f; report only release-relevant P0-P2 findings. @copilot review exact head f92d45f. @coderabbitai review exact head f92d45f; report only release blockers. |
evaOS review status: completedPR: #709 - fix(evaos): fail closed on incomplete staging signer evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #709 Review URL: #709 (review) |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git fetch origin f92d45f984db29c132e65f458df85567f04186ca --depth=5 2>&1 | tail -20
git checkout f92d45f984db29c132e65f458df85567f04186ca 2>&1 | tail -5
echo "---FILE---"
fd cli.py resources/evaos-beta/bridge/src/evaos_desktop_bridge/ 2>/dev/null
cat -n resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.py | sed -n '1,400p'Length of output: 23175 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n -i "staging\|signer" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.py | head -80Length of output: 319 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1750,1950p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 9446 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "CONNECTOR_MAC_CONTROL_CANARY_ENV_KEYS\|CONNECTOR_MAC_CONTROL_CANARY_MODE_ENV_KEY\|CONNECTOR_MAC_CONTROL_CANARY_MODE\b\|_mac_control_canary\|incomplete_configuration\|_write_connector_plist\|launchctl" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 4183 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '3310,3400p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.py
echo "---write_connector_plist calls---"
grep -n "_write_connector\|def _write_plist\|plistlib.dump\|plist_path.write\|def _connector_plist\|def _launchctl_start\|_connector_service_start\b" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 3675 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '3260,3315p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.py
echo "---callers of _ensure_connector_plist / start---"
grep -n "_ensure_connector_plist\|def _connector_service_status\|def _connector_service_start\|connector_service.start\|_ensure_connector_plist(" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 2735 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "_ensure_connector_user_plist" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 408 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '2100,2170p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.py
echo "---"
sed -n '2745,2762p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 3481 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "_run_public_connector_service\|except RuntimeError\|except Exception as exc" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.py | head -60Length of output: 734 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1010,1070p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 2933 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "^def entrypoint\|^def main\|def _dispatch\|def run_cli" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 337 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '766,915p' resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 6834 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd evaosDesktopBridgeReceipt.test.ts
cat -n tests/unit/evaos/evaosDesktopBridgeReceipt.test.ts | sed -n '1,200p'Length of output: 9153 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "CONNECTOR_SYSTEM_PLIST" resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pyLength of output: 434 Only read via
Verification of the fail-closed fix:
Non-staging startup paths are untouched ( No P0-P2 findings to report. ✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
added
area:native-companion
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![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: #709 - fix(evaos): fail closed on incomplete staging signer
Head: f92d45f984db29c132e65f458df85567f04186ca into evaos/beta-rc-20260612. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~12 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
tests/unit/evaos/evaosDesktopBridgeReceipt.test.ts |
modified | +15/-1 | Test coverage | Low |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
1 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.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
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: #708, #699, #707, #236, #349.
Suggested labels: 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.
评审结论✅ 通过 — 已审查精确 head f92d45f。
|
1 participant
Pull Request
Description
PR #708 merged the v2.1.36 signed Mac-control and selected-binding contracts, but a post-merge exact-head review found one P2 gap: explicit staging canary mode with none of the four signer settings could still rewrite and restart the connector LaunchAgent before failing later.
This superseding one-commit fix makes explicit staging mode require the complete signer environment before any plist write or launchctl call. Non-staging connector startup remains unchanged.
Related Issues
Type of Change
fix- Bug fix (non-breaking change which fixes an issue)feat- New feature (non-breaking change which adds functionality)perf- Performance improvementrefactor- Code restructuring (no behavior change)docs- Documentation updateAtomic PR Checklist (Rule 1)
<type>(<scope>): <subject>in EnglishLocal Checks (Rule 2)
bun run format/oxfmt --check- formatting passesbun run lint- 0 errorsbunx tsc --noEmit- passesbunx vitest run- 301 files passed, 1 skipped; 2,812 tests passed, 3 skippedgit diff --check- no whitespace errorsCoverage note: one optional full coverage run hit the pre-existing 10-second dynamic-import timeout in
PreviewPanel.dom.test.tsx; that test passed immediately in isolation (3/3) and the ordinary full suite passed twice. This PR does not touch preview code or coverage configuration.Runtime Verification
Source-level regression proof on macOS:
5c86e8e91660772da5b1b6f49b43f2de3afee737: explicit staging mode with zero signer settings returned an empty environment.f92d45f984db29c132e65f458df85567f04186ca: the same input raises a sanitized incomplete-configuration error before plist or launchctl mutation.Agent Handoff
100yenadmin/evaOS-GUIonly.resources/evaos-beta/bridge/src/evaos_desktop_bridge/cli.pytests/unit/evaos/evaosDesktopBridgeReceipt.test.ts5b1308fadc481f83116c54de2b9713ab2363bed25c86e8e91660772da5b1b6f49b43f2de3afee737f92d45f984db29c132e65f458df85567f04186cacom.evaos.workbenchand the existing bundled bridge for v2.1.36; this PR creates no standalone identity.packages/mac-access, nopackages/mac-connector-core, nocom.evaos.mac-access*identities, no standalone helper ownership/pairing, and no Workbench-as-client refactor.f92d45f984db29c132e65f458df85567f04186ca; no data migration or external state rollback.Screenshots
N/A. No UI change.
Additional Context
PR #708 is already merged. This PR exists solely to close the exact post-merge P2 without expanding into the separately owned Mac Access v0.1 implementation.
No merge, tag, release, or publication is requested by this PR.
Summary by CodeRabbit
Bug Fixes
Tests