Finish BuilderSync publication flow#2382
Conversation
This comment has been minimized.
This comment has been minimized.
|
Here's a visual recap of what changed: Open the full interactive recap |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🟡
Review Details
Code Review Summary
This update adds leased, compare-and-swap coordination for Builder CMS refreshes, propagates the observed continuation offset through automatic UI refreshes, fences completion against the owning worker, and hardens Netlify preview relay routing with immutable deploy origins. The core approach is sound: the exact-offset guard, owner-specific renewal/release, orphan recovery, and database-backed regression coverage address the reported out-of-order pagination failure. I assessed this as standard risk because it changes shared refresh/data coordination and callback routing, but not credentials or publication APIs.
Key Findings
🟡 MEDIUM
- Metadata claim ownership is not preserved across all existing metadata writers. A stale writer can erase an active claim or resurrect a claim after the refresh has completed, allowing later refreshes to be blocked or to resume stale continuation state.
The new tests cover competing claims, orphan recovery, completion fencing, and preview-origin cases well. The remaining concern is that the claim is embedded in metadataJson while legacy Builder-source mutations can still replace that JSON outside the new CAS/fencing helpers.
🧪 Browser testing: Will run after this review (PR touches UI code)
| const updated = await db | ||
| .update(schema.contentDatabaseSources) | ||
| .set({ | ||
| syncState: args.syncState ?? "linked", | ||
| freshness: | ||
| args.readState === "error" || | ||
| args.progress?.partial || | ||
| args.suspiciousEmpty | ||
| ? "stale" | ||
| : "fresh", | ||
| capabilitiesJson: nextJson.capabilitiesJson, | ||
| metadataJson: nextJson.metadataJson, |
There was a problem hiding this comment.
🟡 Stale metadata writers can resurrect a completed refresh claim
Other Builder-source mutations still replace metadataJson without a version predicate. A writer that read the source while this claim was active can commit after the refresh completion removes the claim, restoring the old claim, continuation offset, and fetch state; it can also erase an active claim before the worker mutates rows. Fence all Builder metadata writers with the same CAS/merge mechanism (including write-mode and federation/required-field paths) so claims cannot be erased or resurrected.
Additional Info
Found independently by 2 of 3 code-review agents; deduplicated as one concurrency issue.
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🟡
Review Details
Code Review Summary
This increment adds a bounded Builder review snapshot path alongside the refresh coordination work. It avoids loading heavy Builder body sidecars for every review row and improves review ordering so new draft creations are considered before in-place updates. The prior metadata-writer fencing comment remains unresolved and is intentionally not reposted. I assessed this as standard risk because the change affects shared data loading and review behavior for large collections.
Key Findings
🟡 MEDIUM
- The initial candidate scan still selects every local document body before it knows which documents need a heavy review snapshot. This undermines the advertised bounded-body optimization and can retain/transfer all article bodies for a large Builder collection, recreating the memory and timeout risk the change is intended to reduce.
The new review snapshot helpers, dialect-specific JSON projection, candidate classification, and regression coverage are otherwise well structured. One agent raised a null-metadata CAS concern, but a claimed source necessarily has non-null claim metadata, so that scenario is not actionable. The existing stale metadata-writer race remains open and was skipped as required.
🧪 Browser testing: Will run after this review (PR touches UI code)
| const rows = await getDb() | ||
| .select({ | ||
| documentId: sourceRows.documentId, | ||
| sourceRowId: sourceRows.sourceRowId, | ||
| sourceQualifiedId: sourceRows.sourceQualifiedId, | ||
| provenance: sourceRows.provenance, | ||
| bodyHydrationStatus: items.bodyHydrationStatus, | ||
| currentHash: builderReviewSourceValueTextProjection( | ||
| BUILDER_CMS_BODY_BLOCKS_HASH_KEY, | ||
| ), | ||
| currentContent: builderReviewSourceValueTextProjection( | ||
| BUILDER_CMS_BODY_CONTENT_KEY, | ||
| ), | ||
| localContent: documents.content, |
There was a problem hiding this comment.
🟡 Review candidate scan still transfers every document body
This candidate query selects documents.content for every source row before narrowing to documents that need a heavy review. On a large Builder collection, opening a review still transfers and retains all local article bodies, defeating the bounded-body optimization and risking the same memory/request-time limits this path is intended to avoid. Use a compact persisted hash/baseline indicator or a server-side candidate comparison, then fetch full content only for candidate document IDs.
Additional Info
Found independently by 2 of 3 code-review agents; the existing metadata-writer fencing issue was excluded as an already-open comment.

Problem
BuilderSync's production user flow still failed across several independent boundaries: large Builder collections could restart pagination, preview OAuth could return to the wrong deploy, first collaborative edits could be lost, review preparation looked like a live send, draft execution could use an unscoped snapshot, and slash-menu choices could disappear when closing the menu unmounted the selected row.
Together these defects prevented a reliable Content → safe Builder publication pass even though individual actions often succeeded in isolation.
Approach
Keep the final-mile behavior narrow and guarded:
agent-native-blog-article-test.What changed
Builder source loading and review
Preview authorization
Content authoring reliability
Safety and operations
blog-articlewrite path is enabled or exercised.agent-native-blog-article-testwith review, validation, explicit push mode, and idempotency checks.Verification
Automated on the current branch:
git diff --checkpassed.2706b53dbf767732b416a71624d0444dad8a9bcf.Hosted frozen QA on the preceding preview:
agent-native-blog-article-test; 581 rows loaded.The exact-head hosted rerun of pointer and keyboard Code Block activation, followed by H1–H10 publication QA, remains the acceptance gate. This PR does not claim publication success until that real-interface evidence is green.
Review focus