fix(test): isolate diagnostic servers from live Codex restart

[luvs01]

Summary

Diagnostic fixture servers previously inherited the live Codex restart service, so an authenticated diagnostic request could reach host process inspection or restart operations despite using temporary homes.

• Inject a non-operating restart service through the common native-main-owner-child.ts startup path, including additional fixture instances. State reads return a synthetic unknown result and restart requests fail before host operations.
• Cover the refusing service, source wiring, and dependency forwarding through two authenticated HTTP listeners. Stub startup ownership inspection so the route test does not inspect the developer's installed service.
• Document that Windows diagnostic mirrors must copy both fixture helper modules.

Verification

Current head: 546a691f5f0b666dd34a576285fe3b6909fb7264.

• Current-head cross-platform CI passed, including Linux test shards, all nine Windows shards, and the aggregate ci check.
• Current-head target-branch gate passed. Catalog async-contract checks on Linux, macOS and Windows also completed successfully.
• Implementation-stage local validation included the focused restart-route suite, typecheck, privacy scan, structure check, and file-size ratchet. The focused Bun 1.4.3 run at ae6ea8756 passed 12 tests. Those local results are historical; the current-head CI links above provide the current revision's broader evidence.
• A full local repository run was not repeated across the concurrent worktrees. The earlier local native-owner lifetime run had three child-event timeouts that also reproduced on its dev baseline; that run is not counted as a pass.
• Existing review threads are resolved. The upstream comparison at 6da2774369aa32552e318e333902499683d053bc found one release-version commit beyond the shared base, with no file overlap.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Diagnostic documentation matches the fixture boundary.
• Focused validation and current-head CI evidence are documented.
• Existing review findings are addressed and resolved.

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Summary by CodeRabbit

• Tests
  • Expanded coverage of restart status and rejected restart requests in isolated test environments, including verification that requests are handled by the configured service.
• Documentation
  • Clarified the limits of restart isolation in test fixtures and updated source comparison guidance to require matching copies of both helper files.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: luvs01/opencodex/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 3db3e7ad-53b3-4d38-8287-fa3b9ece84ec

📥 Commits

Reviewing files that changed from the base of the PR and between ae6ea87 and 1dcb4bd.

📒 Files selected for processing (1)

• tests/codex-integration/codex-restart-route.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The native-owner fixture injects a restart service that returns synthetic unknown state and refuses restart requests. Integration tests verify these responses through authenticated requests to primary and extra servers. The README documents the fixture boundary and updated mirror requirements.

Changes: Native-owner restart fixture

Layer / File(s)	Summary
Fixture restart service and wiring tests/helpers/native-owner-restart-service.ts, tests/helpers/native-main-owner-child.ts, tests/codex-integration/codex-restart-route.test.ts	The fixture service returns unknown state and rejects restart requests. The native-owner helper injects it into server configuration, and a test checks the common startup path.
Boundary validation and documentation tests/codex-integration/codex-restart-route.test.ts, scripts/diagnostics/windows-version-control/README.md	Tests cover service responses and authenticated requests to primary and extra servers. The README describes the process-control boundary and requires byte-identical copies of both helper files in the scratch mirror.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 1dcb4

The fixture isolates diagnostic restart requests from the live restart service, and the reviewed changes establish the intended boundary for both servers. No concrete merge-blocking risk remains in the supplied scope.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		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
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preventing diagnostic servers from invoking the live Codex restart service. It matches the fixture-service injection and restart-isolation ch…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[devin-ai-integration]

Addressed in ae6ea87. The new servers on the common startup path serve the injected service over authenticated HTTP case boots two real startServer listeners with managementApi.codexRestartService injected — the same path the child's start() wraps — and drives authenticated requests against both. GET returns the fixture's synthetic {"state":"unknown","runningCount":0}; POST returns a sentinel body the live implementation could never emit, and a call log proves the injected service (not the live one) fielded every request. A break in deps.managementApi → ctx.deps forwarding now fails this test even though the source guard still passes.

One note on the refusal itself: nativeOwnerRestartService.performRestart() throws, and Bun surfaces an uncaught route error as a transport-level failure rather than a contract response, so the e2e observes the restart path through a sentinel instead of the throwing fixture. The refusal semantics stay verified by the route-dispatch cases below.

