Uh oh!
There was an error while loading. Please reload this page.
fix(drivers): update() on a missing id answers null on MongoDB and Turso's remote face - #14914
Conversation
…rso remote `IDataDriver.update()` declares `Promise<Record<string, unknown> | null>`, and four of six shipped implementations return `null` for an id that names no row. `MongoDBDriver.update()` and `RemoteTransport.update()` fabricated a record instead — the caller's own payload with the id stapled on (and, on Mongo, the `updated_at` the driver had just stamped). Through the engine's by-id door that surfaced as a 200 with a record that does not exist. Both now return `null`. `TursoDriver.update()`'s remote branch needed no edit: `formatRemoteRow` already guards `row && typeof row === 'object'`, so the two faces of that driver converge. `RemoteTransport.bulkUpdate()`'s `if (updated) results.push(updated)` skip stops being dead code. `upsert()` is untouched on both: an upsert never answers "not found". Regression pins added per driver (net-new — no landed test pinned the fabricating posture), each with a positive control so "return null always" cannot pass, plus a local/remote parity pin on TursoDriver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…iver-update-missing-id
📓 Docs Drift CheckThis PR changes 2 package(s): 1 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 — 9 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 02ce5b7fc21fb43e6940bab6290bd579d86bf454 && git checkout 02ce5b7fc21fb43e6940bab6290bd579d86bf454
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b3a63d32c9a0ec291a8be1f8f43cfc7c48c802bf f528ddb17a7a65bf0cd73561ddd43ce800004642 && git checkout -B drift-repro b3a63d32c9a0ec291a8be1f8f43cfc7c48c802bf && git merge --no-ff f528ddb17a7a65bf0cd73561ddd43ce800004642
node scripts/docs-audit/affected-docs.mjs --json b3a63d32c9a0ec291a8be1f8f43cfc7c48c802bf
|
…ongodb-driver.test.ts The widened `update(): Promise<Record<string, unknown> | null>` declaration made `expect(result.title|.status|.id)` three TS18047 errors. The package tsconfig excludes `**/*.test.ts`, so `pnpm typecheck` could not see them; the type-check debt ratchet re-measures with the tests un-hidden and caught the +3 (10 -> 13). Narrowed at the three sites with the file's own `findOne` idiom -- assert the found arm, then read through it. Re-measured with the ratchet's own project shape: back to exactly 10 (TS1309 x7 + TS2550 x3), the ledger's frozen entry. The ledger is NOT raised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…ers to what was measured Contract-review round on PR #14914. Changeset: `patch`/`patch` understated a published TYPE-surface narrowing plus a runtime behaviour change on two published drivers. Now `minor`/`minor` with a `**BREAKING**` sentence naming what breaks for TypeScript consumers, and exactly one ADR-0087 disposition -- `not-required (no-migration-prescription)`, the same shape and reasoning as `.changeset/driver-memory-update-upsert-honest-types.md` (PR #14434) one day earlier in this series. `type-surface-only` is not claimable: its predicate 4 (narrowed-from-erased) is false, and runtime behaviour moves too. `mongodb-update-missing-id.test.ts`: the header claimed a type-level pin here would be "never checked by anything". False -- `pnpm check:type-check-debt` re-measures this package with its tests un-hidden, which is exactly how CI caught the three TS18047 the widened declaration introduced. The section now states both programs, names the tsconfig exclusion as the filed defect (#14917), and gives the real reason the pin lives in the turso twin instead. Its reverse-verification paragraph now reports the OBSERVED leg (2 failed | 2 passed (4), all four ran) rather than predicting a compile-time red for a type pin this file does not have. `turso-update-missing-id.test.ts`: same correction. Restoring the fabricating EXPRESSION leaves the declaration untouched, so the `Equals` const cannot red and nothing fails at compile time; the parity pin is one assertion, not two halves; and the no-fabrication pin, omitted before, does red. The paragraph now names all five reds and all five greens from the run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…iver-update-missing-id
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14428
Ruling implemented: posture A — maintainer, 2026-09-03 (「同意」, decision batch #15 item 1, recorded on the card by the director seat). Both drivers return
nullon a miss;B(throw) was not taken, because it would have been a third posture on top of the two this card exists to collapse.Generics are written in square brackets throughout —
Promise[Record[string, unknown] | null]— because the body sanitizer eats the angle-bracket spelling.What was wrong
IDataDriver.update()declaresPromise[Record[string, unknown] | null]. Four of six shipped implementations returnnullfor an id that names no row. Two invented a record instead:packages/drivers/driver-mongodb/src/mongodb-driver.ts:422(at the merge base)return (updated as Record[string, unknown]) || withoutUndefinedOwnKeys({ id: String(id), ...updateData });packages/drivers/driver-turso/src/remote-transport.ts:1549(at the merge base)return rows[0] || { id, ...data };Both are the same shape:
UPDATE/updateOnematches nothing, the read-back comes back empty, and the caller is handed a row assembled from its own payload — on Mongo with theupdated_atthe driver had just stamped.This is the expensive direction of wrong, not merely the wrong answer. It says succeeded where the truth is not found, and says it in a shape carrying the caller's own fields back, so nothing about it looks wrong. Through the engine's by-id door a REST / SDK / MCP
updateagainst a deleted or mistyped id answered 200 with a record that does not exist — on these two implementations only.The change
Two one-line returns, plus the declared type on each:
MongoDBDriver.update()(mongodb-driver.ts:430declaration,:449return) →return (updated as Record[string, unknown] | null) ?? null;, declaredPromise[Record[string, unknown] | null].RemoteTransport.update()(remote-transport.ts:1564declaration,:1580return) →return rows[0] ?? null;, declaredPromise[Record[string, unknown] | null].Two seams needed no edit, and the PR pins both so that stays true:
TursoDriver.update()'s remote branch wraps the transport result informatRemoteRow, which already guardsrow && typeof row === 'object'— sonullpasses through untouched and the driver's two faces converge. That guard is load-bearing now in a way it was not before, hence a pin.TursoDriver.update()itself is untouched by this PR and still declaresPromise[any].RemoteTransport.bulkUpdate()'sif (updated) results.push(updated)(remote-transport.ts:1676on this branch) — the cross-driver skip conventionSqlDriver.bulkUpdatefollows — stops being dead code. On this transportupdatedcould never be falsy, so a batch over N missing ids answered N invented rows.upsert()is untouched on both drivers: an upsert never answers "not found".packages/specis untouched — the contract side was already ruled and landed by #13878 / PR #14434.Semver:
minor, and BREAKING for TypeScript consumersThe changeset was
patch/patchon the first head. That understated the change, and it is nowminor/minorwith a**BREAKING**sentence. Two things moved on published surfaces, not one:MongoDBDriver.update()andRemoteTransport.update()are both exported from their package index (driver-turso/src/index.ts:38exportsRemoteTransport). Their declared return moves fromPromise[Record[string, unknown]]toPromise[Record[string, unknown] | null], so a consumer that reads a field off either result —result.id,result.title— stops compiling until it narrows. That is the shape ADR-0087's 2026-08-30 addendum names, and it says the token is owed: a published type-surface narrowing "declares**BREAKING**truthfully … ⛔ Dropping the token is no longer an available exit"..d.ts-only release.Disposition: exactly one marker,
not-required (no-migration-prescription). No metadata key is removed, renamed or re-shaped, so there is nothing forobjectstack migrate meta,spec-changes.jsonor the upgrade guide to project, and the changeset prescribes no rewrite; the consumer obligation is a compiler-delivered narrowing at the call site.type-surface-onlyis not claimable, on two independent grounds: its predicate 4 (narrowed-from-erased) is false — neither declared return wasanyat the merge base — and runtime behaviour moves in the same diff. The precedent is one day earlier in this same series:.changeset/driver-memory-update-upsert-honest-types.md(PR #14434,93940d492) carriesminor,**BREAKING**and that same disposition.pnpm check:adr-0087-registrationreads it back:1 declared-breaking changeset(s), each carrying an ADR-0087 disposition. … [BREAKING] not-required (no-migration-prescription).Zone 2 measurements (re-measured on this branch, not inherited)
A — both sites fabricate today. Confirmed, file:line and expressions quoted above.
B — convergence, not invention. Measured across every
async update(objectimplementation in the repo:InMemoryDriver(memory-driver.ts:677)return null(with astrictModethrow branch)SqlDriver(sql-driver.ts:6864)return this.formatOutput(object, updated) || nullSqliteWasmDriverSqlDriver— overrides noupdateTursoDriverlocal branch (turso-driver.ts:779)super.update→SqlDriver→nullMongoDBDriverRemoteTransportNo third posture exists in-repo.
strictModeis adriver-memoryconfig only —grepfinds it in neitherdriver-mongodb,driver-tursonordriver-sql— so no strict-throw arm was owed here.C — no in-repo caller depends on the fabricated record. Not a stop condition.
Three readers of the changed returns, each checked:
packages/objectql/src/engine.ts:11332— the by-id driver exit. Downstream,resultreacheshydrateWriteFormulas(filtersr != nullatengine.ts:1411),coerceBooleanFields(returns the row unchanged when!row,record-validator.ts:455) and the id read atengine.ts:11613, guardedtypeof result === 'object' && result && 'id' in result. All three already receivenullfrom four of six drivers today, so this PR adds no new exposure to that path.packages/drivers/driver-turso/src/turso-driver.ts:778— pass-through via the null-safeformatRemoteRow.packages/drivers/driver-turso/src/remote-transport.ts:1676—bulkUpdate's skip, which is written fornulland was unreachable.Outside the two packages, the argument rests on four facts, each re-measured on this head.⚠️ The first head's version of this paragraph asserted that every import was dynamic and that two carried
as any. Both were false; the corrected enumeration is below, and the conclusion survives on the legs that are true.package.json'sdependencies/devDependencies/peerDependencies/optionalDependencies:@objectstack/cli,@objectstack/dogfood,@objectstack/runtime,@objectstack/service-datasource.git grep "export .*from '@objectstack/driver-(mongodb|turso)'"acrosspackages/**(excluding CHANGELOGs) returns nothing, so no fifth package can hold these types transitively through an export.update()result. Four are dynamicimport(...):runtime/src/turso-driver-factory.ts:210,service-datasource/src/default-datasource-driver-factory.ts:1219and:1314(these three carry theas anycast) andcli/src/utils/storage-driver.ts:522(which does not). The remaining two are static:cli/src/utils/storage-driver.test.ts:7isimport type { TursoDriverConfig }, a type-only import of a config type;packages/qa/dogfood/test/date-bucket-parity-turso.test.ts:67is a static value import ofTursoDriver, and that file contains no.update(call at all (greped).TursoDriver.update()'s declared type did not move. It isPromise[any]atturso-driver.ts:777, andgit diff origin/main...HEAD -- turso-driver.tsis empty. So the one statically-imported driver class in the repo asks nothing new of its one caller.Consumers otherwise reach these drivers through
IDataDriver, which has declared the| nullarm since #13878.Coverage — net-new, with controls
No landed test pinned the miss posture on either driver (the existing
update()cases all read rows that EXIST), so nothing here changes a baseline.packages/drivers/driver-mongodb/src/mongodb-update-missing-id.test.ts— 4 cases: the miss, a no-fabrication pin naming the shape it forbids, a write-still-issued pin (a "fix" that read first would answernullcorrectly and silently stop writing), and a positive control.packages/drivers/driver-turso/src/turso-update-missing-id.test.ts— 10 cases, all ten named: (1) the declared-return-type pin, (2)seeded the fixture (the premise), (3) the transport miss, (4) the no-fabrication pin, (5) the write-still-issued pin, (6) the positive control, (7) thebulkUpdatemixed-batch pin (2 present + 2 missing ids ⇒ 2 rows, and the writes that could land did), (8) the local/remote parity pin, (9) its parity positive control, and (10) theformatRemoteRowpass-through pin.Why the type pin lives only in the turso file
driver-mongodb'stsconfig.jsonexcludes**/*.test.ts, so the package's owntypecheckscript cannot see a type pin written in a test file there:tsc --noEmit --listFilesin that package lists 0 files ending.test.ts, against 43 indriver-turso(whose tsconfig excludes onlynode_modules/dist). vitest transpiles without typechecking, and the roottsconfig.jsonexcludespackagesentirely.pnpm check:type-check-debtre-measures that package with its tests un-hidden and compares against the frozenTEST_DEBTentry — so a brokenEqualsconst there would surface, as a ledger count moving rather than as a named assertion failure. Both statements are corrected in this round, in the body and inmongodb-update-missing-id.test.ts. The exclusion itself is filed as #14917. The mongo declaration is pinned instead bymongodb-driver.tsbeing in the package's own program (leg 3 below).Verification
All commands run on the final head
f528ddb17(git rev-parse --short HEAD), a merge oforigin/maintaken after the gate deriver reported STALE TREE.The debt-ledger red, reproduced and then cleared.
Type Check · debt ledgerfailed on the previous head64a11c8b2:@objectstack/driver-mongodb: TEST_DEBT records 10 raw tsc error(s), tsc --noEmit now reports 13 (+3). The three wereTS18047 'result' is possibly 'null'atmongodb-driver.test.ts:158,159,160— found-arm reads ofdriver.update()that the widened declaration no longer permits, invisible topnpm typecheckbecause that package hides its tests. Reproduced locally by rebuilding the ratchet's own generated project (remeasureProject: extend the package tsconfig, drop only the test glob fromexclude, restore the defaulttypeRoots) and runningtsc --noEmitover it with the dependency closure built:64a11c8b2(before)TS1309x7,TS2550x3,TS18047x3f528ddb17(final)TS1309x7,TS2550x310 is exactly the ledger's frozen entry (
TS1309 x7, TS2550 x3). ⛔ The ledger was not raised — the fix is at the three call sites, narrowed with this file's ownfindOneidiom (assert the found arm, then read through it).Tests —
pnpm --filter @objectstack/driver-mongodb --filter @objectstack/driver-turso run test:(The 147 skips are the pre-existing opt-in
mongodb-memory-serversuites, #5517.)Typecheck — both packages:
tsc --noEmit,Done, exit 0.Lint — a declared narrowing, not a skip.
pnpm lintscans the whole repo and is CI's run. Locally,pnpm exec eslint --no-inline-config --format jsonwas run over the diff, with the three readings a narrowing owes: (1) the population is read from eslint's own config, not guessed — asked about.changeset/driver-update-missing-id-null.md, eslint answersFile ignored because no matching configuration was supplied, so the changeset is outside the linted set by the config's own account and the five.tspaths are the whole linted portion of this diff; (2) the count comes from the JSON output — 5 files linted, 0 errors, 0 warnings; (3) the config enables no type-aware linting for any file (noparserOptions.project, no typed@typescript-eslintrules — stated and measured ineslint.config.mjs:327-335), so nothing in these five files can move the verdict on a file this PR does not touch.Gates — family derived on the final head with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(no path arguments, so the change set comes from git: 6 paths vs merge base7317cf266). 41 commands, all run, exit codes captured before any pipe. 37 exit 0. The other 4 exit 3, each script's own NOT-MEASURED code rather than a finding — quoting their verdict lines:check-test-completeness.mjs— "Nothing was measured … ⛔ It is NOT a finding".check-half-states.mjs— "Treat this exit as an unread instrument, never as a quiet board" (needs repo-scoped egress this container lacks).check:dual-build-cjs-loads— "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. … ⛔ This is NOT a pass"; its self-test passed (93 cases).check:type-check-debt— "--re-measure cannot run: 18 workspace dependenc(ies) … have no built type entry point on disk … ⛔ This is NOT a pass and NOT a finding".node scripts/pm/check-governed-merges.mjs --teston the final 6-file list:0 of 6 path(s) hit the register — NOT governed.Reverse verification — four legs, each re-measured on the final head. Every mutation is proved on disk by counting the injected and the deleted text (never by an editor's exit code — one leg's first attempt was refused by that check for a 2-occurrence anchor, and was re-run with a unique one). Every restore is proved by a
git hash-objectmatch against the HEAD blob and an emptygit diff HEAD; each script carries atrap … EXIT INT TERMwith absolute paths.Tests 2 failed | 2 passed (4)— exactly the miss pin and the no-fabrication pinbulkUpdate, parity, pass-through redTests 5 failed | 5 passed (10)— exactly those five, by nametscred in the driver, proving the declaration is pinned without a test-file type pinsrc/mongodb-driver.ts(449,5): error TS2322: Type 'Record[string, unknown] | null' is not assignable to type 'Record[string, unknown]'tscred at the type pinsrc/turso-update-missing-id.test.ts(113,7): error TS2322: Type 'true' is not assignable to type 'false'Equalsconst can red and no file fails to typecheck: all four mongo cases and all ten turso cases ran. The turso parity pin is likewise one assertion over both faces, not two halves that can red independently. Both test-file headers said otherwise and are corrected in this round. Legs 1 and 2 need no rebuild — both test files import the code under test by relative specifier, so vitest resolves them to this package'ssrc, not to a builtdist. Legs 3 and 4 aretscruns against the packages' own programs, with the dependency closures rebuilt on this head first.Labels
needs:contract-reviewstays applied — it is cleared only at reviewer PASS, and only by the review. The card-side carrier of that dual-carrier label is the PM seat's to set.🤖 Generated with Claude Code
https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68