Skip to content

Commit 61aea8b

Browse files
committed
Merge origin/main into claude/issue-11717-lead-key-allowlist
Resolved scripts/docs-audit/affected-docs.mjs in favour of the key-position allowlist, taking main's version as the resolution base so #11710 (#11630) is carried forward rather than reverted: all of its scan-by-scan `$route:` and member-access fixtures are kept, and its two boundary pins (`-` and Unicode, both left deliberately unmoved there) are FLIPPED rather than duplicated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
2 parents 820cfab + dd4113e commit 61aea8b

5 files changed

Lines changed: 579 additions & 112 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
fix(driver-sql): order the MySQL `introspectForeignKeys` read by the key ordinal (#11379)
6+
7+
`SqlDriver.introspectForeignKeys`' MySQL arm read `information_schema.KEY_COLUMN_USAGE`
8+
with no `ORDER BY`. `ORDINAL_POSITION` is the key ordinal and was selected by neither the
9+
projection nor an order clause, so the row order of a composite foreign key's columns was
10+
whatever the query plan happened to yield.
11+
12+
That order is load-bearing. `IntrospectedForeignKey` is a flat per-column record with no
13+
ordinal field, so a composite key is expressed as **ordered sibling rows** — `(x, y)
14+
references p (a, b)` is `x -> p.a` then `y -> p.b`, and there is nothing for a consumer to
15+
recover the position from if the rows arrive permuted. The Postgres arm pins this with
16+
`ORDER BY … k.ord`; the MySQL arm was leaving it to the optimizer.
17+
18+
This is a determinism fix rather than the repair of a wrong answer, and the measurement is
19+
what distinguishes the two. On MySQL 8.0.46, a foreign key declared out of column sequence
20+
`foreign key (second_col, first_col) references ooo_parent (pa, pb)` — came back in key
21+
order through this predicate with no `ORDER BY` at all. But on the same server, in the
22+
same session, over the same view, the sibling `introspectPrimaryKeys` predicate
23+
(`CONSTRAINT_NAME = 'PRIMARY'`) returned an out-of-sequence primary key in **column**
24+
order — `carrier_code` at ordinal 2 ahead of `shipment_id` at ordinal 1. `KEY_COLUMN_USAGE`
25+
therefore does not preserve the ordinal for free on this server: which of the two orders
26+
you get is decided by the `WHERE` clause, and nothing declared that. The foreign-key
27+
predicate was on the lucky side of a choice nobody made.
28+
29+
Consumers that read composite foreign keys through `introspectSchema` — federated-object
30+
codegen, the persisted `external_catalog` (ADR-0015), and schema-drift comparison — now get
31+
the declared key order from MySQL by construction rather than by plan choice.
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#11379] `introspectForeignKeys`' MySQL arm must ORDER BY the key ordinal.
5+
*
6+
* ## Why this pin is structural, and why that is the honest shape
7+
*
8+
* The card was filed as an OBSERVATION, and it says so up front: it **did not
9+
* reproduce**. Re-measured here on live MySQL 8.0.46 before this pin was
10+
* written, with the reporter's own fixture — a key declared out of column
11+
* sequence, `foreign key (second_col, first_col) references ooo_parent
12+
* (pa, pb)`, so that "key order" and "column order" are different answers —
13+
* the arm's query WITHOUT `ORDER BY` returned:
14+
*
15+
* second_col -> pa (ORDINAL_POSITION 1)
16+
* first_col -> pb (ORDINAL_POSITION 2)
17+
*
18+
* which is key order: the correct answer, unpinned. So a behavioural pin —
19+
* "the columns come back in ordinal order" — is **vacuous** on this predicate.
20+
* It passes today, it passes with the fix, and it passes with the fix reverted.
21+
* A green that cannot go red is not evidence, so this file does not write one,
22+
* and does not dress one up as a guard.
23+
*
24+
* ## What was measured that makes the fix more than cosmetic
25+
*
26+
* On the SAME server, in the SAME session, against the SAME view, the sibling
27+
* `introspectPrimaryKeys` predicate — `CONSTRAINT_NAME = 'PRIMARY'` instead of
28+
* `REFERENCED_TABLE_NAME IS NOT NULL` — read an out-of-sequence primary key
29+
* `PRIMARY KEY (shipment_id, carrier_code)` back as:
30+
*
31+
* carrier_code (ORDINAL_POSITION 2)
32+
* shipment_id (ORDINAL_POSITION 1)
33+
*
34+
* i.e. COLUMN order, the wrong answer — reproducing #11101's measurement
35+
* exactly. `KEY_COLUMN_USAGE` therefore does NOT preserve the ordinal for free
36+
* on this server: which of the two orders comes back is decided by the WHERE
37+
* clause, and nothing declares that. The foreign-key predicate is currently on
38+
* the lucky side of a choice nobody made. That is what the `ORDER BY` removes,
39+
* and it is why "it did not reproduce" is not a reason to leave it out.
40+
*
41+
* ⛔ Deliberately NOT attempted here: proving that some plan shape on some
42+
* supported MySQL version returns the foreign-key predicate out of ordinal
43+
* order. That needs a fixture large enough to change the plan, and the card
44+
* rules it out as beyond what an observation should spend.
45+
*
46+
* ## So the pin is on the emitted SQL, and it can go red
47+
*
48+
* Removing `ORDER BY ORDINAL_POSITION` from the arm turns the first test in
49+
* this file red — verified by doing it, not by assuming it. That is the whole
50+
* claim this file makes, and it is stated no more strongly than that.
51+
*
52+
* ⚠️ It is a pin on **this method's** emitted statement, captured at the knex
53+
* seam — never a grep of the source file for the literal. `sql-driver.ts`
54+
* contains `ORDER BY ORDINAL_POSITION` three times (`introspectColumnOrder`,
55+
* this method, and `introspectPrimaryKeys`), so a file-level match would report
56+
* this arm as fixed while it was still unordered — which is exactly how a live
57+
* defect gets closed as already-absorbed.
58+
*
59+
* The second test pins the other half of the same contract, which lives in TS
60+
* rather than in SQL: the arm must EMIT the rows in the order the server
61+
* returned them. A sort, a `Map` keyed by column name, or a regrouping pass
62+
* inserted into that loop would silently undo the `ORDER BY` above, and unlike
63+
* the row order itself, that one is fully determined here and really can fail.
64+
*/
65+
66+
import { describe, it, expect, afterEach } from 'vitest';
67+
import { SqlDriver } from '../src/index.js';
68+
69+
/** One row of `KEY_COLUMN_USAGE` as the MySQL arm's projection aliases it. */
70+
interface FkRow {
71+
column_name: string;
72+
referenced_table: string;
73+
referenced_column: string;
74+
constraint_name: string;
75+
}
76+
77+
/**
78+
* A driver that DECLARES MySQL and answers from a canned result set.
79+
*
80+
* `isMysql` is derived from `config.client` and from nothing else, and the
81+
* constructor already keeps `this.config` as the DECLARED target while the knex
82+
* instance points somewhere else (#6743 — that split is the documented
83+
* behaviour of this class, not a hole this test opens). So re-declaring the
84+
* client after construction drives the REAL dispatch through the REAL getter,
85+
* while the transport stays an in-memory SQLite handle that is never asked to
86+
* execute anything. No MySQL server, so this pin runs in every CI job rather
87+
* than only in the provisioned live-matrix one.
88+
*/
89+
class MysqlFkEmissionProbe extends SqlDriver {
90+
/** Every statement the arm handed to knex, in order. */
91+
readonly emitted: { sql: string; bindings: unknown }[] = [];
92+
93+
constructor(private readonly rows: FkRow[]) {
94+
super({
95+
client: 'better-sqlite3',
96+
connection: { filename: ':memory:' },
97+
useNullAsDefault: true,
98+
});
99+
100+
(this.config as { client?: string }).client = 'mysql2';
101+
102+
const knex = this.knex as unknown as Record<string, unknown>;
103+
// knex defines `raw` as non-writable (but configurable), so a plain
104+
// assignment throws — the swap has to go through `defineProperty`.
105+
Object.defineProperty(knex, 'raw', {
106+
configurable: true,
107+
value: (sql: unknown, bindings: unknown) => {
108+
this.emitted.push({ sql: String(sql), bindings });
109+
// mysql2 hands knex back `[rows, fields]`; the arm reads `result[0]`.
110+
return [this.rows, []];
111+
},
112+
});
113+
}
114+
115+
foreignKeys(table: string) {
116+
return this.introspectForeignKeys(table);
117+
}
118+
119+
/** The one statement this method emitted. Fails loudly if it was not one. */
120+
soleStatement(): string {
121+
expect(
122+
this.emitted.length,
123+
'introspectForeignKeys emitted no statement, or more than one — the ' +
124+
'capture below would be measuring nothing. Did the dialect dispatch ' +
125+
'stop reaching the MySQL arm?',
126+
).toBe(1);
127+
return this.emitted[0]!.sql;
128+
}
129+
}
130+
131+
/**
132+
* The reporter's fixture, as rows: `(second_col, first_col)` referencing
133+
* `(pa, pb)` — a key declared out of column sequence, so key order and column
134+
* order are different answers and an accidental sort is visible.
135+
*/
136+
const OUT_OF_SEQUENCE_ROWS: FkRow[] = [
137+
{
138+
column_name: 'second_col',
139+
referenced_table: 'ooo_parent',
140+
referenced_column: 'pa',
141+
constraint_name: 'fk_ooo',
142+
},
143+
{
144+
column_name: 'first_col',
145+
referenced_table: 'ooo_parent',
146+
referenced_column: 'pb',
147+
constraint_name: 'fk_ooo',
148+
},
149+
];
150+
151+
describe('introspectForeignKeys (MySQL) orders a composite key by the ordinal (#11379)', () => {
152+
let probe: MysqlFkEmissionProbe | undefined;
153+
154+
afterEach(async () => {
155+
await (probe as unknown as { knex?: { destroy(): Promise<void> } } | undefined)?.knex?.destroy();
156+
probe = undefined;
157+
});
158+
159+
it('emits ORDER BY ORDINAL_POSITION on the KEY_COLUMN_USAGE read', async () => {
160+
probe = new MysqlFkEmissionProbe(OUT_OF_SEQUENCE_ROWS);
161+
await probe.foreignKeys('ooo_child');
162+
163+
const sql = probe.soleStatement();
164+
165+
// Control first: the captured statement really is the foreign-key read of
166+
// this method, not some other statement that happened past the seam. Without
167+
// this, the assertion below could go green on the wrong query — the
168+
// file-level-grep failure mode, one layer in.
169+
expect(sql).toMatch(/information_schema\.KEY_COLUMN_USAGE/i);
170+
expect(sql).toMatch(/REFERENCED_TABLE_NAME IS NOT NULL/i);
171+
expect(sql).not.toMatch(/CONSTRAINT_NAME\s*=\s*'PRIMARY'/i);
172+
173+
// The pin: the ordinal clause is in THIS statement, and it comes after the
174+
// predicate that identifies it, so it cannot be satisfied by a clause that
175+
// belongs to a different read.
176+
expect(sql).toMatch(/REFERENCED_TABLE_NAME IS NOT NULL[\s\S]*ORDER BY\s+ORDINAL_POSITION/i);
177+
});
178+
179+
it('emits the rows in the order the server returned them', async () => {
180+
probe = new MysqlFkEmissionProbe(OUT_OF_SEQUENCE_ROWS);
181+
const keys = await probe.foreignKeys('ooo_child');
182+
183+
// `IntrospectedForeignKey` is a flat per-column record with no ordinal
184+
// field, so ORDERED SIBLING ROWS is the only way a composite key is
185+
// expressed (#11324). Re-sorting or regrouping in the arm would undo the
186+
// `ORDER BY` above without touching the SQL.
187+
expect(keys.map((k) => `${k.columnName} -> ${k.referencedTable}.${k.referencedColumn}`)).toEqual([
188+
'second_col -> ooo_parent.pa',
189+
'first_col -> ooo_parent.pb',
190+
]);
191+
expect(keys.every((k) => k.constraintName === 'fk_ooo')).toBe(true);
192+
});
193+
});

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13883,6 +13883,33 @@ export class SqlDriver implements IDataDriver {
1388313883
});
1388413884
}
1388513885
} else if (this.isMysql) {
13886+
// `KEY_COLUMN_USAGE.ORDINAL_POSITION` IS the key ordinal, and it is
13887+
// selected by neither the projection nor an order clause — so without
13888+
// `ORDER BY` the row order of a composite key's columns is whatever the
13889+
// plan yields. The order is load-bearing for the same reason it is on
13890+
// the Postgres arm above: #11324 made a composite foreign key ORDERED
13891+
// SIBLING ROWS in this flat per-column record — `(x, y) references
13892+
// p (a, b)` is `x -> p.a` then `y -> p.b` — and `IntrospectedForeignKey`
13893+
// carries no ordinal field for a consumer to recover the position from.
13894+
//
13895+
// ⚠️ This clause is NOT a repair of a wrong answer, and the measurement
13896+
// that says so is the reason to keep it. On MySQL 8.0.46, a key declared
13897+
// out of column sequence — `foreign key (second_col, first_col)
13898+
// references ooo_parent (pa, pb)` — came back in KEY order through THIS
13899+
// predicate with no `ORDER BY` at all: the right answer, unpinned. But
13900+
// on the same server, in the same session, the sibling
13901+
// `introspectPrimaryKeys` predicate over the SAME view returned COLUMN
13902+
// order for an out-of-sequence primary key — `carrier_code` (ordinal 2)
13903+
// ahead of `shipment_id` (ordinal 1) — reproducing #11101 exactly. So
13904+
// this view does not preserve the ordinal for free on this server:
13905+
// WHICH of the two orders you get is decided by the WHERE clause, and
13906+
// nothing declares that. (Same conclusion as the primary-key arm: the
13907+
// InnoDB folklore that the view "tends to" return ordinal order does not
13908+
// hold on an out-of-sequence key.) What the clause removes is a
13909+
// dependence on a plan choice nobody chose — see the pin in
13910+
// `sql-driver-11379-introspect-fk-mysql-ordinal-order.test.ts`, which is
13911+
// deliberately a pin on the emitted SQL rather than on the row order,
13912+
// because a row-order assertion passes here with or without this line.
1388613913
const result = await this.knex.raw(
1388713914
`
1388813915
SELECT
@@ -13894,6 +13921,7 @@ export class SqlDriver implements IDataDriver {
1389413921
WHERE TABLE_SCHEMA = DATABASE()
1389513922
AND TABLE_NAME = ?
1389613923
AND REFERENCED_TABLE_NAME IS NOT NULL
13924+
ORDER BY ORDINAL_POSITION
1389713925
`,
1389813926
[tableName],
1389913927
);

packages/lint/scripts/check-doc-formula-expressions.mjs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,30 @@ export function judgeFieldRule(slot, source) {
855855
/**
856856
* The skip report, as a pure function so "it is printed" is pinnable. Returns
857857
* the empty string only when there is genuinely nothing skipped.
858+
*
859+
* ## Why the trailer says what a skip is NOT (#11673)
860+
*
861+
* Each entry's reason names the structure the PARSER could not read a layer
862+
* off. That is the only thing this gate is entitled to say: the layer a
863+
* fragment DOCUMENTS is not derivable from the tree, and a skip entry that
864+
* confidently named one would be believed — the precise failure #11407 was
865+
* built to refuse. So the list quotes its difficulty and concludes nothing.
866+
*
867+
* The cost of that silence is measured, not hypothetical. Three independent
868+
* passes over #11651 (the report, the PM triage, the dispatch) read this list
869+
* as a WORKLIST and partitioned seven skips 4 re-authorable / 3 permanent;
870+
* judging the sites first gave 1 / 6. One of the "re-authorable" four was
871+
* `layout-dsl.mdx:863`, whose predicate is byte-identical to `pages.mdx:165` —
872+
* a skip the same ruling protected BY NAME as a false red on correct docs. The
873+
* two instructions contradicted each other and the contradiction was invisible.
874+
*
875+
* The failure was a framing error, not an information deficit, and that is why
876+
* the fix is a trailer sentence rather than per-entry context: #11651's own
877+
* report QUOTED the layer comments above `:821` and `:824` (`// e.g. on a
878+
* PageComponent`, `// e.g. on a FormSection / FormField`) and filed both under
879+
* "re-authorable" anyway. Per-entry context would have reprinted what that
880+
* author already had in hand and had already published. What was missing was
881+
* the instruction not to read the list as a worklist.
858882
*/
859883
export function renderFieldRuleSkips(skips) {
860884
if (skips.length === 0) return '';
@@ -868,6 +892,16 @@ export function renderFieldRuleSkips(skips) {
868892
' printed so the skips stay visible: a gate that skips in silence is the false-green this\n' +
869893
' surface exists to prevent, one level up.',
870894
);
895+
lines.push(
896+
'\n A skip is NOT a to-do item. Every reason above answers "why could this scan not read a\n' +
897+
' layer here?" — it never answers "what layer does this fragment document?". Those are\n' +
898+
' different questions, and only the second one decides whether a site could be re-authored,\n' +
899+
' so read the layer off the DOCUMENT before re-authoring anything listed here. Triaging this\n' +
900+
' list FROM the list has already gone wrong once: three independent passes partitioned it\n' +
901+
' 4 re-authorable / 3 permanent, where judging the sites first gave 1 / 6 — and one site in\n' +
902+
' the "re-authorable" half held a predicate that is correct exactly where it is (#11651,\n' +
903+
' #11673).',
904+
);
871905
return lines.join('\n');
872906
}
873907

@@ -1629,6 +1663,35 @@ const FIELD_RULE_REPORT_SELF_TEST_CASES = [
16291663
name: 'REPORT — an empty skip list renders nothing (no phantom section on a corpus with no skips)',
16301664
holds: () => renderFieldRuleSkips([]) === '',
16311665
},
1666+
{
1667+
// The trailer is the whole of #11673's fix, and it is a string nobody else
1668+
// reads — deleting it breaks no other assertion in this file and no gate
1669+
// anywhere goes red. Pin the two load-bearing halves: that a skip is not a
1670+
// to-do item, and that its layer comes from the document.
1671+
name: 'REPORT — the trailer says a skip is NOT a to-do item and that the layer comes from the document',
1672+
holds: () => {
1673+
const out = renderFieldRuleSkips([
1674+
{ where: 'content/docs/x.mdx:12', slot: 'visibleWhen', reason: 'because reasons' },
1675+
]);
1676+
return out.includes('A skip is NOT a to-do item')
1677+
&& /read the layer off the DOCUMENT before re-authoring/.test(out);
1678+
},
1679+
},
1680+
{
1681+
// And that it never becomes a CLAIM. The trailer may describe the list's
1682+
// status; the moment it names a layer for a site it did not derive, it has
1683+
// done the one thing #11407 exists to refuse. This is the guard on the fix
1684+
// itself, not on the gate.
1685+
name: 'REPORT — no rendered skip entry names a layer the gate did not derive',
1686+
holds: () => {
1687+
const out = renderFieldRuleSkips([
1688+
{ where: 'content/docs/x.mdx:12', slot: 'visibleWhen', reason: 'because reasons' },
1689+
]);
1690+
// The reason text and the trailer may DISCUSS layers in the abstract; what
1691+
// must never appear is a verdict sentence binding this site to one.
1692+
return !/\bthis (?:site|fragment|example) (?:is|documents|describes) (?:a|an|the)\b/i.test(out);
1693+
},
1694+
},
16321695
{
16331696
name: 'REPORT — the GREEN summary path still PRINTS the skip list, not merely its count',
16341697
holds: () => {

0 commit comments

Comments
 (0)