chore: improve AI PR-review (model selection, cost reporting, instructions)

[mikhail-dcl]

Pull Request Description

What does this PR change?

Improves the automated Claude Review CI so it catches design/architecture defects (not just line bugs) and is cheaper to operate. .github/ only — no runtime/player code.

• Richer review instructions (.github/prompts/review-instructions.md): a Design & Integration check (mandatory lifecycle-owner search + anti-rationalization guards), a teardown/consumption trace, and new blocking categories (resource leaks, dead infrastructure, detached async, nullability, dead conditions). Distilled from a human review whose architectural findings the prior prompt missed. Based on feat: prioritize share-screen source and fix camera-off frozen frame #9043 & feat: poc enable comms for PEXs interacting with the authoritative-server #9059
• Model-selectable on-demand review: @claude review opus runs claude-opus-4-6; default stays claude-sonnet-4-6. The chosen model is shown in the pending status.
• Model-aware --max-turns (Sonnet 50 / Opus 30) to bound spend without truncating a normal review.
• Per-run cost reporting: both review jobs append the run's USD cost to the end of the review comment (after QA_REQUIRED), e.g. Review cost: $0.42 - claude-opus-4-6 - 28 turns. Posts even when a run fails/maxes out (annotated), idempotent across re-runs, and continue-on-error so it never fails the job.

Test Instructions

The @claude review comment handler runs from the default branch, so the model-selection / cost behavior only goes live once this is merged to dev.

Prerequisites

• Merged to dev (required for the comment-triggered path)
• ANTHROPIC_API_KEY has access to claude-opus-4-6

Test Steps

1. On any open PR, comment @claude review → runs on Sonnet; pending status shows (sonnet); the posted review comment ends with a Review cost: block.
2. Comment @claude review opus → runs on claude-opus-4-6; status shows (opus); cost block reflects the opus model.
3. Open a fix:/chore: PR (auto-review path) → its auto-review comment also ends with a cost block.
4. For a run that hits the turn cap, confirm the cost block still appears, annotated did not finish (error_max_turns).

Additional Testing Notes

• Instruction efficacy was validated locally via clean-room reviews against feat: poc enable comms for PEXs interacting with the authoritative-server #9059: the new prompt reproduces the human reviewer's architectural findings on Opus (and the resource/lifecycle ones on Sonnet); the old prompt missed them.
• Workflow YAML can't be fully exercised pre-merge (issue_comment runs from the default branch). The push-triggered auto-review job stays on claude-sonnet-4-6.

Quality Checklist

• Changes have been tested locally (instructions validated via clean-room reviews; workflow wiring verified; full end-to-end requires default-branch merge — see notes)
• Documentation has been updated (if required) — N/A; workflow is self-documented with inline comments
• Performance impact has been considered — expense bounded via model-aware --max-turns; the cost step is read-only and continue-on-error
• For SDK features: Test scene is included — N/A (CI tooling, not an SDK feature)

Code Review Reference

Please review our Branch & PR Standards before submitting.

[github-actions]

Build skipped — no changes detected under Explorer/.

[github-actions]

---

💰 Review cost: unknown · claude-sonnet-4-6

[mikhail-dcl]

@claude review opus

[claude]

Claude finished @mikhail-dcl's task in 3m 4s —— View job

---

Review — chore/ai-review-improvements

• Load tools
• Read changed files and diff
• Analyze review instructions changes
• Analyze workflow YAML changes
• Post findings

---

Root-Cause Check

Problem being solved: The prior review prompt was doing line-level analysis only — it missed architectural placement bugs (wrong owner, lifecycle duplication, per-frame reconciliation of explicit moments) and resource teardown issues. The diff adds a Design & Integration Check, Teardown/Consumption Trace, and five new blocking categories to close that gap. It also adds on-demand model selection and cost reporting.

Cause or symptom fix? Cause. The instructions directly expand what the reviewer looks for, and the cost/model additions improve observability. No workarounds; no deferred issues.

---

Design & Integration Check

Not applicable — this PR introduces no ECS systems, plugins, services, or Unity runtime units. All changes are confined to .github/. No owner search required.

