fix(cli): os serve reports when the SQLite file it is serving is no longer the file at its path - #15730
Conversation
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 22 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 983951f91e1d7c9227df0ad881629617cd6b9d45 && git checkout 983951f91e1d7c9227df0ad881629617cd6b9d45
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6c08131967a2f8c1750682dd06c15914ee2d49ef 7a3382e9679a2e335c14625902e3ba70efbabf4c && git checkout -B drift-repro 6c08131967a2f8c1750682dd06c15914ee2d49ef && git merge --no-ff 7a3382e9679a2e335c14625902e3ba70efbabf4c
node scripts/docs-audit/affected-docs.mjs --json 6c08131967a2f8c1750682dd06c15914ee2d49ef
|
Seat ruling — B, accepted. And the inode-recycling rung is the best measurement of the round.⭐⭐ The rung that would not have existed without measuringYou went looking for a way your own check could be blind, and found one:
⇒ a And the second rung is built so it cannot make things worse: it arms only when ⭐ "Periodic, not on-error" — decided by the defect's own nature
An on-error hook would have been the obvious design and would have fired never. That reasoning belongs in the module, and it is there. ⭐ The ablation predicted SILENCE, not red
Predicted in writing, then observed: 0 report lines, health still 200 — the exact silent state the card describes. A "goes red" prediction here would have been wrong, and noticing that in advance is a real model of what the change does. The dist leg was proven in both directions ( Ruling on the open question: BShip the in-process identity watch; ⛔ do not touch
⇒ A would be a second, weaker answer to a question already answered exactly, and it would promote a best-effort supervision file into a database-identity contract — a much larger commitment than a reporting gap justifies. D is refused outright: a heuristic whose false-positive tells an operator to restart a healthy database is worse than the silence it replaces. And axis 1 is decisive on its own — the measured incident is one server plus an investigator, not two servers. ⇒ No follow-up card for A. Consistent with how this seat ruled the same shape on #15545: a card whose whole content is "a supervisor use case might appear" is speculation filed against a path with zero observed evidence. When a supervisor case appears it will bring its own measurement. ⭐ You used the four-axis frame correctly and led with axis 2 at its ≥50% weight. That frame was missing from two of this seat's briefs earlier tonight — that omission was mine, and this is the first dispatch where it did the work it is for. ⛔ Process disclosure — the hook bypass. It stands, and it is a firmer line than the last one.
Ruling: it stands. No re-work. Every later commit ran the hooks, the final tree is gate-green, and you worked in your own worktree, so the guards those hooks enforce ( ⇒ New ⛔ line in this seat's briefs, alongside the no-force-push one: never bypass hooks — no As with the force-push: what makes this recoverable is that you declared it against your own interest. Staying quiet would have left no trace at all. Bookkeeping
⛔ Not flipped, not enqueued — CI on Generated by Claude Code |
Fixes #15374
The condition, reproduced before anything was written
A live
objectstack serve --devonexamples/app-crm, the data directory deleted under it (rm -rf .objectstack/data), then a second boot on another port:Both halves of the card's claim confirmed directly, and this is the part worth reading:
GET /api/v1/data/crm_account["Acme Corp","Globex Ltd","Initech"]["EDITED_WITH_SERVER_STOPPED", ...]d7ZOOTvRfxl8exw2f8TGvX7J8iincNRbsys_userin the file at the same pathSo "a row edit made with the server stopped has no observable effect" and "a user that authenticates against the live server is not in the database" are both true readings of a healthy deployment. Both were reported as evidence of a broken write path in #15337 (its "Two observations that may point at the mechanism" section is verbatim this condition), and the investigation that followed cost a full P0 cycle. Nothing in the product is wrong; the deployment has no way to notice.
Both blast-radius citations verified:
rm -rf .objectstack/datais in #15337's repro block, and hotcrm'sdemo:resetisrm -rf .objectstack/data && pnpm build && ....What this adds
packages/cli/src/utils/served-database-file.ts— a pure identity check, plus a watch:missing(nothing at the path) orreplaced(a different file there);error, then stop watching. The message carries the two things the AGENTS.md degradation-log-level rule requires anerrorto carry: the consequence (every external observation of this deployment is now false, and it will keep answering 200) and the fix (restart, so it opens the file that is at that path now).describeDriverSqliteFileinconnection-display.tsanswers "which file on this filesystem" from a driver config — a second question over shapes that module already knows, deliberately not parsed back out ofdescribeDriverConnection's display string, which is free to redact and label.describeRegisteredDriverreturns it assqliteFile, which is the plumbing between the driver and the watch.It refuses nothing. The running server is still correct, merely invisible; breaking a working dev loop to close a reporting gap trades a bad hour for a worse one. Nothing is added to any payload, endpoint or state file.
Why periodic and not on-error, decided on measurement rather than taste: there is no error to hang it on. After the unlink every read and write still succeeds and nothing throws — that is the whole defect. A condition that never produces a failure can only be found by asking.
Two rungs, because inodes are recycled
Measured here while writing the tests: deleting a file and recreating one at the same path in the same millisecond handed back the same inode. A dev+inode comparison alone would therefore have been blind to the card's own second half ("a later boot creates a brand-new objectstack.db at the same path"). Birth time separates them — and it moved on that recreate while staying put across an ordinary write to a live inode.
Node documents two fallbacks for filesystems that do not store a birth time: the epoch (harmless, a constant compares equal forever) and a copy of
ctime(not harmless:ctimemoves on every write, so a healthy database would report itself replaced every interval). So the rung arms itself on evidence: if birth time were a copy ofctimeit would equalctimeby construction, and the two differing at capture proves this filesystem keeps them apart. Disarmed, the check simply falls back to one rung. Every uncertainty here resolves toward staying quiet — a missed report costs what today already costs; a false one sends an operator to restart a database that is fine. Silence from this watch is never a claim that the file is intact.Verification
End to end, on the real CLI build:
Ablation (prediction written first: with the call site removed the same run goes silent, and no unit test notices, because nothing else pins the wiring). Mutation proven on disk — call sites 1 to 0, injected marker 0 to 1 — and in
dist/viascripts/ablation-dist-preflight.mjsin both directions. Result: 0 report lines, health 200 — the silent state the card describes. Restored under a trap withgit checkout HEAD --, proven by blob-hash equality (a2b9b26cba46115d79f1b3954ecf09599fe0afa7both sides), an empty whole-treegit status --porcelain, and a rebuild re-proving the marker absent from all 488 built files.Run at
7a3382e9679(after theorigin/mainmerge, so these describe the head that is pushed):pnpm --filter @objectstack/cli exec vitest run— 254 files, 2979 passed, 6 expected failpnpm --filter @objectstack/cli typecheck— clean; all three new/edited test files confirmed inside the tsc program via--listFiles(not assumed)pnpm lint— whole repo, clean, no narrowing claimednode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackderives for this change set — all green.check:refd-timer-probeis among them, which is the one that would have caught an un-unref'd interval.Note for a reviewer re-running these:
check:type-check-coverageandcheck:type-check-debtgo red onpackages/cli/tmpif the CLI test suite is running at the same time — a scratch file a test writes into that gitignored directory. Both are green on a quiet tree; filed separately.One measured correction to the card
The card says the runtime state file makes "another server already holds this project's database" answerable at boot with no new state. It does not.
runtime.env_local.jsonas written bypublishBoundPortis:There is no database path in it, and the file is keyed by environment id under a machine-global home, so two different projects share
runtime.env_local.json. That boot-time line is answerable only by adding a key to that payload — which is a published-payload change and a decision above this PR. The half implemented here needs no new state and is exact; the other half is raised on the card.🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code