FM-SYNC-HARDENING: wire visibility settings, reconnect re-pull, fail-closed permissions#61
Merged
Merged
Conversation
scripts/_ownership.mjs (NEW) shared visibility→ownership mapping honoring dmOnlyHidden + defaultOwnership (§1); module.json download URL v0.1.0 pin → latest pattern (§4).
…connect suites settings.mjs defaultOwnership default 0→2 (§1); tools/test-build-ownership.mjs (§1/§3/§4 + pins) and tools/test-sync-manager-reconnect.mjs (debounce/flap/stop + pins).
…ESTING manual steps note-sync._buildNoteOwnership uses the shared helper (§1); push failures surface + queue for retry (§4). TESTING.md adds manual steps for the visibility toggles, fail-closed path, push surfacing, and reconnect re-pull.
SyncManager re-pulls on reconnect via onStateChange → _onConnectionStateChange → debounced _resyncAfterReconnect, so Chronicle edits during a disconnect are caught without a world reload. Flapping collapses to a single re-pull.
…+ push surfacing _buildOwnership wires dmOnlyHidden/defaultOwnership via the shared helper (§1), fails CLOSED to NONE on a custom-visibility permissions error (§3), and maps per-user grants to Foundry users via the user-mapping table (§4). Create/update push failures surface via ui.notifications; updates queue for retry (§4).
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
1 participant
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.
Replaces #60 (whose branch had a stale base that couldn't be cleanly rebased through the available tooling). Same change set, on a branch that descends directly from
main→ conflict-free.What landed
Decision-free hardening of the sync module's permission model + reconnect path. No new features (no worldState/calendar live-sync — that's Phase 5).
§1 (HIGH) — dead visibility settings wired in
dmOnlyHidden+defaultOwnershipwere registered and written by the dashboard but no sync module read them —journal-sync._buildOwnershiphardcoded its own mapping. The operator toggled controls that silently did nothing.scripts/_ownership.mjs(defaultLevelForVisibility/configuredDefaultOwnership) is the single source of truth, consumed by bothjournal-sync._buildOwnershipandnote-sync._buildNoteOwnership.dmOnlyHiddenON (default) → DM-only/private content isNONEfor players; OFF → it surfaces at the configured default level.defaultOwnershipnow drives the player-visible level.§1 wire-vs-remove decision: WIRED. To keep wiring non-breaking, the registered default of
defaultOwnershipwas changed0 → 2(OBSERVER): the value was never read before while the code hardcoded OBSERVER, so 2 is the always-effective default. Worlds that never touched it behave identically; operators who set it are now honored.§2 (HIGH) — reconnect re-pull
_initialSyncDonewas a one-shot latch, so Chronicle edits during a WS disconnect were lost until a world reload.SyncManagernow drives a debounced re-pull off the connection state machine (onStateChange → _onConnectionStateChange → _scheduleReconnectResync → _resyncAfterReconnect). Genuine reconnects (after initial sync) re-run the pull; the first connect is untouched; flapping collapses to a single re-pull after 3s stable. The pull is the existing delta (/sync/pull?since=lastSyncTime).§3 (MED, security) — custom-visibility fails CLOSED
_buildOwnership's custom-visibility error path fell back tois_private ? NONE : OBSERVER— a GM-restricted custom entity withis_private=falsebecame player-visible on a transient permissions-API error. Now returnsNONEunconditionally. Fail closed, never open.§4 — while-in-here
subject_type:"user"grants map to specific Foundry users via the user-mapping table (getFoundryUserId); unmapped users dropped (under-share, no leak).ui.notifications.warn(already in the dashboard error log via_logError); idempotent PUT updates enqueue for retry on reconnect.downloadURLv0.1.0pin →latestpattern so a real release doesn't 404; the release workflow still overrides both for tagged releases..ai.md"Known API Field Discrepancies" table already documents the aliases; a one-paragraph reframe marking them explicitly load-bearing is a trivial follow-up (omitted to avoid a 50KB+ doc round-trip through the file-push tooling)._syncingboolean; player-notes page forced OBSERVER; pre-release0.1.0+ manual-only release workflow.Automated gates
node --test tools/test-*.mjs→ 392/392 (363 baseline + 21 new ownership + 8 new reconnect).tools/test-build-ownership.mjs(§1/§3/§4 + static-source pins incl. the fail-open regression pin) andtools/test-sync-manager-reconnect.mjs(debounce / flap / stop + pins).node tools/check-package-descriptor.mjs→ OK.node --checkclean on all changed scripts.Manual verification (see TESTING.md)
dmOnlyHidden→ players lose/gain dm-only content on next sync;defaultOwnership = Owner/Nonechanges the player-visible level./permissionsfor acustom-visibility entity → players cannot see it (NONE), even withis_private=false.🤖 Generated with Claude Code
Generated by Claude Code