Skip to content

Commit c4111c6

Browse files
fix(cli,driver-turso): os migrate plan on a Turso remote database lists the changes it cannot apply with the transport's limit, not --allow-destructive (#24014)
Fixes #23925 Clause-②: yes (widening) ## What changes On a Turso REMOTE primary (`libsql://...`), neither `os migrate plan` nor `os migrate apply` (its listing before the prompt, and both of its held-back lines) offers `--allow-destructive` for a drift entry that database cannot apply. The driver that reported each entry declares what its apply door cannot perform, and both commands read that declaration. Neither branches on the dialect, the transport name, the URL or the driver's class. - **The declaration (driver-turso).** `REMOTE_DRIFT_OP_ANSWERS` in `turso-driver.ts` answers every drift op the shared differ emits with `applied`, `needs-table-rebuild` or `report-only`. Its `satisfies Record(DriftOp type, answer)` clause fails the package's typecheck when an op is added to `DriftOp` and left unclassified, the `REMOTE_FACE_ANSWERS` pattern. The REMOTE `applyMigrationEntries` dispatches by it: only `applied` rows are sent, and every other entry is skipped before any DDL and logged with its reason. `TursoDriver.driftApplyLimit(opType)` answers from the same rows: the limit sentence for a `needs-table-rebuild` op, `null` for every other op, and `null` for every op on the local and embedded-replica faces. The commands and the apply door therefore read one object and cannot disagree about an entry. - **The seam (cli, `schema-migrate.ts`).** - `SchemaStack.driftApplyLimit(entry)` asks the driver that reported the entry. That is the `driftOwners` map `applyMigrationEntries` already routes by, read through the optional `SqlDriverLike.driftApplyLimit`. A driver that declares nothing has no limit. - `collectDriftApplyLimits(stack, drift)` gathers the entries that have a limit. - `describeWithApplyLimits(limits)` is the one spelling of the line printed under such an entry: its finding, then `"os migrate apply" cannot apply it on this database: LIMIT.`, in place of the `--allow-destructive` sentence. Both commands read it. - **`os migrate plan`.** An entry with a limit is still listed and counted (it is still drift), under that line. The footer offers `--allow-destructive` only for the destructive entries the database can apply, drops the apply line when it can apply nothing listed, and says how many listed changes remain drift. - **`os migrate apply`: its listing and its two held-back lines.** - The listing before the prompt passes `renderPlan` the same `describe`. - The count before the prompt, `N destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).`, now counts only the destructive entries a re-run could apply. The rest get their own line: `N destructive change(s) remain drift that a re-run with --allow-destructive cannot apply on this database either (each says why above).` - The line after the run splits the held-back entries by the same limits map: `Skipped N destructive change(s) (re-run with --allow-destructive to include them).` for those a re-run could apply, and `Skipped N destructive change(s) that a re-run could not apply on this database either: they remain drift (each says why above).` for the rest. - Nothing else changes: not `notApplied`, not PARTIAL, not the exit code, and not the `--json` document. - **The remedy sentence (driver-sql).** `tighten_not_null`, `narrow_varchar` and `drop_column` messages end with an `os migrate apply --allow-destructive` sentence. That sentence is now spelled once (`DESTRUCTIVE_COLUMN_REMEDY`), with the same bytes. `driftFindingWithoutRemedy(entry)` takes exactly that tail off, and only the limited entries use it. Every other reader keeps the message as written. - **Public surface: the widening the declaration line names.** `TursoDriver.driftApplyLimit` (`@objectstack/driver-turso`) and `driftFindingWithoutRemedy` (the `@objectstack/driver-sql` entry) are new and additive, so those two packages go `minor`. `@objectstack/cli` stays `patch`. No `packages/spec` contract and no accept set moves. - **The CLI docs (`content/docs/deployment/cli.mdx`).** One caveat under the migrate category table: > On a Turso remote primary (`libsql://`), a tightened or relaxed `NOT NULL` and a dropped DEFAULT stay drift, because that transport runs no SQLite table rebuild. `os migrate plan` and `os migrate apply` list each such entry with `"os migrate apply" cannot apply it on this database: it needs the SQLite table rebuild, which the Turso REMOTE transport does not run.`, offer `--allow-destructive` only for the destructive changes that database can apply, and count the rest as drift a re-run cannot apply. - **No declaration, no change.** When no entry has a limit, both commands take the old code path: the same `renderPlan(drift)` call, the same footer string and the same held-back lines. That is every database but a Turso REMOTE one. ### Each drift op, and what the REMOTE face does with it | op | category | emitted by the SQLite differ | REMOTE apply door | |:--|:--|:--|:--| | `drop_column` | destructive | yes | applied | | `drop_index` | destructive | yes | applied | | `create_index` | safe | yes | applied | | `recreate_index` | needs_confirm, or destructive when unique | yes | applied | | `replace_unique_index` | safe | yes | applied | | `relax_not_null` | needs_confirm | yes | skipped: needs the table rebuild | | `tighten_not_null` | destructive | yes | skipped: needs the table rebuild | | `drop_column_default` | safe | yes | skipped: needs the table rebuild | | `widen_varchar` | safe | no (PostgreSQL / MySQL only) | skipped: needs the table rebuild | | `narrow_varchar` | destructive | no (PostgreSQL / MySQL only) | skipped: needs the table rebuild | | `manual_column_type_change` | needs_confirm | no (PostgreSQL / MySQL only) | report only, as on every face | | `manual_widen_varchar_to_text` | needs_confirm | no (PostgreSQL / MySQL only) | report only, as on every face | | `unbuildable_index` | needs_confirm | yes | report only, as on every face | A report-only op answers `null` from `driftApplyLimit`. No face applies it, so its skip is not a limit of this transport, and its finding already names its remedy. The declaration is therefore a per-op answer rather than a set of appliable ops: a plain set would print this transport's rebuild reason against an `unbuildable_index`. ### Who reads the entry's `message`, and why each one stays truthful The message keeps its bytes on every face. `schema-drift.remedy-sentence.test.ts` pins the three destructive column messages character for character. The report stays identical across faces, so `turso-remote-drift-detection.test.ts` keeps holding the local and remote reports equal. Only the human listings change what they print, and only for an entry whose driver declares a limit. - `os migrate plan` (human, `renderPlan`): changed as above. - `os migrate apply` (human, the listing before the prompt and the two held-back lines): changed as above. - `os migrate plan --json` and `os migrate apply --json`: unchanged (acceptance note 1). - The boot `[schema-drift]` warning (`SqlDriver.reconcileAndWarnDrift`): runs only on the Knex faces, where the apply door does rebuild, so its remedy is true. - The `OS_ARTIFACT_URL` boot gate (`artifact-boot-migration.ts`): returns on the REMOTE face's `driftReportCaveat` before it reads a message. - `REMOTE_DRIFT_REPORT_CAVEAT`: re-read, and still true ("applies those and names the rest as not applied"). Untouched. - The organization-ownership ceremony (`messageOf`): reads messages only for its own column drops, which the REMOTE face applies. Its remote pin is green. ## Verification All of it runs against a real `sqld` (libsql-server 0.24.32, `libsql://127.0.0.1:PORT?tls=0`, archive sha256 checked). The fixture is `apply.remote-face.integration.test.ts`'s own: a hand-made `os23848_note` table with three drifts, `legacy_23848` (`drop_column`), `code` (`tighten_not_null`) and `memo` (`relax_not_null`). Each remote case has a local SQLite file CONTROL. CI has no `sqld`, so the unit pins hold the same rules against a stubbed stack whose driver double carries the declaration: `plan.apply-limit.test.ts` and the new block in `apply.remote-posture.test.ts`. ### Red, then green: the plan - **Red at `a512b914f`.** That commit holds the pins alone; its production code equals the base `2e1562091`. - sqld: 1 failed, 6 passed. The failure is the remote plan case, with `Received: "os23848_note.code: metadata declares ... must be backfilled. Run "os migrate apply --allow-destructive"."`. The local-file CONTROL is green. - Unit: 2 failed, 2 passed. The CONTROL and the `--json` case are green. - **Green at `d99ee347e`.** sqld 7 of 7, unit 4 of 4. ### Red, then green: the apply's listing - **Red at `ca87de9ee`.** That commit holds the pins alone; its production code equals `59f489f6d`. - sqld: 2 failed, 7 passed. The failures are the remote `apply --yes` listing case and the listing assertion in the remote `apply --allow-destructive` human case, both receiving `... must be backfilled. Run "os migrate apply --allow-destructive".`. Both local-file CONTROLs are green: `apply --yes` on a file keeps the remedy and `2 destructive change(s) will be SKIPPED (re-run ...)`. - Unit (`apply.remote-posture.test.ts`): 2 failed, 10 passed. The CONTROL and the `--json` equality case are green. - **Green at `48ac51325`.** - sqld: 9 of 9, plus `organization-ownership.remote-face.integration.test.ts` 1 of 1 and `artifact-boot-migration.unbuildable-index.test.ts` 3 of 3. - Unit: 4 files, 28 tests. - The new `--json` pin compares the apply's document under a declaring driver with the one a driver that declares nothing gets, key for key and entry for entry. ### Red, then green: the apply's post-apply held-back line - **Red at `db6056e76`.** That commit holds the pins alone; its production code equals `455c7f288`. - Unit (`apply.remote-posture.test.ts`): 1 failed, 11 passed. The failure is the limited case: it received `[" ⚠ Skipped 2 destructive change(s) (re-run with --allow-destructive to include them)."]` where two lines were expected. The CONTROL (`Skipped 2 ...`, unchanged) is green. - sqld: 1 failed, 8 passed. The failure is the remote `apply --yes` case, with the same received line. The local-file CONTROL (`Skipped 2 ...`) is green. - **Green at `512414eef`.** - Unit: 4 files, 23 tests (`apply.remote-posture`, `apply.account-issuer-json-refusal`, `plan.apply-limit`, `schema-migrate.lazy-driver-import`). - sqld: `apply.remote-face.integration.test.ts` 9 of 9 and `organization-ownership.remote-face.integration.test.ts` 1 of 1. ### A pass for the wrong reason, closed `apply.account-issuer-json-refusal.test.ts` stubs the stack without the new `driftApplyLimit` member. Its human case ("the human face of the same refusal exits 1") therefore still exited 1, but from a TypeError in the listing, before the pre-flight ran. The stub now declares no limit, and the case asserts the pre-flight ran. ### Ablations - **The driver pin can fail.** At `d99ee347e`, `tighten_not_null` was changed from `'needs-table-rebuild'` to `'applied'` in `REMOTE_DRIFT_OP_ANSWERS`. - The mutation went through `scripts/ablation-replace.mjs` in WRAP mode, inside a script with a `trap` restore, and was proven on disk: anchor 1 to 0, blob `1a3a5461` to `37038760`. - `turso-remote-drift-apply-limit.test.ts` failed 1 and passed 3. - The file was restored to the HEAD blob, with `git diff HEAD` empty. - **The issuer pin's new assertion can fail.** At `455c7f288`, the stub's `driftApplyLimit: () => null,` was deleted the same way: anchor 1 to 0, blob `fcd562cd` to `7509aeb0`. - The human case went red with `expected "vi.fn()" to be called 1 times, but got 0 times`: 1 failed, 3 passed. - The file was restored to the HEAD blob, with `git diff HEAD` empty. - **No build.** Both subjects resolve to `src/` through relative imports. ### End to end through the CLI's own entry Both runs went through `tsx bin/run-dev.js`, on a `sqld` this run started and stopped by its PID, with the same three drifts on `os23925_note`. `migrate apply --yes` at `512414eef` exited 0, and read: ```text Destructive (requires --allow-destructive) ✗ os23925_note.code [tighten_not_null] os23925_note.code: metadata declares `storage.notNull` but the column is nullable — existing nulls must be backfilled. "os migrate apply" cannot apply it on this database: it needs the SQLite table rebuild, which the Turso REMOTE transport does not run. ✗ os23925_note.legacy_23925 [drop_column] os23925_note.legacy_23925: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it. ℹ 3 change(s): 0 safe, 1 needs-confirm, 2 destructive ⚠ 1 destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them). ⚠ 1 destructive change(s) remain drift that a re-run with --allow-destructive cannot apply on this database either (each says why above). ✓ Applied 0 change(s). ⚠ Skipped 1 destructive change(s) (re-run with --allow-destructive to include them). ⚠ Skipped 1 destructive change(s) that a re-run could not apply on this database either: they remain drift (each says why above). ⚠ 1 change(s) were NOT applied — the driver could not perform them on this database (its log above says why), so the schema still differs from metadata there: • os23925_note.memo — relax_not_null ``` `migrate plan` at `d99ee347e` exited 0, and its footer read: ```text Apply with: os migrate apply (add --allow-destructive for the 1 destructive change(s) it can apply on this database) ⚠ 2 change(s) above cannot be applied on this database by "os migrate apply" (each says why), so they remain drift after it runs. ``` ### Tests and gates Recorded at `512414eef` unless a line says otherwise. - **Typecheck.** - `@objectstack/cli` (`tsc --noEmit`, then `check:test-typecheck` OK): exit 0 at `512414eef`. - `@objectstack/driver-turso` and `@objectstack/driver-sql`: exit 0 at `59f489f6d`. No driver file has changed since. - **`@objectstack/cli` unit tier.** 3 shards at `455c7f288`: 103 + 102 + 102 files and 1754 + 1335 + 1486 tests, all passed. Since then the diff changes only the post-apply line in `apply.ts` and two test files, and the unit pins that reach it ran at `512414eef`: 4 files, 23 tests. - **`@objectstack/cli` integration, run locally.** - At `512414eef`, on `sqld`: `apply.remote-face.integration.test.ts` 9 of 9, and `organization-ownership.remote-face.integration.test.ts` 1 of 1. - At `48ac51325`: `artifact-boot-migration.unbuildable-index.test.ts` 3 of 3. - At `59f489f6d`: `plan.telemetry-sibling`, `plan.deferred-reads` and `plan.reads-env-files`, 3 files, 10 tests. - The rest of the tier is CI's. - **The driver suites, run at `d99ee347e`.** No driver file has changed since. - `@objectstack/driver-turso`: 92 files, 2429 passed, 33 skipped. - `@objectstack/driver-sql`: 223 files passed and 11 skipped; 3677 tests passed and 208 skipped. - **Derived gate families.** `dispatch-gates --commands --repo objectstack-ai/objectstack` gives 99 commands. That is the 69 from before plus 30 the docs path adds (the doc, docs and spec docs families). Each exit code was captured before any pipe. - `--ran` reconciliation: 99 derived, 97 run with exit 0, 2 NOT MEASURED, 0 UNRUN. - The two NOT MEASURED are `check:dual-build-cjs-loads` (packages outside this build closure have no `dist/`) and `check:i18n-coverage` (a partial round). Both exited 3, PREREQUISITE NOT MET. - `pnpm --filter @objectstack/spec run check:skill-examples` first exited 3, because `@objectstack/client-react` was not built. After building the client SDK closure (all cache hits) it exited 0: 263 prose examples type-check across 3 surfaces. That exit 0 is the one recorded. - **`pnpm check:doc-authoring`.** Exit 0. - **The changeset gates.** - `check-changeset-no-major.mjs --base origin/main` exits 0 with no event, and exits 0 with an event carrying this body. With this body it reads `LEVEL AXIS: this PR declares clause-② yes (widening) ... at minor or above`, naming `@objectstack/driver-sql: minor` and `@objectstack/driver-turso: minor`, with `@objectstack/cli: patch` not refused. - `check-adr-0087-registration.mjs --base origin/main` exits 0: `this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)`. - **CI at `512414eef`, read 20:17Z.** - 34 success, including `Build Docs` and four of the five required contexts (`Lint & Repo Gates`, `TypeScript Type Check`, `Build Core` and `Governed Surface Queue Guard`). - 5 skipped by their selectors. - `Test Core (3/6)` was still in progress, so the `Test Core` roll-up had not reported yet. ## File surface The claim's surface, and its amendments 6222387090, 6223299622 and 6223389541: - `packages/cli/src/commands/migrate/apply.ts`: only the listing before the prompt and the held-back count line (amendment 6222387090). - `packages/cli/src/utils/schema-migrate.ts`: the `driftApplyLimit` seam, the two shared helpers, and an optional `describe` argument to `renderPlan`. - `packages/drivers/driver-sql/src/index.ts`: one re-export line. - `packages/cli/src/commands/migrate/apply.ts`, by amendment 2 (6223299622): the post-apply held-back line as well. - `content/docs/deployment/cli.mdx`, by amendment 3 (6223389541): one caveat under the migrate category table, and nothing else on the page. - Tests beside each: - `plan.apply-limit.test.ts`; - `apply.remote-posture.test.ts`, whose stub stack gains the member, plus a new block; - `apply.account-issuer-json-refusal.test.ts`, whose stub gains the member, plus one assertion; - `apply.remote-face.integration.test.ts`, which holds the sqld fixture; - `schema-drift.remedy-sentence.test.ts`; - `turso-remote-drift-apply-limit.test.ts`. Nothing under `packages/spec/**`, and no published driver contract (`packages/spec/src/contracts/**`) gains a member. Neither command reads `dialect`, the transport, the URL or a class name. ## Acceptance notes 1. **The `--json` documents are unchanged.** In `os migrate plan --json`, `changes[].message` keeps the shared remedy, and no field marks an entry the database cannot apply. `os migrate apply --json` is pinned equal to what a driver that declares nothing produces. Adding a mark widens a document, and no consumer of either JSON document was measured to need one. 2. **`drop_column_default`.** Its message carries `run "os migrate apply"` mid-sentence rather than as a tail. On a remote primary, the listing line therefore shows that sentence with the limit after it. That entry needs a literal runtime-token DEFAULT written by an old build, so a database the current sync created does not reach it. 3. **The REMOTE apply door's log.** A skipped rebuild entry now reads `... skipped: it needs the SQLite table rebuild, which the Turso REMOTE transport does not run`, from the declaration's sentence. It used to end `which this transport does not run`. A report-only `unbuildable_index` on that face is now logged as skipped; it used to be skipped with no log line. No test pins either log line. --- _Generated by [Claude Code](https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b74434c commit c4111c6