---

No Blocking Issues Found

Detailed findings across both files:

.github/prompts/review-instructions.md
The new sections (Design & Integration Check, Teardown/Consumption Trace, blocking categories 7–11, Construction/Encapsulation/Resource Checklist) are well-structured and precisely scoped. The anti-rationalization guards are a valuable addition given how AI reviewers tend to accept the author's framing. No issues.

.github/workflows/claude-pr-review.yml

1. Resolve review model step (lines 401–422): Shell logic is correct. grep -qiE '\bopus\b' <<< "$COMMENT_BODY" uses word-boundary anchors, so it won't match e.g. modus-opus or opus1. Step runs before Set status to pending which uses steps.model.outputs.label, so the ordering is correct.

2. Set status to pending — description length: GitHub commit status descriptions cap at 140 chars. 'Claude review in progress (sonnet)...' is 38 chars — well within limit. ✓

3. Report review cost step (both jobs): Error handling is correct: try/catch around file read, continue-on-error: true at the step level, and the `` marker makes re-runs idempotent. The .pop() to pick the last `claude[bot]` comment is correct given `track_progress: true` creates exactly one per run.

4. context.issue.number in auto-review: For pull_request events, context.issue.number maps to the PR number (GitHub's Issues API treats PRs as issues). Valid. ✓

5. if: condition ordering in auto-review cost step: steps.check-type.outputs.is-auto-review == 'true' && always() && steps.claude.outcome != 'skipped' has always() in the middle. In GitHub Actions, the presence of any status function (always(), success(), etc.) anywhere in the expression suppresses the default implicit success() check. Functionally equivalent to always() && .... The review job consistently writes if: always() && ... (the conventional placement) — not a bug but an inconsistency worth being aware of.

6. Auto-review job --max-turns stays at 40: The auto-review job's Claude Review step keeps --max-turns 40 while the on-demand review job gets 50 (Sonnet) / 30 (Opus). This is intentional — auto-review always uses Sonnet and the existing cap is acceptable for the narrower scope. ✓

7. Duplicate cost-reporting script block: The 40-line JavaScript appears in both auto-review and review jobs. This is expected in GitHub Actions YAML (no native deduplication without composite actions). ✓

8. Set status and approve if eligible step interaction with cost append: That step searches for REVIEW_RESULT: in the claude comment body, which is written by the AI reviewer and unaffected by the cost block appended before it. Parsing is not disturbed. ✓

---

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Changes touch only .github/ CI workflow YAML and an AI prompt markdown — no ECS, runtime, or Unity player code.
QA_REQUIRED: NO

---

💰 Review cost: unknown · claude-sonnet-4-6

[github-actions]

🔒 Claude reviewed this PR — sensitive paths modified (.github/prompts, .github/workflows, or CODEOWNERS). Claude will not auto-approve these PRs — human DEV review is required. No QA needed (Claude reported QA_REQUIRED: NO).

[claude]

Claude finished @mikhail-dcl's task in 1m 22s —— View job

---

Security Review — chore/ai-review-improvements → dev

• Load tools & read changed files
• Dependency-change scan (manifest / lockfile / binaries)
• Workflow & prompt security review (STEP W)
• Assess report-review-cost.js
• Post verdict

This PR touches sensitive paths (.github/workflows, .github/prompts, .github/scripts), so this is a security-focused review per the repo's sensitive-path protocol. The diff since the prior security review adds Read,Glob,Grep to the reviewer's --allowedTools in both jobs — assessed below.

---

Dependency changes — none

No Packages/manifest.json, Packages/packages-lock.json, binary, native plugin, or .asmdef changes. The new-dependency label tracks the new local helper .github/scripts/report-review-cost.js, not an external package. No external supply-chain surface in this PR.

---

Workflow & prompt review (STEP W)

The HIGH controls all hold — re-verified against the current file:

Control	Status	Evidence
W.1 Prompt from base, not PR head	✅ secure	both jobs fetch via gh api .../contents/...?ref=$BASE_SHA (base.sha / steps.pr.outputs.base_sha, lines 88, 421). A PR editing the prompt cannot self-modify its own reviewer.
W.2 Random heredoc delimiter	✅ secure	DELIM="PROMPT_EOF_$(uuidgen)" (lines 85, 418).
W.3 TOCTOU / post-comment push	✅ secure	review job rejects when pr.head.repo.pushed_at > comment.created_at (lines 351–364), using pushed_at — not the forgeable committer date.
W.4 Untrusted event text in prompt	✅ secure	comment.body is never interpolated into the LLM prompt — only a boolean contains(...) selecting between two fixed strings (line 437), and read via the COMMENT_BODY env var for the model grep (grep -qiE '\bopus\b' <<< "$COMMENT_BODY"). No injection channel.
W.6 Actor gate	✅ secure	review job needs: check-member, which calls GET /orgs/decentraland/members/$USER with ORG_ACCESS_TOKEN (lines 313–324).
W.8 pull_request_target	✅ n/a	auto-review uses pull_request; on-demand uses issue_comment behind the member gate. No pull_request_target.
W.9 Artifact retention	✅ secure	retention-days: 7 (lines 120, 454).
W.10 Permissions	✅ secure	explicit minimal per-job permissions: blocks.

New this diff — Read,Glob,Grep added to --allowedTools (lines 110, 444). These are read-only tools scoped to the checked-out PR-head working tree. Risk assessment:

• They broaden the LLM's read surface (repo files), but introduce no new exfil primitive — the allowlist still has no Bash(cat:*), curl, or WebFetch (W.5b). The only outbound channels remain gh pr comment / inline comments, whose contents are the review output and are public regardless.
• The reviewer already consumed PR-head diff content, so reading neighbouring source files is the same trust tier (PR-head code is data, the base-sourced prompt is the instruction set — W.1 keeps that boundary).
• Repo files contain no secrets (workflow secrets live in the Actions env, not the tree), so a Read can't surface ANTHROPIC_API_KEY / GITHUB_TOKEN.
• Gated behind the org-member trigger (on-demand) and a base-sourced prompt.

→ Acceptable, low risk. The addition is proportionate to the prompt's new STEP 1/3/5 requirement that the reviewer search the repo (lifecycle-owner search, leak-mirror grep) rather than work from the diff alone.

report-review-cost.js — low risk. Reads the action's own $RUNNER_TEMP/claude-execution-output.json and appends a cost block to the latest claude[bot] comment. The only non-numeric value interpolated into the comment is subtype (line 31 → did not finish (${subtype})), which comes from the action's own result event, not PR-controlled input. Idempotent (`` marker), continue-on-error, no new tool/permission surface. Falls back to `createComment` if the edit fails. Good extraction of the prior ~33-line duplication.

---

MEDIUM — pre-existing, not introduced by this PR

These live in the file under review but are unchanged by this diff. Flagged per protocol; they should not block this PR.

1. Bash(gh pr review:*) wildcard in allowedTools (lines 110, 444). A wildcard gh pr review grants the LLM approve/request-changes on an arbitrary PR number (W.5a). Mitigated by the org-member gate and the separate enforce-approvals workflow, but worth scoping to the triggering PR next time this file is touched. Bash(gh pr comment:*) is similar, lower impact.
2. Secret-holding action on a mutable tag (W.7a). anthropics/claude-code-action@v1 (lines 97, 429) consumes secrets.ANTHROPIC_API_KEY; actions/github-script@v8, actions/checkout@v6, actions/upload-artifact@v6 are tag-pinned. Consider SHA-pinning in a follow-up.

Neither is a regression from this PR. The prompt changes introduce no tool-allowlist escalation that reaches the network or grants write beyond the existing gh pr comment/create_inline_comment — the new sections only steer analysis depth and the read-only Read/Glob/Grep discussed above.

---

Verdict

The changes this PR introduces are secure: the prompt is sourced from base, no untrusted text reaches the LLM prompt, the model-selection grep is env-quoted, the cost script adds no privileged surface, and the new Read/Glob/Grep tools are read-only with no exfil channel. The two MEDIUM items are pre-existing config worth a human glance but are not blockers.

DEPENDENCY_REVIEW: NEEDS_ATTENTION