Skip to content

Finish BuilderSync publication flow#2382

Open
3mdistal wants to merge 24 commits into
mainfrom
codex/buildersync-production-qa
Open

Finish BuilderSync publication flow#2382
3mdistal wants to merge 24 commits into
mainfrom
codex/buildersync-production-qa

Conversation

@3mdistal

@3mdistal 3mdistal commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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:

  • make Builder refresh, review, and execution state explicit and idempotent;
  • bind preview authorization to the immutable originating Netlify deploy;
  • preserve Content's collaborative/editor state through the first durable save;
  • keep native slash-menu activation while deferring the chosen command across the menu's expected unmount;
  • retain the existing hard safety boundary: live writes are allowed only to agent-native-blog-article-test.

What changed

Builder source loading and review

  • Serialize automatic and manual Builder refreshes with exact-offset claims, renewal, stale-worker fencing, and orphan recovery.
  • Preserve provider field bindings while refreshing large collections.
  • Prioritize newly created Builder drafts in review and bound body loading.
  • Separate review preparation from live execution in both behavior and UI copy.
  • Scope prepared execution snapshots to the selected source/change set.

Preview authorization

  • Route Builder authorization through the approved callback and relay the result to the immutable originating Netlify preview.
  • Bind relay opener state to the originating preview site and embed the Netlify deploy ID at build time.

Content authoring reliability

  • Persist the first collaborative edit instead of letting an initial empty state win.
  • Keep multi-word slash searches open.
  • Execute a clicked slash choice in a microtask so closing the menu cannot cancel it; deduplicate same-turn clicks and skip commands after editor destruction.

Safety and operations

  • No production blog-article write path is enabled or exercised.
  • Guarded live execution remains restricted to agent-native-blog-article-test with review, validation, explicit push mode, and idempotency checks.
  • No destructive migration or backfill is included.
  • Notion OAuth and Notion Sync are out of scope.

Verification

Automated on the current branch:

  • Focused slash-menu suite: 22/22 passed.
  • Content typecheck passed.
  • git diff --check passed.
  • Focused oxlint completed with only pre-existing warnings.
  • CI and the exact Netlify Content preview are running for head 2706b53dbf767732b416a71624d0444dad8a9bcf.

Hosted frozen QA on the preceding preview:

  • H1 passed: created a real Content database.
  • H2 passed: attached only agent-native-blog-article-test; 581 rows loaded.
  • H3 passed: enabled publish updates and per-item publish/unpublish on the safe source.
  • H4 exposed the slash-choice unmount race; no Builder draft or publication write occurred.

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

  • Are refresh claims renewed and fenced around every post-provider snapshot mutation?
  • Can a prepared Builder execution ever escape its selected safe source/change set?
  • Is preview relay state bound tightly enough to the originating immutable deploy?
  • Does the slash-menu microtask preserve one chosen command across menu unmount without touching a destroyed editor?

@netlify

This comment has been minimized.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment on lines +6268 to +6279
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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Fix in Builder

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment on lines +3277 to +3290
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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Fix in Builder

@netlify

This comment has been minimized.

@3mdistal 3mdistal changed the title Fix Builder collection pagination restarts Finish BuilderSync publication flow Jul 24, 2026
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.

2 participants