Skip to content

fix(driver-sql): envelope the unbacked-conflict-target upsert refusal - #8571

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-8445-upsert-conflictkeys-envelope
Aug 13, 2026
Merged

fix(driver-sql): envelope the unbacked-conflict-target upsert refusal#8571
os-zhuang merged 1 commit into
mainfrom
claude/issue-8445-upsert-conflictkeys-envelope

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8445

The local twin of #8413's remote-face refusal. SqlDriver.upsert let SQLite's error escape exactly as raised when conflictKeys named columns with no backing PRIMARY KEY or UNIQUE index.

Premise verified before implementing

Measured on this branch's base (origin/main @ 719a21bf), local face, knex + better-sqlite3:

upsert('plain', { email: 'a@b.com', title: 'x' }, ['email'])
-> THREW name=SqliteError code=SQLITE_ERROR status=undefined
ownProps=[stack, message, code]
msg=insert into `plain` (`created_at`, `email`, `id`, `title`, `updated_at`)
values ('2026-08-13T…', 'a@b.com', 'ib21mSZ…', 'x', '2026-08-13T…')
on conflict (`email`) do update set … - ON CONFLICT clause does not
match any PRIMARY KEY or UNIQUE constraint

The card's premise holds exactly as filed. One detail it did not record, and the fix depends on it: knex builds the message as statement, then -, then SQLite's own sentence — so SQLite's text, the only channel that can discriminate this condition, survives to this face and the remote twin's recognition applies here unchanged.

What changed

packages/drivers/driver-sql/src/sql-driver.ts only:

  • isUnbackedConflictTargetError — a narrow match on SQLite's sentence. Not code: SQLite raises a plain SQLITE_ERROR, the same generic code a syntax error carries, so a code-based test would swallow every other statement failure. Deliberately not isUniqueViolationError, which answers the opposite condition (an index exists and the row violated it).
  • refuseUnbackedConflictTargetVALIDATION_ERROR / 400, original error kept as cause.
  • One line in upsert's catch, placed before the autonumber retry branch: the condition is never an autonumber collision (collidingAutoNumberReservations asks isUniqueViolationError, false here, so the probe would query the sequences table for nothing and rethrow raw), it is never transient, and it is the only placement that envelopes both exits — a recognition after the branch still escapes raw whenever mayRetry is false.

The payload was the larger half.mapDataError builds the envelope from code/status; with neither set it falls through and serves the thrown message as the whole body — the statement, bound values inlined. So the pins assert the leak as well as the envelope.

The judgment call triage delegated: narrow, by measurement

SQLite-recognition-first at the driver-sql call site, not a shared dialect-spanning predicate in @objectstack/types. Ground: this container has no Postgres or MySQL server, so their wording for this condition could not be measured, and transcribing it from memory is the guesswork the dispatch ruled out. MySQL is not even obviously the same condition — knex compiles onConflict().merge() there to ON DUPLICATE KEY UPDATE, which takes no conflict target. Postgres and MySQL therefore keep exactly the behaviour they have today. Measuring them, and deciding whether the predicate then belongs beside isUniqueViolationError, is tracked on #8567 (unassigned).

Wording

#8413's remote refusal, verbatim, first sentence for first sentence (#5240 — one condition, one wording). TursoDriver picks its face from url, so a second wording would make the answer a property of the connection string. The remote text is right for this face as written, including its "table created before its unique declaration was emitted as DDL" clause: this face does create declared indexes, but it cannot create one over a table already holding duplicates, so the same table arrives here with the same missing index.

The wording pin here is one-way by construction — it catches a reword of this face, not of the remote one. The two-way runtime comparison needs a package that can import both faces, which is driver-turso; that is outside this card's declared file surface, so it is filed as #8568 (finding) rather than added here.

Tests

New: sql-driver-upsert-conflict-target-envelope.test.ts — 6 pins. Envelope (codeandstatus, never a bare toThrow() — the un-fixed driver threw for this input too); leak (statement and bound values absent from the caller-visible message, still reachable via cause); wording; and three controls that must not have moved — the same conflictKeys upsert merges when a declared unique: true backs it, the default id merge key still works, and an unrelated statement failure is not swallowed as this refusal.

pnpm --filter @objectstack/driver-sql test → 91 files | 1463 passed | 52 skipped
pnpm --filter @objectstack/driver-sql typecheck → clean
pnpm --filter @objectstack/driver-turso test → 37 files | 984 passed

Reverse verification, direction predicted before running: with the call-site line removed, the envelope pin goes red on its first assertion (not through an "it resolved" branch — this input was always refused), the leak and wording pins red with it, and the three controls stay green. Measured: 3 failed | 3 passed, first failure expected 'SQLITE_ERROR' to be 'VALIDATION_ERROR'. Restored with git checkout from the commit.

Scope

No change to what upsert accepts or rejects — the same calls fail, they fail legibly. Hence patch, not minor: #8413's driver-turso half was minor because it started refusing writes accepted before, which this does not. Untouched: #8441's batch-verb code limb, and schema-drift.ts (#8557).


Generated by Claude Code

…#8445)
`SqlDriver.upsert` let SQLite's error escape as raised when `conflictKeys`
named columns with no backing PRIMARY KEY or UNIQUE index: `code:
'SQLITE_ERROR'`, `status: undefined`, and a message that is the statement
with its bound values. `mapDataError` reads `code`/`status` to build the
response envelope, so with neither set it fell through to its default branch
and shipped that SQL text to the caller as the whole body.
The condition is now recognised at the throw site and re-raised as
`VALIDATION_ERROR` / 400 with the original error kept as `cause`. The wording
is `driver-turso`'s remote refusal (#8413), first sentence for first sentence
(#5240 — one condition, one wording).
Recognition matches SQLite's own sentence, the only channel SQLite fills for
this condition. Postgres/MySQL wording is unmeasured and deliberately not
guessed at; generalising is tracked on #8567.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VoxQqG5FiUHZKCST7KDoZC
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 10:14pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via @objectstack/driver-sql)
  • content/docs/getting-started/glossary.mdx(via @objectstack/driver-sql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/driver-sql)
  • content/docs/plugins/anatomy.mdx(via @objectstack/driver-sql)
  • content/docs/plugins/packages.mdx(via @objectstack/driver-sql)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/driver-sql)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/driver-sql)
  • content/docs/protocol/objectql/query-syntax.mdx(via @objectstack/driver-sql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/driver-sql)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 13, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 13, 2026 22:35
@os-zhuang
os-zhuang added this pull request to the merge queueAug 13, 2026
Merged via the queue into main with commit 2342ee4Aug 13, 2026
27 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8445-upsert-conflictkeys-envelope branch August 13, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

drivers(sql): an upsert whose conflictKeys have no backing unique index throws a raw SqliteError — the local twin of #8413's consequence 2

2 participants

@os-zhuang@claude