Skip to content

Conversation

@Cali93
Copy link
Contributor

@Cali93 Cali93 commented Jan 19, 2026

Summary

  • Update the auto-review prompt to use a parallel multi‑agent flow (bugs, security, patterns) with consolidation and deduplication rules.
  • Extend findings parsing to infer agent source from ID prefixes and support multi‑agent output fixtures.
  • Add tests and fixtures for multi‑agent parsing, and allow Task tool usage in the Claude run.

Changes

  • Revamp the auto-review prompt to spawn three specialized sub‑agents in parallel, then consolidate issues into a single unified <details> list with severity ordering.
  • Enhance parseClaudeComment to map bug-, sec-, pat- ID prefixes to agent names.
  • Add multi‑agent output fixture and tests validating agent extraction and prefix handling.
  • Expand allowed tools to include Task for parallel sub‑agent execution.

Notes

  • Domain‑specific checks remain owned by the patterns agent per updated prompt.

Test run example here https://github.com/WalletConnect/gha-playground/pull/3#issuecomment-3772901080

@claude
Copy link

claude bot commented Jan 19, 2026

Claude finished @Cali93's task in 1m 53s —— View job


Code Review

  • Read changed files
  • Review action.yml multi-agent orchestration
  • Review agent specification files
  • Review parsing logic changes
  • Review test coverage
  • Check for security issues
  • Provide final feedback

Found 3 issue(s)

Issue 1: Agent specification files are unused

ID: pat-action-unused-agent-files-a1b2
File: claude/auto-review/action.yml:111
Severity: HIGH
Category: architecture
Context: The PR adds three agent specification files (review-bugs.md, review-patterns.md, review-security.md) with YAML frontmatter defining tools and model preferences, but action.yml never references or uses these files. The action.yml instead embeds all agent instructions directly in the prompt text at lines 111-143. This means the frontmatter metadata (tools, model, description) serves no purpose.
Recommendation: Either:

  1. Remove the frontmatter from agent files and use them as pure documentation/reference, OR
  2. Modify action.yml to read and use these agent specification files instead of embedding instructions inline

The current implementation creates maintenance burden - agent instructions exist in two places and can drift out of sync.

Issue 2: Missing instruction to read agent specification files

ID: pat-action-missing-file-refs-c3d4
File: claude/auto-review/action.yml:135
Severity: MEDIUM
Category: code_quality
Context: The prompt instructs Claude to spawn agents with embedded instructions but doesn't reference the agent specification files in claude/auto-review/agents/. If these files are meant to be used by spawned agents (which have Read access), the main prompt should instruct Claude to read and use them.
Recommendation: Add instruction to read agent specification files:

**For EACH agent prompt, include:**
- Read the agent specification from claude/auto-review/agents/review-{type}.md
- This preamble: \"You are a code reviewer...\"

Or document that the .md files are reference material only, not runtime specifications.

Issue 3: Test fixture doesn't match actual multi-agent output format

ID: bug-multi-agent-fixture-mismatch-8b91
File: claude/auto-review/scripts/tests/fixtures/claude-comments/multi-agent-output.md:1
Severity: MEDIUM
Category: bug
Context: The multi-agent-output.md fixture shows a consolidated output format (lines 1-4: "I've conducted a comprehensive review using specialized agents for bugs, security, and patterns. Here are the consolidated findings:"), but the action.yml prompt at line 162 instructs to "Produce a SINGLE consolidated summary containing all unique issues in the standard collapsed <details> format." The fixture should match what the actual prompt will produce. The test validates agent prefix extraction (lines 226-254) which is good, but doesn't validate the consolidation flow described in action.yml lines 150-162.
Recommendation: Update test fixture to match the actual expected output format from action.yml, or add a test case that validates the consolidation instructions are correctly followed.


Summary

