feat: add reusable PR risk classifier#4
Merged
Merged
Conversation
Workflow that reads .github/risk-paths.yml from the caller repo, classifies the PR's changed files, and exposes the highest-priority risk class as both a PR label and a job output for downstream auto-merge gating. Priority (high → low): blocked > sensitive > standard > safe_test > safe_deps > safe_config > trivial `standard` is the implicit fallback for unmatched files — fail-safe. Includes a Node script (classify.mjs) using yaml@2 + minimatch@10 for deterministic glob matching. Smoke-tested locally with 15 cases covering single-file, multi-file, and priority-resolution behavior. Workflow fetches its own classify.mjs via the GitHub API at runtime so caller repos don't need to vendor the script. Caller only owns .github/risk-paths.yml (per-repo rules) plus the caller workflow. Pairs with the auto-merge implementation plan §4 in: ~/.claude/plans/wxa-jake-ai-2026-04-30-auto-merge-impl.md Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
| rules = parse(readFileSync(RULES_PATH, 'utf8')); | ||
| } catch (e) { | ||
| fail(`failed to read ${RULES_PATH}: ${e.message}`); | ||
| } |
There was a problem hiding this comment.
Bug: rules is not guarded against null
yaml.parse() returns null for an empty file or one that contains only comments. The try/catch above catches I/O and parse errors, but a valid-but-empty YAML file silently produces null. The first call to rules[cls] in classify() (line 66) then throws an uncaught TypeError: Cannot read properties of null (reading 'blocked'), which exits with code 1 and a stack trace instead of the clean fail() message.
The "Verify risk-paths.yml exists" workflow step only checks for file presence, so this path is reachable.
Suggested change
| } | |
| } | |
| if (!rules || typeof rules !== 'object' || Array.isArray(rules)) { | |
| fail(`${RULES_PATH} must be a non-empty YAML mapping (got ${rules === null ? 'null' : typeof rules})`); | |
| } |
|
Flagged 1 issue inline — null-rules dereference in classify.mjs when risk-paths.yml is empty or comment-only. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New reusable workflow `pr-classify.yml` + classifier script `classify.mjs`. Caller repos point at `topcoder1/ci-workflows/.github/workflows/pr-classify.yml@main`, ship a `.github/risk-paths.yml` with their own path rules, and consume the resulting `outputs.risk_class`.
Architecture
Smoke test (local)
15/15 tests pass:
```
Single-file: trivial, standard, sensitive, blocked, safe_test, safe_deps,
multi-class sensitive, multi-class blocked, etc.
Multi-file: highest-priority resolution across {trivial+blocked},
{trivial+safe_test}, {safe_test+sensitive}, {standard+trivial}
```
Caller usage
```yaml
jobs:
classify:
uses: topcoder1/ci-workflows/.github/workflows/pr-classify.yml@main
```
After this lands, downstream consumers reference the output:
```yaml
some-gated-job:
needs: classify
if: needs.classify.outputs.risk_class == 'sensitive'
```
Pairs with
🤖 Generated with Claude Code