14 files changed

Lines changed: 1058 additions & 23 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/cli': patch
3+
'@objectstack/driver-sql': minor
4+
'@objectstack/driver-turso': minor
5+
---
6+
7+
fix(cli,driver-turso): `os migrate plan` and `os migrate apply` on a Turso REMOTE database no longer offer `--allow-destructive` for a change that database cannot apply
8+
9+
Clause-②: yes (widening)
10+
11+
On a primary reached over the Turso REMOTE transport (`libsql://…`), `os migrate plan` listed a NOT NULL tightening with the remedy `Run "os migrate apply --allow-destructive".` and closed with `(add --allow-destructive for drops / tightenings)`. That transport runs no SQLite table rebuild, so the apply then named the tightening as not applied and exited 1. The plan now says so before the apply: each change the database cannot apply (a tightened or relaxed NOT NULL, a dropped DEFAULT) is still listed, with its finding and `"os migrate apply" cannot apply it on this database: it needs the SQLite table rebuild, which the Turso REMOTE transport does not run.` in place of the remedy. The footer offers `--allow-destructive` only for the destructive changes the database can apply (a column drop, an index change) and says how many listed changes remain drift after the apply. The listing `os migrate apply` prints before its prompt reads the same way. Its counts of held-back destructive changes, before the prompt and after the run, offer a re-run with `--allow-destructive` only for the ones that re-run could apply, and name the rest as drift it cannot apply either. On every other database both commands read exactly as before, and their `--json` documents are unchanged on every database.
12+
13+
`@objectstack/driver-turso`: `TursoDriver.driftApplyLimit(opType)` is new. It returns why the driver's `applyMigrationEntries` cannot perform a drift op of that type on this face, or `null` when it can. It answers from the same per-op declaration the REMOTE apply door dispatches by, and returns `null` for every op on the local and embedded-replica faces. A driver that does not implement it is read as applying every op the differ means it to.
14+
15+
`@objectstack/driver-sql`: `driftFindingWithoutRemedy(entry)` is a new export. It returns a drift entry's finding without the `os migrate apply --allow-destructive` sentence that a destructive column entry's message ends with, and returns every other message unchanged; the messages themselves keep their exact text.

‎content/docs/deployment/cli.mdx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,8 @@ occupancy on its own.
10091009
| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` — except `manual_column_type_change` (only `os migrate multi-value-columns --apply` runs it), and `manual_widen_varchar_to_text` and `unbuildable_index`, which nothing applies |
10101010
| `destructive` | drop an orphaned column or index, tighten `NOT NULL`, narrow a type, rebuild an index as `UNIQUE`, create a `UNIQUE` index existing rows already violate | `os migrate apply --allow-destructive` |
10111011
1012+
On a Turso remote primary (`libsql://`), a tightened or relaxed `NOT NULL` and a dropped DEFAULT stay drift, because that transport runs no SQLite table rebuild. `os migrate plan` and `os migrate apply` list each such entry with `"os migrate apply" cannot apply it on this database: it needs the SQLite table rebuild, which the Turso REMOTE transport does not run.`, offer `--allow-destructive` only for the destructive changes that database can apply, and count the rest as drift a re-run cannot apply.
1013+
10121014
`--allow-destructive` refuses some drops until the data they would lose is
10131015
safe. No flag skips these refusals:
10141016

‎packages/cli/src/commands/migrate/apply.account-issuer-json-refusal.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ function bootWithIssuerDrop(): void {
9494
telemetryDatabase: null,
9595
composition: { plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, notes: [] },
9696
detectManagedDrift: async () => [ISSUER_DROP],
97+
// [#23925] The SchemaStack member the listing reads: this driver declares no apply limit.
98+
driftApplyLimit: () => null,
9799
pendingSchemaWork: [],
98100
kernel: { getService: () => ({}) },
99101
flushSchemaDdl,
@@ -186,6 +188,8 @@ describe('controls', () => {
186188
const { exit } = await runApply(ARGV);
187189

188190
expect(exit).toBe(1);
191+
// Refused by the pre-flight, not by anything earlier on the human face.
192+
expect(mocks.probeAccountIdentityCollisions).toHaveBeenCalledTimes(1);
189193
expect(applyMigrationEntries).not.toHaveBeenCalled();
190194
}, RUN_TIMEOUT);
191195

‎packages/cli/src/commands/migrate/apply.remote-face.integration.test.ts‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
* commands, and every claim about the server is read back through a SECOND
3434
* libSQL connection.
3535
*
36+
* [#23925] The human plan says the same thing up front, read from the driver's
37+
* declaration (`TursoDriver.driftApplyLimit`): the tightening and the relax
38+
* are listed with the transport's limit instead of a remedy it cannot run, and
39+
* the drop keeps `--allow-destructive`. So does the apply's own listing before
40+
* its prompt, whose held-back count offers a re-run only for the drop. The
41+
* local file is the control: its plan and its apply read as they always have.
42+
*
3643
* ## How to run it
3744
*
3845
* It needs a `sqld` binary (libsql-server), so the remote cases are skipped
@@ -242,6 +249,28 @@ const DROP = `${TABLE}.legacy_23848 drop_column`;
242249
const TIGHTEN = `${TABLE}.code tighten_not_null`;
243250
const RELAX = `${TABLE}.memo relax_not_null`;
244251

252+
/**
253+
* [#23925] The line the human plan prints under an entry's `[op]` heading on
254+
* the fixture table: the entry's finding and its remedy.
255+
*/
256+
function planEntryLine(lines: string, op: string): string {
257+
const all = lines.split('\n');
258+
const at = all.findIndex((l) => l.includes(`${TABLE}.`) && l.includes(`[${op}]`));
259+
expect(at, `no ${TABLE} [${op}] entry in:\n${lines.slice(-3000)}`).toBeGreaterThanOrEqual(0);
260+
return all[at + 1]!;
261+
}
262+
263+
/** [#23925] The plan's closing lines: what to apply with, and what this database cannot apply. */
264+
const planFooter = (lines: string) =>
265+
lines.split('\n').filter((l) => /Apply with:|cannot be applied on this database/.test(l)).join('\n');
266+
267+
/** [#23925] The apply's held-back lines before its prompt: what a re-run would apply, and what it would not. */
268+
const heldBackLines = (lines: string) =>
269+
lines.split('\n').filter((l) => /will be SKIPPED|re-run with --allow-destructive cannot/.test(l));
270+
271+
/** [#23925] The apply's held-back lines after it ran. */
272+
const postApplySkippedLines = (lines: string) => lines.split('\n').filter((l) => l.startsWith(' ⚠ Skipped '));
273+
245274
beforeAll(() => {
246275
for (const key of OVERRIDING_ENV) {
247276
savedEnv[key] = process.env[key];
@@ -271,6 +300,40 @@ afterAll(() => {
271300
// control's document whenever a remote case before it failed at its boot (the
272301
// positive control and the ablation are exactly such runs).
273302
describe('CONTROL: the same apply on a local SQLite file applies all three, as it always has', () => {
303+
// [#23925] The local face declares no limit, so its plan reads as it always
304+
// has: the tightening keeps the differ's remedy, and the footer offers
305+
// --allow-destructive for drops and tightenings.
306+
it('plan on a file, human face: the tightening keeps "--allow-destructive", the footer is unchanged', async () => {
307+
const file = join(dir, 'local-plan.db');
308+
await buildFixture(`file:${file}`, (seed) => local(file, seed));
309+
310+
const plan = await run(MigratePlan, ['--database-url', `file:${file}`]);
311+
expect(plan.exitCode, plan.lines.slice(-1500)).toBe(0);
312+
expect(planEntryLine(plan.lines, 'tighten_not_null')).toContain(
313+
`${TABLE}.code: metadata declares \`storage.notNull\` but the column is nullable — existing nulls must be backfilled. `
314+
+ 'Run "os migrate apply --allow-destructive".',
315+
);
316+
expect(planEntryLine(plan.lines, 'drop_column')).toContain('"os migrate apply --allow-destructive" to drop it.');
317+
expect(plan.lines).not.toContain('cannot apply it on this database');
318+
expect(planFooter(plan.lines)).toBe(' Apply with: os migrate apply (add --allow-destructive for drops / tightenings)');
319+
}, CASE_TIMEOUT_MS);
320+
321+
// [#23925] …and so does its apply's listing before the prompt.
322+
it('apply --yes on a file, human face: the tightening keeps "--allow-destructive", both destructive entries are offered a re-run', async () => {
323+
const file = join(dir, 'local-apply.db');
324+
await buildFixture(`file:${file}`, (seed) => local(file, seed));
325+
326+
const apply = await run(MigrateApply, ['--database-url', `file:${file}`, '--yes']);
327+
expect(apply.exitCode, apply.lines.slice(-1500)).toBe(0);
328+
expect(planEntryLine(apply.lines, 'tighten_not_null')).toContain(
329+
`${TABLE}.code: metadata declares \`storage.notNull\` but the column is nullable — existing nulls must be backfilled. `
330+
+ 'Run "os migrate apply --allow-destructive".',
331+
);
332+
expect(apply.lines).not.toContain('cannot apply it on this database');
333+
expect(heldBackLines(apply.lines)).toEqual([' ⚠ 2 destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).']);
334+
expect(postApplySkippedLines(apply.lines)).toEqual([' ⚠ Skipped 2 destructive change(s) (re-run with --allow-destructive to include them).']);
335+
}, CASE_TIMEOUT_MS);
336+
274337
it('apply --allow-destructive on a file: exit 0, every entry applied, nothing skipped', async () => {
275338
const file = join(dir, 'local.db');
276339
await buildFixture(`file:${file}`, (seed) => local(file, seed));
@@ -319,6 +382,59 @@ describe.skipIf(!SQLD)('os migrate plan / apply on a Turso REMOTE database (sqld
319382
expect(await logicalDump()).toBe(before);
320383
}, CASE_TIMEOUT_MS);
321384

385+
// [#23925] Before the apply below names them as not applied, the plan says so:
386+
// read from the driver's declaration, never from the dialect or the URL.
387+
it('plan, human face: the tightening and the relax name the transport\'s limit, never --allow-destructive; the drop keeps it', async () => {
388+
const before = await logicalDump();
389+
const plan = await run(MigratePlan, ['--database-url', url]);
390+
expect(plan.exitCode, plan.lines.slice(-1500)).toBe(0);
391+
392+
const tighten = planEntryLine(plan.lines, 'tighten_not_null');
393+
expect(tighten).not.toContain('--allow-destructive');
394+
expect(tighten).toContain(
395+
`${TABLE}.code: metadata declares \`storage.notNull\` but the column is nullable — existing nulls must be backfilled. `
396+
+ '"os migrate apply" cannot apply it on this database: it needs the SQLite table rebuild, which the Turso REMOTE '
397+
+ 'transport does not run.',
398+
);
399+
expect(planEntryLine(plan.lines, 'relax_not_null')).toContain('"os migrate apply" cannot apply it on this database');
400+
const drop = planEntryLine(plan.lines, 'drop_column');
401+
expect(drop).toContain('"os migrate apply --allow-destructive" to drop it.');
402+
expect(drop).not.toContain('cannot apply it on this database');
403+
404+
const footer = planFooter(plan.lines);
405+
expect(footer).not.toContain('drops / tightenings');
406+
expect(footer).toContain('Apply with: os migrate apply (add --allow-destructive for the 1 destructive change(s) it can apply on this database)');
407+
expect(footer).toMatch(/2 change\(s\) above cannot be applied on this database by "os migrate apply"/);
408+
expect(await logicalDump()).toBe(before);
409+
}, CASE_TIMEOUT_MS);
410+
411+
// [#23925] The apply's own listing says the same before its prompt, and offers
412+
// a re-run with --allow-destructive only for the drop that re-run could apply.
413+
it('apply --yes, human face: the listing names the limit, and only the drop is offered a re-run', async () => {
414+
const apply = await run(MigrateApply, ['--database-url', url, '--yes']);
415+
expect(apply.exitCode, apply.lines.slice(-1500)).toBe(0);
416+
const tighten = planEntryLine(apply.lines, 'tighten_not_null');
417+
expect(tighten).not.toContain('--allow-destructive');
418+
expect(tighten).toContain(
419+
'"os migrate apply" cannot apply it on this database: it needs the SQLite table rebuild, which the Turso REMOTE '
420+
+ 'transport does not run.',
421+
);
422+
expect(planEntryLine(apply.lines, 'relax_not_null')).toContain('"os migrate apply" cannot apply it on this database');
423+
expect(planEntryLine(apply.lines, 'drop_column')).toContain('"os migrate apply --allow-destructive" to drop it.');
424+
expect(heldBackLines(apply.lines)).toEqual([
425+
' ⚠ 1 destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).',
426+
' ⚠ 1 destructive change(s) remain drift that a re-run with --allow-destructive cannot apply on this database either (each says why above).',
427+
]);
428+
// Unchanged after the prompt: the relax the driver could not apply is still named.
429+
expect(apply.lines).toContain(`${TABLE}.memo — relax_not_null`);
430+
// The post-apply held-back line splits the same way.
431+
expect(postApplySkippedLines(apply.lines)).toEqual([
432+
' ⚠ Skipped 1 destructive change(s) (re-run with --allow-destructive to include them).',
433+
' ⚠ Skipped 1 destructive change(s) that a re-run could not apply on this database either: they remain drift (each says why above).',
434+
]);
435+
expect(await remote(columnsOf)).toMatchObject({ code: false, memo: true, legacy_23848: false });
436+
}, CASE_TIMEOUT_MS);
437+
322438
it('apply without --allow-destructive: names the entry it could not apply, keeps the destructive two back, exits 0', async () => {
323439
const apply = await run(MigrateApply, ['--database-url', url, '--yes', '--json']);
324440
expect(apply.payload.error, JSON.stringify(apply.payload).slice(0, 800)).toBeUndefined();
@@ -353,6 +469,9 @@ describe.skipIf(!SQLD)('os migrate plan / apply on a Turso REMOTE database (sqld
353469
const apply = await run(MigrateApply, ['--database-url', url, '--allow-destructive', '--yes']);
354470
expect(apply.exitCode, apply.lines.slice(-1500)).toBe(1);
355471
expect(apply.lines).toMatch(/additive schema work was NOT examined/i);
472+
// [#23925] The listing before the prompt already said it could not apply the tightening.
473+
expect(planEntryLine(apply.lines, 'tighten_not_null')).toContain('"os migrate apply" cannot apply it on this database');
474+
expect(planEntryLine(apply.lines, 'tighten_not_null')).not.toContain('--allow-destructive');
356475
expect(apply.lines).toContain(`${TABLE}.code — tighten_not_null`);
357476
expect(apply.lines).toContain(`${TABLE}.memo — relax_not_null`);
358477
expect(apply.lines).toMatch(/PARTIAL/);

0 commit comments

Comments
 (0)