feat: Closed PR Review Auto-Improver (automated feedback loop)

[nick-inkeep]

Summary

Introduces an automated system that learns from human reviewers to continuously improve our AI code review agents.

The Problem

When human reviewers catch issues that our pr-review-* agents miss, that knowledge currently dies with the PR. We manually noticed patterns like "Type Definition Discipline" (PR #1737) but there's no systematic way to:

1. Detect when humans catch something bots missed
2. Determine if it's a generalizable pattern (vs repo-specific)
3. Propose improvements to the reviewer agents

The Solution

A GitHub Actions workflow that triggers after every PR merge (or manually for testing):

PR Merged → Extract Human Comments → Analyze Gaps → Apply Generalizability Test → Create Draft PR (if HIGH)

Key innovation: Git time-travel — The agent reconstructs what the human reviewer saw at comment time (not the final merged state), since issues are often fixed before merge.

Architecture

Plugin isolation: The agent and skills live in a private repo (inkeep/internal-cc-plugins) and are loaded via --plugin-dir at CI runtime. This keeps CI/CD-only capabilities out of the main workspace.

Cross-repo auth: Uses a GitHub App (inkeep-internal-ci) with 8-hour tokens instead of PATs for better security and zero manual rotation.

inkeep/agents                          inkeep/internal-cc-plugins (private)
├── .github/workflows/                 ├── closed-pr-review-auto-improver/
│   └── closed-pr-review-auto-         │   ├── agents/
│       improver.yml ─────────────────────►│   │   └── closed-pr-review-auto-improver.md
│       (clones plugin repo,           │   └── skills/
│        uses --plugin-dir)            │       ├── find-similar-patterns/
                                       │       ├── pr-review-subagents-available/
                                       │       └── pr-review-subagents-guidelines/

How It Works

1. Trigger: pull_request_target: [closed] + merged == true, or workflow_dispatch for manual testing
2. Auth: GitHub App generates 8-hour token to clone internal-cc-plugins
3. Extract: GraphQL query fetches all comment types (discussion, inline, reviews)
4. Filter: Removes bot comments and trivial human comments ("LGTM", "thanks")
5. Analyze: Agent investigates each promising comment:
   • Uses git rev-list --before + git show to see code at comment time
   • Progressive context gathering (diffHunk → full file → PR diff → other files)
   • Explicit stop conditions: EXIT A (not generalizable) or EXIT B (pattern found)
6. Test: 4-criteria generalizability test (cross-codebase, universal principle, expressible, industry-recognized)
7. Output: If HIGH generalizability → creates draft PR with improvements to pr-review-*.md

Generalizability Test (all must pass)

Criterion	Question
Cross-codebase	Would this pattern appear in other TS/React/Node codebases?
Universal principle	Is it DRY, SOLID, separation of concerns, etc.?
Expressible	Can it be a checklist item, detection pattern, or failure mode?
Industry recognition	Would senior engineers elsewhere recognize this?

Conservative by default: Better to miss a good pattern than pollute reviewers with repo-specific noise.

Manual Testing

After merge, you can test against historical PRs:

1. Go to Actions → Closed PR Review Auto-Improver
2. Click Run workflow
3. Enter a PR number (e.g., 1737 for the type/schema feedback)
4. Watch it analyze the historical human comments

Files Changed

File	Purpose
.github/workflows/closed-pr-review-auto-improver.yml	Workflow: trigger, GitHub App auth, comment extraction, plugin loading

Prerequisites (already set up)

• GitHub App inkeep-internal-ci created with Contents:read permission
• App installed on internal-cc-plugins repo
• Secrets added: INTERNAL_CI_APP_ID, INTERNAL_CI_APP_PRIVATE_KEY
• Plugin repo inkeep/internal-cc-plugins with agent + skills

Design Decisions

Decision	Choice	Rationale
Auth method	GitHub App	8-hour tokens, no rotation, survives user departure
Plugin location	Private repo	Keeps CI-only agents out of main workspace
Output action	Draft PRs	Human review of proposed improvements
Generalizability threshold	HIGH only	Conservative; MEDIUM noted but no PR
Model	Opus	Pattern recognition requires strong reasoning

---

Test Plan

• Verify workflow triggers on merged PRs (not closed-without-merge)
• Test workflow_dispatch with historical PR number (e.g., 1737)
• Verify GitHub App token generation and plugin repo clone
• Test comment extraction against a known PR with human comments
• Verify bot filtering excludes claude-code, dependabot, etc.
• Test git time-travel reconstructs correct code state
• Verify agent correctly classifies repo-specific vs generalizable patterns
• Confirm draft PR creation with proper formatting

---

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agents-api	Ready	Preview, Comment	Feb 6, 2026 5:52am
agents-docs	Ready	Preview, Comment	Feb 6, 2026 5:52am
agents-manage-ui	Ready	Preview, Comment	Feb 6, 2026 5:52am

[changeset-bot]

⚠️ No Changeset found

Latest commit: 454eff2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Claude finished @nick-inkeep's task in 4m 9s —— View job

---

PR Review Summary

5 Key Findings | Risk: Medium

🟠⚠️ Major (3) 🟠⚠️

🟠 1) closed-pr-review-auto-improver.yml:16 Missing merge validation for workflow_dispatch

