Skip to content

Keep Content's unpushed edits through a Builder refresh - #8174

Open
3mdistal wants to merge 14 commits into
alice/content-builder-status-editingfrom
alice/content-builder-refresh-keeps-edits
Open

3mdistal wants to merge 14 commits into
alice/content-builder-status-editingfrom
alice/content-builder-refresh-keeps-edits

Conversation

@3mdistal

@3mdistal 3mdistal commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #8168. A Builder refresh treats Builder as the only source, so it drops Content's edits that haven't been pushed yet:

  • Fields. A refresh rewrites every mapped property with Builder's value. A property edited in Content goes back to Builder's.
  • Titles. A refresh never updates an existing page's title. After Builder retitles an entry, the page keeps the old title and the review offers to push it back over Builder's.
  • Bodies. A body read already keeps an edited body. Nothing tells the reviewer when Builder changed that body too.

What changes

  • Fields. A refresh compares each mapped property with the value from the previous refresh. If they differ, the property is Content's edit and the refresh keeps it. If Builder changed that field too, the row records the conflict. The review then shows the item's existing "changed in Builder since sync" state until the edit is pushed or matches Builder. A property that wasn't edited takes Builder's value, as before.
  • Saves during a refresh. Properties are written under the refresh's row lock, after it re-reads the rows. A property saved while the refresh was reading Builder is compared, not overwritten.
  • Titles. Titles follow the same rule. An unedited title takes Builder's. An edited title stays, and is flagged when Builder retitled the entry. An empty title doesn't count as an edit.
  • Bodies. A body read keeps an edited body as before, and now flags it when Builder changed the body too. A refresh keeps that flag; the next body read or a push settles it. When Content never stored a Builder body for the entry, an empty body from Builder could be one Builder deleted, so a page body kept over it is flagged too, and the review offers to push it.
  • Body reads that bring field values. A body read that fetches the live entry, and a bulk read that fills many empty pages at once, also store Builder's current field values and title. The row's properties and page title now follow them under the same rules, so the next refresh doesn't mistake an old value or title for Content's edit.
  • Pushes. A push clears the conflicts for the fields and body it sends, and keeps the rest.
  • Setting an edit back. Staging the value Builder already has used to report "unchanged" and leave Content's edit in place, which the next refresh then wiped. Now that a refresh keeps the edit, that stage sets the property back to Builder's value, drops that property's own change from the row's open review item (a page-title change to the same Builder field stays), and clears the field's conflict flag, in one step under the row lock. An item left with nothing to push is closed. When the same request stages other rows, both commit together. The stage is refused with a 409, and nothing is reset, when Builder's value or the property changed after the stage read them, or while a push of the row's review item is running or awaiting reconciliation. Staging a new value takes the same lock, recheck and push check, and adds to the review item as it stands under the lock.

Each writer replaces only the conflict keys it owns: a refresh its mapped fields and the title, a body read the body, a push what it sent. The title has an entry of its own, so a property named "Title", which maps to Builder's title field too, doesn't clear the title's flag or the other way round. A body read takes the row's lock before its queue row, in the order queuing a read takes them.

Not covered

  • When Builder clears a field, Content keeps its value. A refresh skips empty values, as it already did.
  • An unreadable conflict record flags the item in the review until the next refresh replaces it.
  • Found while testing, unchanged here: while other rows have field edits pending, the source review loads bodies only for rows with known changes. So a body-only edit doesn't show in that review. That predates this PR.

Live check

Run against the agent-native-blog-article-test model, with a local Content server behind a guard that allows only that model. Content made no Builder writes. Builder-side edits went through Builder's MCP.

  1. A note staged in Content survived a full refresh. The review showed it with no conflict.
  2. After the note was changed in Builder, two refreshes each kept Content's note, and the review flagged the item as changed in Builder.
  3. Staging Builder's note in Content set Content's note back to it, and after a refresh the item had no review row. This step found the "setting an edit back" gap above.
  4. After Builder's note was restored, a refresh moved Content's unedited note to it.

Checks

templates/content: 892 tests in 35 files pass. They cover every Builder adapter, source, review, push, refresh, stage, and body hydration test file, plus BuilderSourceReviewDialog.ui, body-hydration, builder-body-hydration-pump, use-content-database, db.spec, and the new builder-refresh-local-edits.db. Each part fails its own test when reverted on its own: the review flag, the title rule, the body flag, the record carried through a rebuild, the body read's property and title writes, the bulk read's property write, the empty-body flag (its rule, its known-empty marker, and the kept-body check), and the stage that sets an edit back (its value, its conflict clear, its reread of the review item, its recheck of the row, its push check for setting back and for staging, its recheck of staged rows, and its single commit with the staged rows), the title's own conflict entry, the set-back keeping a page-title change, and the body read's lock order. tsc is clean apart from the existing virtual:react-router/server-build error. pnpm guards passes apart from the existing dead-settings-keys baseline. No copy changed.

