Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 37 additions & 11 deletions .agents/skills/local-qa/scripts/validate-opencode.bats
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
#!/usr/bin/env bats
# Validate .opencode/ agent frontmatter, review-pr command/skill references,
# that opencode.jsonc parses, and that its external_directory permission
# allow-lists the resolver path review-pr.md actually sources and the runtime
# review-state directory pattern.
# allow-lists the resolver path the pr-review skill actually sources and the
# runtime review-state directory pattern.

setup() {
repo_root="$(git -C "${BATS_TEST_DIRNAME}" rev-parse --show-toplevel)"
agents_dir="${repo_root}/.opencode/agents"
review_pr_doc="${repo_root}/.opencode/commands/review-pr.md"
review_pr_command="${repo_root}/.opencode/commands/review-pr.md"
review_pr_doc="${repo_root}/.opencode/skills/pr-review/SKILL.md"
orchestrator="${agents_dir}/review-pr-orchestrator.md"
opencode_jsonc="${repo_root}/.opencode/opencode.jsonc"
required_keys=(name description mode permission)
# Backtick-quoted identifiers in review-pr.md that are skills, toolkits, or
# config inputs rather than agents.
non_agents=(pr-feedback-triage pr-review-toolkit use-github-token)
# Backtick-quoted identifiers in the review-pr command and pr-review skill
# that are skills, toolkits, or config inputs rather than agents.
non_agents=(pr-feedback-triage pr-review pr-review-toolkit use-github-token)
}

agent_files() {
Expand Down Expand Up @@ -66,11 +68,11 @@ frontmatter() {
}
}

