Skip to content

fix(types): require a violation phrasing in isUniqueViolationError's message limb (#8590) - #8730

Merged
qq9340100 merged 3 commits into
mainfrom
claude/issue-8590-unique-violation-superstring
Aug 14, 2026
Merged

fix(types): require a violation phrasing in isUniqueViolationError's message limb (#8590)#8730
qq9340100 merged 3 commits into
mainfrom
claude/issue-8590-unique-violation-superstring

Conversation

@qq9340100

@qq9340100qq9340100 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Fixes#8590

isUniqueViolationError's message limb was a bare unique constraint, and a word pair is not a condition. Every dialect that can say "this row violated a unique constraint" can also say "there is no unique constraint here", and the same two words sit adjacent in both — so the shared predicate answered true for errors meaning the exact opposite of what it detects. rest-server.ts maps that verdict to 409 UNIQUE_VIOLATION: a client told to change a value when nothing was ever compared, on a status an SDK will not retry.

The limb now requires a violation phrasingunique constraint failed (SQLite) or violates unique constraint (Postgres).

Measured per dialect, on live servers

Ruling 1 of the dispatch, and the card's own caution. All three supported dialect families (sql-driver.ts recognises sqlite / postgres / mysql and no others), each driven through both conditions plus the NOT NULL and FOREIGN KEY near misses that share the wording:

  • SQLite via better-sqlite3, knex 3.3.0
  • PostgreSQL 16.13 via pg 8.22.0, knex 3.3.0
  • MariaDB 10.11.14 via mysql2 3.23.1, knex 3.3.0 — the MySQL-wire family the Duplicate entry / ER_DUP_ENTRY / errno 1062 vocabulary was written for
sqlite ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint
was true, WRONG (the reported defect)
postgres there is no unique constraint matching given keys for referenced table "t"
was true, WRONG (42830 -- found by this sweep, see below)
postgres there is no unique or exclusion constraint matching the ON CONFLICT specification
false (the pair is not adjacent in this one)
mysql the condition cannot arise: knex compiles to ON DUPLICATE KEY UPDATE, which
carries no conflict target. Confirmed against a live server (the statement
simply succeeds), strengthening #8567's `.toSQL()`-only evidence.

Postgres was not clean either, and that chose the fix

The card was filed reading the collision as SQLite-only, with Postgres escaping "by luck of word order". That reading was too kind. Sweeping the dialects raised PostgreSQL 42830 — a FOREIGN KEY referencing a non-unique column — where Postgres puts unique constraintadjacent in its own absence sentence, with no PRIMARY KEY or in front of it.

That is what decided between the card's two candidates:

candidateSQLite ON CONFLICTPG 42830
negative lookahead on the missing-index sentencefixedstill wrong
require a violation phrasingfixedfixed

A lookahead is a blocklist: it can only ever enumerate the absence sentences somebody already tripped over, and it is keyed to one dialect's current wording. The allowlist restores the module's own stated default — unrecognised is false — to the message channel, which is the direction the module already argues for everywhere else.

Blast radius (the main risk, per Ruling 5)

Both spellings the retired limb covered are preserved exactly, per Ruling 2 — it was inherited verbatim from the REST branch #6250 replaced and covered SQLite's UNIQUE constraint failed: t.cand Postgres' violates unique constraint "...". The unique violation, duplicate key and duplicate entry limbs are untouched, as are the code and errno channels, so MySQL's Duplicate entry path never went through the narrowed limb at all.

A mechanical scan of every string literal in the repo found 17 whose verdict moves. None is a regression: they are prose, a different predicate's vocabulary (looksLikeInternalErrorLeak keeps its own separate list in error-leak.ts), fixtures asserted through the status-passthrough path, or the defect sentences themselves. Verified by reading and by test — the five consumer packages were run, not edited:

@objectstack/types 333 passed
@objectstack/objectql 3591 passed
@objectstack/rest 1903 passed
@objectstack/driver-sql 1637 passed (54 skipped)
@objectstack/metadata-protocol 1317 passed
@objectstack/service-messaging 229 passed

Reachability: latent, confirmed — not raised

The card assumed latent. Confirmed by reading both gates. SqlDriver.upsert is the only site compiling a caller-supplied conflict target, and its isUnbackedConflictTargetError check runs first and unconditionally in the catch, throwing a refusal that declares status: 400. mapDataError then reads declaredHttpStatus (line 721) before the unique-violation branch (line 871), so the 400 wins.

