feat(agents): add review agent and pr-review skill

[ben-alkov]

Summary

• Add agents/review.md — Claude Code agent definition for PR review
  across six dimensions (correctness, intent alignment, platform/content
  security, injection defense, style/conventions)
• Add skills/pr-review/SKILL.md — step-by-step review procedure:
  fetch PR context, read full source files, evaluate each dimension,
  compile findings by severity, post via gh pr review
• Add .claude/agents -> ../agents symlink (follows existing
  .claude/skills pattern)

Design grounded in zero-trust: the agent does not trust the PR author,
other agents, or claims in the PR description. It evaluates the code
directly and pins reviews to the evaluated SHA.

Test plan

• .claude/agents symlink resolves to agents/
• @review agent loads after /agents reload
• pr-review skill appears in skill list
• Invoke @review against a test PR — verify it reads the diff,
  evaluates six dimensions, and posts a structured review comment
  with findings grouped by severity and the head SHA pinned

Ran review agent locally against nonflux/integration-service/pull/23; review is at https://github.com/nonflux/integration-service/pull/23#pullrequestreview-4057269059

Toward #128

[ralphbean]

Review: #171

Head SHA: d6105cc
Timestamp: 2026-04-05T20:00:00Z
Outcome: approve

Summary

Clean, well-scoped PR that delivers exactly what was agreed in the issue #128 discussion — a single review agent definition, a supporting skill, and the symlink to expose it. The agent definition enforces appropriate constraints (no code push, no merge, read-only tools plus Bash for gh pr review), follows zero-trust principles aligned with the repo's threat model, and the symlink follows the established .claude/skills -> ../skills pattern. No critical, high, or medium findings. No hidden Unicode or injection patterns detected in raw bytes.

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

