Run one merge poll chain at a time in the browser (#48)

[mchwang]

Part of #48: the browser part. The server part merged as #171. This PR is stacked on F6b (#161, codex/f6b-merge-handoff), which rewrites merge polling in web/public/app.js.

Defects

Both were reproduced on F6b at 2a9534d:

• Poll ordering. renderMerge() calls scheduleMergePoll() on every render. While a poll was in flight its timer was already cleared, so any render (for example selecting another item) started a second poll chain in the same mergeGeneration. Nothing ordered the two answers, so an older "queued" answer could be applied after a newer "merged" one, and the control went back to "Merge queued".
• Stale errors. In the same way, an older poll's error could replace a newer confirmed merge in the banner.

Fix

One merge poll chain at a time (web/public/app.js):

• mergePollInFlight = { generation, sequence } is set when a poll starts. scheduleMergePoll() starts nothing while a poll of the current generation is in flight; that poll reschedules itself when it settles.
• A settling poll clears the marker only if it is still the latest, and applies its answer or error only if it is the latest and its generation is current. An older generation's late poll therefore neither applies nor releases the newer generation's guard.
• Applicability is decided once, so an error thrown while applying a current answer is still reported and polling continues.
• The merge click also clears the old poll timer, like every other generation change.

docs/implementation/merge-queue.md describes the single poll chain.

Traced, not tested:

• The page sets no timeout of its own on a poll; the server bounds /api/merge.
• A re-render during a merge-job poll takes the same guard.

Tests

• test/browser/issue-48-poll-ordering.spec.ts:
  • A re-render while a poll is held starts no second chain, and the page ends on "Merged".
  • An older generation's late poll, holding a distinguishable "merged" answer, is never applied, and a re-render then starts no third poll.
• test/browser/issue-48-stale-errors.spec.ts:
  • A held error starts no second chain, and polling recovers to "Merged".
  • An unusable current answer (200 with a null body) is reported, and polling continues.

Break checks:

• On F6b's original app.js, the specs fail with 2 polls where 1 is expected.
• Clearing the marker unconditionally fails the cross-generation spec.
• Ignoring sequence and generation applies the old "merged" answer and fails it.
• Settling twice in the catch fails the unusable-answer spec.

F6b has since merged main (#170 made load()/act() async). 63b5525 merges that F6b (445081f) in: the specs now await the review service, and the merge-queue.md conflict keeps main's #48 regression list with this PR's browser sentence. F6b's new click answers (mergeRefreshRequired, an already-ended job) do not interact with the guard: the click raises the generation, so an older in-flight poll neither applies nor blocks the job's poll.

Validation

Current head 8bdeaf9, base codex/f6b-merge-handoff cffdf4c (#161, now based on main after F6a #153 merged as 209707e). This PR's diff has been byte-identical since fb8915b; the later merges brought in #161 changes only.

Gate	State
Browser: plans.spec.ts, review.spec.ts and the four #48 specs	107/107 pass on fb8915b; the cross-generation spec passes on 8bdeaf9
CI test	See checks on 8bdeaf9

Earlier checkpoints: d21cb0e on 7b1b5fa (CI test pass; Copilot 2 clean); ec2e2e4 on 2a9534d (browser 102/102, CI test pass).

Review record

Round	Reviewer	Base / head	Result
1	Independent agent	2a9534d / uncommitted	Found: an error after accepting an answer was swallowed and polling stopped; spec titles and headers described the old interleaving; no cross-generation test; the merge click left the old timer; docs. All fixed.
2	Independent agent	2a9534d / uncommitted	The cross-generation spec could not see a stale answer applied; an assertion inside a route handler could hang. Fixed and break-checked.
3	Independent agent	2a9534d / uncommitted	Clean; one nit (late "not applied" check), applied.
4	Independent agent	2a9534d / uncommitted (committed unchanged as ec2e2e4)	Clean
Copilot 1	copilot-pull-request-reviewer	2a9534d / ec2e2e4	0 open findings; approval recommended.
5	Independent agent	445081f / 63b5525 (merge of updated F6b)	Clean. Checked the guard against F6b's new click branches, the doc conflict resolution, and the async spec port. Not a defect: the job-poll path still has no browser test (listed under "Traced, not tested").
6	Independent agent	7b1b5fa / ff4aac9 (F6b test fixes merged in; PR diff byte-identical)	One low finding: after the item-selection re-render cleared the banner, the "still not applied, however late" check looked only at the banner. Fixed in d21cb0e: it also checks the control still reads "Merge queued". With settle() applying every answer, the test fails.
7	Independent agent	7b1b5fa / d21cb0e	Clean. With the earlier checks removed and settle() applying every answer, the new check alone fails. Removing the generation check fails three existing plans.spec.ts tests.
Copilot 2	copilot-pull-request-reviewer	7b1b5fa / d21cb0e	Requested 2026-10-15 after round 7 was clean and CI passed on d21cb0e. 0 open findings; approval recommended.
8	Independent agent	82661d6 / fb8915b (F6b's Copilot 5 fixes merged in: job observations advance the observation generation, newer job records survive full responses, the retry key records its job)	Clean. A job poll advances the observation generation only after settle() accepts it; a full response that keeps a newer job record starts no second applied chain.
9	Independent agent	82661d6 / fb8915b	Clean (fresh full pass).
10	Independent agent	0eb8a21 / 7e62e34	Clean; PR diff byte-identical to fb8915b's.
11	Independent agent	45bff7a / de8af78	Clean; PR diff byte-identical.
12	Independent agent	cffdf4c / 8bdeaf9	Clean on code and tests; flagged this body's stale Validation section, updated here.

Review-lesson audit

• A second poll chain started by a render: covered by AGENTS.md "Never apply a background response without proving it is still current", and the race "Poll starts, then a user action completes, then the old poll returns".

• The swallowed error and stopped polling: covered by "Preserve the original timeout, cancellation, and shutdown reason through every layer. Do not replace actionable errors…".

• Specs that did not assert their interleaving, or could hang: covered by "A race regression must assert that the interleaving it names actually happened" (added in Keep merge stop reasons and GitHub answers through every layer (#48) #171).

• The merge click timer and docs: covered by "Treat every behavioural claim … as something to verify".

• An assertion on a banner that a re-render had already cleared (round 6): covered by "A race regression must assert that the interleaving it names actually happened" and the Design system and plan format #1 rule of E2E tests.

No new rules.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

No blocking issues were identified in the focused polling changes, recovery paths, or regression coverage.

0 open findings

What changed in this PR

Addresses the browser portion of #48 by preventing overlapping merge poll chains and stale responses from overwriting current UI state.

Changes:

• Tracks poll ownership by generation and sequence.
• Adds regressions for ordering, stale responses, and error recovery.
• Documents polling guarantees and validation limits.

File	Description
web/​public/​app.js	Guards poll ownership, settlement, and timer cleanup.
test/​browser/​issue-48-stale-errors.spec.ts	Tests held errors and unusable-response recovery.
test/​browser/​issue-48-poll-ordering.spec.ts	Tests single-chain polling and cross-generation isolation.
docs/​implementation/​merge-queue.md	Documents polling behavior and regression evidence.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

No blocking issues were identified in the polling guards, error recovery, or regression coverage; tests were not rerun.

0 open findings

🧠 Review effort: Balanced