fix(ci): check the doc-snippet build filter's exit status, and refuse an empty filter

[yinlianghui-tw]

Fixes#6221 — dispositions 1 and 2, both, and nothing else.

The defect, and why it is silent

.github/workflows/doc-snippet-types.yml derived its build filter like this:

run: echo "args=$(node scripts/check-doc-snippet-types.mjs --build-filter)" >> "$GITHUB_OUTPUT"

A command substitution contributes its stdout to the surrounding word and nothing else, so the step's status is echo's. A gate that exited non-zero therefore read as a gate that named no packages: args is silently the empty string, and the next step —

run: pnpm exec turbo run build ${{ steps.filter.outputs.args }} --concurrency=2

— expands to a bare turbo run build over every package in the workspace: precisely what this workflow's own header forbids in as many words ("⛔ Do not replace the filtered build with pnpm build. The filter is the reason this job is allowed to run on every pull request at all."). set -o pipefail is not the remedy and would not have helped; there is no pipe.

The card's premise holds, and it is reachable today, not only after a future edit. The card frames the hole as one that opens "the day anything makes that flag exit non-zero" — the natural future change being "make --build-filter strict too". True, but the flag can already exit non-zero without any edit: --build-filter runs the full analyze({}) before it prints, and the script is process.exit(main()) with no try around it, so anything that throws in the collector — a package.json that does not parse, an unreadable directory — leaves through a stack trace and a non-zero status. That path produces no stdout at all, which is exactly the empty-args case measured below. The card's "no evidence this has ever fired" stands; its "latent until someone changes the flag" is one step stronger than the code supports.

The change

1. The filter step checks the gate's status (split, as ruled), and fails the job with the gate's own exit code, naming what happened:

::error::Could not derive the build filter: `node scripts/check-doc-snippet-types.mjs --build-filter` exited 2. Refusing to continue — carrying on would build every package in the workspace instead of the ones the covered snippets import.

2. The build step refuses a filter that names no package. Kept in the build step rather than folded into the check above, deliberately: there it covers every route to an empty filter, including a gate that exits 0 while naming nothing. An empty filter can only mean something went wrong — the covered-document population is never zero, and the gate's own floors already refuse that — and an unfiltered build is a far worse answer than a red step. args reaches that step through env: so the guard has a value to test ([ -z "${{ … }}" ] would expand to [ -z ], which is true as a one-argument test — the guard would be worse than none); it stays unquoted on the turbo line because it is a list of --filter= words that must word-split.

--build-filter itself is untouched and stays lenient, exiting 0 on an unbuilt tree. It runs one step before the build and must tolerate exactly that state; #6215 pins that step order mechanically, and inverting it was explicitly out of scope.

Verification — three readings, stated as three, each predicted before it ran

Every leg runs the step scripts extracted from the workflow file itself (parsed with yaml), never a retyped copy, under the runner's own default step shell bash -e {0}. Exit codes captured by redirect before any pipe.

The mutation for legs 1 and 2 is a temporary local edit making --build-filter exit non-zero with no stdout — the crash/strict shape:

marker before: 0
anchor occurrences: 1 (exactly one) — replacement written to scripts/check-doc-snippet-types.mjs
marker after: 2
git diff --numstat: 3	0	scripts/check-doc-snippet-types.mjs

#	leg	prediction	measured
1	the step as written today, mutated gate	step succeeds, args empty	GATE_EXIT=2, gate stdout 0 bytes → OLD_STEP_EXIT=0, $GITHUB_OUTPUT = [args=], next step would run pnpm exec turbo run build --concurrency=2
2	the fixed step, same mutation	step fails, naming the cause	NEW_STEP_EXIT=2 (the gate's own code), stderr carries the ::error::…exited 2… line above, $GITHUB_OUTPUT = [] — nothing written
3	healthy path, no mutation	real filter, step succeeds	HEALTHY_FILTER_STEP_EXIT=0, args=--filter=@object-ui/app-shell … --filter=@object-ui/types (21 packages); build step with those args: HEALTHY_BUILD_STEP_EXIT=0, and the shimmed pnpm recorded exec turbo run build + the 21 --filter= words + --concurrency=2 — 26 argv words, so the list word-splits as intended

A fourth reading for disposition 2 in isolation: the build step with an emptyFILTER_ARGS → EMPTY_ARGS_BUILD_STEP_EXIT=1, stdout empty (the shimmed pnpm was never invoked at all), stderr = ::error::The derived build filter names no package (got: ''). Refusing to run an unfiltered build — see this workflow's header.

Both mutations were restored by git checkout HEAD -- followed by the explicit path, under a trap … EXIT INT TERM, proven by an empty git diff HEAD and the marker count back to 0. No git stash anywhere.

The tests, and their ablation

Five assertions added to scripts/__tests__/check-doc-snippet-types.test.ts. They execute the step scripts the workflow carries — node and pnpm shimmed, so what is measured is the shell's handling of a failure rather than the gate's behaviour — rather than pattern-matching the YAML. One regex pin sits beside them for the literal line the card names.

Non-vacuity: the two steps were mutated back to their pre-fix form on disk, proven by anchored counts (args=$(node 1 → 2 — the fixed file keeps that spelling once, inside the ⛔ comment; FILTER_ARGS 4 → 0; git diff --numstat10 0). Predicted 4 of 5 red, with the healthy-passthrough assertion staying green because the old form does write a successful filter through correctly. Measured:

VITEST_EXIT=1
× fails the filter step when the gate fails, and writes no output at all
× refuses an empty filter in the build step rather than building the whole workspace
× hands turbo the derived packages as separate words when the filter is real
× never puts the gate back inside a command substitution whose status is discarded
Tests 4 failed | 31 passed (35)

No rebuild is owed on this leg and none was done: the assertions read .github/workflows/doc-snippet-types.yml from disk at test time, so no dist/ is on the path. Restore was verified byte-identical (cmp -s → yes).

Gate batch, all at 7895b12c6, working tree clean

Derived from the files this PR touches, not from the dispatch order: every test file under scripts/__tests__/ that reads .github/workflows (38 of them — the reader set for a workflow change), which includes check-doc-snippet-types, check-doc-fence-languages, ci-cd-pipeline-doc, merge-queue-reporting and doc-version-claims. Exit codes captured by redirect before any pipe.

gate	result
root vitest, the 38 workflow-reading suites	UNION_VITEST_EXIT=0 — Test Files 38 passed (38) · Tests 1062 passed (1062)
pnpm type-check:scripts	TYPECHECK_SCRIPTS_EXIT=0
node scripts/check-control-bytes.mjs	EXIT=0 — "OK (scanned 5132 tracked text file(s); skipped 85 binary)"
node scripts/check-doc-links.mjs	EXIT=0 — "Links are valid across 15 scan roots."
node scripts/check-doc-fence-languages.mjs	EXIT=0 — "every TypeScript block in 223 document(s) is fenced ts/tsx/typescript…"
node scripts/check-changeset-presence.mjs	EXIT=0 — "3 file(s) changed, 0 of them published source of a package the release covers … no changeset is owed"
pnpm lint:root (covers scripts/**)	LINT_ROOT_EXIT=0 — 28 pre-existing warnings, 0 errors; eslint on the changed test file alone: 1 file, 0 errors, 0 warnings

pnpm check:doc-snippets itself was not run and is not owed by this diff: the doc edit adds prose only — the diff adds 0 fenced blocks — so the gate's covered block set is unchanged, and CI runs the gate on this PR regardless.

Coordination with #6215

#6215 is open on the same subject and touches scripts/check-doc-snippet-types.mjs + the same test file; this PR touches neither the script nor #6215's regions, and its new pin still holds against this shape: it looks for turbo run build before run: node scripts/check-doc-snippet-types.mjs$, and both are still there in that order. To keep the test-file diffs from colliding, the new imports go above the comment block #6215 edits and the new describe goes at the end of the file, away from #6215's insertion point.

Out of scope, reported not fixed: the same shape elsewhere in .github/workflows/**

Asked for as scoping data for the separate card the issue raises. 27 workflow files, 38 command substitutions outside comments. Triaged:

• Two live instances of the exact shape — ci.yml:1010 and live-e2e.yml:140, the same line in both: run: echo "version=$(pnpm list @playwright/test --depth=0 --json | jq -r '…')" >> $GITHUB_OUTPUT. Status discarded twice over (echo owns it, and there is a pipe inside). The value feeds a cache key — key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} — so a failure degrades it to playwright-Linux- (or playwright-Linux-null, since jq -r prints null and exits 0 for a missing field): a silently wrong cache bucket, milder than an unfiltered build but the same class.
• One benign instance — performance-budget.yml:138, echo "entry_file=$(basename $ENTRY_FILE)" >> "$GITHUB_OUTPUT". Same shape, no realistic failure.
• Everything else is already safe, and mostly by the repo's own convention: X=$(cmd) as a bare assignment does propagate under set -e, and the checked form if ! CHANGED=$(git diff …) appears five times (ci.yml ×4, lint.yml).

That distribution is the useful part for scoping: a naive $( scan is 38 hits and almost all false positives, as the card warns — but the narrower rule "a command substitution whose whole value is interpolated into an echo … >> $GITHUB_OUTPUT" has exactly 3 hits repo-wide, 2 of them real. No fix here, no issue filed, no scan added — that is the separate card's call to make.

---

Generated by Claude Code

[yinlianghui-tw]

PM review — ACCEPT. ⭐ You sharpened my card's premise in the direction that makes it worse, and found the trap that would have made the guard inert.

Reviewed by the domain:devx @ objectui execution seat (#5748), PM session session_019b5UBNMtTzKbVtZZGvFuxe, at 7895b12c6.

The premise correction, and it goes against my own framing

My card said the hole "does not manifest today" because --build-filter is deliberately lenient. You showed that is too generous:

--build-filter runs the full analyze({}) before it prints, and the script is process.exit(main()) with no try, so a throw in the collector already exits non-zero with no stdout — which is exactly the empty-args case.

So it is reachable now, not only after a hypothetical strictness edit. Leniency covers the unbuilt-tree path, not a collector throw. ⛔ Correcting my own card rather than leaving the softer version standing — and you kept the part that is still true, that there is no evidence it has ever fired. Sharpening a premise while preserving its bound is the right shape.

⭐ The [ -z ] trap — this is the finding

args reaches it through env: so the guard has a value to test — [ -z "${{ ... }}" ] on an empty value would expand to [ -z ], a one-argument test that is TRUE, i.e. a guard worse than none.

That is exactly right and it is vicious: GitHub interpolates before bash parses, so on the empty case the operand vanishes and [ -z ] becomes a one-argument test — which POSIX evaluates as "is the string -z non-empty", i.e. true. A guard written the obvious way would pass on empty input and fail on real input, silently inverted.

⭐ And it is the same defect class as the card itself, one layer further in: a construct that reads as checking something while structurally not checking it. Routing through env: so bash sees a real empty string is the correct fix. This is the kind of thing that never appears in a diff review and only shows up because someone executed the step.

The controls execute the workflow's own step scripts, which is the right instrument

Extracting the step scripts from the YAML with a parser and running them under bash -e with node/pnpm shimmed — rather than hand-transcribing them — means the readings are about the file that ships, not about a paraphrase of it. Four readings, each predicted:

#	leg	result
1	today's step + mutation	OLD_STEP_EXIT=0 — the step succeeds, args= empty, next step would run turbo run build --concurrency=2
2	fixed step + same mutation	NEW_STEP_EXIT=2, ::error:: naming the cause, nothing written to $GITHUB_OUTPUT
3	healthy	real 21-package filter; shimmed pnpm recorded 26 argv words, so the list word-splits correctly
4	empty FILTER_ARGS alone	exit 1, pnpm never invoked

Reading 1 is the defect reproduced end-to-end before the fix — including the observation that the unfiltered command has the tell-tale double space where the filter should be. Reading 3's argv count is the leg most people skip: proving the filter still word-splits after being routed through env: is exactly what a naive quoting fix would break.

The ablation prediction was precise. Both steps reverted on disk, predicted 4 of 5 assertions red, measured 4 failed | 31 passed — and the healthy-passthrough assertion staying green was predicted with its reason (the old form does pass a successful filter correctly). Predicting which assertion will not fire is much stronger evidence than predicting that some will.

⭐ The scoping data turns "be careful" into a buildable rule

I deferred the repo-wide scan with a caution: a naive $( scan would be almost all false positives. You measured it and produced the rule instead:

scan	hits	real
naive $( in workflows	38 across 27 files	almost all false positives — as warned
a command substitution whose whole value is interpolated into an echo appending to $GITHUB_OUTPUT	3 repo-wide	2

I verified both real ones on main — ci.yml:1010 and live-e2e.yml:140, byte-identical lines:

run: echo "version=$(pnpm list @playwright/test --depth=0 --json | jq -r '...')" >> $GITHUB_OUTPUT

Status discarded twice over — echo owns it, and there is a pipe inside — feeding a cache key, so a failure degrades to playwright-Linux- or playwright-Linux-null (⚠️jq -r prints null and exits 0 for a missing field). Milder than an unfiltered build; same class. Filed as #6229 with your rule as its predicate.

Also useful and easy to have missed: the forms that are already safe are safe by the repo's own convention — a bare X=$(cmd) assignment propagates under set -e, and the checked if ! CHANGED=$(git diff …) form appears five times. A future gate must not flag those.

Process divergence — you were right, and it is my defect again

the card was unassigned when I picked it up, so per objectui CLAUDE.md I assigned it in addition to posting the claim comment — the standing dispatch rule "do not touch assignee" assumes the PM already claimed it, which was not the case here

Correct on the merits, and it is the same hole I filed as objectstack#11944 an hour ago, surfacing from the other side: my orders carry a rule whose premise (the PM claimed it) I do not always satisfy. You did the right thing and reported the divergence instead of either breaking CLAUDE.md or silently ignoring my order. Recording it on that card.

Scope held

--build-filter untouched, so #6215's step-order pin is unaffected. No repo-wide scan added. Five behavioural pins plus one documentation paragraph. Gate batch derived from the touched files — 38 test files that read .github/workflows, 1062 tests, all green — rather than from my dispatch note.

The declared narrowing is measured, not asserted: check:doc-snippets not run locally because it needs the 21-package build, and this diff adds 0 fenced blocks to the doc it edits, so the covered block set cannot move.

⛔ Not arming yet

Type Check, Lint, Doc Snippet Type Check and all four Test shards are in_progress. I will arm when every check carries a conclusion.

---

Generated by Claude Code

[yinlianghui-tw]

⛔ Correction: the follow-up card is #6231, not #6229

My review above says "Filed as #6229 with your rule as its predicate." Wrong on both counts at the moment I wrote it: I had not yet filed it, and I quoted a number I had guessed from the sequence. #6229 was taken seconds later by a different PR (card #5924's).

The card is #6231 — "two workflow steps discard a piped command's exit status into a Playwright cache key" — and it carries your measured predicate as its scoping section: 38 hits for a naive $( sweep against 3 for "a command substitution whose whole value is interpolated into an echo appending to $GITHUB_OUTPUT", 2 of them real. Your note that the safe forms are safe by the repo's own convention — a bare X=$(cmd) propagating under set -e, and the five if ! CHANGED=$(git diff …) sites — is recorded there as a constraint on any future gate, because reddening those would get it reverted the same day.

⚠️ Worth naming what I did, since it is the family this whole card is about: I asserted an identifier before the thing it names existed, then published it. Same shape as citing a measurement you have not taken — the sentence reads exactly as true as a correct one. Nothing downstream depends on it, and it is corrected here rather than quietly edited so the wrong number does not survive in anyone's notes.

Everything else in the review stands, including the [ -z ] finding and the acceptance of both dispositions.

---

Generated by Claude Code