Sync fixes: partial pushes were erasing data Chronicle had, and resync stopped at 500 - #83
Merged
Merged
Conversation
…bility rules and pairing key `PUT /api/v1/campaigns/:id/maps/:mapID/markers/:markerID` is a full replace: Chronicle binds the body into `apiUpdateMarkerRequest` (pointer fields for the optional columns), `mapService.UpdateMarker` assigns every one of them onto the loaded row, and `mapRepo.UpdateMarker` UPDATEs `entity_id`, `visibility_rules` and `foundry_id` with no COALESCE. A key absent from the JSON body therefore binds to nil and lands on disk as NULL. `ChronicleMarkerConfigDialog.#onSave` rebuilt its payload from scratch with exactly eight keys — name, description, x, y, pin_category, color, icon, visibility — so every GM edit of a Chronicle marker from the Foundry map viewer silently cleared the marker's entity link, its per-user allow/deny list, and the module's own Foundry pairing key. Spread the stored marker under the edited fields, matching the sibling `PinConfigDialog.#onSave` in the same file (and the Chronicle web UI's own marker form, which carries entity_id on the PUT). The dialog already holds a full copy of the marker. The read-only keys the spread carries along (id, map_id, created_at, entity_name, …) are undeclared on the wire struct and ignored by the binder; `updated_at` is not `expected_updated_at`, so the spread does not accidentally engage optimistic concurrency. tools/test-marker-config-payload.mjs drives the real save action off DEFAULT_OPTIONS.actions['save-marker'] against a stubbed form and pins all three columns through an edit, plus the edits-win, safe-fallback and no-concurrency-token behaviours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Aa645128uaFxbQyvabMizq
…ntract says why
Chronicle's cross-repo half of this sweep made every JSON update endpoint a
partial update: an absent key preserves, an explicit null clears, a present
value replaces. That is what makes this module's bodies correct rather than
lossy. Before it, actor-sync's {name} rename push bound is_private=false on
the server and PUBLISHED a hidden character entity to every player, and
calendar-sync's five-key note pushes turned is_recurring and all_day off and
cleared the entity link.
API-CONTRACT.md said the opposite — that an absent is_private meant public.
It says the contract now, at the endpoints it governs (entities, calendar
events, map markers), with the version-skew note that a pre-R4 Chronicle
still whole-replaces.
The client change is discipline, not payload. Each narrow body carries a
comment saying it is narrow ON PURPOSE and must not be "hardened" by echoing
untouched fields back — an echo re-arms the endpoint for the next writer and
goes stale, which is exactly how the marker dialog lost its pairing key.
tools/test-partial-put-contract.mjs pins the shapes by parsing the real
object literals.
The marker dialog's spread stays: harmless against a merging server,
load-bearing against an older one.
Red-then-green: adding is_private/parent_id back to nameBody reddens
"actor-sync: a rename pushes only {name}". Full suite 770/770.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aa645128uaFxbQyvabMizq
… was done
Chronicle stage 18 lifted the server-side ceiling on the sync pull. The
module carried the matching one on its own side, and fixing only the server
would have left the operator exactly as stuck.
Both places that need every entity in the campaign had this loop inline:
while (hasMore && page <= 5) { … per_page=100 … }
JournalSync.resyncAll and the dashboard's _buildEntityGroups. A hard
500-entity ceiling with no signal in either. Past it, entities were not
synced slowly or partially — they were never fetched, and the GM was shown a
completed resync and a full-looking dashboard regardless. The dashboard's own
comment even said "up to 500 for now", which is the kind of note that stops
being read.
Replaced both with the shared, pure scripts/_entity-page-walk.mjs. It keeps a
bound — a broken server that always answers with a full page would otherwise
spin forever — but sets it at 200 pages (20,000 entities), past any real
campaign, and returns `truncated` when it stops early. A bound is fine. A
bound nobody is told about is the defect.
resyncAll now warns the GM by name when the walk was truncated: "resync
covered the first N entities only — the campaign has more". The dashboard
warns to console and records it on its cache.
One implementation rather than two, for the same reason _realtime-date-guard
and _calendar-subresources exist: two copies of a walk drift, and this pair
had already drifted in their unwrappers while sharing the same wrong ceiling.
tools/test-entity-page-walk.mjs, proven red then green by restoring
MAX_ENTITY_PAGES=5 and dropping the truncated flag:
- 1234 entities all arrive, in order, with no duplicates, including the last.
With the old ceiling: "expected 500 to equal 1234".
- a short page ends the walk with no extra request; an exact multiple costs
one confirming request, because a full final page cannot be told from more
without asking.
- hitting the bound sets truncated. With the old loop it did not, which was
the whole bug: "a walk that stopped early must say so".
- an empty campaign, a custom page size, and a normalizer returning undefined.
- a source pin: neither caller may keep a `page <= 5` literal, and both must
route through walkEntityPages so they cannot drift apart again.
CLAUDE.md records the rule and points at Chronicle's server-side twin,
including the honest note that this module does not consume the new
next_cursor because it pulls via GET /entities rather than POST /sync.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aa645128uaFxbQyvabMizq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
The module's half of a product-wide fix. Chronicle's update endpoints treat an absent key as a write, so any client sending a partial body silently erased the fields it left out. This module was one of those clients, in several places — and its own resync was quietly incomplete.
Chronicle's matching server-side fixes are in
keyxmakerx/Chronicle#588. Merge that one too — several of these fixes are one half of a pair.What was being erased
The marker dialog wiped entity, visibility rules and the pairing key. Editing or dragging a map marker sent a narrow body; everything omitted was cleared, including the marker's Foundry pairing — so the link between a Chronicle marker and its Foundry counterpart was being destroyed by the act of moving it.
Calendar event pushes turned off recurrence, all-day and the entity link. A five-key push cleared
is_recurring,all_dayandentity_id, because those are value-typed on the server and assigned unguarded. A GM editing an event title in Foundry could un-repeat a recurring event without touching it.The module's narrow bodies are now safe by construction, and
API-CONTRACT.mdrecords why — the next person writing a push needs to know that omission is destructive, not neutral, before they write it.The resync that lied
resyncAllstopped at 500 entities and reported completion. Past that point entities were never seen, and the GM got a "resync complete" message with a full-looking result. Now walks the whole set and surfaces truncation instead of hiding it.Also in this branch
sync.statusfrom a shape the server no longer emits, so it silently never ran/revealvisibility, flat relation routesOne contract change to be aware of
Chronicle's
PUT /calendar/datenow requiresyear. It previously accepted a body with no year and silently wrote year 0 — and this module is the live caller: all four push sites sendyear: date.year, and anundefinedfrom a calendar adapter is dropped byJSON.stringifyand arrives as an absent key. So a sync could zero a campaign's date with no error. It is now a loud 422. Updating this module'sAPI-CONTRACT.mdto record that is booked.🤖 Generated with Claude Code
https://claude.ai/code/session_01Aa645128uaFxbQyvabMizq
Generated by Claude Code