Repository navigation
Commit 93f3526
fix(cli): the read-only migrate arms say they write no row or table, and that opening a SQLite file can still switch its journal mode (#23608)
Fixes #23562
Clause-②: no
## What changed
The ownership ceremony's plan arm (`os migrate organization-ownership`
without `--apply`) printed "The plan below writes nothing, but --apply
will refuse until the database is free." beside a holder, and closed
with "Plan written to FILE — nothing in the database was changed." Its
boot is read-only, but it still opens the file through the SQL driver,
and the driver's connect switches a SQLite file still on a rollback
journal to WAL. Measured below: header bytes 18 and 19 go from 1,1 to
2,2 and the change counter moves; no row or table does.
Both sentences now claim no row or table, followed by one shared note,
`JOURNAL_MODE_SWITCH_NOTE`, exported beside `describeOccupancy` in
`packages/cli/src/utils/sqlite-occupancy.ts`:
> Opening a SQLite file can still switch its journal mode (to WAL by
default), which changes its header only.
The closing line carries the note only on a SQLite target: the occupancy
probe answers `not_applicable` for PostgreSQL, MySQL and `:memory:`. The
same sentence is corrected in `os migrate plan` (triage's fold-in) and
in six sibling read-only arms (the four-condition fold-ins below). The
driver's first-connect rule does not change: no
`packages/drivers/driver-sql` path is touched. No exit code, `--json`
document or refusal changes.
## Measurements (real CLI through `bin/run-dev.js`, at `9474bc6ea9`)
Each run used a fresh SQLite file on a rollback journal, holding one
`sys_organization` row. An idle better-sqlite3 connection in ANOTHER
process held it open, which is the case the fd probe sees. Each command
ran once.
| command | header bytes 18,19 | change counter | exit | busy warning
printed |
|---|---|---|---|---|
| `os migrate organization-ownership` (plan) | 1,1 to 2,2 | 2 to 3 | 0 |
yes, the new text |
| `os migrate plan` | 1,1 to 2,2 | 2 to 3 | 0 | yes, the new text |
| `os migrate summary-nulls` (dry run) | 1,1 to 2,2 | 2 to 3 | 1 (the
scratch project has no compiled artifact; the boot had already run) |
yes, the new text |
Measured with better-sqlite3 alone:
- A switch to WAL and back moves only header bytes 18, 19, 24-27 and
92-95. After a clean close, no sidecar remains.
- The `auto_vacuum` setter the driver sends to a legacy NONE file that
holds tables moves no byte (md5 unchanged, under both WAL and a rollback
journal). So the journal-mode switch is the only header change.
- **(a) The call path:** plan arm, then `bootSchemaStack`
(`utils/schema-migrate.ts`), `createStandaloneStack`
(`@objectstack/runtime`), `DefaultDatasourcePlugin`,
`SqlDriver.connect()`, `applyAutoVacuumIncremental()`,
`applySqliteJournalMode()`.
- **(b) `readOnlyProbe` acts only on an ABSENT file.** It sets
`sqliteAbsentFile: 'empty-in-memory'` behind an `existsSync` guard in
`standalone-stack.ts`, and `SqlDriver.knexConfigFor` redirects only a
missing file. An existing file opens as it is, and `connect()` runs both
pragmas. So `plan.ts:134`'s "nothing is written" was the same false
sentence. It is folded in, and the table above measures it.
- **(e) Where the driver reads `OS_DATABASE_SQLITE_JOURNAL_MODE`:** from
`process.env` at connect. `bootSchemaStack` first loads the project's
`.env*` into `process.env` through dotenv-flow, which never overrides an
exported variable. So the arm reads its own shell and the project
`.env`, and never the serving process's shell.
- Measured: with `=delete` exported in the arm's shell, a WAL file is
switched to a rollback journal (2,2 to 1,1).
- Measured: with `=delete` only in the project `.env`, a rollback file
is left alone (1,1 to 1,1).
- This changes which sentence is true: a note saying only "switches to
WAL" is false under the opt-out. So the note says "switch its journal
mode (to WAL by default)".
## Fold-ins: the four conditions, one row each
Every row is the warning a read-only arm prints beside a holder. Each
arm boots `bootSchemaStack` with `readOnlyProbe` on that arm, which
reaches the same `connect()` as (a).
| site | ① same class | ② mechanical | ③ no other claim on the file | ④
same gate family | folded |
|---|---|---|---|---|---|
| `plan.ts:134` | yes, measured | one sentence + import | none open (the
claim's read at `19c554ddc1`) | `packages/cli`, one derived list | yes,
triage's fold-in |
| `audit-metadata-bodies.ts:118` | yes, read | one sentence + import |
none open | same | yes |
| `comment-reactions.ts:139` | yes, read | one sentence + import | none
open | same | yes |
| `files-to-references.ts:195` | yes, read | one sentence + import |
none open | same | yes |
| `meta.ts:1442` | yes, read | one sentence + import | none open | same
| yes |
| `multi-value-columns.ts:397` | yes, read | one sentence + import |
none open | same | yes |
| `summary-nulls.ts:159` | yes, measured (table above) | one sentence +
import | none open | same | yes |
| `security-catalog-overlays.ts:198` | yes, read | yes | **no**: PR
#23469 (open, draft) edits this file at `:290` | same | **no**, see
Acceptance notes |
## Tests, round 0 (all on `86ce1100c3`, the head this PR opened with)
- `pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2
--reporter=verbose` over three files:
`organization-ownership.journal-mode.integration.test.ts`,
`organization-ownership.occupancy.integration.test.ts` and
`utils/sqlite-occupancy.test.ts`. Result: `Test Files 3 passed (3)`,
`Tests 21 passed (21)`, `VERDICT command-exit 0`. The new real-boot case
took 5442 ms (shared box).
- `pnpm --filter @objectstack/cli exec vitest run --project unit
--maxWorkers=2`: `Test Files 290 passed (290)`, `Tests 4324 passed
(4324)`, `VERDICT command-exit 0`. The integration layer is declared to
CI. The two integration files this PR touches ran by path, above.
- `pnpm --filter @objectstack/cli build && pnpm --filter
@objectstack/cli typecheck`: exit 0. `check:test-typecheck` held its
debt at 3 files, 28 errors, 6 signatures. `tsc --noEmit --listFiles -p
tsconfig.json` puts both touched test files and `sqlite-occupancy.ts` in
its 1564-file program.
- **New pin,
`organization-ownership.journal-mode.integration.test.ts`:** the real
boot runs on a rollback-journal file. It asserts:
- header bytes 1,1 before and 2,2 after;
- rows unchanged;
- the closing line says "no row or table in the database was changed."
and carries `JOURNAL_MODE_SWITCH_NOTE`;
- no line says "nothing in the database was changed".
If a read-only boot ever stops switching the file, the header assertion
goes red, and the note and this pin can both go.
- **Extended pin, `organization-ownership.occupancy.integration.test.ts`
(plan case):** the busy warning says "The plan below writes no row or
table" and carries the note, and no line says "writes nothing".
- **Not pinned:** the six siblings and `plan.ts`. No existing test
asserted their old text.
## Ablation (one-shot, run through `scripts/ablation-replace.mjs` with
HEAD restores, at `6a65958a88`)
The fix was committed first. Each leg puts one old sentence back into
`organization-ownership.ts` and runs both pin files:
- **Leg 1, the closing line:** anchor hit once, blob `fa72f5c40052` to
`b853ff316a93`. The new pin went RED: `Received: " ✓ Plan written to
…/plan.json — nothing in the database was changed."`. The header
assertion before it passed, so the file was switched under the old text
too. `1 failed | 5 passed`.
- **Leg 2, the busy warning:** blob `fa72f5c40052` to `9996f6490f4f`.
The occupancy plan case went RED: `expected ' ⚠
/tmp/os-ownership-occupancy-…' to contain 'The plan below writes no row
or table'`. `1 failed | 5 passed`.
- **Restore, both legs:** blob equals HEAD (`fa72f5c40052`), and `git
diff HEAD` is empty. The wrapping script's own trap proved it again.
- The direction was red, as expected.
## Gates, round 0 (on `86ce1100c3`)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 64 families from the merge base
`19c554ddc`, over 12 paths and 235 changed lines. All 64 ran, each exit
code captured before any pipe.
- `check:dual-build-cjs-loads` and `check:i18n-coverage` first exited 3
(PREREQUISITE NOT MET: missing `dist/`). After `turbo run build
--filter=@objectstack/connector-slack`, both re-ran and exited 0.
`check:i18n-coverage` reported `OK (14 config(s), 621 baselined
untranslated string(s), none new)`.
- `--ran` then printed `64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN`.
- **Lint, by the narrowing:** `pnpm exec eslint --no-inline-config` over
the 11 changed `.ts` files exited 0, with no "ignored" notice.
`--print-config` shows 5 and 6 active rules on two of them. The
repo-wide `pnpm lint` is CI's.
- Every family CI runs is read from this PR's check-runs, not re-run
here.
## Patch round 1 (head `cc578600b0`; the seat's rulings are PR comment
6184246424)
The seat answered the three open questions: Q1 A
(`security-catalog-overlays.ts` stays out; the seat files the remainder
as its own card), Q2 A (fixed in this round), Q3 A (keep the text). This
round makes four changes, and nothing else changes.
1. **`.changeset/23400-ownership-occupancy-gate.md:15`** (unreleased):
"The plan (no `--apply`) writes nothing, so it still runs." now reads
"The plan (no `--apply`) writes no row or table, so it still runs." The
rest of that file is byte-identical: the committed file equals the HEAD
copy with that one replacement, 3143 to 3151 bytes.
2. **`multi-value-columns.ts:334`, the help description**: "… and the
database they would run against, and writes nothing." now reads "… and
writes no row or table." This PR's changeset gains a FROM and TO line
for it. The help text carries no note, so the changeset's note sentence
now says "each warning". No test asserted the old description (`git
grep` over the repo: only the source line).
3. **Developer comments in files this PR already touches**, one phrase
each:
- `organization-ownership.ts` header (:75): "The one thing it writes is
the plan FILE" now reads "It writes no row or table; the one thing it
writes is the plan FILE" (re-wrapped over the same three lines).
- `files-to-references.ts:98`: "writes NOTHING" now reads "writes NO
ROW". `:191`: "writes nothing" now reads "writes no row".
- `plan.ts:55` and `:143`: "writes nothing" now reads "writes no row".
- `audit-metadata-bodies.ts:57`: "(writes nothing)" now reads "(writes
no row)".
- `summary-nulls.ts:54`: "(writes nothing at all)" now reads "(writes no
row at all)".
- `meta.ts:1489`: the quoted report text "writes nothing" now reads
"writes no row".
- Left as written, because they are true: `plan.ts:65` ("nothing is
written to disk", a never-started project opened in memory), and the
`organization-ownership.ts` header's "no database file brought into
existence".
- `organization-ownership.ts:25-29` and `:579` (PR #23469's hunks) are
untouched.
4. **Read only, no edit: `recorded-by.ts:158` and `resume.ts:172`.**
Both print "Nothing was changed." after the same read-only boot through
the same connect, so on a SQLite file still on a rollback journal both
sentences are false the same way. The answer is in the Acceptance notes.
**Tests on `cc578600b0`:**
- `pnpm --filter @objectstack/cli build && pnpm --filter
@objectstack/cli typecheck && vitest run` by path over the three pin
files: `Test Files 3 passed (3)`, `Tests 21 passed (21)`, `VERDICT
command-exit 0`. `check:test-typecheck` held its debt at 3 files, 28
errors, 6 signatures. The real-boot case took 5421 ms.
- `vitest run --project unit --maxWorkers=2`: `Test Files 290 passed
(290)`, `Tests 4324 passed (4324)`, `VERDICT command-exit 0`. Narrowed
`eslint --no-inline-config` over the 11 changed `.ts` files exited 0.
- No pin changed structure, so there is no ablation this round.
**Gates on `cc578600b0`:**
- `dispatch-gates --commands --repo objectstack-ai/objectstack` derived
the same 64 families as round 0, from 13 paths and 260 changed lines.
All 64 ran, each exit captured before any pipe. 61 exited 0.
`check:dual-build-cjs-loads` and `check:i18n-coverage` first exited 3
(missing `dist/` in the fresh worktree); both re-ran after the later
gates had built it and exited 0. `--ran` printed `64 derived, 64 run, 0
NOT-MEASURED, 0 UNRUN`.
- **One gate is red, by design: `node scripts/check-empty-changeset.mjs
--base origin/main` exits 1.** It reports: "This PR changes a changeset
it did not add: .changeset/23400-ownership-occupancy-gate.md". This is
the gate's DELIBERATE CORRECTION class, not a filename collision. The
PR's own changeset keeps its issue-scoped name, and the 23400 note is
corrected because its plan sentence was false. The gate's own remedy for
this class is to say so on the PR and get it confirmed. The confirmation
is the seat's ruling, PR comment 6184246424 (Q2: A). The gate stays red
until a person takes that decision, and no `skip-changeset` label is
applied.
- The tool printed a STALE TREE notice:
`scripts/measure-test-shard-timings.mjs` changed on `origin/main`
(`fc1a55a212`) after the base.
- `git merge-tree --write-tree HEAD origin/main` is clean (exit 0, tree
`84fc586f90`), so `origin/main` was not merged in, by the order. Since
the base, main has touched nothing in `packages/cli`,
`packages/runtime`, `packages/drivers` or the 23400 changeset.
## Acceptance notes
- **`os migrate recorded-by` and `os migrate resume` (read, not
measured, not edited).** Both boot the same read-only stack and print
"Nothing was changed.", so on a SQLite file still on a rollback journal
both sentences are false in the same way:
- `recorded-by.ts:105-110`: the dry run calls `bootSchemaStack({ …,
deferSchemaDdl: true, readOnlyProbe: true })` (`:109`, when no `--apply`
is given).
- `resume.ts:114-119`: the list mode calls the same (`:118`, when no
`--run` is given).
- That boot takes the call path measured above: `createStandaloneStack`,
`SqlDriver.connect()`, `applySqliteJournalMode()`.
- Each then prints `printInfo("Nothing was changed. …")`:
`recorded-by.ts:158` (pending sentinel rows exist) and `resume.ts:172`
(interrupted runs exist).
- Neither command probes occupancy, so neither knows whether its target
is a SQLite file, and the sentence prints on every dialect.
- The seat files these with the `security-catalog-overlays.ts:198`
remainder.
- **`security-catalog-overlays.ts:198`** ("The preview below writes
nothing.") stays out by the seat's Q1 ruling: PR #23469 holds the file.
The seat files it as its own card, ordered after PR #23469, using the
exported `JOURNAL_MODE_SWITCH_NOTE`.
- **Not built (seat's Q3 ruling: keep the text):** a read-only boot that
keeps the file's journal mode by declaring the driver's
`sqliteJournalMode` from the header it finds.
- **`organization-ownership.ts` imports the constant on its own line**
(24), beside the `describeOccupancy` import, because the claim holds
line 23 off-limits.
This body was updated for patch round 1 by session
`session_01Q9PZ5Pzqcus4woVvTZwWNU` through the fleet relay; the original
was written by the same session at PR creation.
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent af25552 commit 93f3526
13 files changed
Lines changed: 227 additions & 33 deletions
File tree
- .changeset
- packages/cli/src
- commands/migrate
- utils
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
| 15 | + | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| |||
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
| 21 | + | |
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
57 | | - | |
| 57 | + | |
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
| |||
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | | - | |
| 118 | + | |
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| |||
136 | 136 | | |
137 | 137 | | |
138 | 138 | | |
139 | | - | |
| 139 | + | |
140 | 140 | | |
141 | 141 | | |
142 | 142 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| |||
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
98 | | - | |
| 98 | + | |
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
| |||
188 | 188 | | |
189 | 189 | | |
190 | 190 | | |
191 | | - | |
| 191 | + | |
192 | 192 | | |
193 | 193 | | |
194 | 194 | | |
195 | | - | |
| 195 | + | |
196 | 196 | | |
197 | 197 | | |
198 | 198 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| |||
1439 | 1439 | | |
1440 | 1440 | | |
1441 | 1441 | | |
1442 | | - | |
1443 | | - | |
| 1442 | + | |
| 1443 | + | |
1444 | 1444 | | |
1445 | 1445 | | |
1446 | 1446 | | |
| |||
1486 | 1486 | | |
1487 | 1487 | | |
1488 | 1488 | | |
1489 | | - | |
| 1489 | + | |
1490 | 1490 | | |
1491 | 1491 | | |
1492 | 1492 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| |||
331 | 331 | | |
332 | 332 | | |
333 | 333 | | |
334 | | - | |
| 334 | + | |
335 | 335 | | |
336 | 336 | | |
337 | 337 | | |
| |||
394 | 394 | | |
395 | 395 | | |
396 | 396 | | |
397 | | - | |
| 397 | + | |
398 | 398 | | |
399 | 399 | | |
400 | 400 | | |
| |||
0 commit comments