Seed sites3.next_page_id after a full site import (fixes TyE306KSH4)#738
Open
IvanTheGeek wants to merge 1 commit into
Open
Seed sites3.next_page_id after a full site import (fixes TyE306KSH4)#738IvanTheGeek wants to merge 1 commit into
IvanTheGeek wants to merge 1 commit into
Conversation
IvanTheGeek
force-pushed
the
fix-next-page-id-after-import
branch
from
July 6, 2026 06:50
fa89004 to
66bb46f
Compare
After importing/restoring a whole site, sites3.next_page_id was left at its default (1) instead of being advanced past the imported pages' ids. Page ids come from that stored counter (inc_next_page_id), unlike post and category ids which self-heal via max(id)+1. So the next new page started at id 1, collided with the just-imported pages, and nextPageId() gave up after 100 laps [TyE306KSH4]. Real-world repro: a site imported with pages up to id 145 while next_page_id stayed at 1 -> new-topic creation broke. Fix: in importCreateSite(), after inserting the pages, seed next_page_id to max(highest numeric imported page id + 1, the dump's declared next_page_id), via a new bumpNextPageId() transaction method: `update sites3 set next_page_id = greatest(next_page_id, ?) where id = ?` (GREATEST => idempotent, never lowers the counter). Non-numeric/special page ids are ignored (toIntOption); the seed is computed in Long and rejected at the Int page-id ceiling (throwBadRequestIf, TyEIMPPAGEIDMAX), so it can't silently overflow if a page id is exactly Int.MaxValue. This changes only the id handed to *new* pages created after the import — existing pages keep their ids, so links/backlinks to them are unaffected. The upsert path (upsertIntoExistingSite) already advances the counter via nextPageId() and is left untouched. Adds an app-level regression test (SitePatcherAppSpec) that reproduces the bug (fails on unpatched code: "1 was not equal to 4") and a DumpMaker.importCreateSite() helper for it. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> I, Ivan Rainbolt <talkyard@github-commits.ivanthegeek.com>, agree to the Contributor License Agreement, docs/CLA-v2.txt.
IvanTheGeek
force-pushed
the
fix-next-page-id-after-import
branch
from
July 6, 2026 06:52
66bb46f to
ab3d5b9
Compare
IvanTheGeek
marked this pull request as ready for review
July 6, 2026 06:55
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
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.
The bug (
TyE306KSH4)After a full site import/restore,
sites3.next_page_idis left at its default (1) instead of being advanced past the imported pages' ids. Page ids come from that stored counter (inc_next_page_id), unlike post & category ids which self-heal viamax(id)+1. So the next new page starts at id 1, collides with the just-imported pages, andnextPageId()gives up after 100 laps →TyE306KSH4.Real-world repro: a self-hosted site imported with pages up to id 145 while
next_page_idstayed at1→ creating any new topic failed.The fix
In
importCreateSite(), after inserting the pages, seednext_page_idtomax(highest numeric imported page id + 1, dump's declared next_page_id)via a newbumpNextPageId()transaction method:GREATEST⇒ idempotent, never lowers the counter. Non-numeric/special page ids and ids larger thanIntare ignored (toIntOption), so it can't overflow.This changes only the id handed to new pages created after the import. Existing pages keep their ids, so links/backlinks to them are unaffected. The
upsertIntoExistingSitepath already advances the counter vianextPageId()and is left untouched.Tests
SitePatcherAppSpec+ aDumpMaker.importCreateSitehelper): imports a dump with a stale counter, assertsnext_page_idis seeded past the max page id, and creates a new page withoutTyE306KSH4. Verified it fails on unpatched code (1 was not equal to 4) and passes with the fix.Heads-up: 3 e2e specs need their expected page ids updated (not a fix bug)
With the fix, 3 specs regress:
api-upsert-posts.2br.d,alias-anons-basic-perm.2br.f,alias-anons-basic-temp.2br.f. The exact cause:The e2e site-builder assigns page ids as
opts.id + 19000(tests/e2e-wdio7/utils/site-builder.ts:127), so test dumps carry pages at ids ~19002–19004 while declaringnextPageId = 100— internally inconsistent, i.e. exactly the stale-counter condition this fix heals. On unpatched code the counter isn't seeded, so new pages gap-fill the free low ids (2, 3…), which these specs hard-code (c.SecondPageId = '2'). With the fix, the counter is correctly seeded past the max (→ 19005), so new pages get high ids and the hard-coded assertions fail.Only pages created after import are affected — existing pages/links are not, and real-world sites (contiguous ids) behave identically to before. These 3 specs — or the builder's
+19000/nextPageIdinconsistency — just need their expected ids updated. I left them untouched since they encode intent about your test harness; happy to follow up whichever way you prefer.Already-imported sites (one-time heal)
The fix prevents new damage; it doesn't retro-heal sites imported before it (like the repro). A safe, idempotent one-liner (no-op on healthy sites):
Deliberately not shipped as a migration — a bulk row-update across all tenants at startup felt like your call, not something to run unasked.
Full working log & investigation
The complete diagnosis, the app-test proof, and the full e2e testing (including a fairly deep investigation into why the fix changes those 3 specs) are documented here, if useful:
https://forum.ivanthegeek.com/-189
Take it as-is, modify freely, or just use it as a pointer — whatever's most helpful. 🙂
Authored by Ivan (IvanTheGeek); the fix + analysis were done with AI assistance (Claude), as noted in the commit and the working log — flagging for transparency.