One nuance worth recording: that refusal keeps the raw driver error as its cause, and this predicate walks cause — so the refusal object itself answered true before this change. Only the status gate stood between that and a wrong code on the wire, which is why the verdict is now pinned rather than left to it.

Pins

Reverse verification: restoring the bare limb was predicted to produce 13 failures — 12 in the new suite and the flipped SQLite pin. It produced exactly those 13, test for test. The fix was committed first, so the restore came out of a real commit.

Docs correction folded in (second commit)

unbacked-conflict-target.ts's module head still described the collision as live, reasoned from it, and conditioned the disjointness prohibition on "while #8590 is open". All three claims are false after this change, and the prohibition was never meant to expire — the two predicates answer inverse questions permanently. Corrected in place, and the paragraph now also records that Postgres did not escape "by luck of word order" (42830). Prose only, no emitted code, so no second changeset. Folds in #8732, which is closed as shipped here.

This was originally left out because the card's file surface was drawn to keep a verdict change off consumer packages; the PM extended the surface by this one file rather than leave a knowingly-false instruction about these two predicates in the tree — the same failure mode this card exists to fix, one file over.

Verification

Gates run after the final commit, at HEAD 3ebd8608e. main moved twice during this card; it is merged in (c1d030823, merging ff3e3fcc5) and the whole closure rebuilt before measuring, so the ratchets measure the tree that merges rather than a stale one:

pnpm check:nul-bytes OK (5750 files, no raw control bytes)
pnpm check:query-options-erasure OK (ratchet holds, none new; baseline verified against ff3e3fc)
pnpm check:type-check-coverage OK (64/77 packages)
pnpm check:type-check-debt OK (33 ledger entries re-measured, none above its recorded number)
pnpm --filter @objectstack/types test 333 passed (12 files)
pnpm --filter @objectstack/types typecheck clean

Re-derived against the actual diff with scripts/pm/dispatch-gates.mjs; it surfaced the changeset family, which the dispatch prompt did not name because the changeset did not exist yet. Those were run too: check-empty-changeset, check-changeset-no-major, check-adr-0087-registration all pass.

Generated by Claude Code

…message limb (#8590)
The `unique constraint` limb matched a word pair, not a condition, so every
sentence saying a unique constraint is ABSENT was claimed as a violation of one.
Measured on live servers across all three supported dialect families (SQLite via
better-sqlite3, PostgreSQL 16.13 via pg 8.22.0, MariaDB 10.11.14 via mysql2,
all through knex 3.3.0), in both directions plus the NOT NULL / FOREIGN KEY
near misses.
The dialect sweep found a SECOND instance the card did not know about: PG 42830
(`there is no unique constraint matching given keys for referenced table`),
raised by a FOREIGN KEY referencing a non-unique column, puts the pair adjacent
in Postgres' own absence sentence. That rules out the negative-lookahead
candidate, which is a blocklist keyed on SQLite's wording and still answers true
there. The limb is now an allowlist of violation phrasings, restoring the
module's stated default (unrecognised is false) to the message channel.
Both spellings the retired limb covered are preserved: SQLite's
`UNIQUE constraint failed: t.c` and Postgres' `violates unique constraint "..."`.
The code/errno channels and the duplicate key/entry limbs are untouched.
#8567's pin is inverted rather than deleted, and the absence sentences are
pinned per dialect in a new suite covering the code channel too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@vercel

vercelBot commented Aug 14, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 14, 2026 5:05pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/types.

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

  • content/docs/plugins/packages.mdx(via @objectstack/types)

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.

… (#8590)
The paragraph still described the superstring collision as live and reasoned
from it, and conditioned the disjointness prohibition on "while #8590 is open".
All three claims are now false: the predicate no longer claims that error, the
issue is closed, and the prohibition was never meant to expire — the two
predicates answer inverse questions, so a limb travelling between them produces
a confident inverted answer permanently, not until some card lands.
Also records what the dialect sweep disproved: Postgres did not escape the
collision "by luck of word order". PG 42830 puts `unique constraint` adjacent in
its own absence sentence, which is why the fix is an allowlist of violation
phrasings rather than a negative lookahead on SQLite's wording.
Prose only — no emitted code changes, so no changeset. Folds in #8732.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@qq9340100
qq9340100 marked this pull request as ready for review August 14, 2026 17:24
@qq9340100
qq9340100 added this pull request to the merge queueAug 14, 2026
Merged via the queue into main with commit bbbfcfcAug 14, 2026
29 checks passed
@qq9340100
qq9340100 deleted the claude/issue-8590-unique-violation-superstring branch August 14, 2026 17:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@qq9340100@claude