fix(cli): stop start from shadowing a live configured-port proxy, and retry health

[lidge-jun]

Summary

Reimplements #3078 (author @Veritas-7) on dev. Both production hunks there are correct; the PR targets main, and its test file does not typecheck — tests/cli-health-retry.test.ts declares const servers: Server[] while importing only IncomingMessage and ServerResponse.

start shadowed a live proxy. handleStart skipped the configured-port probe whenever the pid file and the runtime-port record were both absent (src/cli/index.ts:205-211). That absence proves nothing: a fallback-port sibling overwrites both records when it starts and removes them when it stops. So start bound an ephemeral port over a healthy configured-port proxy, re-pointed client config at the ephemeral copy, and the next sibling shutdown left no runtime record for discovery at all. handleEnsure already passed probeConfiguredPort: true; handleStart is the one path that did not.

health probed once. A proxy that has only just bound can miss a single probe while its event loop is still settling startup work, so ocx health run seconds after a service restart reported Proxy not healthy and exited 1 for a proxy that was serving. The stop paths already retry this exact race under SERVICE_STOP_LIVENESS (#764).

What changed from #3078

The tests. #3078 stands up real loopback servers and spawns a real child to exercise the shadow path. That is a lot of machinery for two one-line decisions, and one of its files does not compile. Here the retry budget is asserted through dispatchCommand's injected findLiveProxy, and the start guard through a source oracle placed next to the handleEnsure call site that was already correct — the same source-oracle pattern tests/cli-ready.test.ts already uses for handleStart wiring.

Verification

bun test tests/cli-dispatch.test.ts   -> 29 pass / 0 fail / 116 expect()
bun x tsc --noEmit                    -> exit 0

Mutation-checked, both restored:

mutation	result
drop probeConfiguredPort from handleStart	28 pass / 1 fail — every findProxyOwnerBeforeJournalRecovery call site asks for the probe
drop the health retry budget	28 pass / 1 fail — passes a retry budget to findLiveProxy

still reports an absent proxy as unhealthy is the control: retrying does not turn a dead proxy into a live one.

The retry is scoped to the health runner only — the other four findLiveProxy call sites in dispatch.ts keep their single probe.

Checklist

• Focused tests for the changed subsystem pass
• bun x tsc --noEmit clean
• Regression tests present and mutation-verified
• Targets dev
• No docs-site change needed (CLI probe behavior, no interface change)

Triaged in the 2026-08-31 non-priority-70 bug round.

Summary by CodeRabbit

• Bug Fixes

  • Improved proxy health checks by retrying liveness probes during startup.
  • Ensured startup detects existing proxies on the configured port, including instances using fallback ports.
  • Preserved clear failure behavior when no active proxy is available.

• Tests

  • Added coverage for health-check retries and configured-port detection during start and ensure operations.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T18:58:19.513972Z	d8a195c	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 088c7648-61f9-4ceb-8cb8-c2f1fce822c3

📥 Commits

Reviewing files that changed from the base of the PR and between 9d122dd and d8a195c.

📒 Files selected for processing (3)

• src/cli/dispatch.ts
• src/cli/index.ts
• tests/cli-dispatch.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The CLI health command now performs three liveness probes. Proxy startup now checks the configured port even without persisted process-state records. Tests cover retry configuration, exit status, call-site options, and explicit probing gates.

Changes

CLI liveness behavior

Layer / File(s)	Summary
Health probe retry contract src/cli/dispatch.ts, tests/cli-dispatch.test.ts	findLiveProxy accepts optional LivenessIo settings. The health command requests three probe attempts. Tests verify the retry budget and the exit code when no proxy exists.
Configured-port startup probing src/cli/index.ts, tests/cli-dispatch.test.ts	handleStart enables configured-port probing before starting another proxy. Source-level tests verify the relevant call sites and require an explicit true option.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d8a19

The PR prevents a live configured-port proxy from being shadowed and makes health checks tolerate brief startup races through a bounded retry. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: ingwannu, wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both primary changes: preventing start from shadowing a live configured-port proxy and adding retries to health. It is specific, concise, and directly related to the p…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title clearly summarizes both primary changes: preventing start from shadowing a live configured-port proxy and adding retries to health. It is specific, concise, and directly related to the pull request objectives.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/3078-start-shadow-health-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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 63 / 80

설명

이 PR은 지금 dev HEAD 6123be31f (#3103 forked-rollout restore) 위에서, CLI의 ocx start / ocx health 두 경로가 가진 운영 버그를 고칩니다. 기여자 @Veritas-7 의 #3078이 같은 수정을 냈지만 main 을 타깃으로 했고 테스트 파일이 typecheck를 깨서 [WRONG BRANCH] 로 닫힌 상태입니다. 이 PR은 그 프로덕션 의도를 dev 에 다시 올리고, 실포트 바인딩/자식 스폰 대신 의존성 주입과 소스 오라클로 검증을 다시 짠 버전입니다.

현재 체크아웃의 src/cli/index.ts 를 보면 findProxyOwnerBeforeJournalRecovery 는 pid 파일이나 runtime-port 기록이 있을 때만 기본적으로 프로브하고, options.probeConfiguredPort === true 일 때만 상태 파일이 없어도 설정된 포트를 봅니다 (대략 205–211행). handleEnsure 는 이미 { probeConfiguredPort: true } 를 넘기지만 (대략 461행), handleStart 는 옵션 없이 호출합니다 (대략 233행). 그래서 fallback 포트로 뜬 형제 프로세스가 pid/runtime 기록을 덮어썼다가 종료하며 지워 버리면, 설정된 포트에 아직 건강한 프록시가 살아 있어도 start 는 “아무도 없다”고 판단하고 ephemeral 포트에 새 프록시를 띄운 뒤 클라이언트 설정을 그쪽으로 돌립니다. 그다음 형제까지 내려가면 discovery용 runtime 기록이 통째로 사라져서, 사용자는 “분명 띄웠는데 어디에도 안 잡히는” 상태에 빠집니다. 이게 이 PR의 첫 번째 핵심입니다.

두 번째는 src/cli/dispatch.ts 의 health 러너입니다. 지금 dev 에서는 deps.findLiveProxy() 를 한 번만 호출합니다 (대략 545행). src/server/proxy-liveness.ts 의 LivenessIo.attempts 기본값은 1이고, stop 경로는 이미 SERVICE_STOP_LIVENESS (attempts: 3) 으로 같은 “방금 bind 직후 이벤트 루프가 아직 바쁜” 레이스를 재시도합니다 (#764). 서비스 재시작 직후 ocx health 를 돌리면 실제로는 서빙 중인 프록시를 Proxy not healthy / exit 1 로 오판할 수 있습니다. 이 PR은 health에만 { attempts: 3 } 을 넣고, dispatch 안의 다른 findLiveProxy 네 곳은 단일 프로브로 남겨 둡니다.

테스트는 tests/cli-dispatch.test.ts 에 붙습니다. health는 dispatchCommand 에 주입한 findLiveProxy 가 attempts === 3 을 받는지, 그리고 null이면 여전히 exit 1인지 확인합니다. start 가드는 index.ts 소스에서 findProxyOwnerBeforeJournalRecovery(...) 호출이 모두 probeConfiguredPort: true 를 쓰는지, 그리고 게이트가 여전히 === true 인지를 소스 오라클로 고정합니다. PR 본문 기준 mutation도 둘 다 복원됩니다 (프로브 옵션 제거 / 재시도 예산 제거 시 각각 1 fail). bun x tsc --noEmit 도 통과했다고 적혀 있습니다.

현재 dev 스냅샷 기준으로 round-2 prio≥70 남은 축은 #3029(풀 버스트 창), #3008(history-only stop abort), #3019(WHAM 401) 쪽이고, 이 PR은 그 열차 밖 “2026-08-31 non-priority-70 bug round” 항목입니다. 그래도 start 가 건강한 설정된 포트 프록시를 가리고 클라이언트 설정을 잘못된 포트로 돌리는 문제는 일상 운영에서 재현되면 체감이 크고, 변경 범위도 파일 3개·+85/−4로 작아서 열차와 충돌할 면이 거의 없습니다. types.ts/config.ts 분할 캠페인과도 무관해서 close-don't-rebase 대상이 아닙니다. #3078은 이미 CLOSED([WRONG BRANCH])라 leftover 정리도 별도로 안 남습니다.

라인 src/cli/dispatch.ts health findLiveProxy({ attempts: 3 }) - 숫자 3을 리터럴로 박아 두었습니다. stop 경로의 SERVICE_STOP_LIVENESS.attempts 와 “같은 예산”이라고 주석/본문에 적혀 있으므로, 나중에 stop 쪽 attempts만 바꾸면 health가 조용히 어긋날 수 있습니다.
라인 tests/cli-dispatch.test.ts 소스 오라클 정규식 findProxyOwnerBeforeJournalRecovery\s*\(([^)]*)\) - 인자가 한 줄·괄호 중첩 없을 때만 맞습니다. 지금 호출 형태에는 충분하지만, 나중에 옵션 객체를 여러 줄로 쪼개면 오라클이 거짓 실패하거나 호출을 놓칠 수 있습니다.
경로 tests/cli-dispatch.test.ts health fake 시그니처 io?: { attempts?: number } - 프로덕션은 LivenessIo 를 import해 deps 타입을 넓혔는데, 테스트 fake는 로컬 부분 타입만 씁니다. 동작 검증에는 문제 없지만 deps 계약과 테스트 타입이 살짝 어긋납니다.
경로 src/cli/dispatch.ts 다른 findLiveProxy() 호출 네 곳 - PR이 health만 재시도하도록 의도적으로 남겨 둔 것은 맞습니다. 다만 GUI 오픈·status 계열도 “방금 뜬 직후” 레이스를 맞을 수 있는지 한 번은 의식해 두면 좋습니다. 이번 범위 밖입니다.
경로 전체 - types/config 분할·카탈로그·엔타이틀먼트 열차와 겹치는 파일이 없고, #3078 재구현이라 중복 오픈 PR을 남기지 않습니다. 프로덕션 변경 자체는 handleStart 한 줄 옵션 + health attempts 전달이라 리스크 면이 작습니다.

메인테이너의 판단이 필요한 지점

• health 재시도 예산을 { attempts: 3 } 리터럴로 둘지, SERVICE_STOP_LIVENESS 를 import해 공유할지
• round-2 prio≥70 ([Bug]: Codex pool intermittently fails to switch accounts after 5-hour quota exhaustion #3029/[Bug][Windows]: dashboard update aborts after proxy stops when history restore exits non-zero #3008/[Bug]: pool quota WHAM 401 bypasses stored-token refresh after plan changes #3019) 과 병렬로 바로 머지할지, 아니면 그 열차 빈 칸에만 squish 할지 (이 PR은 충돌면이 거의 없음)
• health 외 dispatch 단일 프로브 호출을 “의도적 단발”로 문서화만 할지, 후속 이슈로 남길지

너의 추천
머지 쪽으로 진행하는 것을 추천합니다. dev 에 실재하는 handleStart/ensure 비대칭과 health 단발 프로브를 정확히 겨냥했고, #3078보다 테스트가 가볍고 typecheck도 통과하며 mutation으로 두 가드가 복원됩니다. 머지 전에 선택적으로 attempts: 3 을 SERVICE_STOP_LIVENESS 공유로 바꾸면 주석과 코드가 더 맞아떨어지지만, 없어도 차단 사유는 아닙니다. Protect dev 리뷰 후 랜딩하면 됩니다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Administrator bypass record\n\nOwner authorization was explicit in Codex session 01a05a34-1e3a-73f2-8607-15e517cbec11 on 2026-09-01 KST. Exact head: d8a195c. Current exact-head check rollup: 0 failing, 0 pending. Independent A-gate review found no unresolved code blocker. Using the maintain/admin pull_request bypass documented in MAINTAINERS.md because the author cannot approve their own PR. This bypass does not waive CI or security review.