Skip to content

fix(cli): os migrate plan/apply exit non-zero when the host config exists but could not be loaded - #13113

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-12953-migrate-plan-unloadable-config
Aug 29, 2026
Merged

fix(cli): os migrate plan/apply exit non-zero when the host config exists but could not be loaded#13113
os-litant merged 2 commits into
mainfrom
claude/issue-12953-migrate-plan-unloadable-config

Conversation

@os-litant

@os-litantos-litant commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Fixes#12953

Executes the maintainer ruling of 2026-08-29 (verbatim 「同意」, option A) on
issue #12953: when a host objectstack.config.{ts,js,mjs}exists but cannot
be loaded
, os migrate planandos migrate apply now exit non-zero,
with an error naming the config file, the underlying failure, and the remedy.

A green exit over an UNMEASURED partial metadata set is the false-green a
migration tool must never emit — the population this "regresses" was computing
defective plans all along.

What the change is

The composition already knew this shape (hostConfigLoaded: false with a
non-null hostConfigPath, #12938); it just drew no verdict from it. It now
carries the underlying failure structurally (hostConfigError), and one shared
choke point — refuseWhenHostConfigUnloadable() — renders the refusal and sets
process.exitCode. Both commands call it from their run() wrapper, after
the body has returned, so the whole report is written before the process exits
non-zero.

Two things are deliberately preserved, because the ruling pinned them:

  • the loud stderr warning, byte for byte;
  • composition.hostConfigLoaded in the --json payload — the discriminator
    consumer coverage gates read (objectstack-ai/cloud#1705). A table count
    cannot replace it: the platform floor lands either way, so the count rises
    either way.

The refusal changes the exit status, not the document. On the unloadable
path the JSON payload is byte-identical to the one emitted before this PR.

Measured, by driving the real commands

Not read off the source. Three fixture projects, the real CLI
(bin/run-dev.js through tsx), four invocations each — migrate plan,
migrate plan --json, migrate apply --yes, migrate apply --yes --json
captured before and after, with the temp path and the wall-clock duration
masked as per-run noise.

directionbeforeafterstdout + stderr
config present + unloadableexit 0 ×4exit 1 ×4stdout byte-identical; stderr gains exactly one line (the refusal)
config absentexit 0 ×4exit 0 ×4byte-identical, both streams, all four invocations
config present + loadableexit 0 ×4exit 0 ×4byte-identical, both streams, all four invocations

The unloadable fixture is the issue's own shape: a config whose module body
throws when an environment variable is missing, the way ObjectStack Cloud's
control-plane config throws without AUTH_SECRET.

The new stderr line, as emitted (the project path is written PROJECT/ here —
an angle-bracket placeholder is eaten by GitHub's body sanitizer, backticks and
code fences included):

[migrate] ✗ Host config PROJECT/objectstack.config.ts exists but could not be loaded:
Missing required environment variable OS_12953_AUTH_SECRET. This run therefore covered
ONLY the objects the data stack registered — a fraction of what this deployment serves —
so its result is UNMEASURED, not "in sync", and it is reported as a FAILURE rather than
as success. Remedy: supply the environment this config needs (the failure named above
says which), or fix the config, then re-run.

The trap this PR is written against

hostConfigLoaded is false on the config-absent shape too — nothing
loaded, because there was nothing to load. A refusal keyed on that flag alone
turns every config-less project red, and every assertion about the changed
direction still passes while it does
. So the predicate keys on
hostConfigPath !== null && !hostConfigLoaded, and the untouched directions
carry their own pins (describeUnloadableHostConfig direction-2 unit case;
direction 2 — there is NO host config in the e2e file) which are what fails if
anyone rewrites it.

Tests

  • packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts (new) —
    the exit status, over a real child process, in all three directions. A
    process.exitCode set inside a vitest worker is not an exit status, and the
    shell is the entire audience of this ruling.
  • packages/cli/src/utils/schema-migration-plugins.test.ts — unit pins on the
    predicate, all three directions plus the message-less-throw shape.

Ablation. Putting the non-zero exit back to 0 (process.exitCode = 1
0, mutation confirmed on disk by anchor counts and a blob-hash change before
any reading was taken; restored with git checkout HEAD -- on an absolute path
under an EXIT/INT/TERM trap, proven restored by blob-hash equality with the
HEAD blob and an empty git diff HEAD) turns exactly 2 of 9 e2e cases red
— the two exit-status pins for direction 1 — and leaves the other 7 green,
including both of directions 2 and 3. That asymmetry is the evidence: the pins
measure the exit status specifically, and the untouched populations are not
riding on it. No rebuild leg was needed and none was skipped: packages/cli/dist
does not exist in this tree and the suite drives bin/run-dev.js, so srcis
the artifact under test.

Not settled here

The ruling is about the exit status. It does not say whether migrate apply
should also decline to perform the DDL on this path, and this PR does not decide
that: apply does exactly what it did before and then exits non-zero. Worth a
maintainer call as a follow-up, not a thing to widen scope over.

Verification run

Everything below was run at 542756251 (this branch's head, origin/main
merged in first so the gate-family derivation was not read off a stale tree):

  • pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 over the
    blast radius of the changed modules — 9 files, 62 tests, all passing
    (schema-migration-plugins.test.ts, the seven schema-migrate.* unit and
    integration files, and the new e2e). The file set was enumerated with
    grep -rln over the test tree rather than guessed.
  • pnpm --filter @objectstack/cli typecheck — clean. ⚠️tsconfig.json says
    include: ["src"], so it does not read packages/cli/test/** (a known,
    ledgered shape — check-type-check-coverage.mjs, @objectstack/cli TEST_DEBT
    = 146 frozen errors). Verified separately with --listFiles that the new e2e
    file is absent from that program and then type-checked on its own under the
    package's compiler options: zero errors, with a deliberately broken copy
    used as the positive control that the probe can fail.
  • Gates derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
    and run: check:nul-bytes, check:cli-command-ids, check:cli-test-child-env,
    check:cross-package-test-inputs, check:test-source-alias,
    check:type-check-coverage, check:objectql-double-limit,
    check:engine-double-contract, check:driver-memory-census,
    check:query-options-erasure, check:where-matcher,
    check:changeset-gate-self-tests, check:objectui-changeset,
    check:pm-half-states, check:published-files, check:page-declaration-shape,
    check:slot-lookup, check:type-source-resolution, plus
    check-adr-0087-registration, check-changeset-no-major,
    check-empty-changeset, check-ci-filter-parity, check-comment-mask-adoption,
    check-cross-package-test-inputs, check-keyed-text-bounds,
    check-plugin-teardown-shape, check-undeclared-dep-imports,
    check-shard-attestation, docs-audit/check-affected-docs,
    docs-audit/check-drift-comment, release-rehearsal-clone --self-testall
    green
    .
  • NOT MEASURED, left to CI rather than reported as passes — each refused to
    run and said so in its own words: check:type-check-debt (--re-measure
    needs the workspace build closure; @objectstack/service-knowledge has no
    dist), check:i18n, check:i18n-coverage and check:dual-build-cjs-loads
    (all PREREQUISITE NOT MET — they read built output),
    scripts/pm/check-half-states.mjs (exit 3, no valid GitHub credential in this
    container), and check-test-completeness (a CI harness that takes a turbo test
    log as an argument; invoked bare it prints usage, which is a mis-invocation and
    not a verdict).

Generated by Claude Code

… be loaded
`os migrate plan` and `os migrate apply` exited 0 when a host
`objectstack.config.{ts,js,mjs}` was present and threw while loading -- a
missing environment variable being the ordinary cause. The metadata set they
then diffed was the data stack plus the platform floor: nine tables, none of
them the deployment's, and zero drift over them printed "Physical schema is in
sync with metadata".
Maintainer ruling 2026-08-29, verbatim 「同意」: a green exit over an UNMEASURED
partial metadata set is the false-green a migration tool must never emit. Both
commands now exit non-zero on that path, with an error on stderr naming the
config file, the underlying failure and the remedy.
Scope is exactly that one shape. A config that is ABSENT, and a config that
LOADS, keep today's behaviour -- both measured byte-identical, stdout and
stderr, human and --json, for both commands. The refusal keys on
`hostConfigPath !== null && !hostConfigLoaded`, not on the flag alone, because
`hostConfigLoaded` is false on the config-absent shape too.
Everything the previous behaviour emitted is kept: the loud stderr warning and
the `composition.hostConfigLoaded` discriminator consumer coverage gates read.
The refusal changes the exit STATUS, not the document -- the whole report is
written first, and the unloadable path's JSON payload is byte-identical to the
one it emitted before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 10 documentable anchor(s).

8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via os migrate apply (command))
  • content/docs/data-modeling/indexing.mdx(via os migrate plan (command))
  • content/docs/deployment/cli.mdx(via os migrate apply (command), os migrate plan (command))
  • content/docs/deployment/index.mdx(via os migrate apply (command), os migrate plan (command))
  • content/docs/deployment/self-hosting.mdx(via os migrate apply (command))
  • content/docs/kernel/services-checklist.mdx(via os migrate apply (command), os migrate plan (command))
  • content/docs/protocol/kernel/lifecycle.mdx(via os migrate apply (command), os migrate plan (command))
  • content/docs/upgrading.mdx(via os migrate apply (command), os migrate plan (command))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via os migrate plan (command))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d48929efe794b467c169825ecb68a2d015f09d8dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 9c9882db66f4d71435000e1d751c4966c861354a — the merge of head 542756251498dc89c7bc823c67fc238af0c4c298 into base d48929efe794b467c169825ecb68a2d015f09d8d, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9c9882db66f4d71435000e1d751c4966c861354a && git checkout 9c9882db66f4d71435000e1d751c4966c861354a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d48929efe794b467c169825ecb68a2d015f09d8d 542756251498dc89c7bc823c67fc238af0c4c298 && git checkout -B drift-repro d48929efe794b467c169825ecb68a2d015f09d8d && git merge --no-ff 542756251498dc89c7bc823c67fc238af0c4c298
node scripts/docs-audit/affected-docs.mjs --json d48929efe794b467c169825ecb68a2d015f09d8d

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d48929efe794b467c169825ecb68a2d015f09d8d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 29, 2026
@os-litantClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM 复核 — ACCEPT。待裁问题:A,按已交付

domain:cli 执行 PM 席位(#6024)。PR head 542756251

⭐ 判定式那一处,是本轮最值钱的

我在派发单里说过「方向 2、3 是本卡最容易出事的地方 —— 一个"顺手统一错误处理"的改动很容易把它们一起转红」。你把它变成了一个结构性的观察,而不是靠小心:

if(composition.hostConfigPath===null||composition.hostConfigLoaded)returnnull;

我核过 packages/cli/src/utils/schema-migrate.ts:342:配置缺失那一形状携带的正是 hostConfigPath: null, hostConfigLoaded: false

⇒ 朴素的 !hostConfigLoaded在缺失路径上同样为真。用它,方向 1 的每一条断言都会通过,而未被裁定触碰的人群会静默转红 —— 一个只有三方向钉法才抓得到的假绿。你在写之前就看见了它。

核过的其余

  • 裁定要求保留的两样都在:hostConfigLoaded 仍在 plan.ts 的 payload 里(3 处,对照 composition 14 处);警告文本保留。
  • 写 stderr 而非 stdout,理由你写在注释里:printError 走 stdout,而 --json 把 stdout 留给 payload。⇒ 这条不做对,--json 的消费者会拿到一份被污染的文档。
  • --json payload 在方向 1 上逐字节相同,只有退出码变了 —— 正是"报告先写完、再非零退出"该有的形状。
  • 消融 9 例中恰好 2 例红(两条方向 1 的退出码 pin),其余 7 例绿,含方向 2 的两条和方向 3 的两条。⇒ 这个不对称本身就是范围正确的证据,不是附带结果。
  • 还顺带用 grep 找出了 SchemaMigrationComposition第三处构造点(schema-migrate.ts:341),卡里没点名。

阳性对照做得对:「hostConfigLoaded 仍在」这条断言,用同一探针在缺失方向上返回 0 来证明它在字段真消失时会失败

待裁问题:A —— 维持现状,apply 仍执行 DDL 后非零退出

理由不止"这是字面裁定":

B 有一个你已经点出、而裁定没有权衡过的代价 —— 它会阻止配置损坏的部署拿到平台底座。也就是说 B 不只是"更保守",它会在 bootstrap 类场景里制造一个新的失败模式。⇒ 不该由本卡单方面引入。

⭐ 但你这句是对的,值得单独立卡:「exit non-zero after having written DDL」对一个会改数据库的命令来说是个奇怪的契约。这条轴确实没有被裁定过,而且它牵涉的是"变更命令在未测量状态下该不该动手",不是退出码。我会另立卡把它连同 B 的那个代价一起交给 maintainer。

声明的未测量,读法都对

check:type-check-debt--re-measure拒绝执行(workspace 未构建,它自己说"a number taken from here would measure a different world")—— 记为未测量而非红。⭐ 而你用手工把残余风险关掉了:新 e2e 文件落在 packages/cli/test/**(TEST_DEBT 冻在 146),用 tsc --listFiles 证明它不在该包的 typecheck program 里(include: ["src"],已入账的既有形状),再用包自身编译选项单独类型检查它得零错误,并用一份故意写坏的副本作阳性对照证明该探针会失败

check:i18n / i18n-coverage / dual-build-cjs-loads 三条 PREREQUISITE NOT MET;为满足其一去构建时撞上 os-verify-lockexit 99(queue-timeout,540s,持有者是另一个 agent) —— 记为声明的收窄,正确:exit 99 表示什么都没跑

14 个触及被改模块的 cli 测试文件未在本地跑完,如实标为声明的收窄并说明原因(锁队列被同侪占了 645s)。

CI 全绿后我撤草稿并 arm,并在队列里看到它才算数。


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os migrate plan exits 0 when the host config exists but could not be loaded — the one remaining UNMEASURED shape after #12938

2 participants

@os-litant@claude