• [info] [style] agents/review.md:84 — The review comment structure mentions "include agent role attribution per finding," which is forward-looking (single-agent setup today, multi-agent swarm per issue #128's full scope). Not an issue — just noting it's a deliberate future-proofing choice.

• [info] [design-choice] agents/review.md:8 — model: sonnet pins the agent to a specific model tier. Reasonable cost/capability trade-off for a review agent. Worth revisiting if review quality needs to improve.

• [info] [intent-alignment] The PR correctly scopes to the "5a" deliverables identified in issue discussion (single review agent, no coordinator, no third-party reviewer integration). The broader Story 5 scope (swarm coordination, verdict aggregation, external reviewer shims) is left for follow-up work.

Footer

Outcome: approve
This review applies to SHA d6105cc2bb4a30213de51e54864c4a02a2d53091. Any push to the PR head clears this review and requires a new evaluation.

[ralphbean]

I used the agent definition here to produce the review comment at #171 (review). That's decent output for a first pass.

[ralphbean]

Review notes

Three actionable items:

1. Don't close Story 5: Review Agent Swarm & Coordinator #128 — this PR is a partial delivery of the story (single reviewer, no swarm/coordinator/verdict aggregation). Consider changing "Closes Story 5: Review Agent Swarm & Coordinator #128" to "Toward Story 5: Review Agent Swarm & Coordinator #128" or filing a follow-up issue for the remaining work.

2. Tools/steps mismatch — the agent's tool list is scoped to Bash(gh pr:*),Bash(gh issue:*), but the skill instructs it to run git log, xxd, and grep (steps 3 and the injection defense Unicode inspection). The agent cannot execute several of its own documented steps. Either expand the tool list (e.g. Bash(git log:*), Bash(xxd:*), Bash(grep:*)) or trim those steps until a sandbox layer is in place.

3. cat review-comment.txt contradicts "never modify files" — step 6 uses $(cat review-comment.txt) to post the review, which implies writing a temp file, but the constraints say "Never modify repository files." A heredoc (matching the pattern in filing-issues/SKILL.md) would avoid the contradiction.

[waynesun09]

Review: #171

Three concerns from reviewing the agent definition, skills, and integration surface:

1. [High] Exit code semantics undefined

PR #208's ExitCodeReader interprets the review agent's exit code: 0 = approved, nonzero = rejected. But agents/review.md never specifies how outcomes map to process exit codes.

This creates a concrete risk: if the agent produces a comment-only outcome with medium findings and exits 0, the automation layer in #208 treats it as an approval. The three-outcome model (approve / request-changes / comment-only) collapses to a binary gate with no documented contract.

Suggestion: Add an "Exit code contract" section to agents/review.md:

• approve → exit 0
• request-changes → exit 1
• comment-only → decide and document (exit 0 or exit 1)

2. [Medium] Tools/steps mismatch — xxd and grep not in tool list

The injection defense steps in both code-review/SKILL.md and pr-review/SKILL.md reference xxd | grep pipelines for Unicode/steganography detection, but the agent's tool list only permits Bash(git diff:*), Bash(git log:*), Bash(gh pr:*), Bash(gh issue:*). The agent cannot execute its own injection defense procedures as written.

Ralph flagged this in the initial review and the git commands were added, but xxd/grep remain unresolved. Either:

• Add Bash(xxd:*) and Bash(grep:*) to the tool list, or
• Rewrite the injection defense steps to use the Grep tool with Unicode-aware patterns (preferred — keeps the tool surface minimal)

3. [Medium] No skill routing guidance

The agent has two skills (code-review and pr-review) but no instructions for when to use which. When invoked via --print for pre-push review (as #208 does), the agent should use code-review. When invoked for a GitHub PR, it should use pr-review.

Currently the LLM must infer the correct skill from context, which is fragile. A routing section in the agent definition would make this deterministic:

• Prompt mentions a PR number or gh pr context → pr-review
• Prompt is about a local branch diff with no PR → code-review

[waynesun09]

Agent/skill practice review — 8 findings

Reviewed against the conventions established by the code agent (PR #189), the experiment triage agents, and the architecture/glossary docs.

What's done well: Two-skill decomposition (code-review + pr-review) is clean and matches team consensus. Zero-trust is well-articulated. SHA pinning, severity-based outcome mapping, and treating the PR body as a separate untrusted input are all solid. The delegation from pr-review → code-review follows the coordinator/specialist pattern from the triage experiment.

Root cause of issues 1, 5, and 8: The agent definition is trying to be both a role card and a procedure document. The code agent established a clear split: agent = role card (identity, trust, constraints, tool surface, failure contract); skill = procedure manual (steps, output formats, tool usage). Fixing the role/procedure boundary fixes three of the eight findings.

Summary

#	Issue	File	Severity
1	Agent duplicates skill content (review dimensions in both)	agents/review.md	medium
2	No disallowedTools — "cannot modify files" is prose-only	agents/review.md	high
3	Tool list doesn't cover commands skills instruct agent to run	agents/review.md	high
4	xxd pipeline for Unicode detection has wrong regex, fragile	skills/code-review/SKILL.md	high
5	Skills restate agent constraints instead of citing authority	skills/code-review/SKILL.md	medium
6	Hardcoded main branch — false-clean on non-main repos	skills/code-review/SKILL.md	medium
7	No structured failure output for coordinator to parse	agents/review.md	high
8	Output format ownership split across 3 files	agents/review.md	medium

All fixable without architectural changes. The high items (2, 3, 4, 7) are the ones I'd want addressed before merge — they affect security enforcement or functional correctness.

[github-actions]

Site preview

Preview: https://d0dd3512-site.fullsend-ai.workers.dev

Commit: 8167a9ebd0fb0b89c28608eb32a573ea603e1ba5

[ralphbean]

Review: #171

Head SHA: b807890
Outcome: request-changes

Summary

PR #264 (merged) built the scaffold infrastructure that deploys and runs agents in the .fullsend repo — including a review.yml workflow that invokes fullsend run review. That workflow expects to find the agent definition in the deployed .fullsend repo's agents/ directory. This PR only places the review agent at the repo root (agents/review.md), with no corresponding scaffold copy. The review workflow will fail at runtime.

Findings

High

• [deployment-gap] agents/review.md / internal/scaffold/fullsend-repo/agents/ — The triage agent established the pattern: the development-time agent lives at agents/triage.md (repo root), and the scaffold-deployed copy lives at internal/scaffold/fullsend-repo/agents/triage.md. The scaffold's review.yml workflow calls fullsend run review, which resolves the agent from the .fullsend repo's agents/ directory at runtime. Since no internal/scaffold/fullsend-repo/agents/review.md exists, the review workflow will find no agent definition and fail.

  Additionally, the scaffold's review.yml sets GITHUB_PR_URL and GITHUB_ISSUE_URL as environment variables (matching the triage pattern), but neither the review agent nor the pr-review skill references these env vars. Compare with internal/scaffold/fullsend-repo/agents/triage.md, which explicitly documents: "The environment variable GITHUB_ISSUE_URL contains the HTML URL to the issue."

  Remediation:

  1. Add internal/scaffold/fullsend-repo/agents/review.md — the scaffold-deployed version of the review agent, with an Inputs section documenting that GITHUB_PR_URL provides the PR to review.
  2. Update the pr-review skill's step 1 to check GITHUB_PR_URL as its first resolution path (before falling back to CLI args or gh pr view).
  3. Decide whether skills also need scaffold copies or are resolved via the CLI binary, and document the resolution mechanism.

Footer

Outcome: request-changes
This review applies to SHA b8078905f95931c7ebc97e66dc14f84ca990ebb7. Any push to the PR head clears this review and requires a new evaluation.

[ralphbean]

Review: #171

Head SHA: 8167a9e
Outcome: approve

All prior review findings have been addressed:

• Credential isolation (dual-token architecture) ✓
• Structured output pipeline (ADR 0022) ✓
• Sandbox policy with wildcard googleapis.com ✓
• scan-unicode coverage aligned with Go scanner ✓
• Stale-head guard in post-review.sh ✓
• disallowedTools added to agent definition ✓
• Non-UTF-8 warning instead of silent skip ✓
• jsonschema dependency added to workflow ✓
• Action mapping table clarified with required fields ✓

Filed #340 to investigate whether the jsonschema pip install step is truly needed (may be pre-installed in the container image).

Pipeline tested successfully against appdumpster — run 24804834625 completed with a valid review posted.

[ralphbean]

Anything else we need to deal with here, please file as a follow-up issue.

I believe all of @waynesun09's comments were addressed.