test(codex): hold prompt-probe admission through document edits

[luvs01]

Summary

Prompt-document admission tests assume a probe is still active when a child start marker appears, then change a file and expect a second request to return busy. Their children exit after 200 ms. If the parent resumes later, the first probe can already have completed and the second request correctly returns fresh text, failing the test.

Use the existing test-only close barrier to hold admission explicitly through each edit and busy assertion. Let fixture children write their output and exit immediately. Release the barrier and drain the original request in finally, observing early request errors without overwriting the held-phase assertion error.

The change is confined to cases 28–40 in one test file: 25 parameterized cases, 14 held request lifetimes, and 13 child timer fixtures. Case 31 gets a fresh barrier for each create/delete transition. Existing fingerprint, old/fresh text, spawn-count, marker-count, and decoy checks remain. Runtime behavior, cancellation tests, and actual timeout/size limits are unchanged.

Verification

• Pinned Bun 1.4.0 on Windows; head 50929c1008f382fa4f47edcc34ad4cabe24b8403, based on dev 7797586a8899c673eab48886a490e85b480c6d72.
• The motivating macOS control failure received ok: true with new-team instead of the expected busy response in the quoted-key/comment-after-bracket case.
• Isolated reproduction: delay the parent by 350 ms after the marker in that case. The original fixture failed with the same busy-versus-fresh mismatch. The close-barrier version passed with the same delay. The delay was removed from the final patch; this demonstrates the race mechanism without claiming the exact hosted scheduling sequence.
• bun run test -- --timeout 60000 --parallel=1 tests/codex-integration/codex-prompt-route.test.ts: 75 tests / 851 assertions passed on the final candidate, including all 25 affected cases.
• Typecheck, privacy scan and git diff --check passed. Independent read-only review verified the preserved assertions, separate barriers for the two transitions, and release/drain/error handling.
• Full contributor CI passed all 26 jobs on head 50929c100, including the six Windows shards and unsharded macOS control. CodeRabbit's same-head review found no correctness issues and verified the helper's release, drain, and error behavior.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

• All CI tests are green on my local testing.
• I pushed my PR to the latest dev commit.
• I resolved all correct Codex and CodeRabbit findings.
• My PR is ready for review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 22053b5b-e530-4b2e-8e21-80c3f3905d45

📥 Commits

Reviewing files that changed from the base of the PR and between 7797586 and 50929c1.

📒 Files selected for processing (1)

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

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

---

📝 Walkthrough

Walkthrough

The integration tests add deterministic synchronization for in-flight prompt probes. Tests 28–40 now verify that mutations return busy while the held probe preserves pre-edit content across multiple document-selection scenarios.

Changes

Prompt probe race tests

Layer / File(s)	Summary
Prompt probe close-barrier harness tests/codex-integration/codex-prompt-route.test.ts	Lines 18 and 148–174 import the test seam and add withHeldPromptProbeClose, which holds a pending prompt probe, runs a mutation, releases the barrier, and awaits the probe result.
Mutation race scenarios tests/codex-integration/codex-prompt-route.test.ts	Lines 911–1480 update tests 28–40 to write probe output immediately, assert busy responses after mutations, and verify that held probes return pre-mutation content for configuration, creation, deletion, fallback, ancestor, and manifest cases.

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

Merge Risk: ⚪ Minimal · up to 50929

Prompt-probe mutation tests now deterministically hold in-flight probes while verifying busy responses and pre-edit content, replacing timer-based synchronization without changing production behavior. No merge-blocking risk remains.

Suggested reviewers: lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: updating Codex prompt-probe tests to hold admission through document edits.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

Hygiene

✅ Deterministic PR hygiene checks passed.

[luvs01]

@coderabbitai review

Please review head 50929c1008f382fa4f47edcc34ad4cabe24b8403, based on dev 7797586a8899c673eab48886a490e85b480c6d72. This changes one test file. The 25 document-admission cases now hold the existing close barrier explicitly instead of relying on 200 ms child timers. The original busy, old/fresh text, spawn/marker and decoy assertions are preserved; both transitions in case 31 use separate barriers.

The final file passed 75 tests / 851 assertions on Windows with Bun 1.4.0. Full contributor CI is https://github.com/luvs01/opencodex/actions/runs/34210267444. Please also check that the helper observes request rejection immediately and releases/drains the original request without replacing a held-phase assertion error.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="7797586a8899c673eab48886a490e85b480c6d72"
head="50929c1008f382fa4f47edcc34ad4cabe24b8403"

git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
git diff --check "$base" "$head"
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --find-renames --unified=80 "$base" "$head"

