Skip to content

Add PR syntax check for modified config/content files#693

Merged
fredericsimard merged 5 commits into
mainfrom
ci/pr-syntax-check/2026-07-21
Jul 22, 2026
Merged

Add PR syntax check for modified config/content files#693
fredericsimard merged 5 commits into
mainfrom
ci/pr-syntax-check/2026-07-21

Conversation

@fredericsimard

Copy link
Copy Markdown
Contributor

Summary

Adds a pull_request GitHub Actions workflow that validates the syntax of files a PR modifies and fails visibly (red X) when something is malformed, so problems are caught before merge.

  • Covered file types: .yml/.yaml, .json, .md/.markdown, requirements.txt, Makefile.
  • Not a required status check — it does not block merging, it only surfaces issues (per the requested behavior).
  • Only PR-modified files are scanned (via the base...head diff), so pre-existing issues in untouched files never trip it.

Files

  • .github/workflows/pr-syntax-check.yml — the workflow (runs on opened/synchronize/reopened).
  • scripts/ci/check-syntax.py — per-type validation; emits GitHub error annotations + a findings JSON.
  • scripts/ci/post-review.py — posts one grouped inline review with suggestion blocks (same-repo PRs only).

Design notes for reviewers

  • Syntax validity only, not style/formatting.
  • YAML is validated with yaml.compose_all, which tolerates the !!python/name: tags in config/*/mkdocs.yml that a plain safe_load would falsely reject — while still catching genuine YAML errors.
  • Findings are delivered two ways: annotations (render inline on the Files-changed tab, and work for fork PRs), and, on same-repo PRs, inline review suggestions with one-click fixes where the repair is unambiguous (e.g. a Makefile recipe line that must start with a tab). Suggestions are filtered to lines actually present in the PR diff, so GitHub's reviews API won't reject the review.
  • Fork PRs get a read-only token and can't post reviews, so they receive annotations + the failing check only.
  • Deliberately avoids pull_request_target and make -n so untrusted PR content is never executed on the runner.

Testing

  • Verified locally: all real repo files pass clean (including mkdocs.yml with !!python/name: tags); deliberately-broken YAML/JSON/requirements/Makefile/Markdown samples are each caught with the correct line number; the diff-hunk line filtering and the changed-file type filter behave as intended.
  • Not yet exercised live (requires a real PR): the base...head diff inside the runner and the review-POST to the API. This PR modifies only .yml and .py files, so opening it should trigger the workflow to validate its own diff as a smoke test.

Note

Uses actions/checkout@v7.0.0 and actions/setup-python@v6.3.0, matching the pins already used in update-requirements.yaml.

Adds a pull_request workflow that validates the syntax of files a PR
modifies (.yml/.yaml, .json, .md/.markdown, requirements.txt,
Makefile) and fails visibly when something is malformed, so problems
are caught before merge. It is intentionally NOT a required status
check — it does not block merging, it only surfaces issues.

Design notes:
- Syntax validity only, not style/formatting. YAML is validated with
  yaml.compose_all so it tolerates the !!python/name: tags in
  config/*/mkdocs.yml that a plain safe_load would falsely reject,
  while still catching genuine YAML errors.
- Only PR-modified files are scanned (base...head diff), so
  pre-existing issues in untouched files never trip the check.
- Findings are emitted as GitHub error annotations (work for fork PRs
  too) and, on same-repo PRs, as one grouped inline review with
  ```suggestion blocks where the fix is unambiguous (Makefile
  tab-indent). Suggestions are filtered to lines actually in the PR
  diff so the reviews API won't reject them.
- Avoids pull_request_target and `make -n` so untrusted PR content is
  never executed on the runner.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a GitHub Actions pull_request workflow to validate the syntax of PR-modified config/content files and surface problems via annotations (and, for same-repo PRs, an inline review with suggestions) before merge.

Changes:

  • Introduces .github/workflows/pr-syntax-check.yml to detect modified files in a PR and run syntax validation.
  • Adds scripts/ci/check-syntax.py to validate YAML/JSON/Markdown fences/requirements/Makefile recipe indentation and emit annotations + a findings JSON.
  • Adds scripts/ci/post-review.py to post a single grouped PR review with inline comments/suggestions limited to changed lines.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/pr-syntax-check.yml New PR workflow to run syntax checks on changed files, post suggestions (same-repo), and fail the check when findings exist.
scripts/ci/check-syntax.py Implements per-file-type syntax validation and emits GitHub Actions annotations + findings JSON.
scripts/ci/post-review.py Posts a single PR review with inline comments/suggestions for findings that land on changed lines.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/ci/check-syntax.py
Annotation messages can include untrusted text (e.g. an invalid
requirement string from a PR file). Only \n was handled before,
leaving % and \r unescaped, so crafted content could inject extra
::error:: workflow commands. Apply the canonical GitHub Actions
escaping (% first, then \r/\n) to messages, plus : and , to property
values.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread scripts/ci/post-review.py Outdated
Comment thread scripts/ci/post-review.py Outdated
added_lines() had two bugs that could drop valid findings from the
posted review:
- Skipping any line starting with "+++" also skipped real added lines
  whose content begins with "+++". Now only the pre-hunk header region
  is skipped, so the "+++ b/path" header is ignored while genuine
  "+++..." content lines (which only appear inside hunks) are counted.
- The "\ No newline at end of file" marker was treated as a context
  line and advanced the new-side counter, desyncing later hunks. It is
  now skipped without advancing.

Also reworded the module docstring so it no longer opens an unclosed
```-fence at column 0.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread scripts/ci/post-review.py
Comment thread scripts/ci/check-syntax.py
check_makefile: skip define ... endef blocks, where any indentation is
valid. Previously a tab-indented line inside such a block set in_rule,
producing false-positive space-indentation findings on later lines.

post_review: also catch OSError (covers URLError from DNS/TLS/refused
connections and bare timeouts, both OSError subclasses) so a network
failure while posting the review can never crash the script and fail
the job — annotations and the enforce step already report findings.
Also added a 30s request timeout.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread scripts/ci/check-syntax.py
Comment thread scripts/ci/post-review.py Outdated
Comment thread .github/workflows/pr-syntax-check.yml Outdated
- workflow: pin pyyaml/packaging to the exact versions from
  requirements.txt (via grep) so the checker's parser behaviour is
  reproducible and stays in sync with the site build, rather than
  installing whatever is latest.
- check_makefile: match the define/endef directives with leading
  spaces only, not \s* — a tab-indented line is a recipe, never a
  directive, so a tab-indented "define ..." no longer wrongly flips
  in_define and suppresses later findings.
- post-review main: read the findings JSON best-effort; a missing or
  malformed file now logs a warning and exits 0 instead of crashing
  the job, matching the script's report-via-annotations intent.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@fredericsimard
fredericsimard merged commit 4f29cf4 into main Jul 22, 2026
1 check passed
@fredericsimard
fredericsimard deleted the ci/pr-syntax-check/2026-07-21 branch July 22, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants