fix(metadata-protocol): the dialect arm of classifyIndexFailure walks cause to the conflict arm's depth (#6848)

.changeset/index-failure-dialect-cause-walk.md

@@ -0,0 +1,46 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): the dialect arm of `classifyIndexFailure` walks `cause` to the same depth the conflict arm does (#6848)
+
+`classifyIndexFailure` had two arms reading two different wrap-depths. #6699
+moved the first arm onto `@objectstack/types`' `isUniqueViolationError`, which
+follows `error.cause` four levels down because pool and query-builder layers
+re-throw with the original attached. The second — the dialect arm — kept reading
+`err.message` and stopping there.
+
+So a dialect refusal arriving behind a wrapper (outer prose `Write failed` or
+`pool query failed`, the actual `near "WHERE": syntax error` one step down
+`cause`) was graded `failed` instead of `unsupported`. The private
+`indexFailureText` helper now collects the message channel of the thrown value
+**and** of each `cause` below it, bounded at the same `MAX_CAUSE_DEPTH` of 4 the
+predicate uses and counted the same way (the thrown value is depth 0). The
+dialect vocabulary itself is unchanged — only the text fed to it.
+
+**Why the verdict matters beyond wording.** The two consumers dispose of
+`unsupported` and `failed` differently. `view-definition-active-index.ts` treats
+them the same (keep the previous index, report at `error`; only the wording
+differs). But `ensureOverlayStateIndex` builds the composite **fallback lookup
+index** on the `unsupported` branch and on no other — offered precisely because
+a dialect that cannot take the partial form should still get the lookup. Under
+a `failed` verdict that branch never ran, so `fallback` came back
+`not-attempted` rather than `ensured` / `refused` and the degradation target was
+silently never attempted.
+
+**Dormant, not a live regression.** No driver shipped today produces the wrapped
+shape — each hands knex's error back with the dialect text on the outer message,
+which is why every existing case matched on the first read. This closes an
+asymmetry before a wrapping raw-SQL driver can land on it; it is also not a
+regression from #6699, which only made the contrast visible by deepening the
+first arm.
+
+Two details worth knowing if you touch this: the collected levels are joined
+with a **newline**, never a space, because two of the dialect alternatives are
+multi-word (`where clause`, `near "where"`) and a space would let a phrase be
+synthesised across a wrapper boundary that no single driver wrote. And a looping
+`cause` chain is **bounded rather than detected** — no visited set — which is
+exactly what the predicate this mirrors does.
+
+Arm order is unchanged and still load-bearing: a conflict reported anywhere in
+the chain still beats a dialect refusal in the outer prose.

packages/metadata-protocol/src/migrations/overlay-index.test.ts

@@ -302,6 +302,44 @@ describe('sys_metadata overlay uniqueness (#6418)', () => {
  expect(logger.info).not.toHaveBeenCalled();
  });
 
+ /**
+ * #6848 — the same MariaDB refusal, behind a pooled wrapper.
+ *
+ * This is the consequence the classifier's wrap-depth actually decides, and
+ * the reason the dialect arm was given the `cause` walk rather than a doc
+ * comment. The fallback lookup index is built on the `unsupported` branch
+ * and on no other, so while the second arm stopped at the outer message this
+ * case graded `failed` and the degradation target was never attempted — the
+ * dialect's answer was present, one step down, and unread.
+ */
+ it('a POOLED wrapper around the dialect refusal still reaches the fallback index (#6848)', async () => {
+ const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn() };
+ const pooled: IndexExec = async (sql: string) => {
+ if (/where/i.test(sql)) {
+ throw Object.assign(new Error('Write failed'), {
+ cause: new Error(
+ "You have an error in your SQL syntax; check the manual … near 'WHERE state = 'active''",
+ ),
+ });
+ }
+ return db.exec(sql);
+ };
+
+ const result = await ensureOverlayStateIndex(pooled, 'active', logger);
+
+ expect(result.status).toBe('unsupported');
+ // The whole point: `not-attempted` here would mean the dialect that
+ // cannot take the partial form silently got no lookup index either.
+ expect(result.fallback).toBe('ensured');
+ // `detail` stays the operator-facing OUTER prose, unchanged.
+ expect(result.detail).toBe('Write failed');
+ // Nothing was downgraded — the declared UNIQUE index is byte-for-byte there.
+ expect(indexDdl(OVERLAY_INDEX_NAMES.active)).toEqual(DECLARED_ACTIVE_INDEX_DDL);
+ expect(insert('w1', 'view', 'lead.w', 'org1', 'pkg1', 'active').ok).toBe(true);
+ expect(insert('w2', 'view', 'lead.w', 'org1', 'pkg1', 'active').ok).toBe(false);
+ expect(String(logger.error.mock.calls[0]![0])).toContain('NOT enforced as specified on this dialect');
+ });
+
  /**
  * MySQL proper, where the old code was safe only by ACCIDENT: it has
  * neither `DROP INDEX IF EXISTS` nor `CREATE INDEX IF NOT EXISTS`, so every

packages/metadata-protocol/src/migrations/partial-index-probe.test.ts

@@ -248,6 +248,128 @@ describe('probe-first partial index replacement (#6418)', () => {
  expect(indexDdl(PROBE)).toBeUndefined();
  });
 
+ /* ────────────────────────────────────────────────────────────────────── *
+ * #6848 — the DIALECT arm walks `cause` to the same depth as the first
+ * ────────────────────────────────────────────────────────────────────── */
+
+ /** A neutral wrapper: matches NEITHER vocabulary, so it can only carry. */
+ const wrap = (cause: unknown): Error => Object.assign(new Error('pool query failed'), { cause });
+
+ /** `leaf` behind `depth` neutral wrappers (`depth: 0` is the leaf itself). */
+ const nest = (depth: number, leaf: unknown): unknown =>
+ depth === 0 ? leaf : wrap(nest(depth - 1, leaf));
+
+ it('grades a WRAPPED dialect refusal `unsupported`, not `failed` (#6848)', () => {
+ // The shape a pooled or query-builder layer produces: useless outer
+ // prose, the dialect's actual answer one step down `cause`. Before this
+ // the second arm stopped at the outer message and returned `failed` —
+ // which costs `overlay-index` its fallback lookup index, because that
+ // branch is reached on `unsupported` and nowhere else.
+ const wrapped = Object.assign(new Error('Write failed'), {
+ cause: new Error('near "WHERE": syntax error'),
+ });
+ expect(classifyIndexFailure(wrapped)).toBe('unsupported');
+ // The control: the outer prose alone is still, correctly, `failed`.
+ expect(classifyIndexFailure('Write failed')).toBe('failed');
+
+ // …and the same for the functional-key-parts refusal, one layer deeper
+ // and on a plain object rather than an Error.
+ expect(classifyIndexFailure({ message: 'Write failed', cause: { cause: { message: 'Functional index on a column is not supported' } } })).toBe(
+ 'unsupported',
+ );
+ });
+
+ it('keeps the data verdict ahead of the dialect verdict ACROSS the chain (#6848)', () => {
+ // The inversion risk of widening the second arm: both arms now walk, so
+ // the ordering has to hold at every depth, not just at the top. Outer
+ // prose is a dialect refusal; the real condition is a conflict reported
+ // on `code` two levels down. `conflict` must still win.
+ const misleading = Object.assign(new Error('near "WHERE": syntax error'), {
+ cause: wrap(Object.assign(new Error('insert failed'), { code: '23505' })),
+ });
+ expect(classifyIndexFailure(misleading)).toBe('conflict');
+ });
+
+ it('reads the dialect arm to exactly the depth the conflict arm reads (#6848)', () => {
+ // The card's whole point, pinned as a PARITY rather than as a number:
+ // whatever depth `isUniqueViolationError` reaches, this module's dialect
+ // arm reaches the same one. Expressed this way the assertion survives a
+ // deliberate change to the shared bound and still goes red the moment
+ // the two arms drift apart again.
+ const DEPTHS = [0, 1, 2, 3, 4, 5, 6];
+ const dialectReach = DEPTHS.map(
+ (d) => classifyIndexFailure(nest(d, new Error('near "WHERE": syntax error'))) === 'unsupported',
+ );
+ const conflictReach = DEPTHS.map(
+ (d) =>
+ classifyIndexFailure(nest(d, Object.assign(new Error('insert failed'), { code: '23505' }))) ===
+ 'conflict',
+ );
+
+ expect(dialectReach).toEqual(conflictReach);
+ // …and the shared profile is the predicate's `MAX_CAUSE_DEPTH` of 4
+ // counted from the thrown value, so the equality above cannot pass by
+ // both arms reaching nothing (or everything).
+ expect(dialectReach).toEqual([true, true, true, true, true, false, false]);
+ });
+
+ it('joins the chain with a newline, so no phrase is synthesised across a wrapper (#6848)', () => {
+ // Two of the dialect alternatives are multi-word. Neither message below
+ // is a refusal on its own, and joining them with a SPACE would forge
+ // `where clause` out of text no layer ever wrote.
+ const spliced = Object.assign(new Error('rebuild attempt landed where'), {
+ cause: new Error('clause parsing completed'),
+ });
+ expect(classifyIndexFailure('rebuild attempt landed where')).toBe('failed');
+ expect(classifyIndexFailure('clause parsing completed')).toBe('failed');
+ expect(classifyIndexFailure(spliced)).toBe('failed');
+ });
+
+ it('terminates on a `cause` chain that loops, exactly as the predicate does (#6848)', () => {
+ // `isUniqueViolationError` bounds rather than detects cycles — it keeps
+ // no visited set — so this walk must not either, and the bound has to be
+ // what stops both. A self-referential cause must return a verdict rather
+ // than exhaust the stack.
+ const loop = new Error('disk I/O error') as Error & { cause?: unknown };
+ loop.cause = loop;
+ expect(classifyIndexFailure(loop)).toBe('failed');
+
+ // …and a two-node cycle whose refusal is only on the INNER node is still
+ // found, because the bound is reached after the answer, not before it.
+ const outer = new Error('Write failed') as Error & { cause?: unknown };
+ const inner = new Error('near "WHERE": syntax error') as Error & { cause?: unknown };
+ outer.cause = inner;
+ inner.cause = outer;
+ expect(classifyIndexFailure(outer)).toBe('unsupported');
+ });
+
+ it('the probe classifies a wrapped refusal, and leaves `detail` the OUTER prose (#6848)', async () => {
+ // End-to-end through `probeThenReplaceIndex`: the verdict widens, the
+ // operator-facing text does not. `detail` stays the driver's own outer
+ // message, which is the contract `probeThenReplaceIndex` already had.
+ const wrapping: IndexExec = async (sql: string) => {
+ if (sql.startsWith('CREATE')) {
+ throw Object.assign(new Error('Write failed'), {
+ cause: new Error('near "WHERE": syntax error'),
+ });
+ }
+ return db.exec(sql);
+ };
+
+ const outcome = await probeThenReplaceIndex(wrapping, {
+ indexName: REAL,
+ probeIndexName: PROBE,
+ buildSql,
+ });
+
+ expect(outcome.status).toBe('unsupported');
+ expect(outcome.failedAt).toBe('probe');
+ expect(outcome.detail).toBe('Write failed');
+ // The probe is what failed, so the previous index is untouched.
+ expect(indexDdl(REAL)).toEqual(EXISTING_DDL);
+ expect(indexDdl(PROBE)).toBeUndefined();
+ });
+
  it('logProblem prefers error(), falls back to warn(), and tolerates neither', () => {
  const full = { warn: vi.fn(), error: vi.fn() };
  logProblem(full, 'msg', 'detail');

packages/metadata-protocol/src/migrations/partial-index-probe.ts

@@ -87,21 +87,70 @@ export type PartialIndexStatus =
  /** Anything else, best-effort. Previous index kept. */
  | 'failed';
 
+/**
+ * How far to follow an `error.cause` chain — deliberately the same bound as
+ * `isUniqueViolationError`'s `MAX_CAUSE_DEPTH` in `@objectstack/types` (#6848).
+ *
+ * Counted the same way too: the thrown value itself is depth 0, so this admits
+ * the outer error plus four wrapper levels below it. The two arms of {@link
+ * classifyIndexFailure} reading the SAME depth is the whole point — see that
+ * function's "Why both arms walk" note.
+ *
+ * It is also the only cycle guard, again matching the predicate: a `cause`
+ * chain that loops back on itself is bounded rather than detected, because a
+ * bound terminates a cycle just as well as a visited-set does and the predicate
+ * this mirrors has no visited-set to mirror.
+ */
+const MAX_CAUSE_DEPTH = 4;
+
+/**
+ * Every message channel on one thrown value and its `cause` chain, in order.
+ *
+ * Deliberately a local walk. `@objectstack/types` owns the *conflict* question
+ * and exports a predicate for it, but it exposes no reusable message-collecting
+ * helper — its own chain walkers (`matchesUniqueViolation`,
+ * `findUniqueViolationColumn`) are private and each answers its own question
+ * rather than handing back text. Hoisting a shared collector there would widen
+ * that package's contract for a single consumer, so this stays here and stays
+ * pinned to the bound above.
+ */
+function collectIndexFailureText(error: unknown, depth: number, into: string[]): void {
+ if (error === null || error === undefined || depth > MAX_CAUSE_DEPTH) return;
+ if (typeof error === 'string') {
+ into.push(error);
+ return;
+ }
+ if (typeof error !== 'object') {
+ into.push(String(error));
+ return;
+ }
+ const err = error as { message?: unknown; cause?: unknown };
+ if (typeof err.message === 'string') into.push(err.message);
+ collectIndexFailureText(err.cause, depth + 1, into);
+}
+
 /**
  * The text the DIALECT arm judges, from a thrown value of any shape.
  *
- * `message` first, because that is the channel a driver writes its refusal on
- * and the only one this arm has ever read; `String()` only as the last resort
- * — which is what a bare string resolves to unchanged, so a caller holding
- * nothing but prose is judged exactly as before.
+ * `message` first, because that is the channel a driver writes its refusal on;
+ * then the same channel one step at a time down `cause`, because pool and
+ * query-builder layers re-throw with the original attached and the refusal is
+ * then the ONLY copy of the dialect's answer (#6848). `String()` only as the
+ * last resort — which is what a bare string resolves to unchanged, so a caller
+ * holding nothing but prose is judged exactly as before.
+ *
+ * ⚠️ The levels are joined with a NEWLINE, never a space. Two of the dialect
+ * vocabulary's alternatives are multi-word (`where clause`, `near "where"`), so
+ * a space would let a phrase be synthesised across a wrapper boundary that no
+ * single driver ever wrote — an outer message ending in `where` above a cause
+ * beginning with `clause` would read as a dialect refusal. A newline cannot
+ * match the literal space in those alternatives, so each level is still judged
+ * on text some layer actually emitted.
  */
 function indexFailureText(error: unknown): string {
- if (typeof error === 'string') return error;
- if (typeof error === 'object' && error !== null) {
- const { message } = error as { message?: unknown };
- if (typeof message === 'string') return message;
- }
- return String(error);
+ const texts: string[] = [];
+ collectIndexFailureText(error, 0, texts);
+ return texts.length > 0 ? texts.join('\n') : String(error);
 }
 
 /**
@@ -138,10 +187,34 @@ function indexFailureText(error: unknown): string {
  * The predicate answers the FIRST arm only. It has no opinion about dialect
  * support, so the second arm stays this module's own — and stays second.
  *
+ * ## Why both arms walk `cause` (#6848)
+ *
+ * They read the same depth because they are asked the same way. #6699 gave the
+ * first arm the shared predicate's four-level `cause` walk and left the second
+ * on the outer message alone; the two then disagreed about how deeply a driver
+ * is allowed to wrap. A dialect refusal arriving behind a pooled wrapper —
+ * outer prose `Write failed`, the real `near "WHERE": syntax error` one step
+ * down — was graded `failed` rather than `unsupported`.
+ *
+ * That gap is **not** a wording difference, which is why it was worth closing
+ * rather than documenting. `view-definition-active-index.ts` disposes of the
+ * two verdicts identically (keep the previous index, report at `error`), but
+ * `overlay-index.ts` builds the composite **fallback lookup index** on
+ * `unsupported` and only there — offered precisely because a dialect that
+ * cannot take the partial form should still get the lookup. Under a `failed`
+ * verdict that branch never runs and the fallback is reported `not-attempted`
+ * instead of `ensured` / `refused`, so the wrap depth silently decides whether
+ * the degradation target is built at all.
+ *
+ * No driver shipped today produces that shape — every one hands knex's error
+ * back with the dialect text on the outer message, which is why every case here
+ * matched on the first read. This closes a dormant asymmetry, not a live defect.
+ *
  * ⚠️ Pass the **error**, not `err.message`. A string still works (the predicate
  * reads it on the message channel, and so does {@link indexFailureText}), but a
  * caller that unwraps first throws away the `code` / `errno` / `cause` channels
- * that are the whole reason this reads the object.
+ * that are the whole reason this reads the object — and, since #6848, the
+ * dialect answer too when a wrapper holds the useless half.
  */
 export function classifyIndexFailure(error: unknown): PartialIndexStatus {
  if (isUniqueViolationError(error)) {