Skip to content

Harden challenge-progress SQLite storage and legacy migration for 3.5.0#146

Merged
minoneer merged 4 commits into
masterfrom
fix/sqlite-prerelease-hardening
Jun 11, 2026
Merged

Harden challenge-progress SQLite storage and legacy migration for 3.5.0#146
minoneer merged 4 commits into
masterfrom
fix/sqlite-prerelease-hardening

Conversation

@minoneer

Copy link
Copy Markdown
Member

Pre-release hardening of #137, fixing the confirmed findings from a deep adversarial review of the SQLite challenge-progress feature.

Release blocker fixed

  • Islands with a completed no-cooldown challenge could not be loaded at all. sqlite-jdbc's getObject(column, Long.class) throws Bad value for type Long on SQL NULL instead of returning null, so any island with a stored completion without an active cooldown (e.g. any completed one-time challenge) made load() throw on every challenge command, GUI open, and member join. Now read via getLong + wasNull, with a regression test that fails on master.

Legacy migration made faithful to the old sharing semantics

The one-shot YAML→SQLite import now reproduces exactly what the pre-SQLite runtime read, per the server's configured challengeSharing (per-island is the only model going forward):

  • challengeSharing: player servers no longer lose non-leader progress. Per-player completion files were live data; they're resolved through the player's own recorded island coordinates (deterministic, the source the old runtime keyed by), with island membership (party.members) as fallback, and max-merged into the island.
  • challengeSharing: island (default) servers no longer resurrect stale data. Only island-named files and the leader's file (the old loader promoted it when no island file existed — and only then) are imported; other per-player files are residue from a former player-sharing era and stay in place.
  • players/*.yml fallback gating matches the old loader: per player under player sharing, per island under island sharing, never when superseded by a completion file.
  • Scan failures now abort the migration (and plugin enable) instead of writing the one-shot marker with data unread; per-file YAML corruption still skips gracefully. The migration logs summary counts for unmapped and stale sources.

Robustness fixes

  • The completion cache's graceful-degradation catch could never fire: Guava wraps runtime failures in UncheckedExecutionException, not ExecutionException. DB read failures now degrade to an empty map with a WARNING instead of uncaught player-facing errors.
  • Member-join permission re-grant no longer throws NoSuchElementException (aborting all grants) when stored progress references a challenge removed from challenges.yml.
  • SQLite repository now gets the plugin logger via @PluginLog (was Guice's JIT JUL logger) and logs store/pragma failures at WARNING instead of FINE.
  • Removed the dead startup re-creation of the legacy completion/ directory that undid the migration's cleanup every boot.
  • Admin docs: new "Progress storage" section (DB path + backup guidance), challengeSharing documented as ignored — with the explicit warning to leave the key in place for the first start after upgrading, since the importer reads it to interpret legacy data.

Verification

  • 9 new tests; 7 fail against master's sources (NULL-cooldown round-trip ×2, member-file import, per-player/per-island yml gating ×2, leader-file precedence, repository-failure fallback). The member-join guard is verified by inspection (constructing ChallengeLogic in a test is impractical); the island-sharing residue test pins the no-import invariant.
  • The migration changes were themselves adversarially reviewed against the actual pre-SQLite loader (3599595e); all confirmed findings from that second pass are fixed in the last commit.
  • Full multi-module build green.

Consciously deferred to 4.0 (where the catalog rework replaces the completion runtime): write-behind cache crash window, PRAGMA synchronous connection scope, sqlite-jdbc version shadowing by Paper's bundled driver, IslandKey strictness. Tracking issue to follow.

🤖 Generated with Claude Code

minoneer and others added 4 commits June 11, 2026 23:32
sqlite-jdbc's getObject(column, Long.class) throws "Bad value for
type Long" for SQL NULL instead of returning null, so any island
with a stored completion without an active cooldown became
unreadable, breaking all challenge access for that island. Read
the column via getLong + wasNull instead.

Also raise repository error logging from FINE to WARNING so store
and pragma failures are visible in production logs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The migration now reproduces exactly what the previous version
read, per the configured challengeSharing mode:

- challengeSharing=player: per-player completion files were live
  data; resolve them through island membership (party.members,
  not just party.leader-uuid) and merge them into the island, so
  non-leader members no longer lose their progress.
- challengeSharing=island: only island-named files and the
  leader's file (which the old loader promoted on first load) are
  imported; other per-player files are stale residue from a former
  player-sharing setup and are left in place instead of inflating
  island progress.
- players/*.yml challenge sections only merge when the island has
  no completion file, matching the old fallback-only behavior, and
  are skipped for players without an island.

Scan failures now abort the migration (and plugin enable) instead
of marking the one-shot import complete with data unread, and the
migration logs summary counts for unmapped and stale sources.

Also catch Guava's UncheckedExecutionException in the completion
cache fallback (the previous catch of ExecutionException could
never fire for runtime failures) and drop the dead re-creation of
the legacy completion/ directory on startup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Skip stored progress for challenges no longer present in
challenges.yml when re-granting permission rewards on member join
(previously threw NoSuchElementException and skipped all grants).
Inject the plugin logger into the SQLite repository via @PluginLog
instead of Guice's JIT JUL logger. Document the new per-island
SQLite storage and the challengeSharing removal in the admin docs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Player sharing: gate the players.yml fallback per player (matching
  the old per-player completion-file check) instead of per island,
  so an inactive member's progress is not skipped just because
  another member had a completion file; resolve per-player
  completion files through the player's own recorded island
  coordinates first (deterministic, the source the old runtime
  used) with island membership as fallback; leave island-named
  files in place (the player-sharing runtime never read them).
- Island sharing: only promote the leader's completion file when no
  island-named file exists, matching the old rename-on-absence
  behavior instead of max-merging stale leader data.
- Count superseded player-yml sources in the residue summary, and
  document that challengeSharing must stay in challenges.yml for
  the first start after upgrading so the importer can interpret
  legacy data.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@minoneer minoneer merged commit 431869e into master Jun 11, 2026
1 check passed
@minoneer minoneer deleted the fix/sqlite-prerelease-hardening branch June 11, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant