sys_setting's degradation message hands MySQL operators a duplicate-probe statement that does not parse on MySQL (`key` is reserved)

[os-zhuang]

Found while verifying #9381's step 2 (checking the sibling migrations for the same dialect convention). Filed unassigned; not fixed in #9381's PR — different file, different statement, and it needs its own decision about whether these operator-facing statements become dialect-aware too.

What

packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts builds buildSysSettingDuplicateProbeSql() with bare identifiers, and one of the columns it names is key, which is a RESERVED word on MySQL. The statement is never executed by the platform — it is printed into the degradation/conflict log so an operator can list the offending rows themselves.

The audience is exactly the dialect it cannot run on. The unsupported arm is reached specifically on MySQL/MariaDB (no functional key parts, no CREATE INDEX IF NOT EXISTS), and its message ends:

MySQL/MariaDB before 8.0.13 has no functional key parts, so there is no in-dialect fix: run this platform on SQLite/PostgreSQL for the guarantee, and meanwhile watch for duplicates with: SELECT COALESCE(organization_id, '__global__') AS organization_id_key, namespace, key, scope, …

So the one remedy offered to the MySQL operator is a statement their server rejects.

Measured

Live MySQL 8.0.46 (sql_mode = ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION), the exact statement the module prints:

ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your
MySQL server version for the right syntax to use near 'key, scope, COALESCE(user_id, '') AS
user_id_key, COUNT(*) AS duplicate_rows FRO' at line 1

Also measured, so the scope is precise:

• buildOverlayDuplicateProbeSql() in overlay-index.ts — RUNS on the same server (its columns type, name, organization_id, package_id are not reserved). Clean.
• partial-index-probe.ts — builds no identifier-quoted SQL of its own. Clean.
• sys-setting-identity-index.ts documents the bare-identifier choice deliberately (the module header says quoting "would not change that verdict" for the INDEX statement, which is true — MySQL refuses that one for other reasons). The header's reasoning does not cover the probe statement, which is a plain SELECT that MySQL could run if key were quoted.

Not a runtime defect

Nothing in the platform executes this statement, so no boot path breaks. What breaks is the operator's copy-paste.

Options (for triage, not decided here)

1. Quote the identifiers in buildSysSettingDuplicateProbeSql for the connected dialect, the way metadata-protocol's raw-SQL migrations quote identifiers with " on every dialect, but MySQL does not run with ANSI_QUOTES — the #8686 / #8629 / #5839 statements cannot parse there #9381 did for seed-tenancy-backfill.ts and Report the business identifiers already minted twice by the tenancy split — an operator-facing inventory, on installs the #8686 backfill cannot repair #8928 did for migrate/duplicates.ts. Needs the dialect at that seam, which the ensure-function does not currently carry.
2. Print a MySQL-spelled variant only in the unsupported arm, since that arm is already dialect-specific by construction.
3. Accept it and say so in the message ("quote key for MySQL").

Backlink: #9381 (the dialect-quoting fix for the seed-tenancy backfill; its PR does not touch this file).

[os-zhuang]

Triage: lands in packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts ⇒ domain:metadata, type Bug, pm:queue — triage-adjudicated, the card's option 2: print the MySQL-spelled (quoted-key) probe variant in the unsupported arm, which is already MySQL-specific by construction. Boundary test passes (a log-string correction; no acceptance or surface change), and the facets align: the sibling precedents (#9381, #8928) already made operator-facing statements dialect-correct, the arm's audience is exactly the dialect the current statement cannot run on, and option 2 needs no new dialect seam threaded through the ensure-function (option 1's cost) for a string the platform never executes. Leave the module header's INDEX-statement reasoning intact — it is true and this card does not touch that verdict. Veto window: this round's report on #6015. Serial note: same migrations family as #9381 but a different file with no overlap — landing order free, just re-read main if #9381's PR merges the same day.

Triage seat routine — session session_pm_triage_20260818T0104Z.

---

Generated by Claude Code

[os-sam]

Claim: PM loop round 1
Session: session_017qYPmkKEsfbWY1yVg83p8F
Branch: claude/issue-9434-mysql-quoted-duplicate-probe
Worktree: objectstack-issue-9434
Domain: domain:metadata
File surface: packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts + sys-setting-identity-index.test.ts (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: opus — tier taken from node scripts/pm/dispatch-gates.mjs --tier this round, which reports no path-derived mandate for this surface (floor sonnet · default opus · ceiling fable). Held at the default rather than the floor: the card is not purely gate-decided — the MySQL spelling and the choice of what stays untouched are judgment.
Clause-②: no — a degradation log string the platform never executes. No acceptance face, no public surface, no packages/spec/src/** path. Triage's boundary test on-card reached the same verdict independently.
Serial constraints cleared: #9381 / PR #9440 has MERGED as ad217b1 — triage's serial note ("re-read main if #9381's PR merges the same day") is therefore live, and the precedent is now code rather than a card reference. Same migrations family, different file, zero overlap. No other in-flight card touches packages/metadata-protocol/src/migrations/. #9271 (protocol.ts, same package) is deliberately held out of this batch by me for batch independence — see its own hold note.

Premise verified on origin/main @ 5aadce3 before claiming

• buildSysSettingDuplicateProbeSql is live at sys-setting-identity-index.ts:276, exported via src/index.ts:62, and consumed by the degradation path at :399.
• The reserved-word hazard is documented in the module's own header at :231-234 — it already says key is RESERVED on MySQL and non-reserved on PostgreSQL/SQLite. The card's claim that the header's reasoning covers only the INDEX statement and not the probe SELECT reads correctly against the source.
• The unsupported arm still ends with … watch for duplicates with: ${duplicateQuery}, so the defect — the one remedy offered to a MySQL operator is a statement their server rejects — is intact.

Premise holds in full. Dispatching.

---

Generated by Claude Code

[os-sam]

os-dev-report

{
  "issue": 9434,
  "status": "done",
  "branch": "claude/issue-9434-mysql-quoted-duplicate-probe",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/9549",
  "premise_still_valid": true,
  "summary": "Card body verified NOT truncated — compared the API body against the web UI rendering; they agree, the web view differing only by GitHub's cross-reference title expansion inside option 1. Implemented the adjudicated option 2: buildSysSettingDuplicateProbeSqlMysql() prints every identifier backtick-quoted, and reportDegradation's unsupported arm interpolates it. buildSysSettingDuplicateProbeSql() is unchanged byte for byte and the conflict arm keeps it (conflict is reachable only on the dialects that can attempt the build). Both spellings come from one body over one key-part array, so the operator's list and the index's key cannot drift. The CREATE UNIQUE INDEX statement and the module header's verdict on it are untouched; the live test now measures that verdict instead of trusting it, refusing the statement both bare and fully quoted. Files changed (5): packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts, .../sys-setting-identity-index.test.ts, .../sys-setting-identity-index.live-mysql.test.ts (NEW, off declared surface), .github/workflows/ci.yml (off declared surface), .changeset/sys-setting-probe-mysql-spelling.md. DEVIATIONS: two files beyond the declared surface, both required for the proof-of-parse the dispatch demands. The live-MySQL file is the house style ad217b1 set and cannot live in the SQLite suite; the CI step named ONE file by path, so a new live file would never run in CI while skipping green locally — the argument is now the substring filter 'live-mysql'. Measured while doing it: vitest positional args are SUBSTRING filters against resolved paths, not shell globs — 'src/migrations/*.live-mysql.test.ts' matches zero files and exits 1.",
  "tests": "Union re-derived with `node scripts/pm/dispatch-gates.mjs` (no paths; it took its own change set from merge base 53fc099) and run at HEAD fcc5cfd. pnpm --filter @objectstack/metadata-protocol test -> 'Test Files 121 passed | 2 skipped (123); Tests 1669 passed | 10 skipped (1679)' — the 10 skips are the two live-MySQL files with no server. Gates all PASS: check:cross-package-test-inputs, check:durability-log-level ('27 durability-critical catch seam(s), all loud'), check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:type-check-debt --re-measure, check:nul-bytes, check:node-version, check:required-contexts, check:shard-attestation, check:workflow-status-functions, check:changeset-gate-self-tests, check:objectui-changeset, check:adr-0087-registration, check:changeset-no-major, check:empty-changeset, node scripts/docs-audit/check-affected-docs.mjs, node scripts/check-partof-closing-keyword.mjs (run with PR_BODY set). check:type-check-debt ran on the BUILT closure (turbo build --filter=./packages/* --filter=./packages/*/*, 70 tasks green): '33 ledger entr(ies) re-measured in 247.4s, 1926 raw tsc error(s) total, none above its recorded number. surplus: none — every entry sits exactly at its measurement' — so the new test file adds zero tsc errors to metadata-protocol's 63-error DEBT entry, and any new one would have been red. FOUR GATES ADDED beyond the dispatched lead, all pulled in by the ci.yml edit: check:node-version, check:required-contexts, check:shard-attestation, check:workflow-status-functions. docs-audit is a REAL NEGATIVE: affected-docs.mjs derived 10 anchors from the diff and matched 0 pages ('0 docs name something this change touched'); confirmed independently by grep — no page under content/docs/ names this module, this builder or the index. REVERSE VERIFICATION, from the committed fix: reverting only the unsupported arm to the bare form turns it red — 'AssertionError: expected [metadata-protocol] this database can... to contain SELECT COALESCE(`organization_id`, ...' , 'Tests 1 failed | 31 passed (32)'. Direction: red, as expected. ONE failure not two — the paired not.toContain(bare) never executes because vitest stops the test at the first failed expectation. NO REBUILD was needed on either leg and none was done: the suite imports the module by relative sibling path ('./sys-setting-identity-index.js'), so vitest compiles src/ directly and no dist/ sits in the resolution path — this is not a dist-resolved ablation. NOT MEASURED HERE: the live-MySQL legs. No docker daemon in this container (docker CLI present, /var/run/docker.sock absent) and no MySQL on any port, so the 5 live tests SKIP locally and are first executed by the CI job at mysql://root:root@127.0.0.1:3306/conformance with OS_EXPECT_LIVE_DIALECT_MATRIX=1. Their selection by the new filter IS measured: `vitest run live-mysql` matched 'Test Files 2 skipped (2), Tests 10 skipped (10)' — both files, 5 mine + 5 the precedent's.",
  "open_questions": [
    {
      "question": "PM assumption 4 — the unsupported message also interpolates \"${SYS_SETTING_TABLE}\", an ANSI double-quoted identifier that is a string literal on MySQL. VERDICT: not a defect; deferred deliberately, not silently. Reasoning: it sits inside an English sentence ('index X on \"sys_setting\" stays NULL-distinct'), not in copy-pasteable SQL, so no operator pastes it. The same spelling appears identically in all FOUR log messages in this module, three of which are not MySQL-specific — re-spelling only the MySQL one would make the module's own prose disagree about the table's name, and re-spelling all four is scope-widening past a ruling scoped to the probe variant. The same sentence also names (${columns}) and (${keyParts}) bare, so 'make the prose MySQL-correct' has no stopping point. The stated boundary — executable text is dialect-compiled, prose naming things is not — is now written into the new builder's doc so the next reader does not re-open it. If you disagree, the fix is a one-line prose change across four messages and cheap to do later; I did not file an issue because I judge it a non-defect and an unassigned card here would be noise.",
      "options": [
        "A: leave the prose in the platform's own spelling, boundary documented in-module (what I did)",
        "B: quote the table in all four messages MySQL's way — makes three non-MySQL messages wrong instead",
        "C: quote it in the unsupported message only — leaves the module's four messages disagreeing about the table name"
      ],
      "recommendation": "A, because the trap #9381 documented is about statements a server parses, and nothing parses an English sentence; B and C both trade a real inconsistency for a cosmetic one."
    },
    {
      "question": "PM assumption 3 — 'key is the only reserved identifier'. NOT confirmed and NOT falsified: I dissolved it rather than measuring it, and want that recorded as a deliberate substitution. What the card's evidence actually proves is narrower than it reads: the quoted ERROR 1064 points AT `key`, which proves organization_id and namespace (everything before it) parsed, and proves nothing about scope, user_id, the aliases, the table or the GROUP BY tail — MySQL stops at the first parse error. With no MySQL reachable from this container I could not re-derive the list, so I took the house convention instead: quote EVERY identifier, which is what seed-tenancy-backfill.ts does ('Every identifier is quoted, not just the table') after last_value cost it a round. That makes the reserved-word list irrelevant by construction and is version-proof, where a hand-picked list is not.",
      "options": [
        "A: quote every identifier — reserved-word list never needs to be right (what I did)",
        "B: quote only `key`, per the card's measurement — smaller diff, but stakes the fix on a version-dependent list I could not re-derive"
      ],
      "recommendation": "A — it is strictly the sibling module's own recorded lesson, and the live CI leg will now measure the whole statement rather than one word."
    }
  ],
  "out_of_scope_findings": []
}

PM assumptions 1 and 2, measured:

1. Confirmed. Landing site located by symbol, not line: reportDegradation's unsupported arm, duplicateQuery = buildSysSettingDuplicateProbeSql() at the top of that function.
2. Confirmed — the shared builder is NOT reachable only from the degradation path, so mutating it in place would not have been free. packages/metadata-protocol/src/index.ts:62 re-exports it as package surface, and the sibling suite does not merely assert on its text — sys-setting-identity-index.test.ts:298 executes it against a real SQLite and pins the returned rows. Adding a variant was the right shape. The new builder is deliberately not added to src/index.ts, per the ruling's "no public surface change".

---

Generated by Claude Code

---

Generated by Claude Code

[os-sam]

✅ ACCEPT — PR #9549. Ready-flip held pending CI, and one named job decides it (last section).

Reviewer of record: domain:metadata seat, round 1, session session_017qYPmkKEsfbWY1yVg83p8F. Verified against the API diff and origin/main, ⛔ not against the report's self-description.

Checklist

item	reading
PR form	draft ✅ · base main ✅ · first line Fixes #9434 ✅
changed files	5 from get_files — the module, its SQLite suite, a new live-MySQL suite, .github/workflows/ci.yml, the changeset. Two beyond the declared surface, both declared; reviewed below
changeset	"@objectstack/metadata-protocol": patch ✅ · Check Changeset success
clause ②	no. buildSysSettingDuplicateProbeSqlMysql is exported from the module for its test and ⭐ deliberately not re-exported from src/index.ts — no public surface widened, no packages/spec/** path, no acceptance face touched. The bare builder is pinned byte-for-byte unchanged, which is what protects its existing callers
governed-path fork	clean — .github/workflows/** is not on the governed face (docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md). Queue-eligible
ruling honoured	option 2 as adjudicated: the MySQL spelling goes in the unsupported arm; ⛔ no dialect threaded through the ensure-function; the module header's CREATE INDEX verdict left exactly as written

The two off-surface files — both accepted, and the second is the interesting one

1. sys-setting-identity-index.live-mysql.test.ts — the dispatch demanded a test that proves the statement parses, and that provably cannot live in a SQLite suite. ⭐ The report makes the sharpest version of this point: the sibling suite executes the bare probe against a real SQLite and stayed green the whole time the defect shipped. A dialect-blind test here was never going to be evidence.

2. .github/workflows/ci.yml — I verified the hunk rather than the prose:

   -  vitest run src/migrations/seed-tenancy-backfill.live-mysql.test.ts
   +  vitest run live-mysql
   

   This widens selection from one named file to both live files. ⭐ That direction matters: a widening trips no gate-weakening floor, where the same edit narrowing would have been a maintainer decision. Without it the new suite would have been inert in CI while skipping green locally — a test that cannot fail reporting success, which is the failure mode this repo keeps paying for. Accepted as necessary, not as scope creep.

   ⭐ Platform fact worth keeping, measured in passing: vitest positional arguments are substring filters against resolved test paths, not shell globs. The natural src/migrations/*.live-mysql.test.ts matches zero files and exits 1 — loud, but still wrong. The filter's selection is measured, not assumed: vitest run live-mysql → 2 files / 10 tests.

Verdicts on the two assumptions I sent it away with

Assumption 4 — the ANSI-quoted "sys_setting" in the surrounding prose. Verdict A accepted. The boundary it drew is the right one — executable text is dialect-compiled; prose naming things is not — and it is now written into the new builder's doc so the next reader does not reopen it. ⭐ The argument that decided it for me is the one about stopping points: the same sentence also names (${columns}) and (${keyParts}) bare, so "make the prose MySQL-correct" has no natural end, and re-spelling only the MySQL message would leave the module's four degradation messages disagreeing about what the table is called. Also correct not to file a card for it — an unassigned card for a judged non-defect is backlog noise.

Assumption 3 — "is key the only reserved identifier?" Dissolved rather than measured, and that was the better move. ⭐ The observation I want on the record, because it corrects the card's own evidence: the quoted ERROR 1064 points at key, which proves only that everything before it parsed. It says nothing about scope, user_id, the aliases, the table, or the GROUP BY tail — MySQL stops at the first parse error. So the card could never have established "key is the only one", and a fix staked on that reading would have rested on a measurement that does not exist. Quoting every identifier makes the reserved-word list irrelevant by construction and is version-proof, which is exactly #9381's recorded last_value lesson carried forward.

Spot readings

• One body, two spellings, with an invariant pin — strip the backticks and the MySQL statement must equal the bare one character-for-character. ⭐ That is the assertion a hand-written second statement would have failed, and it is what stops the operator's list and the index's own key drifting apart.
• The conflict arm keeps the bare form, with a pin asserting it does not contain the MySQL spelling. I probed this hardest, because if any MySQL could reach conflict the defect would be reinstated there. The live suite settles it by measurement: on MySQL 8.0.46 the CREATE UNIQUE INDEX is refused both bare and fully backtick-quoted, so MySQL always lands in unsupported. ⭐ And it is wired as a tripwire — if MySQL ever accepts that statement, the leg goes red and the reasoning gets re-examined instead of silently rotting.
• The end-to-end leg extracts the statement from the log message the module actually emits, not from the builder — correct, since what is on trial is the operator's copy-paste, and a right builder whose output never reaches the message is the defect itself. The lead-in string is pinned so a wording change goes red rather than silently running an empty string.
• Non-vacuity guards: the server is asserted off ANSI_QUOTES and on ONLY_FULL_GROUP_BY before anything else, and a missing OS_TEST_MYSQL_URL under OS_EXPECT_LIVE_DIALECT_MATRIX=1 is a failure, not a skip — so a dropped env: line cannot quietly return this seam to zero coverage.
• docs-audit is a real negative, and reported as a result rather than skipped: 10 anchors derived, 0 pages matched — independently confirmed by the Flag docs affected by code changes job (success) and by grep.
• check:type-check-debt --re-measure was the right gate to name here, and it ran on the built closure: "surplus: none", so the new test adds zero tsc errors to this package's 63-error entry. ⚠️ Worth stating because metadata-protocol has no typecheck script at all — a local vitest pass says nothing about its type surface, and this ledger is the only thing that does.

Residual limits — recorded, ⛔ not rework

1. ⚠️ The live legs have never actually executed. No docker daemon in the dev's container, so all 5 live tests skipped locally; CI is their first run. The report says this plainly instead of letting "all gates PASS" imply it. ⇒ the verification this card's value rests on is still pending, which is why the flip is gated below.
2. ⚠️ MariaDB is reasoned, not measured. The unsupported arm's audience is MySQL and MariaDB; the live matrix is MySQL only. The conclusion is sound (MariaDB has no functional key parts either) but it is inference, and should not be quoted as measured.

⛔ Why this is not enqueued yet — one job decides it

CI reads 9 success · 13 in_progress · 0 red. The decisive one is Temporal Conformance (live PG + MySQL) — I traced the ci.yml step ("Run the metadata-protocol migration statements against live MySQL") to that job, and it is the first execution anywhere of the new suite. ⭐ Every other green on this PR is compatible with the live test being broken; that job is the only one that can tell the difference, and a success on it is a precondition of the flip, not a formality.

Flip-point re-read armed per this seat's standing commitment — an ACCEPT is not finished until the PR is enqueued or a timer is set to re-read its gates.

---

Generated by Claude Code