fix(driver-sql): declare the indexes key initObjects / registerObjectMetadata already read - #16710
Conversation
…bjectMetadata` already read
Both entry points took `Array<{ name; fields?; tenancy? }>` with no `indexes`,
while `registerManagedObjectMetadata` read the key out of those very objects
through an `(obj as any)` cast and filled `managedObjectIndexes` from it — the
map `syncDeclaredIndexes` renders every declared UNIQUE from. The sibling
`detectManagedDrift` on the same class already declared `indexes?: any[]`, so
the two halves of one class disagreed about the shape of the same input. This
is the shape #4311 fixed for `tenancy`, one key over.
Nothing tripped over it because TypeScript's excess-property check fires on a
fresh object literal and not on one bound to a variable first, and every caller
in the package happened to bind first — a green that held for a reason
unrelated to correctness.
- Add `indexes?: any[]` to `registerObjectMetadata`, `initObjects` and the
shared `registerManagedObjectMetadata` helper, spelled as
`detectManagedDrift` spells it.
- Delete the `as any` at the `managedObjectIndexes.set` read site: the cast was
the evidence that the declaration and the read disagreed.
- Pin the fresh-object-literal form, which is the only form that can go red on
this defect; a variable-bound call measures nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 994771a9ee3809de5c1ae6f991394a3fad199ac1 && git checkout 994771a9ee3809de5c1ae6f991394a3fad199ac1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ed7243d52bbc1b6d00a3b621b0dcea4925df32b5 695026b24d0e07b4d352a884bc09d82a6f1d94c2 && git checkout -B drift-repro ed7243d52bbc1b6d00a3b621b0dcea4925df32b5 && git merge --no-ff 695026b24d0e07b4d352a884bc09d82a6f1d94c2
node scripts/docs-audit/affected-docs.mjs --json ed7243d52bbc1b6d00a3b621b0dcea4925df32b5
|
In-seat contract review — NOT PASSED. Verdict adopted verbatim by the dispatching seat.
Tier verification — the only admissible reading⛔ The reviewer's own tier line is a self-declaration, not a reading, and it said so. Per
⇒ every model stamp in the transcript is Independence pairDistinct branch vs. isolated reviewer ⇒ ⛔ not a SELF-REVIEW. The reviewer was fed only the card, the existing rulings and the PR itself — ⛔ never the dispatch order nor this seat's conclusions. ⛔ The seat's only two lawful acts on a subagent verdict are verbatim adoption or wholesale voiding. I adopt. Nothing below is rewritten, abridged or polished. The single edit is restoring Contract review — PR #16710 / issue #16570Read: card #16570 body + all 3 comments (card reports Tier reading (protocol "降档保险丝"): I run as Independent reproduction (own worktree at
① Derived judgments — every accept-set / public-surface change in the diff
Clause-② declaration. Widen / narrow / unchanged, measured (probe files compiled by the package's own
Pin form (verified in the diff, not from the PR body): all four call sites are object literals in argument position — What the pin drops: it does not cover the subclass surfaces (1e/1f — it cannot, from inside driver-sql), and its compile leg is enforced only by Skips: the 10 skipped files are ② Semver vs the changeset
③ Open questions and the seven out-of-scope findingsOQ1 (class closure,
Zero-hit greps and their positive controls: docs quoting the signature 0 / VerdictNOT PASSED
What is right and should be kept: the three-site declaration with Seat disposition⛔ NOT PASSED ⇒ the PR stays draft, both carriers stay hung, nothing is cleared and nothing lands. A patch round is dispatched to the implementer for findings 1–4. Findings 5 and 6 are escalations the seat carries, not the implementer's work:
Generated by Claude Code |
…itobjects-indexes-param
…` parameter Contract-review finding 4. Both sites read `indexes` off the same `initObjects` parameter this change types, so once the signature declares the key the casts state a disagreement that no longer exists — and the changeset's "path closed" sentence is only true with them gone. Both lines sit inside this card's declared region and no other in-flight claim holds them. The remaining casts are deliberate: `ensureShardTable` reads through its own narrow parameter type outside the region, and the `lifecycle` read is the third-key class carried by a separate card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…nd state the narrowing Contract-review findings 1, 2 and 7. Level: the governing text is the WHICH LEVEL maintainer ruling in .github/workflows/pr-automation.yml — "A purely additive widening of a published package's public surface (a new exported symbol on an index, a new accepted key or value) takes at least `minor` ... a `fix(` that widens an index is therefore `minor`" — mechanized by check-changeset-no-major.mjs, which reads a declared clause-② plus a `patch` grade as a self-contradiction inside one PR. The AGENTS.md sentence previously cited is the floor against `none`, not a ceiling, and the historical `patch` precedents are named pre-rule by that same ruling. Text: the previous "no call that compiles today stops compiling" was measured false. The accept set moves both ways — widened for fresh object literals, narrowed for variable-bound `indexes` spelled as a record, a readonly tuple or null, all three of which compiled before and now fail TS2322. Measured in both directions on this package's own tsc. No migration is owed: no caller here is affected, and every newly rejected shape was already discarded at run time by the Array.isArray guard. Scope: names @objectstack/driver-sqlite-wasm, whose SqliteWasmDriver extends SqlDriver and overrides neither method, so both signatures land in its published .d.ts. Same fixed version group, so this is a CHANGELOG effect only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…itobjects-indexes-param
…itobjects-indexes-param
Patch round received — head
|
| shape | readClause2Line says |
|---|---|
`Clause-②: yes` — because the accept set grew (backticked, trailing prose — the PR's ORIGINAL carrier) |
{kind:'declared', value:'yes'} |
Clause-②: yes (bare) |
{kind:'declared', value:'yes'} |
**Clause-②**: yes (bold) |
{kind:'declared', value:'yes'} |
- Clause-②: no (list bullet) |
{kind:'declared', value:'no'} |
### `Clause-②: yes` (heading) |
{kind:'near-miss'} |
### Clause-②: yes (heading, unbackticked) |
{kind:'near-miss'} |
| (no such line) — negative control | null ✅ fires |
⇒ Finding 3 was a wrong-mechanism defect, ⛔ not a gate-readability one. No carrier on this PR was ever unreadable. Backticks, bold, list bullets and trailing prose are all deliberately accepted (the script's own comment names #13914's control case); the only shape that fails is a heading prefix.
⭐ The implementer's reason for raising it is the right one and I am recording it in its words rather than paraphrasing: "a seat reading this thread later should not conclude that a backtick-wrapped carrier is unreadable and go 'fix' correct lines elsewhere."
5577336659, 5577338325, 5577339678) — those used ## Claim: and ### Clause-②: headings, which the table above shows are exactly the failing shape. Both things are true: my claims were genuinely illegible, and this PR's carrier never was.
What the round delivered
patch→minor, on the WHICH LEVEL maintainer ruling, which the implementer read in the repo and quotes in the PR — ⛔ not on this seat's say-so.- Changeset text restated in both directions, measured rather than relayed: one probe file, three variable-bound shapes (record,
as constreadonly tuple,null) ⇒ 3×TS2322post-fix, 0 errors pre-fix. The reviewer was right; the old universal sentence was false. No BREAKING banner, no ADR-0087 disposition, per the seat's ruling. - Carrier line rewritten to the correct mechanism (
SqlDriver's own published accept set), left as a bare line. - The two dead casts deleted — and, as asked, by symbol, not by line number:
(obj as any).indexesinsql-driver.tswent 4 → 2, both edits made by exact surrounding-context match and then verified by grep. The 2 survivors areensureShardTable's (:9441/:9468, through its own narrow parameter type) and the(obj as any).lifecycleread at:9886is likewise left — all three belong to [finding]SqlDriverreads keys off caller objects through(obj as any)at 7 sites while 3 parameter types declare none of them — a class, not a third coincidence (after #4311tenancy, #16570indexes) #16711. @objectstack/driver-sqlite-wasmnamed in the changeset.
⛔ Untouched as instructed: the fix shape, the pin, driver-turso, and no third card filed.
⭐ The discipline worth naming
A first probe attempt was reported as NOT a measurement. tsc exited 2 with zero errors in either file — which was TS2688 'Cannot find type definition file for node', because the recreated worktree had no node_modules. The implementer installed, rebuilt and re-ran, and said so. ⇒ it did not read that exit 2 as a result. An exit code without a matching error body is exactly the shape that gets mistaken for a finding, and this round refused it.
Likewise: the local check-changeset-no-major printed LEVEL AXIS: NOT MEASURED — no clause-② declaration was readable for this PR, and it was reported as NOT MEASURED, ⛔ not as a pass — with the patch→minor change made because the ruling requires it, ⛔ not because a gate demanded it. (In CI the same axis is silent for the different reason carded as #16713.)
Gate reconciliation, verbatim: Run reconciliation — 57 derived, 57 run, 0 NOT-MEASURED, 0 UNRUN. — all 57 exit 0 in one pass, because the closure was built first.
⭐⭐ A correction to THIS SEAT'S practice, adopted
the hot-file fence was defined in absolute line numbers on a tree that has since moved … a fence expressed as absolute lines is fragile against exactly this and may be worth expressing by symbol next time.
origin/main moved three times during this one round (2e6a2ea4 → 6ba0db4e → 7ceb4167 → be92d468), and PR #16619's landing shifted every anchor in sql-driver.ts by roughly +71 lines. My fences named absolute ranges. ⇒ adopted: hot-file fences are stated by SYMBOL, with line numbers as orientation only. That is the same rule this seat already imposed on every dev (「按符号重新推导」) and had not applied to its own fences.
On the second open question — already carded
The measured divergence from AGENTS.md's PR-body edit rule (it predicts the platform appends a bare footer on edit; the implementer sent a body with no footer, read it back, and none was appended) is already filed as #16633, from a round earlier today that reproduced it three times. ⇒ recommendation A adopted — leave it, attribution lives in the commit trailers and this thread; ⛔ do not re-send, which is the one thing that rule forbids. ⛔ No new card.
⛔ Why this is not cleared yet
The patch round changed the changeset level, the changeset text, the Clause-② carrier line and the code (two deletions) ⇒ it is a new contract increment and owes a re-review at CONTRACT_REVIEW_TIER. This session has reached its Fable limit (HTTP 429 on claude-fable-5-1), and contract-review.md is explicit that contract review ⛔ does not get the quota-exhaustion downgrade — 「豁免对象是派发,复核正为补偿低档派发而存在」. ⇒ the PR stays draft, both carriers stay hung, and it waits outside the queue, which the protocol names as the safe state. ⛔ It will not be landed on a lower-tier reading.
Generated by Claude Code
Contract review (
|
| prior finding | state at ade4372 |
|---|---|
1 — patch → minor |
discharged. Frontmatter: "@objectstack/driver-sql": minor, "@objectstack/driver-sqlite-wasm": minor. |
| 2 — changeset overclaim ("no call … stops compiling") | partially discharged. The universal sentence is gone and both directions are stated. A new universal sentence replaced it and is false for the readonly tuple — F1. |
3 — Clause-② carrier mechanism |
discharged. Body line now grounds on SqlDriver's own accept set. Ran the gate's own CLAUSE2_KEY_LINE + readValueToken over the body line: {"kind":"declared","value":"yes"}; heading control → near-miss; absent control → null. |
4 — two dead casts in initObjects |
discharged. (obj as any).indexes in sql-driver.ts: 2 at this head (:9441, :9468), both inside ensureShardTable, whose own parameter is { fields?: Record<string, any>; tenancy?: any } (:9427). (obj as any).lifecycle?.storage at :9886 left, as instructed. |
5 — TursoDriver override |
escalated, not owed here. On #16711 as comment 5577441703; triage ruled option B (5578361301). Still override async initObjects(objects: Array<{ name: string; fields?: Record<string, any> }>) at turso-driver.ts:1548; its super.initObjects(objects) is assignable to the widened base. |
| 6 — level-axis regex blind spot | carded as #16713, not owed here. |
7 — name driver-sqlite-wasm |
discharged (see §4). |
8 — Lint & Repo Gates pending |
discharged — success on this head. |
Verification at ade4372
1. The change. indexes?: any[] present at :9673 (registerManagedObjectMetadata), :9803 (registerObjectMetadata), :9828 (initObjects), byte-identical to detectManagedDrift's spelling at :11095. packages/spec/src/contracts/data-driver.ts: git diff origin/main ade4372 -- <path> is empty; registerObjectMetadata?(schemas: unknown[]) at :384; initObjects appears only in prose at :353.
2. Accept-set movement, measured independently. A standalone tsc 6.0.2 probe with the old and new element types side by side: fresh literal { ...bare, indexes: [] } → TS2353 against the old type, clean against the new; variable-bound indexes as a record, an as const tuple, and null → clean against the old type, 3× TS2322 against the new. Same result as the PR reports.
3. In-repo callers. Non-test call sites of .initObjects( / .registerObjectMetadata( on this head: 4. examples/app-showcase/src/system/datasources/external-fixture.ts:103 calls through a structural cast declaring initObjects: (objs: unknown[]) => … (:92-98) and passes CUSTOMER_TABLE/ORDER_TABLE, neither of which has indexes; packages/objectql/src/plugin.ts:1568 calls on driver: any; driver-turso/src/turso-driver.ts:1570 is super.initObjects(objects) with a narrower, assignable element type; scripts/check-durability-degradation-log-level.mjs:4748 is untyped. 211 test files call these methods; grep for indexes: bound to a non-array in *.ts/*.tsx returns 3 hits, none an argument to either method (memory-declared-index-unique.test.ts:325 is a helper call, registry.ts:854 is a comment, spec/.../memory.test.ts:188 is a MemoryConfigSchema parse). No in-repo caller is affected — the PR's claim holds.
BREAKING / ADR-0087, decided independently. Neither gate mechanically demands a banner: check-changeset-no-major.mjs refuses only major (:1-8) and clause-② + patch (:765); check-adr-0087-registration.mjs fires only on a changeset that declares breaking (a major bump, a **BREAKING marker, or a ! summary — :555-560). So this is a judgment, and it is the seat's to make, not this reviewer's. My reading: the banner is not owed, on grounds (i) and (iii) — zero affected callers, and any[] already published on the sibling for the same key. But ground (ii) as the changeset now states it is not true for the readonly tuple, and the prior review said so explicitly: as const is type-only, so that array is a plain array at run time, Array.isArray returns true, and the index was synced. That shape is the only one that both compiled and worked before and is rejected now — the one genuine (out-of-repo-only) regression surface. The repo has a vocabulary for exactly this: ADR-0087's CATEGORIES (:467-473) include type-surface-only. Either disposition is acceptable to this seat; a false sentence as the ground is not (F1).
4. Changeset. minor on both packages. SqliteWasmDriver extends SqlDriver (sqlite-wasm-driver.ts:67); grep for initObjects/registerObjectMetadata definitions (with and without override) across packages/** finds only sql-driver.ts and the Turso override — no sqlite-wasm override. .changeset/config.json: one fixed group of 70 packages containing both. Body states the before (Array<{ name; fields?; tenancy? }>), the after, and the narrowing with all three shapes — but see F1 on the sentence that follows.
5. Hot-file fence. git diff -U0 origin/main ade4372 -- sql-driver.ts: exactly six hunks, new-side :9673, :9682-9683, :9803, :9814-9829, :9927, :10002. Symbol anchors on this head: aggregate() :8549, readPresentationKind :12982, presentReadValue :13032, formatOutput :16802 — none touched. Current origin/main (73053ed2) has advanced past the merge-base be92d468 without touching sql-driver.ts; git merge-tree 16710↔main: clean. Overlap with the two open siblings: #16720 (8b73784c) hunks in sql-driver.ts at :23, :8501, :8636, :8733-8786; #16716 (89819679) one hunk at :9006-9062. No line overlap with this PR; merge-tree is clean for all three pairs (16710↔16720, 16710↔16716, 16720↔16716). Note only: if #16716 lands first, this PR's anchors shift by +56 lines with no conflict.
6. Pin. sql-driver-16570-init-objects-indexes-param.test.ts: the four sites at :98, :109, :124, :129 each spell indexes inside a fresh object literal in argument position (:98 puts indexes at column 56, matching the TS2353 coordinate the PR reports). bare at :127 is variable-bound, but the literal that carries indexes is fresh, which is what the excess-property check keys on. packages/drivers/driver-sql/tsconfig.json has "include": ["src/**/*"] and typecheck is tsc --noEmit, so the file is in the graded program. Reverting the two public parameter types would redden all four with TS2353 (the standalone probe reproduces the error class; prior review reproduced the exact four coordinates on the original head). The pin does not cover the narrowing axis — informational, not owed (F3).
7. Files. Exactly 3: .changeset/sql-driver-init-objects-indexes-param.md (+27), packages/drivers/driver-sql/src/sql-driver-16570-init-objects-indexes-param.test.ts (+132), packages/drivers/driver-sql/src/sql-driver.ts (+22/−7). Governed paths touched: none (docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/** — 0 of 6).
8. CI on ade4372. 36 check runs: 33 success, 3 skipped (Console Pin Gate, Build Docs, Packed-tarball smoke (opt-in)), 0 failure, 0 in progress. mergeable_state: clean. Draft, no reviews.
Findings
F1 (required) — the changeset's run-time sentence is false for the readonly tuple. .changeset/sql-driver-init-objects-indexes-param.md says: "Every shape newly rejected at compile time was already discarded at run time by the Array.isArray(obj.indexes) guard the driver has always applied — an author who wrote one of them got no index and no diagnostic." The changeset's own previous sentence names three shapes; for the as const tuple, Array.isArray is true at run time and the index was synced, so that author got the index and now gets TS2322. The prior review's ground (ii) carved this out ("except readonly arrays"); the restatement dropped the carve-out, and the PR body repeats the dropped form ("every newly rejected shape was already discarded at run time"). Expectation: restate it truthfully — record and null were discarded at run time; the readonly tuple compiled and worked, is now rejected at compile time only, has no in-repo caller, and could never have been passed to detectManagedDrift on the same class. Then the seat records which disposition it takes on that corrected ground: no banner on (i)+(iii), or **BREAKING** + <!-- adr-0087: not-required (type-surface-only …) --> together. Mirror the correction in the PR body. No code change.
F2 (informational) — fence and overlap. Six hunks measured where the PR says they are; no overlap with #16720 or #16716; all three pairwise merges clean. Nothing owed.
F3 (informational) — the narrowing axis is undocumented on the tree. No test pins that a variable-bound record or null is rejected at compile time (an // @ts-expect-error line would do it). Not owed by the card; worth a line if a patch round is opened for F1 anyway.
F4 (informational, carried elsewhere) — TursoDriver.initObjects still declares neither indexes nor tenancy, so @objectstack/driver-turso consumers see none of this widening; #16711 carries it (triage: option B). Nothing owed here.
What holds and is not repeated: the three-site declaration in the sibling's spelling, the Array.isArray read site now typed, the cast count 4 → 2 with the survivors outside the parameter this PR types, the fresh-literal pin in the graded program, minor on the maintainer's ruling, the readable carrier line, and the clean merge against current main.
Generated by Claude Code
…itobjects-indexes-param
…piled AND worked Re-review F1. The changeset said every shape newly rejected at compile time was already discarded at run time by the `Array.isArray` guard. That is false for one of the three shapes it names: `as const` is type-only, so at run time the value is a plain array, `Array.isArray` returns true, and the index WAS synced. Measured against the real driver rather than reasoned about: all three shapes passed through `registerObjectMetadata` with the argument cast to `any`, so the value reaching the driver is exactly what a pre-fix caller passed (the read path is unchanged by this PR — the cast removal is type-only). Result: the `as const` tuple is recorded in `managedObjectIndexes`; the record and `null` are deleted. So the three are now stated apart: a record and `null` never survived the guard and got no index and no diagnostic, while the `readonly` tuple compiled and worked and is now rejected on the type surface alone. No banner and no ADR-0087 disposition are owed, on the two grounds that survive the correction: zero affected callers, and `any[]` already published on `detectManagedDrift` for the same key on the same class. Also pins the narrowing axis, which nothing on the tree covered: two `@ts-expect-error` lines in the existing pin file. They are the assertion — if either stops erroring, tsc fails it as TS2578. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…itobjects-indexes-param
Patch round 2 received — head
|
| shape | Array.isArray |
managedObjectIndexes.has |
outcome |
|---|---|---|---|
as const tuple |
true | true | ⭐ the index WAS synced |
| record | false | false | entry deleted |
null |
false | false | entry deleted |
⇒ F1 confirmed; the sentence was false. The changeset and the PR body now state three distinct outcomes, and ⛔ ground (ii) is not restated anywhere in either surface — the dev's own words: "I never write 'already discarded at run time' as a general claim." The no-banner disposition stands on (i) zero affected callers and (iii) any[] already published on detectManagedDrift for the same key.
⚠️ The first TWO probe attempts were reported as NOT measurements
Attempt 1 returned vitest Test Files 1 failed, Tests no tests — a collection error (Failed to resolve entry for package @objectstack/types), i.e. stale dist after the merge. The round built the closure and re-ran rather than reading a red as a result.
⇒ ⛔ And that is my process defect, twice over. The dev's open question names it precisely:
this is the second round running where a first measurement attempt returned a non-zero exit with NO matching error body … both from the same cause — a worktree recreated after cleanup, or merged forward, without install+build before anything was measured.
Last round it was TS2688 'cannot find type definition file for node'. And there is a third instance the dev could not see: mine. When I tried to re-measure this PR's sibling claim on #16738 I hit the same TS2688 from the same cause, in a worktree I had just created.
⇒ Recommendation A adopted, and it is a change to this seat's dispatch practice, not the dev's: install + build the closure before the first measurement is now an explicit first step in any patch-round order that follows a worktree teardown or a merge-forward. The dev's reason for A is the one that decides it:
The cost of missing it is not a wrong number but a confidently reported one: a dev that reported that first vitest run as "1 failed" would have looked like a real regression in the pin.
F3 taken, and its assertion mechanism stated honestly
Two @ts-expect-error lines now pin the narrowing axis (a variable-bound record and null). ⭐ The round is precise about why that is a real assertion and not decoration: the directives ARE the assertion — an unused one fails as TS2578 — so driver-sql's typecheck passing with them present is the proof both lines still error. And the file is in the graded program (verified with --listFiles), so they are ⛔ not phantom pins. It also says plainly: "⛔ I claim no ablation for this; none was owed or run."
Verification
turbo build 72/72; driver-sql and driver-sqlite-wasm typecheck green; driver-sql Test Files 159 passed | 10 skipped (169), Tests 2422 passed | 143 skipped (2565); repo-wide eslint . --no-inline-config exit 0, not narrowed. Base-sensitive gates: check-changeset-no-major 0, check-adr-0087-registration 0 ("adds no declared-breaking changeset (1 non-breaking changeset(s) seen)" — consistent with the no-banner disposition), check-empty-changeset 0. Full reconciliation, verbatim: Run reconciliation — 57 derived, 57 run, 0 NOT-MEASURED, 0 UNRUN.
LEVEL AXIS again printed NOT MEASURED and was reported as such, ⛔ not as a clearance — the #16713 blind spot. ⭐ And when an intermediate derivation warned STALE TREE, the round stopped its own in-flight sweep, fetched, merged and re-derived — ⛔ rather than reconciling against a list nobody was on.
⭐ One carry-forward worth more than this PR
the corrected
as constreading … is the only shape in the class that compiled AND worked, and the same reasoning applies verbatim when #16711 widensensureShardTable,rotateShards/lifecycleand theTursoDriveroverride — each narrows the same variable-bound axis and should state it the same way rather than reusing the shorter, false form.
⇒ recorded here and it will be carried into #16711's dispatch. The false short form is exactly what propagated this time, through my own paraphrase; the class card is where it would propagate next.
Generated by Claude Code
Re-review F1. The changeset's run-time sentence was false for one of the three shapes it names: `as const` is type-only, so the value is a plain array at run time, `Array.isArray` returns true, and the index WAS synced. The three shapes are stated apart — a record and `null` were discarded at run time by the guard and got no index and no diagnostic; the `readonly` tuple compiled and worked and is now rejected on the type surface alone (`TS2322`), with no in-repo caller and no way to have reached `detectManagedDrift` on the same class, which already publishes `any[]` for the same key. This commit records the seat's disposition on that corrected ground, which the restatement had left unstated: no `BREAKING` banner and no ADR-0087 disposition, resting on grounds (i) zero affected callers and (iii) the sibling's already published `any[]` spelling — never on the run-time ground, which is false for the `readonly` tuple. No code change. Gates at this head, verbatim: node scripts/check-changeset-no-major.mjs --base origin/main EXIT=0 node scripts/check-adr-0087-registration.mjs --base origin/main EXIT=0 node scripts/check-empty-changeset.mjs --base origin/main EXIT=0 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH
Patch round — F1 discharged (director seat)Head is now What F1 asked for, and where it already stoodThe re-review ( The truthful statement, as it now reads in the changeset (unchanged this round, quoted so the correction is in one place):
The new sentence — the seat's disposition, recorded as the ground:
⛔ Ground (ii) is named only to disclaim it. The disposition does not rest on it and never did — the prior review's own wording carved The Mirrored in the PR bodyThe "The accept set moves in BOTH directions" section already carried the three-shape table and the corrected prose; its closing disposition line now names the same two grounds and the same exclusion, so the body and the changeset cannot drift apart on this point:
Body re-fetched after the edit and diffed byte-for-byte against the intended text: identical. Gates at
|
Contract review (
|
| finding | state at 695026b |
|---|---|
F1 (required) — changeset run-time sentence false for the readonly tuple; seat to record its disposition on the corrected ground |
discharged. The sentence "Every shape newly rejected at compile time was already discarded at run time…" is gone (removed in 09dec14e6, the 66fcc5e40 round). The three shapes are stated apart: record and null never survived Array.isArray; the as const tuple "compiled and worked, and is now rejected at compile time … the rejection is entirely on the type surface". 695026b adds the disposition, quoted verbatim in §2 below. PR body mirrors it (§4). |
| F2 (informational) — fence / sibling overlap | unchanged; sql-driver.ts diff vs merge-base still +22/−7 in the same neighbourhood; not re-argued. |
| F3 (informational) — narrowing axis unpinned | taken up in 09dec14e6: pinsTheNarrowingAxis() at sql-driver-16570-init-objects-indexes-param.test.ts:141-149, two // @ts-expect-error TS2322 lines (record, null). tsconfig.json is "strict": true with "include": ["src/**/*"], so the null directive is live under strictNullChecks and the file is in the graded program. Not owed; noted as present. |
F4 (informational, carried by #16711) — TursoDriver.initObjects |
unchanged, not owed here. |
Verification
1. Diff since the re-review head. ade4372 is an ancestor of 695026b. The PR-side delta between them, restricted to the PR's file set, is two commits, not one: 09dec14e6 (changeset +7/−1, pin file +17) and 695026b24 (changeset +2, nothing else). git show --stat 695026b24 → 1 file changed, 2 insertions(+), .changeset/sql-driver-init-objects-indexes-param.md only. So "changeset only, +2 lines, no code" is true of 695026b relative to 66fcc5e40; relative to ade4372 the pin file also grew by the 17 F3 lines. Everything else in git diff ade4372..695026b is origin/main merged in (164d0dd39, 66fcc5e40). Whole PR vs merge-base 8b37a0973: still exactly 3 files (+205/−7).
2. The new sentence, verbatim, and its truth. Line 30 of the changeset:
The disposition on that corrected ground, recorded here because the ground itself moved: no
BREAKINGbanner and no ADR-0087 disposition, resting on grounds (i) and (iii) alone — zero affected callers, and theany[]spelling already published ondetectManagedDriftfor the same key on the same class. The ground that every newly rejected shape had already been discarded at run time is not among them: it is false for thereadonlytuple, and nothing here leans on it.
Each clause checked: (i) caller census re-run on the head tree (two origin/main merges landed after ade4372, so it was not carried over): non-test call sites of .initObjects( / .registerObjectMetadata( are the same four external ones as before (objectql/src/plugin.ts:1568 on driver: any; turso-driver.ts:1570 super.initObjects(objects) with a narrower, assignable element type; examples/app-showcase/.../external-fixture.ts:103 through a structural unknown[] cast with no indexes; an untyped .mjs) plus three self-calls inside SqlDriver (:9178, :9839, :10820) that compile under the class's own types; indexes: bound to a record/null/as const in *.ts/*.tsx returns the same three unrelated hits as before plus the two new pin lines. 211 test files call the methods; Type Check · source gates is green on the head. Zero affected callers holds; (iii) detectManagedDrift declares indexes?: any[] on the same class (unchanged in this PR); the disclaimed ground is indeed false for as const — reproduced with tsc 6.0.3 on a standalone probe: against the old element type the fresh literal fails TS2353 and the three bound shapes pass; against the new type the fresh literal passes and all three fail TS2322, the tuple specifically with "The type 'readonly [...]' is 'readonly' and cannot be assigned to the mutable type 'any[]'". as const being type-only, Array.isArray is true at run time, so that caller was synced before — exactly what the changeset now says.
3. Gates, measured with the head's own scripts (byte-identical to 695026b:scripts/*, verified by blob hash; --base origin/main --head refs/review/16710b, merge-base resolved to 8b37a0973):
check-changeset-no-major.mjs EXIT=0 ✓ no `major`; LEVEL AXIS: NOT MEASURED (no payload)
… with --event {labels: PR's 5 labels, body: PR body}:
EXIT=0 ✓ LEVEL AXIS: declares clause-② `yes`, no grown package graded `patch`
check-adr-0087-registration.mjs EXIT=0 ✓ adds no declared-breaking changeset (1 non-breaking seen)
check-empty-changeset.mjs EXIT=0 ✓ 1 declaring changeset added
Real-function probes, not grep: breakingDeclaration(parseChangeset(head changeset)) → {"breaking":false,"signals":[]}; bumps → driver-sql: minor, driver-sqlite-wasm: minor; readClause2Line(PR body) → {"kind":"declared","value":"yes"}. The backticked `BREAKING` on line 30 is preceded by **no , so /\*\*BREAKING/i cannot match it; /^\s*BREAKING[ -]CHANGE/mi and the !: summary form are absent; no <!-- adr-0087: … --> marker is present (none is owed on a non-breaking changeset).
In CI, all three run inside the Check Changeset job (pr-automation.yml:776-784, :823-824, :918-959): success on this head (101945962178, completed 05:17:25Z). Note the level axis is genuinely blind to packages/drivers/*/src (PUBLISHED_SOURCE_PATH, #16713) — my --event run passed for that reason, not because it judged this package; the minor grade rests on the maintainer's WHICH LEVEL ruling, as the PR says.
4. Body/changeset drift. PR body, section "The accept set moves in BOTH directions": "Per the seat's disposition on that corrected ground: no BREAKING banner and no ADR-0087 disposition, resting on grounds (i) zero affected callers and (iii) the sibling's already-published any[] spelling — never on the run-time ground, which is false for the readonly tuple. The changeset records the same disposition, on the same two grounds and with the same exclusion." Same disposition, same two grounds, same exclusion — no drift. The body also carries the three-row run-time table and the explicit retraction of the earlier claim.
5. The disposition itself — my view. Neither gate infers breaking-ness: check-changeset-no-major.mjs refuses only a major bump and a clause-②-plus-patch self-contradiction, and check-adr-0087-registration.mjs fires only on an author-declared signal (major, **BREAKING, BREAKING CHANGE, or a !: summary). So "no banner" is a judgment the author is entitled to make, and the gates accept it as written. I concur with it. The one shape with anything to lose — a variable-bound as const tuple — has zero in-repo callers, was already unusable against the sibling detectManagedDrift on the same class (so no consumer could have been treating readonly as the class's accept set), and loses nothing at run time; the fix at the call site is dropping two words. During the launch window the ruling is that breaking-ness is carried by the banner plus the ADR-0087 disposition rather than the level, and a type-surface-only not-required disposition would also have been acceptable to this seat — but a disposition is only owed once a banner is declared, and declaring one here would overstate a type-only narrowing with no reachable caller. What mattered was that the ground be true and stated, and it now is.
6. CI on 695026b. 34 check runs on 695026b2 at the time of writing: 0 failure, 7 in_progress (Test Core 1/3/4/5/6, Type Check · workspace, Lint & Repo Gates), Check Changeset success, Governed Surface Queue Guard success; the 27 completed runs are 24 success + 3 skipped (Build Docs, Console Pin Gate, Packed-tarball smoke (opt-in)). Nothing has gone red. Type Check · source gates, Type Check · consumer gates, Type Check · debt ledger, Build Core, Dogfood Regression Gate (1/3, 2/3, 3/3 and the rollup), Dogfood Verify CLI, Temporal Conformance, Test Core (2/6) are already green. This verdict is conditional on the 7 in-progress runs finishing green; Type Check · workspace is the one that grades the two @ts-expect-error directives, so it is the one to read before landing.
Residual
None required. Informational only: the @ts-expect-error pair covers record and null but not the readonly tuple — the one shape the changeset singles out; a third directive would pin the exact case F1 turned on. Not owed by the card and not blocking.
Generated by Claude Code
Fixes #16570
Clause-②: yes — this adds a new accepted key to the published accept set of
SqlDriver.initObjects()andSqlDriver.registerObjectMetadata(), both public methods of a class a released package exports. The ground isSqlDriver's own published accept set, not theIDataDrivercontract:packages/spec/src/contracts/data-driver.tsdeclaresregisterObjectMetadata?(schemas: unknown[])and this diff does not change it, andinitObjectsis not a contract member at all (prose only).needs:contract-reviewis applied. Draft only: the PM lands it after the contract review returns. ⛔ Not ready, not auto-merge, not queued.Measurements below were taken at
66fcc5e40f, merged up toorigin/main8b37a0973d.The defect
packages/drivers/driver-sql/src/sql-driver.ts. Both entry points that take a caller's object list declared a structural parameter type with noindexesin it, while the driver readindexesout of those very objects one call deep, through an(obj as any)cast, and filledmanagedObjectIndexesfrom it — the mapsyncDeclaredIndexesrenders every declared UNIQUE from. The siblingdetectManagedDrift, on the same class, had always declaredindexes?: any[]: the two halves of one class disagreed about the shape of the same input.Nothing had tripped over it because TypeScript's excess-property check fires on a fresh object literal and not on one bound to a variable first, so the same object was accepted or rejected by nothing but where it was spelled. Every caller in the package happened to bind first — a green that held for a reason unrelated to correctness.
The loud symptom is a compile error on a correct call. The quiet one is the reachable branch: an author, or an AI, reading the signature concludes
indexesis not accepted and drops the key. The declared UNIQUE is then never synced, with no error at authoring time and none at boot.What changed
indexes?: any[]added toregisterObjectMetadata, toinitObjects, and to the sharedregisterManagedObjectMetadatahelper — spelled exactly asdetectManagedDriftspells it. The helper is included because the read happens on its parameter: without it the cast could not be deleted at all. (The review's leg 3 confirms it: reverting the helper declaration alone yieldsTS2339at the two read lines.)as anyreadingindexesoff those parameters is gone — the one at themanagedObjectIndexes.setsite and the two insideinitObjects' own create/alter path. Occurrences of(obj as any).indexesin the file: 4 before, 2 after. The read site now reads:#4311comment aboveinitObjectsgained a paragraph recording the same reasoning forindexesand naming the pin.src/sql-driver-16570-init-objects-indexes-param.test.ts— the pin, now covering both axes (below).The 2 surviving
(obj as any).indexesare deliberate and out of scope: both sit inensureShardTable, which reads through its own narrow parameter type, and belong to the class card #16711. The(obj as any).lifecycleread is likewise left to that card.Symbols, re-derived at the final head
sql-driver.tsmoved under this branch while the rounds ran (PR #16619's read-presentation work landed onmain), so every anchor below was re-derived by symbol on this checkout rather than carried over. The branch merges clean andmainnever touched the lines this PR edits.66fcc5e40fregisterManagedObjectMetadata(obj):9601-9603:9672-9674managedObjectIndexes.setread site:9611-9612:9682-9683registerObjectMetadata(objects):9731-9733:9802-9804initObjects(objects):9743:9827-9829detectManagedDrift(objects?)— the spelling copied:11008-11009:11094-11095ensureShardTable— out of scope, still cast:9356:9427, reads at:9441/:9468The pin covers both axes
Widening axis — every call is a fresh inline literal in argument position. A variable-bound pin cannot go red on this defect; that spelling is exactly what made the package green before, so it measures nothing.
Narrowing axis (new this round, review F3) — two
@ts-expect-errorlines pin that a variable-bound record andnullare rejected. The directives are the assertion: if either line ever stops erroring,tscfails it asTS2578, so the green typecheck below is load-bearing rather than incidental.Proof the pin is inside the program that grades it:
tsc --noEmit --listFileslists the pin file (1 hit) among the package's.test.tsfiles; a nonsense control string matched 0. It is not a phantom check.Two-leg ablation (from the first round; no ablation is claimed for the text rounds)
Leg 1 — with the fix,
tsc --noEmitgreen. Leg 2 — the two public parameter types reverted, everything else untouched:tscexit 2, verbatim:The mutation was proven on disk before measuring and the restore proven by blob hash, with an
EXIT INT TERMtrap restoring throughgit checkout HEAD -- <path>. Nothing is left behind.The accept set moves in BOTH directions — and the three narrowed shapes are NOT alike
Compile-time movement, measured on this package's own
tsc: a fresh literal{ ...bare, indexes: [...] }was rejected and is now accepted (widened); variable-boundindexesas a record, as areadonlytuple (as const), or asnullcompiled before and now failsTS2322(narrowed).registerObjectMetadatawith the argument cast toany, so the value reaching the driver is exactly what a pre-fix caller passed, and the read path is unchanged by this diff:Array.isArrayat run timefalsenullfalsereadonlytuple (as const)trueas constis type-only, so at run time it is a plain array. That caller compiled and worked, and is the one shape with anything to lose — its run-time behaviour is unchanged, the rejection is entirely on the type surface.No migration is owed even so, on the two grounds that survive the correction: no caller in this repository is affected, and the shape could never have reached
detectManagedDrifton the same class either, which publishes the very sameany[]spelling for the very same key — so areadonlycaller was already unable to use half of this driver's declared-index surface. A caller in that position spells the array withoutas const, or widens it at the call site. Per the seat's disposition on that corrected ground: noBREAKINGbanner and no ADR-0087 disposition, resting on grounds (i) zero affected callers and (iii) the sibling's already-publishedany[]spelling — never on the run-time ground, which is false for thereadonlytuple. The changeset records the same disposition, on the same two grounds and with the same exclusion.Changeset:
minor, notpatchThe governing text is the WHICH LEVEL maintainer ruling in
.github/workflows/pr-automation.yml(2026-09-04, decision batch #35), verified in the repo:scripts/check-changeset-no-major.mjsmechanizes it: a PR that declares clause ② may not grade a package it grewpatch— "a self-contradiction inside one PR". TheAGENTS.mdsentence this PR previously cited is the floor againstnone/skip-changeset, not a ceiling, and the same ruling names the 64 historicalpatchprecedents as pre-rule.PUBLISHED_SOURCE_PATH = /^packages\/([^/]+)\/src\//never matchespackages/drivers/driver-sql/src/…, so a green there is a blind spot (carded as #16713). Run locally at this head the axis reportsNOT MEASURED — no clause-② declaration was readable, which is neither a pass nor a failure.@objectstack/driver-sqlite-wasmis named alongside:SqliteWasmDriver extends SqlDriverand overrides neither method, so both widened signatures land in its published.d.ts. Both packages are in the samefixedversion group in.changeset/config.json, so this is a CHANGELOG effect, not a version one.Verification at
66fcc5e40fpnpm --filter @objectstack/driver-sql typecheck(tsc --noEmit, the only leg)@ts-expect-errordirectivespnpm --filter @objectstack/driver-sqlite-wasm typecheck(the inheriting package)tsc --noEmit, green in the same chainpnpm --filter @objectstack/driver-sql testTest Files 159 passed | 10 skipped (169)·Tests 2422 passed | 143 skipped (2565)turbo run buildover all packagesTasks: 72 successful, 72 totalpnpm lint(eslint . --no-inline-config, whole repo, not narrowed)Base-sensitive gates at this head, exit codes captured after the redirect:
Full gate reconciliation, re-derived on this tree:
All 57 exit 0.
Acceptance notes
lifecycleis reported, not changed here; it and theensureShardTablereads are carried by the class card [finding]SqlDriverreads keys off caller objects through(obj as any)at 7 sites while 3 parameter types declare none of them — a class, not a third coincidence (after #4311tenancy, #16570indexes) #16711, where triage ruled option B.TursoDriver.initObjectsdeclares neither key and is carried there too.src/sql-driver-11794-richtext-text-family.test.tssays "Hoisted (not an inline literal) …indexesrides throughinitObjectsbeyond its narrow parameter type".Fence
Held. All six hunks in
sql-driver.tsland at:9673,:9682-9683,:9803,:9814,:9927and:10002— theregisterObjectMetadata/initObjects/registerManagedObjectMetadataneighbourhood in every case. Nothing inreadPresentationKind,presentReadValueorformatOutput, and nothing nearaggregate(). The diff is still exactly 3 files.