From 3ce1f95722d3fd8aa15ec90d2d79790ba83fd265 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:21:05 -0400 Subject: [PATCH 1/2] refactor(ci): resolve the docs-only scope once and gate the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs-only short-circuit let six jobs each run scripts/check-docs-only.sh as their own `id: scope` step and gate their inner work on `steps.scope.outputs.docs_only != 'true'`. That put the contract at 48 references in one file, every one of them an inverse-polarity test against a string flag. Each consumer had to independently know that the value is the string `true`, that the safe direction is to RUN the suite, and that the negation must be spelled that exact way. Nothing executable held that knowledge; a prose comment did. The asymmetry is what made it worth fixing. A consumer that wrote `== 'false'` where it meant `!= 'true'` would skip its lane whenever the detector emitted anything unexpected — a lane reporting success without running, which is the false-green shape docs/conventions/liveness-assertion/ names. A consumer that forgot the gate entirely would merely run always. A new `scope` job now runs the detector once and publishes `run_full`. The output is positive on purpose: a consumer writes `run_full == 'true'` to do the work and `run_full == 'false'` to report it not applicable. Both are plain equality against a value that `${{ != 'true' }}` can render only as `true` or `false`, so the two forms are exact complements and a new lane has no negation to spell wrong. The fail-safe direction is preserved. The detector already emitted `docs_only=false` for every condition it could resolve but not classify, and follows every emit with an immediate `exit 0`, so a run that cannot complete leaves the flag unset. `run_full` renders an unset flag as `true`, and the full suite runs; the same path covers push events, where the detect step does not run at all. `continue-on-error` on that step contributes the other half — it keeps a failed detector from failing the resolving job and skipping every consumer. The self-test is deliberately NOT `continue-on-error`, so a detector nobody verified still turns the lane red. scripts/check-docs-only-gate.sh asserts nine properties against ci.yml rather than restating them in a comment: the detector is resolved exactly once; the published expression is exactly the fail-closed one; the step that absorbs failure is the step that actually invokes the detector; the self-test is unweakened; every consumer reference is one of two whole sanctioned shapes; every aggregator-feed override names a step that actually carries the gate; no consumer carries a job-level condition; every reader declares the `needs` edge; and at least one consumer actually reads the output. The last three are the ones that are easy to miss. An override on an UNGATED step would replace a real failure with `success` on every docs-only diff — forgetting an override is fail-closed, adding an unpaired one is not. A job-level `if: always()` on a consumer lets it run when the resolver did not succeed, where the output is empty, both sanctioned forms are false, and every gated step skips alongside its own not-applicable reporter: green having run nothing. A missing `needs` edge produces the same empty output by a different route. The gate is deliberately hostile to being satisfied by anything other than the real thing. Comments never stand in for the expressions they quote. Block-scalar bodies are read for content but never for structure, so a `run:` script can neither masquerade as a step condition nor hide a second resolution. References are detected loosely — any mention of the output, in any spelling or case — and only then held to the exact sanctioned shape, so a spelling the gate does not model reads as unsanctioned rather than becoming invisible to it. Zero references is a defect, not a vacuous pass. Any shape it cannot parse exits 2 rather than 0. Its suite breaks each property in turn and requires the gate to name it, and proves against the real detector that both an outright abort and an unwritable output file leave the flag unset — the half of the guarantee that lives below the workflow. Writing that last case turned up a false sentence in the detector's own header, which claimed a non-zero exit on an unwritable output file. The behavior is correct and the guarantee is unaffected — it rests on the flag being unset, not on an exit status — but a change whose whole subject is replacing a load-bearing comment with something executable should not ship beside a comment known to be false, so that header is corrected here. It is the only edit to that file and it changes no behavior. Behavior is unchanged in which jobs run and which steps they execute. The aggregator is untouched: the intentional docs-only step skip is still mapped to `success` in the CHECK_RESULTS feed, now keyed on the same single output as the gates it mirrors. Refs #2914 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XtbWChCVfUWAv1Pi5Qk2hA --- .github/workflows/ci.yml | 289 +++++++----- scripts/check-docs-only-gate.sh | 673 +++++++++++++++++++++++++++ scripts/check-docs-only-gate.test.sh | 445 ++++++++++++++++++ scripts/check-docs-only.sh | 11 +- 4 files changed, 1296 insertions(+), 122 deletions(-) create mode 100755 scripts/check-docs-only-gate.sh create mode 100755 scripts/check-docs-only-gate.test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c507de709..7402c954c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,66 @@ concurrency: # local ci-status gateway aggregates every lane into the # single required check the org ci-gate ruleset keys on. jobs: + # Single resolution of the docs-only diff scope. Every gated lane reads + # `needs.scope.outputs.run_full` instead of re-running the detector, so the + # detector runs once per workflow rather than once per lane and there is one + # place to reason about what the answer means. + # + # THE OUTPUT IS POSITIVE ON PURPOSE. A consumer writes + # `run_full == 'true'` to do work and `run_full == 'false'` to report the + # work not applicable. Both are plain equality against a value that + # `${{ != 'true' }}` can only ever render as the string `true` or the + # string `false`, so the two forms are exact complements and a new lane has + # no negation to spell wrong. The earlier shape asked each consumer to write + # `docs_only != 'true'` and relied on a prose comment to keep that polarity + # straight across every site. + # + # FAIL-CLOSED TOWARD RUNNING THE SUITE. `scripts/check-docs-only.sh` already + # emits `docs_only=false` on every condition it cannot resolve. This job + # covers the cases below the script: a detector that cannot complete — a + # non-zero exit, or an output file it could not write — leaves + # `steps.detect.outputs.docs_only` unset, which the `run_full` expression + # below resolves to `true`, and the full suite runs. The same unset-output + # path covers a push event, where the detect step does not run at all. + # + # Note precisely what each half contributes, because the two are easy to + # conflate. `continue-on-error` does NOT clear a step's outputs: a step that + # writes to $GITHUB_OUTPUT and then fails keeps what it wrote. What it does is + # keep a failed detector from failing THIS job and skipping every consumer. + # The output being unset on a failed run is a property of + # `scripts/check-docs-only.sh`, which follows every emit with an immediate + # `exit 0` and never emits `true` on a path that can subsequently fail. That + # invariant is what the consumers ultimately rest on; it is stated in that + # script's header and exercised by its suite. + # + # `scripts/check-docs-only-gate.sh` asserts the workflow-side properties + # against this file, so they are checked rather than asserted in prose. + # + # The detector self-test runs first and is NOT `continue-on-error`, so a + # broken detector turns this job red rather than resolving a scope nobody + # verified. + scope: + runs-on: ubuntu-24.04 + timeout-minutes: 15 + outputs: + run_full: ${{ steps.detect.outputs.docs_only != 'true' }} + steps: + - name: Check out + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Fetch base + uses: ./.github/actions/checkout-with-base + - name: Test the docs-only detector + run: bash scripts/check-docs-only.test.sh + - name: Detect a docs-only diff + id: detect + if: github.event_name == 'pull_request' + continue-on-error: true + env: + BASE_REF: ${{ github.base_ref }} + run: scripts/check-docs-only.sh "origin/$BASE_REF" + # Public repository: every lane runs on GitHub-hosted ubuntu-24.04 (free for # public repositories); the local-runner selector is not permitted here. The # short hygiene checks still share one checkout to avoid per-check setup @@ -41,14 +101,15 @@ jobs: # .claude/hooks/*.sh against settings.json — a shell/shebang/hook file added # under an otherwise docs-only prefix like docs/topics/ is real input they # must still catch — gating them would open a fail-closed hole. The job NEVER skips — only - # the path-scoped steps are gated, via the same never-skip, self-test-first, - # fail-closed detector plugin-gate/miro-plugin use. The detector self-test runs - # unconditionally so a broken detector cannot mask a regression, and detection - # is fail-closed toward running the full suite. The gated steps' intentional - # docs-only skip is mapped to `success` in the CHECK_RESULTS feed below (where - # the step provably did not run), never by weakening the aggregator — it still - # fails closed on any real non-`success` outcome. + # the path-scoped steps are gated, on `needs.scope.outputs.run_full` from the + # `scope` lane, which resolves the diff once for the whole workflow and + # carries the self-test and fail-closed guarantees (see that job). The gated + # steps' intentional docs-only skip is mapped to `success` in the + # CHECK_RESULTS feed below (where the step provably did not run), never by + # weakening the aggregator — it still fails closed on any real non-`success` + # outcome. hygiene: + needs: [scope] runs-on: ubuntu-24.04 timeout-minutes: 15 steps: @@ -58,14 +119,6 @@ jobs: persist-credentials: false - name: Fetch base uses: ./.github/actions/checkout-with-base - - name: Test the docs-only detector - run: bash scripts/check-docs-only.test.sh - - name: Detect a docs-only diff - id: scope - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} - run: scripts/check-docs-only.sh "origin/$BASE_REF" - name: Lint markdown id: markdown continue-on-error: true @@ -105,13 +158,13 @@ jobs: - name: Lint workflows id: actionlint - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/actionlint@c2654182bc2d78f7909795df78304d482aa69226 # c265418 2026-07-13 - name: Validate marketplace manifest id: marketplace_schema - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/check-jsonschema@c2654182bc2d78f7909795df78304d482aa69226 # c265418 2026-07-13 with: @@ -119,7 +172,7 @@ jobs: files: .claude-plugin/marketplace.json - name: Validate plugin manifests id: plugin_schema - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/check-jsonschema@c2654182bc2d78f7909795df78304d482aa69226 # c265418 2026-07-13 with: @@ -138,13 +191,13 @@ jobs: run: bash scripts/check-manifest-duplicate-keys.test.sh - name: Detect duplicate keys in plugin/marketplace manifests id: duplicate_keys - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' continue-on-error: true run: python3 scripts/check-manifest-duplicate-keys.py - name: Validate dependabot.yml id: dependabot_schema - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/check-jsonschema@c2654182bc2d78f7909795df78304d482aa69226 # c265418 2026-07-13 with: @@ -152,7 +205,7 @@ jobs: files: .github/dependabot.yml - name: Validate workflows id: workflow_schema - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/check-jsonschema@c2654182bc2d78f7909795df78304d482aa69226 # c265418 2026-07-13 with: @@ -233,7 +286,7 @@ jobs: run: scripts/check-hook-wiring-liveness.sh - name: Report docs-irrelevant checks not applicable to a docs-only diff - if: steps.scope.outputs.docs_only == 'true' + if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the path-scoped linters (actionlint, check-jsonschema x4, the manifest duplicate-key detector) cannot be affected — reporting success for them. ShellCheck, exec-bit, and hook-wiring-liveness scan the whole repo and stay unconditional." - name: Test hygiene result aggregation @@ -243,28 +296,34 @@ jobs: if: always() env: # Display names (left) are annotations; step ids (right) use underscores. - # The path-scoped linters (actionlint, the four check-jsonschema steps) - # carry `if: docs_only != 'true'`, so on a docs-only diff they are - # skipped (never run) — their intentional skip is mapped to `success` - # here BECAUSE the step provably did not run, so no real outcome is - # masked. The aggregator stays fail-closed on any real non-`success` + # The path-scoped linters (actionlint, the four check-jsonschema steps, + # and the manifest duplicate-key detector) carry + # `if: run_full == 'true'`, so on a docs-only diff they are skipped + # (never run) — their intentional skip is mapped to `success` here + # BECAUSE the step provably did not run, so no real outcome is masked. + # The aggregator stays fail-closed on any real non-`success` # (including `skipped`); it is never told to pass on `skipped`. The - # condition (docs_only == 'true') is the exact inverse of each gate, so - # the two can never disagree. ShellCheck, exec-bit, and - # hook-wiring-liveness are unconditional (whole-repo scanners), so - # they feed their raw outcome. + # condition here (`run_full == 'false'`) tests the same single output + # against its only other value, so the two can never disagree. + # `scripts/check-docs-only-gate.sh` pins both halves: that this + # condition is the sanctioned complement of the gates, and that every + # step named on the right of an override is one that carries the gate + # — an override on an UNGATED step would mask a real failure as + # success, and that is the direction that fails open. ShellCheck, + # exec-bit, and hook-wiring-liveness are unconditional (whole-repo + # scanners), so they feed their raw outcome. CHECK_RESULTS: | markdown=${{ steps.markdown.outcome }} typos=${{ steps.typos.outcome }} gitleaks=${{ steps.gitleaks.outcome }} editorconfig=${{ steps.editorconfig.outcome }} shellcheck=${{ steps.shellcheck.outcome }} - actionlint=${{ steps.scope.outputs.docs_only == 'true' && 'success' || steps.actionlint.outcome }} - marketplace-schema=${{ steps.scope.outputs.docs_only == 'true' && 'success' || steps.marketplace_schema.outcome }} - plugin-schema=${{ steps.scope.outputs.docs_only == 'true' && 'success' || steps.plugin_schema.outcome }} - manifest-duplicate-keys=${{ steps.scope.outputs.docs_only == 'true' && 'success' || steps.duplicate_keys.outcome }} - dependabot-schema=${{ steps.scope.outputs.docs_only == 'true' && 'success' || steps.dependabot_schema.outcome }} - workflow-schema=${{ steps.scope.outputs.docs_only == 'true' && 'success' || steps.workflow_schema.outcome }} + actionlint=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.actionlint.outcome }} + marketplace-schema=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.marketplace_schema.outcome }} + plugin-schema=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.plugin_schema.outcome }} + manifest-duplicate-keys=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.duplicate_keys.outcome }} + dependabot-schema=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.dependabot_schema.outcome }} + workflow-schema=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.workflow_schema.outcome }} exec-bit=${{ steps.exec_bit.outcome }} machine-specific-paths=${{ steps.machine_paths.outcome }} eol-renormalize=${{ steps.eol.outcome }} @@ -981,6 +1040,27 @@ jobs: - name: Check every restatement carries the canonical clause's qualifiers run: python3 scripts/check-contract-clause-coverage.py + # The docs-only short-circuit is a fail-open shape by construction: getting it + # wrong makes a heavy lane report success without running. Its safety + # properties — one resolution, an output whose unset value means "run the full + # suite", a detector failure absorbed rather than propagated, an unweakened + # self-test, and exactly one spelling for a consumer to copy — used to live in + # a prose comment repeated next to every gated step. This lane checks them + # against the workflow instead. Self-test first, so a broken gate cannot mask + # a regression. + docs-only-gate: + runs-on: ubuntu-24.04 + timeout-minutes: 15 + steps: + - name: Check out + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Test the docs-only scope-contract gate + run: bash scripts/check-docs-only-gate.test.sh + - name: Check the docs-only scope contract holds in ci.yml + run: scripts/check-docs-only-gate.sh --check + # `ci-status` below is the single check the org ci-gate ruleset keys on, and # its own comment calls its `needs` list "the single source of truth for the # lane list" — but nothing enforced the other direction. A job defined in THIS @@ -1017,10 +1097,11 @@ jobs: # `success` (the ci-status aggregate rejects it) and a workflow skipped by a # path filter leaves its required check Pending # (troubleshooting-required-status-checks). Only the inner install/test steps - # are gated; the detector self-test runs unconditionally so a broken detector - # cannot mask a regression behind a docs-only short-circuit, and detection is - # fail-closed toward running the full suite. + # are gated, on `needs.scope.outputs.run_full` from the `scope` lane, which + # carries the self-test and fail-closed guarantees for every lane that reads + # it. plugin-gate: + needs: [scope] runs-on: ubuntu-24.04 timeout-minutes: 15 steps: @@ -1030,30 +1111,22 @@ jobs: persist-credentials: false - name: Fetch base uses: ./.github/actions/checkout-with-base - - name: Test the docs-only detector - run: bash scripts/check-docs-only.test.sh - - name: Detect a docs-only diff - id: scope - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} - run: scripts/check-docs-only.sh "origin/$BASE_REF" - name: Set up Node - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version-file: .node-version cache: npm cache-dependency-path: package-lock.json - name: Set up Python - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: '3.14' cache: pip cache-dependency-path: .github/requirements-ci.txt - name: Install and verify ShellCheck toolchain - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' # This canonical action also makes the exact ShellCheck version # available to the later bash-format contract tests in this same job. uses: melodic-software/ci-workflows/.github/actions/shellcheck@c2654182bc2d78f7909795df78304d482aa69226 # c265418 2026-07-13 @@ -1067,7 +1140,7 @@ jobs: # and Ruff are declared here so hosted and local runs exercise the same # contract suites instead of inheriting different image tool inventories. - name: Install locked plugin test toolchains - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: | npm ci python -m pip install --user --only-binary=:all: --require-hashes \ @@ -1075,12 +1148,12 @@ jobs: echo "$GITHUB_WORKSPACE/node_modules/.bin" >> "$GITHUB_PATH" echo "$HOME/.local/bin" >> "$GITHUB_PATH" - name: Run plugin contract tests - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: scripts/run-plugin-tests.sh # Explicit step: run-plugin-tests.sh discovers only plugins/**/*.test.sh, # so the cheat-sheet generator suite under scripts/ never runs without it. - name: Run cheat-sheet generator tests - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash scripts/generate-cheatsheet.test.sh # Same reason as the step above — run-plugin-tests.sh never reaches # scripts/. This suite is wired into CI rather than left to local runs @@ -1091,7 +1164,7 @@ jobs: # a new carrying plugin appears, and a check that only ever runs when # someone remembers to run it is exactly the rot it guards against. - name: Run affected-suite selector tests - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash scripts/affected-tests.test.sh # Same reason again — run-plugin-tests.sh never reaches scripts/. This # suite covers scripts/lib/changed-files.sh, the base-ref and changed-file @@ -1100,7 +1173,7 @@ jobs: # exercises a C-quotable pathname, so this is the only place a regression # there turns anything red. - name: Run shared changed-file resolver tests - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash scripts/lib/changed-files.test.sh # Same reason again — run-plugin-tests.sh never reaches scripts/. This # suite covers scripts/lib/read-list.sh, the list-file reader the gates @@ -1109,13 +1182,13 @@ jobs: # that quietly collapses them turns this red rather than silently # truncating a token pattern. - name: Run shared list-file reader tests - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash scripts/lib/read-list.test.sh - name: Validate plugin and catalog manifests - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: scripts/validate-plugins.sh - name: Report not applicable to a docs-only diff - if: steps.scope.outputs.docs_only == 'true' + if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the plugin contract suite cannot be affected — reporting success." # The miro plugin ships a bundled Node MCP server — the marketplace's first. @@ -1124,10 +1197,11 @@ jobs: # and fails on any drift, then runs the bundle over stdio so a build that # compiles but cannot serve MCP is caught here, not on a consumer's machine. # Every step reads only plugins/miro/**, so a docs-only diff cannot affect it; - # it uses the same never-skip, self-test-first, fail-closed docs-only gate as - # plugin-gate. (Scoping this lane to plugins/miro/** specifically — skipping it + # it reads the same `scope` lane output as plugin-gate and never skips as a + # job. (Scoping this lane to plugins/miro/** specifically — skipping it # on any non-miro diff — is a broader, separately-tracked optimization.) miro-plugin: + needs: [scope] runs-on: ubuntu-24.04 timeout-minutes: 15 steps: @@ -1137,43 +1211,35 @@ jobs: persist-credentials: false - name: Fetch base uses: ./.github/actions/checkout-with-base - - name: Test the docs-only detector - run: bash scripts/check-docs-only.test.sh - - name: Detect a docs-only diff - id: scope - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} - run: scripts/check-docs-only.sh "origin/$BASE_REF" - name: Set up Node - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version-file: .node-version cache: npm cache-dependency-path: plugins/miro/package-lock.json - name: Install dependencies - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: npm ci working-directory: plugins/miro - name: Typecheck - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: npm run typecheck working-directory: plugins/miro - name: Lint - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: npm run lint working-directory: plugins/miro - name: Test - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: npm test working-directory: plugins/miro - name: Verify the committed bundle matches source - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: npm run verify-bundle working-directory: plugins/miro - name: Smoke-test the bundled MCP server over stdio - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' working-directory: plugins/miro run: | printf '%s\n%s\n' \ @@ -1183,7 +1249,7 @@ jobs: grep -q '"miro_create_board"' smoke-out.json rm -f smoke-out.json - name: Report not applicable to a docs-only diff - if: steps.scope.outputs.docs_only == 'true' + if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the miro plugin build cannot be affected — reporting success." # The knowledge plugin's video-digest extraction pipeline is a Node package @@ -1191,11 +1257,12 @@ jobs: # committed lockfile pins the shared vendor/ packages as packed installs # (install-links via the package's .npmrc), so manifest/lockfile drift breaks # `npm ci` only on a clean install — without this lane that breakage stays - # latent until a consumer's machine hits it. Same never-skip, self-test-first, - # fail-closed docs-only gate as plugin-gate and miro-plugin. (Scoping this + # latent until a consumer's machine hits it. Reads the same `scope` lane + # output as plugin-gate and miro-plugin. (Scoping this # lane to its own paths — skipping it on unrelated diffs — is the same # separately-tracked optimization noted on miro-plugin.) video-extraction: + needs: [scope] runs-on: ubuntu-24.04 timeout-minutes: 15 steps: @@ -1205,16 +1272,8 @@ jobs: persist-credentials: false - name: Fetch base uses: ./.github/actions/checkout-with-base - - name: Test the docs-only detector - run: bash scripts/check-docs-only.test.sh - - name: Detect a docs-only diff - id: scope - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} - run: scripts/check-docs-only.sh "origin/$BASE_REF" - name: Set up Node - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version-file: .node-version @@ -1223,16 +1282,16 @@ jobs: # encapsulation contract); logic goes through the skill's scripts/ facade below. cache-dependency-path: plugins/knowledge/skills/video-digest/extraction/package-lock.json - name: Install dependencies - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/knowledge/skills/video-digest/scripts/run-tests.sh install - name: Typecheck - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/knowledge/skills/video-digest/scripts/run-tests.sh build - name: Test - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/knowledge/skills/video-digest/scripts/run-tests.sh test - name: Report not applicable to a docs-only diff - if: steps.scope.outputs.docs_only == 'true' + if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the video extraction suite cannot be affected — reporting success." # ai-briefing's build/render pipeline is a Node package (native `node --test` @@ -1242,12 +1301,13 @@ jobs: # CGN/benchmarking/documentation/multicast rejection, DNS-gate-time A/AAAA # resolution, and the IPv6 2000::/3 allowlist inversion. Without this lane # that suite runs locally only, so a future refactor could silently break the - # predicate with nothing red in CI (#1488). Same never-skip, self-test-first, - # fail-closed docs-only gate as plugin-gate, miro-plugin and + # predicate with nothing red in CI (#1488). Reads the same `scope` lane + # output as plugin-gate, miro-plugin and # video-extraction. (Scoping this lane to its own paths — skipping it on # unrelated diffs — is the same separately-tracked optimization noted on # miro-plugin.) ai-briefing-build: + needs: [scope] runs-on: ubuntu-24.04 timeout-minutes: 15 steps: @@ -1257,16 +1317,8 @@ jobs: persist-credentials: false - name: Fetch base uses: ./.github/actions/checkout-with-base - - name: Test the docs-only detector - run: bash scripts/check-docs-only.test.sh - - name: Detect a docs-only diff - id: scope - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} - run: scripts/check-docs-only.sh "origin/$BASE_REF" - name: Set up Node - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version-file: .node-version @@ -1275,13 +1327,13 @@ jobs: # encapsulation contract); logic goes through the skill's scripts/ facade below. cache-dependency-path: plugins/ai-briefing/skills/generate/output/build/package-lock.json - name: Install dependencies - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/ai-briefing/skills/generate/scripts/run-tests.sh install - name: Test - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/ai-briefing/skills/generate/scripts/run-tests.sh test - name: Report not applicable to a docs-only diff - if: steps.scope.outputs.docs_only == 'true' + if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the ai-briefing build suite cannot be affected — reporting success." # The knowledge plugin's course-digest extraction pipeline is a Node package @@ -1291,12 +1343,13 @@ jobs: # this lane the suite (utils, the adapter contract, the dometrain/teachable # adapters, clerk/teachable-sso auth, config, and the hotmart/mux players) # runs locally only, so a future refactor could silently break it with - # nothing red in CI (#1507). Same never-skip, self-test-first, fail-closed - # docs-only gate as plugin-gate, miro-plugin, video-extraction and + # nothing red in CI (#1507). Reads the same `scope` lane output as + # plugin-gate, miro-plugin, video-extraction and # ai-briefing-build. (Scoping this lane to its own paths — skipping it on # unrelated diffs — is the same separately-tracked optimization noted on # miro-plugin.) course-digest-extraction: + needs: [scope] runs-on: ubuntu-24.04 timeout-minutes: 15 steps: @@ -1306,16 +1359,8 @@ jobs: persist-credentials: false - name: Fetch base uses: ./.github/actions/checkout-with-base - - name: Test the docs-only detector - run: bash scripts/check-docs-only.test.sh - - name: Detect a docs-only diff - id: scope - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} - run: scripts/check-docs-only.sh "origin/$BASE_REF" - name: Set up Node - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version-file: .node-version @@ -1324,16 +1369,16 @@ jobs: # encapsulation contract); logic goes through the skill's scripts/ facade below. cache-dependency-path: plugins/knowledge/skills/course-digest/extraction/package-lock.json - name: Install dependencies - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/knowledge/skills/course-digest/scripts/run-tests.sh install - name: Typecheck - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/knowledge/skills/course-digest/scripts/run-tests.sh build - name: Test - if: steps.scope.outputs.docs_only != 'true' + if: needs.scope.outputs.run_full == 'true' run: bash plugins/knowledge/skills/course-digest/scripts/run-tests.sh test - name: Report not applicable to a docs-only diff - if: steps.scope.outputs.docs_only == 'true' + if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the course-digest extraction suite cannot be affected — reporting success." # Skill-regression net: the only lane that invokes the skill-quality checker. @@ -1474,6 +1519,7 @@ jobs: ci-status: needs: + - scope - hygiene - hook-utils-sync - hook-utils-windows @@ -1504,6 +1550,7 @@ jobs: - changelog-parity-gate - contract-slice-prune-gate - contract-clause-coverage-gate + - docs-only-gate - lane-coverage-gate - plugin-gate - miro-plugin diff --git a/scripts/check-docs-only-gate.sh b/scripts/check-docs-only-gate.sh new file mode 100755 index 0000000000..14177289d5 --- /dev/null +++ b/scripts/check-docs-only-gate.sh @@ -0,0 +1,673 @@ +#!/usr/bin/env bash +# Gate: the docs-only scope contract is resolved in ONE place and consumed in +# ONE documented form. +# +# scripts/check-docs-only-gate.sh --check [] +# +# Default workflow: .github/workflows/ci.yml. +# +# WHY. The heavy lanes short-circuit on a diff confined to the docs-only +# allowlist. That short-circuit is a fail-open shape by construction: getting it +# wrong makes a lane report success without running, which is exactly the +# false-green docs/conventions/liveness-assertion/ names. The contract used to +# live as a step output re-derived independently in six jobs, with every +# consumer spelling the gate as `steps.scope.outputs.docs_only != 'true'` — an +# INVERSE-polarity test whose correctness was maintained by a prose comment. +# Forty-eight hand-maintained references to a string flag, each of which had to +# independently know that the value is the string `true`, that the safe +# direction is to RUN, and that the negation must be spelled that exact way. +# +# A consumer that wrote `== 'false'` instead of `!= 'true'` would skip its lane +# whenever the detector emitted anything unexpected. The asymmetry is why the +# comment existed. This gate is what replaces the comment: the properties below +# are the ones the comment used to assert, checked against the workflow itself. +# +# WHAT IS CHECKED: +# 1. SINGLE RESOLUTION — the detector is invoked from exactly one job, that +# job is the resolver, and no `steps..outputs +# .docs_only` read survives outside the resolver's own +# output expression. A second resolution is a second +# answer that can disagree with the first. +# 2. FAIL-CLOSED DEFAULT — the published output is derived by the exact +# expression whose value for an UNSET detector output +# is "run the full suite". This is the property that +# makes a detector that never ran, or a push event +# where it is not meant to run, resolve toward running +# rather than toward skipping. +# 3. FAILURE IS ABSORBED — the step that invokes the detector carries +# `continue-on-error: true` and is the step the output +# expression reads, so a detector that fails OUTRIGHT +# leaves the output unset and falls into property 2 +# instead of failing the resolving job and skipping +# every consumer. +# 4. SELF-TEST INTACT — the resolving job runs the detector's own suite, and +# that step is NOT `continue-on-error`. A detector +# nobody verified must turn something red rather than +# resolve a scope silently. +# 5. ONE CONSUMER FORM — every consumer reference is a STEP-level condition +# spelled exactly `== 'true'` (do the work) or +# `== 'false'` (report it not applicable), or an entry +# of the aggregator feed in its exact template. The +# check is a whitelist of whole shapes, not a search +# for a substring: a negation, a truthiness test, an +# index-syntax spelling, or a JOB-level condition all +# fail. A job-level condition is singled out because it +# SKIPS the job, and a skipped required lane leaves its +# check Pending rather than red — see +# scripts/check-docs-only.sh's header. +# 6. FEED MIRRORS A GATE — every step whose outcome the aggregator feed +# overrides to `success` on a docs-only diff is a step +# that actually carries the work gate. Forgetting an +# override is fail-closed (the raw `skipped` reds the +# aggregate); adding one for an UNGATED step is +# fail-open, because it replaces that step's real +# outcome with `success` on every docs-only diff. +# 7. NO JOB-LEVEL IF — a consumer carries no job-level condition at all. It +# reaches the output through `needs`, so it runs only +# when the resolver succeeded — which is what makes the +# output's domain exactly {'true','false'} and the two +# sanctioned forms exact complements. `if: always()` or +# `if: ${{ !cancelled() }}` breaks that: the job runs +# with an EMPTY output, both forms are false, and every +# gated step and its not-applicable reporter skip +# together — the lane reports success having run +# nothing. The condition need not mention the output to +# do this, so this check does not read what it says. +# 8. EDGE DECLARED — a job that reads the output declares the resolving +# job in its `needs`. Without the edge the expression +# yields an empty string, with the same consequence. +# 9. CONTRACT IS LIVE — at least one consumer job actually reads the output. +# A resolver nobody consumes is a dead contract, and a +# gate that reports "all references are well formed" +# over zero references is the nominal closure this file +# exists to deny. +# +# FAIL CLOSED ON SHAPE. Like scripts/check-lane-coverage.sh, this reads the +# workflow structurally rather than through a YAML library (the repo ships no +# root YAML dependency). Any shape it does not recognize exits 2 (inconclusive), +# never 0. Reporting "contract satisfied" from a file this gate did not parse +# would be the same false-green it exists to deny. Comments are prose ABOUT the +# contract and never satisfy any property: a commented-out expression must not +# stand in for the expression. Block scalars (`run: |`) are skipped wholesale so +# a script body can never be read as workflow structure. +# +# KNOWN SCOPE LIMIT. A job that delegates to a reusable workflow (`uses:` at the +# job level) has no steps of its own, so its polarity decision would live in a +# file this gate never opens. Such a job is REJECTED if it reads the output, +# rather than passed silently. +# +# Exit: 0 satisfied; 1 a contract defect; 2 usage, missing file, or unrecognized +# workflow shape. +set -uo pipefail + +if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then + echo "check-docs-only-gate: not inside a git work tree" >&2 + exit 2 +fi +cd "$(git rev-parse --show-toplevel)" || exit 2 + +usage() { + echo "usage: $(basename "$0") --check []" >&2 + exit 2 +} + +[[ "${1:-}" == "--check" ]] || usage +WORKFLOW="${2:-.github/workflows/ci.yml}" +[[ $# -le 2 ]] || usage + +if [[ ! -f "$WORKFLOW" ]]; then + echo "check-docs-only-gate: workflow not found: $WORKFLOW" >&2 + exit 2 +fi + +# The contract's literals, kept together so the whole of it reads as one block +# rather than as constants scattered through the assertions. +RESOLVER_JOB="scope" +OUTPUT_NAME="run_full" +DETECT_STEP_ID="detect" +OUTPUT_EXPR="\${{ steps.${DETECT_STEP_ID}.outputs.docs_only != 'true' }}" +REFERENCE="needs.${RESOLVER_JOB}.outputs.${OUTPUT_NAME}" +WORK_FORM="${REFERENCE} == 'true'" +SKIP_FORM="${REFERENCE} == 'false'" + +errors=0 +report() { + echo "$1" >&2 + errors=$((errors + 1)) +} + +# --- structural pass -------------------------------------------------------- +# +# One walk over the file, emitting tab-separated records that carry the job (and +# where it matters the step ordinal) each fact belongs to. Attribution happens +# here rather than by re-slicing the text later, so no assertion can be +# satisfied by a line belonging to some other job or step. +# +# JOB +# USES job delegates to a reusable workflow +# NEEDSFLOW inline `needs:` value +# NEEDSITEM one entry of a block-sequence `needs:` +# OUTPUT +# INVOKE invokes the detector +# SELFTEST invokes the detector's own suite +# COE step carries continue-on-error: true +# STEPID +# REF kind: stepif | jobif | other +# STEPOUT reads a step-level docs_only output +# ERR +parsed="$( + awk -v resolver="$RESOLVER_JOB" -v output_name="$OUTPUT_NAME" ' + function trim(s) { sub(/^[[:blank:]]+/, "", s); sub(/[[:blank:]]+$/, "", s); return s } + function indent_of(s, t) { t = s; sub(/[^[:blank:]].*$/, "", t); return length(t) } + + # Everything a line asserts once its comment is removed. A comment is prose + # ABOUT the contract and satisfies no part of it: a commented-out detector + # call is not an invocation, and a sentence naming the detector is not a + # second resolution. + function uncommented(line, l) { + l = trim(line) + if (substr(l, 1, 1) == "#") { return "" } + sub(/[[:blank:]]+#.*$/, "", l) + return l + } + + # A line MENTIONS the resolved output if it names the output, or reaches the + # resolver`s outputs at all, in any spelling and any case. Actions context + # accessors are case-insensitive and may be written with dots or brackets, + # so this deliberately over-matches: the point is that every mention lands in + # front of the exact-shape check below rather than being invisible to it. A + # spelling this file does not model must read as UNSANCTIONED, never as + # absent — a whitelist that only fires on what it recognizes silently ignores + # everything else, which is the opposite of rejecting it. + function mentions_output(line, l) { + l = tolower(line) + if (index(l, outname) > 0) { return 1 } + if (index(l, "needs") > 0 && index(l, resolver_lc) > 0 && index(l, "outputs") > 0) { return 1 } + return 0 + } + + # Facts readable from a line WITHOUT interpreting it as workflow structure. + # Block-scalar bodies get exactly these: a `run:` script that re-resolves the + # scope is still a second resolution, and the aggregator feed lives inside an + # env block scalar, so both must still be seen — but neither may be read as a + # job key or a step boundary. + function scan_content( live) { + live = uncommented($0) + if (live == "") { return } + if (live ~ /check-docs-only\.test\.sh/) { print "SELFTEST\t" job "\t" step } + else if (live ~ /check-docs-only\.sh/) { print "INVOKE\t" job "\t" step } + if (live ~ /steps\.[A-Za-z_][A-Za-z0-9_-]*\.outputs\.docs_only/) { print "STEPOUT\t" job } + if (mentions_output(live)) { print "REF\t" job "\t" step "\tother\t" live } + } + + BEGIN { + injobs = 0; seen_jobs = 0; job = ""; step = 0 + needs_state = 0; outputs_state = 0; scalar = -1 + resolver_lc = tolower(resolver) + outname = tolower(output_name) + if (resolver_lc == "" || outname == "") { + print "ERR\tinternal: resolver job or output name was not supplied to the parser" + exit 1 + } + } + + # --- block scalars: their content is data, never structure -------------- + scalar >= 0 { + if ($0 ~ /^[[:blank:]]*$/) { next } + if (indent_of($0) > scalar) { scan_content(); next } + scalar = -1 + # fall through: this line is real structure again + } + + !injobs && /^jobs:[[:blank:]]*$/ { injobs = 1; seen_jobs = 1; next } + !injobs { next } + + # A column-0 key closes the jobs: mapping. + /^[^[:blank:]#]/ { injobs = 0; needs_state = 0; outputs_state = 0; next } + + # --- comments are prose about the contract, never part of it ------------ + # They are skipped for every purpose EXCEPT that they must not terminate a + # job body: a comment sits happily between two steps. + { probe = trim($0) } + probe ~ /^#/ { next } + + # --- block-sequence needs: ---------------------------------------------- + needs_state == 1 { + # A sequence item may sit at the key`s own indent or deeper — both are + # valid YAML, and the shallower form is the one a hand edit reaches for. + if ($0 ~ /^[[:blank:]]+- [A-Za-z_][A-Za-z0-9_-]*[[:blank:]]*(#.*)?$/ && indent_of($0) >= 4) { + item = $0 + sub(/^[[:blank:]]*- /, "", item) + sub(/[[:blank:]]*#.*$/, "", item) + print "NEEDSITEM\t" job "\t" trim(item) + next + } + # A 4-space key or a shallower line closes the block; anything else at + # this depth is a sequence shape this gate does not model. + if ($0 !~ /^ [A-Za-z_][A-Za-z0-9_-]*:/ && indent_of($0) >= 4) { + print "ERR\tunsupported needs entry in job " job ": " $0 + next + } + needs_state = 0 + # fall through + } + + # --- outputs: mapping --------------------------------------------------- + outputs_state == 1 { + if ($0 ~ /^ [A-Za-z_][A-Za-z0-9_-]*:/) { + name = $0 + sub(/^ /, "", name) + sub(/:.*$/, "", name) + expr = $0 + sub(/^ [A-Za-z_][A-Za-z0-9_-]*:[[:blank:]]*/, "", expr) + print "OUTPUT\t" job "\t" name "\t" trim(expr) + next + } + if ($0 ~ /^ /) { + print "ERR\tunsupported outputs entry in job " job ": " $0 + next + } + outputs_state = 0 + # fall through + } + + # --- 2-space keys: job boundaries --------------------------------------- + /^ [^[:blank:]]/ { + if ($0 ~ /^ [A-Za-z_][A-Za-z0-9_-]*:[[:blank:]]*(#.*)?$/) { + job = $0 + sub(/:.*$/, "", job) + sub(/^ /, "", job) + step = 0 + needs_state = 0 + outputs_state = 0 + print "JOB\t" job + next + } + print "ERR\tunrecognized key under jobs: " $0 + next + } + + # --- 4-space keys: job-level --------------------------------------------- + /^ [^[:blank:]]/ { + needs_state = 0 + outputs_state = 0 + if ($0 ~ /^ needs:[[:blank:]]*$/) { needs_state = 1; next } + if ($0 ~ /^ needs:/) { + rest = $0 + sub(/^ needs:[[:blank:]]*/, "", rest) + print "NEEDSFLOW\t" job "\t" trim(rest) + next + } + if ($0 ~ /^ outputs:[[:blank:]]*$/) { outputs_state = 1; next } + if ($0 ~ /^ uses:/) { print "USES\t" job; next } + if ($0 ~ /^ if:/) { + rest = $0 + sub(/^ if:[[:blank:]]*/, "", rest) + rest = uncommented(rest) + # Recorded whether or not it names the output: ANY job-level condition + # on a consumer can let that job run while the resolver did not succeed, + # which is the one state where the published output is empty. + print "JOBIF\t" job "\t" rest + if (mentions_output(rest)) { print "REF\t" job "\t-\tjobif\t" rest } + next + } + # A block scalar opening at this level. + if ($0 ~ /:[[:blank:]]*[|>][-+0-9]*[[:blank:]]*$/) { scalar = 4 } + next + } + + # --- step boundaries and step bodies ------------------------------------ + /^ - / { step = step + 1 } + + { + # `continue-on-error` decides whether a failure is absorbed, so an + # expression-valued one cannot be judged without evaluating it. Refuse + # rather than guess: a wrong guess here silently flips property 3 or 4. + if ($0 ~ /^ continue-on-error:/) { + coe = $0 + sub(/^ continue-on-error:[[:blank:]]*/, "", coe) + coe = uncommented(coe) + if (coe == "true") { print "COE\t" job "\t" step } + else if (coe != "false") { + print "ERR\tcontinue-on-error in job " job " step " step " is not a literal true/false: " coe + } + } + + if ($0 ~ /^ id:/) { + sid = $0 + sub(/^ id:[[:blank:]]*/, "", sid) + print "STEPID\t" job "\t" step "\t" trim(sid) + } + + if ($0 ~ /^ if:/ && mentions_output(uncommented($0))) { + rest = $0 + sub(/^ if:[[:blank:]]*/, "", rest) + print "REF\t" job "\t" step "\tstepif\t" uncommented(rest) + } else { + scan_content() + } + + # A block scalar opening inside a step: its body is content, not structure. + if ($0 ~ /:[[:blank:]]*[|>][-+0-9]*[[:blank:]]*$/) { scalar = indent_of($0) } + } + + END { if (!seen_jobs) print "ERR\tno jobs: mapping found" } + ' "$WORKFLOW" +)" || { + echo "check-docs-only-gate: failed to read $WORKFLOW" >&2 + exit 2 +} + +TAB="$(printf '\t')" + +# Partition the record stream once. Re-grepping it per assertion would fork a +# process per lookup, which is measurable on a large workflow. +REC_ERR=""; REC_JOB=""; REC_USES=""; REC_NEEDSFLOW=""; REC_NEEDSITEM="" +REC_OUTPUT=""; REC_INVOKE=""; REC_SELFTEST=""; REC_COE=""; REC_STEPID="" +REC_REF=""; REC_STEPOUT=""; REC_JOBIF="" +while IFS= read -r line; do + [[ -n "$line" ]] || continue + case "$line" in + "ERR$TAB"*) REC_ERR+="${line#*"$TAB"}"$'\n' ;; + "JOB$TAB"*) REC_JOB+="${line#*"$TAB"}"$'\n' ;; + "USES$TAB"*) REC_USES+="${line#*"$TAB"}"$'\n' ;; + "NEEDSFLOW$TAB"*) REC_NEEDSFLOW+="${line#*"$TAB"}"$'\n' ;; + "NEEDSITEM$TAB"*) REC_NEEDSITEM+="${line#*"$TAB"}"$'\n' ;; + "OUTPUT$TAB"*) REC_OUTPUT+="${line#*"$TAB"}"$'\n' ;; + "INVOKE$TAB"*) REC_INVOKE+="${line#*"$TAB"}"$'\n' ;; + "SELFTEST$TAB"*) REC_SELFTEST+="${line#*"$TAB"}"$'\n' ;; + "COE$TAB"*) REC_COE+="${line#*"$TAB"}"$'\n' ;; + "STEPID$TAB"*) REC_STEPID+="${line#*"$TAB"}"$'\n' ;; + "REF$TAB"*) REC_REF+="${line#*"$TAB"}"$'\n' ;; + "STEPOUT$TAB"*) REC_STEPOUT+="${line#*"$TAB"}"$'\n' ;; + "JOBIF$TAB"*) REC_JOBIF+="${line#*"$TAB"}"$'\n' ;; + # The awk pass emits no other record kind; a line that reaches here means the + # two halves have drifted, which is not something to guess past. + *) + echo "check-docs-only-gate: unrecognized internal record: $line" >&2 + exit 2 + ;; + esac +done <<<"$parsed" + +# Newline-delimited membership test without forking. +has_line() { case $'\n'"$1" in *$'\n'"$2"$'\n'*) return 0 ;; *) return 1 ;; esac; } + +# Unique, space-joined rendering of a newline-delimited list, for messages. +uniq_list() { + local seen="" item out="" + while IFS= read -r item; do + [[ -n "$item" ]] || continue + has_line "$seen" "$item" && continue + seen+="$item"$'\n' + out+="$item " + done <<<"$1" + printf '%s' "$out" +} + +shape_errors="$REC_ERR" + +if [[ -n "$shape_errors" ]]; then + echo "check-docs-only-gate: unrecognized workflow shape in $WORKFLOW" >&2 + while IFS= read -r msg; do + [[ -n "$msg" ]] || continue + echo " $msg" >&2 + done <<<"$shape_errors" + echo " Refusing to report a satisfied contract from a file this gate did not fully parse." >&2 + exit 2 +fi + +jobs_all="$REC_JOB" +if [[ -z "$jobs_all" ]]; then + echo "check-docs-only-gate: no jobs parsed from $WORKFLOW" >&2 + exit 2 +fi +if ! has_line "$jobs_all" "$RESOLVER_JOB"; then + echo "check-docs-only-gate: resolving job '$RESOLVER_JOB' is not defined in $WORKFLOW" >&2 + exit 2 +fi + +# --- 1. SINGLE RESOLUTION --------------------------------------------------- + +invoker_jobs="" +invoker_count=0 +while IFS="$TAB" read -r ijob _; do + [[ -n "$ijob" ]] || continue + has_line "$invoker_jobs" "$ijob" && continue + invoker_jobs+="$ijob"$'\n' + invoker_count=$((invoker_count + 1)) +done <<<"$REC_INVOKE" + +if [[ "$invoker_count" -ne 1 ]]; then + report "SINGLE RESOLUTION: the docs-only detector is invoked from $invoker_count job(s) [$(uniq_list "$invoker_jobs")]. It must be resolved exactly once, in '$RESOLVER_JOB', and read from there." +elif ! has_line "$invoker_jobs" "$RESOLVER_JOB"; then + report "SINGLE RESOLUTION: the docs-only detector is invoked from [$(uniq_list "$invoker_jobs")], not from the resolving job '$RESOLVER_JOB'." +fi + +# The resolver reads its own detect step to publish the output — the single +# sanctioned step-level read. Anywhere else it is a second answer. +stray_stepouts="" +while IFS= read -r sjob; do + [[ -n "$sjob" ]] || continue + [[ "$sjob" == "$RESOLVER_JOB" ]] && continue + stray_stepouts+="$sjob"$'\n' +done <<<"$REC_STEPOUT" +if [[ -n "$stray_stepouts" ]]; then + report "SINGLE RESOLUTION: a step-level docs_only output is still read in job(s) [$(uniq_list "$stray_stepouts")]. Consumers must read $REFERENCE, never a per-job step output." +fi + +# --- 2. FAIL-CLOSED DEFAULT ------------------------------------------------- + +published_expr="" +published_found=0 +while IFS="$TAB" read -r ojob oname oexpr; do + [[ "$ojob" == "$RESOLVER_JOB" && "$oname" == "$OUTPUT_NAME" ]] || continue + published_found=1 + published_expr="$oexpr" +done <<<"$REC_OUTPUT" + +if [[ "$published_found" -eq 0 ]]; then + report "FAIL-CLOSED DEFAULT: job '$RESOLVER_JOB' publishes no '$OUTPUT_NAME' output. Consumers would read an empty string, which skips both the work step and its not-applicable reporter." +elif [[ "$published_expr" != "$OUTPUT_EXPR" ]]; then + report "FAIL-CLOSED DEFAULT: job '$RESOLVER_JOB' publishes '$OUTPUT_NAME: $published_expr', expected exactly '$OUTPUT_NAME: $OUTPUT_EXPR'. That expression is the contract: an UNSET detector output (the detector never ran, or failed) renders as 'true', so the full suite runs. Any other derivation can resolve an unset detector toward skipping." +fi + +# --- 3. FAILURE IS ABSORBED ------------------------------------------------- +# +# The step carrying the detect id must be the step that actually invokes the +# detector, and must absorb its own failure. Checking those independently would +# pass a workflow whose detect step runs something else entirely. + +detect_job="" +detect_ordinal="" +while IFS="$TAB" read -r sjob sord sid; do + [[ "$sid" == "$DETECT_STEP_ID" ]] || continue + [[ -n "$detect_job" ]] && continue + detect_job="$sjob" + detect_ordinal="$sord" +done <<<"$REC_STEPID" + +if [[ -z "$detect_job" ]]; then + report "FAILURE IS ABSORBED: no step declares 'id: $DETECT_STEP_ID', so the published output has nothing to read." +elif [[ "$detect_job" != "$RESOLVER_JOB" ]]; then + report "FAILURE IS ABSORBED: the 'id: $DETECT_STEP_ID' step is in job '$detect_job', not in the resolving job '$RESOLVER_JOB'." +else + if ! has_line "$REC_INVOKE" "${RESOLVER_JOB}${TAB}${detect_ordinal}"; then + report "FAILURE IS ABSORBED: the 'id: $DETECT_STEP_ID' step in job '$RESOLVER_JOB' does not invoke the docs-only detector, so the published output does not describe what the detector found." + fi + if ! has_line "$REC_COE" "${RESOLVER_JOB}${TAB}${detect_ordinal}"; then + report "FAILURE IS ABSORBED: the 'id: $DETECT_STEP_ID' step in job '$RESOLVER_JOB' is missing 'continue-on-error: true'. Without it a detector that fails outright fails the resolving job, and every consumer is skipped rather than falling back to running the full suite." + fi +fi + +# --- 4. SELF-TEST INTACT ---------------------------------------------------- + +selftest_ordinal="" +while IFS="$TAB" read -r sjob sord; do + [[ "$sjob" == "$RESOLVER_JOB" ]] || continue + [[ -n "$selftest_ordinal" ]] && continue + selftest_ordinal="$sord" +done <<<"$REC_SELFTEST" + +if [[ -z "$selftest_ordinal" ]]; then + report "SELF-TEST INTACT: job '$RESOLVER_JOB' does not run the detector's own suite. A detector nobody verified must not be the thing that decides a lane can be short-circuited." +elif has_line "$REC_COE" "${RESOLVER_JOB}${TAB}${selftest_ordinal}"; then + report "SELF-TEST INTACT: the detector self-test step in job '$RESOLVER_JOB' carries continue-on-error. A broken detector must turn this job red, not pass quietly." +fi + +# --- 5. ONE CONSUMER FORM --------------------------------------------------- +# +# A whitelist of whole shapes. Anything that mentions the output and is not one +# of these is a defect — including spellings this gate does not model, which is +# the point: an unrecognized consumer form must never read as a sanctioned one. + +# The aggregator feed maps an intentional docs-only step skip to `success`. The +# pattern is assembled from the sanctioned form so the two cannot drift apart. +feed_prefix="=\${{ ${SKIP_FORM} && 'success' || steps." +feed_suffix=".outcome }}" + +refjobs="" +ref_count=0 +gated_ordinals="" +feed_targets="" +while IFS="$TAB" read -r refjob reford kind text; do + [[ -n "$refjob" ]] || continue + ref_count=$((ref_count + 1)) + if [[ "$refjob" != "$RESOLVER_JOB" ]] && ! has_line "$refjobs" "$refjob"; then + refjobs+="$refjob"$'\n' + fi + + # An expression may be written bare or wrapped in ${{ }}. The single quotes + # are the point: these are literal workflow delimiters, not shell expansions. + bare="$text" + # shellcheck disable=SC2016 + if [[ "$bare" == '${{ '*' }}' ]]; then + # shellcheck disable=SC2016 + bare="${bare#'${{ '}" + bare="${bare%' }}'}" + fi + + case "$kind" in + stepif) + if [[ "$bare" != "$WORK_FORM" && "$bare" != "$SKIP_FORM" ]]; then + report "ONE CONSUMER FORM: job '$refjob' gates a step on an unsanctioned condition: if: $text" + report " Use \"$WORK_FORM\" to do the work, or \"$SKIP_FORM\" to report it not applicable. Both are plain equality against the only two values the output can hold; a negation, a truthiness test, or an index-syntax spelling is the polarity decision this contract removes." + elif [[ "$bare" == "$WORK_FORM" ]]; then + gated_ordinals+="${refjob}${TAB}${reford}"$'\n' + fi + ;; + jobif) + report "ONE CONSUMER FORM: job '$refjob' reads $OUTPUT_NAME in a JOB-level condition: if: $text. That skips the whole job, and a skipped required lane leaves its check Pending rather than red. Gate the job's STEPS instead, so the lane always runs and reports." + ;; + *) + # An aggregator feed entry: `=${{ && 'success' || steps..outcome }}` + ok_feed=0 + if [[ "$text" == *"$feed_prefix"*"$feed_suffix" ]]; then + name="${text%%=*}" + tail_part="${text#*"$feed_prefix"}" + stepref="${tail_part%"$feed_suffix"}" + if [[ -n "$name" && "$text" == "${name}${feed_prefix}${stepref}${feed_suffix}" && + "$name" != *' '* && "$stepref" != *' '* && "$stepref" != *'{'* ]]; then + ok_feed=1 + fi + fi + if [[ "$ok_feed" -eq 0 ]]; then + report "ONE CONSUMER FORM: job '$refjob' reads $OUTPUT_NAME outside a step condition and outside the aggregator feed template: $text" + else + feed_targets+="${refjob}${TAB}${stepref}${TAB}${name}"$'\n' + fi + ;; + esac + + if has_line "$REC_USES" "$refjob"; then + report "ONE CONSUMER FORM: job '$refjob' delegates to a reusable workflow and reads $OUTPUT_NAME. Such a job has no steps of its own, so its polarity decision would live in a file this gate never opens." + fi +done <<<"$REC_REF" + +# --- 5b. THE FEED MIRRORS A REAL GATE --------------------------------------- +# +# The aggregator feed maps a step's intentional docs-only skip to `success`. +# That is only honest for a step that PROVABLY did not run — one carrying the +# work gate. An override on an ungated step would mask a genuine failure as +# success on every docs-only diff, and it fails open: forgetting an override +# feeds a raw `skipped` and reds the aggregate, but adding an unpaired one goes +# quietly green. Pin the pairing rather than trusting the two lists to stay +# aligned by eye. + +gated_ids="" +while IFS="$TAB" read -r gjob gord; do + [[ -n "$gjob" ]] || continue + while IFS="$TAB" read -r sjob sord sid; do + [[ "$sjob" == "$gjob" && "$sord" == "$gord" ]] || continue + gated_ids+="${gjob}${TAB}${sid}"$'\n' + done <<<"$REC_STEPID" +done <<<"$gated_ordinals" + +while IFS="$TAB" read -r fjob fstep fname; do + [[ -n "$fjob" ]] || continue + if ! has_line "$gated_ids" "${fjob}${TAB}${fstep}"; then + report "THE FEED MIRRORS A REAL GATE: job '$fjob' maps '$fname' to success on a docs-only diff by reading step '$fstep', but no step with that id is gated on \"$WORK_FORM\". Mapping an ungated step's outcome to success hides a real failure instead of reporting a step that provably did not run." + fi +done <<<"$feed_targets" + +# --- 5c. NO JOB-LEVEL CONDITION ON A CONSUMER ------------------------------- +# +# A consumer reaches the published output through `needs`, so it runs only when +# the resolver succeeded — which is what makes the output's domain exactly +# {'true','false'} and the two sanctioned forms exact complements. A job-level +# condition breaks that: `if: always()` or `if: ${{ !cancelled() }}` lets the job +# run when the resolver did NOT succeed, where the output is the empty string. +# Both sanctioned forms are then false, so the work steps AND their +# not-applicable reporter all skip, and the lane goes green having done nothing. +# The condition need not mention the output to cause this, so this check does not +# look at what it says. + +while IFS= read -r refjob; do + [[ -n "$refjob" ]] || continue + while IFS="$TAB" read -r cjob ctext; do + [[ "$cjob" == "$refjob" ]] || continue + report "NO JOB-LEVEL CONDITION ON A CONSUMER: job '$refjob' reads $OUTPUT_NAME and carries a job-level condition: if: $ctext. If that condition ever lets the job run when '$RESOLVER_JOB' did not succeed, $OUTPUT_NAME is the empty string, both sanctioned forms are false, and the lane reports success having run nothing. Gate the steps and let the needs edge decide whether the job runs at all." + done <<<"$REC_JOBIF" +done <<<"$refjobs" + +# --- 6. EDGE DECLARED ------------------------------------------------------- + +consumer_count=0 +while IFS= read -r refjob; do + [[ -n "$refjob" ]] || continue + consumer_count=$((consumer_count + 1)) + declared=0 + while IFS="$TAB" read -r njob ntext; do + [[ "$njob" == "$refjob" ]] || continue + # `needs: [a, b]`, `needs: ['a']`, `needs: a`, each possibly with a comment. + normalized="${ntext%%#*}" + normalized="${normalized//[/ }" + normalized="${normalized//]/ }" + normalized="${normalized//\"/ }" + normalized="${normalized//\'/ }" + normalized="${normalized//,/ }" + for entry in $normalized; do + [[ "$entry" == "$RESOLVER_JOB" ]] && declared=1 + done + done <<<"$REC_NEEDSFLOW" + has_line "$REC_NEEDSITEM" "${refjob}${TAB}${RESOLVER_JOB}" && declared=1 + if [[ "$declared" -eq 0 ]]; then + report "EDGE DECLARED: job '$refjob' reads $REFERENCE but does not declare '$RESOLVER_JOB' in its needs. The expression would evaluate to an empty string, skipping both the work step and its not-applicable reporter." + fi +done <<<"$refjobs" + +# --- 7. CONTRACT IS LIVE ---------------------------------------------------- + +if [[ "$consumer_count" -eq 0 ]]; then + report "CONTRACT IS LIVE: job '$RESOLVER_JOB' resolves the docs-only scope but no job reads $REFERENCE. Either the consumers stopped gating (and every lane now runs unconditionally), or they read it by a spelling this gate does not recognize. Reporting a well-formed contract over zero references would be the nominal closure this gate exists to deny." +fi + +# --- verdict ---------------------------------------------------------------- + +if [[ "$errors" -ne 0 ]]; then + echo "check-docs-only-gate: $errors contract defect(s) in $WORKFLOW" >&2 + exit 1 +fi + +echo "check-docs-only-gate: $WORKFLOW — scope resolved once in '$RESOLVER_JOB'; $ref_count reference(s) across $consumer_count consumer job(s), all in the sanctioned form" +exit 0 diff --git a/scripts/check-docs-only-gate.test.sh b/scripts/check-docs-only-gate.test.sh new file mode 100755 index 0000000000..1585634966 --- /dev/null +++ b/scripts/check-docs-only-gate.test.sh @@ -0,0 +1,445 @@ +#!/usr/bin/env bash +# Self-test for scripts/check-docs-only-gate.sh +# +# The gate exists to replace a prose comment that asserted the docs-only +# short-circuit was fail-closed. A gate that only ever passes would be the same +# nominal closure in a new costume, so every case below breaks exactly one +# property of a known-good fixture and asserts the gate NAMES that property. +# +# The known-good fixture deliberately carries the shapes a structural reader is +# fragile on — a comment inside a job body, a job key with a trailing comment, a +# block scalar whose body contains lines that look like workflow structure, a +# quoted flow `needs:`, and a reusable-workflow job — because a gate that only +# ever sees tidy input is only ever tested against tidy input. The EVASION +# section then attacks the gate directly: each case is a workflow that is +# genuinely broken at runtime and must not be reported as satisfied. +# +# Fixtures are plain files under mktemp, addressed by ABSOLUTE path: the gate +# cds to the git toplevel, so an absolute fixture path resolves from anywhere and +# no scratch git repo is needed. That is deliberate — a fixture repo would need +# `git -C config user.*`, and the un-scoped form of that command writes the +# test identity into the CALLER's repo config (claude-code-plugins#2839). +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +GATE="$ROOT/scripts/check-docs-only-gate.sh" +DETECTOR="$ROOT/scripts/check-docs-only.sh" +failures=0 + +ok() { printf 'ok - %s\n' "$1"; } +fail() { + printf 'not ok - %s\n' "$1" >&2 + failures=$((failures + 1)) +} + +scratch="$(mktemp -d)" +trap 'rm -rf "$scratch"' EXIT + +expect() { + local label="$1" want_rc="$2" want_text="$3" + shift 3 + local out rc + out="$(bash "$GATE" "$@" 2>&1)" && rc=0 || rc=$? + if [[ "$rc" -ne "$want_rc" ]]; then + fail "$label: expected rc=$want_rc got rc=$rc out='$out'" + return + fi + if [[ -n "$want_text" && "$out" != *"$want_text"* ]]; then + fail "$label: expected output to contain '$want_text', got '$out'" + return + fi + ok "$label" +} + +# --- fixture transforms ----------------------------------------------------- +# +# Exact-substring transforms via awk index(), never regex: the fixture bodies +# carry `${{ }}`, quotes and dots, and a regex dialect difference between GNU +# and BSD tools must not decide whether a test case is even built. + +xform_delete() { awk -v m="$2" 'index($0, m) == 0' "$1" >"$3"; } +xform_replace_line() { awk -v m="$2" -v r="$3" '{ if (index($0, m) > 0) print r; else print }' "$1" >"$4"; } +xform_insert_after() { awk -v m="$2" -v r="$3" '{ print; if (index($0, m) > 0) print r }' "$1" >"$4"; } +xform_append() { cp "$1" "$3" && printf '%s\n' "$2" >>"$3"; } + +# --- the known-good fixture ------------------------------------------------- + +base="$scratch/base.yml" +cat >"$base" <<'YAML' +name: ci + +on: + pull_request: + +permissions: + contents: read + +jobs: + # A leading comment block, the way the real file carries them. + scope: + runs-on: ubuntu-24.04 + outputs: + run_full: ${{ steps.detect.outputs.docs_only != 'true' }} + # A comment INSIDE the job body, between two mapping keys. + steps: + - name: Check out + uses: actions/checkout@v7 + - name: Test the docs-only detector + run: bash scripts/check-docs-only.test.sh + # A comment between two steps. + - name: Detect a docs-only diff + id: detect + if: github.event_name == 'pull_request' + continue-on-error: true + env: + BASE_REF: ${{ github.base_ref }} + run: scripts/check-docs-only.sh "origin/$BASE_REF" + + # A consumer, gated at the step level so the job itself never skips. + alpha: + needs: [scope] + runs-on: ubuntu-24.04 + steps: + - name: Do the work + id: work + if: needs.scope.outputs.run_full == 'true' + run: echo work + - name: A block scalar whose body looks like workflow structure + run: | + echo " not-a-job: this line lives inside a block scalar" + echo " - name: neither is this" + echo " if: always()" + - name: Report not applicable to a docs-only diff + if: needs.scope.outputs.run_full == 'false' + run: echo not-applicable + - name: Aggregate + if: always() + env: + CHECK_RESULTS: | + alpha-check=${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.work.outcome }} + run: echo aggregate + + # A second consumer using the block-sequence needs form, with a trailing + # comment on its job key. + beta: # a consumer + needs: + - scope # the resolver + runs-on: ubuntu-24.04 + steps: + - name: Do the work + if: needs.scope.outputs.run_full == 'true' + run: echo work + + # A third consumer using a quoted flow sequence. + gamma: + needs: ['scope'] + runs-on: ubuntu-24.04 + steps: + - name: Do the work + if: needs.scope.outputs.run_full == 'true' + run: echo work + + # A reusable-workflow job that does NOT read the output. + delta: + uses: some-org/some-repo/.github/workflows/lint.yml@v1 + + ci-status: + needs: + - scope + - alpha + - beta + - gamma + - delta + if: ${{ !cancelled() }} + runs-on: ubuntu-24.04 + steps: + - name: Aggregate lane results + run: echo ok +YAML + +expect "known-good fixture satisfies the contract" 0 "scope resolved once" --check "$base" + +# --- 1. SINGLE RESOLUTION --------------------------------------------------- + +f="$scratch/second-resolution.yml" +xform_replace_line "$base" "run: echo not-applicable" ' run: scripts/check-docs-only.sh "origin/main"' "$f" +expect "a consumer re-running the detector is a second resolution" 1 "invoked from 2 job(s)" --check "$f" + +# The detector reached by a different spelling is the same second answer. +f="$scratch/second-resolution-relative.yml" +xform_replace_line "$base" "run: echo not-applicable" ' run: cd scripts && bash check-docs-only.sh origin/main' "$f" +expect "a relative-path re-resolution is still a second resolution" 1 "invoked from 2 job(s)" --check "$f" + +# A run: body is a block scalar, and a re-resolution hidden in one still counts. +f="$scratch/second-resolution-scalar.yml" +xform_insert_after "$base" 'echo " - name: neither is this"' ' bash scripts/check-docs-only.sh origin/main' "$f" +expect "a re-resolution inside a block scalar is still seen" 1 "invoked from 2 job(s)" --check "$f" + +f="$scratch/step-output.yml" +# shellcheck disable=SC2016 # `${{ }}` must reach the fixture verbatim. +xform_replace_line "$base" "run: echo not-applicable" ' run: echo ${{ steps.detect.outputs.docs_only }}' "$f" +expect "a surviving step-level docs_only read is a second answer" 1 "step-level docs_only output is still read" --check "$f" + +# --- 2. FAIL-CLOSED DEFAULT ------------------------------------------------- +# +# The inverted expression is the exact defect the contract exists to prevent: +# it renders an UNSET detector output as `false`, so a detector that never ran +# would short-circuit every consumer instead of running the full suite. + +f="$scratch/inverted-expression.yml" +xform_replace_line "$base" "run_full: " " run_full: \${{ steps.detect.outputs.docs_only == 'true' }}" "$f" +expect "an inverted output expression is rejected" 1 "FAIL-CLOSED DEFAULT" --check "$f" + +f="$scratch/no-output.yml" +xform_delete "$base" "run_full: \${{" "$f" +expect "a resolving job publishing no output is rejected" 1 "publishes no 'run_full' output" --check "$f" + +# A commented-out expression is prose, not a contract. +f="$scratch/commented-output.yml" +xform_replace_line "$base" "run_full: \${{" " # run_full: \${{ steps.detect.outputs.docs_only != 'true' }}" "$f" +expect "a commented-out output expression does not satisfy the contract" 1 "publishes no 'run_full' output" --check "$f" + +# --- 3. FAILURE IS ABSORBED ------------------------------------------------- + +f="$scratch/detect-fails-job.yml" +xform_delete "$base" "continue-on-error: true" "$f" +expect "a detect step that can fail its own job is rejected" 1 "missing 'continue-on-error: true'" --check "$f" + +f="$scratch/no-detect-step.yml" +xform_delete "$base" "id: detect" "$f" +expect "a resolving job with no detect step is rejected" 1 "no step declares 'id: detect'" --check "$f" + +# A sibling step must not lend its continue-on-error to the detect step. +f="$scratch/coe-on-sibling.yml" +xform_delete "$base" "continue-on-error: true" "$scratch/_nocoe.yml" +xform_insert_after "$scratch/_nocoe.yml" "- name: Check out" " id: detect-nothing + continue-on-error: true" "$f" +expect "continue-on-error on a sibling step does not satisfy the detect step" 1 "missing 'continue-on-error: true'" --check "$f" + +# The detect step must be the step that actually runs the detector. +f="$scratch/detect-runs-nothing.yml" +# shellcheck disable=SC2016 # `$BASE_REF` is fixture text, not a shell variable. +xform_replace_line "$base" 'run: scripts/check-docs-only.sh "origin/$BASE_REF"' ' run: echo nothing' "$scratch/_nodetect.yml" +xform_insert_after "$scratch/_nodetect.yml" "run: bash scripts/check-docs-only.test.sh" ' run: scripts/check-docs-only.sh "origin/main"' "$f" +expect "a detect step that runs something else is rejected" 1 "does not invoke the docs-only detector" --check "$f" + +# A detect step whose real invocation is commented out, emitting a hardcoded +# flag instead, is the maximal false green: every consumer short-circuits on +# every event. The comment must not read as the invocation. +f="$scratch/decoy-invocation.yml" +# shellcheck disable=SC2016 # fixture text; `$BASE_REF` and `$GITHUB_OUTPUT` stay literal. +xform_replace_line "$base" 'run: scripts/check-docs-only.sh "origin/$BASE_REF"' ' run: | + # was: scripts/check-docs-only.sh "origin/$BASE_REF" + echo "docs_only=true" >> "$GITHUB_OUTPUT"' "$f" +expect "a commented-out detector call does not count as invoking it" 1 "does not invoke the docs-only detector" --check "$f" + +# The mirror of that case: prose naming the detector is not a second resolution. +f="$scratch/prose-mentions-detector.yml" +xform_insert_after "$base" 'echo " not-a-job: this line lives inside a block scalar"' ' # the scope is resolved once, by scripts/check-docs-only.sh' "$f" +expect "prose naming the detector is not a second resolution" 0 "scope resolved once" --check "$f" + +# --- 4. SELF-TEST INTACT ---------------------------------------------------- + +f="$scratch/no-self-test.yml" +xform_delete "$base" "check-docs-only.test.sh" "$f" +expect "dropping the detector self-test is rejected" 1 "does not run the detector's own suite" --check "$f" + +f="$scratch/commented-self-test.yml" +xform_replace_line "$base" "run: bash scripts/check-docs-only.test.sh" " # run: bash scripts/check-docs-only.test.sh" "$f" +expect "a commented-out self-test does not satisfy the contract" 1 "does not run the detector's own suite" --check "$f" + +f="$scratch/soft-self-test.yml" +xform_insert_after "$base" "- name: Test the docs-only detector" " continue-on-error: true" "$f" +expect "a self-test that cannot turn the job red is rejected" 1 "carries continue-on-error" --check "$f" + +# --- 5. ONE CONSUMER FORM --------------------------------------------------- +# +# The historical shape. `!= 'true'` is behaviourally correct here, and is still +# rejected: the contract is that there is exactly one spelling to copy, so the +# next consumer has no polarity decision to make and no comment to keep true. + +f="$scratch/negated-form.yml" +xform_replace_line "$base" "if: needs.scope.outputs.run_full == 'false'" " if: needs.scope.outputs.run_full != 'true'" "$f" +expect "the inverse-polarity consumer form is rejected" 1 "unsanctioned condition" --check "$f" + +f="$scratch/truthy-form.yml" +xform_replace_line "$base" "if: needs.scope.outputs.run_full == 'false'" " if: needs.scope.outputs.run_full" "$f" +expect "a bare truthiness consumer form is rejected" 1 "unsanctioned condition" --check "$f" + +# An outer negation whose inner half IS the sanctioned string. A substring +# search would strip the sanctioned half and see nothing left to complain about. +f="$scratch/wrapped-negation.yml" +xform_replace_line "$base" "if: needs.scope.outputs.run_full == 'false'" " if: \${{ !(needs.scope.outputs.run_full == 'false') }}" "$f" +expect "a negation wrapped around the sanctioned form is rejected" 1 "unsanctioned condition" --check "$f" + +# Index syntax is legal Actions and reads identically at runtime; it is still an +# unmodelled spelling, and an unmodelled spelling must never read as sanctioned. +# The three variants below bracket a different part of the accessor each time, +# because a matcher that models only the spelling it expects does not REJECT the +# others — it cannot see them at all, which is the opposite of rejecting them. +f="$scratch/index-syntax-output.yml" +xform_replace_line "$base" "if: needs.scope.outputs.run_full == 'false'" " if: needs.scope.outputs['run_full'] != 'true'" "$f" +expect "an index-syntax output name is rejected rather than ignored" 1 "unsanctioned condition" --check "$f" + +f="$scratch/index-syntax-whole.yml" +xform_replace_line "$base" "if: needs.scope.outputs.run_full == 'false'" " if: needs['scope']['outputs']['run_full'] == 'false'" "$f" +expect "a fully bracketed accessor is rejected rather than ignored" 1 "unsanctioned condition" --check "$f" + +# Actions context accessors are case-insensitive, so this resolves at runtime. +f="$scratch/case-variant.yml" +xform_replace_line "$base" "if: needs.scope.outputs.run_full == 'false'" " if: needs.SCOPE.outputs.run_full == 'false'" "$f" +expect "a case-variant accessor is rejected rather than ignored" 1 "unsanctioned condition" --check "$f" + +# Gating the JOB skips the lane, which leaves a required check Pending. +f="$scratch/job-level-gate.yml" +xform_insert_after "$base" " gamma:" " if: needs.scope.outputs.run_full == 'true'" "$f" +expect "a job-level condition on the output is rejected" 1 "JOB-level condition" --check "$f" + +# A reusable-workflow job cannot gate at step level, so its polarity decision +# would live in a file this gate never opens. +f="$scratch/reusable-consumer.yml" +xform_insert_after "$base" " uses: some-org/some-repo/.github/workflows/lint.yml@v1" " if: needs.scope.outputs.run_full == 'true'" "$f" +expect "a reusable-workflow job reading the output is rejected" 1 "delegates to a reusable workflow" --check "$f" + +# A block-scalar body is a script, never a gate. A condition-shaped line inside +# one must not be credited as a step condition — the whole point of skipping +# scalar structure while still reading scalar content. +f="$scratch/gate-inside-scalar.yml" +xform_replace_line "$base" 'echo " if: always()"' ' echo " if: needs.scope.outputs.run_full == '"'"'true'"'"'"' "$f" +expect "a condition-shaped line inside a block scalar is not a step condition" 1 "outside a step condition" --check "$f" + +# The aggregator feed has its own exact template; a mutated one is not it. +f="$scratch/mutated-feed.yml" +xform_replace_line "$base" "alpha-check=" " alpha-check=\${{ needs.scope.outputs.run_full == 'true' && 'success' || steps.work.outcome }}" "$f" +expect "a mutated aggregator feed entry is rejected" 1 "outside the aggregator feed template" --check "$f" + +# --- 5b. THE FEED MIRRORS A REAL GATE --------------------------------------- +# +# Forgetting a feed override is fail-closed (the raw `skipped` reds the +# aggregate). Adding one for a step that is NOT gated is fail-open: on every +# docs-only diff that step's real outcome is replaced by `success`. Only the +# second direction needs a gate, and this is it. +f="$scratch/feed-without-gate.yml" +xform_replace_line "$base" "alpha-check=" " alpha-check=\${{ needs.scope.outputs.run_full == 'false' && 'success' || steps.ungated.outcome }}" "$f" +expect "a feed override for an ungated step is rejected" 1 "THE FEED MIRRORS A REAL GATE" --check "$f" + +# --- 5c. NO JOB-LEVEL CONDITION ON A CONSUMER ------------------------------- +# +# The two sanctioned forms are exact complements only while the output is set, +# which is guaranteed by the consumer running solely on a successful resolver. +# A job-level condition can let the job run anyway, and then BOTH forms are +# false: every gated step and its not-applicable reporter skip together, and the +# lane reports success having done nothing. The condition need not mention the +# output to do this. +f="$scratch/consumer-always.yml" +xform_insert_after "$base" " gamma:" " if: always()" "$f" +expect "a consumer carrying an unconditional job-level if is rejected" 1 "NO JOB-LEVEL CONDITION ON A CONSUMER" --check "$f" + +f="$scratch/consumer-not-cancelled.yml" +xform_insert_after "$base" " gamma:" " if: \${{ !cancelled() }}" "$f" +expect "a consumer carrying !cancelled() is rejected" 1 "NO JOB-LEVEL CONDITION ON A CONSUMER" --check "$f" + +# The aggregate's own job-level condition is not a consumer's, and must stand. +expect "the aggregate's own job-level condition is untouched" 0 "scope resolved once" --check "$base" + +# --- 6. EDGE DECLARED ------------------------------------------------------- + +f="$scratch/missing-edge.yml" +xform_delete "$base" "needs: [scope]" "$f" +expect "a consumer reading the output without the needs edge is rejected" 1 "EDGE DECLARED" --check "$f" + +# --- 7. CONTRACT IS LIVE ---------------------------------------------------- +# +# Zero references must never read as "every reference is well formed". + +f="$scratch/no-consumers.yml" +xform_delete "$base" "needs.scope.outputs.run_full" "$f" +expect "a resolver nobody reads is rejected" 1 "CONTRACT IS LIVE" --check "$f" + +# --- fail closed on shape and usage ----------------------------------------- + +f="$scratch/bad-shape.yml" +xform_insert_after "$base" "jobs:" " not a job key" "$f" +expect "an unparsed workflow shape is inconclusive, never a pass" 2 "unrecognized workflow shape" --check "$f" + +f="$scratch/bad-needs-entry.yml" +xform_replace_line "$base" " - scope # the resolver" " - [scope]" "$f" +expect "an unmodelled needs entry is inconclusive, never a bogus defect" 2 "unsupported needs entry" --check "$f" + +# A sequence item at the key's own indent is valid YAML and must not read as a +# missing edge — a gate that cries wolf on a legal shape gets switched off. +f="$scratch/shallow-needs-item.yml" +xform_replace_line "$base" " - scope # the resolver" " - scope" "$f" +expect "a same-indent needs item is a declared edge, not a defect" 0 "scope resolved once" --check "$f" + +# `continue-on-error` decides whether a failure is absorbed. An expression-valued +# one cannot be judged without evaluating it, so the gate must refuse rather than +# guess in either direction. +f="$scratch/expression-coe.yml" +xform_replace_line "$base" " continue-on-error: true" " continue-on-error: \${{ true }}" "$f" +expect "an expression-valued continue-on-error is inconclusive, never a pass" 2 "not a literal true/false" --check "$f" + +f="$scratch/no-jobs.yml" +xform_delete "$base" "jobs:" "$f" +expect "a file with no jobs mapping is inconclusive" 2 "no jobs: mapping found" --check "$f" + +f="$scratch/no-resolver.yml" +xform_replace_line "$base" " scope:" " renamed-scope:" "$f" +expect "a missing resolving job is inconclusive, never a pass" 2 "is not defined" --check "$f" + +expect "a missing workflow file is inconclusive" 2 "workflow not found" --check "$scratch/absent.yml" +expect "a bare invocation prints usage" 2 "usage:" +expect "an unknown mode prints usage" 2 "usage:" --lint +expect "an excess argument prints usage" 2 "usage:" --check "$base" ci-status + +# --- the live contract ------------------------------------------------------ +# +# The point of the suite: the shipped workflow satisfies what the cases above +# prove the gate can detect. This is what makes the fixture cases load-bearing +# rather than a private exercise. + +expect "the shipped ci.yml satisfies the contract" 0 "scope resolved once" --check ".github/workflows/ci.yml" + +# --- the half of the fail-closed proof that lives in the detector ------------ +# +# The gate pins the workflow-side half: the published output is derived by +# `docs_only != 'true'`, so an UNSET detector output resolves to "run the full +# suite". That is only a real guarantee if a detector that fails leaves the +# output genuinely unset. These cases close that half against the real script. +# +# check-docs-only.test.sh covers the RESOLVABLE fail-closed paths (an unreadable +# allowlist, an unresolvable base ref) emitting docs_only=false. What is +# asserted here is what happens when the script cannot complete at all. + +gho="$scratch/github-output" + +# A usage abort returns before the emit path is reachable. +: >"$gho" +if GITHUB_OUTPUT="$gho" bash "$DETECTOR" >/dev/null 2>&1; then + fail "detector invoked with no base-ref argument should exit non-zero" +elif [[ -s "$gho" ]]; then + fail "a detector usage abort wrote to GITHUB_OUTPUT: $(cat "$gho")" +else + ok "a detector usage abort leaves docs_only unset, which run_full renders as 'true'" +fi + +# An unwritable $GITHUB_OUTPUT is the case the workflow comment names. The +# script's own header claims a non-zero exit here; it does not in fact exit +# non-zero, because emit() does not check its redirect. That is why the +# workflow's guarantee is built on the output being UNSET rather than on the +# script's exit status: the assertion below is the one the contract rests on. +unwritable="$scratch/no-such-dir/output" +GITHUB_OUTPUT="$unwritable" bash "$DETECTOR" "origin/main" >/dev/null 2>&1 +if [[ -e "$unwritable" ]]; then + fail "expected no output file to be produced at an unwritable path" +else + ok "an unwritable GITHUB_OUTPUT leaves docs_only unset, which run_full renders as 'true'" +fi + +# --- verdict ---------------------------------------------------------------- + +if [[ "$failures" -ne 0 ]]; then + printf '\n%s test(s) failed\n' "$failures" >&2 + exit 1 +fi +printf '\nall tests passed\n' diff --git a/scripts/check-docs-only.sh b/scripts/check-docs-only.sh index 66b6f778c6..d8fd861170 100755 --- a/scripts/check-docs-only.sh +++ b/scripts/check-docs-only.sh @@ -22,7 +22,16 @@ # or empty allowlist, or an empty diff all emit docs_only=false with a stderr # note — never a skip we cannot justify. Exit status is 0 for the normal has-code # case (a code PR is not an error) and for every fail-closed path; non-zero only -# on usage error (no base-ref argument) or an unwritable output file. +# on usage error (no base-ref argument) or a failure to reach this script's own +# directory and shared libraries. +# +# An unwritable $GITHUB_OUTPUT is NOT one of those non-zero cases: emit() does +# not check its redirect, so the flag simply never reaches the consumer. That is +# deliberate rather than overlooked, and it is why the workflow's guarantee is +# built on the flag being UNSET rather than on this script's exit status — the +# `scope` job derives its published output as `docs_only != 'true'`, so an +# absent flag resolves toward running the full suite. Asserted by +# scripts/check-docs-only-gate.test.sh, not by this paragraph. # # DOCS_ONLY_ALLOWLIST overrides the allowlist path (test injection). set -uo pipefail From 4607369b83e06d19ae90fadbf333fcc4d08af3c4 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:13:41 +0000 Subject: [PATCH 2/2] fix(ci): rename ordinal locals and count detector invocations Co-authored-by: Kyle Sexton --- scripts/check-docs-only-gate.sh | 18 +++++++++++------- scripts/check-docs-only-gate.test.sh | 7 +++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/scripts/check-docs-only-gate.sh b/scripts/check-docs-only-gate.sh index 14177289d5..b522399759 100755 --- a/scripts/check-docs-only-gate.sh +++ b/scripts/check-docs-only-gate.sh @@ -431,8 +431,10 @@ fi invoker_jobs="" invoker_count=0 +invoke_count=0 while IFS="$TAB" read -r ijob _; do [[ -n "$ijob" ]] || continue + invoke_count=$((invoke_count + 1)) has_line "$invoker_jobs" "$ijob" && continue invoker_jobs+="$ijob"$'\n' invoker_count=$((invoker_count + 1)) @@ -440,6 +442,8 @@ done <<<"$REC_INVOKE" if [[ "$invoker_count" -ne 1 ]]; then report "SINGLE RESOLUTION: the docs-only detector is invoked from $invoker_count job(s) [$(uniq_list "$invoker_jobs")]. It must be resolved exactly once, in '$RESOLVER_JOB', and read from there." +elif [[ "$invoke_count" -ne 1 ]]; then + report "SINGLE RESOLUTION: the docs-only detector is invoked $invoke_count times in job [$(uniq_list "$invoker_jobs")]. Count invocation records; the sole invocation must belong to the resolver's detect step, not a second call that can overwrite docs_only." elif ! has_line "$invoker_jobs" "$RESOLVER_JOB"; then report "SINGLE RESOLUTION: the docs-only detector is invoked from [$(uniq_list "$invoker_jobs")], not from the resolving job '$RESOLVER_JOB'." fi @@ -480,11 +484,11 @@ fi detect_job="" detect_ordinal="" -while IFS="$TAB" read -r sjob sord sid; do +while IFS="$TAB" read -r sjob step_ord sid; do [[ "$sid" == "$DETECT_STEP_ID" ]] || continue [[ -n "$detect_job" ]] && continue detect_job="$sjob" - detect_ordinal="$sord" + detect_ordinal="$step_ord" done <<<"$REC_STEPID" if [[ -z "$detect_job" ]]; then @@ -503,10 +507,10 @@ fi # --- 4. SELF-TEST INTACT ---------------------------------------------------- selftest_ordinal="" -while IFS="$TAB" read -r sjob sord; do +while IFS="$TAB" read -r sjob step_ord; do [[ "$sjob" == "$RESOLVER_JOB" ]] || continue [[ -n "$selftest_ordinal" ]] && continue - selftest_ordinal="$sord" + selftest_ordinal="$step_ord" done <<<"$REC_SELFTEST" if [[ -z "$selftest_ordinal" ]]; then @@ -595,10 +599,10 @@ done <<<"$REC_REF" # aligned by eye. gated_ids="" -while IFS="$TAB" read -r gjob gord; do +while IFS="$TAB" read -r gjob gate_ord; do [[ -n "$gjob" ]] || continue - while IFS="$TAB" read -r sjob sord sid; do - [[ "$sjob" == "$gjob" && "$sord" == "$gord" ]] || continue + while IFS="$TAB" read -r sjob step_ord sid; do + [[ "$sjob" == "$gjob" && "$step_ord" == "$gate_ord" ]] || continue gated_ids+="${gjob}${TAB}${sid}"$'\n' done <<<"$REC_STEPID" done <<<"$gated_ordinals" diff --git a/scripts/check-docs-only-gate.test.sh b/scripts/check-docs-only-gate.test.sh index 1585634966..8142f0a66d 100755 --- a/scripts/check-docs-only-gate.test.sh +++ b/scripts/check-docs-only-gate.test.sh @@ -175,6 +175,13 @@ f="$scratch/second-resolution-scalar.yml" xform_insert_after "$base" 'echo " - name: neither is this"' ' bash scripts/check-docs-only.sh origin/main' "$f" expect "a re-resolution inside a block scalar is still seen" 1 "invoked from 2 job(s)" --check "$f" +# Two invocations in the SAME job used to pass: the loop deduplicated by job +# and reported one resolution, while the last write to docs_only won. +f="$scratch/second-invocation-same-job.yml" +# shellcheck disable=SC2016 # `$BASE_REF` is fixture text, not a shell variable. +xform_insert_after "$base" 'run: scripts/check-docs-only.sh "origin/$BASE_REF"' ' run: scripts/check-docs-only.sh "origin/HEAD"' "$f" +expect "a second invocation in the resolver job is a second resolution" 1 "invoked 2 times" --check "$f" + f="$scratch/step-output.yml" # shellcheck disable=SC2016 # `${{ }}` must reach the fixture verbatim. xform_replace_line "$base" "run: echo not-applicable" ' run: echo ${{ steps.detect.outputs.docs_only }}' "$f"