Skip to content

Make Content database row writes reliable - #2779

Merged
3mdistal merged 11 commits into
mainfrom
codex/content-feedback-row-mutations
Aug 13, 2026
Merged

Make Content database row writes reliable#2779
3mdistal merged 11 commits into
mainfrom
codex/content-feedback-row-mutations

Conversation

@3mdistal

@3mdistal 3mdistal commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

Content's AI and table UI couldn't reliably create one database row, update only the fields a caller named, or find and update a row by a stable business identifier. Unknown or malformed values could be silently skipped or coerced, retrying could create duplicates, and a writer could overwrite changes made after its last read.

That made ordinary workflows such as recording feedback exactly once much harder to trust than they should be.

What changes

get-content-database now tells callers exactly which workspace authority, space, database, and backing page they are targeting. It also returns the current schema revision and the stable ID, type, writable status, and accepted shape of every property.

Using that contract, Content can now:

  • create one row with strictly validated non-Blocks values;
  • sparsely update one exact item/document pair while preserving every omitted field; and
  • upsert by a configured natural key: a text property, such as “Feedback ID,” whose value uniquely identifies the logical row.

One shared mutation path validates the exact table, schema, permissions, and current row, applies one atomic write, and reads the result back inside that transaction. Its receipt includes stable row IDs and link, the target tuple, schema and row revisions, affected fields, payload digest, idempotency result, and verified values.

Two safeguards make concurrency and retries predictable:

  • A compare-and-swap conflict means the caller sends the schema and row revisions it just read. If either has changed, the action returns a typed conflict and writes nothing instead of clobbering newer work.
  • An idempotent retry means the caller supplies a stable retry key. Repeating the same request returns the original receipt instead of applying it again; reusing the key with a different payload fails loudly.

Unknown, computed, system, source-managed, Blocks, relation, and malformed values are rejected instead of disappearing or becoming plausible empty data. The existing permissive import/source normalizer remains unchanged. Blocks mutation, relationships, form submission, source sync, whole-database migration, Slack routing, and live table configuration remain outside this PR.

Implementation notes

  • add-database-item, exact-ID update-database-item, and upsert-database-item-by-key are thin action wrappers over the shared transaction path.
  • Databases can designate one ordinary text property as their natural key. Existing values must be usable and unique; durable claims prevent two writers from claiming the same key.
  • Additive migrations store the natural-key setting and durable mutation receipts for SQLite and PostgreSQL.
  • A narrow shared action error type carries safe conflict codes and details across the HTTP action transport and browser export. Arbitrary internal errors remain generic.
  • Authorization is rechecked inside the lock window. Receipt replay checks current row visibility before returning stored data, and success uses the row snapshot verified inside the transaction so a later legitimate edit cannot turn an already-committed write into a false failure.
  • The table UI omits its empty default title and successful mutations publish the existing owner-scoped action event so other clients refresh.

Reconciliation with stable Blocks identity

PR #2778 landed first, so this branch was merged with current main before its final review. The overlap was narrow and both contracts remain intact:

Verification

Exact artifact: main 33e390c4374b517cfbc215122efe0a5f713d4dd9 → head e47b413a2d9671db6fe9b8acce780cbf06dd205f.

Local prospective-merge-tree checks pass:

  • 120/120 focused row-mutation, Blocks seeding/identity, Content database lifecycle, and migration tests;
  • 219/219 Content database tests and 115/115 Content parity tests;
  • Content typecheck, formatting, git diff --check, all 53 repository guards, and the Content product-impact checker tests.

Exact-head GitHub checks pass for the branch-relevant lanes, including Content DB, Content parity, typecheck, build, lint/format, security, core integration, and PostgreSQL row locking/concurrency. The PostgreSQL 17 job initializes the framework and Content schemas and runs the 13 row-mutation tests, including strict values, sparse compare-and-swap updates, authorization and replay privacy, natural-key claims, stale conflicts, and concurrent idempotent retries.

One bounded exact-head review focused only on the conflict resolution and row/identity interaction found no actionable defects. It confirmed the combined deletion transaction and the v81-v84 migration ordering preserve both frozen contracts.

Authenticated local-interface acceptance used task-owned disposable accounts and a disposable SQLite database. It demonstrated contract discovery, an untitled UI row, strict create, exact-ID sparse update with omitted-field preservation, natural-key create and update, idempotent replay with the same stable row, schema and row-revision no-write conflicts, receipt/read-back truth, automatic visibility in a second open client, and denial to unshared identities.

Cleanup read-back returned not_found; direct SQL found zero fixture documents, database rows, or memberships; the server port closed; and the fixture path was removed. No new fixture was created for the base reconciliation.

