fix(publish): republish the site when an entry's public state changes - #397
Draft
asachs01 wants to merge 1 commit into
Draft
fix(publish): republish the site when an entry's public state changes#397asachs01 wants to merge 1 commit into
asachs01 wants to merge 1 commit into
Conversation
A listing page is a static artefact: its `base.loop` over a content table is expanded once, at full-publish time, and baked into the slot. Per-entry publishing rewrites that entry's own artefact and nothing else, so the moment an entry enters or leaves public visibility every index that links to it is wrong, and stays wrong until a human presses Publish. A deleted post keeps a live card pointing at a 404; a post scheduled for 09:00 is missing from the index until someone notices. Publish, scheduled publish, unpublish, and delete now ask `server/publish/autoSitePublish.ts` for a background full-site republish, which is the only thing that re-expands a loop. Four rules make that affordable and safe: - Coalesced: the first request opens a 5s batch window and every request inside it is absorbed, so a backlog of forty posts costs one publish. The window is half a `publishScheduler` tick, so one tick's due rows land in a single batch. - Never re-entrant: at most one run is in flight, because two would race the slot swap. Requests raised during a run collapse into exactly one follow-up window. - Never recursive: guaranteed structurally, with an architecture test that fails the build if a new caller appears. A runtime origin check would lie — plugin publish.* handlers run in the QuickJS worker and their RPCs return on their own event-loop task. - Never a surprise publish: `publishDraftSite` promotes the draft, so a run proceeds only while the draft already matches what is published. It then changes no page and re-expands the loops and nothing else. With unpublished site edits present the run is skipped and logged. The rebuild is background work, so an author's request returns as soon as their entry commits. A failed run is logged and dropped: the entry publish already committed and the bake reaches `swapSlot` only on success, so the live site is untouched. Attribution is the system actor, the convention the scheduled-publish tick already uses — nobody asked for this site publish, which is also why `publishDraftSite` now takes `string | null`. `AUTO_SITE_PUBLISH_ON_ENTRY_CHANGE=0` (also false/off/no) restores the old behaviour for operators who publish on their own cadence. Default is on: the stale listing is a correctness bug, not a preference.
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.
Summary
A published listing page never learns that its entries changed.
A listing is a static artefact like any other page. Its
base.loopover a content table is expanded once, at full-publish time, and the resulting cards are baked into the slot.publishDataRowrewrites the entry's own artefact and nothing else — it never re-expands anybody else's loop. So the moment an entry enters or leaves public visibility, every index that links to it disagrees with reality, and keeps disagreeing until a human presses Publish.We hit both halves of that on a production install this week:
handleRowItemDeleteprunes the artefact — but its card stayed on/blog, linking readers to the 404./blogfor hours.publishScheduler.tscallspublishDataRowto flipstatusand does nothing else, so the index was never rebuilt. Three more posts were scheduled behind it, each queued up to do the same.Neither is a misconfiguration. The listing is simply frozen at whatever the last full publish saw.
Why this can't live in a plugin
We tried that route first. The
content.entry.created/.updated/.deletedhooks insrc/core/plugin-sdk/types/hooks.tsfire at the right moments, but there is nothing a handler can call to fix the listing:cms.content.republishAll(server/plugins/protocol/targets.ts).republishSinglePage(server/publish/republish.ts) reads the existing snapshot viagetPublishedPageSnapshotById, re-renders it, and discards the HTML. Its documented purpose is firing hook side-effects, not writing artefacts. It cannot see content published after that snapshot, and it cannot rewrite a listing.cms.site.publishtarget.The gap is in the CMS, so the fix is too.
The change
New
server/publish/autoSitePublish.ts. The four paths that change an entry's public visibility — publish, scheduled publish, unpublish, delete — callrequestAutoSitePublish(db, uploadsDir)next to theemitContentEntry*call each already makes, and the module runs one backgroundpublishDraftSite, which is the only thing that re-expands a loop.Everything else in the module exists to make "a full publish per entry change" affordable and safe.
Coalescing
The first request opens a 5-second batch window; every request inside it is absorbed. Draining a backlog of forty posts costs one site publish, not forty. Same shape as the collab relay's
schedulePersist— arm once, ignore while armed.Five seconds is bounded on both sides. The floor: long enough to swallow a burst — a human working down a publish queue, an agent looping over a backlog, or one
tickPublishSchedulerpass firing up toTICK_BATCH_LIMITrows in sequence. The ceiling: the scheduler polls every 10s, so a wider window would let a 09:00 post stay off the listing longer than it took to notice it was due. It is a fixed window from the first request rather than a resetting debounce, because a resetting debounce can be held open indefinitely by a steady trickle of publishes.Re-entrancy
A site publish swaps the static slot, so at most one run is ever in flight. Requests raised during a run collapse into exactly one follow-up window — they may describe an entry the running publish read the database too early to see.
publishDraftSite's ownwithPublishLockis untouched and still serializes against manual publishes.Recursion
A site publish must not be able to trigger a site publish. That is guaranteed structurally, and pinned by
src/__tests__/architecture/auto-site-publish-callers.test.ts: only the named visibility-change call sites may referencerequestAutoSitePublish, andpublishSite.tsmay not import it.We deliberately did not add a runtime origin check. Plugin
publish.before/publish.html/publish.afterhandlers run in the QuickJS worker and their RPCs come back on their own event-loop task, so anAsyncLocalStorage-style flag would report "not inside a publish" for the one caller that could actually recurse. A guard that lies is worse than no guard; the gate is honest about being structural.Never a surprise publish
This is the load-bearing decision and the one worth arguing about.
publishDraftSitepromotes the draft site. If an operator is mid-redesign, promoting it because an author published a blog post would push unfinished work live — exactly the leak that the explicit, step-up-gated Publish button andsite_publish's "publishing stays a separate operation" contract exist to prevent.So a run proceeds only while
getDraftPublishStatusreports the draft already matches what is published. In that state the run changes no page at all: its only effect is re-expanding the loops against current content. When the draft has unpublished edits the run is skipped and logged — and that operator is about to publish anyway, which fixes the listing.The cost is stated plainly: while site edits are pending, listings stay as stale as they are today.
Background and failure behaviour
The author's request returns as soon as their entry commits; the rebuild happens on the timer. A failed run is logged under
[publish:auto]and dropped — the entry publish already committed, and the bake reachesswapSlotonly after it succeeds, so a failure leaves the live site byte-for-byte as it was and the entry in the state the author asked for.Attribution is the system actor (
published_by_user_id = null), the conventionpublishDataRow's scheduled-publish path already uses: nobody asked for this site publish, and blaming it on whoever happened to publish a post would be a lie. That is whypublishDraftSite'sadminUserIdwidens tostring | null, matchingpublishDataRow.Configuration
AUTO_SITE_PUBLISH_ON_ENTRY_CHANGE— default on, because a listing that contradicts its entries is a correctness bug rather than a preference. Operators who publish on their own cadence set0/false/off/no. Only an explicit off-token disables; an unset, empty, or unrecognised value keeps the default rather than silently turning a correctness fix off because of a typo.It is read by the module that owns the feature, the way
renderCache.tsreadsRENDER_CACHE_MAX_ENTRIES, rather than being threaded throughserver/config.tsto three unrelated call sites.compose.prod.ymlpasses it through.Test coverage
src/__tests__/server/autoSitePublish.test.ts— 7 tests, all against a real SQLite database through the real repositories. Nothing is mocked, so a run that claims to publish has actually written a snapshot. Assertions count rows insite_snapshots, one per full publish.src/__tests__/architecture/auto-site-publish-callers.test.ts— 4 gates on the caller set and the structural non-recursion rule.The coalescing assertion was mutation-tested: giving each deferred request its own run turns the expected
2into11, so the test genuinely bites rather than passing by construction.Verification
Checklist
docs/features/publisher.md(new "Keeping listings honest" section),docs/server.md,docs/features/content-storage.md,docs/reference/architecture-tests.md,docs/deployment/README.md,.env.example,.env.production.example,CHANGELOG.md.Known costs and things left alone
site_snapshotsrow plus onedata_row_versionsrow per page — the same amplification a manual Publish causes, now happening more often. Worth knowing before enabling on a high-churn install.handleContentEntriesPublish/handleContentEntriesDelete/handleContentEntriesDeleteManyhave nouploadsDirand already skip artefact writes and prunes entirely, so wiring them here would have meant fixing a second, unrelated bug in this PR. Left for a separate change.Happy to adjust the batch window, the flag name, or the draft-drift policy if you'd rather these behave differently.