From 51d4f21d6bae641631ff4f2b471c5936ff1dd9a6 Mon Sep 17 00:00:00 2001 From: Kamil Krzywda Date: Sat, 19 Sep 2026 15:23:20 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20v0.17.0=20=E2=80=94=20sim-control=20POST?= =?UTF-8?q?=20retries=20+=20permission-aware=20notifications=20e2e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 1 + components/LeftPane.tsx | 78 ++++++++++++++++++++++++++++-------- components/left-pane.test.ts | 71 +++++++++++++++++++++++++++++++- e2e/toasts.spec.ts | 22 ++++++---- 4 files changed, 147 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6015cb3..111e2cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - No server or OpenAPI changes — diffing uses only existing snapshot fields (verified: planes by id/status/locationName, packages by id/status/reward/origin+dest names) - Tests: unit — new `lib/deriveEvents.test.ts` suite pins every event format (landing, delivery with reward, expiry from open and loaded, new-package appearance, bought/sold, identical snapshots → [], first snapshot → [], non-events like takeoff/load ignored); `lib/useToasts.test.ts` covers the queue cap. e2e — new `e2e/toasts.spec.ts`: deterministic insufficient-funds path (select the most expensive model in `[data-testid="market-buy-model"]`, click Buy, expect a visible error toast matching /insufficient funds/ AND the inline `right-error` still showing; skips only if cash already covers the plane on a long-lived local instance) and the notifications opt-in (unsupported → disabled + title; otherwise fresh context shows "Enable notifications" and after clicking the label reflects the result — soft, since headless CI may deny permission) - Verified: `tsc --noEmit` clean; unit suite green locally; e2e not run locally by design — CI verifies against an isolated stack +- **Fix (CI runs #44/#47): sim-control POSTs are retried before surfacing the error.** A dropped `POST /api/sim` on the runner's flaky uplink left the UI optimistic while the server kept the old scale — panes.spec.ts:74 timed out with timeScale stuck at 1000 (run #47). `applySimControls` now retries each commit up to 3× total (~500 ms backoff) before surfacing the error toast; a newer intent arriving mid-retry supersedes the in-flight body instead of waiting out its retries (last-wins queueing kept), and the endpoint's set-semantics make re-sending an already-applied value idempotent. Pure retry core `retrySimPost` exported from LeftPane + unit-tested. Also run #47: toasts.spec.ts Part B is now permission-state-aware — headless Chromium reports `Notification.permission` as 'denied' (not 'default'), so the test branches on the actual state (denied → assert blocked label without clicking; granted → enabled label; default → prompt, click, accept either resulting label) ## v0.16.1 — 2026-09-19 14:00 - **Smooth client-side sim clock with per-snapshot re-sync (ISS-007).** The left-pane "Time" display used to jump once per 1 s poll because it rendered the snapshot's `clockMs` verbatim. It now ticks smoothly on the client between polls: new hook `lib/useSimClock.ts` keeps an anchor `{clockMs, at, timeScale, paused}` and re-derives the display clock every ~100 ms as `anchor.clockMs + (now − at) × timeScale` — exactly the server's own tick model (`tickOnce`: real-time × timeScale), so client extrapolation mirrors authoritative game time. Every new snapshot re-anchors to server truth, bounding any drift by network latency only; while paused the display is frozen at the anchor value (advance 0). The pure core `advanceSimClock(anchor, nowMs)` is exported and unit-tested in node diff --git a/components/LeftPane.tsx b/components/LeftPane.tsx index a0c1477..6d73475 100644 --- a/components/LeftPane.tsx +++ b/components/LeftPane.tsx @@ -48,16 +48,52 @@ export function pendingScaleConfirmed(pendingScale: number | null, serverTimeSca return pendingScale !== null && serverTimeScale === pendingScale; } +// ISS-008 fix (v0.17.0, CI runs #44/#47): bounded retry for POST /api/sim — a commit dropped by +// the runner's flaky uplink used to leave the UI optimistic while the server kept the old scale. +// Up to maxAttempts tries with delayMs between them; stops early once superseded() flips (a newer +// intent arrived and the caller's loop sends it instead). post is idempotent set-semantics, so +// re-sending a body whose response was lost cannot double-apply. Pure core — unit-tested in node. +export const SIM_POST_MAX_ATTEMPTS = 3; +export const SIM_POST_RETRY_DELAY_MS = 500; + +export type SimPostRetryResult = + | { ok: true } + | { ok: false; reason: 'exhausted'; error: unknown } + | { ok: false; reason: 'superseded' }; + +export async function retrySimPost( + post: () => Promise, + superseded: () => boolean, + sleep: (ms: number) => Promise = (ms) => new Promise((r) => setTimeout(r, ms)), + maxAttempts: number = SIM_POST_MAX_ATTEMPTS, + delayMs: number = SIM_POST_RETRY_DELAY_MS, +): Promise { + let lastError: unknown; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + if (superseded()) return { ok: false, reason: 'superseded' }; // a newer intent takes over — don't even send this body + try { + await post(); + return { ok: true }; + } catch (err) { + lastError = err; + if (superseded()) return { ok: false, reason: 'superseded' }; // a newer intent arrived during the attempt — don't wait out the backoff + if (attempt < maxAttempts) await sleep(delayMs); + } + } + return { ok: false, reason: 'exhausted', error: lastError }; +} + export default function LeftPane({ state, loading, error, onRefresh, onHover, onError }: LeftPaneProps) { const [busy, setBusy] = useState(false); - // POST /api/sim, then refetch the full snapshot immediately so the pane reflects the new - // controls without waiting for the next poll tick. On failure keep the stale UI — the - // running poll retries on its own and surfaces the error state. A commit that lands while - // another POST is in flight is held in a single slot (last intent wins) and flushed by the - // in-flight call, so every user intent reaches the server (the [1,1000] clamp stays as backstop). - // The gate is a REF, not React state: closures never see a stale "busy" value, which used to - // let a flush re-queue instead of send and silently drop a commit. + // POST /api/sim (retried up to SIM_POST_MAX_ATTEMPTS× on failure — CI runs #44/#47), then + // refetch the full snapshot immediately so the pane reflects the new controls without waiting + // for the next poll tick. After the final attempt fails keep the stale UI and surface an error + // toast (the running poll retries on its own). A commit that lands while another POST is in + // flight — including mid-retry — is held in a single slot (last intent wins) and supersedes + // the in-flight body, so every user intent reaches the server (the [1,1000] clamp stays as + // backstop). The gate is a REF, not React state: closures never see a stale "busy" value, + // which used to let a flush re-queue instead of send and silently drop a commit. const queuedRef = useRef<{ timeScale?: number; paused?: boolean } | null>(null); const busyRef = useRef(false); @@ -78,15 +114,25 @@ export default function LeftPane({ state, loading, error, onRefresh, onHover, on try { let current: { timeScale?: number; paused?: boolean } | null = body; while (current !== null) { - const res = await fetch('/api/sim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(current), - cache: 'no-store', - }); - if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`); - onRefresh(); - if (current === body) sentOwn = true; // object identity — queued bodies are fresh literals + const target = current; // stable across this retry round + const result = await retrySimPost( + async () => { + const res = await fetch('/api/sim', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(target), + cache: 'no-store', + }); + if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`); + }, + () => queuedRef.current !== null, // a newer intent supersedes the in-flight body mid-retry + ); + if (result.ok) { + onRefresh(); + if (target === body) sentOwn = true; // object identity — queued bodies are fresh literals + } else if (result.reason === 'exhausted') { + throw result.error; // only after the final attempt fails → error toast below + } current = queuedRef.current; // pick up anything that arrived while we were in flight queuedRef.current = null; } diff --git a/components/left-pane.test.ts b/components/left-pane.test.ts index c6b0b03..dc5f103 100644 --- a/components/left-pane.test.ts +++ b/components/left-pane.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, it } from 'vitest'; -import { pendingScaleConfirmed } from './LeftPane'; +import { describe, expect, it, vi } from 'vitest'; +import { pendingScaleConfirmed, retrySimPost, SIM_POST_MAX_ATTEMPTS, SIM_POST_RETRY_DELAY_MS } from './LeftPane'; // ISS-001 — the optimistic slider value is cleared exactly when a rendered snapshot confirms // it (server scale === pending), never earlier. Clearing at commit time would snap the pointer @@ -22,3 +22,70 @@ describe('pendingScaleConfirmed', () => { expect(pendingScaleConfirmed(1, 1)).toBe(true); // committing the current value confirms immediately }); }); + +// ISS-008 fix (CI runs #44/#47): a dropped POST /api/sim must be retried before surfacing the +// error — up to SIM_POST_MAX_ATTEMPTS tries with SIM_POST_RETRY_DELAY_MS between them, and a newer +// intent supersedes the in-flight body instead of waiting out its retries. + +describe('retrySimPost', () => { + it('pins the retry budget: 3 attempts total, ~500 ms backoff', () => { + expect(SIM_POST_MAX_ATTEMPTS).toBe(3); + expect(SIM_POST_RETRY_DELAY_MS).toBe(500); + }); + + it('succeeds on the first attempt without sleeping or retrying', async () => { + const post = vi.fn(async () => {}); + const sleep = vi.fn(async () => {}); + await expect(retrySimPost(post, () => false, sleep)).resolves.toEqual({ ok: true }); + expect(post).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + }); + + it('retries with the backoff delay and succeeds on a later attempt', async () => { + let calls = 0; + const post = vi.fn(async () => { + if (++calls < 3) throw new Error('dropped'); + }); + const sleep = vi.fn(async () => {}); + await expect(retrySimPost(post, () => false, sleep)).resolves.toEqual({ ok: true }); + expect(post).toHaveBeenCalledTimes(3); + expect(sleep).toHaveBeenCalledTimes(2); // one backoff between each pair of attempts + expect(sleep).toHaveBeenCalledWith(SIM_POST_RETRY_DELAY_MS); + }); + + it('gives up after the final attempt with the last error (no trailing sleep)', async () => { + const err = new Error('down'); + const post = vi.fn(async () => { + throw err; + }); + const sleep = vi.fn(async () => {}); + const result = await retrySimPost(post, () => false, sleep); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('exhausted'); + if (result.reason === 'exhausted') expect(result.error).toBe(err); // the LAST attempt's error + } + expect(post).toHaveBeenCalledTimes(SIM_POST_MAX_ATTEMPTS); + expect(sleep).toHaveBeenCalledTimes(SIM_POST_MAX_ATTEMPTS - 1); + }); + + it('stops retrying once a newer intent supersedes the in-flight body (no further attempts or sleeps)', async () => { + let calls = 0; + const post = vi.fn(async () => { + calls++; // flip "superseded" right after the first failure, like a queued commitNow would + throw new Error('dropped'); + }); + const sleep = vi.fn(async () => {}); + await expect(retrySimPost(post, () => calls >= 1, sleep)).resolves.toEqual({ ok: false, reason: 'superseded' }); + expect(post).toHaveBeenCalledTimes(1); // the caller's loop sends the newer body instead + expect(sleep).not.toHaveBeenCalled(); // supersede is checked before the backoff sleep + }); + + it('never even sends a body that is already superseded', async () => { + const post = vi.fn(async () => {}); + const sleep = vi.fn(async () => {}); + await expect(retrySimPost(post, () => true, sleep)).resolves.toEqual({ ok: false, reason: 'superseded' }); + expect(post).not.toHaveBeenCalled(); + expect(sleep).not.toHaveBeenCalled(); + }); +}); diff --git a/e2e/toasts.spec.ts b/e2e/toasts.spec.ts index f653781..3f58db8 100644 --- a/e2e/toasts.spec.ts +++ b/e2e/toasts.spec.ts @@ -58,11 +58,19 @@ test('failed actions toast bottom-right (inline error stays) and the Time sectio return; } - // Fresh context → permission starts as 'default' (the opt-in prompt)… - await expect(btn, 'fresh context shows the opt-in prompt').toHaveText('Enable notifications', { timeout: 10_000 }); - - // …and clicking requests it. Headless CI may deny — keep the assertion soft: accept either - // outcome, but the label must reflect the RESULT of the request. - await btn.click(); - await expect(btn).toHaveText(/Notifications (enabled|blocked)/i, { timeout: 10_000 }); + // Branch on the ACTUAL permission state — headless CI reports 'denied' (not 'default'), so a + // fresh context is not guaranteed to show the opt-in prompt (CI run #47). + const permission = await page.evaluate(() => Notification.permission); + if (permission === 'denied') { + // LeftPane renders the blocked-state label without any click. + await expect(btn, 'denied context shows the blocked-state label').toHaveText('Notifications blocked', { timeout: 10_000 }); + } else if (permission === 'granted') { + await expect(btn, 'pre-granted context shows the enabled label').toHaveText('Notifications enabled', { timeout: 10_000 }); + } else { + // 'default' — the opt-in prompt. Clicking requests it; headless may deny, so accept either + // outcome, but the label must CHANGE from the prompt to reflect the result. + await expect(btn, 'non-denied context shows the opt-in prompt').toHaveText('Enable notifications', { timeout: 10_000 }); + await btn.click(); + await expect(btn).not.toHaveText('Enable notifications', { timeout: 10_000 }); + } });