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 was stacked on F6b (#161), which rewrote merge polling in web/public/app.js; F6b is now merged (b4a1977) and this PR targets main.

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. This is defensive: a click needs ready, which excludes a running job or an active attempt, so no timer is pending then (traced, not tested; removing it fails no test).

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 merged into main as squash commit b4a1977. 6744f95 brings that in: since the squash duplicates F6b's commits, the merge takes main's tree and re-applies this PR's own diff (0857fa2→792581a) on top with git apply -3; its diff against main is exactly this PR's four files, identical to the diff it had against F6b. 121769c strengthens one assertion and a later commit labels the click's timer clear defensive (below).

Validation

Current head 47d496b, base main b4a1977 (47d496b changes a comment and a doc sentence only since 121769c):

Gate	State
Full browser suite (test/browser, one worker)	194/194 pass on 6744f95; the cross-generation spec passes on 121769c
npm run typecheck	pass on 6744f95
CI test	pass on 47d496b

Earlier checkpoints: 8bdeaf9 on F6b cffdf4c; d21cb0e on F6b 7b1b5fa (CI test pass; Copilot 2 clean); ec2e2e4 on F6b 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.
13	Independent agent	b4a1977 / 6744f95 (new base main)	Code clean: the one-chain guard holds with F6b's final browser code (generation bumps clear the timer; the job→queue handoff, a render during a pending click and a refused click's follow each start one chain; every observation passes settle()). Findings: this body was stale (fixed here); the cross-generation spec's last assertion could not fail, since the control already read "Merge queued" before poll 2 answered — fixed in 121769c by waiting for a third poll (fails with 2 polls when a settled answer stops the chain).
14	Independent agent	b4a1977 / 121769c	Code and specs clean; mutation checks of the guard, settle's sequence check, current ??= settle() and the new third-poll wait each fail a spec (settle's generation check fails three plans.spec.ts tests). One finding: the merge click's timer clear is untested and unreachable today (a click needs ready). Labelled defensive in the code, merge-queue.md and this body.
15	Independent agent	b4a1977 / 47d496b	Clean. The defensive reasoning holds against the server (withMergeJob, #withAttempt, statusAfterMerge) and every browser path that replaces data.merge; mutation checks match each break-check claim.
Copilot 3	copilot-pull-request-reviewer	b4a1977 / 47d496b	Requested after round 15 was clean and CI passed on 47d496b. Blocked: Copilot did not review — the requesting user's Copilot review quota is used up. Not a clean result; re-request once quota is available.
Copilot 4	copilot-pull-request-reviewer	b4a1977 / 47d496b	Re-requested 2026-10-17 on the same clean head. No open issues; approval recommended.

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]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[Copilot]

🟢 Approval recommended

The ownership guards address the reproduced races, with focused regression coverage and no unresolved findings.

No open issues 🎉

_{Review effort: Balanced}