feat(review-pr): add exclude-paths input to skip generated files from review

[docker-agent]

Summary

Fixes a timeout issue in the PR review pipeline caused by generated protobuf files dominating the review queue.

Root cause: docker/agentic-platform PR #1965 (4338-line diff) timed out because session_service.pb.go was assigned risk score 6 (matched session in the security-path pattern + >100 added lines), landing in Chunk 1 with highest priority. The agent spent ~77k lines processing it with zero findings, leaving no capacity for real code.

Fix A — exclude-paths input

Adds optional exclude-paths input (newline-separated path prefixes) to both the reusable workflow and the composite action.

New step: 'Filter excluded paths from diff' runs after gh pr diff pre-fetch and before 'Split diff into chunks':

• Uses pure bash+awk (no Python) to process diff section-by-section
• Each section is buffered until the +++ b/<path> line is seen
• If the path starts with any excluded prefix → the entire section is dropped
• Logs each excluded file with ⏭️ Excluded from review: <path>
• Logs summary: ✅ Filtered diff: N files excluded, M files remaining
• Step only runs when pr.diff exists AND exclude-paths is non-empty

Fix B — Risk scorer ignores excluded paths + generated-file markers

Updated 'Score file risk' bash step:

• Excluded paths check: files whose path starts with any exclude-paths prefix → score 0 (redundant safety net if the diff still has those sections for some reason)
• Generated-file marker check: for files NOT in exclude-paths, inspects first 5 added lines for Code generated or DO NOT EDIT → score 0 (catches .pb.go and similar files not covered by a path prefix)

Files changed

• .github/workflows/review-pr.yml: added exclude-paths to workflow_call inputs and threaded through to the review-pr action call
• review-pr/action.yml: added exclude-paths input, new filter step, updated scoring step with EXCLUDE_PATHS env var and two new zero-score conditions

Consumer configuration

Downstream repos (e.g. docker/agentic-platform) can now configure:

uses: docker/cagent-action/.github/workflows/review-pr.yml@v...
with:
  exclude-paths: |
    backend/gen/
    frontend/src/gen/

Refs: analysis artifact 86ed46d9-3606-4072-b570-62204d354c86

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

Two medium-severity bugs in the awk diff-filter bypass excluded paths for deleted files and pure renames. Input threading and main modification-file filtering are correct.

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

One medium-severity issue: the Score file risk step can silently succeed when score-risk.js fails (two-command pattern without set -euo pipefail). Two low-severity issues: the extractFilePath fix lacks a regression test that demonstrates the behavioral difference, and score-risk/ is missing from the AGENTS.md layout.

The TypeScript rewrites are otherwise correct and well-tested: all 207 unit tests pass, all four diff section types (modification, deletion, rename, new file) are handled correctly, scoring rules faithfully replicate the original bash, and applyFilter resolves the prior-round empty-diff guard issue by deleting the file when all sections are excluded.

[docker-agent]

[MEDIUM] score-risk.js failure is silently masked by the trailing echo command

The Score file risk step has two commands with no set -euo pipefail:

node "$ACTION_PATH/dist/score-risk.js" pr.diff "$EXCLUDE_PATHS"
echo "✅ File risk scores: $(jq -c . /tmp/file_risk_scores.json)"

If score-risk.js exits non-zero (binary not found, parse error, write error), bash continues to the echo line. The echo exits 0 even if the embedded jq fails, so the step reports success while /tmp/file_risk_scores.json is absent or stale. The pr-review.yaml agent reads this file directly and will silently operate on wrong/missing risk data, potentially misrouting the review.

Compare: filter-diff is a single-command step — its node failure propagates correctly.

Fix: add set -euo pipefail before the node call, or restructure as a single node ... && echo ... chain:

set -euo pipefail
node "$ACTION_PATH/dist/score-risk.js" pr.diff "$EXCLUDE_PATHS"
echo "✅ File risk scores: $(jq -c . /tmp/file_risk_scores.json)"

[docker-agent]

Fixed in 4bc6182. The Score file risk step now begins with set -euo pipefail:

run: |
  set -euo pipefail
  node "$ACTION_PATH/dist/score-risk.js" pr.diff "$EXCLUDE_PATHS"
  echo "✅ File risk scores: $(jq -c . /tmp/file_risk_scores.json)"

If score-risk.js exits non-zero the step now fails immediately rather than continuing to the echo and masking the error.

