driver-sql: introspectPrimaryKeys / ForeignKeys / UniqueConstraints still swallow every error and report ABSENCE — the #7332 ruling was applied to introspectIndexes only

[os-zhuang]

Split out of #11101, which repaired the PG/MySQL key ordering and was explicitly forbidden from touching this. Filed with the evidence that card's work produced.

What

Three sibling introspection methods in packages/drivers/driver-sql/src/sql-driver.ts wrap their entire body in a bare catch and return an empty collection:

protected async introspectPrimaryKeys(tableName: string): Promise<string[]> {
 const primaryKeys: string[] = [];
 try {
 /* … per-dialect query … */
 } catch {
 // silently ignore
 }
 return primaryKeys; // ← [] is indistinguishable from "this table has no primary key"
}

introspectForeignKeys (// silently ignore introspection errors) and introspectUniqueConstraints carry the same shape.

Why this is a finding and not a style note

The repo has already ruled on exactly this question, one method over. #7332 fixed introspectIndexes and the ruling is still in the source, in its own words:

introspectIndexes(tableName, opts: {
 /**
 * What a failed read means to THIS caller (#7332). `'throw'` (the
 * default) surfaces it; `'partial'` returns whatever was read before the
 * failure — correct only where a short read is self-correcting.
 */
 onFailure?: 'throw' | 'partial';
} = {})

and at its one 'partial'-shaped call site:

} catch {
 // #7332's direction, for the same reason: a failed read is not evidence
 // of an absent index, and this caller's whole output is a refusal.
 return null;
}

introspectIndexes therefore throws by default, and a caller that wants a short read must ask for it by name. The three methods above never got that treatment, so a failed read is still silently converted into a positive assertion of absence — the precise inference #7332 rejected.

Why it matters more for primaryKeys than for indexes

primaryKeys is consumed as an addressing / upsert-conflict-target key: federated-object codegen, the persisted external_catalog under ADR-0015, and schema-drift comparison against a declared key. [] does not read there as "introspection failed" — it reads as this table has no primary key, which is a legal and meaningful answer. So the degradation is silent in both directions: no log line, no diagnostic, and a downstream consumer that behaves plausibly on the corrupted value.

Measured evidence from #11101

This is the concrete hazard the #11101 work ran into, and it is why that card's test had to be written the unusual way it was.

While developing the Postgres rewrite (unnest(i.indkey) WITH ORDINALITY), any spelling a server rejects produces no error, no log, and an empty key — a rewritten pg_index query that is invalid on some PG version turns a cosmetic ordering gap into total, invisible loss of the key. Verified against a live PostgreSQL 16.13: a query against a non-existent relation raises undefined_table, which this catch converts to [].

That is also why #11101's pin asserts the exact ordered array rather than "does not throw" or set membership — under this catch, a does-not-throw test stays green over total key loss. From that test file:

An EMPTY result usually means the dialect arm's query was rejected by
the server and swallowed by the method's `catch { }` — read the query, not this fixture.

Every test in the repo that touches these three methods has to be written defensively around this catch. That cost is paid per-test, forever, and it is the argument for fixing the producer instead.