Issue: The workflow_dispatch trigger allows manual execution with an arbitrary PR number, but there's no validation that the specified PR was actually merged. The condition github.event.pull_request.merged == true only applies to the pull_request_target trigger—for workflow_dispatch, github.event.pull_request is undefined, so it always passes via the || github.event_name == 'workflow_dispatch' clause.

Why: This could allow analysis of unmerged PRs (still in review, abandoned, or even malicious forks). While the impact is limited to creating draft PRs with potentially confusing suggestions, it breaks the core invariant that the system only learns from merged (i.e., human-approved) code.

Fix: Add validation that the PR is merged when using workflow_dispatch:

- name: Validate PR is merged (for workflow_dispatch)
  if: github.event_name == 'workflow_dispatch'
  env:
    GH_TOKEN: ${{ github.token }}
  run: |
    MERGED=$(gh pr view ${{ inputs.pr_number }} --json merged --jq '.merged')
    if [ "$MERGED" != "true" ]; then
      echo "::error::PR #${{ inputs.pr_number }} is not merged. Only merged PRs can be analyzed."
      exit 1
    fi

---

🟠 2) closed-pr-review-auto-improver.yml:60-62,152-158 HEREDOC delimiter collision vulnerability

Issue: The workflow uses EOF as the HEREDOC delimiter for PR body and comments. If a PR description or comment contains EOF on a line by itself, the shell will prematurely terminate the heredoc, causing truncated output or script errors.

Why: PR descriptions and comments are user-controlled content—especially in an open-source repo context. A malformed PR body could silently corrupt the workflow's ability to pass context to the agent.

Fix: Use a unique, unlikely-to-collide delimiter:

# Instead of:
echo "body<<EOF" >> $GITHUB_OUTPUT
echo "$PR_DATA" | jq -r '.body' >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT

# Use:
DELIM="__GITHUB_OUTPUT_DELIM_$(date +%s%N)__"
echo "body<<$DELIM" >> $GITHUB_OUTPUT
echo "$PR_DATA" | jq -r '.body' >> $GITHUB_OUTPUT
echo "$DELIM" >> $GITHUB_OUTPUT

Or use base64 encoding for the content.

---

🟠 3) closed-pr-review-auto-improver.yml:191 Action version inconsistency with claude-code-review.yml

Issue: This workflow uses anthropics/claude-code-action@v1 while claude-code-review.yml pins to a specific SHA (@01e756b34ef7a1447e9508f674143b07d20c2631) due to an AJV validation bug tracked at issue #892.

Why: If the @v1 tag points to a version affected by the AJV bug, this workflow could fail unpredictably. Using inconsistent versions across workflows creates maintenance burden and potential for unexpected behavior.

Fix: Either:

1. Pin to the same SHA as claude-code-review.yml:

   uses: anthropics/claude-code-action@01e756b34ef7a1447e9508f674143b07d20c2631

2. Or add a comment explaining why @v1 is acceptable here if the bug doesn't affect this use case.

---

🟡 Minor (2)

🟡 1) closed-pr-review-auto-improver.yml Missing concurrency control

Issue: No concurrency group is defined. If multiple PRs merge in quick succession or someone triggers workflow_dispatch while another run is in progress, parallel runs could create conflicting branches/PRs.

Why: The workflow creates branches with a predictable naming pattern (closed-pr-review-auto-improver/pr-{number}-learnings). Concurrent runs for the same PR could race.

Fix: Add concurrency control:

concurrency:
  group: closed-pr-review-auto-improver-${{ inputs.pr_number || github.event.pull_request.number }}
  cancel-in-progress: false  # Don't cancel, queue instead

---

🟡 2) closed-pr-review-auto-improver.yml:48-163 Missing shell error handling

