Staged reconcile on config change, destructive handling, and opensaas db update - #1225
Conversation
…nd add `opensaas db update` A config edit under `opensaas dev` now emits the new Contract module, its artifacts and the bundle into `.opensaas/staged/`, plans `prisma db update` against them, and promotes them into place only once the plan has applied — so the app never reloads onto a contract the database does not carry (ADR-0063). Staging is a staged `prisma.config.ts` the CLI is pointed at with `--config`: `db update --to` takes a hash, ref or migration directory rather than a contract file, so the way to plan against a staged contract is a config that names one. That config names the project's own `migrations` directory, and the loop snapshots the graph's refs around every run it does not promote, so a discarded plan leaves no advanced ref to shadow the source contract. A destructive plan mid-session is not applied and never prompts: the loop prints it, leaves database and bundle at the previous schema, and keeps serving. `opensaas db update --confirm postgres` is what applies it. The reconcile runs inside the loop — it holds PGlite's data directory, the staged generation and the app child — so the second terminal opens no database connection of its own; it reaches the loop over a token-guarded loopback channel published at `.opensaas/dev-loop.json`, and fails naming `opensaas dev` when nothing is listening. A destructive promote restarts the app child, since a client cached across a reload keeps querying the dropped column. `generateCommand` can now write to a staging directory and report a refusal by throwing rather than exiting, so a half-saved config no longer takes the loop down with the app it is serving. The project config is loaded with jiti's module cache off: it is keyed by path and outlives the instance, so the loop was staging the schema it booted on. Implements #1158. Part of #1125. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🦋 Changeset detectedLatest commit: 1243532 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| * this refuses — a half-saved config must not take the loop down with it. | ||
| */ | ||
| const stage = async (say: (message: string) => void): Promise<GenerationResult | undefined> => { | ||
| fs.rmSync(stagingDir, { recursive: true, force: true }) |
There was a problem hiding this comment.
stage() wipes the staging directory but leaves staged pointing into it.
staged is only ever cleared in promote(). stage() unconditionally rmSyncs stagingDir and then regenerates into it, so after a destructive edit has parked a GenerationResult in staged, the next config save silently replaces the bytes those paths refer to — staged is really just a path holder, not a snapshot.
Concrete failure: save a destructive edit (staged = gen1, correct and complete). Save again with a config that generation refuses after the Contract module is written — e.g. the relation-graph agreement gate at the end of runGeneration. stage() returns undefined and the loop prints “nothing was staged”, but staged still holds gen1, whose prismaConfig/contractModule paths now contain the refused generation. A later opensaas db update --confirm postgres takes the staged branch in onDatabaseUpdateRequest, plans and applies that refused contract against the database, then promoteStagedGeneration copies whatever partial files exist over the live Contract module and bundle.
Clearing it alongside the wipe closes it:
| fs.rmSync(stagingDir, { recursive: true, force: true }) | |
| staged = undefined | |
| fs.rmSync(stagingDir, { recursive: true, force: true }) |
| live: ResolvedWritePaths, | ||
| stagingDir: string, | ||
| ): void { | ||
| const moves: readonly (readonly [string, string])[] = [ |
There was a problem hiding this comment.
Promotion moves a fixed set of eight files, so anything else a generation wrote into the staged bundle is stranded.
runGeneration writes plugin afterGenerate output into the bundle directory by key, for any key that is not contractModule/types/context:
const filePath = path.join(paths.opensaasDir, filename)
fs.writeFileSync(filePath, content)Under --stagingDir that lands in .opensaas/staged/bundle/<filename>, and moves above never mentions it — so promoteStagedGeneration copies the eight known files and then rmSyncs the staging directory, deleting the plugin file. A project whose plugin emits an extra bundle file gets it on opensaas generate but never under opensaas dev: the live copy stays at whatever the last non-staged generation left (or is absent entirely on a fresh project), while the rest of the bundle moves forward. GeneratedFiles is public plugin surface, so this is reachable without changing any code here.
Copying the whole staged bundle directory (rather than an enumerated list) would also make promotion robust to future bundle files.
| resolve(1) | ||
| }) | ||
| spawned.once('exit', (code, signal) => { | ||
| if (restarting) { |
There was a problem hiding this comment.
Ctrl-C inside the restart window respawns the app instead of shutting the loop down.
restart() sets restarting = true and sends SIGTERM, then returns; the flag is only cleared here, when the child’s exit finally arrives. If the user presses Ctrl-C in that gap — which is exactly the moment a destructive db update has just triggered a restart and the terminal is busy — devCommand’s onSignal calls app.kill("SIGINT") on a child that is still isRunning(), the child exits, and this branch sees restarting === true and spawns a fresh next dev rather than resolving. run() never settles, so the finally that stops the Dev database and removes the state file never runs.
The second Ctrl-C does not recover it either: the handlers are registered with process.once, so SIGINT is back to Node’s default and terminates the process without firing exit — leaving the respawned app child orphaned and .opensaas/dev-loop.json plus the Dev database state file on disk.
A flag the signal path can clear (or restart() recording the intent and onSignal cancelling it) would make the exit deterministic.
| try { | ||
| writeContractModule(contractData, paths.contractModule) | ||
| writePrismaConfig(contractData, paths.prismaConfig, { | ||
| writePrismaConfig(contractData, resolved.prismaConfig, { |
There was a problem hiding this comment.
A staged generation still overwrites the live project-root prisma.config.ts.
This write is unconditional, so the staging isolation the PR describes (“the live prisma/contract.json and .opensaas bundle are untouched until promotion”) has a hole: the root config is rewritten from the new contractData before anything is planned.
The rendered file’s only contract-derived content is the extension import list, so this bites when a config edit changes db.extensions and the plan is then discarded — a destructive plan, or a db update that fails. The loop restores the migration refs and promotes nothing, but the root prisma.config.ts is left describing an extension set that prisma/contract.ts and prisma/contract.json do not carry. The next npx prisma db update a user runs by hand, or the reconcile() on the next opensaas dev start, reads that mismatched config.
Since the staged run writes its own config a few lines below, gating this one on staging === undefined would keep the live file at the last promoted generation.
| const generation = await stage(say) | ||
| if (generation === undefined) return | ||
|
|
||
| const refs = snapshotMigrationRefs(cwd) |
There was a problem hiding this comment.
The refs snapshot is taken after stage(), which has already mutated migrations/.
stage() → generateCommand runs seedExtensionContractSpaces(cwd, contractData), whose own result type reports action: "updated" for “the head ref moved or a package was materialised”, and migrationDirs for packages it wrote. That happens against the project’s real migrations/ directory, before this line runs.
So when a config edit changes db.extensions and the resulting plan is destructive (or the apply fails), restoreMigrationRefs puts back the post-seed refs, not the state the loop found — and the materialised migration packages are never removed at all. The ADR-0063 guarantee this snapshot exists for (“a discarded schema leaves no advanced ref to shadow the source contract”) does not hold for the extension spaces.
Snapshotting before stage() would cover the seed as well as the plan.
| watcher.on('change', async () => { | ||
| console.log(chalk.yellow('\nConfig changed, regenerating...\n')) | ||
| await generateCommand() | ||
| watcher.on('change', () => { |
There was a problem hiding this comment.
The watcher is armed before the startup generate + reconcile, and queue does not serialise against them.
queue only orders reconciles relative to each other. A config save during the initial await generateCommand() / await reconcile(cwd) below queues onConfigChange immediately, which runs its own generateCommand({ stagingDir }) — including seedExtensionContractSpaces writing into the same migrations/ directory — and then prisma db update against the same database, concurrently with the interactive startup reconcile(). Two db update runs overlapping on one database is a materially worse outcome than the pre-existing double-generate this replaces.
Registering the watcher after the startup reconcile succeeds (or seeding queue with that startup work) closes the window.
| } | ||
| }) | ||
|
|
||
| socket.once('error', () => { |
There was a problem hiding this comment.
Any socket error after the exchange has started is reported as “no dev loop is running”.
settled is only set by settle(), which runs on the result line. So an error at any point after a successful connect — the loop crashing mid-apply, the socket being reset while prisma db update is running — rejects with NoDevLoopError, and dbUpdateCommand prints “No opensaas dev loop is running in this project… start opensaas dev in another terminal first.”
That is the one message a user must not get there: the loop was running and may have half-applied a schema change, and the advice is to start a second loop. Distinguishing pre-connect failure (genuinely no listener) from a drop mid-exchange (“the dev loop stopped responding while applying; check its output”) would keep the wording honest.
| clearFile, | ||
| async close() { | ||
| clearFile() | ||
| await new Promise<void>((resolve) => server.close(() => resolve())) |
There was a problem hiding this comment.
server.close() only stops new connections — it never resolves while a socket is still open, and nothing here tracks or destroys them.
stop() in devCommand awaits control?.close() before database?.stop(), so a single lingering connection hangs Ctrl-C with PGlite still holding its data directory and the state file still on disk.
The happy paths are fine (both ends end()), but the server socket has no timeout and the handler ignores everything after the first newline, so a peer that connects and sends a partial line — an aborted client, a port scanner, anything reaching 127.0.0.1 on the published port — parks the connection indefinitely. Keeping a Set of live sockets and destroying them in close() (or server.closeAllConnections()) makes shutdown bounded.
Coverage Report for Core Package Coverage (./packages/core)
File CoverageNo changed files found. |
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Auth Package Coverage (./packages/auth)
File CoverageNo changed files found. |
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
|
|
||
| const file = controlFilePath(cwd) | ||
| fs.mkdirSync(path.dirname(file), { recursive: true }) | ||
| fs.writeFileSync( |
There was a problem hiding this comment.
medium (security) — the 0o600 is not enforced on a control file that already exists.
fs.writeFileSync's mode is passed to open(2) and, per Node's docs, only applies to a newly created file. This path writes over a stale dev-loop.json from a previous run — and, more to the point, over one an attacker pre-created. A process that can write into .opensaas/ (a shared checkout, a group-writable tree, or anything running as the same user) can create dev-loop.json at 0666 beforehand, or make it a symlink to a path it can read, and the loop will write the freshly minted token straight into it. writeFileSync follows symlinks.
The token itself is fine — randomBytes(24), 192 bits, checked on every request, socket bound to 127.0.0.1 only. But the whole channel's authority rests on that token staying unreadable, and holding it lets a caller drive prisma db update --confirm <anything> against the dev database, including a destructive plan.
fs.rmSync(file, { force: true }) immediately before the write (or opening with wx after the unlink) closes both the mode and the symlink case. Worth a test asserting the resulting mode over a pre-existing 0666 file — control.test.ts covers refusal and staleness but not this.
| for (const [from, to] of moves) { | ||
| if (!fs.existsSync(from)) continue | ||
| fs.mkdirSync(path.dirname(to), { recursive: true }) | ||
| fs.copyFileSync(from, to) |
There was a problem hiding this comment.
medium — promotion is eight independent copyFileSync calls with no atomicity, at the one moment the database has already moved.
If any copy after the first throws (EACCES, ENOSPC, a Windows lock on context.ts), the live bundle is left half on the new contract and half on the old. By then prisma db update has already applied, so the database carries the new schema, promote() never reaches staged = undefined, and the throw surfaces only as the queue's generic Staged reconcile failed: — which tells the user nothing about the split state or how to get out of it.
Even without a failure, each copyFileSync is non-atomic in isolation: the app's dev server is watching these paths and can read a truncated contract.ts mid-copy. Copying to a sibling temp name and renameSync-ing into place makes each file swap atomic and cheap; recording progress so a partial failure can report "promote the rest with X" would close the rest.
borisno2
left a comment
borisno2
left a comment
There was a problem hiding this comment.
Code review — REQUEST CHANGES
Reviewed at high effort against the diff alone. The shape of the change is right: staging generation behind reconciliation, refusing to promote a contract the database has not reached, and routing the second-terminal db update through the loop that actually owns PGlite are all the correct calls. The control channel is also better built than this kind of thing usually is — 192-bit randomBytes token, verified on every request before the handler runs, socket bound to 127.0.0.1 only, command a z.literal, and confirm tokens reaching Prisma as spawn argv rather than a shell string. No any, no casts, no @ts- anywhere in the new code. The changeset is present and correctly scoped minor.
Ten findings, all posted inline. Blocking on the first four.
high
commands/dev.ts:159—stage()wipesstagingDirbut never clearsstaged. After a destructive edit parksgen1instaged, a later save whose generation is refused leavesstagedpointing at files that no longer exist; the nextopensaas db update --confirm postgresthen applies and promotes a generation the loop rejected.
medium
dev/control.ts:157— the0o600is not enforced over a control file that already exists. Node appliesmodeonly on creation, andwriteFileSyncfollows symlinks, so a process that can write into.opensaas/can pre-createdev-loop.jsonworld-readable (or as a symlink) and harvest the token. Everything else about the channel is sound; this is the one hole in it.rmSyncbefore the write fixes it, and it wants a test.dev/staged-reconcile.ts:185— promotion is eight independentcopyFileSynccalls with no atomicity, run after the database has already moved. A mid-way failure leaves the live bundle split across two contracts and reports only the queue's generic "Staged reconcile failed". Copy-to-temp +renameper file.dev/staged-reconcile.ts:171—promoteStagedGenerationmoves an enumerated eight files, butrunGenerationalso writes pluginafterGenerateextras intopaths.opensaasDir. Those are never promoted and are deleted with the staging dir, so underopensaas devthe live copy silently stays stale.dev/app-runner.ts:102— Ctrl-C betweenrestart()'s SIGTERM and the child'sexitmakes the exit handler respawn instead of resolving.run()never settles, thefinallythat stops the Dev database never runs, and since the handlers areprocess.oncethe second Ctrl-C kills node without firingexit, orphaning the child.commands/generate.ts:240— a staged generation still overwrites the live rootprisma.config.ts. When adb.extensionschange is discarded as destructive, the root config then describes an extension setprisma/contract.tsdoes not carry.
low
commands/dev.ts:191— the refs snapshot is taken afterstage(), which has already runseedExtensionContractSpacesagainst the realmigrations/. A discarded plan restores post-seed refs and leaves materialised packages behind.commands/dev.ts:282— the watcher is armed before the startupgenerate/reconcileandqueuedoes not serialise against them, so a config save during startup can run a stagedprisma db updateconcurrently with the interactive one.dev/control.ts:253— any socket error after connect rejects asNoDevLoopError, so a loop that dies mid-apply tells the user no loop is running and to start one.dev/control.ts:176—server.close()waits on open sockets with no tracking, timeout, orcloseAllConnections(); one parked connection hangsstop()beforedatabase.stop()ever runs.
Checked and clear
generateCommand's contract change: all three callers updated, the default path still prints the same message andprocess.exit(1)s, every throw is preceded by the console output it replaced, and nothing swallows a failure. One inconsistency worth a thought rather than a change:dev.ts:291deliberately keeps the exiting form, so a startup generation failure skips thefinallythat would stop the Dev database — theexitlistener covers the files but not the sidecar.restoreMigrationRefsis correct for what it snapshots: it deletes refs the run introduced and rewrites the rest byte-for-byte, and cannot resurrect a stale ref over a newer one since the snapshot and restore bracket a single cycle.RefSnapshot.contentsis typedstring | undefinedbut the producer never yieldsundefined, making the guard in the restore loop dead — tighten the type.moduleCache: false: correct fix for the staleness bug and the comment earns its place (external jiti behaviour). Cost is bounded — the other callers are one-shot commands where a re-read is free. The one thing to keep in view is that in the long-running loop every config save now re-evaluates the config's whole import graph, so a plugin that opens a pool or registers global state at module scope will accumulate one per save.- Comments read clean against the repo rule: the file header on
control.ts, theprocess.exitskipsfinallynote, and the jiti note all carry constraints a reader cannot derive; the docblocks are on exported surfaces.
- `stage()` drops the staged pointer with the staging directory it names, so a refused generation can no longer be promoted by a later `db update`. - The control file is removed before it is written, so 0600 holds over a pre-created file and a planted symlink cannot redirect the token. - Promotion swaps each file in through a rename, copies the whole staged bundle directory (a plugin's `afterGenerate` extras included), and reports a partial failure as a named split rather than a bare copy error. - The project-root `prisma.config.ts` is held back with the staged generation and promoted with it. - A signal inside the restart window shuts the app child down instead of respawning it. - The migration refs are snapshotted before generation seeds extension spaces. - The config watcher is armed only after the startup generate and reconcile. - A drop mid-exchange is reported as the loop stopping, not as no loop running, and closing the channel destroys live sockets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks — all ten are addressed in e3db729. Per finding: 1. 2. 3. Promotion atomicity ( 4a. Plugin 4b. Ctrl-C inside the restart window ( 4c. Staged generation overwriting the live root 5a. Refs snapshot after 5b. Watcher armed before the startup generate ( 5c. Post-connect socket errors reported as "no loop running" ( 5d.
Verification: |
borisno2
left a comment
borisno2
left a comment
There was a problem hiding this comment.
Verification of e3db7297 — APPROVE
Checked the ten findings against e3db7297, not the PR from scratch. Each fix was verified by reading the code and by reverting the fix in a scratch worktree and re-running its test, so "discriminates" below means observed, not assumed. packages/cli: 35 files / 369 tests green, the tests/staged-reconcile.test.ts e2e green (7.4s), eslint clean.
All ten are addressed. Four residuals below, none of them the original defects; they are follow-ups, not blockers.
1. HIGH — a refused generation could be promoted — RESOLVED
commands/dev.ts:162-163: staged = undefined precedes fs.rmSync(stagingDir, …), so there is no window where the pointer outlives the bytes.
Checked the invariant beyond the tested path rather than taking the test's word for it. staged has exactly three writers — stage() (162), promote() (179), the destructive park (210) — and stagingDir exactly three touchers — stage() (163), promoteStagedGeneration (staged-reconcile.ts:245), the startup wipe (290, staged is necessarily undefined there). Every pairing agrees:
- refused generation → cleared at 162, dir wiped,
stage()'scatchreturnsundefined— the failure is after both, so no error path separates them; PartialPromotionError→promote()never reaches 179 andpromoteStagedGenerationnever reaches itsrmSync, so pointer and directory both survive together — consistent, and arguably the right state to retry from.
The test at commands/dev.test.ts:232 drives park → refuse → db update and fails when 162 is removed (verified: the reverted code reports ok: true / "Applied, promoted." against a wiped staging dir).
Residual (hair-thin): promote() clears staged at dev.ts:179, i.e. after promoteStagedGeneration's closing fs.rmSync(stagingDir). A throw from that rmSync leaves the pointer naming a removed tree. staged = undefined before the call, or a finally, closes the last one.
2. MEDIUM (security) — control-file mode — RESOLVED
dev/control.ts:161-165: fs.rmSync(file, { force: true }) immediately precedes the write, and that is the only writeFileSync of the control file in the package (clearFile() only unlinks). Both tests discriminate (reverting the rmSync fails each).
The symlink test does assert what it should: expect(fs.readFileSync(decoy, 'utf-8')).toBe('nothing yet') — the decoy target is untouched, not merely "the file ended 0600" — plus lstatSync(file).isSymbolicLink() false.
Residual: unlink-then-create is TOCTOU-racy — an attacker who can write .opensaas/ can re-plant the symlink between the two calls. flag: 'wx' on the write (O_EXCL, which fails on a symlink) would make it race-free at no cost.
3. MEDIUM — promotion atomicity — RESOLVED as scoped
staged-reconcile.ts:180-192: swapIntoPlace copies to ${to}.promoting-${pid} — a sibling of the destination, so the renameSync is within one directory and cannot hit EXDEV. Correct on the point that matters; the cross-device copy is the copyFileSync, which does not need to be atomic. The temp is cleaned up in the catch.
PartialPromotionError (:161-178) names failedOn, the count already moved, and the recovery (opensaas generate), and the docblock at :201-207 states plainly that the set of files is not atomic. Accurate.
Residual (a): the changeset says promotion "swaps each file into place through a rename, so the running app never reads a half-written one" and stops there. The honest limitation the docblock carries — a crash mid-promotion still leaves a split bundle — is absent from the release note, which is the part a user reads. Worth one sentence.
Residual (b): no test discriminates the rename itself. Replacing swapIntoPlace's copy+rename with a plain copyFileSync(from, to) leaves all 8 staged-reconcile.test.ts tests green (verified). "leaves no temp file behind" passes trivially without any temp mechanism, and "names the split" passes because a copy into a directory throws either way. The mechanism is right; it is currently unasserted.
4. Plugin afterGenerate extras — RESOLVED
staged-reconcile.ts:221-229 copies the whole staged bundle directory. Test at staged-reconcile.test.ts:101 discriminates. Non-recursive (if (!entry.isFile()) continue) is fine — generate.ts:332 writes extras as path.join(paths.opensaasDir, filename) with flat GeneratedFiles keys.
5. Ctrl-C during restart() — RESOLVED
dev/app-runner.ts:85: kill() clears restarting unconditionally, before the isRunning() guard — which is the right order, since the race is precisely the one where the child has already gone and the guard would skip. Test at app-runner.test.ts:52 discriminates (reverted, it respawns and run() never settles).
6. Staged generation overwriting the live root prisma.config.ts — RESOLVED, and best-evidenced of the ten
Scrutinised both states as asked.
Parked: generate.ts:241-256 gates the live write on staging === undefined; the staged run writes a root-shaped config to .opensaas/staged/prisma.config.root.ts instead, and its own absolute-path config separately (output-paths.ts:198 vs :29) — two distinct files, so the whole-bundle copy cannot promote the wrong one.
After promotion: the held-back file is rendered with crossReferences.prismaConfigContract / prismaConfigOutput, i.e. the live relative references, with no migrationsDir/envDir override — so envDir stays import.meta.dirname, which resolves to the project root once the file lands there, and is never evaluated while it sits in staging. staged-reconcile.ts:230-231 moves it last.
The e2e asserts both directions with a stamp that distinguishes "left alone" from "rewritten identically" (tests/staged-reconcile.test.ts:245-253, 271-274). Verified it discriminates: reverting the gate in generate.ts fails with expected '// ⚠️ GENERATED FILE…' to contain '// held back until promotion'.
7. Refs snapshot taken too late — RESOLVED, with one asymmetry
Moved to dev.ts:194 (before stage(), so it covers seedExtensionContractSpaces) and dev.ts:242, and the "nothing was staged" branch at :245 now restores.
Residual: the sibling branch in onConfigChange does not. dev.ts:199-200 is if (generation === undefined) return with no restoreMigrationRefs(cwd, refs). A generation that refuses after the seed — the seed runs before contract emit and the relation-graph gate — leaves the seeded refs behind, which is the same class of leak the finding was about. Given :245 was just fixed, the asymmetry looks unintended.
8. Watcher armed before startup generate — RESOLVED
dev.ts:292-307, now after generateCommand() + reconcile(). No test; verified by reading.
9. Post-connect socket errors — RESOLVED
control.ts:205-212 (DevLoopUnreachableError), :279-288 (abandon picks on connected), handled in commands/db.ts:35-36. Test at control.test.ts:113 discriminates.
10. server.close() hanging + RefSnapshot.contents — RESOLVED
control.ts:87-89 tracks sockets, :185-188 destroys them before awaiting the close. Test at control.test.ts:129 (2s deadline) discriminates. RefSnapshot.contents is now string and the dead guard is gone (staged-reconcile.ts:119).
New, from this pass
- Extension spaces un-seeded under a parked destructive change (medium-low, unverified — no pack in the fixture). Now that the snapshot precedes the seed,
onConfigChange's destructive branch (dev.ts:207-208) restores refs and so deletes the ref for an extension contract space that generation just seeded.onDatabaseUpdateRequestthen reusesstagedand never re-runsstage(), so nothing re-seeds beforedb update --confirm. A save that adds a pack and is destructive may plan against a graph missing that space's ref. Worth a fixture with a declared pack. - One cast in new code:
const children = vi.hoisted(() => [] as FakeChild[])(app-runner.test.ts:11). Test-only, the empty-array idiom;vi.hoisted<FakeChild[]>(() => [])avoids it. Noany, no@ts-, no casts anywhere in newsrc/.
Scope
e3db7297 touches packages/cli/src, packages/cli/tests and one changeset — nothing from #1224, #1198, #1219, #1159/#1160. git diff origin/prisma-8... over .changeset/ shows a single A tidy-pandas-stage.md: no other release note was disturbed. The reviewer-cleared areas are untouched — randomBytes(24) and the per-request token check (control.ts:84, 116), the 127.0.0.1 bind (:141), command: z.literal('db-update') (:30), and moduleCache: false (config-load.ts, not in this commit).
Posting as a Comment review rather than an Approve event: the GitHub identity here is the PR's own author. The verdict is APPROVE — the four residuals above are follow-ups.
Restore the migration refs on the config-change branch that stages nothing, matching its sibling on the `db update` path. Assert that promotion replaces the live file rather than rewriting it, so a reader holding the old one keeps whole bytes. Publish the control file with an exclusive create, so the token only ever lands in a file this process made. Name the split-bundle limit in the changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closed the four residuals from the verification pass ( 1. Ref restore on the config-change branch that stages nothing (
Discrimination: reverting the branch to a bare 2. An assertion that the per-file promotion is actually atomic ( The rename mechanism is now observable. The test hard-links the live bundle file before promoting — a second name for the same inode, standing in for a reader holding it open — and asserts that after promotion the live path reads the new bytes, the held name still reads the complete previous bytes, and the live inode has changed. Discrimination: substituting 3. The split-bundle limitation in the changeset ( Added a closing paragraph to the existing entry, in the file's voice: each file lands atomically, the set of them does not, a crash part-way leaves the bundle split across two contracts, the loop names the file it stopped on, and re-running 4. TOCTOU in the control-file write ( The write now uses Honest note on discrimination: I could not write a unit test that genuinely fails against the old Verification: Out of scope as instructed: #1226 (extension-space refs), #1224, #1198, #1219, and the reviewer-cleared areas. |
1 participant
Implements #1158. Part of #1125. Builds on #1157 (
745cc0be).What changed
A config edit under
opensaas devno longer regenerates in place. The new Contract module, its emitted artifacts and the bundle are written into.opensaas/staged/,prisma db updateis planned against them, and they are promoted into their real locations only once the plan has applied — so the app never reloads onto a contract the database does not carry (ADR-0063).Staging, given
--totakes a ref and not a path.db update --toaccepts a hash, ref name or migration directory, so a staged contract cannot be named directly. What is staged is therefore aprisma.config.tsof its own, which names the staged Contract module and output directory by absolute path, and the CLI is pointed at it with--config.prisma contract emitwrites beside that source, and the liveprisma/contract.jsonand.opensaas/bundle are untouched until promotion.Ref control. The staged config names the project's own
migrationsdirectory rather than starting a private graph beside itself, and the loop snapshots everymigrations/*/refs/*.jsonaround any run it does not promote, restoring them if the plan is discarded or the apply fails. A discarded schema therefore leaves no advanceddbref to shadow the source contract on the next plan.Destructive mid-session. The plan is printed, nothing is applied, nothing is promoted, and the app keeps serving. There is no prompt — a consent prompt interleaved with dev-server output is what ADR-0063 rejects. The loop prints the
pnpm db:updateinstruction instead.opensaas db update. A new command, whatpnpm db:updatepoints at. The reconcile itself runs inside the running loop, which holds PGlite's data directory, the staged generation and the app child; the second terminal opens no database connection of its own (the sidecar multiplexes every connection onto one backend, so a second client must not become a pool). It reaches the loop over a token-guarded loopback channel published at.opensaas/dev-loop.json— the same shape as the sidecar's state file, with the pid used to ignore a stale record — passes--confirmstraight through to Prisma, promotes on success, and restarts the app child after a destructive promote, since a client cached across a reload keeps querying the dropped column. With no loop listening it fails namingopensaas dev.Two supporting fixes.
generateCommandcan now write to a staging directory and report a refusal by throwing rather than exiting, so a half-saved config no longer takes the loop down with the app it is serving. And the project config is loaded with jiti's module cache off: that cache is keyed by path and outlives the instance holding it, so the loop was staging the schema the process booted on.Acceptance criteria
opensaas db update --confirm postgresapplies, promotes and restarts the child, after which the dropped column is gonepnpm db:updatewith no sidecar errors namingopensaas devdb updateagainst the sidecar does not crash the running appTests
packages/cli/tests/staged-reconcile.test.tsruns the real binary against a fixture project and observes the database and the served app, never the watcher's internals. The app is a small HTTP server on onepgconnection that reports its pid, the contract it loaded from disk and the columnsNoteactually has; the test drives an additive edit, a destructive edit,opensaas db update --confirm postgresand a second no-opdb updateagainst the live sidecar. Unit suites cover the control channel's token, staleness and failure wording, and the ref snapshot/restore and promote.Both CLI and core suites pass with and without
DATABASE_URLset.🤖 Generated with Claude Code