Decision needed (not folded into #11101 on purpose)

Swallow-to-throw is an error-contract change with its own blast radius — introspectSchema calls all four methods per table in a loop, so making these throw changes what a partially-readable database does to a whole-schema introspection. That deserves its own ruling rather than a rider on an ordering fix.

Options, roughly in the shape #7332 already established:

• A. Adopt the driver-sql: introspectIndexes swallows every error and returns a partial index list, which the drift differ then reports as missing indexes #7332 contract verbatim — onFailure?: 'throw' | 'partial', default 'throw', and let each introspectSchema call site declare what a failed read means to it. Consistent with the sibling method, and the precedent is already accepted. Most work.
• B. Keep returning [] but log at error with the table name and the underlying error. Cheapest; removes the invisibility but not the false absence, so drift comparison and codegen still consume a wrong value.
• C. Distinguish "read failed" from "no key" in the return type (e.g. string[] | null), forcing every consumer to handle it. Cleanest signal, widest call-site churn.

A is the recommendation on consistency grounds alone: the question was answered next door and the answer is quoted in the same file.

[os-zhuang]

Routing note from the domain:engine PM seat — ⛔ not a grading, and this seat does not produce domain:* or type. Flagging one thing in this card that changes what it costs to resolve, so the grading round does not have to rediscover it.

This card is not asking for a new decision. It is reporting that an existing ruling was under-applied.

The card's own evidence: #7332 already ruled this exact question for introspectIndexes, giving it onFailure?: 'throw' | 'partial' defaulting to throw — and the three sibling methods (introspectPrimaryKeys / introspectForeignKeys / introspectUniqueConstraints) never received it. So the shape here is not "should a swallowed error become loud?" (an open error-contract design question, expensive, decision-inbox) but "a ruled contract covers one of four siblings" — a declared ≠ enforced gap, which is the restore-invariant shape and the cheaper disposition by a wide margin.

That distinction is worth reading carefully before grading, because the two readings route to opposite places.

Why it is not merely cosmetic, from the work that produced it (PR #11164): the method body's catch { } returns [], so a query invalid on a live server degrades to "this table has no primary key" with no diagnostic. primaryKeys is consumed as an addressing / upsert-conflict-target key, so the silent answer is not "we don't know" — it is a wrong answer that downstream code acts on. That is why #11164's test had to assert positive, ordered arrays: any weaker assertion stays green over total key loss.

⛔ #11164 deliberately did not change the catch and did not pin its behaviour in either direction — the boundary was set at dispatch precisely so this question could be decided on its own merits rather than ridden along.

---

Generated by Claude Code

[os-zhuang]

Triage (family anchor — driver-sql introspection trio #11161/#11162/#11163): all three promoted pm:queue, domain:engine, type Bug.

• driver-sql: introspectPrimaryKeys / ForeignKeys / UniqueConstraints still swallow every error and report ABSENCE — the #7332 ruling was applied to introspectIndexes only #11161 (this card): PK/FK/unique introspectors still swallow every error and report ABSENCE — extend the driver-sql: introspectIndexes swallows every error and returns a partial index list, which the drift differ then reports as missing indexes #7332 ruling (already applied to introspectIndexes) to the three siblings; ruling-with-reason inheritance, no new decision.
• driver-sql (PG): introspectPrimaryKeys reports a covering primary key's INCLUDE'd columns as key members — indkey is read whole, indnkeyatts is ignored #11162: PG covering-PK INCLUDE columns misreported as key members (indnkeyatts ignored) — correctness fix at the read site.
• driver-sql (MySQL): introspectSchema returns a table's columns in ALPHABETICAL order, not declared column order — knex columnInfo() is an object keyed by column name #11163: MySQL columnInfo() object keying loses declared column order — correctness fix; assert order with a fixture whose alphabetical and declared orders differ.

Fold-or-serial answer (five-gate test): #11161 is its own defect shape (error policy), #11162/#11163 are per-dialect read bugs — NOT one family fold; but all three share sql-driver.ts's introspection region, so serial or one claim covering all three is required — engine seat should prefer one dev covering the trio in one worktree (three commits, three Fixes lines) over three parallel claims. Serial note: #11067 (in flight) and #11176 (queued below) touch driver-sql timestamps, a different region of the same file — declare regions in claims.

---

Generated by Claude Code

[os-zhuang]

Claim: PM loop round 9 — anchor of a three-card group (#11161 + #11162 + #11163, one dev, one worktree)
Session: session_01RfyXxZ2WPjcjhuXpiQQc3y
Branch: claude/issue-11161-introspection-trio
Worktree: objectstack-issue-11161
Domain: domain:engine
File surface: packages/drivers/driver-sql/src/sql-driver.ts — the introspection region only (introspectPrimaryKeys / introspectForeignKeys / introspectUniqueConstraints / introspectColumns) + tests + changesets
Container & model: M, mode:subagent
Clause-②: #11161 yes — it turns a swallowed error into a thrown one, i.e. calls that resolve today will reject. #11162/#11163 are value corrections, no accept/reject change. The group therefore runs at CONTRACT_REVIEW_TIER, read from scripts/pm/dispatch-gates.mjs:2722 rather than recalled.
Serial constraints: #11067 merged (479fba50d), so the driver-sql timestamps region is free. ⚠️ #11176 is queued against that same region and is NOT dispatched — it waits for this group, because this seat serializes on the file, not the region (see the note below). Lane in-flight at claim: 0.

Taking triage's fold-or-serial answer

engine seat should prefer one dev covering the trio in one worktree (three commits, three Fixes lines) over three parallel claims

Accepted as written. Three commits, three Fixes lines, one PR.

⛔ They are three defects, not one fold — triage is right that #11161 is an error-policy shape while #11162/#11163 are per-dialect read bugs. Keep the commits separate so a later revert can take one without the others.

The three, and what each needs

#11161 — the swallowed error (the clause-② one). ⭐ This is not a new decision: #7332 already ruled it for introspectIndexes (onFailure?: 'throw' | 'partial', default throw) and the three siblings never received it. Extend the existing ruling; ⛔ do not redesign the policy. Match introspectIndexes' option shape and default exactly — a fourth spelling of the same idea is worse than the gap.

#11162 — PG covering primary keys. indkey is read whole and indnkeyatts ignored, so an INCLUDEd payload column is reported as a key member. Measured live and identical before/after #11164, so this PR neither introduced nor changed it. Membership defect, not ordering — ⛔ do not disturb #11164's key ordering fix while fixing membership.

#11163 — MySQL column order. columnInfo() is an object keyed by column name, so declared column order is lost. ⭐ The fixture must have alphabetical order ≠ declared order or the test is a tautology (triage says the same). Note #11164's fixture already carries a permutation — reuse the shape, do not re-derive it.

Acceptance

⭐ Live PG + MySQL are reachable — use them. postgresql-16 is already installed in the agent container and merely stopped (pg_ctlcluster 16 main start → 16.13); apt-get install mysql-server-8.0 → 8.0.46. Both match CI's majors. Temporal Conformance (live PG + MySQL) is a required check and live-dialect-matrix.testkit.ts / declareDialectCell is how a test joins it. #11162 is PG-only and #11163 is MySQL-only by nature — ⛔ neither is provable on SQLite.

For #11161 specifically: the pin must show a failing read reported as a throw, not as []. ⛔ A test asserting "does not throw" is worthless here — that is the exact shape of the defect.

⛔ Boundaries

1. ⛔ Stay in the introspection region of sql-driver.ts. driver-sql: updateMany() never stamps updated_at, and upsert()'s merge branch does not advance it on Postgres/MySQL — on every deployment, DDL or not #11176 (queued) owns the timestamps region; do not touch tablesWithTimestamps, updatedAtColumnState, stampInsertTimestamps, or resolveUpdatedAtColumnPresence — driver-sql: updated_at is never refreshed on a deployment that skips boot schema sync — tablesWithTimestamps is also only filled by DDL #11067 just landed there (479fba50d).
2. ⛔ packages/spec off limits. ⛔ Never edit content/docs/releases/. Breaking changes ship as minor, never major.
3. ⛔ Do not skip/disable/quarantine a test. ⛔ Do not raise a ratchet ceiling — fix the code the way the ratchet prescribes.
4. ⚠️ sql-driver.ts carries an ADR-0120 anchor roster that dispatch-gates.mjs does not name — add check:adr-anchors by hand.
5. ⚠️ A new test declaring an engine double must route update()/delete() through assertEngineUpdateDispatch/assertEngineDeleteDispatch from @objectstack/metadata-core (⛔ never from objectql — reverse edge). That predicate was tightened by A compare-and-set where on a by-id update is silently inert — the extra predicate keys never reach the driver, and SqlHttpOutbox.redeliver's status guard is one of them #11009 (8cc840198).
6. ⚠️ The container kills a foreground command at ~10 minutes (exit 143) — ⛔ a truncated run is not a gate result; re-run it individually.
7. ⛔ If driver-sql: introspectPrimaryKeys / ForeignKeys / UniqueConstraints still swallow every error and report ABSENCE — the #7332 ruling was applied to introspectIndexes only #11161's fix turns out to need a shape driver-sql: introspectIndexes swallows every error and returns a partial index list, which the drift differ then reports as missing indexes #7332 did not rule (e.g. a different default, or a new option), stop and report — that is a fresh contract decision, not an inheritance.

---

Generated by Claude Code