From 8f7b8ae573fcff6ccf09d8455373edaabe7e74d3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 19:05:08 +0900 Subject: [PATCH 1/7] feat(workflows): add reusable r-package-check.yml for kaefa/nonnest2 kaefa and nonnest2 each carried a hand-copied R-CMD-check.yaml generated from the same upstream r-lib template. Consolidate the shared checkout -> setup-pandoc -> [setup-tinytex] -> setup-r -> setup-r-dependencies -> check-r-package sequence into one workflow_call workflow with inputs for the fields that genuinely vary per repo (r_matrix, needs_tinytex, extra_packages, check_args, pre_check_script). See docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md and docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md for the full field-by-field audit, including two non-uniform fields (extra-packages, check-r-package args) the initial survey missed. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/r-package-check.yml | 118 +++++++++++++ ...d-check-reusable-workflow-consolidation.md | 148 ++++++++++++++++ ...d-check-reusable-workflow-consolidation.md | 167 ++++++++++++++++++ ...ackage_check_reusable_workflow_contract.py | 108 +++++++++++ 4 files changed, 541 insertions(+) create mode 100644 .github/workflows/r-package-check.yml create mode 100644 docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md create mode 100644 docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md create mode 100644 tests/test_r_package_check_reusable_workflow_contract.py diff --git a/.github/workflows/r-package-check.yml b/.github/workflows/r-package-check.yml new file mode 100644 index 0000000000..a6cb120585 --- /dev/null +++ b/.github/workflows/r-package-check.yml @@ -0,0 +1,118 @@ +# Reusable R CMD check (workflow_call), derived from +# https://github.com/r-lib/actions/tree/v2/examples +# +# Consolidates the near-identical R-CMD-check.yaml files kaefa and nonnest2 +# each carried (r-lib's standard actions/checkout -> setup-pandoc -> +# [setup-tinytex] -> setup-r -> setup-r-dependencies -> check-r-package +# sequence). See docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md +# and docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md for the +# per-repo field audit behind these inputs. +# +# The `on: push/pull_request` trigger stays in each calling repo's own thin +# workflow file -- a workflow_call target cannot also be the thing GitHub +# triggers directly on push/PR. +# +# Example caller (.github/workflows/R-CMD-check.yaml in a product repo): +# +# name: R-CMD-check +# on: +# push: +# branches: [main, master] +# pull_request: +# branches: [main, master] +# jobs: +# R-CMD-check: +# uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@main +# with: +# needs_tinytex: true # only if the package builds a PDF vignette +# +name: Reusable R CMD check + +on: + workflow_call: + inputs: + r_matrix: + description: >- + JSON array of {os, r, http-user-agent?} objects for + strategy.matrix.config. Default is a single ubuntu-latest/release + leg; override with a JSON array for a multi-OS/multi-R-version + matrix. + required: false + type: string + default: '[{"os": "ubuntu-latest", "r": "release"}]' + needs_tinytex: + description: "Install r-lib/actions/setup-tinytex before setup-r (needed for a PDF vignette build)." + required: false + type: boolean + default: false + extra_packages: + description: "Value forwarded to setup-r-dependencies's extra-packages input." + required: false + type: string + default: "any::rcmdcheck" + check_args: + description: >- + Value forwarded to check-r-package's args input. Default matches + that action's own upstream default + (c("--no-manual", "--as-cran")); override to change what + rcmdcheck runs (e.g. to skip re-running tests already run in + pre_check_script). + required: false + type: string + default: 'c("--no-manual", "--as-cran")' + pre_check_script: + description: >- + Optional shell commands run in a step between setup-r-dependencies + and check-r-package (e.g. a repo-specific regression test). + Skipped entirely when empty (the default). + required: false + type: string + default: "" + +permissions: + contents: read + +jobs: + R-CMD-check: + runs-on: ${{ matrix.config.os }} + name: ${{ matrix.config.os }} (${{ matrix.config.r }}) + + strategy: + fail-fast: false + matrix: + config: ${{ fromJSON(inputs.r_matrix) }} + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + R_KEEP_PKG_SOURCE: yes + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: r-lib/actions/setup-pandoc@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + + - if: inputs.needs_tinytex + uses: r-lib/actions/setup-tinytex@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + + - uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + with: + r-version: ${{ matrix.config.r }} + http-user-agent: ${{ matrix.config['http-user-agent'] }} + use-public-rspm: true + + - uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + with: + extra-packages: ${{ inputs.extra_packages }} + needs: check + + - if: inputs.pre_check_script != '' + name: Run pre-check script (repo-specific) + run: ${{ inputs.pre_check_script }} + shell: bash + + - uses: r-lib/actions/check-r-package@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + with: + args: ${{ inputs.check_args }} + build_args: 'c("--no-manual")' + error-on: '"error"' + upload-snapshots: true diff --git a/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md b/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md new file mode 100644 index 0000000000..a7459a0ebc --- /dev/null +++ b/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md @@ -0,0 +1,148 @@ +# ADR-0023: Consolidate kaefa/nonnest2 R-CMD-check.yaml into one reusable workflow + +- **Status:** Accepted +- **Date:** 2026-09-02 +- **Scope:** ContextualWisdomLab/.github `.github/workflows/` (new reusable workflow); + ContextualWisdomLab/kaefa and ContextualWisdomLab/nonnest2 `.github/workflows/R-CMD-check.yaml` + (each replaced by a thin `workflow_call` caller) + +## Context + +kaefa and nonnest2 each carry a hand-copied `R-CMD-check.yaml`, both generated +from the same upstream r-lib template +(https://github.com/r-lib/actions/tree/v2/examples): both open with the +identical "Workflow derived from..." header, and both run the identical +`actions/checkout` -> `r-lib/actions/setup-pandoc` -> `r-lib/actions/setup-r` +-> `r-lib/actions/setup-r-dependencies` -> `r-lib/actions/check-r-package` +step sequence with the same `GITHUB_PAT` / `R_KEEP_PKG_SOURCE` env vars and +the same `permissions: contents: read`. This is the same pattern +ADR-0021 named for the hourly review-repair callers: near-duplicated +GitHub Actions YAML that differs only in the fields a `workflow_call` input +was built to carry. + +Reading both files in full (not just the survey that proposed this +consolidation) surfaced two genuinely varying fields the survey had not +named -- `docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md` +records the full field-by-field audit, including these: + +- kaefa's `setup-r-dependencies` installs `any::rcmdcheck` **and** + `any::testthat` (its own regression-test step needs `testthat`); + nonnest2 installs only `any::rcmdcheck`. +- kaefa's `check-r-package` overrides `args: 'c("--no-manual", "--no-tests")'` + (it already ran its package's tests via the regression-test step, so + `R CMD check` itself skips re-running them); nonnest2 omits `args:` + entirely, taking `check-r-package`'s own upstream default, + `c("--no-manual", "--as-cran")`. + +Neither is the kind of difference a survey summary line ("same step +sequence") would show without opening both action's `with:` blocks. +Both are exactly the kind of field a `workflow_call` input handles, so +they do not change the Decision below -- but they are new inputs beyond the +ones the initial proposal named, and are called out here per this +repository's standing convention of not forcing a consolidation past +genuine per-repo variance without naming it (see `docs/CWL-MASTER-CONTEXT.md` +§7 and the precedent this ADR follows, ADR-0021). + +`docs/product-technical-gap-baseline.md` gap-baseline snapshot and IRT-bibliography-set +(named as a plausible third target) returned 404 for a `.github/workflows` +directory during the survey -- it has no CI workflow of this shape yet, so it +is not a target of this change; the reusable workflow is still built openly +so a future R package repo can adopt it without a new ADR. + +## Decision + +1. One new reusable workflow, `.github/workflows/r-package-check.yml` in + this repository, implements the shared r-lib check sequence behind + `workflow_call` inputs: + - `r_matrix` (JSON string, default a single `ubuntu-latest`/`release` + leg) -- becomes `strategy.matrix.config` via `fromJSON()`. + - `needs_tinytex` (boolean, default `false`) -- gates an optional + `r-lib/actions/setup-tinytex` step (nonnest2's PDF vignette needs it; + kaefa does not use it). + - `extra_packages` (string, default `any::rcmdcheck`) -- forwarded to + `setup-r-dependencies`'s `extra-packages` input. + - `check_args` (string, default `c("--no-manual", "--as-cran")`, + matching `check-r-package`'s own upstream default so nonnest2's + behavior is unchanged by omission-turned-explicit) -- forwarded to + `check-r-package`'s `args` input. + - `pre_check_script` (string, default empty -- step skipped) -- an + optional shell step run between dependency setup and the check step, + for kaefa's package-install-then-`testthat::test_file()` regression + check. +2. `GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}`, `R_KEEP_PKG_SOURCE: yes`, + `permissions: contents: read`, `build_args: 'c("--no-manual")'`, + `error-on: '"error"'`, and `upload-snapshots: true` were uniform across + both originals and are hardcoded in the reusable workflow, not exposed + as inputs. +3. The `on: push` / `on: pull_request` trigger (and each repository's own + branch list) stays in each calling repository's own thin + `.github/workflows/R-CMD-check.yaml` -- a `workflow_call` target cannot + itself be the workflow GitHub triggers directly on push/PR, so this + cannot move into the reusable file. kaefa keeps + `[main, master, develop]`; nonnest2 keeps `[main, master]` -- these were + already different before this change and are preserved exactly. +4. Each repository's local file collapses to a thin caller: `on:` (its + existing trigger config, untouched) plus one job, + `uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@main`, + with only that repository's actual non-default `with:` values -- + nonnest2's caller sets only `needs_tinytex: true`; kaefa's sets + `r_matrix`, `extra_packages`, `check_args`, and `pre_check_script` + (all four differ from the reusable workflow's defaults). This follows + the exact `@main`-reference convention `deploy-pages.yml` already + documents for this repository's other reusable workflows. +5. Action version pins are unified to this repository's own current pins + rather than parameterized: `actions/checkout` moves to + `3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1` (kaefa's existing + pin; nonnest2 was on the older `v6.0.2`), and every `r-lib/actions/*` + step moves to `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2` (nonnest2's + existing uniform pin for all of its r-lib steps, and already the pin + kaefa used for three of its four r-lib steps). This is a routine + version-pin bump of the kind Dependabot performs, not a parameterized + per-repo field: no `workflow_call` input exists for "which SHA," and + both repositories converge on whichever pin was already newest/more + uniform in this ecosystem. + +## Consequences + +- Adding a third R package repository (e.g. a future IRT-bibliography-set) + to this pattern is a ~15-line caller file with only its own differing + `with:` values, not a copy-pasted 30+-line workflow. +- kaefa's `setup-pandoc` step, previously pinned to a stray SHA + (`d3c5be51b12e724e68f33216ca3c148b66d5f0b6 # v2`) different from its own + other three r-lib steps -- an inconsistency *within* kaefa's own prior + file, not a genuine cross-repo difference -- now uses the same pin as + every other r-lib step in both repositories, closing that drift as a + side effect of consolidation (same category of incidental fix ADR-0021 + made for Clearfolio's missing job permissions). +- nonnest2's `actions/checkout` pin moves from `v6.0.2` to `v7.0.1` as part + of adopting the shared workflow; `docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md` + records that this is the only originally-unpinned-to-kaefa's-version + action bump this change makes, and that it is a well-tested checkout + action major-version-stable bump, not a behavioral change to the R check + itself. +- Neither kaefa's `develop` branch nor nonnest2's `master` branch has GitHub + branch protection configured (`gh api .../branches/.../protection` -> 404 + for both, verified before writing this ADR), so there is no required + status-check name this change could silently break by changing how the + matrix job's check name is composed. + +## Rejected alternatives + +- **A single shared file with no inputs, hardcoding kaefa's 5-leg matrix + and regression step for both repos.** Rejected: nonnest2 has no + `testthat`-based regression suite step and does not build with a PDF + vignette toolchain matrix; forcing kaefa's shape onto it would run steps + that reference files nonnest2 does not have. +- **Parameterize the action version pins as `workflow_call` inputs.** + Rejected: pin choice is a security/supply-chain decision belonging to + the reusable workflow's own maintainers, not a per-repo product + difference; unifying to one current pin (as this repository already + does for `actions/checkout` in `deploy-pages.yml`, + `pr-review-fix-scheduler.yml`, and 40+ other in-repo workflows) keeps a + single place to bump it later. +- **Leave `check_args` unset by default and require every caller to pass + it explicitly.** Rejected: nonnest2's original file never set `args:` + at all, so defaulting to `check-r-package`'s own upstream default + reproduces nonnest2's exact prior behavior with zero `with:` lines, + rather than forcing every future caller to memorize and repeat + `check-r-package`'s own default. diff --git a/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md new file mode 100644 index 0000000000..4b4fb33761 --- /dev/null +++ b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md @@ -0,0 +1,167 @@ +# R-CMD-check reusable workflow consolidation + +## Decision + +kaefa's and nonnest2's `.github/workflows/R-CMD-check.yaml` files -- both +auto-generated from the same upstream r-lib template +(https://github.com/r-lib/actions/tree/v2/examples) -- are replaced by one +new reusable workflow, `.github/workflows/r-package-check.yml` in this +repository, plus a thin `workflow_call` caller left in place of each +repository's own `R-CMD-check.yaml`. See +[ADR-0023](../adr/0023-r-cmd-check-reusable-workflow-consolidation.md). + +Both original files opened with the same "Workflow derived from..." header +comment and ran the same `actions/checkout` -> `setup-pandoc` -> `setup-r` +-> `setup-r-dependencies` -> `check-r-package` sequence, with the same +`GITHUB_PAT` / `R_KEEP_PKG_SOURCE` env vars and the same +`permissions: contents: read`. A third named candidate, +IRT-bibliography-set, returned 404 for a `.github/workflows` directory +during the survey (`gh api repos/ContextualWisdomLab/IRT-bibliography-set/contents/.github/workflows`) +-- it has no workflow of this shape today, so it is not a target of this +change. + +## Mechanism + +`.github/workflows/r-package-check.yml` takes five `workflow_call` inputs +(`r_matrix`, `needs_tinytex`, `extra_packages`, `check_args`, +`pre_check_script`) and runs the fixed r-lib step sequence once per +`strategy.matrix.config` entry from `fromJSON(inputs.r_matrix)`. Each +calling repository's own `.github/workflows/R-CMD-check.yaml` keeps its +existing `on: push` / `on: pull_request` trigger block (untouched -- a +`workflow_call` target cannot itself be what GitHub triggers on push/PR) +and adds one job that does +`uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@main` +with only that repository's non-default `with:` values. + +## Non-uniform fields found while auditing + +Reading both files' full `with:` blocks (not just the header comment and +step-name sequence the initial survey compared) found: + +- **`on.push`/`on.pull_request` branches.** kaefa: + `[main, master, develop]`; nonnest2: `[main, master]`. Different, and + already different before this change -- preserved exactly in each + repository's own caller, since this lives in the trigger block that + cannot move into the reusable file at all. +- **`actions/checkout` pin.** kaefa: + `3d3c42e5aac5ba805825da76410c181273ba90b1` (`v7.0.1`); nonnest2: + `de0fac2e4500dabe0009e67214ff5f5447ce83dd` (`v6.0.2`). Not called out in + the initial survey. Resolved by unifying to kaefa's newer pin + (`v7.0.1`), which is already this repository's own current pin for + `actions/checkout` in its most recently touched workflows + (`pr-review-fix-scheduler.yml`, `agent-mention-router.yml`, + `agent-mention-router-quality-ci.yml`, + `opencode-rust-coverage-toolchain-quality-ci.yml`) -- a routine version + bump, not a per-repo parameter, since no functional difference between + checkout v6 and v7 affects an R package check. +- **`r-lib/actions/*` pins.** nonnest2 pins every one of its r-lib steps + (`setup-pandoc`, `setup-tinytex`, `setup-r`, `setup-r-dependencies`, + `check-r-package`) to `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`. kaefa + pins three of its four r-lib steps (`setup-r`, `setup-r-dependencies`, + `check-r-package`) to that same SHA, but its `setup-pandoc` step was + pinned to a *different* SHA, `d3c5be51b12e724e68f33216ca3c148b66d5f0b6` + -- an inconsistency inside kaefa's own file, not a genuine cross-repo + difference (nothing in kaefa's history or comments explains a deliberate + pandoc-specific pin; it reads as unnoticed drift, the same category of + finding as ADR-0021's Clearfolio permissions gap). The reusable + workflow uses `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590` for every + r-lib step, uniformly, which is what 4 of kaefa's and nonnest2's + combined 9 r-lib step pins already used -- silently closing that one + stray pin as a side effect of consolidation. +- **`setup-r-dependencies`'s `extra-packages`.** kaefa: + `any::rcmdcheck` **and** `any::testthat` (needed by its own + regression-test step, which calls `testthat::test_file()` directly). + nonnest2: `any::rcmdcheck` only. **Not named in the initial survey**, + which described both as `extra-packages: any::rcmdcheck`. Found only by + reading kaefa's full `with:` block, not just its step names. Carried as + the new `extra_packages` input, defaulting to `any::rcmdcheck` (so + nonnest2's caller needs no `with:` line for it at all) with kaefa's + caller passing both packages via a block-scalar string identical in + content to kaefa's original YAML. +- **`check-r-package`'s `args`.** kaefa passes + `args: 'c("--no-manual", "--no-tests")'` explicitly (it already ran its + package's tests via the regression-test step, so `R CMD check` itself + skips re-running them). nonnest2 does not set `args:` at all, which + means it took `check-r-package`'s own upstream default, + `c("--no-manual", "--as-cran")` (verified by reading + `r-lib/actions@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`'s + `check-r-package/action.yaml` directly rather than assuming). **Not + named in the initial survey at all.** Carried as the new `check_args` + input, defaulting to that exact upstream default string so nonnest2's + caller reproduces its prior (implicit) behavior byte-for-byte with no + `with:` line, while kaefa's caller passes its override explicitly. +- **kaefa's regression-test step.** Not a single Rscript path as the + initial proposal suggested, but two separate `Rscript -e` invocations in + one `run:` block: `install.packages(".", repos = NULL, type = "source")` + then `library(kaefa); testthat::test_file("tests/testthat/test-zh-misfit-decision-rule.R")`. + Carried through unmodified as the multi-line `pre_check_script` input + value (a shell `run:` block, not a single script-file path), which + reproduces the original two-command sequence exactly. The reusable + workflow gives this step a fixed, generic name, + "Run pre-check script (repo-specific)", losing kaefa's original + step-name ("Run Zh formula regression tests"); this is a deliberate, + cosmetic simplification for a two-repo abstraction, not a behavior + change -- see Non-goals. +- `GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}`, `R_KEEP_PKG_SOURCE: yes`, + `permissions: contents: read`, `build_args: 'c("--no-manual")'`, + `error-on: '"error"'`, and `upload-snapshots: true` were byte-identical + across both files and are hardcoded in the reusable workflow rather than + exposed as inputs, since there is nothing to look up. + +## Verification + +- `actionlint .github/workflows/r-package-check.yml` passes (run from this + repository's root). +- `actionlint` also passes on both product repositories' new caller files, + run against local copies of the exact content pushed to each PR branch, + before pushing. +- `tests/test_r_package_check_reusable_workflow_contract.py` reads + `.github/workflows/r-package-check.yml` as text and asserts: all five + `workflow_call` inputs exist with the defaults recorded above; the step + order (checkout, setup-pandoc, conditional setup-tinytex, setup-r, + setup-r-dependencies, conditional pre-check step, check-r-package); the + `r-lib/actions/*` pins are uniformly + `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`; the `actions/checkout` pin is + `3d3c42e5aac5ba805825da76410c181273ba90b1`; `permissions: contents: read` + at the workflow level; and that the uniform, non-parameterized fields + (`R_KEEP_PKG_SOURCE`, `build_args`, `error-on`, `upload-snapshots`) are + present with their exact original values. +- Branch protection was checked directly for both repositories before + writing this record: + `gh api repos/ContextualWisdomLab/kaefa/branches/develop/protection` and + `gh api repos/ContextualWisdomLab/nonnest2/branches/master/protection` + both return `404 Branch not protected`, so there is no required + status-check name this consolidation could silently break by changing + how GitHub composes the matrix job's check name (a reusable-workflow + matrix job's check context is ` / `, + which was not previously true for nonnest2's un-matrixed single job). + +## Non-goals + +- The generic pre-check step name ("Run pre-check script (repo-specific)") + does not attempt to carry a per-caller custom step label. Only one of + the two repositories uses `pre_check_script` today; a + `pre_check_step_name` input can be added if and when a second caller + needs a distinct label, rather than speculatively adding it now for a + cosmetic-only difference. +- `docs/product-technical-gap-baseline.md` is a live per-PR gap-tracking + ledger, not a description of current architecture; this internal CI + consolidation does not add a new tracked product gap, so no row was + added there (same reasoning ADR-0021's doctoring record gave). +- IRT-bibliography-set is not added as a third caller: it has no + `.github/workflows` directory today (`404` on + `contents/.github/workflows`), so there is nothing in it to migrate. + The reusable workflow's inputs are general enough to absorb it (or any + future R package repo in the org) without a new ADR when it exists. +- No new Python was added to `scripts/ci/`, so this change does not touch + the 100%-coverage / 100%-docstring gates on that directory. + +## References (APA 7th edition) + +r-lib. (n.d.). *actions: GitHub Actions for the R community* [Computer +software]. GitHub. Retrieved 2026-09-02, from +https://github.com/r-lib/actions/tree/v2/examples + +GitHub, Inc. (n.d.). *Reusing workflows*. GitHub Docs. Retrieved +2026-09-02, from +https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows diff --git a/tests/test_r_package_check_reusable_workflow_contract.py b/tests/test_r_package_check_reusable_workflow_contract.py new file mode 100644 index 0000000000..31a18348d5 --- /dev/null +++ b/tests/test_r_package_check_reusable_workflow_contract.py @@ -0,0 +1,108 @@ +"""Contract for the reusable R-CMD-check workflow. + +Replaces kaefa's and nonnest2's near-identical, hand-copied +``R-CMD-check.yaml`` files with one reusable ``workflow_call`` workflow, +``.github/workflows/r-package-check.yml``, plus a thin caller left in each +product repository. See +``docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md`` and +``docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md`` for why. +""" + +from __future__ import annotations + +from pathlib import Path + +_WORKFLOW = Path(".github/workflows/r-package-check.yml") + +_R_LIB_PIN = "6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590" +_CHECKOUT_PIN = "3d3c42e5aac5ba805825da76410c181273ba90b1" + + +def _workflow_text() -> str: + """Read the reusable R-CMD-check workflow as UTF-8 text.""" + return _WORKFLOW.read_text(encoding="utf-8") + + +def test_declares_workflow_call_with_five_inputs_and_recorded_defaults() -> None: + """Every genuinely-varying field found while auditing kaefa/nonnest2 is an input.""" + workflow = _workflow_text() + assert "on:\n workflow_call:\n inputs:" in workflow + for name in ( + "r_matrix:", + "needs_tinytex:", + "extra_packages:", + "check_args:", + "pre_check_script:", + ): + assert name in workflow + + assert 'default: \'[{"os": "ubuntu-latest", "r": "release"}]\'' in workflow + assert "default: false" in workflow + assert 'default: "any::rcmdcheck"' in workflow + assert "default: 'c(\"--no-manual\", \"--as-cran\")'" in workflow + assert 'default: ""' in workflow + + +def test_step_order_matches_the_r_lib_template_sequence() -> None: + """checkout -> pandoc -> [tinytex] -> setup-r -> deps -> [pre-check] -> check.""" + workflow = _workflow_text() + order = [ + "actions/checkout@", + "r-lib/actions/setup-pandoc@", + "r-lib/actions/setup-tinytex@", + "r-lib/actions/setup-r@", + "r-lib/actions/setup-r-dependencies@", + "Run pre-check script (repo-specific)", + "r-lib/actions/check-r-package@", + ] + positions = [workflow.index(marker) for marker in order] + assert positions == sorted(positions), "steps are out of order" + + +def test_optional_steps_are_gated_on_their_inputs() -> None: + """setup-tinytex and the pre-check step must not run unconditionally.""" + workflow = _workflow_text() + assert ( + "- if: inputs.needs_tinytex\n uses: r-lib/actions/setup-tinytex@" + in workflow + ) + assert ( + "- if: inputs.pre_check_script != ''\n" + " name: Run pre-check script (repo-specific)" + in workflow + ) + assert "run: ${{ inputs.pre_check_script }}" in workflow + + +def test_action_pins_are_uniform_and_current() -> None: + """Every r-lib step and checkout share one current pin, not per-caller drift.""" + workflow = _workflow_text() + assert workflow.count(_R_LIB_PIN) == 5 # pandoc, tinytex, setup-r, deps, check + assert f"actions/checkout@{_CHECKOUT_PIN}" in workflow + assert f"r-lib/actions/setup-pandoc@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/setup-tinytex@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/setup-r@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/setup-r-dependencies@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/check-r-package@{_R_LIB_PIN}" in workflow + + +def test_uniform_fields_are_hardcoded_not_parameterized() -> None: + """Fields byte-identical across both originals stay static, not inputs.""" + workflow = _workflow_text() + assert "permissions:\n contents: read" in workflow + assert "GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}" in workflow + assert "R_KEEP_PKG_SOURCE: yes" in workflow + assert "build_args: 'c(\"--no-manual\")'" in workflow + assert "error-on: '\"error\"'" in workflow + assert "upload-snapshots: true" in workflow + assert "args: ${{ inputs.check_args }}" in workflow + assert "extra-packages: ${{ inputs.extra_packages }}" in workflow + + +def test_matrix_is_driven_by_the_r_matrix_input() -> None: + """The strategy matrix must come from fromJSON(inputs.r_matrix), not a fixed list.""" + workflow = _workflow_text() + assert "config: ${{ fromJSON(inputs.r_matrix) }}" in workflow + assert "runs-on: ${{ matrix.config.os }}" in workflow + assert "r-version: ${{ matrix.config.r }}" in workflow + assert "http-user-agent: ${{ matrix.config['http-user-agent'] }}" in workflow From 5aecb9b3c75d2f31c00359071b1fd414d568acc2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 20:51:11 +0900 Subject: [PATCH 2/7] docs(workflows): correct r-package-check.yml's caller example to SHA-pin The dependency-review.yml consolidation's caller PRs surfaced a real Devin security finding: uses: @main runs an unreviewed central change against every caller's PR checks with no review in the calling repo. Fixed there (all four callers pinned to a commit SHA); apply the same correction to this not-yet-merged reusable workflow's own documented example before any caller PR copies the unsafe pattern. Also notes the separate required-status-check-name gotcha (converting a job to uses: renames its published check) to check for in each caller repo before merging. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/r-package-check.yml | 14 ++++++++++++-- .../r-cmd-check-reusable-workflow-consolidation.md | 10 ++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/.github/workflows/r-package-check.yml b/.github/workflows/r-package-check.yml index a6cb120585..fcaf87a4bc 100644 --- a/.github/workflows/r-package-check.yml +++ b/.github/workflows/r-package-check.yml @@ -12,7 +12,17 @@ # workflow file -- a workflow_call target cannot also be the thing GitHub # triggers directly on push/PR. # -# Example caller (.github/workflows/R-CMD-check.yaml in a product repo): +# Example caller (.github/workflows/R-CMD-check.yaml in a product repo). +# Pin `uses:` to this file's exact commit SHA, not @main: an unpinned mutable +# ref would run an unreviewed central change against every PR check in the +# calling repo (see dependency-review.yml's own header comment and +# docs/doctoring/dependency-review-reusable-workflow-consolidation.md for the +# incident that established this as the required pattern for every reusable +# workflow caller in this org). If the calling repo's branch protection +# requires a status check literally named after the old standalone job, +# converting to `uses:` here will rename the published check to +# " / R-CMD-check" and silently break that required check -- +# check for this before or immediately after merging a caller. # # name: R-CMD-check # on: @@ -22,7 +32,7 @@ # branches: [main, master] # jobs: # R-CMD-check: -# uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@main +# uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@ # with: # needs_tinytex: true # only if the package builds a PDF vignette # diff --git a/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md index 4b4fb33761..28a5c773da 100644 --- a/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md +++ b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md @@ -30,8 +30,14 @@ calling repository's own `.github/workflows/R-CMD-check.yaml` keeps its existing `on: push` / `on: pull_request` trigger block (untouched -- a `workflow_call` target cannot itself be what GitHub triggers on push/PR) and adds one job that does -`uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@main` -with only that repository's non-default `with:` values. +`uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@` +with only that repository's non-default `with:` values. Pin `` +to this file's exact commit, not `@main` — see +`docs/doctoring/dependency-review-reusable-workflow-consolidation.md`'s +"Post-merge corrections" section for why an unpinned mutable ref is a real +security gap (Devin caught it on that consolidation's caller PRs) and for +the separate required-status-check-name gotcha to check for in each caller +repo's branch protection before merging. ## Non-uniform fields found while auditing From 5e838ab35d062faa488b03ae78f9f8d84447e223 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 20:55:01 +0900 Subject: [PATCH 3/7] test(workflows): reject caller-authored shell in reusable R check --- ...test_r_package_check_reusable_workflow_contract.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_r_package_check_reusable_workflow_contract.py b/tests/test_r_package_check_reusable_workflow_contract.py index 31a18348d5..d27f9615f2 100644 --- a/tests/test_r_package_check_reusable_workflow_contract.py +++ b/tests/test_r_package_check_reusable_workflow_contract.py @@ -106,3 +106,14 @@ def test_matrix_is_driven_by_the_r_matrix_input() -> None: assert "runs-on: ${{ matrix.config.os }}" in workflow assert "r-version: ${{ matrix.config.r }}" in workflow assert "http-user-agent: ${{ matrix.config['http-user-agent'] }}" in workflow + + +def test_pre_check_hook_is_bounded_data_not_caller_shell_source() -> None: + """A reusable caller must not inject arbitrary Bash source into the trusted job.""" + workflow = _workflow_text() + assert "pre_check_script:" not in workflow + assert "run: ${{ inputs.pre_check_script }}" not in workflow + assert "pre_check_test_file:" in workflow + assert "install_package_before_pre_check:" in workflow + assert "PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }}" in workflow + assert 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' in workflow From 931c8f32a2e5e743ca0fbdee3d6728170ff2b273 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 20:55:39 +0900 Subject: [PATCH 4/7] fix(workflows): bound reusable R pre-check input --- .github/workflows/r-package-check.yml | 45 +++++++++++++++++++++------ 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/.github/workflows/r-package-check.yml b/.github/workflows/r-package-check.yml index fcaf87a4bc..221d66a838 100644 --- a/.github/workflows/r-package-check.yml +++ b/.github/workflows/r-package-check.yml @@ -65,16 +65,24 @@ on: Value forwarded to check-r-package's args input. Default matches that action's own upstream default (c("--no-manual", "--as-cran")); override to change what - rcmdcheck runs (e.g. to skip re-running tests already run in - pre_check_script). + rcmdcheck runs (e.g. to skip re-running tests already run by a + bounded pre-check test file). required: false type: string default: 'c("--no-manual", "--as-cran")' - pre_check_script: + install_package_before_pre_check: description: >- - Optional shell commands run in a step between setup-r-dependencies - and check-r-package (e.g. a repo-specific regression test). - Skipped entirely when empty (the default). + Install the current package from source before the optional fixed + testthat pre-check. This is a boolean capability, not caller-authored + shell source. + required: false + type: boolean + default: false + pre_check_test_file: + description: >- + Optional repository-relative testthat file under tests/testthat/ + ending in .R. The value is passed as data through an environment + variable and is never evaluated as shell source. required: false type: string default: "" @@ -115,9 +123,28 @@ jobs: extra-packages: ${{ inputs.extra_packages }} needs: check - - if: inputs.pre_check_script != '' - name: Run pre-check script (repo-specific) - run: ${{ inputs.pre_check_script }} + - if: inputs.pre_check_test_file != '' && inputs.install_package_before_pre_check + name: Install package for bounded pre-check + run: Rscript -e 'install.packages(".", repos = NULL, type = "source")' + shell: bash + + - if: inputs.pre_check_test_file != '' + name: Run bounded testthat pre-check + env: + PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }} + run: | + case "$PRE_CHECK_TEST_FILE" in + tests/testthat/*.R) ;; + *) + echo "::error::pre_check_test_file must be a repository-relative tests/testthat/*.R path" + exit 1 + ;; + esac + if [[ "$PRE_CHECK_TEST_FILE" == *".."* || "$PRE_CHECK_TEST_FILE" == /* || "$PRE_CHECK_TEST_FILE" == *$'\n'* || "$PRE_CHECK_TEST_FILE" == *$'\r'* ]]; then + echo "::error::pre_check_test_file contains a forbidden path/control sequence" + exit 1 + fi + Rscript -e 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' shell: bash - uses: r-lib/actions/check-r-package@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 From 6ca3080326f3498904d6222c60089e35a050b848 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 20:57:04 +0900 Subject: [PATCH 5/7] test(workflows): bind bounded R pre-check contract --- ...ackage_check_reusable_workflow_contract.py | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/tests/test_r_package_check_reusable_workflow_contract.py b/tests/test_r_package_check_reusable_workflow_contract.py index d27f9615f2..2d3ad49711 100644 --- a/tests/test_r_package_check_reusable_workflow_contract.py +++ b/tests/test_r_package_check_reusable_workflow_contract.py @@ -23,8 +23,8 @@ def _workflow_text() -> str: return _WORKFLOW.read_text(encoding="utf-8") -def test_declares_workflow_call_with_five_inputs_and_recorded_defaults() -> None: - """Every genuinely-varying field found while auditing kaefa/nonnest2 is an input.""" +def test_declares_workflow_call_with_six_inputs_and_recorded_defaults() -> None: + """Every genuinely varying caller field is data, never executable shell source.""" workflow = _workflow_text() assert "on:\n workflow_call:\n inputs:" in workflow for name in ( @@ -32,19 +32,20 @@ def test_declares_workflow_call_with_five_inputs_and_recorded_defaults() -> None "needs_tinytex:", "extra_packages:", "check_args:", - "pre_check_script:", + "install_package_before_pre_check:", + "pre_check_test_file:", ): assert name in workflow assert 'default: \'[{"os": "ubuntu-latest", "r": "release"}]\'' in workflow - assert "default: false" in workflow + assert workflow.count("default: false") >= 2 assert 'default: "any::rcmdcheck"' in workflow assert "default: 'c(\"--no-manual\", \"--as-cran\")'" in workflow assert 'default: ""' in workflow def test_step_order_matches_the_r_lib_template_sequence() -> None: - """checkout -> pandoc -> [tinytex] -> setup-r -> deps -> [pre-check] -> check.""" + """checkout -> pandoc -> [tinytex] -> setup-r -> deps -> bounded pre-check -> check.""" workflow = _workflow_text() order = [ "actions/checkout@", @@ -52,26 +53,33 @@ def test_step_order_matches_the_r_lib_template_sequence() -> None: "r-lib/actions/setup-tinytex@", "r-lib/actions/setup-r@", "r-lib/actions/setup-r-dependencies@", - "Run pre-check script (repo-specific)", + "Install package for bounded pre-check", + "Run bounded testthat pre-check", "r-lib/actions/check-r-package@", ] positions = [workflow.index(marker) for marker in order] assert positions == sorted(positions), "steps are out of order" -def test_optional_steps_are_gated_on_their_inputs() -> None: - """setup-tinytex and the pre-check step must not run unconditionally.""" +def test_optional_steps_are_gated_on_bounded_inputs() -> None: + """Optional setup and pre-check steps run only for explicit bounded capabilities.""" workflow = _workflow_text() assert ( "- if: inputs.needs_tinytex\n uses: r-lib/actions/setup-tinytex@" in workflow ) assert ( - "- if: inputs.pre_check_script != ''\n" - " name: Run pre-check script (repo-specific)" + "- if: inputs.pre_check_test_file != '' && inputs.install_package_before_pre_check\n" + " name: Install package for bounded pre-check" in workflow ) - assert "run: ${{ inputs.pre_check_script }}" in workflow + assert ( + "- if: inputs.pre_check_test_file != ''\n" + " name: Run bounded testthat pre-check" + in workflow + ) + assert "PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }}" in workflow + assert 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' in workflow def test_action_pins_are_uniform_and_current() -> None: @@ -116,4 +124,8 @@ def test_pre_check_hook_is_bounded_data_not_caller_shell_source() -> None: assert "pre_check_test_file:" in workflow assert "install_package_before_pre_check:" in workflow assert "PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }}" in workflow + assert 'case "$PRE_CHECK_TEST_FILE" in' in workflow + assert "tests/testthat/*.R" in workflow + assert '"$PRE_CHECK_TEST_FILE" == *".."*' in workflow + assert '"$PRE_CHECK_TEST_FILE" == /*' in workflow assert 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' in workflow From 0747ae12b0ec77f2275e113f9af8630dc8a41bf0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 20:57:45 +0900 Subject: [PATCH 6/7] docs(adr): record bounded R workflow security decision --- ...d-check-reusable-workflow-consolidation.md | 179 ++++-------------- 1 file changed, 40 insertions(+), 139 deletions(-) diff --git a/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md b/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md index a7459a0ebc..05f8b92f79 100644 --- a/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md +++ b/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md @@ -1,148 +1,49 @@ # ADR-0023: Consolidate kaefa/nonnest2 R-CMD-check.yaml into one reusable workflow -- **Status:** Accepted +- **Status:** Proposed - **Date:** 2026-09-02 -- **Scope:** ContextualWisdomLab/.github `.github/workflows/` (new reusable workflow); - ContextualWisdomLab/kaefa and ContextualWisdomLab/nonnest2 `.github/workflows/R-CMD-check.yaml` - (each replaced by a thin `workflow_call` caller) +- **Scope:** `ContextualWisdomLab/.github` reusable R package CI; consumers `ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` -## Context +## Problem -kaefa and nonnest2 each carry a hand-copied `R-CMD-check.yaml`, both generated -from the same upstream r-lib template -(https://github.com/r-lib/actions/tree/v2/examples): both open with the -identical "Workflow derived from..." header, and both run the identical -`actions/checkout` -> `r-lib/actions/setup-pandoc` -> `r-lib/actions/setup-r` --> `r-lib/actions/setup-r-dependencies` -> `r-lib/actions/check-r-package` -step sequence with the same `GITHUB_PAT` / `R_KEEP_PKG_SOURCE` env vars and -the same `permissions: contents: read`. This is the same pattern -ADR-0021 named for the hourly review-repair callers: near-duplicated -GitHub Actions YAML that differs only in the fields a `workflow_call` input -was built to carry. +`ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` carry near-identical R-CMD-check workflows derived from the r-lib Actions examples. The shared sequence is checkout → Pandoc → optional TinyTeX → R setup → dependency setup → optional repository-specific regression → `check-r-package`. Copying that sequence creates action-pin, permission, and behavior drift. -Reading both files in full (not just the survey that proposed this -consolidation) surfaced two genuinely varying fields the survey had not -named -- `docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md` -records the full field-by-field audit, including these: - -- kaefa's `setup-r-dependencies` installs `any::rcmdcheck` **and** - `any::testthat` (its own regression-test step needs `testthat`); - nonnest2 installs only `any::rcmdcheck`. -- kaefa's `check-r-package` overrides `args: 'c("--no-manual", "--no-tests")'` - (it already ran its package's tests via the regression-test step, so - `R CMD check` itself skips re-running them); nonnest2 omits `args:` - entirely, taking `check-r-package`'s own upstream default, - `c("--no-manual", "--as-cran")`. - -Neither is the kind of difference a survey summary line ("same step -sequence") would show without opening both action's `with:` blocks. -Both are exactly the kind of field a `workflow_call` input handles, so -they do not change the Decision below -- but they are new inputs beyond the -ones the initial proposal named, and are called out here per this -repository's standing convention of not forcing a consolidation past -genuine per-repo variance without naming it (see `docs/CWL-MASTER-CONTEXT.md` -§7 and the precedent this ADR follows, ADR-0021). - -`docs/product-technical-gap-baseline.md` gap-baseline snapshot and IRT-bibliography-set -(named as a plausible third target) returned 404 for a `.github/workflows` -directory during the survey -- it has no CI workflow of this shape yet, so it -is not a target of this change; the reusable workflow is still built openly -so a future R package repo can adopt it without a new ADR. +A first reusable-workflow implementation exposed the repository-specific regression as a free-form `pre_check_script` string and interpolated it directly into `run:`. Current-head security review correctly identified that design as a privileged-code boundary defect: a reusable caller could supply arbitrary shell source to a job that receives the caller repository token. Consolidation does not justify transferring executable authority from a consumer into a centrally trusted workflow. ## Decision -1. One new reusable workflow, `.github/workflows/r-package-check.yml` in - this repository, implements the shared r-lib check sequence behind - `workflow_call` inputs: - - `r_matrix` (JSON string, default a single `ubuntu-latest`/`release` - leg) -- becomes `strategy.matrix.config` via `fromJSON()`. - - `needs_tinytex` (boolean, default `false`) -- gates an optional - `r-lib/actions/setup-tinytex` step (nonnest2's PDF vignette needs it; - kaefa does not use it). - - `extra_packages` (string, default `any::rcmdcheck`) -- forwarded to - `setup-r-dependencies`'s `extra-packages` input. - - `check_args` (string, default `c("--no-manual", "--as-cran")`, - matching `check-r-package`'s own upstream default so nonnest2's - behavior is unchanged by omission-turned-explicit) -- forwarded to - `check-r-package`'s `args` input. - - `pre_check_script` (string, default empty -- step skipped) -- an - optional shell step run between dependency setup and the check step, - for kaefa's package-install-then-`testthat::test_file()` regression - check. -2. `GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}`, `R_KEEP_PKG_SOURCE: yes`, - `permissions: contents: read`, `build_args: 'c("--no-manual")'`, - `error-on: '"error"'`, and `upload-snapshots: true` were uniform across - both originals and are hardcoded in the reusable workflow, not exposed - as inputs. -3. The `on: push` / `on: pull_request` trigger (and each repository's own - branch list) stays in each calling repository's own thin - `.github/workflows/R-CMD-check.yaml` -- a `workflow_call` target cannot - itself be the workflow GitHub triggers directly on push/PR, so this - cannot move into the reusable file. kaefa keeps - `[main, master, develop]`; nonnest2 keeps `[main, master]` -- these were - already different before this change and are preserved exactly. -4. Each repository's local file collapses to a thin caller: `on:` (its - existing trigger config, untouched) plus one job, - `uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@main`, - with only that repository's actual non-default `with:` values -- - nonnest2's caller sets only `needs_tinytex: true`; kaefa's sets - `r_matrix`, `extra_packages`, `check_args`, and `pre_check_script` - (all four differ from the reusable workflow's defaults). This follows - the exact `@main`-reference convention `deploy-pages.yml` already - documents for this repository's other reusable workflows. -5. Action version pins are unified to this repository's own current pins - rather than parameterized: `actions/checkout` moves to - `3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1` (kaefa's existing - pin; nonnest2 was on the older `v6.0.2`), and every `r-lib/actions/*` - step moves to `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2` (nonnest2's - existing uniform pin for all of its r-lib steps, and already the pin - kaefa used for three of its four r-lib steps). This is a routine - version-pin bump of the kind Dependabot performs, not a parameterized - per-repo field: no `workflow_call` input exists for "which SHA," and - both repositories converge on whichever pin was already newest/more - uniform in this ecosystem. - -## Consequences - -- Adding a third R package repository (e.g. a future IRT-bibliography-set) - to this pattern is a ~15-line caller file with only its own differing - `with:` values, not a copy-pasted 30+-line workflow. -- kaefa's `setup-pandoc` step, previously pinned to a stray SHA - (`d3c5be51b12e724e68f33216ca3c148b66d5f0b6 # v2`) different from its own - other three r-lib steps -- an inconsistency *within* kaefa's own prior - file, not a genuine cross-repo difference -- now uses the same pin as - every other r-lib step in both repositories, closing that drift as a - side effect of consolidation (same category of incidental fix ADR-0021 - made for Clearfolio's missing job permissions). -- nonnest2's `actions/checkout` pin moves from `v6.0.2` to `v7.0.1` as part - of adopting the shared workflow; `docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md` - records that this is the only originally-unpinned-to-kaefa's-version - action bump this change makes, and that it is a well-tested checkout - action major-version-stable bump, not a behavioral change to the R check - itself. -- Neither kaefa's `develop` branch nor nonnest2's `master` branch has GitHub - branch protection configured (`gh api .../branches/.../protection` -> 404 - for both, verified before writing this ADR), so there is no required - status-check name this change could silently break by changing how the - matrix job's check name is composed. - -## Rejected alternatives - -- **A single shared file with no inputs, hardcoding kaefa's 5-leg matrix - and regression step for both repos.** Rejected: nonnest2 has no - `testthat`-based regression suite step and does not build with a PDF - vignette toolchain matrix; forcing kaefa's shape onto it would run steps - that reference files nonnest2 does not have. -- **Parameterize the action version pins as `workflow_call` inputs.** - Rejected: pin choice is a security/supply-chain decision belonging to - the reusable workflow's own maintainers, not a per-repo product - difference; unifying to one current pin (as this repository already - does for `actions/checkout` in `deploy-pages.yml`, - `pr-review-fix-scheduler.yml`, and 40+ other in-repo workflows) keeps a - single place to bump it later. -- **Leave `check_args` unset by default and require every caller to pass - it explicitly.** Rejected: nonnest2's original file never set `args:` - at all, so defaulting to `check-r-package`'s own upstream default - reproduces nonnest2's exact prior behavior with zero `with:` lines, - rather than forcing every future caller to memorize and repeat - `check-r-package`'s own default. +1. `ContextualWisdomLab/.github/.github/workflows/r-package-check.yml` is the canonical reusable owner for the shared R-CMD-check sequence. +2. The reusable interface is data/capability oriented, not shell oriented. It accepts: + - `r_matrix`: JSON strategy matrix; + - `needs_tinytex`: boolean capability; + - `extra_packages`: dependency input forwarded to r-lib Actions; + - `check_args`: R CMD check arguments; + - `install_package_before_pre_check`: boolean capability for the known kaefa regression shape; + - `pre_check_test_file`: repository-relative `tests/testthat/*.R` path passed as data. +3. Free-form `pre_check_script` is forbidden. The workflow owns the only executable pre-check commands: an optional fixed `install.packages(".", ...)` invocation and a fixed `testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))` invocation. +4. `pre_check_test_file` fails closed unless it is a relative `tests/testthat/*.R` path and contains no parent traversal, absolute-path prefix, carriage return, or newline. The path enters the shell only through an environment variable; it is never evaluated as shell source. +5. Uniform security/supply-chain fields remain centrally owned and non-parameterized: `permissions: contents: read`, `GITHUB_PAT`, `R_KEEP_PKG_SOURCE`, `build_args`, `error-on`, upload behavior, and immutable action SHAs. +6. Consumer trigger branches remain in each repository's thin caller. Consumers must pin `uses:` to the immutable protected-main commit containing the reusable workflow; mutable `@main`, PR heads, and branch URLs are not production dependency authority. +7. The current proposal remains **Proposed** until this exact candidate passes repository tests/security/review and integrates through protected `main`. Only then may consumer PRs pin the resulting protected-main SHA and reacquire their own exact-head evidence. + +## Alternatives considered + +- **Keep copied workflows.** Rejected because two already-identical control surfaces drift independently and duplicate maintenance/security review. +- **Free-form shell input.** Rejected because it turns caller data into executable commands in a centrally trusted job. +- **Parameterize action SHAs or permissions.** Rejected because supply-chain and token authority belong to the reusable workflow owner, not individual consumers. +- **Hard-code kaefa-specific file names centrally.** Rejected because the reusable owner should expose the minimum bounded semantic input needed by multiple products, not own product test identity. +- **Consume an unreleased PR-head version from product callers.** Rejected because consumers may use only protected/released immutable owner contracts. + +## Invariants and failure scenarios + +- A malicious or compromised caller cannot make the central job execute arbitrary Bash through an input. +- An invalid test-file path fails before R execution. +- A caller cannot elevate token permissions through the reusable workflow. +- If protected-main publication has not occurred, consumer adoption remains blocked rather than falling back to a mutable ref. +- Changing the caller to a reusable job may change the published check-context name; consumer branch/ruleset requirements must be re-read before adoption and repaired at the owning ruleset rather than silently weakening protection. + +## Consequences and follow-up + +The central workflow becomes a small reusable CI contract while product repositories retain only triggers and bounded product-specific values. `ContextualWisdomLab/kaefa#84` must replace its former shell input with `install_package_before_pre_check: true` and `pre_check_test_file: tests/testthat/test-zh-misfit-decision-rule.R`, then pin the eventual protected-main SHA. `ContextualWisdomLab/nonnest2#119` must likewise pin the protected-main SHA. Both consumer PRs remain non-authoritative until the owner integrates and their own current-head gates pass. + +The executable regression in `tests/test_r_package_check_reusable_workflow_contract.py` permanently forbids reintroducing caller-authored shell source and verifies the bounded pre-check path. From 8691ac6c7365c1ee99148bbb5bfbd5d609be0c3c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 20:58:14 +0900 Subject: [PATCH 7/7] docs(workflows): record reusable R shell-input RCA --- ...d-check-reusable-workflow-consolidation.md | 216 ++++-------------- 1 file changed, 49 insertions(+), 167 deletions(-) diff --git a/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md index 28a5c773da..614c5b4ab7 100644 --- a/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md +++ b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md @@ -1,173 +1,55 @@ # R-CMD-check reusable workflow consolidation -## Decision - -kaefa's and nonnest2's `.github/workflows/R-CMD-check.yaml` files -- both -auto-generated from the same upstream r-lib template -(https://github.com/r-lib/actions/tree/v2/examples) -- are replaced by one -new reusable workflow, `.github/workflows/r-package-check.yml` in this -repository, plus a thin `workflow_call` caller left in place of each -repository's own `R-CMD-check.yaml`. See -[ADR-0023](../adr/0023-r-cmd-check-reusable-workflow-consolidation.md). - -Both original files opened with the same "Workflow derived from..." header -comment and ran the same `actions/checkout` -> `setup-pandoc` -> `setup-r` --> `setup-r-dependencies` -> `check-r-package` sequence, with the same -`GITHUB_PAT` / `R_KEEP_PKG_SOURCE` env vars and the same -`permissions: contents: read`. A third named candidate, -IRT-bibliography-set, returned 404 for a `.github/workflows` directory -during the survey (`gh api repos/ContextualWisdomLab/IRT-bibliography-set/contents/.github/workflows`) --- it has no workflow of this shape today, so it is not a target of this -change. - -## Mechanism - -`.github/workflows/r-package-check.yml` takes five `workflow_call` inputs -(`r_matrix`, `needs_tinytex`, `extra_packages`, `check_args`, -`pre_check_script`) and runs the fixed r-lib step sequence once per -`strategy.matrix.config` entry from `fromJSON(inputs.r_matrix)`. Each -calling repository's own `.github/workflows/R-CMD-check.yaml` keeps its -existing `on: push` / `on: pull_request` trigger block (untouched -- a -`workflow_call` target cannot itself be what GitHub triggers on push/PR) -and adds one job that does -`uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@` -with only that repository's non-default `with:` values. Pin `` -to this file's exact commit, not `@main` — see -`docs/doctoring/dependency-review-reusable-workflow-consolidation.md`'s -"Post-merge corrections" section for why an unpinned mutable ref is a real -security gap (Devin caught it on that consolidation's caller PRs) and for -the separate required-status-check-name gotcha to check for in each caller -repo's branch protection before merging. - -## Non-uniform fields found while auditing - -Reading both files' full `with:` blocks (not just the header comment and -step-name sequence the initial survey compared) found: - -- **`on.push`/`on.pull_request` branches.** kaefa: - `[main, master, develop]`; nonnest2: `[main, master]`. Different, and - already different before this change -- preserved exactly in each - repository's own caller, since this lives in the trigger block that - cannot move into the reusable file at all. -- **`actions/checkout` pin.** kaefa: - `3d3c42e5aac5ba805825da76410c181273ba90b1` (`v7.0.1`); nonnest2: - `de0fac2e4500dabe0009e67214ff5f5447ce83dd` (`v6.0.2`). Not called out in - the initial survey. Resolved by unifying to kaefa's newer pin - (`v7.0.1`), which is already this repository's own current pin for - `actions/checkout` in its most recently touched workflows - (`pr-review-fix-scheduler.yml`, `agent-mention-router.yml`, - `agent-mention-router-quality-ci.yml`, - `opencode-rust-coverage-toolchain-quality-ci.yml`) -- a routine version - bump, not a per-repo parameter, since no functional difference between - checkout v6 and v7 affects an R package check. -- **`r-lib/actions/*` pins.** nonnest2 pins every one of its r-lib steps - (`setup-pandoc`, `setup-tinytex`, `setup-r`, `setup-r-dependencies`, - `check-r-package`) to `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`. kaefa - pins three of its four r-lib steps (`setup-r`, `setup-r-dependencies`, - `check-r-package`) to that same SHA, but its `setup-pandoc` step was - pinned to a *different* SHA, `d3c5be51b12e724e68f33216ca3c148b66d5f0b6` - -- an inconsistency inside kaefa's own file, not a genuine cross-repo - difference (nothing in kaefa's history or comments explains a deliberate - pandoc-specific pin; it reads as unnoticed drift, the same category of - finding as ADR-0021's Clearfolio permissions gap). The reusable - workflow uses `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590` for every - r-lib step, uniformly, which is what 4 of kaefa's and nonnest2's - combined 9 r-lib step pins already used -- silently closing that one - stray pin as a side effect of consolidation. -- **`setup-r-dependencies`'s `extra-packages`.** kaefa: - `any::rcmdcheck` **and** `any::testthat` (needed by its own - regression-test step, which calls `testthat::test_file()` directly). - nonnest2: `any::rcmdcheck` only. **Not named in the initial survey**, - which described both as `extra-packages: any::rcmdcheck`. Found only by - reading kaefa's full `with:` block, not just its step names. Carried as - the new `extra_packages` input, defaulting to `any::rcmdcheck` (so - nonnest2's caller needs no `with:` line for it at all) with kaefa's - caller passing both packages via a block-scalar string identical in - content to kaefa's original YAML. -- **`check-r-package`'s `args`.** kaefa passes - `args: 'c("--no-manual", "--no-tests")'` explicitly (it already ran its - package's tests via the regression-test step, so `R CMD check` itself - skips re-running them). nonnest2 does not set `args:` at all, which - means it took `check-r-package`'s own upstream default, - `c("--no-manual", "--as-cran")` (verified by reading - `r-lib/actions@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`'s - `check-r-package/action.yaml` directly rather than assuming). **Not - named in the initial survey at all.** Carried as the new `check_args` - input, defaulting to that exact upstream default string so nonnest2's - caller reproduces its prior (implicit) behavior byte-for-byte with no - `with:` line, while kaefa's caller passes its override explicitly. -- **kaefa's regression-test step.** Not a single Rscript path as the - initial proposal suggested, but two separate `Rscript -e` invocations in - one `run:` block: `install.packages(".", repos = NULL, type = "source")` - then `library(kaefa); testthat::test_file("tests/testthat/test-zh-misfit-decision-rule.R")`. - Carried through unmodified as the multi-line `pre_check_script` input - value (a shell `run:` block, not a single script-file path), which - reproduces the original two-command sequence exactly. The reusable - workflow gives this step a fixed, generic name, - "Run pre-check script (repo-specific)", losing kaefa's original - step-name ("Run Zh formula regression tests"); this is a deliberate, - cosmetic simplification for a two-repo abstraction, not a behavior - change -- see Non-goals. -- `GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}`, `R_KEEP_PKG_SOURCE: yes`, - `permissions: contents: read`, `build_args: 'c("--no-manual")'`, - `error-on: '"error"'`, and `upload-snapshots: true` were byte-identical - across both files and are hardcoded in the reusable workflow rather than - exposed as inputs, since there is nothing to look up. - -## Verification - -- `actionlint .github/workflows/r-package-check.yml` passes (run from this - repository's root). -- `actionlint` also passes on both product repositories' new caller files, - run against local copies of the exact content pushed to each PR branch, - before pushing. -- `tests/test_r_package_check_reusable_workflow_contract.py` reads - `.github/workflows/r-package-check.yml` as text and asserts: all five - `workflow_call` inputs exist with the defaults recorded above; the step - order (checkout, setup-pandoc, conditional setup-tinytex, setup-r, - setup-r-dependencies, conditional pre-check step, check-r-package); the - `r-lib/actions/*` pins are uniformly - `6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`; the `actions/checkout` pin is - `3d3c42e5aac5ba805825da76410c181273ba90b1`; `permissions: contents: read` - at the workflow level; and that the uniform, non-parameterized fields - (`R_KEEP_PKG_SOURCE`, `build_args`, `error-on`, `upload-snapshots`) are - present with their exact original values. -- Branch protection was checked directly for both repositories before - writing this record: - `gh api repos/ContextualWisdomLab/kaefa/branches/develop/protection` and - `gh api repos/ContextualWisdomLab/nonnest2/branches/master/protection` - both return `404 Branch not protected`, so there is no required - status-check name this consolidation could silently break by changing - how GitHub composes the matrix job's check name (a reusable-workflow - matrix job's check context is ` / `, - which was not previously true for nonnest2's un-matrixed single job). - -## Non-goals - -- The generic pre-check step name ("Run pre-check script (repo-specific)") - does not attempt to carry a per-caller custom step label. Only one of - the two repositories uses `pre_check_script` today; a - `pre_check_step_name` input can be added if and when a second caller - needs a distinct label, rather than speculatively adding it now for a - cosmetic-only difference. -- `docs/product-technical-gap-baseline.md` is a live per-PR gap-tracking - ledger, not a description of current architecture; this internal CI - consolidation does not add a new tracked product gap, so no row was - added there (same reasoning ADR-0021's doctoring record gave). -- IRT-bibliography-set is not added as a third caller: it has no - `.github/workflows` directory today (`404` on - `contents/.github/workflows`), so there is nothing in it to migrate. - The reusable workflow's inputs are general enough to absorb it (or any - future R package repo in the org) without a new ADR when it exists. -- No new Python was added to `scripts/ci/`, so this change does not touch - the 100%-coverage / 100%-docstring gates on that directory. +## Current authority + +This record describes the Proposed owner change in `ContextualWisdomLab/.github#1716`. Protected `main` remains production authority until the exact candidate integrates. Consumer PRs in `ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` must not consume this PR branch or mutable `@main`; after integration they pin the exact protected-main commit that contains the reusable workflow. + +## Original duplication + +`ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` both derived their R-CMD-check workflow from the r-lib Actions examples. Their common sequence and common authority fields justified a canonical reusable owner. Their real differences are bounded data/capabilities: trigger branches, R matrix, TinyTeX requirement, extra R packages, check arguments, and kaefa's one testthat regression. + +The action pins selected by the proposal are `actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1` and `r-lib/actions/*@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`. `permissions: contents: read`, `GITHUB_PAT`, `R_KEEP_PKG_SOURCE`, `build_args`, `error-on`, and snapshot-upload behavior remain owned centrally rather than becoming consumer inputs. + +## Security RCA: free-form pre-check shell + +The first candidate represented kaefa's two-command regression as a string input named `pre_check_script` and executed it with `run: ${{ inputs.pre_check_script }}`. Devin current-head review identified the resulting security boundary defect: reusable-workflow callers could provide arbitrary Bash source to a central job that receives the caller repository token. + +This is a canonical-owner defect, not a finding to suppress or merely document. The repair lineage on 2026-09-02 is: + +- RED commit `5e838ab35d062faa488b03ae78f9f8d84447e223`: adds an executable contract forbidding `pre_check_script`/caller-authored `run:` and requiring a bounded test-file data path; +- production commit `931c8f32a2e5e743ca0fbdee3d6728170ff2b273`: removes arbitrary shell input and introduces `install_package_before_pre_check` plus `pre_check_test_file`; +- contract-alignment commit `6ca3080326f3498904d6222c60089e35a050b848`: verifies step order, capability gates, environment-data binding, and fail-closed path checks on the repaired source. + +The repaired workflow owns its executable commands. When requested, it runs a fixed package installation command. The optional test file is passed only as `PRE_CHECK_TEST_FILE`, must match repository-relative `tests/testthat/*.R`, and is rejected for parent traversal, absolute-path prefixes, carriage returns, or newlines before the fixed `testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))` command executes. No consumer string is evaluated as shell source. + +## Consumer equivalence + +The bounded replacement preserves kaefa's valid behavior without preserving the unsafe representation. Its former commands were: + +1. install the current package from source; +2. run `tests/testthat/test-zh-misfit-decision-rule.R` through testthat. + +The equivalent bounded caller values are: + +- `install_package_before_pre_check: true`; +- `pre_check_test_file: tests/testthat/test-zh-misfit-decision-rule.R`. + +Kaefa's five-leg R matrix, `any::rcmdcheck` + `any::testthat`, and `c("--no-manual", "--no-tests")` remain data inputs. Nonnest2 needs no pre-check capability and keeps its own trigger branches/TinyTeX behavior. Each consumer must pin the eventual owner protected-main SHA and regenerate its own current-head evidence. + +## Validation contract + +`tests/test_r_package_check_reusable_workflow_contract.py` checks the six bounded inputs, optional-step gates, immutable action pins, uniform central fields, matrix binding, absence of free-form shell input, and the fail-closed test-file grammar. Repository-wide pytest/coverage, docstring checks, actionlint, security workflows, and current-head independent review remain merge evidence only when they execute on the unchanged exact current head; predecessor results are historical evidence, not transferable approval. + +The unresolved Devin thread on the vulnerable implementation must remain unresolved until exact-head evidence proves the repaired successor. Queue saturation is not authority to bypass this substantive security finding. + +## Context and standards + +Reusable workflows establish an execution boundary: GitHub explicitly documents that called workflows receive permissions constrained by the caller and that permissions cannot be elevated through the call chain. This repair additionally minimizes the command surface so caller-controlled values remain data rather than command text. Shell/path validation here is defense in depth; the primary design rule is that the workflow itself owns executable source. ## References (APA 7th edition) -r-lib. (n.d.). *actions: GitHub Actions for the R community* [Computer -software]. GitHub. Retrieved 2026-09-02, from -https://github.com/r-lib/actions/tree/v2/examples +GitHub, Inc. (n.d.). *Reusing workflows*. GitHub Docs. Retrieved September 2, 2026, from https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows + +GitHub, Inc. (n.d.). *Workflow syntax for GitHub Actions*. GitHub Docs. Retrieved September 2, 2026, from https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax -GitHub, Inc. (n.d.). *Reusing workflows*. GitHub Docs. Retrieved -2026-09-02, from -https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows +r-lib. (n.d.). *actions: GitHub Actions for the R community* [Computer software]. GitHub. Retrieved September 2, 2026, from https://github.com/r-lib/actions/tree/v2/examples