changed_file="$(git diff --name-only "$base" "$head" | head -n 1)"
printf '\n--- AST outline: %s ---\n' "$changed_file"
ast-grep outline "$changed_file" --items all || true

printf '\n--- Barrier and request-related references ---\n'
rg -n -C 5 'closeBarrier|barrier|Promise\.withResolvers|requestError|requestPromise|finally|busy' "$changed_file"

Length of output: 61718

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

test_file="tests/codex-integration/codex-prompt-route.test.ts"
probe_file="src/codex/prompt-text-probe.ts"

printf '%s\n' '--- Helper with line numbers ---'
nl -ba "$test_file" | sed -n '145,178p'

printf '%s\n' '--- Held helper call sites ---'
rg -n -C 3 'withHeldPromptProbeClose\s*\(' "$test_file"
printf '%s\n' '--- Counts ---'
printf 'helper definitions: '
rg -n 'async function withHeldPromptProbeClose\s*\(' "$test_file" | wc -l
printf 'helper invocations excluding definition: '
rg -n 'withHeldPromptProbeClose\s*\(' "$test_file" | tail -n +2 | wc -l
printf 'timer fixtures in affected cases: '
sed -n '900,1320p' "$test_file" | rg -n 'setTimeout\s*\(' | wc -l

printf '%s\n' '--- Probe module outline ---'
ast-grep outline "$probe_file" --items all

printf '%s\n' '--- Close barrier definitions and use context ---'
rg -n -C 12 'setPromptTextProbeCloseBarrierForTests|closeBarrier|await.*Barrier|\.close\s*\(' "$probe_file"

printf '%s\n' '--- Changed cases: post-helper assertions ---'
nl -ba "$test_file" | sed -n '900,1335p'

Length of output: 1446

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

test_file="tests/codex-integration/codex-prompt-route.test.ts"
probe_file="src/codex/prompt-text-probe.ts"

printf '%s\n' '--- Helper source (lines 145-178) ---'
sed -n '145,178p' "$test_file"

printf '%s\n' '--- Helper call sites and nearby assertions ---'
rg -n -C 5 'withHeldPromptProbeClose\s*\(' "$test_file" || true

printf '%s\n' '--- Held-lifetime and timer counts ---'
printf 'helper occurrences: '
rg -n 'withHeldPromptProbeClose\s*\(' "$test_file" | wc -l
printf 'setTimeout occurrences in cases 28-40: '
sed -n '900,1320p' "$test_file" | rg -n 'setTimeout\s*\(' | wc -l

printf '%s\n' '--- Probe module outline ---'
ast-grep outline "$probe_file" --items all || true

printf '%s\n' '--- Close barrier lifecycle ---'
rg -n -C 14 'setPromptTextProbeCloseBarrierForTests|closeBarrier|await.*Barrier|close\s*\(' "$probe_file" || true

printf '%s\n' '--- Cases 28-40 source (lines 900-1320) ---'
sed -n '900,1320p' "$test_file"

Length of output: 12482

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

probe_file="src/codex/prompt-text-probe.ts"
test_file="tests/codex-integration/codex-prompt-route.test.ts"

printf '%s\n' '--- Probe module outline ---'
ast-grep outline "$probe_file" --items all || true

printf '%s\n' '--- Test-only close barrier declarations, setter, reset, and close lifecycle ---'
rg -n -C 20 'setPromptTextProbeCloseBarrierForTests|closeBarrier|resetPromptTextProbeForTests|child\.once\("close"|\.once\("close"' "$probe_file" || true

printf '%s\n' '--- Full relevant probe module ---'
sed -n '1,340p' "$probe_file"

printf '%s\n' '--- Remaining affected cases and timer-fixture check ---'
sed -n '1320,1500p' "$test_file"
if sed -n '900,1500p' "$test_file" | rg -n 'setTimeout\s*\('; then
  exit 1
else
  printf '%s\n' 'No setTimeout fixture remains in cases 28-40.'
fi

Length of output: 26871

---

@luvs01 Reviewed head 50929c1008f382fa4f47edcc34ad4cabe24b8403 against base 7797586a8899c673eab48886a490e85b480c6d72.

I found no correctness issues.

Key checks completed:

