From 7895b12c6b1c08ebf6e4fe2c06fde817694330c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:44:22 +0000 Subject: [PATCH] fix(ci): check the build filter's exit status, and refuse an empty filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `doc-snippet-types.yml` derived its build filter with `echo "args=$(node …--build-filter)" >> "$GITHUB_OUTPUT"`. A command substitution contributes its stdout and nothing else, so the step's status was `echo`'s: a gate that exited non-zero read as a gate that named no packages, `args` was silently empty, and the next step expanded to a bare `turbo run build` over the whole workspace — the one thing this workflow's header forbids, with no signal anywhere. The filter step now captures the gate's status and fails the job with it. The build step separately refuses a filter that names no package, which covers every other route to an empty filter. `--build-filter` itself is unchanged and stays lenient on an unbuilt tree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe --- .github/workflows/doc-snippet-types.yml | 40 ++++++- content/docs/guide/ci-cd-pipeline.md | 7 ++ .../__tests__/check-doc-snippet-types.test.ts | 107 ++++++++++++++++++ 3 files changed, 152 insertions(+), 2 deletions(-) diff --git a/.github/workflows/doc-snippet-types.yml b/.github/workflows/doc-snippet-types.yml index 69adb6a34b..f8bcfb8dfc 100644 --- a/.github/workflows/doc-snippet-types.yml +++ b/.github/workflows/doc-snippet-types.yml @@ -85,12 +85,48 @@ jobs: # covered documents import. A package the snippets need but nothing built # is reported by the gate as `unbuilt-package` — its own failure reason, # never as a page full of broken imports. + # + # ⛔ Do not fold this back into `echo "args=$(node …)" >> "$GITHUB_OUTPUT"` + # (objectui#6221). A command substitution contributes its STDOUT to the + # surrounding word and nothing else — the step's status is `echo`'s — so a + # gate that failed reads as a gate that named no packages, `args` is + # silently empty, and the step below expands to a bare `turbo run build` + # over the whole workspace: the one thing this workflow's header forbids, + # with no signal anywhere. `set -o pipefail` is not the remedy and would + # not help; there is no pipe here. Capture the status, then write the + # output. - name: Derive the packages the covered snippets import id: filter - run: echo "args=$(node scripts/check-doc-snippet-types.mjs --build-filter)" >> "$GITHUB_OUTPUT" + run: | + status=0 + args="$(node scripts/check-doc-snippet-types.mjs --build-filter)" || status=$? + if [ "$status" -ne 0 ]; then + echo "::error::Could not derive the build filter: \`node scripts/check-doc-snippet-types.mjs --build-filter\` exited $status. Refusing to continue — carrying on would build every package in the workspace instead of the ones the covered snippets import." >&2 + exit "$status" + fi + echo "args=$args" >> "$GITHUB_OUTPUT" + # The empty-filter refusal is the second half, deliberately kept HERE + # rather than beside the status check above: it holds for every route to + # an empty filter, including a gate that exits 0 while naming nothing. + # An empty filter can only ever mean something went wrong — the covered + # document population is never zero and the gate's own floors already + # refuse that — and an unfiltered `turbo run build` is a far worse answer + # than a red step. `args` arrives through the environment so the check has + # a value to test; it stays unquoted on the `turbo` line because it is a + # LIST of `--filter=` words that must word-split. - name: Build those packages - run: pnpm exec turbo run build ${{ steps.filter.outputs.args }} --concurrency=2 + env: + FILTER_ARGS: ${{ steps.filter.outputs.args }} + run: | + case "$FILTER_ARGS" in + *--filter=*) ;; + *) + echo "::error::The derived build filter names no package (got: '$FILTER_ARGS'). Refusing to run an unfiltered build — see this workflow's header." >&2 + exit 1 + ;; + esac + pnpm exec turbo run build $FILTER_ARGS --concurrency=2 - name: Compile documentation snippets against the built types run: node scripts/check-doc-snippet-types.mjs diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index 6e70bb68ea..b00901086e 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -652,6 +652,13 @@ per-PR full-repo build the 2026-08-16 ruling on [#4846](https://github.com/objectstack-ai/objectui/issues/4846) rejected; see *Published Dist Gate* below. +**And the filter is checked, twice.** The step that derives it fails the job if the gate exits +non-zero, and the build step refuses a filter that names no package +([#6221](https://github.com/objectstack-ai/objectui/issues/6221)). Written the obvious way — +`echo "args=$(node …)" >> "$GITHUB_OUTPUT"` — the step's status is `echo`'s, so a gate that failed +would read as a gate that named nothing, and `turbo run build` with no filter is the whole-workspace +build this section just said the job must never run. + **Fragments are declared, never guessed.** Documentation legitimately carries partial snippets, so a block that is not meant to compile carries a marker line immediately above its fence with a written reason — `{/* doc-snippet: fragment - why */}` in `.mdx`, the HTML-comment form in `.md`. A block diff --git a/scripts/__tests__/check-doc-snippet-types.test.ts b/scripts/__tests__/check-doc-snippet-types.test.ts index 571f20a73a..0786091478 100644 --- a/scripts/__tests__/check-doc-snippet-types.test.ts +++ b/scripts/__tests__/check-doc-snippet-types.test.ts @@ -3,6 +3,8 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { spawnSync } from 'node:child_process'; +import { parse as parseYaml } from 'yaml'; // Plain-JS CI helper; its types are inferred from the .mjs source by // `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here. @@ -412,3 +414,108 @@ describe('wiring — a script nothing runs is not a gate', () => { expect(pkg.scripts['check:doc-snippets']).toBe(`node ${SCRIPT}`); }); }); + +/** + * The step's own shell, executed — objectui#6221. + * + * The defect this pins was not in the gate but in the SHELL wrapped around it: + * `echo "args=$(node …)" >> "$GITHUB_OUTPUT"` gives the step `echo`'s status, so + * a gate that exited non-zero read as a gate that named no packages, and the + * build step below it expanded to a bare `turbo run build` over the whole + * workspace — the one thing this workflow's header forbids, with no signal + * anywhere. A regex over the YAML would pin the letter of the fix; these run the + * step scripts the workflow actually carries, under the runner's own default + * shell (`bash -e {0}`), with `node` and `pnpm` shimmed so that what is measured + * is the shell's handling of a failure rather than the gate's behaviour. + */ +describe('the build-filter steps propagate failure instead of silently building everything (objectui#6221)', () => { + const stepScript = (name: string): string => { + const workflow = parseYaml(fs.readFileSync(path.join(repoRoot, '.github/workflows/doc-snippet-types.yml'), 'utf8')) as { + jobs: Record; + }; + const step = Object.values(workflow.jobs) + .flatMap((job) => job.steps ?? []) + .find((s) => s.name === name); + expect(step?.run, `doc-snippet-types.yml must keep a step named "${name}" with a run: script`).toBeTypeOf( + 'string', + ); + return step!.run!; + }; + + /** Run a step's `run:` script as the runner does, with the named executables shimmed. */ + const runStep = (script: string, shims: Record, env: Record = {}) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'doc-snippet-step-')); + const bin = path.join(dir, 'bin'); + fs.mkdirSync(bin); + for (const [name, body] of Object.entries(shims)) { + fs.writeFileSync(path.join(bin, name), body); + fs.chmodSync(path.join(bin, name), 0o755); + } + const scriptPath = path.join(dir, 'step.sh'); + fs.writeFileSync(scriptPath, script); + const githubOutput = path.join(dir, 'github_output'); + fs.writeFileSync(githubOutput, ''); + const turboArgv = path.join(dir, 'turbo_argv'); + // `bash -e {0}` is the default shell for a `run:` step on a Linux runner. + const proc = spawnSync('bash', ['-e', scriptPath], { + encoding: 'utf8', + env: { ...process.env, PATH: `${bin}:${process.env.PATH}`, GITHUB_OUTPUT: githubOutput, TURBO_ARGV: turboArgv, ...env }, + }); + return { + status: proc.status, + stderr: proc.stderr, + githubOutput: fs.readFileSync(githubOutput, 'utf8'), + turboInvoked: fs.existsSync(turboArgv), + turboArgv: fs.existsSync(turboArgv) ? fs.readFileSync(turboArgv, 'utf8').trim() : null, + }; + }; + + const failingGate = '#!/usr/bin/env bash\necho "the filter could not be derived" >&2\nexit 3\n'; + const healthyGate = '#!/usr/bin/env bash\necho "--filter=@object-ui/core --filter=@object-ui/react"\n'; + const recordingPnpm = '#!/usr/bin/env bash\necho "$*" > "$TURBO_ARGV"\n'; + + it('fails the filter step when the gate fails, and writes no output at all', () => { + const result = runStep(stepScript('Derive the packages the covered snippets import'), { node: failingGate }); + expect(result.status, 'a failed filter must not read as a successful step').not.toBe(0); + expect(result.stderr, 'the step must say what failed, not just fail').toContain('--build-filter'); + expect(result.stderr, 'a failure the log does not annotate is a failure someone has to go looking for').toContain( + '::error::', + ); + expect( + result.githubOutput, + 'an `args=` line written after a failed gate is the empty filter that becomes an unfiltered build', + ).toBe(''); + }); + + it('writes the derived filter through unchanged when the gate succeeds', () => { + const result = runStep(stepScript('Derive the packages the covered snippets import'), { node: healthyGate }); + expect(result.status).toBe(0); + expect(result.githubOutput.trim()).toBe('args=--filter=@object-ui/core --filter=@object-ui/react'); + }); + + it('refuses an empty filter in the build step rather than building the whole workspace', () => { + const result = runStep(stepScript('Build those packages'), { pnpm: recordingPnpm }, { FILTER_ARGS: '' }); + expect(result.status, 'an empty filter can only mean something upstream went wrong').not.toBe(0); + expect(result.turboInvoked, 'turbo must not run at all on an empty filter').toBe(false); + expect(result.stderr, 'the refusal must be the step\'s own, not a shell error that happens to mention a filter').toContain( + '::error::', + ); + }); + + it('hands turbo the derived packages as separate words when the filter is real', () => { + const result = runStep(stepScript('Build those packages'), { pnpm: recordingPnpm }, { + FILTER_ARGS: '--filter=@object-ui/core --filter=@object-ui/react', + }); + expect(result.status).toBe(0); + expect(result.turboArgv).toBe( + 'exec turbo run build --filter=@object-ui/core --filter=@object-ui/react --concurrency=2', + ); + }); + + it('never puts the gate back inside a command substitution whose status is discarded', () => { + expect( + stepScript('Derive the packages the covered snippets import'), + 'the step status would be `echo`\'s again, and a failed gate would read as an empty filter', + ).not.toMatch(/echo\s+"?args=\$\(/); + }); +});