Conversation
…xpected exits Root cause: the desktop app only started a runtime at launch and from the failure page's retry. After Ready it merely recorded its child's exit. A restart the runtime performed itself (Connect as Child, memory or package restart, the recycle after a disconnect) handed the port to a detached grandchild the app could not see, stop or quit, and one that failed to start left no proxy. A crash or a terminal `ocx stop` left the port refusing connections until the app was quit and reopened. A failed in-app update left the app in a terminal Drained phase with no runtime. Fix: - The sidecar is spawned with OCX_DESKTOP_SUPERVISED=1. handleStart consumes it with the other start markers and records the parent pid. While that app is still the parent and alive, acceptSystemRestart (completed and deadline paths) marks recycling and exits 75, and recycleStandalone exits 75 after its cleanup, instead of spawning; both detached replacement environments drop the marker. A link-mode client runtime reclaims its port for 20 s under the marker (25 s with the pinned prefer-retry, inside the app's 30 s startup deadline), and its runtime-port.json carries an attestation secret like a standalone start. - desktop/src-tauri/src/supervisor.rs: the sidecar watch reports each child's exit once it is recorded; a pure decide() respawns only for the tracked child with the coordinator idle, the runtime wanted and no ending claimed. Exit 75 goes after 0.5 s when no recovery ran in the last healthy two minutes, otherwise it takes the 3/6/12/24/30 s backoff like other exits. Recovery re-runs the startup sequence in Mode::Recover: resolve stays the only authority, only a proven absence spawns, a live runtime is attached as a guest, and no window or takeover prompt is shown. A 5 s watchdog while Ready recovers on a different pid, or on 3 refused connections in a row (12 for a guest runtime). Decisions go to a 256 KiB runtime-supervisor.log. - A Child's client runtime (resolve role=client) is attached to as a guest at launch and in recovery, never offered a takeover, and owned when it is the app's own child. Refusing it failed every recovery on a Child whose runtime restarted outside the app, forever, one bundled `ocx resolve` per 30 s. - A recovery that finds the port held by a listener the app cannot use (bound off loopback) is not rescheduled: the supervisor parks and the watchdog only asks whether the endpoint changed. - The retry guard waits on the child the app tracks (spawned under 90 s ago, no exit reported) instead of the ownership flag attach() had just reset, so a recovery no longer spawns a second sidecar beside a still-starting one. - A recovery that lands while the update page is up leaves it on screen. - exit.rs: a sticky `wanted` intent, cleared by a tray Stop that takes the phase and by any drain claim (Quit, update), restored by the failure page's retry; abort_restart() returns a coordinated restart's settled drain to Idle, and a failed install re-runs startup in Recover mode. - POST /api/stop from a dashboard session answers 409 desktop_supervised while the desktop app supervises the proxy, before anything is touched: the app would start it again after a full native-Codex teardown. `ocx stop` (tray Stop, Quit, update drain, terminal) is unaffected. Performance: nothing on the standalone or Home request path changes. The marker is read once at start; the parent check runs only at restart time, at a link-mode start and on a dashboard stop. On the desktop, the watchdog is one loopback GET /healthz every 5 s while Ready or parked, and supervision work otherwise happens only on exits and run ends. A held port no longer costs a resolve every 30 s. Security: no new network surface. Nothing is killed or signalled; a released child handle is only dropped. The marker is honored only against the live parent recorded at start, and a runtime whose app died falls back to the detached replacement. The attestation secret goes into the same private runtime-port.json a standalone start writes. The client runtime is attached on loopback only and never taken over. The supervisor log holds pids, exit codes, verdicts and startup failure reasons, never credentials, and is not written through a symlink. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
✅ Deterministic PR hygiene checks passed. |
![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: 9b5afd4f50
ℹ️ 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".
| (self.streak >= GUEST_UNREACHABLE_LIMIT) | ||
| .then_some("the listener that held the port stopped answering") | ||
| } | ||
| Probe::Unreachable => None, |
There was a problem hiding this comment.
When recovery finds a live runtime bound somewhere the desktop cannot reach (for example ::1 or an external address), Parked starts with answered == false, and every loopback probe is Unreachable. This branch ignores those probes forever, so even after the recorded holder exits and the configured port becomes free, the supervisor never runs resolution again and the desktop remains without a usable runtime until a manual retry or relaunch. Periodically re-resolve the recorded holder, or otherwise detect that it has disappeared, rather than treating permanent loopback refusal as proof that nothing changed.
Useful? React with 👍 / 👎.
| let owns_live_child = app | ||
| .try_state::<AppState>() | ||
| .is_some_and(|state| state.owns_runtime()) | ||
| .is_some_and(|state| waits_on_child(state.child_age())) | ||
| && watch.exit().is_none(); |
There was a problem hiding this comment.
Once a tracked child exceeds CHILD_START_GRACE without reporting an exit, this condition permits spawning and adopting another child while the old process may still be alive. All per-child followers share the same SidecarWatch.inner, however, so a later exit from the old child overwrites the shared exit record even though on_exit correctly rejects its stale PID. Subsequent startup waits can then fail immediately at watch.exit(), and drain_current can treat an unreachable current child as already gone, potentially leaving it unmanaged; exit state needs to be keyed to the tracked PID or the old follower must be prevented from updating the current record.
Useful? React with 👍 / 👎.
리뷰 · 우선순위 52 / 80데스크톱 앱은 켤 때와 실패 화면의 다시 시도에서만 런타임을 띄웠습니다. 그 뒤에 자식이 끝나면 기록만 남기고 포트를 다시 열지 않았습니다. 런타임이 스스로 재시작하면, 앱이 보지도 멈추지도 못하는 손자 프로세스가 포트를 가져갔습니다. 그 손자가 뜨지 못하면 프록시가 없었습니다. 터미널의 이 PR은 앱이 그 런타임을 계속 돌보게 합니다. 사이드카에 라인 - 라인 - 메인테이너의 판단이 필요한 지점 90초를 넘긴 자식을 두고 새 자식을 띄울 때, 종료 기록을 pid마다 나눌지 정하면 됩니다. 루프백 밖에 묶인 점유자가 사라진 것을 주기적으로 다시 볼지, 지금처럼 답이 올 때까지 세워 둘지도 정하면 됩니다. 대시보드 중지를 409로 막을지는 이미 이 PR의 선택입니다. 바탕은 너의 추천 앱이 죽은 런타임을 다시 띄우는 방향은 맞습니다. 종료 칸을 자식 pid에 묶기 전에는, 90초 뒤의 재시작이 새 자식을 바로 실패로 만들 수 있습니다. 루프백 밖 점유가 죽어도 감독자는 다시 보지 않습니다. 그 둘을 고치고, #5971 다음에 바탕을 이 댓글은 grok-bot이 작성했습니다 |
…ch 9E) (#5998) Carry the owner's Remote Link, restart, desktop supervision and Codex routing fixes onto dev in their original order, then carry RHODIZSECURITY's combo reasoning fix as one attributed commit. | PR | Change | Author | | --- | --- | --- | | #5970 | Turn a standalone computer into a Child from its dashboard; keep Codex on its local loopback URL and protect the linked data plane. | lidge-jun | | #5973 | Reconnect the Child's SSH tunnel after sleep, outages and crashes. | lidge-jun | | #5972 | Heal opencodex-owned Codex routing left on a dead loopback endpoint, with ownership and race gates. | lidge-jun | | #5971 | Keep a proxy on the configured port through a Child restart. | lidge-jun | | #5974 | Let the desktop app supervise runtime restarts and unexpected exits. | lidge-jun | | #5990 | Apply forced combo defaults over declared none/minimal reasoning sentinels. | RHODIZSECURITY | The carry keeps the 9D one-use sibling handoff and passes link status, cached key and tunnel gate through the Child listener. Separate integration commits bound the port-conflict regression test and keep carried files below the file-size guard, including newer dev's layout entries. The five owner commits retain JUN's authorship; the #5990 squash retains RHODIZSECURITY's commit identity and noreply co-author trailer. An independent review found four integration defects. Each repair is a separate Codex-authored commit: | Finding | Commit | Repair | | --- | --- | --- | | Linked requests could fetch without a connected tunnel. | 6a29f7b | Require a positive supervisor connected verdict before every fetch; missing, failed and stopped supervision return 503 without forwarding key or body. | | IPv4 and IPv6 destinations on one port shared one probe/streak key. | 8903998 | Probe and track each hostname and port separately; a live endpoint blocks healing and an address change starts a fresh dead-probe streak. | | A same-port route change could pass the locked write guard. | f62e43b | Abort when admitted config bytes or the complete destination set changes under the lock, then require fresh probes. | | Orphan reaping could KILL a reused PID. | e0ef426 | Record the process start identity and revalidate argv, start time and orphan status before TERM and before KILL; legacy records lacking start identity never authorize a signal. | A second review confirmed those four repairs and found two remaining blockers: | Finding | Commit | Repair | | --- | --- | --- | | A competing local listener received the readiness key and private relay traffic before SSH bound the tunnel port. | 39f246a | Require an exclusive local LISTEN socket owner PID matching the SSH child before every keyed probe and relay admission; adopted processes also need matching pidfile argv and start time. Unknown scans fail closed. | | Newer dev mappings made the merge result exceed the layout file-size guard. | 3bb2ab6, 46ee24f | Merge origin/dev at a91568e, then compact formatting while retaining every explicit mapping. | A third review confirmed the competing-listener and layout repairs, then found three lookup defects: | Finding | Commit | Repair | | --- | --- | --- | | Minimal Linux lacks lsof/netstat and never proves the SSH listener. | 9c66251 | Use tool-independent async /proc/net/tcp{,6} inode lookup, checking the expected SSH PID's fd symlinks first. | | A foreign ::1 listener shares the numeric port with the owned IPv4 forward. | 9c66251 | Match only the exact 127.0.0.1 address and port on Linux, macOS and Windows. | | Synchronous owner scans block Bun on every relayed fetch. | b5e565d | Use bounded async lookups and a one-second positive proof keyed by port, SSH PID, start identity and tunnel generation; re-prove after restart. | The branch also merged current dev at 35f267d in 51747c9. The merged test registries retain both lanes' mappings and the management contract retains Kiro's account projection and Child join. Current dev through `2a3cfa5abe` was merged again in `5856179cd1` without conflicts. It brings #6012's macOS plugin ACL fix and dev's Kiro projection test clock correction; `scripts/test-layout/layout.json` stays at 1,997 lines. The merge changes no link/relay or server-management-auth files. A fourth review found that a one-second proof cache could survive a local port takeover, and that an adopted PID's start identity was only checked at adoption: | Finding | Commit | Repair | | --- | --- | --- | | Cached ownership authorized the next keyed probe or relay after a port takeover. | b8142ad | Every keyed probe and every relayed fetch now obtains a fresh bounded asynchronous socket-owner proof; concurrent admissions do not share a cached success. | | A reused adopted PID retained its old trusted start identity. | b8142ad | Re-read current argv and start time on every adopted admission, invalidate trust and mark the link failed on mismatch. | | The TCP listener can change after the check and before connect. | bee1613 | Record this pre-existing residual race and a private Unix-domain SSH forward as future hardening in the link structure contract. | A fifth review found that transient unreadable adopted identity was treated like a confirmed replacement: | Finding | Commit | Repair | | --- | --- | --- | | One null or timed-out identity read permanently disabled a live adopted link. | 0eefebf | Return an explicit unknown verdict; deny only the current keyed admission and retry on the next check without discarding the adopted record. | | A confirmed changed identity left the old adopted PID blocking recovery. | 0eefebf | Release the stale adoption and pidfile without signalling that PID; the next supervisor tick starts its own SSH tunnel. | Windows CI follow-up: `88fbb539af` samples the relay hold clock once per attempt. The initial reconnect wait now receives the full 15-second budget even when the wall clock ticks during admission; later retries still subtract elapsed time. Windows teardown follow-up: `0171b4856c` makes `server.stop(true)` await any timed-out `icacls.exe` child still reaping after config-directory hardening settles. The stop promise now marks the actual handle-release boundary before a caller removes the home. Security review: Child join still refuses Tailscale identity, a non-standalone role and a mismatched live port before SSH. Linked data routes retain the Host/Origin gate, committed-key fingerprint, caller-credential stripping and inbound byte cap. The relay now sends no key or request without positive tunnel supervision, and the supervisor obtains a fresh bounded asynchronous exact-IPv4 owner proof before every keyed probe and relay fetch; adopted processes also have their current argv and start time checked each time. An unknown read refuses only that admission; a confirmed mismatch releases the adopted PID without signalling it. Desktop supervision stays bound to its live parent, and dashboard Stop is refused before teardown while CLI/tray Stop remains available. Routing self-heal writes only owned loopback routing after all distinct endpoints were proven dead and the locked bytes were rechecked. The existing home-bound stop proof and sibling Desktop-write gate remain intact.   Co-authored-by: RHODIZSECURITY <180237049+RHODIZSECURITY@users.noreply.github.com>
|
Landed on |
1 participant
Summary
Stacked on #5971 (
claude/child-restart-handoff). Retarget after it lands.The desktop app started a runtime only at launch and from the failure page's retry. After Ready it merely recorded its child's exit, so the runtime stayed down in several cases:
ocx stopleft the port refusing connections until the app was quit and reopened. That is what happened on 2026-09-26 at 12:07.Drainedphase with no runtime.Changes:
OCX_DESKTOP_SUPERVISED=1. While that app is the live parent:acceptSystemRestart(completed and deadline paths) marks recycling and exits 75, andrecycleStandaloneexits 75, instead of spawning a detached replacement;runtime-port.jsoncarries an attestation secret like a standalone start.supervisor.rs. The sidecar watch reports each exit once it is recorded. A puredecide()respawns only when all of these hold: the exit is the tracked child's, the coordinator is idle, the runtime is still wanted, and no ending was claimed.Mode::Recover:ocx resolvestays the only authority, only a proven absence spawns, a live runtime is attached as a guest, and no window or takeover prompt is shown.runtime-supervisor.log.exit.rs. A stickywantedintent is cleared by tray Stop and by any drain claim (Quit, update).abort_restart()returns a failed update's drain to Idle and re-runs startup.POST /api/stopfrom a dashboard session answers409 desktop_supervisedwhile the app supervises the proxy, because the app would start it again after a full teardown.ocx stopfrom tray Stop, Quit, the update drain or a terminal is unaffected.Performance
GET /healthzevery 5 s while Ready or parked. Supervision work otherwise happens only on exits and run ends.Security
runtime-port.jsona standalone writes.Verification
cargo fmt --check,cargo check,cargo clippy --all-targets -- -D warnings: clean (desktop/src-tauri).cargo test --lib -- supervisor:: startup:: resolve:: window::covers only the touched modules: 52 pass.bun run typecheck,bun run structure:check,bun run privacy:scan: pass.file-size-ratchet, the test-layout guards and the three changed TS test files: 70 pass.cd docs-site && bun run build: 521 pages built, 70421 internal links checked.Checklist
🤖 Generated with Claude Code