Multi-agent architecture implementation with agent-specific ID prefixes and parsing logic. Main concerns:

  1. Unused files: Agent specification .md files with frontmatter aren't referenced or used by action.yml
  2. Unclear architecture: Is action.yml meant to embed all instructions, or read from agent files?
  3. Test coverage gap: Fixture doesn't fully validate the consolidation flow described in the prompt

Recommend clarifying whether agent .md files are runtime specs or documentation, and aligning implementation accordingly.

@Cali93 Cali93 marked this pull request as draft January 19, 2026 14:09
@bkrem
Copy link
Member

bkrem commented Jan 20, 2026

FYI - think you will need to rebase due to this base prompt compression I just got over the line, apologies 🙏 #56

Cali93 and others added 2 commits January 20, 2026 09:54
Resolved conflicts in action.yml by:
- Keeping multi-agent review approach
- Adopting new condensed Context format (Pattern/Risk/Impact/Trigger)
- Updated agent spec files to use new format
- Updated test fixtures to match new format

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@Cali93 Cali93 marked this pull request as ready for review January 20, 2026 13:39
---
## AUTOMATED CHECKS
Copy link
Member

Choose a reason for hiding this comment

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

I see this was moved into review-patterns, but let's ensure these checks are preserved in the main prompt if agents only trigger on heuristics like PR size? 🙏 We need them to happen for sure on every PR.


You are a code reviewer. Provide actionable feedback on code changes.

**Diffs alone are not enough.** Read the full file(s) being modified to understand context. Code that looks wrong in isolation may be correct given surrounding logic.
Copy link
Member

Choose a reason for hiding this comment

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

2. PR number, repository, list of changed files
**Agents:**
1. **Bug Agent** - Spec: ${{ github.action_path }}/agents/review-bugs.md - ID prefix: bug-
Copy link
Member

Choose a reason for hiding this comment

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

It already has lots of explicit guidance on how to generate the IDs for inline commenting deduplication: https://github.com/WalletConnect/actions/pull/64/changes#diff-742c4c628098679724a584600295de6d9bba6d01b7d7b5c0cb60f03ce427288fR142

I don't think we need the ID prefix: specifications here and it doesn't seem to follow them anyways based on what I'm seeing in https://github.com/WalletConnect/gha-playground/pull/12#issuecomment-3774813787

@@ -0,0 +1,108 @@
# Security Review Agent
Copy link
Member

@bkrem bkrem Jan 22, 2026

Choose a reason for hiding this comment

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

Maybe we retire the usage of https://github.com/anthropics/claude-code-security-review/ in favour of this long term and use Opus for this agent, somewhat duplicative if we have both in some repos.

bkrem added 3 commits January 23, 2026 14:37
Each agent's spec file already defines its own ID format (bug-, sec-, pat-).
Removes redundant prefix specs from agent list that weren't being followed.

Addresses review feedback from @bkrem on PR #64.
- Use consistent ID template placeholders ({filename}-{2-4-key-terms}-{hash})
- Change severity/category separator from | to / to match main prompt
- Rename "ID Format" to "ID Generation" for consistency
- Update recommendation wording to match main action.yml
- Add CRITICAL severity to patterns agent for domain-specific checks
- Remove redundant details block instruction (handled by orchestrator)
Implement heuristic to selectively spawn review agents based on PR
characteristics, avoiding unnecessary agent invocation for simple PRs.

Key changes:
- Add determine-agents.js with file categorization and keyword analysis
- Skip review entirely for docs-only, rename-only, and empty PRs
- Spawn security agent for workflow/auth/SQL/infra/deps files or keywords
- Spawn patterns agent for large files (>300 lines) or >5 code files
- Always spawn all agents for large PRs (>500 lines or >15 files)
- Add force_all_agents input and full-review/skip-review label overrides
- Add comprehensive test suite (47 tests)

Heuristic behavior:
- Docs/rename/empty PR: Skip review entirely
- Lockfile-only: Security agent only
- Test-only: Bug agent only
- Large PR: All 3 agents
- Small code PR: Bug agent only (+ security/patterns if triggered)
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