Record remote mobile host boundary review#93
Conversation
📝 WalkthroughWalkthroughAdds maintainer documentation that defines a host-state evidence matrix for remote-control and Codex mobile review, updates related docs to reference it, and adds two integration tests validating enablement-bridge auto-connect behavior for missing identity and empty connection-snapshot cases. ChangesIssue
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
@CodeRabbit-ai review |
|
✅ Actions performedReview triggered.
|
|
| Filename | Overview |
|---|---|
| port-integrations/remote-mobile-control/test.js | Adds two new integration tests covering the "no local identity → leave all hosts disconnected" and "empty snapshot → refresh before auto-connect" branches; previous review gaps (missing enabled and key assertions) are now addressed in these tests. |
| docs/maintainers/remote-mobile-host-boundary-review.md | New durable evidence surface for issue #59; clearly separates repo-local test coverage from live evidence requirements and scopes security review assertions appropriately. |
| docs/maintainers/security-backlog.md | Adds a cross-reference from the issue #59 backlog entry to the new host-boundary review doc; no logic changes. |
| docs/maintainers/threat-model.md | Expands the "Evidence" and "Gaps" paragraphs to reflect the new test coverage and adds a link to the new boundary review doc; prose is accurate and consistent with the test additions. |
| docs/README.md | Index entry added for the new boundary review doc; no issues. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[set-remote-control-connections-enabled\nenabled: true] --> B{remoteControlConnections\nempty?}
B -- No --> C[get-global-state\nkey: electron-local-remote-control-installation-id]
B -- Yes --> D[refresh-remote-control-connections]
D --> E[Use local_remote_control_installation_id\nfrom sharedObjects]
C --> F{localId\npresent?}
E --> G[Match each connection\ninstallation_id vs localId]
F -- No --> H[set-remote-connection-auto-connect\nautoConnect: false for ALL hosts]
F -- Yes --> I[Match each connection\ninstallationId vs localId]
I --> J{installationId\n== localId?}
G --> K{installation_id\n== localId?}
J -- Yes --> L[autoConnect: true]
J -- No --> M[autoConnect: false]
K -- Yes --> N[autoConnect: true]
K -- No --> O[autoConnect: false]
Reviews (4): Last reviewed commit: "test(remote-mobile): normalize refresh p..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@port-integrations/remote-mobile-control/test.js`:
- Around line 1283-1284: Replace the two-line check of calls[1].params with a
single idiomatic assertion that verifies it is exactly an empty object;
specifically, remove the assert.equal(calls[1].params != null, true) and
assert.equal(Object.keys(calls[1].params).length, 0) and use a single
deep/object equality assertion (e.g., assert.deepStrictEqual or
assert.deepEqual) against {} for calls[1].params so the test fails if params is
null, undefined, an array, or any non-empty object.
🪄 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 Plus
Run ID: 85f198ac-90d8-4fab-b0a5-28509c0f1021
📒 Files selected for processing (1)
port-integrations/remote-mobile-control/test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@port-integrations/remote-mobile-control/test.js`:
- Line 1283: The assertion uses an unnecessary structuredClone wrapper; update
the test to call assert.deepEqual directly on the params object (replace
assert.deepEqual(structuredClone(calls[1].params), {}); with
assert.deepEqual(calls[1].params, {});) so the deep comparison is idiomatic and
avoids an unneeded clone of calls[1].params.
🪄 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 Plus
Run ID: ac0fcf7e-1846-4813-9798-e86a6282416c
📒 Files selected for processing (1)
port-integrations/remote-mobile-control/test.js
nisavid
dismissed
coderabbitai[bot]’s stale review
Dismissed as invalid after local verification: direct assert.deepEqual(calls[1].params, {}) fails for the VM-realm params object, while the structuredClone-normalized assertion preserves the exact empty-object check and node --test port-integrations/remote-mobile-control/test.js passes 49/49.
1 participant
Summary
docs/maintainers/remote-mobile-host-boundary-review.mdas the durable host-state matrix and security-review evidence surface for issue Review experimental remote-control and Codex mobile host boundary #59.Refs #59.
Current Status
Ready to merge for the repo-local evidence and tests in this PR. This does not close #59: issue closure still needs live mobile/account validation that the mobile side sees the intended thread/session and that the first mobile action reaches that live host thread/session.
Security Review
remote-control-uiis documented as Linux UI/copy exposure only, not authorization or enrollment proof.remote-mobile-controlevidence records the local boundaries for XDG software device keys, Desktop app-server ownership, standalone daemon fallback, and local-installation-only auto-connect.Verification
Local:
git fetch origin maingit rebase --autostash origin/mainnode --test port-integrations/remote-mobile-control/test.jspassed (49/49)node --test port-integrations/remote-control-ui/test.jspassed (4/4)node --test scripts/patch-linux-window-ui.test.jspassed (161/161)git diff --checkpassedgit diff --cached --checkpassed before commitsassert.deepEqual(params, {})fails for the test harness, whileassert.deepEqual(structuredClone(params), {})passesGitHub:
c8a9f73d0c451a5f24a47eea73645d9f5d0c9837Review Notes