Issue: Shell scripts don't use set -e or set -o pipefail. If gh pr view, gh api graphql, or jq commands fail, the workflow continues with empty/corrupted data rather than failing fast.

Why: Silent failures could lead to the agent receiving incomplete context, potentially producing low-quality or confusing outputs.

Fix: Add error handling to shell blocks:

run: |
  set -eo pipefail
  PR_NUMBER=${{ inputs.pr_number || github.event.pull_request.number }}
  ...

---

Other Findings (3)

Potentially valid

(these are minor and not fully confident)

Location	Issue	Reason Excluded
yml:172-186	Diff truncation at 200KB vs 500KB in other workflow	Likely intentional given different use case; lower is more conservative
yml:18-21	Missing id-token: write permission	May not be needed since this workflow doesn't use OIDC

Discarded as invalid or not applicable

Location	Issue	Reason Excluded
pull_request_target security	Potential for secret exfiltration	Mitigated by merged == true check; only runs on approved code
Agent has Edit/Write tools	Broad permissions	Acceptable since it creates draft PRs requiring human approval

---

💡 APPROVE WITH SUGGESTIONS

Summary: This is a well-designed feedback loop system with sound architecture. The security model (merged PRs only + draft PRs for outputs) is appropriate. The three major findings are all straightforward fixes: (1) validate merge status for workflow_dispatch, (2) use unique HEREDOC delimiters, and (3) align action versions. None are blocking, but addressing them before merge would improve robustness. Excited to see the pr-review agents learning from human feedback! 🎓

---

Completed Tasks

• Analyzed PR context and understood the changes
• Explored codebase for relevant patterns and architecture
• Performed security review (pull_request_target usage, tokens)
• Performed standards and quality review (shell scripting, error handling)
• Checked consistency with existing workflows
• Aggregated and filtered findings
• Posted final review summary

[claude]

Claude finished @nick-inkeep's task in 3m 53s —— View job

---

PR Review Summary

0 Key Findings | Risk: Low

---

✅ Previous Issues Addressed

All 5 issues from the previous review have been properly addressed:

Finding	Resolution
Missing merge validation for workflow_dispatch	✅ Added validation step (lines 34-45) with set -eo pipefail
HEREDOC delimiter collision vulnerability	✅ Using unique delimiters: __BODY_DELIM_7f3a9b2c__, __HUMAN_COMMENTS_DELIM_8e4b1d3f__, etc.
Action version inconsistency	✅ Pinned to SHA @01e756b34ef7a1447e9508f674143b07d20c2631 with tracking comment (lines 211-213)
Missing concurrency control	✅ Added concurrency group (lines 13-15) with cancel-in-progress: false
Missing shell error handling	✅ set -eo pipefail added to all shell scripts (lines 39, 66, 87, 191)

---

🎯 Quality Observations

Security Model - Well-designed:

• pull_request_target with merged == true guard ensures only approved code is analyzed
• GitHub App tokens (8-hour expiry, repo-scoped) are preferred over PATs
• workflow_dispatch validates PR is merged before proceeding
• Tool allowlist restricts agent capabilities appropriately
• Output is draft PRs requiring human approval

Improvements Over Existing Patterns - This workflow actually demonstrates better practices than the existing claude-code-review.yml:

• Unique HEREDOC delimiters (vs plain EOF in claude-code-review.yml)
• Consistent set -eo pipefail error handling
• Explicit tracking comment for SHA pin

---

🟡 Optional Enhancement

One optional improvement to consider (not blocking):

Debug artifact upload - The claude-code-review.yml uploads debug artifacts for troubleshooting (lines 215-224). Adding similar artifact upload to this workflow could help with debugging failed runs:

- name: Upload Debug Artifacts
  if: always()
  uses: actions/upload-artifact@v4
  with:
    name: auto-improver-debug-pr-${{ steps.pr-meta.outputs.pr_number }}-${{ github.run_id }}
    path: ${{ steps.analyze.outputs.execution_file }}
    retention-days: 7
    if-no-files-found: ignore

This is purely optional and doesn't affect functionality.

---

Other Findings (2)

Potentially valid but out of scope

Location	Observation	Reason Not Raised
claude-code-review.yml	Uses plain EOF delimiters	Separate PR opportunity; this PR is an improvement
claude-code-review.yml	Missing set -eo pipefail	Separate PR opportunity; this PR is an improvement

Discarded as not applicable

