feat: support Codex sessions over SSH remote bridge - #628
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCodex hook payloads now carry remote-session status through bridge session events. Remote setup configures Codex hooks and documents SSH behavior. Selected application and core tests migrate from XCTest to Swift Testing. ChangesCodex remote-session support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Codex
participant CodexHookPayload
participant BridgeServer
participant SessionStartEvent
Codex->>CodexHookPayload: Send session-start payload
CodexHookPayload->>BridgeServer: Decode remote flag
BridgeServer->>SessionStartEvent: Set isRemote
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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/remote-setup.sh`:
- Around line 115-116: Update the Codex event handling in
scripts/open-island-hooks.py so send_command uses a 3600-second timeout when
hook_event_name is PermissionRequest, while retaining the existing 45-second
timeout for all other events.
In `@Tests/OpenIslandAppTests/TerminalJumpServiceTests.swift`:
- Around line 56-62: Update
testGhosttyJumpIntegrationMatchesFocusedTerminalForLiveSurfaces so an enabled
integration run does not silently pass when liveGhosttyTerminals() returns no
terminals. Use a supported test-skip mechanism if available; otherwise record an
explicit test failure while preserving the existing behavior when terminals are
present.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2508795f-308a-4435-9069-c8acf3c5bdba
📒 Files selected for processing (11)
Sources/OpenIslandApp/Views/SettingsView.swiftSources/OpenIslandCore/BridgeServer.swiftSources/OpenIslandCore/CodexHooks.swiftTests/OpenIslandAppTests/ForegroundTerminalSessionProbeTests.swiftTests/OpenIslandAppTests/KeystrokeInjectorTests.swiftTests/OpenIslandAppTests/TerminalJumpServiceTests.swiftTests/OpenIslandCoreTests/CodexRemoteSessionTests.swiftTests/OpenIslandCoreTests/OpenCodeSessionRegistryTests.swiftdocs/hooks.mddocs/ssh-setup.mdscripts/remote-setup.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/remote-setup.sh (1)
174-218: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTighten the
[features]table-header detection to avoid corruptingconfig.tomlon nested arrays.The section-boundary check at Line 194 (
stripped.startswith("[") and stripped.endswith("]")) can misfire on a bracketed line inside a multi-line array that has no trailing comma, for example the last row of a nested array like[3, 4]right before the array's closing]. If such a line appears inside the[features]table, the script exitsin_featuresprematurely and can inserthooks = truein the middle of that array, corrupting the file.This is a narrow edge case for a
[features]table (which today likely only holds simple booleans), and a full switch to a TOML library isn't a clean option here:tomllibis stdlib only from Python 3.11 and cannot write TOML, and style-preserving editing of an existing file would need the third-partytomlkitpackage, which conflicts with the script's stated "Python 3.6+, no guaranteed extra packages" remote prerequisite. A small heuristic tightening lowers the risk without adding a dependency:🛡️ Proposed heuristic tightening
+import re + +TABLE_HEADER_RE = re.compile(r'^\[+[^\[\]="]+\]+$') + for line in lines: stripped = line.strip() if stripped == "[features]": in_features = True has_features = True out.append(line) continue - if in_features and stripped.startswith("[") and stripped.endswith("]"): + if in_features and TABLE_HEADER_RE.match(stripped): if not hooks_seen: out.append("hooks = true") in_features = False out.append(line) continue🤖 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/remote-setup.sh` around lines 174 - 218, tighten the section-boundary detection in the remote setup script’s features-processing loop so array elements such as bracketed rows inside multi-line TOML arrays are not treated as table headers. Recognize only valid single-table headers, excluding nested-array syntax and bracketed array values, while preserving insertion and replacement of hooks within the actual [features] table.
🤖 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/remote-setup.sh`:
- Around line 147-152: Update the Python hook client’s send_command timeout
selection so hook_event_name == PermissionRequest uses 3600 seconds, while all
other Codex events retain the existing 45-second timeout. Locate the change in
the send_command flow of open-island-hooks.py and preserve the existing event
handling.
- Around line 34-38: Update the REMOTE_UID resolution command to remove the
`2>/dev/null` stderr suppression from the `ssh "$REMOTE" "id -u"` probe, while
preserving the existing output normalization and numeric validation in the
surrounding block.
---
Nitpick comments:
In `@scripts/remote-setup.sh`:
- Around line 174-218: tighten the section-boundary detection in the remote
setup script’s features-processing loop so array elements such as bracketed rows
inside multi-line TOML arrays are not treated as table headers. Recognize only
valid single-table headers, excluding nested-array syntax and bracketed array
values, while preserving insertion and replacement of hooks within the actual
[features] table.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2968c576-de23-4b2f-8d99-079455c36ba6
📒 Files selected for processing (2)
docs/ssh-setup.mdscripts/remote-setup.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/ssh-setup.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/remote-setup.sh (2)
204-233: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winParse TOML table headers with trailing comments.
The updater detects
[features]by exact string match and treats only a line ending with"]"as a section boundary. Valid comments after table headers are not part of the header, so[features] # existing commentcan create a duplicate[features]section and commented headers can let the next detected truehookswriting stay in the wrong section. Parse table headers by stripping trailing comments, or use a TOML parser.🤖 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/remote-setup.sh` around lines 204 - 233, The configuration updater’s section detection in the shown loop fails for TOML table headers with trailing comments. Normalize each line by removing trailing comments before checking for “[features]” or identifying section boundaries, while preserving the original line in out; ensure hooks = true is inserted into the existing features section and not duplicated or carried into a later section.
74-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve mixed hook groups and identify managed hooks by exact command.
The current cleanup removes any hook group whose command text contains
open-island-hooks.py. If a user adds another hook to that group, rerunning setup removes the whole group. Remove only matching Open Island hook entries, keep the group when other hooks remain, and use exact command identity from the installer-managed commands rather than a filename substring.🤖 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/remote-setup.sh` around lines 74 - 82, Update group_has_open_island and the cleanup logic in scripts/remote-setup.sh to match only the exact installer-managed Open Island commands, not filename substrings. Remove matching hook entries individually, preserve any non-managed hooks, and retain the group when other hooks remain.
🤖 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.
Outside diff comments:
In `@scripts/remote-setup.sh`:
- Around line 204-233: The configuration updater’s section detection in the
shown loop fails for TOML table headers with trailing comments. Normalize each
line by removing trailing comments before checking for “[features]” or
identifying section boundaries, while preserving the original line in out;
ensure hooks = true is inserted into the existing features section and not
duplicated or carried into a later section.
- Around line 74-82: Update group_has_open_island and the cleanup logic in
scripts/remote-setup.sh to match only the exact installer-managed Open Island
commands, not filename substrings. Remove matching hook entries individually,
preserve any non-managed hooks, and retain the group when other hooks remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 785e19f6-998a-4770-a4bf-02c438b07494
📒 Files selected for processing (1)
scripts/remote-setup.sh
2 participants
Summary
Extends the SSH remote bridge so Codex sessions running on a remote server (including via Codex Desktop's SSH remote /
codex app-server) appear in Open Island, matching the existing Claude Code remote flow.Changes
feat: support Codex sessions over SSH remote bridge(a1ac4a7)scripts/open-island-hooks.py) supports--source codexCodexHookPayloadcarries aremoteflag set by the Python hook clientSessionStart/ensureSessionExistssessions are markedisRemotescripts/remote-setup.shdeploys and merges Codex hooks on the remotetest: migrate remaining XCTest suites to Swift Testing(ef69115)docs: note hook trust timing and tunnel ownership for Codex SSH remote(7cf872f)Verification
codex execand Codex Desktop SSH remote sessions create sessions in Open Island (verified end-to-end over theRemoteForwardUnix socket tunnel)acknowledgedbridge responses; session persisted insession-terminals.jsonSummary by CodeRabbit
New Features
Documentation