• git diff --check passes.
• The change is limited to tests/codex-integration/codex-prompt-route.test.ts.
• withHeldPromptProbeClose at Lines 149–174 installs the close barrier before it starts the original /text request.
• The helper attaches both request outcome handlers at Lines 158–161. This observes a request rejection when it occurs and prevents an unhandled rejection.
• The finally block at Lines 164–168 releases the captured barrier before it clears the test seam. This matches the probe close handler in src/codex/prompt-text-probe.ts, which captures the barrier when the child emits close.
• await pending cannot replace an error from whileHeld, because both request outcomes resolve to an outcome object. Therefore, held-phase assertion failures remain the reported failure.
• After a successful held phase, Lines 170–173 rethrow an observed request error. The helper does not hide an early failure.
• There are 14 held request lifetimes. Case 31 uses separate helper calls for the create and delete transitions.
• No setTimeout fixture remains in cases 28–40.
• The affected cases retain the busy response, stale/fresh text, spawn-count, marker-count, and decoy assertions.

I did not execute the Bun test suite in this sandbox. The static review supports the reported Windows test result.

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

[lidge-jun]

리뷰 · 우선순위 48 / 80

이 PR은 prompt-document admission 테스트가 느린 부모에서 flake 나는 문제를 고친다. 테스트는 “probe가 아직 살아 있을 때 문서를 고치면 두 번째 요청이 busy”를 가정하는데, fixture child가 200ms 만에 끝나면 첫 probe가 이미 끝나 두 번째 요청이 새 텍스트를 돌려 실패한다. 지금 dev HEAD 29bb221c3에는 이미 src/codex/prompt-text-probe.ts에 setPromptTextProbeCloseBarrierForTests seam이 있다. 이 PR은 그 seam만 테스트에서 쓰고, 프로덕션 파일은 건드리지 않는다. review-ready 라벨·hygiene/enforce-target 초록이라 테스트 chore 중에서는 상대적으로 준비도가 높다.

구체적 패턴은 withHeldPromptProbeClose다. close barrier Promise를 걸어 admission을 붙잡아 두고, 그 안에서 파일/베이스 문서 편집과 busy 단언을 한 뒤 finally에서 release한다. child는 setTimeout(..., 200) 대신 즉시 stdout을 쓰게 바꿨다. 타이밍 레이스에 기대지 않고 “닫힘이 보류된 동안 busy”를 강제하니, 테스트가 말하려는 계약과 더 잘 맞다. 적용 범위는 config write, selected base edit, AGENTS.md create/delete/transition, framing shift 등 020 coverage 묶음이다.

주의할 점: held 구간에서 던진 에러도 finally가 release하도록 되어 있어 누수는 막는다. pending을 finally와 반환 경로에서 두 번 await하는 것은 Promise라 문제 없다. 다만 이 헬퍼는 테스트 전용 seam에 강하게 묶여 있으므로, 나중에 probe close 구현이 바뀌면 같이 깨진다. 제품 tip과는 무관한 CI 안정화라 점수는 중하이다. types/config 분할 무관, 중복 close 대상 아님.

라인 tests/codex-integration/codex-prompt-route.test.ts withHeldPromptProbeClose - 기존 setPromptTextProbeCloseBarrierForTests를 올바르게 쓴다. child 지연에 의존하던 200ms setTimeout을 제거한 것도 맞다.
경로 src/codex/prompt-text-probe.ts - 이 PR diff에는 없지만 HEAD에 seam이 이미 있다. 프로덕션 동작 변경 없이 테스트만 고친다는 전제가 성립한다.
경로 busy 단언 another prompt probe is still finishing; retry shortly - held 중에 spawn 시도가 1로 남는지도 같이 본다. 계약 단언으로 충분하다.
경로 PR readiness - draft=false, review-ready, hygiene/enforce-target 초록. 테스트 chore로는 merge 준비에 가깝다.

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

• prompt-probe admission flake를 2.49 tip 전에 넣을지, CI 잡음이 실제로 남아 있는 shard만 보고 넣을지
• 같은 close-barrier 헬퍼를 다른 prompt 테스트에도 공통으로 뺄지

너의 추천
Merge 후보다. 제품 코드가 없고 seam 사용도 HEAD와 맞다. CI 한 번 더 초록 확인 후 랜딩하라. 닫을 중복이 아니다.

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

[lidge-jun]

Landed on dev in cfff026 via #4049, which carried all nine PRs of this train as one sequentially reviewed integration branch. Your commit is preserved in the squash body with a Co-authored-by trailer, so the contribution stays attributed to you.

Verification at the exact merged head 8c58bac0863f1f2ca2b430e1177a0e417b474c85: Cross-platform CI run 34256853348 finished with 19 jobs successful and 0 failures, bun x tsc --noEmit exit 0, and 312 focused tests passing across the ten affected files. An independent review confirmed the stacked tree is byte-identical to the union of the nine PR diffs.

Closing this PR because the change is already on dev. Thank you.

[lidge-jun]

Landed via #4049 at cfff026