Skip to content

fix(wait): read the last interval before a timeout and pages past a full one - #181

Merged
kixelated merged 7 commits into
mainfrom
quest/m0/helpers/wait-gaps
Oct 22, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m0/helpers/wait-gaps

Conversation

@kixelated

@kixelated kixelated commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Finishes quest/m0/helpers/wait-gaps.md: quest wait no 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 wait polled on a fixed 5-minute schedule and slept until the deadline after its last scheduled poll. A --timeout 540 wait 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 --since from 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 longer X-Poll-Interval from 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 no Link header, 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 its next link shows up.

Q1. cursor: encodeCursor only ever prints q1., and the CLI prints lines unchanged, so quest wait never prints Q1.. 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's X-Poll-Interval), and does not send a conditional request for a full last page (PAGE names the 100-row page size the listings use).
  • src/cli/main.ts: the quest wait help 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 s X-Poll-Interval with a 540 s timeout skips the final poll instead of polling early.
  • Deletes the quest and its link from quest/m0/helpers/README.md.

Validation

  • nix develop -c just check passed, including quest check --base origin/main.
  • nix develop -c just test passed: 391 CLI tests (2 skipped), the shell regressions, both lifecycle checks, and 306 Worker tests.
  • With src/cli/wait.ts reverted, the first three new tests fail; the X-Poll-Interval test fails with this PR's earlier final-poll logic.
  • Against live GitHub: conditional requests (304 with no Link) and default waits on merged PRs, as described above.

Decisions

The maintainer settled both choices:

  1. Final-poll lead. The last poll starts 30 s before the timeout, so quest wait never 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.
  2. Scope of the pager fix. Only a full last page is reread unconditionally; conditional polling is otherwise unchanged, so unchanged pages stay free 304s.

Follow-ups for /quest-plan

  • (Recommend: plan only if it recurs) The waits reread only the last page, so an event that GitHub inserts on an earlier page of a multi-page listing would be missed. I have not seen GitHub do this for timeline events. If it shows up, the simpler fix is to reread every page on each poll, as reactions and checks already do, and drop conditional polling.
  • (Recommend: drop) A default wait on a merged PR whose head branch was later recreated for another PR reports that PR's pushes, because repository activity is filtered only by ref. For example, a default wait on #178 printed a push from #186. It only affects waits on closed PRs.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 2 commits October 21, 2026 22:38
…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>
@kixelated kixelated changed the title Never miss events in quest wait fix(wait): read the last interval before a timeout and pages past a full one Oct 22, 2026
@kixelated

Copy link
Copy Markdown
Owner Author

Outcome: implemented and handed back as a draft with two ## Decisions needed items, since a background agent cannot settle them.

  • Root cause from feat: add prunable worktree lifecycle primitives #127: a timed wait never read its last poll interval. A 540 s wait polled only at 0 s and 300 s, and Codex's review of 42ba20d landed 659 s after that push. The wait now polls once more 30 s before the timeout. Later activity is reported on retry with the timeout's cursor.
  • Second gap: a full 100-row last page answered every conditional poll with a 304, which has no Link header, so later pages were never read. Full pages are now reread without a conditional header.
  • Q1.: quest wait only prints q1., so that was a display quirk and nothing changed.
  • just check and just test passed locally. CI has not run yet, since the PR is a draft.

(Written by Claude Opus 5.5)

@kixelated
kixelated marked this pull request as ready for review October 22, 2026 15:38
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-22T15:58:26.570676Z 30d9f5b New commits
🔒 Security Review ✅ Completed 2026-10-22T15:59:48.285138Z 30d9f5b New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/cli/wait.ts Outdated
kixelated and others added 5 commits October 22, 2026 08:53
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
@kixelated

Copy link
Copy Markdown
Owner Author

Merge summary:

  • Merged main three times (no rebase); each conflict was quest/m0/helpers/README.md, resolved by dropping the finished quests on both sides (claims, ci-guard, and this PR's wait-gaps).
  • Fixed the Codex P2 thread in 768245b: GitHub's X-Poll-Interval is a floor. The final poll 30 s before the timeout shortens only the default 5-minute schedule; when a server interval runs past it, the wait skips the final poll and times out with its cursor, never overrunning --timeout. Regression test: a 600 s interval with a 540 s timeout sleeps 540 s with no early poll. Codex then approved the head with a thumbs-up.
  • Local: just check and just test passed after the fix and the second merge; after the last (clean) merge, quest check --base origin/main and the wait and CLI tests passed. CI passed on 08a75bd. The base branch's quest check against the head passed.

Enabling auto-merge (squash).

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit d1589bb into main Oct 22, 2026
14 checks passed
@kixelated
kixelated deleted the quest/m0/helpers/wait-gaps branch October 22, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant