Skip to content

feat(lint): prettier checks only PR-changed files by default#29

Merged
topcoder1 merged 1 commit into
mainfrom
lint-changed-only
May 3, 2026
Merged

feat(lint): prettier checks only PR-changed files by default#29
topcoder1 merged 1 commit into
mainfrom
lint-changed-only

Conversation

@topcoder1

Copy link
Copy Markdown
Owner

Summary

  • Adds prettier_changed_only input (default true).
  • On PR events, the prettier job intersects the PR's added/modified/renamed file list (via gh api .../pulls/N/files) with markdown_glob and runs prettier --check on just those paths.
  • On push events, or when callers opt out (prettier_changed_only: false), behavior is unchanged: full-glob scan.
  • Backwards-compatible by design: if the caller's GITHUB_TOKEN lacks pull-requests: read, the API call fails 403, the step emits a ::warning::, and falls back to full-glob mode. No synchronized fleet update required.

Why

One badly-formatted markdown file landing on main currently poisons every subsequent docs PR's lint check across all 44 fleet repos. Discovered 2026-05-03 on PR review of topcoder1/wxa_webcat#179 where lint flagged 4 files even though the PR touched only 2 — the other 2 were pre-existing drift on main. wxa_webcat patched around it locally with .prettierignore (#180); this PR fixes the root cause for the whole fleet.

How it intersects glob ∩ changed-files

  1. gh api --paginate .../pulls/N/files --jq '.[] | select(.status != "removed") | .filename' → PR-changed list.
  2. shopt -s globstar nullglob; compgen -G "$GLOB" → working-tree files matching the glob (handles **, brace expansion).
  3. comm -12 of the two sorted sets → targets.

If the intersection is empty, mode=none and the step logs "no targets" and exits 0 (PR that touched no markdown files gets a fast pass).

Auto-merge rationale

In-band CI tooling, scoped to the prettier step, not on the high-risk surface list (no auth/secrets/migrations/billing/destructive ops/deploy infra). Backwards-compatible by design via the graceful fallback. Manual click recommended despite that — this affects every repo's lint check so a human eyeball on the self-test before fleet exposure is worth 30s.

Codex pre-review

Skipping — this is a workflow-only change, no src/** code paths, ~150 lines (just at the heuristic threshold). Self-test on this PR will exercise the new code path: EVENT_NAME=pull_request, no caller with:, queries pulls/N/files, finds lint.yml in the diff, intersects with **/*.md → empty, mode=none → no-op pass.

Test plan

  • actionlint passes on edited workflow locally
  • Self-test on this PR: prettier job's "Resolve prettier targets" step logs Mode: files (or mode=none if no MD changed) and prettier --check runs only on files in the PR diff
  • Verification PR in topcoder1/wxa_webcat (deliberate prettier issue in one MD file, caller pinned to @lint-changed-only): lint flags exactly that one file, NOT the pre-existing offenders on main

Out of scope

  • Changing what prettier considers a violation
  • Auto-rolling out pull-requests: read to existing caller permissions: blocks (separate fleet rollout — see follow-up)

🤖 Generated with Claude Code

Adds a `prettier_changed_only` input (default `true`). On `pull_request`
events, the prettier job intersects the PR's added/modified/renamed
file list (via `gh api .../pulls/N/files`) with `markdown_glob` and
runs `prettier --check` on just those paths. On `push` events, or
when the caller sets `prettier_changed_only: false`, behaviour is
unchanged: full-glob scan.

**Why:** today, one badly-formatted markdown file landing on `main`
poisons every subsequent docs PR's lint check across all 44 fleet
repos. Discovered during 2026-05-03 PR review on topcoder1/wxa_webcat
where lint flagged 4 files (2 in the PR, 2 pre-existing on main)
even though the PR only touched 2.

**Backwards compatibility:** the API call needs `pull-requests: read`
on the caller's GITHUB_TOKEN. If the caller's job permissions don't
grant it (existing 40 callers across the fleet pin the older template
that only requests `contents: read`), the gh call returns 403, the
step emits a `::warning::`, and falls back to full-glob mode. So this
deploys safely without a synchronized fleet update — repos opt into
the new behaviour by adding `pull-requests: read` to their caller's
`permissions:` block.

The workflow-level `permissions:` block already declares
`pull-requests: read` (max grant for self-test), so PRs to this
repo exercise the new code path on first install.

**Auto-merge rationale:** in-band CI tooling (scoped to the prettier
step), not on the high-risk surface list (no auth/secrets/migrations/
billing/destructive ops/deploy infra). Backwards-compatible by design
via the graceful fallback. Manual click recommended despite that —
this affects every repo's lint check, so a human eyeball on the
self-test result before fleet exposure is worth the extra 30s.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@claude

claude Bot commented May 3, 2026

Copy link
Copy Markdown

No issues found. Logic is sound, fallback paths are well-covered, and shell quoting/array handling is correct throughout.

@topcoder1
topcoder1 merged commit cd65826 into main May 3, 2026
4 checks passed
@topcoder1
topcoder1 deleted the lint-changed-only branch May 3, 2026 23:54
topcoder1 added a commit to whois-api-llc/ProfessionalServices that referenced this pull request May 4, 2026
## Summary

Adds `pull-requests: read` to this repo's `.github/workflows/lint.yml`
caller permissions block. Single-line read-scope grant, no privilege
escalation.

## Why

Required by
[topcoder1/ci-workflows#29](topcoder1/ci-workflows#29)
(merged 2026-05-03), which adds a `prettier_changed_only` mode (default
true) that lists PR-changed files via `gh api .../pulls/N/files`.
Without this permission the reusable workflow falls back to full-glob
mode and emits a `::warning::` on every PR — functional but noisy.
Granting the permission silences the warning and enables the intended
diff-only behavior.

The fleet-wide motivation is that pre-existing markdown drift on `main`
poisons every subsequent docs PR's lint check; PR-changed-only mode
fixes the root cause.

## Auto-merge rationale

Workflow-permission change → in the high-risk surface list
(`.github/workflows/**`), so manual click-merge per fleet policy. The
change itself is one line of additional `pull-requests: read` scope; no
auth, no secrets, no destructive ops.

## Codex pre-review

Skipping — workflow-only change, 1 functional line (plus 2 comment
lines), not in `src/**`.

## Test plan

- [ ] CI passes
- [ ] First post-merge PR shows the prettier job logs `Mode: files
(...)` instead of `::warning::Could not list PR files`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
topcoder1 added a commit to topcoder1/docs that referenced this pull request May 4, 2026
Required by topcoder1/ci-workflows#29 (merged 2026-05-03), which
adds a prettier_changed_only mode that lists PR-changed files via
`gh api .../pulls/N/files`. Without this permission the reusable
workflow falls back to full-glob mode and emits a ::warning:: on
every PR — functional but noisy.

Strictly grants an additional read scope, no privilege escalation.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
topcoder1 added a commit to topcoder1/jz_skills that referenced this pull request May 4, 2026
## Summary

Adds `pull-requests: read` to this repo's `.github/workflows/lint.yml`
caller permissions block. Single-line read-scope grant, no privilege
escalation.

## Why

Required by
[topcoder1/ci-workflows#29](topcoder1/ci-workflows#29)
(merged 2026-05-03), which adds a `prettier_changed_only` mode (default
true) that lists PR-changed files via `gh api .../pulls/N/files`.
Without this permission the reusable workflow falls back to full-glob
mode and emits a `::warning::` on every PR — functional but noisy.
Granting the permission silences the warning and enables the intended
diff-only behavior.

The fleet-wide motivation is that pre-existing markdown drift on `main`
poisons every subsequent docs PR's lint check; PR-changed-only mode
fixes the root cause.

## Auto-merge rationale

Workflow-permission change → in the high-risk surface list
(`.github/workflows/**`), so manual click-merge per fleet policy. The
change itself is one line of additional `pull-requests: read` scope; no
auth, no secrets, no destructive ops.

## Codex pre-review

Skipping — workflow-only change, 1 functional line (plus 2 comment
lines), not in `src/**`.

## Test plan

- [ ] CI passes
- [ ] First post-merge PR shows the prettier job logs `Mode: files
(...)` instead of `::warning::Could not list PR files`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
topcoder1 added a commit that referenced this pull request May 4, 2026
…p_failure) (#31)

## Summary

#29 added \`pull-requests: read\` to the **workflow-level**
\`permissions:\` block of the reusable Lint workflow. Workflow-level
permissions in a reusable are a **hard ceiling** — exceeding the
caller's grant causes \`startup_failure\` (the workflow won't start at
all). The graceful-fallback in \"Resolve prettier targets\" never gets a
chance to run.

## Confirmed fleet damage (since #29 merged at 23:54 UTC)

- \`whois-api-llc/wxa_vpn\` — 2 startup_failures, 2 PRs blocked
- \`whois-api-llc/wxa-jake-ai\` — 2 startup_failures, 1 PR blocked
- Likely affects every fleet repo whose Lint caller grants only
\`contents: read\`

PRs sit indefinitely at \`lint / actionlint — Expected — Waiting for
status to be reported\` because the required check never produces a
status.

## Fix

Move \`pull-requests: read\` from workflow-level to **job-level** on
just the \`prettier\` job. Job-level perms in a reusable are silently
capped to the caller's grant — the gh api call inside the job 403s, and
the existing graceful-fallback path correctly degrades to full-glob mode
(and emits \`::warning::\`).

This is exactly the behavior #29's comment claimed but couldn't deliver
from workflow-level.

## After-merge cleanup

Previously-stuck PRs need a push (empty commit) or close/reopen to clear
the cached \`startup_failure\` status. The reusable itself starts
working immediately on the next event.

**Auto-merge rationale:** in the manual-merge category — this is
reusable CI infra. But it's a fleet hot-fix; please merge as soon as
it's green.

## Test plan

- [ ] CI green on this PR
- [ ] After merge, push a no-op commit to wxa_vpn#241 — confirm Lint
workflow starts (no startup_failure)
- [ ] Confirm prettier emits the \`::warning::\` graceful-degradation
message in repos without \`pull-requests: read\`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Jonathan Zhang <[email protected]>
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
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.

1 participant