Skip to content

fix: preserve Content database property mutations - #3314

Open
3mdistal wants to merge 9 commits into
mainfrom
codex/content-ai-property-mutation-parity
Open

fix: preserve Content database property mutations#3314
3mdistal wants to merge 9 commits into
mainfrom
codex/content-ai-property-mutation-parity

Conversation

@3mdistal

@3mdistal 3mdistal commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Problem

Content users can ask the built-in AI to create or update rows in a database. In live testing, the AI reached the correct mutation action but either dropped requested property values or confused a number in the database title (such as PR #3314) with the database's stable ID. The action then truthfully rejected the request or stored only the incomplete input it received.

Approach

Make the agent discover the database and its mutation contract first, then give it a typed property-entry format that is difficult to construct incorrectly. Authority identity is no longer authored by the model: the authenticated server derives it from the selected database after the normal access check.

Typed entries are canonicalized into the existing propertyValues contract before validation, retry hashing, or writing. UI and programmatic callers can continue using propertyValues, and equivalent inputs retain the same retry identity.

What changed

  • Agent-facing create, update, and natural-key upsert schemas now accept discriminated property entries such as { propertyId, propertyType: "number", value: 3314 }; each property type exposes the correct JSON value shape.
  • Agent-facing targets contain only stable space, database, and backing-document IDs. The server still accepts an optional legacy authority assertion, rejects it when it conflicts, and returns the server-derived authority in receipts.
  • The mutation boundary verifies every asserted property type against the live discovered schema and excludes model-only type assertions from the idempotency digest.
  • The regression eval now requires exact-title discovery, inspection of the resolved stable database ID, typed values, and discovery-before-mutation ordering.
  • The existing roadmap projection test now exercises the PR's discovered target, schema revision, compare-and-swap, and idempotent replay contract instead of its obsolete pre-PR upsert signature.

Safety and operations

The authorization boundary remains server-side: selecting a stable target does not grant access, and the backing database document is authorized before its authority scope is derived. Schema revision checks, row revision checks, sparse-update behavior, idempotency receipts, verified read-back, and loud typed failures are preserved. There is no migration, feature flag, credential, or destructive data operation in this follow-up.

Verification

  • The complete Content suite passes: 188 files and 2,361 tests, with 3 expected failures and 21 gated skips. This covers create, sparse update, replay, conflicting retry-key reuse, authorization revocation, and the corrected roadmap projection.
  • The focused mutation database suite passes all 14 SQLite tests, including omitted authority and a mismatched typed property assertion.
  • The focused schema and discovery-first eval suites pass all 29 tests.
  • Content typechecking passes, and all 63 repository guards pass.
  • Independent authorization review found no security or correctness blocker. Its one P2 eval-coverage finding was repaired and confirmed resolved in the allowed follow-up.
  • Exact-head CI and deployed beta Content AI/UI acceptance are pending for d01629941a4bddd18d7186747706525f8441995c; earlier live evidence is intentionally not claimed for this revised artifact.

Review focus

  • Does server-derived authority preserve the intended access boundary for personal and organization databases?
  • Are the typed agent entries expressive enough while remaining canonical with existing propertyValues callers and retry identities?
  • Does the eval now prove that the misleading title is resolved to the exact stable database target before mutation?
  • Are compatibility and failure modes clear for optional legacy authority assertions and stale schema/row revisions?
content_product_impact:
  lane: contract_repair
  features:
    - content.feature.durable-foundations
  capabilities:
    - content.agent.action-parity
    - content.object.database
  record_change: none
  proof:
    - pnpm --filter content test
    - pnpm guards
  rationale: The change repairs model-to-Action mutation preparation while preserving the canonical database mutation and authorization contracts.

@netlify

This comment has been minimized.

@3mdistal
3mdistal marked this pull request as ready for review August 21, 2026 02:01
builder-io-integration[bot]

This comment was marked as outdated.

@github-actions

github-actions Bot commented Aug 21, 2026

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.

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

Review Details

Incremental Code Review Summary

The latest revision broadens the change from property-shape repair to typed agent entries, server-derived authority, discovery-first evaluation, and compatibility updates to database mutation callers. The typed schema and server-side type assertions are directionally sound, and the tests cover the main happy path plus mismatched types and discovery behavior. Four parallel reviewers found four new medium-severity issues, including two idempotency compatibility gaps and a dedicated eval runner that cannot satisfy its own discovery requirements. This is standard risk because it changes shared mutation contracts, replay semantics, and evaluation gates.

Key Findings

  • 🟡 MEDIUM — Legacy authority changes retry identity: the optional legacy authorityScope remains in the digest even though authority is now server-derived.
  • 🟡 MEDIUM — Empty typed entry lists execute as empty mutations: propertyEntries: [] is accepted despite the contract requiring omission when no values are requested.
  • 🟡 MEDIUM — Typed retries can fail after schema changes: type assertions run before durable receipt replay, breaking retries that should be independent of model-only assertions.
  • 🟡 MEDIUM — Dedicated preservation runner lacks required discovery tools: it registers only the mutation action while the scenario/scorer requires list-then-inspect discovery, making the runner fail even for correct model behavior.

🧪 Browser testing: Skipped — the changes affect actions, eval infrastructure, and tests, with no browser-visible UI surface changed.

Comment on lines +744 to +746
const { propertyTypeAssertions: _propertyTypeAssertions, ...canonicalInput } =
input;
return digest({ operation, ...canonicalInput });

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.

🟡 Exclude legacy authority assertions from retry identity

authorityScope is now an optional legacy assertion while the server derives the authoritative scope from the selected database, but databaseMutationPayloadDigest still hashes the target unchanged. The same mutation and idempotency key can therefore hash differently when retried through an old caller with authorityScope versus the new agent contract without it, returning IDEMPOTENCY_KEY_REUSED instead of replaying. Hash only the server-derived stable target fields while retaining the assertion for mismatch validation.

Fix in Builder

Comment on lines +134 to +136
export const databasePropertyEntriesSchema = z
.array(databasePropertyEntrySchema)
.max(1_000)

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.

🟡 Reject empty typed property entry lists

propertyEntries is optional, but when supplied as [] it is accepted and canonicalized to an empty propertyValues object plus empty type assertions. This lets an agent perform a successful no-op create/update/upsert despite the contract saying an empty list must never be used, preserving the original dropped-property failure mode. Require .min(1) when the field is present or canonicalize an empty list to the omitted representation.

Fix in Builder

Comment on lines 1189 to +1190
const initial = await loadContext(input.target, "editor");
const inputDigest = payloadDigest("create", input);
assertPropertyTypeAssertions(initial, input.propertyTypeAssertions);

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.

🟡 Check durable replay before typed assertion validation

assertPropertyTypeAssertions now runs before replayReceipt. If a committed typed mutation is retried after its property definition changes type, the assertion throws before the existing durable receipt can be returned, even though propertyTypeAssertions is excluded from the payload digest and legacy propertyValues retries still replay. Preserve the replay-first path, then validate type assertions only before a new write.

Fix in Builder

Comment on lines +21 to +22
const runner = await createAgentRunner({
actions: {

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.

🟡 Register discovery actions in the dedicated preservation runner

The dedicated runner registers only add-database-item, but the scenario and expected_property_values scorer require list-content-databases followed by get-content-database and validate their inputs. A correct model cannot perform discovery in this runner, so the gated pnpm eval:property-preservation command will always fail the discovery checks. Register fixture-only discovery actions with deterministic responses, or use a create-only scenario/scorer for this runner.

Fix in Builder

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