Repository navigation
test(cli): await sibling startup completion instead of a polling deadline - #8062
Conversation
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. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Track and await startup completion tests/helpers/startup-child-output.ts, tests/cli/sibling-home-client-sync.test.ts |
The output capture helper exposes waitForStartup, which resolves on the startup marker or detects incomplete startup. Tests cover clock changes, child exit, stdout end or read failure, and split UTF-8 output. |
Use startup capture in sibling-start tests tests/cli/cli-start-journal-order.test.ts |
startSibling waits for startup before validating the runtime record and /readyz response. Cleanup finalizes captured output, and stop-output assertions use the captured stdout tail. |
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Other
Possibly related PRs
- lidge-jun/opencodex#7740: Both changes affect the
startSiblingfixture’s runtime-record and readiness checks. - lidge-jun/opencodex#8003: Both changes address how
startSiblinghandles a child that has not published its runtime record.
Suggested labels: chore
Merge Risk: 
Minimal · up to 55350
-
The fixture now waits for startup completion before checking readiness. No actionable merge-blocking risk was established.
Pre-merge checks: 
5
-
Full details: Title check
-
Explanation
The title "test(cli): await sibling startup completion instead of a polling deadline" matches the main change. The raw summary and objectives show that the sibling-start fixture now waits for the child's startup signal or exit instead of polling for a runtime record and readiness against a 45-second deadline. The title names the test scope, the new waiting mechanism, and the replaced polling deadline. It does not list the added /readyz validation or the captured output helper, but the PR does not need to cover every detail. The title is 73 characters and 10 words, which fits the requested concise, single-sentence form.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 3 files.
-
Finishing Touches
-
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.
Summary
d5708c37c0235194c1d7bdba63bdc587099f6fcf. It now awaits the real CLI's existingClient startup work complete.output event or child exit, then asserts the runtime PID/port and/readyzidentity/status. Both pipes are drained immediately; early exit or stdout EOF/read failure reports bounded output tails and exit state. Existing test budgets are unchanged.ocx-sibling/config.json. Production publishesruntime-port.jsonunderOPENCODEX_HOMEafter bind and before the startup-complete signal. The confirmed harness defect is imposing a startup latency deadline and then killing a still-running child. The historical log does not trace the exact pre-bind stage, so slow initialization versus a stall inside that stage remains unproven; this PR makes no production-race or port-contention claim.git logandgit diffthat the intervening merges are fix(anthropic): support Ultra quota windows and show subscription names #8055 (Anthropic quota/GUI) and test(messaging): verify handle closure without racy fd probes #8058 (messaging test only), with no changes to CLI start, sibling handoff, runtime records, or port selection. Queried runs 41205004779 (67bdeb03a3) and 41214537933 (e972776ece): all 11 Windows shards succeeded in each.tests/helpers/startup-child-output.ts, found by searching for the CLI's startup-complete marker, rather than adding a production hook. Preserve routing/journal/handoff/stop assertions and allowlisted runtime diagnostics. No new test files or layout/baseline changes are needed.Verification
Windows host, Bun 1.4.4:
waitForimplementation: the same published record was accepted at elapsed 0 and rejected at simulated elapsed 90,000 ms. This proves the deadline failure mechanism, not reproduction of the hosted runner's internal stall.bun test tests/cli/cli-start-journal-order.test.ts: 12 pass, 0 fail.bun test tests/cli/sibling-home-client-sync.test.ts: 36 pass, 1 existing skip, 0 fail. Covers real-child exit diagnostics, stdout EOF/read failure, split UTF-8/markers, tail eviction and pipe backpressure. The pre-existing FIFO skip is unchanged.bun run test:changed: 63 pass, 1 existing skip, 0 fail, 3 selected files, including the final journal fixture.bun run typecheck: no type errors in 2,259 files.bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 38 pass, 0 fail.bun run privacy:scan: passed.git diff --check: passed.Checklist
Summary by CodeRabbit