diff --git a/CHANGELOG.md b/CHANGELOG.md index 718b95c..7b788f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ The application version is maintained in `package.json` and displayed in the adm No changes recorded yet. +## 1.4.3 - 2026-09-09 + +### Fixed + +- Two admin browsers open at once could silently overwrite each other's work: any save pushed a browser's entire in-memory copy of the data, so an idle tab's next save - even an unrelated one - could revert another browser's more recent changes, including a live race's in-progress results. Live results computed from real timing data are now protected from being reverted for a short window after they arrive, and every view now quietly refreshes its data in the background every 15 seconds (without ever discarding an open, unsaved edit), so the data behind any save stays fresh instead of getting stale over hours. + ## 1.4.2 - 2026-09-09 ### Fixed diff --git a/ROADMAP.md b/ROADMAP.md index 77c551b..b442810 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -47,10 +47,19 @@ Acceptance: users can distinguish server contact, pending edits, an active save - [x] Verify save failure/reconnection in a real browser. - [x] Extend stale-data feedback to the personal mobile profile: last applied retrieval, 30-second stale warning, timeout and Retry without discarding loaded data. - [x] Detect and explain a Backend URL/page-origin mismatch, the real cause behind an otherwise unexplained "Failed to fetch" on every sync and backup. -- [ ] Review concurrent writes, reconnect recovery and multiple controllers editing the same race. +- [x] Review concurrent writes with two admin browsers open at once; protect live race results from being overwritten, and shrink how stale a browser's data can get before its next save. Reported 2026-09-09, fixed the same day. +- [ ] Full per-field/optimistic-locking protection so no concurrent edit (not just live results) can silently overwrite another admin's - deferred, see the report below for why. Fixed 2026-09-09: a real Windows install showed "Backup failed: Sync failed: Failed to fetch" and every other backend call failing the same way, while the server itself was healthy (confirmed via `Invoke-WebRequest` returning 200 for `/api/health`, `/api/state` and `/api/backups`) - only the browser's own `fetch()` calls failed. Root cause: the page was opened via the LAN IP (`http://192.168.1.100:8081`) while the persisted Settings "Backend URL" still said `http://127.0.0.1:8081` from an earlier session. Browsers treat those as different origins even on the same machine, so the server's CORS check (`server.js`, comparing the request's `Origin` header against its own `Host` header) correctly rejected it - but nothing told the user why, since a CORS-rejected response surfaces to `fetch()` as a generic "Failed to fetch" with no further detail, and a command-line tool like `Invoke-WebRequest` doesn't send an `Origin` header at all, so it never reproduced the symptom. The Settings page now compares `window.location.origin` against the configured Backend URL on every render and, when they differ, shows a plain-language warning naming the page's actual address plus a one-click "Use `
`" button that fixes it immediately. Verified with a headless-browser check (seeding a deliberately mismatched Backend URL, confirming the warning text names the real origin, and confirming the fix button both clears the warning and persists the corrected value to the backend) against an isolated temp SQLite instance. +Reported and fixed 2026-09-09: investigated whether two admin browsers open at once can overwrite each other's changes, per an explicit request to report the risk before making bigger changes. Findings: `POST /api/state` ([server.js](server.js)) always replaces the *entire* stored state with whatever the client's in-memory copy was - no version check, no per-field merge. A browser only ever pushes; it never re-fetches server state after its initial page load, except the Overlay view, which already refreshes every 800ms. So the longer any other view sits open without a save, the staler its copy gets, and its *next* save - even one editing something unrelated, like a driver's name - pushes that entire stale snapshot, silently reverting any changes another browser made in the meantime to drivers, classes, cars, events, sessions, settings, training data, `activeSessionId` and `resultsBySession` alike. The worst case: an idle browser's save reverting another browser's live, in-progress race results. Two things are already safe from this: the raw lap-crossing log (the separate, append-only `passings` SQLite table) can't be lost this way, and the mobile driver self-service endpoints (markers, "Jämna varv", "Stegträning") already read-modify-write just their own slice server-side rather than accepting a client's full blob. + +Fixed with two scoped, low-risk changes rather than the bigger optimistic-locking rework (deferred - it needs real UX decisions about what a rejected/conflicting save should look like to the user, across every `saveState()` call site): +1. **Protect live results specifically** ([server.js](server.js)): the server now remembers, in memory, which `resultsBySession[sessionId]` entries were just updated by a real live decoder passing (`/api/passings`, the only route driven by actual timing hardware). For 15 seconds after that, an incoming `POST /api/state` whose body disagrees with that session's just-written result has that one key overwritten back to the server's copy before being saved - every other field in the same save still goes through normally. Verified with an integration test (`tests/server_persistence.integration.test.js`) simulating exactly the idle-browser-reverts-live-race scenario, using a shortened protection window via `RC_RESULT_PROTECTION_WINDOW_MS` to keep the test fast, plus a check that the protection expires and a later legitimate change to that same session still takes effect. +2. **Shrink the staleness window everywhere else** ([src/runtime_services.js](src/runtime_services.js)): every non-Overlay view now silently re-fetches server state in the background every 15 seconds (skipped while there's an unsynced local edit pending, same guard Overlay already uses) - without ever calling `renderView()`, so it can never discard an open, unsaved edit form's draft. This doesn't touch Overlay's own faster, render-driven refresh at all. Verified with a headless-browser check: an open, unsaved driver-name edit survives a full 17-second background-hydration cycle untouched, and a *different*, unrelated save from that same browser afterward is confirmed to carry a driver-name change a "second browser" made in the meantime, instead of the original stale page-load copy. + +This closes the highest-impact case (losing live race data) and cuts the realistic exposure window for everything else from potentially hours to about 15 seconds. It does not prevent two admins from overwriting the exact same non-race field within that same ~15-second window - that needs the full optimistic-locking approach, tracked as a separate, deliberately deferred item above. + ### Implemented: decoder/socket diagnostics Acceptance: each configured channel distinguishes an open socket from actual received data and successfully parsed passing messages. Inspecting diagnostics must not interrupt timing or rerender an active settings form. diff --git a/package-lock.json b/package-lock.json index 49f3963..dc66084 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rc-racecore", - "version": "1.4.2", + "version": "1.4.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "rc-racecore", - "version": "1.4.2", + "version": "1.4.3", "hasInstallScript": true, "dependencies": { "better-sqlite3": "^12.11.1", diff --git a/package.json b/package.json index 40219cc..7854bed 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rc-racecore", - "version": "1.4.2", + "version": "1.4.3", "private": true, "description": "RC RaceCore live timing and race control with AMMC ingest and SQLite persistence", "engines": { diff --git a/server.js b/server.js index 61e877b..3161666 100644 --- a/server.js +++ b/server.js @@ -196,6 +196,35 @@ app.post("/api/ammc/stop", async (_req, res) => { } }); +// Two admin browsers can be open at once, and each pushes its own full in-memory copy of +// state on every save - whichever save lands last silently overwrites everything else, +// including any other browser's edits since its own last page load. That risk is most +// severe for resultsBySession: a browser that's been idle for a while (so its copy is +// stale) can revert live race results computed moments ago by a different, active browser. +// /api/passings is the only route driven by live decoder data, so a recent write through it +// is a reliable signal that a session's results are currently "hot" and worth protecting. +const RESULT_PROTECTION_WINDOW_MS = Number(process.env.RC_RESULT_PROTECTION_WINDOW_MS || 15000); +const recentSessionResultWrites = new Map(); + +function protectRecentSessionResults(incomingState) { + const stored = readStoredAppState(); + if (!stored?.state?.resultsBySession || typeof stored.state.resultsBySession !== "object") return incomingState; + const now = Date.now(); + for (const [sessionId, writtenAt] of recentSessionResultWrites) { + if (now - writtenAt > RESULT_PROTECTION_WINDOW_MS) { + recentSessionResultWrites.delete(sessionId); + continue; + } + const storedResult = stored.state.resultsBySession[sessionId]; + if (storedResult === undefined) continue; + const incomingResult = incomingState.resultsBySession?.[sessionId]; + if (JSON.stringify(incomingResult) === JSON.stringify(storedResult)) continue; + if (!incomingState.resultsBySession || typeof incomingState.resultsBySession !== "object") incomingState.resultsBySession = {}; + incomingState.resultsBySession[sessionId] = storedResult; + } + return incomingState; +} + app.get("/api/state", (_req, res) => { const row = db.prepare("SELECT state_json, updated_at FROM app_state WHERE id = 1").get(); if (!row) { @@ -216,7 +245,7 @@ app.post("/api/state", (req, res) => { return; } - const stateJson = JSON.stringify(req.body); + const stateJson = JSON.stringify(protectRecentSessionResults(req.body)); const nowIso = new Date().toISOString(); db.prepare( @@ -285,6 +314,7 @@ app.post("/api/passings", (req, res) => { parsed.resultsBySession = {}; } parsed.resultsBySession[String(sessionId)] = sessionResult; + recentSessionResultWrites.set(String(sessionId), Date.now()); db.prepare( ` INSERT INTO app_state (id, state_json, updated_at) diff --git a/src/app.js b/src/app.js index 03f93ea..bbfdf88 100644 --- a/src/app.js +++ b/src/app.js @@ -22,7 +22,7 @@ import { getSessionsForEventHelper, getModeLabelHelper, normalizeStartModeHelper import { renderEventWorkspaceView } from "./event_workspace_controller.js"; import { renderEventManagerMarkup } from "./event_manager_view.js"; import { renderEventManagerView } from "./event_manager_controller.js"; -import { createDefaultAmmcConfigHelper, getManagedWsUrlHelper, loadAmmcConfigFromBackendHelper, saveAmmcConfigToBackendHelper, refreshAmmcStatusHelper, startManagedAmmcHelper, stopManagedAmmcHelper, applyPersistedStateHelper, hydrateFromBackendHelper, scheduleBackendSyncHelper, syncStateToBackendHelper, pingBackendHelper, checkAppVersionHelper, startAppVersionPollingHelper, startOverlaySyncHelper, startOverlayRotationHelper, startOverlayLiveRefreshHelper, ensureAudioContextHelper, playPassingBeepHelper, playFinishSirenHelper, playLeaderCueHelper, playStartCueHelper, playBestLapCueHelper, pushOverlayEventHelper, getSpeechVoicesHelper, speakTextHelper, announcePassingHelper, announceRaceStartedHelper, announceRaceFinishedHelper, handleSessionTimerTickHelper, tickClockHelper } from "./runtime_services.js"; +import { createDefaultAmmcConfigHelper, getManagedWsUrlHelper, loadAmmcConfigFromBackendHelper, saveAmmcConfigToBackendHelper, refreshAmmcStatusHelper, startManagedAmmcHelper, stopManagedAmmcHelper, applyPersistedStateHelper, hydrateFromBackendHelper, scheduleBackendSyncHelper, syncStateToBackendHelper, pingBackendHelper, checkAppVersionHelper, startAppVersionPollingHelper, startOverlaySyncHelper, startBackgroundHydrationHelper, startOverlayRotationHelper, startOverlayLiveRefreshHelper, ensureAudioContextHelper, playPassingBeepHelper, playFinishSirenHelper, playLeaderCueHelper, playStartCueHelper, playBestLapCueHelper, pushOverlayEventHelper, getSpeechVoicesHelper, speakTextHelper, announcePassingHelper, announceRaceStartedHelper, announceRaceFinishedHelper, handleSessionTimerTickHelper, tickClockHelper } from "./runtime_services.js"; import { connectDecoderHelper, disconnectDecoderHelper, processDecoderMessageHelper } from "./decoder_runtime.js"; import { normalizeTrainingState as normalizeTrainingStateLogic, syncTrainingLedger as syncTrainingLedgerLogic, buildDriverTrainingStats as buildDriverTrainingStatsLogic, buildDriverTimingChannelStats as buildDriverTimingChannelStatsLogic, buildDriverActivePracticeStats as buildDriverActivePracticeStatsLogic, buildEvenLapsProgress as buildEvenLapsProgressLogic, buildStepTrainingProgress as buildStepTrainingProgressLogic, buildTrainingRecommendation as buildTrainingRecommendationLogic, getTrainingGoalProgress as getTrainingGoalProgressLogic, getNextTrainingMilestone as getNextTrainingMilestoneLogic, buildTrainingMissions as buildTrainingMissionsLogic, buildTrainingHighlights as buildTrainingHighlightsLogic, getClubTrainingGoalProgress as getClubTrainingGoalProgressLogic, buildTrainingPaceBuckets as buildTrainingPaceBucketsLogic, buildTrainingWeekdayStats as buildTrainingWeekdayStatsLogic, buildTrainingStintAnalysis as buildTrainingStintAnalysisLogic, buildTrainingMarkerComparisons as buildTrainingMarkerComparisonsLogic } from "./training_logic.js"; import { renderTrainingView } from "./training_views.js"; @@ -154,6 +154,7 @@ const checkAppVersion = () => checkAppVersionHelper({ onContact: (online) => { }, getBackendUrl, getBaselineAppVersion: () => baselineAppVersion, setBaselineAppVersion: (value) => { baselineAppVersion = value; }, getPendingAppVersion: () => pendingAppVersion, setPendingAppVersion: (value) => { pendingAppVersion = value; }, getLocalStateDirty: () => localStateDirty }); const startAppVersionPolling = () => startAppVersionPollingHelper({ checkAppVersion, getAppVersionPollTimer: () => appVersionPollTimer, setAppVersionPollTimer: (value) => { appVersionPollTimer = value; } }); const startOverlaySync = () => startOverlaySyncHelper({ getOverlaySyncTimer: () => overlaySyncTimer, setOverlaySyncTimer: (value) => { overlaySyncTimer = value; }, getLocalStateDirty: () => localStateDirty, hydrateFromBackend, getCurrentView: () => currentView, renderView }); +const startBackgroundHydration = () => startBackgroundHydrationHelper({ getBackgroundHydrationTimer: () => backgroundHydrationTimer, setBackgroundHydrationTimer: (value) => { backgroundHydrationTimer = value; }, getLocalStateDirty: () => localStateDirty, hydrateFromBackend, getCurrentView: () => currentView }); const startOverlayRotation = () => startOverlayRotationHelper({ getOverlayRotationTimer: () => overlayRotationTimer, setOverlayRotationTimer: (value) => { overlayRotationTimer = value; }, getOverlayRotationIndex: () => overlayRotationIndex, setOverlayRotationIndex: (value) => { overlayRotationIndex = value; }, getCurrentView: () => currentView, getOverlayViewMode: () => overlayViewMode, renderView }); const startOverlayLiveRefresh = () => startOverlayLiveRefreshHelper({ getOverlayLiveRefreshTimer: () => overlayLiveRefreshTimer, setOverlayLiveRefreshTimer: (value) => { overlayLiveRefreshTimer = value; }, getCurrentView: () => currentView, getOverlayViewMode: () => overlayViewMode, refreshOverlayDynamics }); const ensureAudioContext = () => ensureAudioContextHelper({ state, getAudioCtx: () => audioCtx, setAudioCtx: (value) => { audioCtx = value; } }); @@ -3414,6 +3415,7 @@ let raceWizardDraft = null; let overlaySyncTimer = null; let overlayRotationTimer = null; let overlayLiveRefreshTimer = null; +let backgroundHydrationTimer = null; let overlayRotationIndex = 0; let overlayEvents = []; let lastOverlayLeaderKeyBySession = {}; @@ -3512,6 +3514,7 @@ async function init() { }); startOverlaySync(); + startBackgroundHydration(); startOverlayRotation(); startOverlayLiveRefresh(); diff --git a/src/runtime_services.js b/src/runtime_services.js index dfc2f0f..2b93efc 100644 --- a/src/runtime_services.js +++ b/src/runtime_services.js @@ -254,6 +254,22 @@ export function startOverlaySyncHelper({ getOverlaySyncTimer, setOverlaySyncTime }, 800)); } +// A browser left open on any other view never re-fetches state on its own, so the copy it +// would push on its next save only gets staler over time - the longer it sits, the more of +// another browser's meanwhile-made edits its next save can silently discard (see +// syncStateToBackendHelper/the whole-document overwrite it does). This keeps that copy fresh +// in the background without ever touching the DOM, so it can never interrupt an open, unsaved +// edit form the way calling renderView() here could. Overlay already refreshes and renders +// itself on its own, faster cycle above, so it's left alone. +export function startBackgroundHydrationHelper({ getBackgroundHydrationTimer, setBackgroundHydrationTimer, getLocalStateDirty, hydrateFromBackend, getCurrentView, intervalMs = 15000 }) { + clearInterval(getBackgroundHydrationTimer()); + setBackgroundHydrationTimer(setInterval(async () => { + if (getCurrentView() === "overlay") return; + if (getLocalStateDirty()) return; + await hydrateFromBackend(); + }, intervalMs)); +} + export function startOverlayRotationHelper({ getOverlayRotationTimer, setOverlayRotationTimer, getOverlayRotationIndex, setOverlayRotationIndex, getCurrentView, getOverlayViewMode, renderView }) { clearInterval(getOverlayRotationTimer()); setOverlayRotationTimer(setInterval(() => { diff --git a/tests/server_persistence.integration.test.js b/tests/server_persistence.integration.test.js index ecae331..04b0c13 100644 --- a/tests/server_persistence.integration.test.js +++ b/tests/server_persistence.integration.test.js @@ -287,3 +287,93 @@ test("SQLite API persists state, passings, and overlay session results", { timeo rmSync(dataDir, { recursive: true, force: true }); } }); + +test("a stale full-state save from a second browser cannot revert a session's just-arrived live results", { timeout: 20000 }, async () => { + const dataDir = mkdtempSync(join(tmpdir(), "live-rc-test-concurrent-")); + const port = await reservePort(); + const baseUrl = `http://127.0.0.1:${port}`; + const child = spawn(process.execPath, [join(projectRoot, "server.js")], { + cwd: projectRoot, + env: { + ...process.env, + HOST: "127.0.0.1", + PORT: String(port), + RC_TIMING_DATA_DIR: dataDir, + RC_TIMING_PID_FILE: "", + RC_RESULT_PROTECTION_WINDOW_MS: "300", + }, + stdio: ["ignore", "pipe", "pipe"], + }); + + try { + await waitForServer(child); + + const staleSessionResult = { competitors: {}, passings: [], adjustments: [] }; + const baseState = { + events: [{ id: "event_1", name: "Club race" }], + classes: [{ id: "class_1", name: "Buggy" }], + drivers: [{ id: "driver_1", name: "Driver One", classId: "class_1" }], + sessions: [{ id: "session_1", eventId: "event_1", name: "A final" }], + resultsBySession: { session_1: staleSessionResult }, + settings: {}, + training: {}, + }; + await fetch(`${baseUrl}/api/state`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(baseState), + }); + + // Browser A is actively timing: a live passing arrives and updates session_1's result. + const liveSessionResult = { + competitors: { driver_1: { key: "driver_1", driverName: "Driver One", laps: 1, bestLapMs: 16000 } }, + passings: [{ competitorKey: "driver_1", timestamp: 5000, lapMs: 16000, validLap: true }], + adjustments: [], + }; + await fetch(`${baseUrl}/api/passings`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + sessionId: "session_1", + passing: { timestamp: 5000, transponder: "1234567", driverId: "driver_1" }, + sessionResult: liveSessionResult, + }), + }); + + // Browser B has been idle since before the race and still has driver_1's old name and the + // stale (empty) session_1 result in memory. It now saves an unrelated edit (renaming the + // driver) and pushes its entire stale snapshot along with it. + const staleBrowserState = { + ...baseState, + drivers: [{ id: "driver_1", name: "Driver One (renamed)", classId: "class_1" }], + }; + const staleSaveResponse = await fetch(`${baseUrl}/api/state`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(staleBrowserState), + }); + assert.equal(staleSaveResponse.status, 200); + + const afterStaleSave = await (await fetch(`${baseUrl}/api/state`)).json(); + // The unrelated edit went through... + assert.equal(afterStaleSave.state.drivers[0].name, "Driver One (renamed)"); + // ...but the live race result survived the stale overwrite instead of reverting to empty. + assert.deepEqual(afterStaleSave.state.resultsBySession.session_1, liveSessionResult); + + // The protection is time-bounded, not permanent: once the window passes, a save is trusted + // again, so a legitimate later change to that same session's results (e.g. an admin + // deleting the session) still takes effect. + await new Promise((resolve) => setTimeout(resolve, 400)); + const afterWindowState = { ...staleBrowserState, resultsBySession: {} }; + await fetch(`${baseUrl}/api/state`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(afterWindowState), + }); + const afterWindow = await (await fetch(`${baseUrl}/api/state`)).json(); + assert.deepEqual(afterWindow.state.resultsBySession, {}); + } finally { + await stopServer(child); + rmSync(dataDir, { recursive: true, force: true }); + } +});