Skip to content

test(cli): pin the conversions-only exit-code cell of os validate --json --strict - #11609

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11301-conversions-only-exit-pin
Aug 24, 2026
Merged

test(cli): pin the conversions-only exit-code cell of os validate --json --strict#11609
os-zhuang merged 1 commit into
mainfrom
claude/issue-11301-conversions-only-exit-pin

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#11301

Test-only. Pins the one documented exit-code cell of os validate --json --strict that no fixture exercised.

The unpinned cell

--strict gates on the text face's warning list, which folds in the ADR-0087 D2 load-time conversion notices. The JSON payload carries those notices separately, under conversions, and its own warnings field is the five-way spread without them. So a config whose only advisories are conversion notices exits 1 carrying { valid: true, warnings: [], conversions: [...] }.

That is deliberate, and commands/validate.ts states the reasoning at the emitJson call. It was also the one cell where the exit code is decided by a collection absent from the payload field a reader reaches for first — and every existing fixture raises zero conversions, so a regression narrowing the gate back to payload.warnings would have restored the original divergence with the whole suite green.

The behaviour was not on trial here and was not touched: no production file is in this diff.

The fixture is a minimal pair, not a single input

Both members come from one template, headerPageSource(key), and differ in a single key on one page:header component:

fixture--json--json --strict--strict (text)validwarningsconversions
description (the live page-header-subtitle-alias window)011true[]1 notice
subtitle (canonical spelling)000true[][]

So the pin discriminates on the conversion, not on "a page is present". Both rows were measured before the test was written.

The issue's suggested candidate, object-compactLayout-to-highlightFields, turned out to be unusable and the test comment records why: it is already retiredFromLoadPath, so the schema tombstones the key and it raises a validation error, not a notice. page-header-subtitle-alias was selected from the 17 entries that are still live on the load path, and measured to raise no other advisory.

Anti-vacuity

conversions is asserted non-empty. page-header-subtitle-alias retires from the load path at protocol 18; on that day the fixture raises nothing, and without that assertion the file would keep passing while pinning an empty cell — exactly the failure this card exists to remove. It goes red instead, with a message naming the remedy.

Discrimination ablation (trap-restored, mutation proven on disk by grepping both the injected and the removed text, prediction recorded before the run):

  • mutation: headerPageSource('description')headerPageSource('subtitle') in beforeAll, making the fixture byte-identical to its canonical twin. Removed-text hits 1 → 0, injected-text hits 0 → 1, sha 7cde9871…391ebbd5….
  • predicted: 1 failed / 5 passed, failing on the floor assertion (expect(text.code).not.toBe(0)) rather than on the conversions guard, since the floor is asserted first in the body.
  • observed, as predicted: Tests 1 failed | 5 passed (6), AssertionError: text --strict must fail on the conversions-only config: expected +0 not to be +0. The canonical control stayed green.
  • restore leg: sha back to 7cde9871…, grep counts back to 1/0, git status clean.

Verification — all at 32ecb986

  • pnpm --filter @objectstack/cli exec vitest run src/commands/validate-json-strict-exit.e2e.test.tsTest Files 1 passed (1) / Tests 6 passed (6)
  • pnpm --filter @objectstack/cli typechecktsc --noEmit, exit 0
  • Gate set derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, not a hand-built list. All 11 matched + convention-triggered families green, including check-i18n-bundles: OK (9 package(s) — all bundles in sync…), check-i18n-coverage: OK (12 config(s), 602 baselined untranslated string(s), none new). and check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured…, none above its recorded number.
  • check:i18n and check:i18n-coverage first refused (PREREQUISITE NOT MET — the workspace CLI is not built, "Nothing was checked"). Recorded as not measured, then measured for real after turbo run build.

No changeset

Nothing user-visible ships: the diff is one packages/cli/src/**/*.test.ts file, which tsconfig.build.json excludes from the published output. skip-changeset applied.

Note on #11599

