Commit c7ad16f
fix(driver-sql): a declared index that can never be built is logged at error and reported in drift (#20519)
Fixes #20432
Clause-②: yes (widening)
Step 2 of 2 on this card, and the step that completes it. Step 1, the
lint refusal, landed as PR #20479.
## What was wrong (measured on `origin/main` 9bf5e67, SQLite)
Take an object whose only column is `status` and that declares `indexes:
[{ fields: ['statsu'], unique: true }]`:
- `initObjects` skipped the whole index and logged one **warn**:
`[sql-driver] skipping declared index on "t_probe" — column(s) not
materialized: statsu`.
- `detectManagedDrift`, which is what `os migrate plan` renders,
returned `[]`, because `expectedIndexes` drops the index from the
expected set.
- A virtual `formula` field named in an index was skipped the same way,
also at warn.
So the declared uniqueness was not enforced, and every instrument said
nothing was wrong. AGENTS.md "Degradation log levels" asks: does the
system still look normal from the outside while something it claims is
persisted has not actually landed? Yes, so the level is `error`. The
duplicate-row arms of the same loop already answered their version at
`error` through `logDurabilityFailure`.
## What changed
**`sql-driver.ts`, `syncDeclaredIndexes`.** The skip now goes through
`logDurabilityFailure`, at **error**, as triage's direction says. One
line per skipped index per sync names:
- the object and the index;
- each missing column with its reason: `not a field of the object`, `a
formula field: computed on read, never stored`, or `a declared field
whose column the table does not have`;
- whether the index is `UNIQUE`.
The line also states the consequence and the fix. A unique index gets
"The uniqueness it declares is NOT enforced: duplicate rows are
accepted…". A plain index gets "The index does not exist…".
The structured meta carries `{ tableName, index, fields, missing, unique
}`. The object's `fields` are threaded in from `syncTableIndexes` and
from the shard path; the drift-op apply paths pass none and get the bare
column names. H5: a skipped non-unique index is also at error, because
it is DDL that was supposed to run and did not. The message tells the
two apart by the `UNIQUE` word and the consequence sentence, and the
meta tells them apart by `unique`.
**`schema-drift.ts`, drift.** A new pure differ,
`diffUnbuildableIndexes`, reports the half of the declared set that
`expectedIndexes` leaves out. It shares `declaredIndexSet` with
`expectedIndexes`, so the two cannot disagree about which indexes
metadata asks for. An entry is emitted only when a missing key column
will NEVER materialize: not a declared field, or a declared virtual
`formula`. A declared, column-materializing field that the table merely
lacks is pending additive work (`add_columns`), and is not reported. The
entry:
- `kind: 'index_mismatch'` (an existing `SchemaDiffEntryKind`, so spec
is unchanged), `actual: '(absent)'`;
- `category: 'needs_confirm'` (the report-only precedent of
`manual_column_type_change`);
- `severity: 'error'` when unique, `'warning'` otherwise (the
`recreate_index` convention);
- op: the new **report-only** `DriftOp` member, `{ type:
'unbuildable_index'; table; column?; indexName; unique; missingColumns
}`.
It sits in `INDEX_DRIFT_OPS`. `applyIndexDriftOp` answers `false` for it
before any read, so apply reports it `skipped` on every dialect and it
never triggers a SQLite rebuild.
**Public surface (H3).** This is the one widening, and the seat
pre-cleared it before the build: amended claim 5877196132, `Clause-②:
yes (widening)`. The changeset is `@objectstack/driver-sql` **minor**.
No CLI source changes.
**The object form's help text (deviation, see below).** In
`packages/spec/src/data/object.form.ts`, the `indexes` → Fields help
said the skip leaves "a warning in the server log". This change made
that false, so it now says an error. The en bundle is regenerated with
`node scripts/check-i18n-bundles.mjs --write`. The zh-CN, ja-JP and
es-ES values are edited by hand, one word each. The changeset adds
`@objectstack/spec` and `@objectstack/platform-objects` as patch, and
(patch round 2) `@objectstack/lint` as patch for the corrected rule
message.
## Consumer census: every in-repo reader of `DriftOp`, `DriftOp['type']`
and `INDEX_DRIFT_OPS`
I searched with `git grep` (outside `dist/`) for `DriftOp`,
`INDEX_DRIFT_OPS`, `isIndexDriftOp`, `ColumnDriftOp`, `IndexDriftOp`,
`op.type` and `ManagedDriftEntry`. No `never`-exhaustiveness check over
`op.type` exists anywhere, and no `Record` is keyed by
`DriftOp['type']`, so the new member breaks no typecheck. Measured:
`driver-sql`, `driver-turso` and `cli` typecheck exit 0 at 7ec990a.
| Consumer | What it does with `unbuildable_index` |
|:---|:---|
| `driver-sql` `applyMigrationEntries` | `isIndexDriftOp` is true, so it
takes the index path, never the SQLite rebuild. **Pinned.** |
| `driver-sql` `applyIndexDriftOp` / `applyDriftOpInPlace` | Explicit
early `return false`, so the entry is reported `skipped`. **Pinned.** |
| `driver-sql` `applyNullSafeUniquePreflight` | Probes `create_index` /
`recreate_index` only, so this entry is untouched. |
| `driver-sql` `reconcileAndWarnDrift` (boot) | Auto-reconciles only
`safe` entries. This one is `needs_confirm`, so it is warned once per
process through the existing `[schema-drift]` line. `driftKey` includes
`indexName`. |
| CLI `schema-migrate.ts` `renderPlan` / `driftTarget` /
`groupByCategory` / `summarize` | Generic: `category`, `op.indexName`,
`op.type`, `message`. `os migrate plan` lists the entry under "Needs
confirmation" as `table [indexName] [unbuildable_index]`. **Pinned.** |
| CLI `migrate/plan.ts` | Renders through the above, and `--json` emits
the entry as-is. The exit code does not depend on drift. |
| CLI `migrate/apply.ts` | The entry counts like any `needs_confirm`
entry (non-TTY without `--yes`: "Confirmation required"), then it is
skipped, and the summary prints "Skipped N change(s)". The only
`op.type` read (line 348) is a `drop_column` / `sys_account` filter,
which this entry does not match. |
| CLI `multi-value-columns.ts` | Selects only
`manual_column_type_change`. Unaffected. |
| CLI `artifact-boot-migration.ts` (the artifact-pinned boot gate) |
Refuses only `category === 'destructive'`. This entry is handed to the
driver, reported `skipped`, and **warned** ("schema change not applied
by the driver"), and the **boot continues**. **Pinned.** |
| `driver-turso` | Extends `SqlDriver`, so the local face inherits the
fix. The remote face already refuses `detectManagedDrift` /
`applyMigrationEntries`. Suite: 76 files, 2037 passed. |
## The two folded boundaries
- **A virtual `formula` column in an index: pinned.** It is not
materialized, and it is skipped. The error names it `'doubled' (a
formula field: computed on read, never stored)`, is not marked `UNIQUE`,
and has meta `unique: false`. Drift reports it at `severity: 'warning'`.
A field-level `unique` on a formula field takes the same route
(pure-differ pin).
- **`objectExtensions` fields: not reached.** Measured at BASE with the
real `SchemaRegistry` (a throwaway probe, not committed):
- Register a base object whose index names an extension's field, plus
two extensions, one of whose indexes names the other's field.
- `getAllObjects()` returns the merged `fields` (base and both
extensions) and the merged `indexes`, and `SqlDriver.syncSchema` of that
merged object built both indexes (`uniq_h4b_account_ext_code`,
`idx_h4b_account_ext_tier`), with no skip line and empty drift.
- `ObjectQLPlugin` syncs from `registry.getAllObjects()`, so the driver
only ever sees merged objects. The lint-graph gap is step 1's boundary,
and sync does not reproduce it.
## Tests (at 7ec990a unless stated)
- New
`packages/drivers/driver-sql/src/sql-driver-20432-unbuildable-declared-index.test.ts`:
- the dialect matrix through `declareDialectCell`: misspelt UNIQUE (log
+ drift + apply-skips), formula plain index (log + drift), and a
buildable control that is enforced, with no error line;
- 5 pure-differ cases: pending column not reported, a mixed index names
only the never-materializing column, field-level unique on a formula,
the split against `expectedIndexes`, and reason kinds.
- SQLite plus live PostgreSQL 16 (local server, `Asia/Shanghai` server
zone, `TZ=America/New_York`): 15 passed, 1 skipped. **Live MySQL: NOT
MEASURED locally** (no server in this container); declared to CI's
`Temporal Conformance (live PG + MySQL)`.
- New
`packages/cli/src/utils/artifact-boot-migration.unbuildable-index.test.ts`
(integration tier, real `SqlDriver` from `dist`, real gate and
renderer): 3 passed.
- `driver-sql` full suite at 2f21af6, the head before the merges (the
merges bring nothing under `packages/drivers`): SQLite plus live
PostgreSQL gave 206 files passed / 3 skipped and 3968 tests passed / 93
skipped. SQLite only gave 198 / 11 and 3198 / 184.
- `cli` unit tier at 2062104: 233 files, 3336 passed. `driver-turso`:
76 files, 2037 passed / 18 skipped.
- `spec` `--project local`, 3 shards, at 797da30: 6070 + 5193 (+1
todo) + 5531 passed. `platform-objects` at 7ec990a: 56 files, 921
passed.
- Typecheck exit 0: `driver-sql`, `driver-turso`, `cli` (including
`check:test-typecheck`) at 7ec990a; `spec`, `platform-objects` at
797da30.
- **Cross-package reverse check.** A throwaway CLI file typed `{ type:
'unbuildable_index', … }` without `missingColumns` got `TS2322 …
Property 'missingColumns' is missing … required in type '{ type:
"unbuildable_index"; …'`, so the CLI reads the rebuilt `.d.ts`. The file
was removed and the tree is clean.
- **Ablations**, from committed state through
`scripts/ablation-replace.mjs` (anchor 1 to 0, blob changed),
trap-restored:
- A, the skip back to `logger.warn`: 4 failed (2 per cell, SQLite and
PG).
- B, the drift push deleted: 4 failed.
- C, `columnEverMaterializes` forced false: 3 failed (pure).
- Restore: `git diff HEAD` 0 bytes, blobs equal to HEAD (`1ede8d9d…`,
`50ae8546…`).
- D, a dist ablation for the CLI pin: category mutated to `destructive`,
`driver-sql` rebuilt, and `ablation-dist-preflight` confirmed the marker
in 2 built files (exit 0). CLI test 3/3 failed. After restore and
rebuild, `--absent` exit 0 with a whole-tree clean state, and CLI test
3/3 passed.
- ESLint, a proven narrowing:
- The population is from `eslint.config.mjs` (`files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']`).
- `--format json` read 4 files with 0 errors and 0 warnings.
- There is no `parserOptions.project` (type-aware linting is off), so no
untouched file's verdict can move. The repo-wide `pnpm lint` is CI's.
## Gates (measured head 7ec990a, after merging `origin/main`
9449512)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 90 commands. All 90 were run, and
each exit code was captured before any pipe.
- `--ran`: `90 derived, 90 run, 0 NOT-MEASURED, 0 UNRUN`, a derived zero
(all 90 recorded exit 0).
- Roster gates flagged for this diff's directories, all exit 0:
`check-changeset-fixed`, spec `check:meta-url-spelling`, spec
`check:spec-changes`, `check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`. Plus `check:durability-log-level`, exit 0.
- `check:driver-conformance`: 50 covered, 0 DEBT, 0 exempt before and
after. The dialect axis is unchanged (8 suites, 0 in the DIALECT
ledger). The ledger did not rise.
## Patch round 1: the g2a note's index-door sentence (deliberate
correction)
Admitted by the seat in the amended claim 5879543809. Text only: this
round changes no code, test, help text or bundle.
**The note:** `.changeset/19332-g2a-fieldgroups-indexes-form-rows.md`
(spec lane, unreleased). One sentence on line 13 carried two clauses
that are false in the same release. Nothing else in the file changes (1
line out, 1 line in).
**Old:**
> `indexes[].fields` is free text, and no authoring door judges its
names: not the schema parse, not the publish door, not `os validate`. A
name that is not a stored column makes the SQL driver skip the whole
index at sync with a warning in the server log, and the help text says
exactly that.
**New:**
> `indexes[].fields` is free text, and the schema parse, so a draft
save, does not judge its names; `os validate`, `os build`, `os lint` and
the publish door refuse a name that is not a field of the object
(`object-field-ref-unknown`, #20479, in the same release). A name that
is not a stored column, a `formula` field say, makes the SQL driver skip
the whole index at sync with an error in the server log, and the help
text says exactly that; `os migrate plan` reports the skipped index too
(#20432, in the same release).
**The doors, measured at the patch-round base 7ec990a** on the built
CLI, against a one-object fixture whose UNIQUE index names `statsu`
(bad) or `status` (clean):
- `os validate`: bad exits 1 with `object-field-ref-unknown` at
`objects[0].indexes[0].fields[0]`, clean exits 0.
- `os build`: bad 1 (same rule and path), clean 0.
- `os lint`: bad 1 (same), clean 0.
- The runtime publish door (`publishPackageDrafts`, throwaway probe on
the package's own stub engine): the bad draft is refused,
`INVALID_METADATA` with the same rule at
`objects.idx_ticket.indexes[0].fields[0]`. The clean draft is
`published`.
- The draft save staged the bad object without complaint (the schema
parse does not judge names), which is what the new sentence says.
**`Check Changeset` stays red by design on that one name.** `node
scripts/check-empty-changeset.mjs --base origin/main` exits **1** at
cef89b8. It names only
`.changeset/19332-g2a-fieldgroups-indexes-form-rows.md` ("present on the
merge base and CHANGED by this PR") and prints the DELIBERATE CORRECTION
class, whose remedy is "do NOT restore it -- say so on the PR and get it
confirmed". This section is that statement, and the at-tier review is
its written confirmation. ⛔ No `skip-changeset`.
**Gates re-run at cef89b8** (after merging `origin/main` 0bbe400
with a true merge commit):
- `dispatch-gates --commands` derived 90. All 90 were run with exit
codes captured before any pipe: 89 exited 0, and 1 exited 1
(`check-empty-changeset`, the by-design red above).
- `--ran`: 90 derived, 90 run, 0 NOT-MEASURED, 0 UNRUN.
- Exit 0 on the roster gates flagged for this diff's directories
(`check-changeset-fixed`, spec `check:meta-url-spelling`, spec
`check:spec-changes`, `check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`), and on `check:durability-log-level`,
`check-adr-0087-registration`, `check-changeset-no-major` and
`check:nul-bytes`.
## Patch round 2: the `object-field-ref-unknown` index message (text
only)
The seat answered patch round 1's open question with **A**. Amended
claim 5879937791 admits
`packages/lint/src/validate-object-field-refs.ts`, text only. No rule
logic, severity, id, code path or test assertion moved.
**The message tail** (the `INDEX_POSITION.consequence` string). The
prescription is unchanged: the hint line reads byte-identically before
and after on the fixture.
> **Old:** The SQL driver skips the WHOLE index at sync with only a
warning, and drift drops it too, so `os migrate plan` never reports it:
a `unique` index is then silently unenforced while everything looks
normal.
>
> **New:** The SQL driver skips the WHOLE index at sync. It logs the
skip at error and `os migrate plan` reports the index as unbuildable,
but the object keeps serving: a `unique` index is then unenforced until
the name is fixed.
**The docblock sentence** is put in the past tense ("the sync's skip was
a `warn` and drift dropped the index") and followed by a parenthesis. It
says that since #20432 step 2 the skip is logged at `error` through
`logDurabilityFailure` and `os migrate plan` reports the unbuildable
index, both still after the authoring doors.
**The test pin:** measured with `git grep` across `packages/**`,
`examples/**`, `content/**` and `skills/**`. Exactly one test pins this
text beyond the subject: `validate-object-field-refs.test.ts` asserted
``'`unique` index is then silently unenforced'``. Only that pinned
string changed, to ``'`unique` index is then unenforced'``. This is a
test-text change, not an assertion change: the same `toContain` on the
same finding. No other test, doc or skill quotes the message.
**Changeset:** `.changeset/20432-skipped-index-durability.md` gains
`'@objectstack/lint': patch` and one paragraph naming the corrected
message.
**Measured at d159ac9** (`origin/main` has not moved since cef89b8,
so no merge was needed):
- The rule's test file passed 60 of 60. The full `@objectstack/lint`
suite passed 115 files and 5363 tests. `pnpm --filter @objectstack/lint
typecheck` (with `check:test-typecheck`) exits 0.
- `check-changeset-fixed` exits 0, and `check-changeset-no-major` exits
0. `check-empty-changeset` exits **1**, still on exactly
`.changeset/19332-g2a-fieldgroups-indexes-form-rows.md` (the DELIBERATE
CORRECTION class, by design).
- `os validate` on the misspelt-index fixture (built CLI) exits 1, and
the clean fixture exits 0. The new tail, quoted from its output:
> …Did you mean "status"? The SQL driver skips the WHOLE index at sync.
It logs the skip at error and `os migrate plan` reports the index as
unbuildable, but the object keeps serving: a `unique` index is then
unenforced until the name is fixed.
- `dispatch-gates --commands` derived 91 (the new family is
`check:docs-transcript-drift`). All 91 were run after a full
`packages/*` and `examples/*` build: 90 exited 0 and 1 exited 1
(`check-empty-changeset`, the by-design red). `--ran`: 91 derived, 91
run, 0 NOT-MEASURED, 0 UNRUN.
- The roster gates (`check-changeset-fixed`, spec
`check:meta-url-spelling`, spec `check:spec-changes`,
`check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`) and `check:durability-log-level` all exit
0.
## Deviations
- **File surface.** This PR also edits
`packages/spec/src/data/object.form.ts` (one help-text word and its
comment) and the four `metadata-forms` translation bundles, beyond the
claim's original surface. The os-dev contract makes a published text
that this change turns false a must-fix in the same PR. The breach was
named in the first report instead of chosen silently, and **the seat
admitted it in the amended claim 5879543809** (text only, no logic).
That claim also admits the deliberate correction in patch round 1 below.
- MySQL cells: not measured locally (see Tests).
## Acceptance notes
- **`driver-memory` mirror** (measured, not touched; under its freeze).
`syncSchema` of `indexes: [{ fields: ['statsu'], unique: true }]` logs
nothing at any level (only `Created in-memory table` at info). The index
then constrains only rows that write the undeclared key `statsu`. Rows
duplicating `status` are accepted silently.
- **`driver-turso` remote transport** (`remote-transport.ts`, about line
2410) keeps its own copy of the skip line through `diagnosticSink`. It
is not in this file surface.
- **Release text.** The pending
`.changeset/19332-g2a-fieldgroups-indexes-form-rows.md` said that no
authoring door judges `indexes[].fields`, and that the skip happens
"with a warning in the server log". Both were false in the same release.
Corrected in patch round 1 below, as the seat admitted.
- **A second published text this PR makes false: fixed in patch round
2.** The `object-field-ref-unknown` message tail on an
`indexes[].fields` position, and the rule's docblock sentence, said the
skip was a warning that drift drops. The seat admitted the fix in
5879937791 (see Patch round 2).
- The CLI's `os migrate apply` skip summary reads "(destructive without
--allow-destructive, or unsupported on this dialect)". Neither reason is
this op's. The plan message states the real one. The wording is CLI
source, outside this file surface.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b2b6a06 commit c7ad16f
13 files changed
Lines changed: 733 additions & 28 deletions
File tree
- .changeset
- packages
- cli/src/utils
- drivers/driver-sql/src
- lint/src
- platform-objects/src/apps/translations
- spec/src/data
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
Lines changed: 126 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
0 commit comments