Cover the preference upload gate with the Node test suite (#86)

[dmccoystephenson]

Summary

The per-account preference sync shipped in #85 had its reconciliation rule (mergePreferences)
covered by the Node suite while the wiring around it stayed inline in game.html, executed by no
suite at all. The state machine has been moved into
web-client/src/main/resources/static/js/game-logic.js as createPreferenceSync, which takes the
reads, writes and requests as callbacks. What is left inline is exactly that browser work — reading
and writing localStorage, re-applying what is stored to the page, and the two fetch calls.

• The upload gate (reconciled / uploadDeferred), the echo guard used while the account copy is
  being applied, and the debounce are now driven by tests through an injected clock, so the
  orderings that carry the risk are exercised without a browser.
• The defect found by hand during the review of Store interface preferences per account (#55) #85 is now covered by a regression test: a
  preference changed while the account copy is still in flight used to schedule an upload that
  could beat the response and put this browser's stale copy over the account's. Removing the gate
  makes that test fail — verified on this branch, see the self-review comment.
• No behaviour was changed. Every branch of the extracted code was carried across as it stood,
  including the failure paths (a failed load still opens the gate, and a failed upload is still
  logged rather than raised over the game).
• The #55 CHANGELOG entry was corrected to say that the gate is covered too, not only the merge
  rule.

Module(s) touched

web-client only. No backend change, and no change to the /api/session/preferences contract on
either side of it.

Test plan

• web-client/src/test/js/game-logic.test.js gains 10 tests for createPreferenceSync
• Regression for the Store interface preferences per account (#55) #85 defect fails when the gate is removed (evidence in the self-review)
• CI — Backend Build and Test and Web Client Build and Test (the latter also runs
  node --test web-client/src/test/js/)

Neither Maven nor Node could be run in the environment this branch was written in, so the CI run on
this PR's head commit is the only anchor for it; local execution is recorded as UNVERIFIED.
ProxyRouteCoverageTest reads fetch('/api/...') calls out of the rendered page in document order,
and the two preference calls were reordered by this change (the GET now precedes the PUT); the
method: option of each is still the first one following its own call, so both are still read as
GET and PUT /api/session/preferences. CI is what confirms it.

Backlog deferred this cycle

#86 was the only open issue at triage, so nothing else was deferred. PR #42 (copilot/*, draft) was
left alone as the work of another author.

Closes #86

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric, scored against the diff and against CI run
31468687551 on head 1d3ed3a.

• Scope: PASS — four files, each required: the extracted state machine (game-logic.js), the
  wiring it replaces (game.html), the tests it exists for (game-logic.test.js), and the one
  CHANGELOG sentence that named mergePreferences as the covered part. No formatting or comment
  churn outside the preference-sync section.
• Tests-new: PASS after a fix — createPreferenceSync is the only new exported function and is
  now driven by 12 tests. The first pass covered only the injected-clock path, leaving the
  setTimeout / clearTimeout / no-op onError defaults — the ones the browser is actually
  handed — exercised by nothing; two tests using the real timers were added in 1d3ed3a. Still not
  asserted: the literal 500 fallback for uploadDelayMs, since the page passes
  PREFERENCE_UPLOAD_DELAY_MS itself and asserting it would mean waiting the delay out.
• Tests-fix: PASS — verified empirically rather than by reasoning. The gate was removed on a
  throwaway commit (1ad11ec) and pushed; CI run
  31468491859 failed
  not ok 49 - a change made while the account copy is in flight is not uploaded ahead of the response (the browser's order reaching the account ahead of the response — the Store interface preferences per account (#55) #85 defect
  exactly), plus tests 50 and 54, with 55 passing. The commit was then dropped and the branch
  force-pushed back; the head carries none of it.
• Sibling structure: PASS — the addition matches its neighbours in game-logic.js: ES5
  (var/function, no arrow or const), a prose comment above the function, and an entry in the
  single exports object at the foot of the file. The test harness matches the test file's own style
  (const, arrows, node:assert/strict).
• Sibling renames: PASS — nothing was renamed. readLocalPreferences / writeLocalPreferences
  stay a pair in game.html; queuePreferenceUpload and loadStoredPreferences keep their names
  as one-line delegates so the three call sites are untouched.
• Docs: PASS — the #55 CHANGELOG entry now names the gate as covered as well as the merge
  rule. README's Development section is unaffected: the JS suite's location and command
  (node --test web-client/src/test/js/) did not move. PLAYER_GUIDE, MVP and DOCS describe player-
  facing behaviour and architecture, neither of which this changes.
• Issue resolution: PASS — The preference-sync wiring in game.html has no automated coverage #86 asked for the upload-gating and debounce behaviour to be
  exercised in CI, and for a regression matching the Store interface preferences per account (#55) #85 defect to fail those tests. Both are
  demonstrated above; the extraction follows the direction the issue suggested (a small object
  taking callbacks, leaving the reads, writes and requests inline).
• CI: PASS — both jobs green on 1d3ed3a; the JS step reports `# tests 60 / # pass 60 /

  fail 0`, 12 of them new.

• Both-modules: PASS — Backend Build and Test and Web Client Build and Test are separately
  green. The backend was not touched, so the backend job stands as the check that it was not
  disturbed.
• API-contract: PASS — no endpoint, method, request body or response shape changed; both
  fetch calls send exactly what they sent before. The two calls did swap document order (GET
  now precedes PUT), which matters because ProxyRouteCoverageTest reads them out of the
  rendered markup positionally; that test is part of the green web-client job, so the reordering is
  confirmed harmless rather than merely argued to be.
• Changelog: PASS — nothing a player can see changed, so no new entry; the existing entry was
  corrected where it had gone stale.
• Constructor-injection: N/A — no Spring component added or changed.
• Override-correct: N/A — no Java changed.

Two observations that belong to the body rather than to any changed line:

• game-logic.js — finishReconciliation sends the merged copy on uploadNeeded without
  consulting isSignedIn(), so the object does not enforce for its load path what it enforces for
  queueUpload. This is the previous inline behaviour carried across unchanged, and it is
  unreachable in the page because checkSession() redirects a signed-out player before
  loadStoredPreferences() is called. It was left alone deliberately: a coverage change is the
  wrong place to alter behaviour.
• The callbacks left inline in game.html — the localStorage reads and writes, the two fetch
  bodies, and applyStoredPreferences — remain executed by no suite, as the direction in The preference-sync wiring in game.html has no automated coverage #86
  anticipated. The Maven job renders the page and asserts on markup; it does not run its script.
  The gap is narrower than before but not closed, and a manual smoke test of the sidebar
  arrangement across two browsers is still what confirms the page end to end.

This review was posted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Merge held for a human decision, not for a defect.

The merge-readiness bar is otherwise met: CI run
31468687551 is green on
head 1d3ed3a for both modules, the self-review above is posted, the regression was demonstrated
empirically against CI, and the documentation sweep found nothing left to correct.

What holds it is the autonomous-merge rule against any single file losing more than 50 lines:
web-client/src/main/resources/templates/game.html shows 39 added against 71 deleted. Those
71 lines were moved rather than removed — they account for most of the 89 added to
web-client/src/main/resources/static/js/game-logic.js, and no behaviour was dropped along the way
— but the rule is deliberately blind to intent, so the merge is left to a maintainer who can
confirm that reading of the diff.

This comment was posted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Merge-readiness review (continued dispatch)

This pull request was left open by an interrupted Gardener dispatch. Its state has been
re-assessed from scratch this run rather than taken on the previous run's word, and the hold
recorded above has been resolved.

The hold. The autonomous-merge rule against any single file losing more than 50 lines was
matched by web-client/src/main/resources/templates/game.html (39 added against 71 deleted). That
diff has now been read line by line against its counterpart in
web-client/src/main/resources/static/js/game-logic.js, and the deletion is confirmed to be a move
rather than a removal:

• game.html:685 — queuePreferenceUpload is reduced to a delegate; the guard it used to hold
  (applying || !isSignedIn(), then the deferral when reconciled is false, then the debounce) is
  reproduced exactly in createPreferenceSync.queueUpload.
• game.html:649 — applyStoredPreferences loses its echo guard, which is reproduced in
  applyWithoutEcho, including the setTimeout(..., 0) that lets the guard outlive the call for
  asynchronous toggle events. The function is referenced from no other call site (only as the
  applyPreferences callback), so no caller was left unguarded by the move.
• game.html:689 — the load chain, the merge, the write-back and both failure paths are reproduced
  in load() and finishReconciliation; a failed load still opens the gate.
• The two fetch calls are carried into the fetchRemote / sendRemote callbacks unaltered, and
  the upload still reads local storage at send time rather than at queue time.

Ordering. createPreferenceSync(...) is now invoked at inline-script evaluation time rather
than inside a deferred callback, which would break were the module not already loaded.
game.html:216 loads game-logic.js as a classic script immediately before the inline script at
line 217, so the global is present; the module's UMD wrapper assigns each export onto window.

Anchor. CI run 31468687551 is green on head 1d3ed3a for both jobs. The Node suite reports
# tests 60 / # pass 60 / # fail 0, and the new cases were confirmed present by name in the job
log rather than inferred from the green tick — among them ok 49 - a change made while the account copy is in flight is not uploaded ahead of the response, the regression for the defect found by
hand during the review of #85.

Documentation. A fresh sweep found nothing left to correct. The README API section still
describes GET and PUT /api/session/preferences as implemented, no request or response shape
moved, and the #55 CHANGELOG entry now names the gate as covered alongside the merge rule.

Backlog. #86 was the only open issue and is closed by this change. PR #42 (copilot/*, draft)
belongs to another author and was left untouched.

Merging proceeds under the operator's explicit pre-authorization for this run, the matched path
having been disclosed on this pull request beforehand and verified above as behaviour-preserving.

This review was posted during a Gardener session (https://github.com/Stephenson-Software/gardener).