Location	Consideration	Reason Excluded
Missing id-token: write	claude-code-review.yml has it	This workflow doesn't use OIDC - intentionally omitted
200KB diff limit vs 500KB	Different than other workflow	Intentional - more conservative for this use case

---

✅ APPROVE

Summary: All 5 issues from the previous review have been properly addressed. The workflow now has robust error handling, secure patterns, and unique HEREDOC delimiters. In fact, this workflow demonstrates better practices than some existing workflows in the repo. The architecture is sound - plugin isolation via private repo, GitHub App authentication, and draft PR output requiring human approval. Ready to merge! 🎉

---

Completed Tasks

• Analyzed PR context and understood the changes
• Explored codebase for relevant patterns (claude-code-review.yml comparison)
• Conducted security review (pull_request_target, tokens, permissions)
• Conducted standards review (shell scripting, YAML, error handling)
• Checked consistency with existing workflows
• Aggregated and filtered findings
• Posted final review summary

[claude]

Claude finished @nick-inkeep's task in 3m 58s —— View job

---

PR Review Summary

0 Key Findings | Risk: Low

---

✅ All Previous Issues Remain Addressed

Verified all 5 issues from the initial review are still properly implemented in the current commit (454eff2):

Finding	Status	Implementation
Missing merge validation for workflow_dispatch	✅	Lines 34-45: Explicit validation step with set -eo pipefail
HEREDOC delimiter collision vulnerability	✅	Unique delimiters throughout: __BODY_DELIM_7f3a9b2c__, __HUMAN_COMMENTS_DELIM_8e4b1d3f__, __BOT_COMMENTS_DELIM_9f5c2e4a__, __DIFF_DELIM_a6d3f8c1__
Action version inconsistency	✅	Pinned to SHA @01e756b34ef7a1447e9508f674143b07d20c2631 with tracking comment (lines 211-213)
Missing concurrency control	✅	Lines 13-15: cancel-in-progress: false for sequential processing
Missing shell error handling	✅	set -eo pipefail in all 4 shell blocks (lines 39, 66, 87, 191)

---

🎯 Quality Assessment

Security Model — Well-designed:

• pull_request_target with merged == true guard prevents untrusted code execution
• workflow_dispatch explicitly validates PR is merged before proceeding
• GitHub App tokens (8-hour expiry, repo-scoped) preferred over PATs
• Tool allowlist appropriately restricts agent capabilities
• Output is draft PRs requiring human approval

Consistency with Existing Patterns — This workflow demonstrates improved practices:

• Unique HEREDOC delimiters (vs plain EOF in claude-code-review.yml)
• Consistent set -eo pipefail error handling in all shell blocks
• Explicit tracking comment for SHA pin
• Debug artifact upload included

Architecture — Sound design:

• Plugin isolation via private repo keeps CI-only agents out of main workspace
• Concurrency control with cancel-in-progress: false appropriate for long-running analysis
• Conservative generalizability threshold (HIGH only) prevents noise

---

🔍 Observations (Not Blocking)

Location	Observation	Assessment
200KB diff limit	Lower than 500KB in claude-code-review.yml	Intentional and appropriate for this use case
Missing id-token: write	Different from claude-code-review.yml	Correct — this workflow doesn't use OIDC
GraphQL first: 100 limits	Could miss comments on very high-volume PRs	Acceptable; consistent with existing patterns

---

Other Findings (0)

Discarded as not applicable

Location	Consideration	Reason Excluded
pull_request_target security	Potential for secret exfiltration	Mitigated by merged == true guard; only runs on approved code
Agent has Edit/Write tools	Broad permissions	Acceptable since output is draft PRs requiring human approval
claude-code-review.yml uses plain EOF	Inconsistency	Out of scope; the new workflow is the improvement

---

✅ APPROVE

Summary: This PR is ready to merge. All previously identified issues have been properly addressed and verified. The workflow implements robust security patterns (merged-PR-only trigger, GitHub App auth, tool allowlisting), comprehensive error handling, and proper concurrency control. The architecture—plugin isolation via private repo with draft PR outputs—is well-designed for an automated feedback loop system. This workflow actually demonstrates better practices than some existing workflows in the repo! 🎉

---

Completed Tasks

• Analyzed PR context and understood the changes
• Explored codebase for relevant patterns (claude-code-review.yml comparison)
• Conducted security review (pull_request_target, tokens, permissions)
• Conducted standards review (shell scripting, YAML, error handling)
• Checked consistency with existing workflows
• Aggregated and filtered findings
• Posted final review summary