Acceptance boundaries

  • The authenticated interface run was local and task-owned, not a deployed fixture. Browser observations were same-context evidence because an independent tester couldn't acquire a separate compatible browser handle.
  • No production/customer data, Alice canonical table, Slack, provider, deployment, or production-connected environment was touched.
  • Alice approved the product contract and explicitly authorized squash merge only if the exact-artifact Land gates pass. Deployment remains outside this PR.

Review focus

  • Does every invalid, unauthorized, or stale input fail before any partial write or receipt disclosure?
  • Do the database lock, natural-key claim, and receipt ordering guarantee one effect on both SQL dialects?
  • Does the row revision cover the intended non-Blocks row fields without pulling computed values into the contract?
  • Are the typed conflict details useful without exposing internal errors?
content_product_impact:
  lane: contract_fulfillment
  features:
    - content.feature.durable-foundations
    - content.feature.data-that-keeps-itself-right
  capabilities:
    - content.object.database
    - content.property.typed
    - content.property.constraints
    - content.agent.action-parity
  record_change: none
  proof:
    - deterministic Content database and parity suites
    - authenticated task-owned local interface fixture
    - "disposable PostgreSQL 17 CI lane: 13/13 row-mutation tests passed on exact head"
    - bounded exact-head review of row and stable Blocks identity interaction
  rationale: This fulfills the approved reliable single-row mutation contract while leaving Blocks relationships Slack routing live table configuration and deployment to their named owners.

@github-actions

github-actions Bot commented Aug 10, 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.

@3mdistal 3mdistal changed the title Make Content database row mutations reliable Make Content database row writes reliable Aug 10, 2026
@3mdistal
3mdistal marked this pull request as ready for review August 12, 2026 15:45

@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

PR #2779 consolidates Content database row creation, exact-row updates, and natural-key upserts behind a strict, transactional contract with schema/CAS checks and durable idempotency receipts. The overall direction is sound: validation is centralized, receipts are persisted atomically, and the action error contract is shared across Core and Content. This is high risk because it changes authorization and database mutation semantics.

Key Findings

  • 🔴 Row-level authorization is stale across the lock window: update and natural-key upsert paths check the target row before waiting, but do not recheck it immediately before writing inside the transaction.
  • 🔴 Receipt replay can expose revoked row data: replay returns stored row snapshots before authorizing the receipt’s document, including create and exact update retries.
  • 🔴 Untitled row creation regresses: the table sends its default empty title, while the new action rejects empty strings.
  • 🟡 Cross-client refresh is missing: the shared mutation path no longer emits the refresh signal used by other open clients.
  • 🟡 Read-back can report a committed write as failure: verification occurs after locks are released and can race with a legitimate subsequent edit.

The migration’s timestamp syntax was reviewed against the repository adapter, which rewrites the SQLite form for PostgreSQL, so no migration finding is included.

🧪 Browser testing: Will run after this review (PR touches UI code)

Comment thread templates/content/actions/_database-row-mutation.ts
Comment thread templates/content/actions/_database-row-mutation.ts
Comment thread templates/content/app/components/editor/database/DatabaseView.tsx Outdated
return built;
}),
);
return verifyCommittedResult(result);

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.

🟡 Publish the database refresh signal after mutation

The consolidated mutation path returns after commit/readback without emitting the refresh signal that the replaced add/upsert actions published. The initiating UI invalidates its own queries, but other open clients relying on useDbSync() can remain stale after a successful create, update, or upsert. Publish the signal only after an applied commit.

Additional Info
Reported by 1 of 4 reviewers; compared against the replaced action implementations and the shared Content sync contract.

Fix in Builder

Comment thread templates/content/actions/_database-row-mutation.ts Outdated
@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

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

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

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

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@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 has a few items to flag 🟡

Review Details

Incremental Code Review Summary

The latest commit closes three previously reported gaps: the preview-menu E2E fixture now sends the mutation envelope, property deletion re-reads the natural-key pointer while holding the database mutation lock, and the natural-key migration DDL is retry-safe. The broader row mutation implementation continues to provide strict validation, CAS revisions, exact-row authorization, durable idempotency receipts, and shared action error transport.

Four parallel reviewers found no genuinely new actionable defects in the latest changes. One reviewer repeated the older PostgreSQL timestamp concern; it was excluded because it is an existing/stale finding already covered by the migration review and the shared migration runner’s dialect rewrite. The refresh-signal issue remains open and was intentionally not reposted.

The three fixed review threads were resolved before this submission:

  • Preview-menu mutation envelope
  • Delete-property natural-key lock re-read
  • Retry-safe natural-key migration

Risk remains high because this PR changes database mutation and concurrency contracts.

🧪 Browser testing: Will run after this review (PR touches UI code)

@netlify

This comment has been minimized.

@3mdistal
3mdistal merged commit 99a8c34 into main Aug 13, 2026
96 checks passed
@3mdistal
3mdistal deleted the codex/content-feedback-row-mutations branch August 13, 2026 20:06
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