Repository navigation
test: give the delivery queue test a budget that covers its wait in the queue - #202
BIackFIame wants to merge 1 commit into
Conversation
…he queue The second task's deadline is set when it is queued, so it also covers the whole first transaction. With the fixture's 150 ms default, a few 25 ms polling ticks on a coarse Windows timer (about 15.6 ms steps) spend it, and the second Enter is not sent. This test checks ordering, not the short-deadline path, so it gets the 5 s budget the first test in this file already uses (6e36e98). The last draft is confirmed by a UserPromptSubmit so it does not sit in the acceptance loop.
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. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Queue timing and extension acceptance tests/prompt-delivery.test.mjs |
The queue test uses a 5,000 ms fixture wait budget for its ordering check. The extension test waits for the Enter write and UserPromptSubmit signal, then asserts that the receipt is confirmed. |
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Other
Suggested reviewers: howdeploy
Merge Risk: 
05c6a
-
The changes improve prompt-delivery test stability and confirmation coverage without introducing a merge-blocking risk.
Architecture risk: 
05c6a
-
The change affects 1 system.
Changed systems:
testsArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — tests (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in tests/prompt-delivery.test.mjs: The queue test now uses a 5,000 ms fixture wait budget instead of the default and documents that its ordering check should not depend on the second task’s shorter deadline.
- observed — Modified behavior in tests/prompt-delivery.test.mjs: The extension test now waits for the Enter write and a
UserPromptSubmitlifecycle signal, then asserts that the extension receipt is confirmed; previously it only awaited completion.
-
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: increasing the delivery queue test budget to cover its wait time. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… 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. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. 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 0 functions across 1 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Follows up on the Windows failure in
tests/prompt-delivery.test.mjsyou asked about on #201. Test-only, one file; no product change.What fails
"the queue retains the entire paste/observe/Enter transaction and supports owned paste-only drafts" fails on
windows-pipe-hostat theuntil(() => f.writes.length === 4)wait (line 179; line 167 is the test's declaration). The second receipt is alwaysstage: "pasted", "The delivery deadline expired after the paste; Enter was not sent.", atelapsedMs151–153, after a first delivery of 72–74 ms:Across the last 60 CI runs it caused 3 failures, all in the Windows job (about 5–6% of Windows runs since the test landed in 5c9771b), never on Ubuntu or macOS.
Cause
The product behaves as designed; the test's budget is too small for it on Windows.
TerminalManager.deliverInputsets a task's deadline when the task is queued, andexecutepasses only what is left todeliverInputNow. The test queues "one" and "two" at the same time with the fixture's defaultwaitMsof 150, so "two" shares one 150 ms window with the whole "one" transaction. (That is intended: there is a separate "timed out while waiting in the delivery queue" reason.)setTimeout, and the test'suntil()with 2 ms. Windows rounds timers up to about 15.6 ms steps, so 25 ms becomes about 31 ms and 2 ms about 15 ms. Two transactions need about five such ticks, which is more than 150 ms. On macOS the same run takes 72 ms and 131 ms.Fix
fixture(t, "codex", 5_000), as the first test does. What it checks (ordering, nothing lost, Enter for each paste) is unchanged.UserPromptSubmitonce that draft is written and asserts it isconfirmed.Verification
setTimeoutup to a 15.625 ms step (a stand-in for Windows timers), the test on c5731f4 fails every run with the CI signature (pasted, "deadline expired after the paste"). With this change the file passes 13/13 every run, also with 31.25 ms steps.Not changed here: at an exaggerated 50 ms step two other tests that rely on the 150 ms default also fail ("Claude's unstyled hint…" and "a staged prompt clears the previous answer…"). They have not failed on real Windows timers. If you want, they can get the same 5 s budget in this PR.
Summary by CodeRabbit