Host the Validate Task and Reshape the Test Pull Request Stub - #760
Conversation
There was a problem hiding this comment.
Pull request overview
This PR moves the fleet validation gate to a single hub-hosted reusable workflow (validate-task.yml) and reshapes the hub and downstream “test pull request” model around a thin caller stub plus an optional validate hook for repo-specific checks. It also updates the spec/audit/doc tooling so the new workflow-reuse architecture is documented and enforceable.
Changes:
- Converts
.github/workflows/validate-task.ymlinto aworkflow_calltask withlint,unit-test, andvalidate(hook) jobs, and adds an actionlint ignore forjob.workflow_sha. - Introduces hub-local validation hooks:
.github/actions/validate(hub override) and.github/actions/validate-default(no-op default). - Updates governance/docs/spec/audit tooling (manifest contract, divergences “retire” entry, prose-lint hub-hosted path allowlist, and new audit selftests).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
TODO.md |
Updates the Stage 2 gates tracker text to match the new hosted validate-task + stub shapes. |
spec/files.json |
Removes carried validate-task.yml and strengthens the test-pull-request.yml interface contract to require reaching validate-task.yml. |
spec/divergences.json |
Adds a retire disposition entry for downstream copies of validate-task.yml now that it is hub-hosted. |
spec/audit.py |
Adds selftests intended to enforce the new “caller stub reaches validate-task” contract. |
scripts/prose_lint.py |
Extends the hub-hosted path allowlist so prose referencing hub-hosted validate-task.yml is not flagged as dead-path. |
GOVERNANCE.md |
Notes that the doc-lint block is hosted in the hub validate-task reusable workflow. |
docs/reusable-workflows.md |
Documents Stage 2 changes and adds an “Adopting the Gates” section with stub templates. |
.github/workflows/validate-task.yml |
Implements the new reusable validation gate with lint/unit-test/hook jobs. |
.github/actions/validate/action.yml |
Adds the hub’s override validate hook (registry/spec validation, selftests, etc.). |
.github/actions/validate-default/action.yml |
Adds the hub default no-op validate hook used when callers do not provide one. |
.github/actionlint.yaml |
Suppresses actionlint’s unknown-property warning for job.workflow_sha in validate-task.yml. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8419c2f to
ce9ecff
Compare
ce9ecff to
d7f5c40
Compare
d7f5c40 to
3ff3f87
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
docs/reusable-workflows.md:392
- The new reference-style link for the hook override run needs a link definition in the reference block, and it should be placed in alphabetical order within the group.
[governance-hub-hosted-tooling]: ../GOVERNANCE.md#hub-hosted-tooling
[governance-workflow-yaml-conventions]: ../GOVERNANCE.md#workflow-yaml-conventions
[issue-585]: https://github.com/ptr727/ProjectTemplate/issues/585
[pr-760]: https://github.com/ptr727/ProjectTemplate/pull/760
[secrets]: ../spec/secrets.json
docs/reusable-workflows.md:154
- This checklist item uses an inline URL, but docs/reusable-workflows.md is not in the inline-link exception set (only AGENTS.md, GOVERNANCE.md, OPERATIONS.md, and .github/copilot-instructions.md). Convert the URL to a reference-style link and add a definition in the link block at the bottom.
This issue also appears on line 388 of the same file.
- [x] Hook override path observed on a hub pull request run, <https://github.com/ptr727/ProjectTemplate/actions/runs/31950332387/job/95172710046> (runs `./.github/actions/validate`, no hub checkout). Default path awaits a repo with no `validate` hook of its own.
.github/workflows/validate-task.yml:214
- The Python unit-test path runs
uv sync --all-groups --frozen, but the fleet spec distinguishes a lint-only Python profile that must not have auv.lock(spec/project-types.json python profileNote). If a repo haspyproject.toml+tests/but nouv.lock, this job will currently attempt a frozen sync and fail. Guard the Python unit-test steps onuv.lockso the build-profile path runs tests/coverage while lint-only subtrees skip cleanly.
- name: Setup uv step
if: hashFiles('pyproject.toml') != '' && hashFiles('tests/**') != ''
uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0
with:
python-version: "3.13"
|
Answering the 3 suppressed findings from the round on
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
.github/workflows/validate-task.yml:170
- The prose gate needs the PR base branch name for
--diff origin/$BASE. In a reusableworkflow_call,github.base_refis not the right source; use the PR payload's base ref so the step works when invoked from a pull request.
BASE: ${{ github.base_ref }}
|
Re the |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/validate-task.yml:115
- The C# lint steps run
dotnet csharpier/dotnet formatwhenever a*.csprojexists, but the workflow only restores tools when.config/dotnet-tools.jsonexists. That makes the gate depend on runner-global tool availability (or fail with a generic "unknown command"), and thedotnet formatinvocation also drops the repo’s canonical arguments (--severity=info --verbosity=detailed). Consider explicitly requiring the tool manifest (or installing the tools) and use the canonicaldotnet formatflags so the check is deterministic and matches the fleet’s clean-compile contract.
- name: Check C# formatting step
if: hashFiles('**/*.csproj') != ''
run: dotnet csharpier check .
|
Answering the suppressed finding from the round on "C# lint steps run csharpier/dotnet format whenever a csproj exists, but tools are only restored when .config/dotnet-tools.json exists" (.github/workflows/validate-task.yml:115): quoting the finding, correct on both points. Fixed: the csharpier step now shares the restore step's two-part guard ( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/validate-task.yml:174
- In the prose gate, the script diffs against
origin/$BASE, butactions/checkouttypically does not fetchrefs/remotes/origin/<base>unless explicitly requested. This can makegit rev-parse origin/$BASEfail even withfetch-depth: 0, breaking the prose gate on PR runs. Fetch the base branch ref before resolving/diffing against it.
if ! git rev-parse --verify --quiet "origin/$BASE^{commit}" >/dev/null; then
echo "::error::Diff base 'origin/$BASE' does not resolve in this checkout." >&2
exit 1
fi
python3 "$script" --diff "origin/$BASE" --check charset --check semicolon --check dash --check dupword --check spelling --check comment-wrap --check comment-case --check home-path --check dead-path .
The PR event payload already carries the exact base commit (github.event.pull_request.base.sha), which needs no remote-tracking branch ref to be present in the checkout, unlike a branch name would. The diff base now reads that SHA directly. The branch name (base.ref) still decides which rules source to read (bundled versus hub develop), since that choice is about the target branch, not the commit.
Reconsiders the prior round's decline: job.workflow_repository names wherever this reusable workflow itself lives, so it is correct for every real caller today (it resolves to ptr727/ProjectTemplate, since that is where the task is hosted) and it also makes a full fork or mirror of the hub test its own develop rules rather than always reaching upstream, which is what the checkout-hub steps already do for the ref. The prose step's curl fallback now reads the script from $HUB_REPO instead of the hard-coded owner, closing the one place that still hard-coded it.
repo_gate.py's sha-pin check shells out to gh api to resolve a same-owner action pin against GitHub. A runner's gh has no credentials of its own, so every call degraded to "GitHub did not answer" in this PR's own CI run (confirmed in the Lint sources job log: resolved 0 pin(s), 1 GitHub did not answer for), silently losing same-owner pin coverage rather than failing loud. Exports GH_TOKEN from the job's own token, which the lint job's existing contents: read grant already covers for a read-only commit lookup.
25f74b9 to
b6c03c5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/validate-task.yml:36
- The hub checkout happens before the repo-wide linters run. Tools like markdownlint (globs '**/*.md') and editorconfig-checker scan the whole workspace and will include the
.hub/tree, so a caller's lint result can become coupled to (and potentially fail on) files from the checked-out hub commit rather than only the caller repo. Consider moving the.hubcheckout to just before the steps that actually need it (prose gate and repo_gate), or explicitly excluding.hub/**from each repo-wide linter invocation.
- name: Checkout hub step
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
repository: ${{ job.workflow_repository }}
ref: ${{ job.workflow_sha }}
GOVERNANCE.md:219
- This CI linter list now claims it is exactly the hub's validate-task.yml block, but validate-task.yml also runs shellcheck on tracked '*.sh' files. Omitting shellcheck here makes the doc misleading for someone trying to understand or mirror the CI lint surface.
- **CI (authoritative)** runs **markdownlint-cli2**, **cspell**, and **actionlint** as pinned action wrappers (Dependabot bumps them), plus **editorconfig-checker** via Docker `:latest` (its action only installs the CLI, so the Docker one-liner is what actually runs the check), and, **for a repo that carries `.ps1` files**, **PSScriptAnalyzer** the same way (it has no action either). markdownlint covers all `**/*.md`, and **cspell is scoped to `README.md` + `HISTORY.md`** (see [CODESTYLE.md](./CODESTYLE.md) "Markdown and Spelling" for why), matching the cspell one-liner below. This whole block is the hub's `validate-task.yml` reusable workflow, per [`docs/reusable-workflows.md`](./docs/reusable-workflows.md), so a fleet repo reaches it rather than carrying a copy of these steps.
The hub checkout landed in the workspace before markdownlint, editorconfig-checker, and the shell and PowerShell checks ran, and each of those scans the whole tree by glob rather than by git ls-files, so a caller's lint result could pick up files from the checked-out hub commit under .hub/ rather than only the caller's own tree. Moves the checkout to immediately before the two steps that actually read it, the prose gate and the repo gate. GOVERNANCE.md's "Running the Linters Locally" list named every doc linter this block runs except shellcheck, which the block does run. The claim that the whole list is exactly the hub's validate-task.yml block, added earlier in this PR, made that omission an inaccuracy rather than a pre-existing gap, so the bullet now names shellcheck alongside PSScriptAnalyzer.
|
Answering the 2 suppressed findings from the round on
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (2)
GOVERNANCE.md:219
- The parenthetical
(neither has an action either)is grammatically incorrect/redundant (both "neither" and "either"). Rephrase to avoid the double-negative construction while keeping the intended meaning.
- **CI (authoritative)** runs **markdownlint-cli2**, **cspell**, and **actionlint** as pinned action wrappers (Dependabot bumps them), plus **editorconfig-checker** via Docker `:latest` (its action only installs the CLI, so the Docker one-liner is what actually runs the check), **shellcheck** the same way for a repo that carries `.sh` files, and, **for a repo that carries `.ps1` files**, **PSScriptAnalyzer** the same way (neither has an action either). markdownlint covers all `**/*.md`, and **cspell is scoped to `README.md` + `HISTORY.md`** (see [CODESTYLE.md](./CODESTYLE.md) "Markdown and Spelling" for why), matching the cspell one-liner below. This whole block is the hub's `validate-task.yml` reusable workflow, per [`docs/reusable-workflows.md`](./docs/reusable-workflows.md), so a fleet repo reaches it rather than carrying a copy of these steps.
.github/workflows/validate-task.yml:169
- The prose-gate step downloads
scripts/prose_lint.pyfrom raw.githubusercontent.com without authentication. This can fail for private forks (or get hit by tighter unauthenticated rate limits) even though${{ github.token }}is available. Include an Authorization header so the fetch works consistently across repo visibility.
script="$RUNNER_TEMP/prose_lint.py"
curl -fsSL --retry 3 --retry-delay 2 --retry-all-errors \
"https://raw.githubusercontent.com/$HUB_REPO/develop/scripts/prose_lint.py" -o "$script"
The curl fallback fetching prose_lint.py from raw.githubusercontent.com ran unauthenticated, which raw.githubusercontent.com refuses outright for a private repository regardless of visibility elsewhere, so a private hub fork's downstream callers could never reach it. Adds an Authorization header from the job's own token, harmless for the public case since the request still succeeds either way. GOVERNANCE.md's "neither has an action either" read as an accidental double negative. Reworded to "neither one has an action."
|
Answering the 2 suppressed findings from the round on
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (4)
spec/audit.py:2160
- Same issue as the case above: this fixture includes
needs: [changes]incheck-workflow-statuseven though the workflow body here does not define achangesjob. Updating theneeds:to referencevalidatekeeps the fixture valid and closer to the intended stub shape.
"PR stub validate job still carrying an inline lint job",
pr_validate_inline + pr_check,
pr_stub_contract,
1,
),
.github/workflows/validate-task.yml:140
- The pyproject section detection is anchored to column 0 (e.g.,
^\[tool\.mypy\]). TOML section headers can be indented (even if uncommon), which would cause the type-check step to skip mypy/pyright unexpectedly. Use a whitespace-tolerant regex so the gate behaves consistently.
set -Eeuo pipefail
if grep -q '^\[tool\.mypy\]' pyproject.toml; then
uvx mypy@latest
elif grep -q '^\[tool\.pyright\]' pyproject.toml; then
uvx pyright@latest
docs/reusable-workflows.md:355
- In a
pull_requestworkflow,${{ github.ref_name }}resolves to a PR ref (e.g.123/merge), not the target branch (main/develop). Sincebuild-release-task.ymlrequires a logicalbranchinput, the stub should pass the base branch for PR runs; otherwise smoke builds will label artifacts/tags with a PR ref string.
smoke: true
github: false
dockerhub: false
branch: ${{ github.ref_name }}
spec/audit.py:2154
- This selftest fixture combines a
validatejob with acheck-workflow-statusjob that stillneeds: [changes], but the fixture does not define achangesjob. That produces an unrealistic (and GitHub-invalid) workflow shape, which makes the test less representative of the stub it is supposed to validate.
This issue also appears on line 2156 of the same file.
"PR stub validate job reaching the hub validate-task",
pr_validate_head + pr_check,
pr_stub_contract,
0,
),
The Adopting the Gates smoke-build example passed github.ref_name as the logical branch, which on a pull_request event is the PR ref (for example 123/merge), not the target branch, so a smoke build's branch-derived config (cache tags, buildcache selection) would key on the wrong value. It now reads github.base_ref first and falls back to ref_name only on a non-PR trigger, matching the same pattern already used elsewhere in the fleet. The pyproject section grep for mypy and pyright was anchored to column 0, which a TOML section header need not be (the grammar allows leading whitespace, however rare in practice). Both patterns now tolerate it. spec/audit.py's two validate-stub selftest fixtures paired a validate job with an aggregator needing changes, a job neither fixture defines. They now carry a needs: [validate] variant instead, matching the job they actually define.
|
Answering the 4 suppressed findings from the round on 1 & 2. "selftest fixtures pair a validate job with an aggregator needing changes" (spec/audit.py:2154, 2156, 2160): quoting the finding, correct:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/validate-task.yml:83
- The PowerShell analyzer step passes the file list via an env var and then splits it on
\s+, which will break any tracked.ps1path that contains spaces (Git allows this, and the bash side already collects paths safely into an array). Prefer passing a newline-delimited list and splitting on newlines so paths are preserved.
docker run --rm --pull=always -e PS_SCRIPTS="$(git ls-files '*.ps1')" -v "$PWD":/mnt --workdir /mnt mcr.microsoft.com/powershell:latest \
pwsh -NoProfile -Command '
Set-PSRepository PSGallery -InstallationPolicy Trusted
Install-Module PSScriptAnalyzer -RequiredVersion 1.23.0 -Force -Scope AllUsers
Import-Module PSScriptAnalyzer
|
Answering the suppressed finding from the round on "PowerShell analyzer step splits the file list on \s+, breaking a .ps1 path containing a space" (.github/workflows/validate-task.yml:83): quoting the finding, declined. This step is carried unchanged from the hub's own pre-existing |
… Default (#768) Promote `develop` to `main`, carrying the remaining stages of the hub-hosted reusable-workflow rollout and the changes that landed beside them: - #759 Host Get-Version and Publish-Plan as Hub Reusable Tasks - #760 Host the Validate Task and Reshape the Test Pull Request Stub (settles #729 by design: the hub's validate task runs `uvx <tool>@latest`, since Dependabot tracks the action pins and not a uvx version) - #761 Host the Type-Specific Tasks and Retire the Date Badge - #762 Host the Release Chain and the Docker Core in the Hub - #748 and #752, the staged rollout tracker and the PhotoCleaner merge-bot pilot record - #758 Flip the Fleet Line-Ending Default from CRLF to LF - #753, #755, #756, #764, host-setup and test-collection changes The release that follows this promotion is the first tag carrying every hub task, so it is the pin the stage 2 to 5 adoptions and their catalog snippets use. It is also the first run of the hub's own `publish-release.yml` through `build-release-task.yml` with every target disabled, which is the live proof that `github-release` runs when its build needs are skipped. Closes #729. Refs #521 (hub half shipped, the merge-bot adoption sweep is what remains).
Hosts the fleet's validation gate once, as
validate-task.yml, perdocs/reusable-workflows.md"Stage 2: The Gates".What ships
.github/workflows/validate-task.ymlbecomes aworkflow_calltask with three jobs:lint: the fleet doc-lint block (markdownlint, cspell, actionlint, editorconfig-checker, shellcheck and PSScriptAnalyzer gated on file presence), language lint by tree detection (a*.csprojruns the C# checks, apyproject.tomlruns ruff/mypy/pyright), the prose gate scoped to a pull request's diff, and the repo gate against the caller's root.unit-test: a genericdotnet testoruv run pytestwith Codecov upload, skipped cleanly where the caller carries no test project.validate: resolves avalidatehook (.github/actions/validatein the caller, else the new no-op.github/actions/validate-defaultin the hub) for a repo's own domain checks (an ESPHome compile, a Hugo build, a KiCad ERC, a codegen-drift check, PowerShell tests).test-pull-request.ymlandpublish-release.ymlkeep calling./.github/workflows/validate-task.ymlunchanged, and the hub's own domain steps (the registry/spec check, the coverage-wrapped self-tests, the fleet-skills freshness check, and the unclassified-character report) move into a new hub-local hook,.github/actions/validate/action.yml. This makes the hub exercise the hook's override path on every hub pull request, and a repo carrying no hook of its own exercise the default.test-pull-request-task.yml: the ruleset-bound aggregator has to stay in the caller stub (a called job's check context would read<caller job> / <callee job>and break the ruleset binding), and a second hub task wrapping the one line a caller stub already writes to callvalidate-task.ymlhosts nothing generic. The two downstream stub shapes (an operational no-build repo, and a release repo with a smoke build) are documented instead, indocs/reusable-workflows.md"Adopting the Gates", closing Operational repos run no CI on a pull request into develop, contradicting D1.2 #585 by construction.uvx <tool>@latestinvocations are pinned or floated in this one place, Dependabot tracking the action SHA that installsuvrather than auvxversion nothing tracks.spec/files.json:validate-task.ymlstops being a manifestintententry, since the hub hosts it rather than every repo carrying a copy.test-pull-request.yml'sinterfacecontract gainsrequireTokensInJob: {"validate": ["validate-task.yml"]}.spec/divergences.json: a newretiregap disposition names the 13 current carriers (PhotoCleaner, PlexCleaner, LanguageTags, Utilities, MediaTools, AudioCleaner, aiopurpleair, Financial-Modeling, Blog, ESPHome-NonRoot, NxWitness, VSCode-Server-DotNetCore, HomeAutomation-Config).spec/audit.py: new_selftestcases for therequireTokensInJobcontract, including a stub still carrying an inline lint job.scripts/prose_lint.py:HUB_HOSTEDgains the retired path, so a repo naming the hub's copy in its own prose is not read as a dead path, with a hub self-test asserting the literal matches the ledger.docs/reusable-workflows.md: the Hook Catalog drops thetest-pull-request-task.ymlrow, a new "Adopting the Gates" section gives both downstream stub shapes, and the Rollout tracker's Stage 2 item is reworded to match what shipped.TODO.md: the gates entry's Checked/Settled lines updated to what shipped, both open questions resolved, and Operational repos run no CI on a pull request into develop, contradicting D1.2 #585/Decide whether a lint-only Python subtree pins its uvx tool version or runs @latest #729 noted settled by design.GOVERNANCE.md: one added sentence on "Running the Linters Locally" naming the doc-lint block as the hub's validate task.Design choices where the brief left room
job.workflow_sha, notgithub.job_workflow_sha. The brief's own text (and the hub'sdocs/reusable-workflows.md) namedgithub.job_workflow_sha, but that property does not exist. The real GitHub Actions property for "the commit SHA of the reusable workflow file that defines the current job" isjob.workflow_sha. Fixed at both call sites in the new task and indocs/reusable-workflows.md's "Layers" section, verified live:github.job_workflow_shafails actionlint (unknown property on thegithubcontext) andjob.workflow_shapasses clean.job.workflow_shayet either (its bundled context schema for thejobcontext lists onlycheck_run_id,container,services,status), even though it is a real, GitHub-documented property. Added a scoped.github/actionlint.yamlignorerule for the one file that reads it, rather than dropping the checkout-at-pin design.unit-test's job-levelif:cannot usehashFiles(GitHub Actions only evaluates it in a step context). Moved the tree-detection guard onto every step instead; a caller with no test project runs the job with every step skipped and reports success, which is the clean skip the design calls for.validatehook makes the hub exercise the override path and a hookless repo exercise the default. A later instruction's parenthetical read the opposite way round ("default path on a hub PR run, override path on the pilot"). I followed the explicit design-decision statement, since it states the actual mechanism and the parenthetical does not, and worded the tracker's proof item to match what the code does.Gates run
spec/validate.py,python3 -m unittest discover -s scripts/tests,spec/audit.py --selftest,spec/workflow_reuse.py --selftest,scripts/repo_gate.py,scripts/build_dist.py --check,scripts/prose_lint.py(charset/semicolon/dash/dupword/spelling/comment-wrap/comment-case/home-path/dead-path),uvx ruff@latest check .,uvx ruff@latest format --check .,uvx mypy@latest, and the Docker linters (actionlint, markdownlint-cli2, editorconfig-checker) all pass clean.Not proven here (proof items left in the tracker)
🤖 Generated with Claude Code