fix: remove null include key causing _reusable-test.yml to fail - #298
Conversation
…dd pkl to mise.toml Agent-Logs-Url: https://github.com/knitli/codeweaver/sessions/a7b5318c-15d0-477f-86af-9ebf2d4a699c Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes a GitHub Actions reusable workflow matrix configuration that was failing schema validation by commenting out an empty Sequence diagram for GitHub Actions reusable test workflow validation behaviorsequenceDiagram
actor Developer
participant GitHub as GitHub
participant GHActions as GitHubActions
participant ReusableWorkflow as ReusableTestWorkflow
participant Validator as SchemaValidator
Developer->>GitHub: Push to main
GitHub->>GHActions: Trigger workflow dispatch
GHActions->>ReusableWorkflow: Load _reusable-test.yml
ReusableWorkflow->>Validator: Validate workflow schema
alt Before_fix
Validator-->>ReusableWorkflow: include field is null
Validator-->>GHActions: Validation error (invalid matrix.include)
GHActions-->>Developer: Workflow fails with 0 jobs
else After_fix
Validator-->>ReusableWorkflow: include field absent (valid)
Validator-->>GHActions: Validation passes
GHActions->>ReusableWorkflow: Evaluate matrix and create jobs
ReusableWorkflow-->>Developer: Tests run successfully
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The commented-out
includeexample in_reusable-test.ymlis a bit confusing and currently showsinclude:twice with mismatched indentation; consider rewriting it as a single commented snippet that can be safely copy-pasted/uncommented into a validstrategy.matrix.includeblock to reduce future mistakes when re-enabling free-threaded Pythons.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The commented-out `include` example in `_reusable-test.yml` is a bit confusing and currently shows `include:` twice with mismatched indentation; consider rewriting it as a single commented snippet that can be safely copy-pasted/uncommented into a valid `strategy.matrix.include` block to reduce future mistakes when re-enabling free-threaded Pythons.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Fixes a GitHub Actions reusable workflow validation failure (0 jobs) by ensuring the matrix include key is not present as null, and updates the dev toolchain to include the Pkl tool required by local hook tooling (hk).
Changes:
- Comment out the empty
strategy.matrix.includekey in_reusable-test.ymlto avoid schema-invalidnull. - Add
pkl = "0.31.1"tomise.tomlsohkcan parsehk.pklduring local hook runs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mise.toml |
Declares Pkl in the global mise tool list to support hk/Pkl-based hook config. |
.github/workflows/_reusable-test.yml |
Removes the schema-invalid empty matrix.include by commenting out the key. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uv = "latest" | ||
| "pipx:exportify" = "latest" | ||
| hk = "1.38.0" | ||
| pkl = "0.31.1" |
There was a problem hiding this comment.
pkl is pinned to 0.31.1 in the global [tools] section, but this file also requests tools.pkl = "latest" in task-level tool definitions (e.g., tasks.check / tasks.format). That can lead to different Pkl versions being used in different contexts (hooks vs tasks vs CI), making failures harder to reproduce. Consider aligning these to a single version (either pin everywhere or rely on latest everywhere) so the toolchain is consistent.
| pkl = "0.31.1" | |
| pkl = "latest" |
| # include: | ||
| # Free-threaded Python versions (no-GIL) are temporarily disabled because upstream | ||
| # package compatibility issues prevent them from building successfully. Re-enable | ||
| # once the blocking dependencies add free-threaded Python support: | ||
| # | ||
| # include: | ||
| # - python-version: 3.13t | ||
| # experimental: true | ||
| # - python-version: 3.14t | ||
| # experimental: true |
There was a problem hiding this comment.
The commented re-enable snippet duplicates include: (once at the correct indentation, then again as # include:). This is easy to misread and could lead to uncommenting the wrong lines later. Prefer a single commented include: block with the list items directly under it (and keep the explanatory note above) so re-enabling is a simple uncomment without having to restructure indentation.
| # include: | |
| # Free-threaded Python versions (no-GIL) are temporarily disabled because upstream | |
| # package compatibility issues prevent them from building successfully. Re-enable | |
| # once the blocking dependencies add free-threaded Python support: | |
| # | |
| # include: | |
| # - python-version: 3.13t | |
| # experimental: true | |
| # - python-version: 3.14t | |
| # experimental: true | |
| # Free-threaded Python versions (no-GIL) are temporarily disabled because upstream | |
| # package compatibility issues prevent them from building successfully. Re-enable | |
| # once the blocking dependencies add free-threaded Python support: | |
| # | |
| # include: | |
| # - python-version: 3.13t | |
| # experimental: true | |
| # - python-version: 3.14t | |
| # experimental: true |
Code Review SummaryI've reviewed PR #298 and the changes look good. This PR successfully addresses a critical GitHub Actions workflow validation issue. What Changed1. (lines 81-90)
2.
Technical AnalysisThe root cause was subtle but important: YAML parses an
The fix properly addresses this by commenting out the key itself (not just its children), which ensures the field is absent rather than Quality Assessment✅ Correctness: The fix properly resolves the schema validation issue Minor Suggestion (Non-blocking)I agree with Sourcery's feedback that the commented-out example in RecommendationAPPROVED ✅ This fix is correct and resolves a blocking issue that was preventing the workflow from running any jobs. The addition of Note: I see this PR is already merged, which makes sense given it was blocking CI. This review is for documentation purposes. |
Code Review SummaryI've reviewed PR #298 and the changes look good. This PR successfully addresses a critical GitHub Actions workflow validation issue. What Changed1. .github/workflows/_reusable-test.yml (lines 81-90)
2. mise.toml (line 64)
Technical AnalysisThe root cause was subtle but important: YAML parses an
The fix properly addresses this by commenting out the key itself (not just its children), which ensures the field is absent rather than Quality Assessment✅ Correctness: The fix properly resolves the schema validation issue Minor Suggestion (Non-blocking)I agree with Sourcery's feedback that the commented-out example in .github/workflows/_reusable-test.yml (lines 81-90) shows RecommendationAPPROVED ✅ This fix is correct and resolves a blocking issue that was preventing the workflow from running any jobs. The addition of Note: I see this PR is already merged, which makes sense given it was blocking CI. This review is for documentation purposes. |
There was a problem hiding this comment.
Review Summary
This PR correctly fixes the GitHub Actions schema validation error that was causing the reusable test workflow to fail with 0 jobs.
Changes Reviewed
1. Workflow Fix ()
- ✅ Correctly comments out the
include:key that was causing schema validation failures - ✅ YAML was parsing
include:with only commented children asnull, which GitHub Actions schema validation rejects - ✅ Preserves documentation about why free-threaded Python versions are temporarily disabled
- ✅ Maintains proper indentation and formatting
2. Tool Addition (mise.toml:64)
- ✅ Adds
pkl = "0.31.1"to the tools section - ✅ Required by the
hkpre-push hook runner to parsehk.pklconfig - ✅ Ensures consistency between CI and local development environments
Technical Assessment
- Root Cause Analysis: Accurate and thorough
- Solution: Minimal, non-invasive, and technically correct
- Documentation: Well-explained in the PR description
- Alternative Considered: Could have removed the entire commented block, but preserving it as commented documentation is better for future reference
- CI Status: Lint/Format checks passing, CodeQL passing
Security & Performance
- No security concerns identified
- No performance impact
- Workflow will now properly execute with the expected Python version matrix
Recommendation
APPROVED ✅
This PR should be merged. The fix is straightforward, well-documented, and solves the immediate problem of workflows failing with 0 jobs. The addition of the pkl tool ensures local development environments match the CI requirements.
Great work identifying and fixing this issue!
There was a problem hiding this comment.
Review Summary
This PR correctly fixes the GitHub Actions schema validation error that was causing the reusable test workflow to fail with 0 jobs.
Changes Reviewed
1. Workflow Fix (.github/workflows/_reusable-test.yml:81-90)
- Correctly comments out the include: key that was causing schema validation failures
- YAML was parsing include: with only commented children as null, which GitHub Actions schema validation rejects
- Preserves documentation about why free-threaded Python versions are temporarily disabled
- Maintains proper indentation and formatting
2. Tool Addition (mise.toml:64)
- Adds pkl = "0.31.1" to the tools section
- Required by the hk pre-push hook runner to parse hk.pkl config
- Ensures consistency between CI and local development environments
Technical Assessment
- Root Cause Analysis: Accurate and thorough
- Solution: Minimal, non-invasive, and technically correct
- Documentation: Well-explained in the PR description
- Alternative Considered: Could have removed the entire commented block, but preserving it as commented documentation is better for future reference
- CI Status: Lint/Format checks passing, CodeQL passing
Security & Performance
- No security concerns identified
- No performance impact
- Workflow will now properly execute with the expected Python version matrix
Recommendation
APPROVED
This PR should be merged. The fix is straightforward, well-documented, and solves the immediate problem of workflows failing with 0 jobs. The addition of the pkl tool ensures local development environments match the CI requirements.
Great work identifying and fixing this issue!
Summary
The
_reusable-test.ymlreusable workflow was failing with 0 jobs on every push tomain. The root cause was a GitHub Actions schema validation error, not a YAML syntax error.Root Cause
In the
strategy.matrixblock, theinclude:key was present but had only commented-out entries:YAML parses
include:with no non-comment children asnull. GitHub Actions schema validation rejectsnullfor theincludefield — it must be a list or absent entirely. This caused the workflow to fail immediately with 0 jobs before any steps ran.Fix
Commented out the
include:key itself, preserving the note about re-enabling free-threaded Python versions once upstream compatibility issues are resolved.Also
pkl = "0.31.1"tomise.toml— required by thehkpre-push hook runner to parsehk.pklconfig (was missing, causing all pushes to fail locally).Validation
check-jsonschema --builtin-schema github-workflowsnow passes on_reusable-test.ymland all other workflow files.Summary by Sourcery
Fix reusable test workflow validation and ensure local hook tooling dependencies are declared.
Bug Fixes:
strategy.matrix.includekey.Build:
pkltool version inmise.tomlso localhkpre-push hooks can run successfully.