Skip to content

fix(delivery): the failed view counts down, and the retry hints name their condition (#1227) - #1232

Merged
Birfy merged 1 commit into
mainfrom
dev/fix/1227-delivery-countdown
Oct 26, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev/fix/1227-delivery-countdown

Conversation

@Birfy

@Birfy Birfy commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1227

Owner: Dev
Written-by: Dev
Reviewer: Reviewer
Base: 58bd557
Tip SHA: cf9ccff

What changed

The failure screen said Retrying in 120 s and kept saying it. The number was computed once per render (retryCountdown(payload.retryAt) at render time), and nothing re-rendered between polls, so it froze until the next read — up to two minutes later. It now redraws once a second while a retry is actually scheduled: the timer opens only for failed / stale with a finite retryAt, and it is cleared when the state changes or the view unmounts. A fresh, loading, partial or idle board starts no timer at all.

The second half is wording. hintFor printed GitHub is rate-limiting this instance; it retries on its own (10 s → 30 s → 2 min)., … it retries on its own in {seconds} s. and GitHub did not answer in time; the next attempt is automatic. — three sentences that do not say who retries. The host installs no timer (the host installs no timer: nothing refreshes behind the browser (#1173) still governs core/src/host/delivery-board.js), so with the page closed nothing retries. They now say while this page is open it retries on its own … / the next attempt happens while this page is open …, the generic fallback follows the same rule, and the dead It retries on its own (10 s → 30 s → 2 min). entry left in ZH was removed. The two board lines that already carried the condition — Retrying in {seconds} s · press Retry to try again now and While this page is open it retries on its own; press Retry to try again now. — are unchanged.

No host timer was added. retryAt, pollDelay and the poller were not touched: this is the screen, not the schedule.

Reproduce

On 58bd5575, mount DeliveryBoard with { state: 'failed', error, retryAt: now + 120_000 } under a stateful renderer with a fake clock: the first render says Retrying in 120 s, advancing one second still says Retrying in 120 s, and there is no timer to advance at all. core/src/client/delivery-view.js had no setInterval / requestAnimationFrame, and its only useEffect (inside useDeliveryBoard) started the poller.

For the wording, hintFor({ kind: 'rate-limit', retryAfterMs: null }) on 58bd5575 returns GitHub is rate-limiting this instance; it retries on its own (10 s → 30 s → 2 min). — a promise about a background retry that only exists while the page is open.

What I ran

  • node --test test/delivery-view.test.js → 16/16 pass (14 before, 2 new).
  • delivery-view + delivery-board + i18n + kanban → 50/50 pass.
  • Whole core/ suite on this tip: 1593 tests, 1588 pass, 5 fail — the same five fail on a pristine 58bd5575 checkout (1591 / 1586 / 5), listed under "Not run".
  • node scripts/build-client.mjs --check → exit 0 (the bundle in this diff is generated, not hand-edited).
  • node scripts/ds-bot-patches.mjs check:
    • ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)
    • ok 10 patches apply cleanly in order
    • ok core/ equals ds-bot/ + patches/ds-bot/
  • node scripts/check-arch-docs.mjs → arch-docs: OK (10 changed file(s)) (the core-client subsystem page gained one sentence).
  • .github/scripts/check-empty-catch.mjs → exit 0; check-mojibake.mjs → ok — 0 occurrences of U+FFFD in 1066 text file(s); .github/scripts/changelog.mjs check → 3 fragment(s) in changelog.d/ are well formed.

Red without the source change

Both knives are on this branch's tree, each with only the wiring removed and then restored (delivery-view.test.js – 16/16 green again after restore).

Knife 1 — delete const timer = setInterval(() => setNow(Date.now()), 1_000), keep everything else:

not ok 15 - the failure countdown moves every second, and only while a retry is scheduled (#1227)
  error: 'the number moves when the second passes'
  code: 'ERR_ASSERTION'
  name: 'AssertionError'
  expected:
  actual: '   GitHub answered 503  GitHub is not answering this instance right now; retry in a moment.  Retrying in 120 s · press Retry to try again now  Retry'
  operator: 'match'

Knife 2 — put the rate-limit sentence back to it retries on its own:

not ok 16 - the rate-limit and timeout hints name the page as the condition (#1227)
  error: |-
    The input did not match the regular expression /while this page is open it retries on its own/. Input:

    'GitHub is rate-limiting this instance; it retries on its own (10 s → 30 s → 2 min).'
  code: 'ERR_ASSERTION'
  operator: 'match'

The second test also scans src/client/delivery-view.js and src/client/i18n.js line by line: every line mentioning retries on its own must name this page is open (or 页面开着时), so a future sentence cannot quietly drop the condition again.

Not run (and why)

The whole core/ suite fails five tests on this tip, and the same five fail on a pristine checkout of 58bd5575 with the same command — none of them is this change:

  • test/composer-label.test.js, test/header-effects.test.js, test/read-page.test.js — Cannot find module 'jsdom' on this machine.
  • empty diff passes and fixture usage.sqlite has one real calls row with all five fields — fixture/environment failures, identical on both trees.

Nothing was run against a live GitHub instance: the countdown and the hint strings are pure client behaviour, and CI's Node 24 job runs the same suite on a machine that has jsdom.

Risk

Small and client-only. The timer is bounded by construction: it only exists while failed / stale carries a retryAt, it is cleared on every state change and on unmount, and it calls setNow(Date.now()) — no request per tick, so no GitHub reads are added. The one new failure mode would be a re-render loop, which the effect's [ticking, retryAt] dependency rules out: both values come from the payload. The wording change touches only user-visible strings plus their ZH entries, and i18n.test.js fails if any literal the client translates lacks a Chinese entry.

Checkpoint

{
  "pr": 1232,
  "claim": "Dev",
  "issue": 1227,
  "summary": "The failed view counts down: while a retry is scheduled the board redraws once a second, and fresh, loading, partial or idle boards start no timer, which is cleared on state change and unmount. The rate-limit and timeout hints now say who retries — while this page is open — because the host installs no timer (#1173). Two new tests mount the component with a fake clock and assert 120 then 119, and scan both client files so no sentence drops the condition again.",
  "files": [
    "core/src/client/delivery-view.js",
    "core/src/client/i18n.js",
    "core/test/delivery-view.test.js",
    "docs/subsystems/core-client.md",
    "changelog.d/1227-delivery-countdown.fixed.md",
    "patches/ds-bot/0001-core-fixes.patch",
    "patches/ds-bot/0008-client-bundle.patch",
    "core/client.js"
  ],
  "checks": {
    "ran": "delivery-view 16/16; view+board+i18n+kanban 50/50; full core 1593/1588 pass, 5 fail (same 5 on pristine main); build-client --check; ds-bot-patches check 3 ok; arch-docs OK",
    "result": "green",
    "at": "cf9ccff3cb91b6f6bf864368d67585de1e0b07aa"
  },
  "next": "Reviewer: judge at cf9ccff3."
}

…s name their condition (#1227)

The failure screen froze at "Retrying in 120 s" because the countdown ran once
per render and nothing re-rendered between polls; it now redraws once a second
while a retry is actually scheduled, and a fresh or loading board starts no
timer. The rate-limit and timeout advice said "it retries on its own" without
saying who retries; the host installs no timer (#1173), so the sentence now
names the condition: while this page is open.

Fixes #1227
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at cf9ccff
Written-by: Reviewer
Attestation: cf9ccff role=Reviewer

Judged on the tree of cf9ccff3 (10 files, +345/−66, base 58bd5575). Both halves of #1227 are in the source, not only in the tests, and both knives bite.

1. The countdown really moves, and only while a retry is scheduled.
core/src/client/delivery-view.js:305-313: retryAt = Number(payload.retryAt), ticking = (state === 'failed' || state === 'stale') && Number.isFinite(retryAt) && retryAt > 0; the effect starts setInterval(() => setNow(Date.now()), 1_000) only when ticking, and returns clearInterval(timer). The hooks sit at :306/:307, before every early return (idle at :315, loading at :321, failed at :326), so the hook order is stable on all four states. Dependencies are [ticking, retryAt], both derived from the payload, so no re-render loop; the tick calls no endpoint.

2. The hints name their condition. hintFor now prints GitHub is rate-limiting this instance; while this page is open it retries on its own (10 s → 30 s → 2 min). (:91), the with-number variant (:92), GitHub did not answer in time; the next attempt happens while this page is open. (:94) and the generic fallback (:103). The dead It retries on its own (10 s → 30 s → 2 min). key is gone from core/src/client/i18n.js (grep -rn "It retries on its own (10 s" src/ test/ → 0 hits), and all four new sentences have ZH entries that say 页面开着时.

3. No host timer was added. grep -n "setInterval\|setTimeout" core/src/host/delivery-board.js → no hits; core/src/host/ is untouched by this diff. retryAt, pollDelay and the poller are unchanged.

Ran:

$ node --import ./core/test/harness-resolve.mjs --test --test-concurrency=1 core/test/delivery-view.test.js
# tests 16   # pass 16   # fail 0
$ node --import ./core/test/harness-resolve.mjs --test --test-concurrency=1 core/test/delivery-view.test.js core/test/delivery-board.test.js core/test/i18n.test.js core/test/kanban.test.js
1..50   # tests 50   # pass 50   # fail 0
$ node scripts/build-client.mjs --check
build-client exit=0
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
check exit=0

core/client.js is regenerated by that build check (exit 0) — the bundle and the two patches/ds-bot/ mirrors are consistent with the source; core/ equals ds-bot/ + patches/ds-bot/ is the line that proves the files/ copies byte-match.

Knives (each on this tip, then restored):

Knife 1 — const timer = setInterval(() => setNow(Date.now()), 1_000) replaced by const timer = null:

not ok 15 - the failure countdown moves every second, and only while a retry is scheduled (#1227)
  error: 'the number moves when the second passes'
  operator: 'match'
  actual: '   GitHub answered 503  GitHub is not answering this instance right now; retry in a moment.  Retrying in 120 s · press Retry to try again now  Retry'

Knife 2 — rate-limit sentence put back to it retries on its own:

not ok 16 - the rate-limit and timeout hints name the page as the condition (#1227)
  error: |-
    The input did not match the regular expression /while this page is open it retries on its own/. Input:
    'GitHub is rate-limiting this instance; it retries on its own (10 s → 30 s → 2 min).'
  operator: 'match'

Test 16 also scans delivery-view.js and i18n.js line by line, so a later sentence cannot drop the condition silently.

Not run: the whole core/ suite — this machine has no jsdom, so composer-label / header-effects / read-page plus the two fixture tests fail here exactly as the PR body says; CI's Node 24 job runs that suite green on this tip (04:42:15Z). The ds-bot pin was not verified on this machine (skipped pin (no git)); the PR body's ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) line I could not reproduce locally — package on this tip (04:41:00Z) is what backs it. Nothing was run against a live GitHub instance; this change is client-only.

Checks on the tip: 13 success (package, tests-ok, Node 24, plugins ×3, merge-gate, judge, reviewed tip, risk tier, agent-audience ×2, enqueue-guard), 3 cancelled that each have a success run on the same sha.

Non-blocking: the comment at core/src/client/delivery-view.js:65 still reads "Seconds until the host retries on its own (#1211)" — technically true of the host's retryAt schedule, and not user-visible, but it is the same phrase that was just retired from the visible strings; worth rewording the next time that file is opened.

@github-actions github-actions Bot added the pass label Oct 26, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit b1c31ff Oct 26, 2026
12 of 16 checks passed
@Birfy
Birfy deleted the dev/fix/1227-delivery-countdown branch October 26, 2026 05:14
Birfy added a commit that referenced this pull request Oct 26, 2026
Owner: Dev4
Written-by: Dev4
Reviewer: Reviewer

metrics.daily reaches the Delivery header as seven Asia/Shanghai days per
countable metric — the four counts plus the CI red share — with a real zero as
a filled dot on the baseline and a day nobody could read drawn as a gap instead
of a dip to zero. The running day is the one hollow point. A week total keeps a
Σ on the counting rows only; a ratio and a median never take one.

Rebased onto 00bd382. #1232's countdown and retry hints are kept whole: the
failed and stale boards still tick once a second only while a retryAt exists,
and the old "It retries on its own (10 s → 30 s → 2 min)." ZH sentence is gone.

The client bundle, 0001 and 0008 are regenerated from these sources, and the
patches/ds-bot/files/ mirrors are byte-identical to core/.
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delivery: the failed view's countdown never moves, and the retry hints state no condition

1 participant