Repository navigation
fix(wait): read the last interval before a timeout and pages past a full one - #181
Conversation
Quest-Claim: 88f9fc2005c19464
…ull one A timed wait polled only on its 5-minute schedule, so a 540 s wait never read its last 240 s; it now polls once more 30 s before the timeout. A full last page answered every conditional poll with a 304, which has no Link header, so a new next page was never seen; full pages are now reread unconditionally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: implemented and handed back as a draft with two
(Written by Claude Opus 5.5) |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fcf4308fa
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The final poll before a timeout shortened any interval, including a longer one GitHub asked for. Only the default 5-minute schedule is shortened now; when a server interval runs past the final poll, the wait skips it and times out with its cursor instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # quest/m0/helpers/README.md
|
Merge summary:
Enabling auto-merge (squash). (Written by Claude Opus 5.5) |
Finishes
quest/m0/helpers/wait-gaps.md:quest waitno longer leaves the end of a timed wait unread, and no longer stops seeing new timeline events when the last page was exactly full.Causes
The last poll interval before a timeout was never read.
quest waitpolled on a fixed 5-minute schedule and slept until the deadline after its last scheduled poll. A--timeout 540wait polled at 0 s and 300 s only, so anything in its last 240 s was left for the cursor and never reported by that wait. On kixelated/quest#127, Codex reviewed most heads 230-340 s after the push, but reviewed 42ba20d 659 s after it (push 19:56:12, review 20:07:11). A wait started about a minute after that push timed out around 20:06-20:07 having read nothing after about 20:02, which matches the report. A rerun with--sincefrom before the push reported the review at once, as it should. The default starting point was correct: it resolves to the head's push time from repository activity, and default waits on #176, #177, and #178 reported every review, reaction, and check result after their last push.Fix: when the next scheduled poll would land past the deadline, the wait polls once more 30 s before the timeout (
FINAL_POLL). Activity after that poll is still left for the timeout's cursor, which comes from setup, so a retry with it reports the activity. Nothing is dropped. A longerX-Poll-Intervalfrom GitHub is a floor: only the default schedule is shortened, and when the server interval runs past the final poll, the wait skips it and times out with its cursor.A full last page hid new pages. The timeline and review-comment listings reread their last page with
If-None-Match. I checked against GitHub: a 304 has noLinkheader, and the ETag covers only the body. So once the last page held exactly 100 rows, every later poll got a 304 and never learned that page 2 existed. Every timeline event after that point was missed for the rest of the wait. Fix: a full last page is reread without a conditional header, so itsnextlink shows up.Q1.cursor:encodeCursoronly ever printsq1., and the CLI prints lines unchanged, soquest waitnever printsQ1.. That is a display quirk in the report from kixelated/quest#172, and this PR does not change it.Changes
src/cli/wait.ts: adds a final poll before the timeout (never sooner than GitHub'sX-Poll-Interval), and does not send a conditional request for a full last page (PAGEnames the 100-row page size the listings use).src/cli/main.ts: thequest waithelp says it polls once more 30 s before the timeout, and that longer GitHub poll intervals delay requests.test/wait.test.ts: four regression tests: a review after the last scheduled poll is reported by the final poll; a review after the final poll is reported on retry with the timeout's cursor; a full last page is reread unconditionally and its new next page is followed; a 600 sX-Poll-Intervalwith a 540 s timeout skips the final poll instead of polling early.quest/m0/helpers/README.md.Validation
nix develop -c just checkpassed, includingquest check --base origin/main.nix develop -c just testpassed: 391 CLI tests (2 skipped), the shell regressions, both lifecycle checks, and 306 Worker tests.src/cli/wait.tsreverted, the first three new tests fail; theX-Poll-Intervaltest fails with this PR's earlier final-poll logic.Link) and default waits on merged PRs, as described above.Decisions
The maintainer settled both choices:
quest waitnever overruns--timeout. Activity in those last 30 s still reaches the caller through the cursor. A poll is four or five requests, which usually take a few seconds.Follow-ups for /quest-plan
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)