[devin-ai-integration]

Review-finding triage (Devin)

Collected every unresolved thread and finding on this PR:

• Devin Review (1 finding, tests/codex-integration/codex-restart-route.test.ts): the source-wiring guard could pass while startServer's managementApi → ctx.deps forwarding silently drops the boundary. Fixed in ae6ea87 — added an e2e case that starts two real servers on the same injection path and proves over authenticated HTTP that the injected service answers GET/POST on both. Thread resolved.
• CodeRabbit: no findings — draft PRs are not auto-reviewed; only the draft notice was posted.
• Codex connector: no comments.

Checks on ae6ea87 (Windows, Bun 1.4.3): bun test tests/codex-integration/codex-restart-route.test.ts 12/12 pass, bun run typecheck, bun run privacy:scan, bun run structure:check all pass. bun run test:changed requires Bun 1.4.0 exactly and could not run here; the focused suite above covers the touched file.

[devin-ai-integration]

Done. Findings triaged on PR #717:

Fixed (1): Devin Review's analysis — the source-wiring guard could pass while startServer's managementApi → ctx.deps forwarding silently drops the restart boundary. Added servers on the common startup path serve the injected service over authenticated HTTP in codex-restart-route.test.ts (ae6ea87): boots two real listeners with the fixture service injected on the same path the child uses, and proves over authenticated GET/POST that the injected service (not the live one) answers on both. Thread replied and resolved.

Nothing to dismiss: CodeRabbit posted only its draft-not-reviewed notice (no findings), and the Codex connector left no comments.

Checks on the pushed head: focused suite 12/12, typecheck, privacy:scan, structure:check all pass. test:changed needs Bun 1.4.0 exactly (box has 1.4.3), so the focused suite covered the touched file instead.

One caveat noted on the thread: the fixture's performRestart throws, which Bun surfaces as a transport error rather than a contract response, so the e2e observes the POST path through a sentinel — refusal semantics remain covered by the route-dispatch cases. Summary posted on the PR. Devin Review check is re-running on the new head; CodeRabbit passed.

[devin-ai-integration]

Readiness status (head ae6ea87)

Ready-for-review per owner request. Exact-head verification is in the updated description; summary:

• Restart-route suite 12/12 — includes the new e2e case proving the managementApi → ctx.deps injection boundary over two real authenticated servers.
• Root gates on ae6ea87: typecheck, privacy:scan, structure:check, file-size-ratchet — all pass.
• Native-owner lifetime suite 7/10 — the 3 failures are @@native-owner@@ child event timeouts that reproduce identically on a dev baseline worktree on this host (environmental, not a branch regression).
• Windows diagnostic controller — not runnable here (no MSVC/MinGW linker); catalog-async-contracts is the validating gate, currently queued.
• Hosted checks: CodeRabbit ✅, Devin Review ✅ on ae6ea87. The github-actions check-suite is queued — a repo-wide runner stall with no completions since 2026-10-08, so it is not claimed as passing evidence.
• Security Cloud finding remains open until the correction lands and is verified.

Written by Devin

[devin-ai-integration]

PR #717 is now ready for review: #717

모든 레디니스 조건을 처리하고 draft를 해제했습니다. PR 본문의 Verification 섹션을 실제 head(ae6ea87) 기준 결과로 갱신하고 요약 코멘트도 남겼습니다.

처리 내용:

• Focused restart-route suite 12/12 통과 (신규 e2e 경계 케이스 포함)
• Root gates on ae6ea87: typecheck, privacy:scan, structure:check, file-size-ratchet 모두 통과
• Native-owner lifetime suite 7/10 — 실패 3건은 dev 베이스라인 worktree에서도 동일하게 재현되는 환경성 타임아웃(브랜치 회귀 아님)으로 확인
• Windows diagnostic — 이 머신에 링커(MSVC/MinGW)가 없어 로컬 실행 불가; catalog-async-contracts 워크플로가 검증 게이트 (현재 큐 대기 중)
• Hosted checks: CodeRabbit ✅, Devin Review ✅. GitHub Actions check-suite는 repo 전체적으로 10/08 이후 하나도 완료되지 않는 runner 정체 상태라 통과로 주장하지 않고 PR 본문에 명시했습니다
• Security Cloud finding은 정책대로 오픈 상태 유지

