feat(runtime,cli,core): ADR-0119 D2 boot reconciliation + os migrate resume (#4617); retire IDataEngine.batch? (#4618) - #4687
Merged
Conversation
`batch?` was declared on `IDataEngine` for as long as that contract existed
and was never implemented by any engine — ObjectQL has no `batch` method and
there is no other engine in the tree — and never called: `DataEngineRequest`
was imported by exactly one file, the contract declaring the member.
Its whole specification was a three-word doc comment, which settles nothing
about partial failure, ordering, cross-object references, rollback scope, or
what `transaction: false` meant. Its neighbours `getDefaultDriverName?` /
`getDriverByName?` earn their optionality by naming an implementer and a
probing caller; this one named nothing.
The tell that nobody designed against it: `DataEngineBatchRequestSchema`
nested the request union recursively — a batch could contain batches — with
no statement about what that meant. The only test was a type pin asserting
the property was defined, which could not fail while the declaration existed.
What it claimed is now covered by members that are real: ADR-0119 D1 made
`transaction` reachable through the contract, D4 made `batchData`'s `atomic`
honest, and the wire batch has always gone through `POST {basePath}/batch`.
So this deletes a false affordance, not a capability (ADR-0049).
- Remove the member and the `DataEngineRequest` import from the contract.
- Remove `DataEngineBatchRequestSchema` and its arm from the request union.
Keep the union itself: every remaining arm is now unread too, but retiring
a published wire protocol is a different decision, tracked separately.
- Drop the type-pin test and the schema's self-parsing suite.
- Registry entry `data-engine-batch-retired` carries the FROM → TO for the
upgrade guide and spec-changes.json.
Deliberately no `retiredKey()` tombstone: a tombstone delivers through a
parse, and nothing ever parsed this schema. Its three authorable-surface
baseline lines, its json-schema.manifest entry and the stale
docs-import-surface baseline line are dropped here, deliberately. The
enforced channel is tsc.
Refs: ADR-0119 D3, ADR-0049, ADR-0078, #4618, #4612
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx
… resume` (#4617) Completes ADR-0119 D2. The runner and sys_migration_journal landed in #4668; this is the discovery channel that makes an interrupted run findable by someone who does not already know it happened. - `MigrationRecoveryPlugin` (@objectstack/runtime) — at kernel:ready, scans for runs that started and never concluded and warns per run: chunks committed, chunks with UNKNOWN outcome (chunk_started with no chunk_done), whether a compensation was left half-finished, and the command that acts. Also owns the `migration-plans` registry service. - `os migrate resume` (@objectstack/cli) — lists interrupted runs (read-only default, per #2186), or acts on one with --run under confirmation. Exits non-zero when a run ends `failed`, so a scripted recovery cannot move on from a migration that needs a human. - `MigrationPlanRegistry` (@objectstack/core) — where a resume finds the plan. Boot discovers, the CLI acts. Resuming is a large, irreversible, potentially hour-long write against production data; doing that as an unrequested side effect of a process starting is the kind of behaviour an operator finds out about from a graph. It is also not always possible at boot — a resume needs the plan's live callbacks, and the package owning them may not be loaded in whichever process happened to restart first. The per-plan `onCrash` policy still decides WHAT acting means; it does not decide WHEN, and "when" is the part a human should own. Deferring is safe because of the runner's re-entrancy: `started ∧ ¬done` is durable, so a run stays exactly as recoverable an hour later as it was at boot. The registry exists because a journal cannot hold a plan: forward/compensate are functions and load() reads the live database, so none of it crosses a process boundary — hence the journal stores the plan HASH. A run whose plan no loaded package registers is REPORTED, never silently skipped: "nothing to resume" and "the code that owns this run is not here" are different facts. Degradation is deliberate in both directions. No engine or no journal object (a lean kernel) → skipped in silence, because such a kernel has no interrupted runs and a warning there would train operators to ignore this plugin's output. A scan that FAILS is reported — "I could not check" is not "there is nothing to find". 11 new runtime tests pin the split (boot writes nothing to the journal), the three states an operator must tell apart, and both degradation paths; 2 new core tests cover the registry. Refs: ADR-0119 D2, #4617, #4668, ADR-0078 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 126 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…removal moved (#4618) CI's "Spec property liveness" job runs four gates, not one. `check:liveness` itself was fine; the two that broke are the ledgers that track the SHAPE of the code the batch removal changed. - `variant-docs.json`: the outer `DataEngineRequestSchema` union carried a `batch` variant, and a SECOND entry described the inner union inside `DataEngineBatchRequestSchema.requests` — the same member set minus `batch`. With the batch schema gone the inner union no longer exists and the outer one narrowed to exactly the inner one's old key, so the two entries collapse into one. Keeping both would have left an entry whose union is gone, which is what the gate reported. - The strictness ledger's `data-engine.zod.ts` row (14 → 13 `z.object(` sites) and the `data/` section header it sums into (162 → 161). Both are hand-maintained maps of the code, and the point of the gates is that a map which drifts is worse than none because it gets followed. Verified by running all 17 `@objectstack/spec` check:* scripts, not just the four in the failing job. Refs: #4618, ADR-0119 D3 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx
…-5akrna # Conflicts: # docs/protocol-upgrade-guide.md # packages/spec/spec-changes.json # packages/spec/src/migrations/registry.ts
This was referenced Aug 2, 2026
os-zhuang
marked this pull request as ready for review
August 2, 2026 19:41
This was referenced Aug 2, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent changes, one per commit, sharing a branch because this session has one designated branch. Review them separately — either can be dropped without touching the other. Say the word and I'll split the second into its own PR.
ed1c772— feat(core,platform-objects): implement the ADR-0119 D2 migration-journal runner (sys_migration_journal + runMigrationJournal) #4617 deliverable 3: boot reconciliation +os migrate resume35137b9— chore(spec): retire IDataEngine.batch? per ADR-0119 D3 — declared-but-unimplemented, zero callers #4618: retireIDataEngine.batch?1. ADR-0119 D2 deliverable 3 (#4617)
The runner and
sys_migration_journallanded in #4668. This is the discovery channel — what makes an interrupted run findable by someone who does not already know it happened. With it, #4617 is complete.MigrationRecoveryPlugin(@objectstack/runtime) — atkernel:ready, scans for runs that started and never concluded, warning per run: chunks committed, chunks with unknown outcome (chunk_startedwith nochunk_done), whether a compensation was left half-finished, and the exact command that acts. Owns themigration-plansregistry service.os migrate resume(@objectstack/cli) — lists interrupted runs (read-only default, per #2186's "a bare command must never mutate by surprise"), or acts on one with--rununder confirmation. Exits non-zero when a run endsfailed, so a scripted recovery cannot move on from a migration that needs a human.MigrationPlanRegistry(@objectstack/core) — where a resume finds the plan.Boot discovers, the CLI acts
The design decision in this change, and deliberate rather than incidental.
Resuming is a large, irreversible, potentially hour-long write against production data. Doing that as an unrequested side effect of a process starting is the kind of behaviour an operator finds out about from a graph. It is also not always possible at boot: a resume needs the plan's live callbacks, and the package owning them may not be loaded in whichever process happened to restart first.
So boot surfaces the run and names the command; the command acts, under explicit operator intent. D2's per-plan
onCrashpolicy still decides what acting means — resume forward, or unwind — it just does not decide when, and "when" is the part a human should own.Deferring is safe precisely because of the runner's re-entrancy:
started ∧ ¬doneis durable, so a run stays exactly as recoverable an hour later as it was at boot. Nothing decays while the operator decides.Why a plan registry exists
A journal cannot hold a plan.
forward/compensateare functions andload()reads the live database, so none of it crosses a process boundary — which is why the journal records the plan hash, not the plan. Recovery needs the plan handed back by the code that owns it, andmigration-plansis that seam.A run whose plan no loaded package registers is reported, never silently skipped. "Nothing to resume" and "the code that owns this run is not here" are different facts, and only one is safe to ignore.
Degradation, deliberate in both directions
No engine or no journal object (a lean kernel) → skipped in silence: such a kernel has no interrupted runs, and a warning there would train operators to ignore this plugin's output — the one thing it cannot afford. A scan that fails is reported, because "I could not check" is not "there is nothing to find".
Verification
11 new runtime tests pin the split (boot writes nothing to the journal), the three states an operator must tell apart (clean / interrupted / half-unwound), and both degradation paths. 2 new core tests cover the registry, including that a re-registered id replaces rather than accumulates — with the journal's plan-hash check as the backstop against resuming a changed plan.
2. Retire
IDataEngine.batch?(#4618)batch?was declared onIDataEnginefor as long as that contract existed and was never implemented by any engine —ObjectQLhas nobatchmethod, and there is no other engine in the tree — and never called:DataEngineRequestwas imported by exactly one file, the contract declaring it.Its whole specification was a three-word doc comment, which settles nothing about partial failure, ordering, cross-object references, rollback scope, or what
transaction: falsemeant. Its neighboursgetDefaultDriverName?/getDriverByName?earn their optionality by naming an implementer and a probing caller; this one named nothing.Two tells that nobody ever designed against it:
DataEngineBatchRequestSchema.requestsnested the request union recursively — a batch could contain batches — with no statement about what that meant for ordering or rollback.batchproperty and asserted it was defined. That pins the type; it could not fail while the declaration existed, and would have passed unchanged for the member's whole life with no engine implementing it.What it claimed is now covered by members that are real — ADR-0119 D1 made
transactioncontract-reachable, D4 madebatchData'satomichonest, and the wire batch has always beenPOST {basePath}/batch. So this deletes a false affordance, not a capability (ADR-0049).Scope calls
CrossObjectBatchRequestSchema/BatchUpdateRequestSchemafromapi/batch.zod.ts— a different schema that never had anything to do with the removed one.DataEngineRequestSchemastays, minus itsbatcharm. Every remaining arm is now unread too (there is no Virtual Data Engine implementation, only this schema describing one) — flagged in-file and worth its own issue, but retiring a published wire protocol is a different decision from retiringbatch?and does not belong in a change whose title promised something narrower.retiredKey()tombstone. A tombstone delivers its prescription through a parse, and nothing ever parsed this schema — the playbook's third route ("a prescription nobody can receive is noise"). Its threeauthorable-surface.jsonbaseline lines, itsjson-schema.manifest.jsonentry, and the now-staledocs-import-surface.baseline.jsonline are dropped here, deliberately. The enforced channel istsc.Registry entry
data-engine-batch-retiredcarries the FROM → TO into the upgrade guide andspec-changes.json.Verification (both commits)
pnpm typecheck— 122/122.pnpm lintclean.check:adr-anchorsOK.check:generated— 8/8 current.os migrate resumeverified present in the built CLI with its flags (--database-url,--run,--yes,--json); oclif discovers it by pattern fromdist/commands, so there is no manifest to regenerate.One note from doing this:
gen:api-surfacereadsdist, and an earlier regeneration here ran against an incomplete one — it silently dropped an unrelatedActivationEvent (type)export from./studioand would have ratcheted a baseline exemption in for it. Caught and reverted by diffing the generated files againstmain; the artifacts in this PR come from a clean rebuild and contain onlybatch-related deletions. Worth knowing before the next spec removal.Refs: ADR-0119 D2/D3, #4617, #4618, #4668, #4612, ADR-0049, ADR-0078
Generated by Claude Code