content_product_impact:
  lane: contract_repair
  features:
    - content.feature.trust-your-connected-sources
  capabilities:
    - content.source.sync-policy
  record_change: none
  proof:
    - pnpm exec vitest --run actions/builder-refresh-local-edits.db actions/stage-builder-source-bulk-update.db actions/execute-builder-source-execution actions/resync-content-database-source.db actions/set-builder-entry-status.db (templates/content)
  rationale: A Builder refresh no longer overwrites Content's unpushed edits, and the review flags the ones Builder changed too.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

…ting' into alice/content-builder-refresh-keeps-edits
builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

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 5 potential issues 🟡

Review Details

Code Review Summary

This PR preserves Content-side Builder CMS edits across refreshes, reconciles page titles and mapped properties against their prior Builder baselines, tracks per-field/body conflicts for review, and applies “set back to Builder” edits transactionally. It also hydrates mapped values and titles from body reads. The approach is sound in principle, and the added PGlite coverage exercises many edit, conflict, and staging races. I classified this as high risk because it changes data mutation and synchronization behavior. The previously open Title-mapped-property stage-back issue is fixed: filtering now uses the local field identity, and that thread was resolved.

Findings

  • 🟡 MEDIUM: Empty-body hydration preserves and flags a previously synced body even when Content did not edit it.
  • 🟡 MEDIUM: Body conflict decisions use a pre-lock document read and can miss a concurrent Content save.
  • 🟡 MEDIUM: Stage-back can reject a now-empty change set, then a stale review decision can approve it again.
  • 🟡 MEDIUM: A concurrent rejection can leave the staged property value without an open review item.
  • 🟡 MEDIUM: Refresh can materialize values using a field-mapping snapshot that became stale before the transaction.
  • Lower-priority observations on lock duration, malformed source-value JSON handling, and page-lock order are included for completeness.

No tests were independently run; the PR description reports 892 Content tests passing. 🧪 Browser testing: Will run after this review (the reconciliation changes affect user-visible Content pages and review flows).

Comment on lines +2859 to +2862
args.bodyKept &&
args.localContent !== args.nextContent &&
(previousBodyUnknown ||
previousContent !== args.nextContent ||

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.

🟡 Unedited body Builder empties is kept and flagged as a conflict

When Builder reports an empty body, this path keeps any non-empty local body and marks it conflicted whenever the previous Builder body differs. A page whose body was synced and never edited therefore retains text Builder deleted and offers to push that stale text back. Compare the local body with the previous Builder baseline here; only preserve and flag it when it was edited or the baseline was unknown, and add a regression test for a known, unedited body Builder empties.

Fix in Builder

Comment on lines +2681 to +2687
nextValues = withBodyReadEditConflict({
previousValues: sourceValues,
nextValues,
bodyKept: !shouldWriteBody,
localContent: currentContent,
nextContent,
});

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.

🟡 Body conflict detection uses content read before the membership lock

currentContent is read before this transaction takes the membership lock, and the keep-body path has no content CAS. A Content save between that read and lock acquisition can make the conflict decision stale, potentially persisting no conflict for a local edit that now differs from Builder. Re-read the document body under the lock and base the keep/write decision and conflict marker on that locked value, or abort/retry if it changed.

Fix in Builder

Comment on lines +484 to +486
state:
fieldChanges.length === 0 && !changeSet.bodyChangeJson
? "rejected"

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 review decisions can re-approve a now-empty change set

When setting back the final field/body change rejects this change set, a review request that read the earlier open state can still update the row by ID after waiting on this transaction, overwriting rejected with approved. That leaves an approved change set with no changes to execute. Make review decisions conditional on the state/payload they read, or otherwise serialize them with this transition and handle a failed compare-and-set.

Fix in Builder

Comment on lines +369 to +370
inArray(schema.contentDatabaseSourceChangeSets.state, [
"pending_push",

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.

🟡 Concurrent rejection can leave a staged value without a review item

This locked query excludes a change set rejected after the staging action's earlier snapshot, but the transaction still writes the staged property and only updates change sets returned here; it does not create a replacement review item. Because review decisions do not take the collection lock, this interleaving can report a staged value with nothing pending to push. Serialize rejection with staging or atomically revalidate/create the review item before committing the property write.

Fix in Builder

Comment on lines +6566 to +6569
await materializeSourceFieldPropertyValues({
database: { ownerEmail: args.ownerEmail },
sourceId: args.sourceId,
fields: args.fields,

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.

🟡 Refresh can rematerialize properties using a stale field mapping

args.fields is read before replaceMockSourceRows enters the transaction, then used here to materialize Builder values. If a concurrent unbind/remap commits between that read and this write, refresh can repopulate a property that is no longer mapped, undoing the unbind and restoring removed values. Read the current mappings under the collection mutation lock inside this transaction, or validate a mapping version and retry.

Fix in Builder

This branch has not been deployed

No deployments
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