@test "every agent referenced in review-pr.md exists under .opencode/agents/" {
@test "every agent referenced in the review-pr command and pr-review skill exists under .opencode/agents/" {
local bt pattern refs ref skip na missing=()
bt=$(printf '\x60')
pattern="${bt}[a-z][a-z0-9]+(-[a-z0-9]+)+${bt}"
mapfile -t refs < <(grep -hoE "${pattern}" "${review_pr_doc}" | tr -d "${bt}" | sort -u)
mapfile -t refs < <(grep -hoE "${pattern}" "${review_pr_command}" "${review_pr_doc}" | tr -d "${bt}" | sort -u)

for ref in "${refs[@]}"; do
skip=0
Expand All @@ -91,6 +93,30 @@ frontmatter() {
sed -E 's#^[[:space:]]*//.*$##' "${opencode_jsonc}" | jq empty
}

@test "review-pr command is a thin wrapper that loads the pr-review skill" {
local bt
bt=$(printf '\x60')
grep -Fq "${bt}pr-review${bt} skill" "${review_pr_command}"
# shellcheck disable=SC2016
grep -Fq '$ARGUMENTS' "${review_pr_command}"
# The canonical workflow must not be duplicated in the command.
run grep -E 'review-pr-(submit|gh)\.sh|resolve-app-token\.sh|review-state' "${review_pr_command}"
[ "${status}" -eq 1 ]
}

@test "pr-review skill frontmatter declares its name and description" {
local fm
fm="$(frontmatter "${review_pr_doc}")"
grep -qE '^name:[[:space:]]*pr-review$' <<<"${fm}"
grep -qE '^description:[[:space:]]*[^[:space:]]' <<<"${fm}"
}

@test "review-pr orchestrator allows loading the pr-review skill" {
local fm
fm="$(frontmatter "${orchestrator}")"
grep -qE '^[[:space:]]+pr-review:[[:space:]]*allow$' <<<"${fm}"
}

@test "review-pr local fallback is limited to a missing trusted PR number" {
# shellcheck disable=SC2016
grep -Fq 'If `context` reports `Trusted pull request number is unavailable.`, continue in local mode; for every other `context` failure, stop.' "${review_pr_doc}"
Expand All @@ -102,12 +128,12 @@ opencode_jsonc_json() {
sed -E 's#^[[:space:]]*//.*$##' "${opencode_jsonc}"
}

@test "review-pr.md sources the resolver from a path opencode.jsonc allow-lists under external_directory" {
@test "pr-review skill sources the resolver from a path opencode.jsonc allow-lists under external_directory" {
local resolver_suffix resolver_path default_action allow_patterns pattern expanded matched=0

resolver_suffix="$(grep -oE 'opencode_app_token_lib="\$\{HOME\}/[^"]+"' "${review_pr_doc}" | head -1 | sed -E 's/^opencode_app_token_lib="\$\{HOME\}\/(.*)"$/\1/')"
[ -n "${resolver_suffix}" ] || {
echo "review-pr.md does not set opencode_app_token_lib to a \${HOME}-relative path"
echo "the pr-review skill does not set opencode_app_token_lib to a \${HOME}-relative path"
return 1
}
resolver_path="${HOME}/${resolver_suffix}"
Expand All @@ -132,7 +158,7 @@ opencode_jsonc_json() {
done

[ "${matched}" -eq 1 ] || {
echo "no external_directory allow pattern (${allow_patterns[*]}) matches the resolver path ${resolver_path} that review-pr.md sources"
echo "no external_directory allow pattern (${allow_patterns[*]}) matches the resolver path ${resolver_path} that the pr-review skill sources"
return 1
}
}
Expand Down
5 changes: 4 additions & 1 deletion .opencode/agents/review-pr-orchestrator.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ permission:
"../*.config/opencode/review-state/update.json": allow
glob: allow
grep: allow
skill:
"*": deny
pr-review: allow
bash:
"*": deny
"git status --short": allow
Expand Down Expand Up @@ -51,4 +54,4 @@ permission:
code-simplifier: allow
---

Coordinate a strictly read-only review. Never modify the checkout. Use only the exact argument-free helper commands, the two fixed review-state JSON files, and the approved reviewer agents.
Coordinate a strictly read-only review. The canonical workflow is the `pr-review` skill; load it and follow it exactly. Never modify the checkout. Use only the exact argument-free helper commands, the two fixed review-state JSON files, and the approved reviewer agents.
82 changes: 2 additions & 80 deletions .opencode/commands/review-pr.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,84 +3,6 @@ description: Strictly read-only GitHub PR review with specialized reviewers, val
agent: review-pr-orchestrator
---

# Strictly Read-Only PR Review
Load the `pr-review` skill with the native skill tool and follow its instructions exactly. Do not start reviewing before the skill is loaded, and do not substitute any other workflow for it.

This is a strictly read-only repository review. Analyze and report only. Do not create, edit, delete, format, generate, install, or fix files. Do not execute repository QA scripts, formatters, generators, package managers, or commands with mutation flags such as `--fix`, `--write`, or equivalent options.

Do not run repository-wide QA scripts, formatters, auto-fixing linters, generators, dependency installers, or anything that can create caches, reports, snapshots, lockfiles, coverage output, scan output, or configuration exports in the checkout.

Every helper this command invokes — the read-only `gh` wrapper, the constrained submission helper, and the App-token resolver they source — lives only at its `${HOME}/.config/opencode/scripts/` path, installed there by the action before the reviewed repository is ever checked out. Never invoke any of them by a repository-relative path such as `.opencode/scripts/...`: the checkout under review is untrusted input, and a repository-relative path would let a malicious PR that edits or adds a same-named file substitute its own script for the trusted one. These helper paths and the dedicated `${HOME}/.config/opencode/review-state/` directory are the sole allow-listed external locations. Despite the directory-level external access required by OpenCode, use the edit tool only for `initial.json` and `update.json` as instructed below. The helpers use `opencode_app_token_lib="${HOME}/.config/opencode/scripts/resolve-app-token.sh"` for authentication.

**Requested review aspects (optional):** "$ARGUMENTS"

## 1. Establish the trusted context

Before any analysis, invoke `bash "$HOME/.config/opencode/scripts/review-pr-submit.sh" prepare` once, followed by `bash "$HOME/.config/opencode/scripts/review-pr-gh.sh" context`. The context is persisted outside the checkout and pins one repository, PR number, and head SHA for the entire review. If `prepare` fails, stop. If `context` reports `Trusted pull request number is unavailable.`, continue in local mode; for every other `context` failure, stop.

The context helper derives the PR number from `.pull_request.number` or `.issue.number`. For `issue_comment`, it fetches and pins the current head SHA through the trusted PR API. Metadata, diff, submission, and update revalidate that the current head still matches the pinned SHA and fail closed otherwise. Obtain metadata and the diff only through these fixed operations:

```bash
bash "$HOME/.config/opencode/scripts/review-pr-gh.sh" metadata
bash "$HOME/.config/opencode/scripts/review-pr-gh.sh" diff
```

If no PR context can be established, use local mode: `git status --short`, `git diff --name-only HEAD`, and `git diff --no-ext-diff`; do not infer a PR from the current branch. Once `context` succeeds, any later metadata, diff, or validation failure must abort the review rather than falling back to local mode.

Capture the full diff, changed-file list, PR title/body, base and head branch names, head SHA, and relevant source context using the read, glob, and grep tools. Pass that complete context to reviewers; they have no shell access and must not need it.

## 2. Select and launch reviewers

Supported aspects are:

- `code`: `code-reviewer`, `code-quality-reviewer`
- `quality`: `code-quality-reviewer`
- `performance`: `performance-reviewer`
- `security`: `security-code-reviewer`
- `tests` or `coverage`: `test-coverage-reviewer`, `pr-test-analyzer`
- `docs` or `documentation`: `documentation-accuracy-reviewer`
- `comments`: `comment-analyzer`
- `errors`: `silent-failure-hunter`
- `types`: `type-design-analyzer`
- `simplify`: `code-simplifier`, returning behavior-preserving simplification proposals as review findings without modifying files
- `all`, or no aspect: the core reviewers `code-quality-reviewer`, `performance-reviewer`, `test-coverage-reviewer`, `documentation-accuracy-reviewer`, `security-code-reviewer`, and `code-reviewer`; include specialty reviewers when the supplied diff is relevant. Run `code-simplifier` only when `simplify` is explicitly requested; never include it in `all`.

Launch only the explicitly permitted reviewer agents. For each, supply the captured diff, changed-file list, metadata, and relevant source context. Tell each reviewer to inspect changed lines and their containing functions only, return high-confidence findings only, and use:

```yaml
- file: path/to/file
line: <head-file line number>
severity: critical | important | suggestion
source: <agent-name>
message: <concise issue description and concrete fix>
```

Do not let a reviewer post to GitHub.

## 3. Normalize and anchor findings

Drop praise, nitpicks, style-only feedback, findings outside the changed-file list, and duplicates. Keep the most specific actionable finding for each root cause. Classify every remaining finding as inline when its file and head-side changed line can be anchored in the captured diff; adjust only to a nearby relevant changed line. Put genuine but unanchorable findings in `summary_only` with a short reason.

Before returning any top-level text in PR mode, including no-finding and summary-only fallback results, invoke `bash "$HOME/.config/opencode/scripts/review-pr-gh.sh" validate`. If validation fails, stop. If there are no findings, then return exactly `No noteworthy issues found.` Do not post an empty review.

For findings, the `prepare` and `context` operations in section 1 have already created the empty payload files and pinned the review context. Do not run them again. Use the edit tool only for `$HOME/.config/opencode/review-state/initial.json`, writing exactly `{body, comments}` with a nonempty body and inline comments array. The helper validates the payload and adds the trusted `commit_id` and `event` itself. Each inline body is `**<severity> · <source>**: <issue and concrete fix>`.

Every finding with a valid diff anchor must be included in the `comments` array and submitted as an inline review comment. Never return anchorable findings only as top-level assistant text. If structured submission fails, fail the run instead of emitting the findings as a top-level completion comment.

When there are summary-only findings, the body begins `OpenCode PR Review: <N> inline finding(s), <M> summary-only finding(s).` and lists them. Otherwise it begins `OpenCode PR Review: <N> inline finding(s).` Never use issue comments or `gh pr comment`.

## 4. Submit through the constrained helper

Use only these exact commands:

```bash
bash "$HOME/.config/opencode/scripts/review-pr-submit.sh" submit-initial
bash "$HOME/.config/opencode/scripts/review-pr-submit.sh" update
```

After the single `prepare` in section 1, write the initial payload only to `$HOME/.config/opencode/review-state/initial.json`. Before `update`, write exactly `{body}` only to `$HOME/.config/opencode/review-state/update.json`. Never add arguments, redirections, pipelines, or process substitutions to helper commands.

You never pass a repository, PR number, target commit, or review ID: the helper derives the repository and PR number from the trusted GitHub Actions context, pins the write to the head commit from the same context, and updates only the review ID it recorded when the initial submission succeeded in this run. It validates the trusted event context, temporary payload, target commit, HTTP method, and exact pull-request-review endpoint. It sources the existing App-token resolver and calls `opencode_require_app_token_for_review` immediately before its permitted POST or PUT. This preserves verified `opencode-agent[bot]` attribution when available, preserves the explicit `use-github-token: true` fallback, and never accepts an unverified candidate for a write.

After successful inline submission, do not repeat findings in the final assistant output. Update the submitted review with final status and the run URL when available; the helper targets the review it recorded, so no review ID is passed. If GitHub rejects inline anchors, retry once only after converting the identified invalid anchors to summary-only; never lose a finding. If no inline anchors remain, return the concise markdown fallback instead of submitting an empty comments array.

Do not clean, reset, restore, stash, commit, or push anything.
Requested review aspects (optional): "$ARGUMENTS"
Loading
Loading