머지 전에는 정체된 Actions 큐가 풀려서 GHA 체크가 실제 완료되어야 합니다.

[luvs01]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[devin-ai-integration]

Fixed in 1dcb4bd — the guard now asserts from >= 0 and to > from before slicing, so a renamed marker fails loudly instead of passing on an unrelated range.

[devin-ai-integration]

Fixed in 1dcb4bd — deps now injects inspectNativeCodexOwnership, the same seam native-main-owner-child.ts uses, so startServer never inspects the developer machine's installed service state.

One deliberate deviation from the suggestion: the stub returns foreign, not owned. An owned verdict makes startServer build a real NativeProfileManager and start the real native-main owner drive (SQLite lock, ACL hardening, recovery probes) on the fixture home — more machinery and more host coupling than this test needs, and a second place where real probes could run. foreign closes native-main admission with an inert fence (no lifecycle, no reprobe hook), and nothing in this restart-route test exercises native admission.

[devin-ai-integration]

Review-finding triage (Devin)

Collected every unresolved thread and finding on this PR (CodeRabbit ASSERTIVE pass on ae6ea87):

• CodeRabbit — source-wiring guard could pass on an unrelated slice range (codex-restart-route.test.ts:176-178): genuine. indexOf returning -1 for a renamed marker made the slice cover the wrong range and could pass for the wrong reason. Fixed in 1dcb4bd — both marker indexes are asserted present and ordered before slicing.
• CodeRabbit — e2e startServer reads real installed service state (codex-restart-route.test.ts:229-231): genuine. inspectStartupOwnership runs on every startServer boot and falls back to the live inspectNativeCodexOwnership when the seam is absent, so the two fixture listeners inspected the developer machine's installed service state — the host coupling this PR exists to remove. Fixed in 1dcb4bd by injecting inspectNativeCodexOwnership into the shared deps, with a foreign verdict rather than the suggested owned: owned would start the real native-main owner drive on the fixture home; foreign fences native admission (unexercised here) with no machinery. Reasoning noted on the thread.
• Devin Review: 1 finding from the previous pass — already fixed in ae6ea87 and its thread resolved; no new findings.
• Codex connector: no comments.

Checks on 1dcb4bd (Windows, Bun 1.4.3): bun test tests/codex-integration/codex-restart-route.test.ts 12/12, bun run typecheck, bun run privacy:scan, bun run structure:check — all pass. bun run test:changed requires Bun 1.4.0 exactly and could not run on this box; the focused suite above covers the touched file.

Written by Devin

[devin-ai-integration]

Done. Triaged every unresolved review thread on PR #717:

Fixed (2 CodeRabbit findings, pushed as 1dcb4bd):

• Source-wiring guard now asserts both slice markers exist and are ordered before slicing — a renamed marker can no longer pass on an unrelated range.
• The e2e startServer calls now inject inspectNativeCodexOwnership — without the seam, startServer inspected the developer machine's installed service state (exactly the host coupling this PR removes). I deviated from CodeRabbit's suggested owned verdict and used foreign: owned would start the real native-main owner drive (SQLite lock, ACL hardening, recovery probes) on the fixture home; foreign fences native admission — unexercised by this restart-route test — with no machinery. Rationale noted on the thread.

Already resolved: the earlier Devin Review finding (fixed in ae6ea87 before this pass). No Codex connector comments, nothing dismissed.

Checks on the pushed head (Bun 1.4.3): focused suite 12/12, typecheck, privacy:scan, structure:check all pass. test:changed needs Bun 1.4.0 exactly, so the focused file covered it. Both threads replied + resolved; summary posted on the PR.

[luvs01]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[luvs01]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[luvs01]

Closing completed duplicate tracking: the full change from source HEAD 546a691f5f0b666dd34a576285fe3b6909fb7264 is already present in merged development commit e6d7e561f566ab343fbf545a6fa3e1b63012f817. All four changed files were compared byte for byte: the diagnostic README, restart-route regression, native main-owner child helper, and restart-service helper. The complete diagnostic-server isolation fix is retained, not just a similarly titled change. The source branch is retained.