[docker-agent]

[LOW] The extractFilePath fix has no test that exercises the behavioral difference

The PR description calls out fixing a greedy-regex bug in path extraction:

• Old bash: sub(/.*b\//, "", file) — greedy, strips up to last b/, mangling paths like .github/workflows/ci.yml into ci.yml
• New TS: indexOf(' b/') — finds the first b/ separator, returning the correct full path

The existing rule-6 test covers .github/workflows/ci.yml and confirms score = 0, but the score is identically 0 under both the old and new extraction:

• Old: extracts ci.yml → matches \.ya?ml$ → reset to 0
• New: extracts .github/workflows/ci.yml → matches \.ya?ml$ → reset to 0

No test demonstrates a case where the fix actually changes the result. A behavioral-difference test would be:

// Path where old bash extracted a segment matching a security keyword,
// but the full correct path does NOT (or vice versa).
// e.g., "diff --git a/.github/workflows/token-refresh.yml b/.github/workflows/token-refresh.yml"
// Old bash extracts: "token-refresh.yml" — no security keyword, no rule 3
// New TS extracts:   ".github/workflows/token-refresh.yml" — no security keyword either
// (both give 0 due to .yml reset — but it's harder to construct a case that differs)
// The cleaner test: verify extractFilePath directly, or test a .github/workflows/auth.sh
// (no .yml reset, but old bash would wrongly extract "auth.sh" vs correct ".github/workflows/auth.sh")

This is not a correctness bug in the shipped code — the fix is right — but a gap in the test's ability to serve as a regression guard.

[docker-agent]

Fixed in 4bc6182. Added a dedicated describe block in src/score-risk/__tests__/score-risk.test.ts:

describe('scoreFiles — extractFilePath: indexOf regression for paths containing b/', () => {
  it('path with security keyword before a b/ directory component scores 2, not 0', () => {
    // Old greedy regex: replace(/.*b\//, '') on
    //   "diff --git a/src/auth/b/helper.ts b/src/auth/b/helper.ts"
    // matches everything up to the last 'b/' giving 'helper.ts'.
    // 'helper.ts' has no security keyword → score 0.
    //
    // New indexOf(' b/'): strips "diff --git a/" prefix and splits at the
    // first ' b/' separator giving 'src/auth/b/helper.ts'.
    // 'src/auth/b/helper.ts' matches SECURITY_PATH_RE ('auth') → score 2.
    const diff = makeDiff('src/auth/b/helper.ts', ['+changed']);
    const scores = scoreFiles(diff, []);
    expect(scores['src/auth/b/helper.ts']).toBe(2);
  });
});

This demonstrates a concrete case where old and new extraction produce different scores (0 vs 2). 208/208 tests pass.

[docker-agent]

[LOW] score-risk/ module missing from AGENTS.md repo layout

AGENTS.md was updated to add the filter-diff/ section (lines 42–45), but src/score-risk/ — the other new module introduced by this PR — is absent from the layout. The layout section is the canonical map of what lives in src/ for future contributors and agents reading this file.

The missing entry should appear after filter-diff/ (alphabetically between filter-diff/ and get-pr-meta/):

│   ├── score-risk/                  # Per-file risk scoring for the PR review pipeline.
│   │   ├── index.ts                 # CLI entry → bundled to dist/score-risk.js
│   │   ├── score-risk.ts            # Core scoreFiles() pure function.
│   │   └── __tests__/

[docker-agent]

Fixed in 06b6243 — added score-risk/ between filter-diff/ and get-pr-meta/ in the repo layout section of AGENTS.md.

[docker-agent]

Already fixed in 06b6243 (the commit immediately before this round's changes). score-risk/ is now present between filter-diff/ and get-pr-meta/ in the repo layout section of AGENTS.md.

[docker-agent]

Assessment: 🟢 APPROVE

All round-2 findings resolved correctly in commit 4bc6182:

• set -euo pipefail added at line 325 of review-pr/action.yml — score-risk.js failure now propagates and fails the step correctly.
• Regression test for extractFilePath fix added: src/auth/b/helper.ts path proves old greedy regex extracts helper.ts (score 0), new indexOf(' b/') extracts src/auth/b/helper.ts (score 2 via auth keyword). The behavioral difference is now demonstrable.
• score-risk/ present in AGENTS.md layout at lines 46–49.

208/208 unit tests pass (verified locally on the PR branch).