check:cli-test-child-env is not in this branch's base and does not reach this diff either way — the file lives in packages/cli/src/commands/, not packages/cli/test/**. No baseline entry was added and no existing violating file was touched.


Generated by Claude Code

…json --strict`
The documented cell — a config whose ONLY advisories are ADR-0087 D2 load-time
conversion notices exits 1 with `{ valid: true, warnings: [], conversions: [...] }`
— had never been exercised: every existing fixture raises zero conversions, so a
regression narrowing the gate back to the payload's `warnings` field would have
left the whole suite green.
Adds a minimal pair built from one template, differing in a single key on one
`page:header` component: `description` (the live `page-header-subtitle-alias`
window) against the canonical `subtitle`. The pin therefore discriminates on the
conversion rather than on the presence of a page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32708418190 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (5/6) — 失败步骤: Run this shard's tests

    × with pnpm on PATH: prints pnpm consistently, never bare npm 2396ms
    FAIL src/scaffold-next-steps-pm.test.ts > scaffolder "Next steps" names the package manager it actually detected (#10322) > with pnpm on PATH: prints pnpm consistently, never bare npm
    ↳ 失败原因: AssertionError: expected '\n cd my-app\n npm install\n …' to match /\bpnpm run dev\b/
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • src/scaffold-next-steps-pm.test.ts — 24h 窗口内只有本 PR 撞到过,暂不汇总(再有一个不同 PR 撞到就会自动开汇总 issue)。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 3 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Aug 24, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Queue-failure triage — not this PR's; re-queueing once

Run 32708418190, Test Core (5/6):

FAIL src/scaffold-next-steps-pm.test.ts > ... > with pnpm on PATH: prints pnpm consistently, never bare npm
AssertionError: expected '\n cd my-app\n npm install\n …' to match /\bpnpm run dev\b/

Checklist item 2 — the failing test is not in a package this diff touches. This PR is one file,
packages/cli/src/commands/validate-json-strict-exit.e2e.test.ts; the failing test lives in
packages/create-objectstack/src/scaffold-next-steps-pm.test.ts. No production file is in the diff
at all, so there is no code path by which it changes what create-objectstack prints.

What the assertion actually says. The scaffolder's detectPackageManager() is
execSync('pnpm --version') inside a try, falling back to 'npm' on any throw
(packages/create-objectstack/src/index.ts:125). npm install in the "Next steps" block means that
probe threw — not that the code chose npm.

Why the probe is fragile in the queue, and why it is a pre-existing condition. The test's own
sanity check (command -v pnpm) runs in the worker's cwd, inside the repo; the scaffold child runs
with cwd = a mkdtemp under os.tmpdir(), i.e. /home/runner/work/_temp/…, which has no
package.json above it
. The queue job runs pnpm through Corepack (COREPACK_HOME: /home/runner/work/_temp/corepack is in this job's env, and the "Restore the Corepack store" step is
part of it) — so inside the repo Corepack resolves the pinned packageManager (pnpm@10.31.0) while
outside it must resolve its own default. Reproduced locally: pnpm --version prints 10.31.0 from
the repo root and 10.33.0 from /tmp — two different resolutions from one binary, the outer one
being the leg that can need a fetch. That is the shape of a probe that can throw for reasons that
have nothing to do with the code under test, and it predates this branch (the file landed in #11013).
Filed separately rather than fixed here: it is not this PR's package and not this PR's card.

Consistent with the bot's own signature ledger: no other PR hit this test in the 24h window, and no
summary issue exists. Grouped-queue batching also means the cache miss that made
create-objectstack#test run at all (37 of 39 tasks were cache hits) may well have come from a
sibling PR in the batch, not from this one.

Action: re-queueing once, per the "at most one re-run to confirm" rule. If it reddens again on the
same signature, that is a second data point and I will stop re-queueing and take it to the finding.


Generated by Claude Code

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

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] the conversions-only exit-code cell of os validate --json --strict is documented but untested — every fixture raises zero conversions

2 participants

@os-zhuang@claude