菜单
Use when senior PHP code review focused on architecture, business logic, and risk detection. Read-only.
用 Codex 或 Claude 帮你安装 复制这段 Prompt，粘贴到 Codex、Claude 或其他助手里，让它检查 Skill 页面并帮你完成安装。
基于 SOC 职业分类
Use when you need an evidence-first, read-only inventory of every automation in this repo (GitHub Actions, Claude Code hooks/settings, MCP servers, composer scripts, the bundled CLI installer, the skills catalog, scheduler/cron) before changing any of them, classifying each as live, broken, or redundant and recommending keep/merge/cut/fix.
Use when choosing how to run Claude Code autonomously on this project — from a single sequential pipeline to multi-agent DAG orchestration. A reference catalog of loop patterns anchored to this repo's real tooling (resolve-issue, autoresolve-oldest-github-issue, code-review-github, process-code-review, merge-github-pr, /loop), with composer build / composer skill-check as the quality gate between iterations.
Use when a goal is vague speed ("make it faster", "reduce p95", "cut query time") and you need a bounded, measured loop that promotes only verified, correctness-preserving wins instead of guessed micro-tweaks.
Use when measuring performance baselines or detecting regressions before and after a change in a Laravel app — page Core Web Vitals, API latency percentiles, build/test velocity, and DB query timing, stored as git-tracked baselines for team comparison.
Use when a single objective is too large for one pull request and must span multiple sessions or PRs. Turns the objective into a sequenced construction plan of 3-12 one-PR steps, each with a cold-start context brief, dependency edges, and exit criteria, then reviews it adversarially and registers it as Markdown.
Use when refactor PHP classes to improve structure, readability, and maintainability while preserving behavior
| name | code-review |
| description | Use when senior PHP code review focused on architecture, business logic, and risk detection. Read-only. |
Perform structured code review focused on:
__invoke request handlers, API Resources / DTOs serialized into responses, FormRequests, status-code / response() / abort() calls, Idempotency-Key handling), walk it against the API contract pillars. The dedicated walk lives in @skills/api-review/SKILL.md (Specialized Reviews → Always run); severities follow that rule's CR Severity Rules section.require / require-dev entry to composer.json, walk the Activity + Compatibility gates from that rule against the PR description / commit body. A missing selection note is a Critical finding; an adopted archived / abandoned / branch-pinned package is a Critical finding on the spot; a single-maintainer adoption without bus-factor flag is a Moderate finding.@rules/laravel/laravel.mdc, @rules/laravel/architecture.mdc, @rules/laravel/filament.mdc, and @rules/laravel/livewire.mdcgit add, git commit, git push, git reset, git checkout -- …, etc.). Switching to the relevant branch and git pull to read the latest diff are allowed; mutating the working tree or pushing to the remote is not. Output is the review markdown only.code-review-github / code-review-jira for publishing on the GitHub PR stays in canonical English per the rule's Exception — technical CR findings on the GitHub PR (severity labels, structured field labels, rule references, and code identifiers are all in English). The non-technical mirror that the wrappers delegate to @skills/pr-summary/SKILL.md follows the language of the source assignment — that is the wrapper's responsibility, not this skill's.The CR wrappers publish the review through a single-comment upsert keyed by the current actor identity (see @skills/code-review-github/SKILL.md and @skills/code-review-jira/SKILL.md). Follow-up runs edit that one comment in place, so the per-run audit trail lives in the tracker's edit history. Do not load prior CR findings from PR comments and do not author a Previous CR Status section in the output — the upsert convention makes it redundant.
Before reviewing code, load and analyze the full issue context:
skills/code-review-jira/scripts/load-issue.sh <KEY|URL> and read all fields off the resulting JSON document — never call acli directly. Fall back to the JIRA MCP server only when the script is unavailable or for data outside its scope (changelog, available transitions, friendly custom-field names). For Bugsnag errors, call skills/code-review-bugsnag/scripts/load-issue.sh <URL|TRIPLE> (requires BUGSNAG_TOKEN) and read the error class, message, context, latestEvent.stacktrace, comments[], and linkedIssues[] off the JSON — never call api.bugsnag.com directly. Fall back to a Bugsnag MCP server only when the script is unavailable.Run this section only when the diff integrates with, modifies, or depends on a third-party API or external service (HTTP clients, vendor SDK calls, webhooks, OAuth flows, payload schemas, queue/event consumers backed by external systems).
@rules/php/core-standards.mdc); only when the diff exposes an out-of-scope structural shortcoming in how the project consumes the API (e.g. missing webhook signature verification across other consumers) raise it under Refactoring Proposals.@rules/code-review/general.mdc Reuse Existing Logic section).interface that has neither at least two non-test consumers nor at least two non-test implementations. Test doubles, mocks, and fakes do not count toward either threshold. Implementing a framework or vendor interface (e.g. ShouldQueue, HasLabel, Arrayable) is always allowed. Require collapsing single-implementation, single-consumer interfaces back into the concrete class unless the PR documents an architectural reason — a published package API surface or a plugin extension point with a written contract — see @rules/php/core-standards.mdc Design Principles.if guards on internal values the caller already validates, fallbacks for unreachable branches); oversized method or class bodies where the same behavior fits in roughly a quarter of the lines without losing clarity ("if you write 200 lines and it could be 50, rewrite it"). The senior-engineer heuristic applies: when a reader would call the changed block overcomplicated, that is the finding. Severity: Moderate by default; escalate to Critical when the speculative abstraction creates a new business-logic home outside the seven allowed Laravel layers (see Architecture conformance). Pre-existing complexity the diff does not touch is out of scope — only added or modified lines are flagged. The YAGNI rules for speculative method parameters and speculative project-owned interfaces are already covered by @rules/php/core-standards.mdc Design Principles and the Speculative interfaces bullet above — do not duplicate findings; this bullet covers the remaining surfaces.__invoke(), or other callable whose signature declares more than 4 parameters on any line added or modified by the diff, per @rules/php/core-standards.mdc Structure section (parameter counting rules, exemption list, and required fix are defined there). Severity: Critical (structural / required-pattern violation; the Strict rule compliance default applies and may not be silently downgraded). Existing methods that the diff does not touch are out of scope; a pre-existing method becomes in scope the moment the diff adds, removes, renames, or re-types any parameter of that method.@rules/php/core-standards.mdc, @rules/api/general.mdc, @rules/code-review/general.mdc, @rules/refactoring/general.mdc, @rules/code-testing/general.mdc, and on Laravel projects @rules/laravel/laravel.mdc, @rules/laravel/architecture.mdc, @rules/laravel/filament.mdc, @rules/laravel/livewire.mdc, plus any project-specific @rules/**/*.mdc), scan the diff for any pattern that matches a numbered or bulleted rule from those files and raise one finding per matched violation. The standard is "every applicable rule must hold on every changed line", but the review process is pattern-matching the diff against the rule set, not asserting each rule against each line individually. Each finding cites file:line and the rule reference (e.g. @rules/php/core-standards.mdc#PHP Practices or @rules/laravel/architecture.mdc#Business Logic Layers). Severity defaults — apply the severity declared in the rule file's CR Severity Rules section if present; otherwise: architectural / structural / required-pattern violations are Critical, PHP-practice violations a fixer doesn't catch (missing return types, raw arrays across boundaries when DTOs exist, magic numbers, unsuppressed errors, generic exceptions, untyped iterables) are Moderate, naming or wording nits without a binding rule are Minor. Do not summarize "all rules apply" as a single finding — each violation needs its own line so the reviewer can verify the citation.@rules/laravel/architecture.mdc that is independent of Strict rule compliance and runs as a separate mandatory block on every Laravel project — detected by laravel/framework in composer.json require. Walk every section of that file against the current diff regardless of which files the diff touches (helpers, routes, configs, migrations, seeders, tests, or even a docs-only commit). The motivation is that architecture is also a verdict on where new code should have lived, not just on what the touched files do; a diff that ignores the business-logic layers entirely is itself an architecture signal worth raising. Walk every section: Architecture, Business Logic Layers (seven allowed homes including the Eloquent-model carve-out — verify each newly added or modified business-logic block sits in one of: Actions / Model Services / Repositories / ModelManagers / Data Validators / Data Builders / Eloquent models; flag every block landing in a controller, middleware, Blade view, Livewire component, job, listener, event handler, or helper with a concrete suggestion where it belongs), Actions (orchestration-only, single __invoke(), final readonly, constructor injection), Action Rules (no inline Eloquent, no DB::, no inline persistence — assessed on every Action file touched by the diff), Model Services (BaseModelService extension, no inline queries), Repositories and ModelManagers (read/write separation, basic queries only in Repositories, batch-first writes in ModelManagers — assessed on every Service / Repository / Manager file touched by the diff), DTOs (typed boundaries, mapping attributes, no raw arrays across layers), Data Modification (DRY), Data Builders (multi-method, no DB), Shared Concerns (Traits) (globally shared, domain-agnostic, reusable-as-is logic only — flag domain-specific code parked under app/Concerns/ and reusable trait logic scattered outside app/Concerns/), Validation Rules (Traits), Data Validators (DataValidator trait when pekral/arch-app-services is installed), Controllers and Other Entry Points (slim, every request-consuming controller method — resource action, single-action __invoke(), or custom method — must type-hint a project-owned FormRequest subclass and never Illuminate\Http\Request / untyped / a generic Request with $request->* reads; one dedicated FormRequest per request shape; no rules() branching on $this->method(); no inline validate() / Validator::make()), Resource Controllers (CRUD-only), Single-Action Controllers, Livewire (entry point, boot() injection, no business logic), Custom Helpers (global app/helpers.php functions, no static-method wrappers), and the CR Severity Rules subsection. Inherit the severity declared in CR Severity Rules; absent that, default to Critical for any orchestration / persistence / query bypass and any new code outside the seven allowed business-logic layers. arch-app-services examples (when installed): if vendor/pekral/arch-app-services exists or composer.json requires pekral/arch-app-services, additionally cross-check the diff against the package's published README.md examples at https://github.com/pekral/arch-app-services/blob/master/README.md — especially the worked examples — so the walk matches the canonical Action / Service / Repository / ModelManager / Data Validator shapes shipped with the package. When the package is not installed, ignore this README cross-check (do not adopt the package and do not invent examples). Report contract: the architecture walk runs on every Laravel CR run, but the published CR comment carries a ## Architecture section only when the walk produces at least one finding. When findings exist, list them with file:line, the cited subsection of @rules/laravel/architecture.mdc, and the standard reproducer fields (Faulty Example / Expected Behavior / Test Hint / Suggested Fix). When the walk produces zero findings, omit the ## Architecture heading entirely — never render a "walked, 0 findings" status line, a "clean" placeholder, or any other confirmation that the walk ran; the absence of the section is the clean signal. The walk itself is still mandatory; only the user-visible section is conditional. On non-Laravel projects (no laravel/framework in composer.json require), skip the walk entirely and omit the ## Architecture section from the CR comment — the section is Laravel-only by design (out of scope per issue #530: no framework-agnostic architecture rule).*.php under the project's test root, or any rename that changes the test file's path or base name), verify three things per @rules/code-testing/general.mdc Test Organization:
uses(...), the first new <Class>(...) / <Class>:: reference, or the file base name without the Test suffix) and confirm the test file sits under the parallel directory tree (src/Service/Billing/InvoiceCalculator.php → tests/Service/Billing/InvoiceCalculatorTest.php). Cross-cutting tests that intentionally target no single production class sit under an intent-named directory (tests/Feature/<flow>, tests/Contract/<vendor>, tests/Integration/<area>). Tests parked next to an unrelated class to satisfy a parallel path are a finding.{ClassName}Test.php; extracted scenario files use {ClassName}{Scenario}Test.php and stay in the same directory. Generic file names (MiscTest.php, TestsTest.php) and missing Test suffix are findings.it() / test() description matches the asserted scenario. For every it('…') / test('…') block introduced or modified by the diff, read the assertions in the body and confirm the description states that scenario in plain language. Findings: generic placeholders (it('it works'), test('test1'), test('happy path')), method-named descriptions (test('calculate'), it('handles getUser')), descriptions that contradict the assertions (it('adds user') over a body asserting removal), and stale descriptions left unchanged after the assertions were rewritten.
Severity: Moderate by default (organization / readability gate; a reviewer must be able to navigate the test tree by walking the parallel folder). Escalate to Critical when the misplacement hides regressions because the test never runs under the test runner's discovery glob, or when the description so misrepresents the assertions that future maintainers would change the wrong test. Suggested Fix must use one of these literal templates so @skills/process-code-review/SKILL.md extracts it deterministically:Move \<current/path/FooTest.php>` to `<tests//FooTest.php>` (rename base name to `<FooTest.php>` if the file name itself is wrong).`In \<path/FooTest.php>`, replace `it('')` with `it('')`. Never emit a vague "rename it". When the discovery fallback chain (Pestuses(...)→ firstnew (...)/::reference → file base name without theTest suffix) cannot resolve a single SUT — typical for legacy single-file test suites covering many classes — degrade to checking that the file sits under an intent-named directory (tests/Feature/, tests/Contract/, tests/Integration/`, or the project test root for an installer-style cross-cutting suite) and skip the per-SUT path check rather than flagging the pre-existing layout as a regression.update(), create(), delete(), or single-row read; require batching via ModelManager batchUpdate / batchInsert, whereIn(...)->delete(), or a single bulk read keyed in memory (see @rules/sql/optimalize.mdc "Batch over per-row operations"). Allowed only when an explicit code comment or PR note justifies an unavoidable per-row side-effect dependency.database/migrations/, model $table metadata, and live SHOW INDEX output when DB access is available) and verify the query is shaped to hit an existing index. Flag queries that bypass an existing covering index — WHERE / JOIN / ORDER BY column order does not match the left-to-right composite order, functions wrap indexed columns (non-SARGable), or SELECT pulls columns outside the covering index. The Suggested Fix must rewrite the query (column re-ordering, SARGable rewrite, covering projection) — propose a new index only when no existing index can cover the query and the schema gap is justified by EXPLAIN. Severity: Moderate, escalated to Critical when the un-indexed query runs on a hot path or large table (see @rules/sql/optimalize.mdc "Reuse existing indexes first").JOIN / WHERE / ORDER BY / GROUP BY / subquery / eager load, pagination change, or a query moved into another layer), verify per @rules/sql/optimalize.mdc "Performance Non-Regression on Query Changes" that the changed query is equal or faster than the original (compare EXPLAIN plan shape, rows examined, access type, index usage, filesort / temporary avoidance; use measured latency when DB access is available). Flag any change that is measurably slower and carries no documented reason + remaining optimization options in the PR description / commit body. The Suggested Fix is either the faster rewrite, or the missing documentation block (why it is slower, what optimization options remain or that none exist and why, and the trade-off that justifies it). Severity: Moderate, escalated to Critical when the slower query runs on a hot path or large table. Fold this into the ## Database Analysis section alongside the mysql-problem-solver findings.@rules/refactoring/general.mdc: behavior must be preserved, migration must be incremental (no big-bang rewrites), and entry points / responsibilities / DRY / concurrency must follow the recommended process. In Laravel projects, combine with @rules/laravel/architecture.mdc.@rules/refactoring/general.mdc Test Coverage Contract:
test(scope): cover <area> before refactor form per @rules/git/general.mdc). Missing pre-refactor coverage commit → Critical finding.app/Concerns/), not inline in Actions, controllers, jobs, commands, listeners, or Livewire components (see @rules/laravel/architecture.mdc Data Validators section)__invoke() body is a single delegating call to one Service / Facade / Model Service method with no orchestration of its own (no validation delegation, no DTO / data transformation, no coordination of multiple collaborators, no extra business step, no return-value reshaping). Such an Action is a redundant indirection layer per @rules/laravel/architecture.mdc Pass-through Action rule. The Suggested Fix must name the resolution: if the wrapped Service / Facade method is used only once in the codebase, inline its logic into the Action and delete the method (the Single-use Service/Facade method rule); if the method is reused, remove the Action and call the Service / Facade method directly from the entry point ($action($payload) → $service->method($payload)), listing the call sites that must change. Severity: Moderate (declared in @rules/laravel/architecture.mdc CR Severity Rules). A pre-existing Action becomes in scope the moment the diff edits its __invoke() body.find, findBy{Attribute}, all, simple where lookups, pagination of a base scope). Feature-specific or use-case–specific query methods in Repositories are a finding; specialization belongs in a Service (single-model) or Action (cross-model / cross-feature) composing basic Repository methods (see @rules/laravel/architecture.mdc Repositories and ModelManagers section)@rules/laravel/architecture.mdc Data Modification (DRY) section). Output the list of modification places explicitly in the review so the duplication picture is visible.Apply this subsection only when the source issue is flagged as highest priority, so the bug fix can deploy as fast as possible without sacrificing the Critical / Moderate gate.
priority: highest, priority/highest, priority-highest, p0, urgent, or blocker.priority field equals Highest or Blocker.Always run:
MODE=cr as a pre-flight before any other specialized review — audits whether the dev database currently holds the fixture data the assignment scenarios need, and surfaces every scenario the diff should cover but the codebase / dev DB cannot reproduce. The CR uses its gap report to ground the rest of the review in real data; an empty gap report means later findings (assignment compliance, security, refactoring) are made against scenarios that genuinely exist, not against guesses. Surface the gap count in the PR comment summary line and treat any behavioral gap the skill reports as a Critical CR finding (the diff cannot satisfy a scenario the codebase does not support).## Assignment Compliance markdown block (only when at least one Critical gap exists), the status no critical gaps — assignment compliance block omitted (when the implementation satisfies every stated requirement), or the status no linked issue — assignment compliance skipped (when no linked tracker exists). The CR wrapper passes the returned block as an embedded block to @skills/pr-summary/SKILL.md only when a block is returned so each linked tracker (GitHub issue or JIRA ticket) receives one consolidated comment per CR run (per issue #498); on either skip status the wrapper embeds nothing and surfaces the status on the PR comment summary line. Do not embed the block into the PR comment — that comment carries technical findings only.@skills/assignment-compliance-check/SKILL.md already raises in its ## Assignment Compliance block — when both would surface the same gap, keep the technical Critical finding here (with reproducer fields, folded into the standard severity buckets) and let the compliance block carry only the non-technical mirror.@rules/api/general.mdc). The skill self-scopes: when the diff touches no HTTP API surface (routes, controllers / __invoke request handlers, API Resources / DTOs in responses, FormRequests, status-code handling, Idempotency-Key logic) it returns no findings. When it does, fold the Critical / Moderate / Minor findings (with their reproducer fields) into the standard severity buckets — do not duplicate a finding the Strict rule compliance walk already raised for @rules/api/general.mdc.MODE=cr — read-only refactoring lens scoped to the PR diff. Use it to surface concrete tech-debt-reducing changes (DRY duplication, single-responsibility breaches, oversized methods) that apply to lines actually touched by the PR. MODE=cr guarantees the lens never modifies code, writes tests, commits, runs fixers, or chains another review — it returns proposals only. Do not propose changes that fall outside the diff.Run conditionally:
@rules/refactoring/general.mdc) → run the full refactoring skill set read-only. Detect this when the PR restructures existing code without adding a feature or changing observable behavior (extracted methods / classes, moved orchestration, renamed-for-clarity, dedup, layer reshuffles). When it fires, additionally invoke @skills/refactor-entry-point-to-action/SKILL.md with MODE=cr (and run the class-refactoring lens above at full depth) to surface the entry-point → Action proposals. Both skills run read-only — no code changes, no commits, no fixers, no review chaining — MODE=cr enforces this. Fold their output into the Refactoring (DRY / tech debt) section (in-scope items) and Refactoring proposals section (out-of-scope items) of the review; this skill never modifies code.@skills/mysql-problem-solver/SKILL.md is mandatory. Trigger this skill whenever the diff touches any of: raw SQL strings, Eloquent / query-builder calls (DB::, ->where(, ->join(, ->whereHas(, ->withCount(, ->orderBy(, ->groupBy(, ->chunk(, ->cursor(, paginate(, simplePaginate(), Eloquent relationship definitions, with( / load( eager loads, model scopes, ModelManager / Repository methods, database migrations (Schema::, up() / down()), seeders, factories that materialise rows, DynamoDB / NoSQL access. Pass the diff scope to mysql-problem-solver and capture its findings — they must appear in the published CR review under the dedicated ## Database Analysis section described in Output Rules, never silently absorbed into the generic Critical / Moderate / Minor buckets.Run this section over the PR diff only — never over untouched code.
@skills/class-refactoring/SKILL.md (run with MODE=cr — read-only):
*.blade.php file added or modified by the diff against the triggers in @rules/laravel/livewire.mdc HTML / Blade Layout Splitting. Raise one Refactoring (DRY / tech debt) entry per match — repeated structural HTML block (same wrapper + same inner skeleton appearing 2+ times in the same view, or once in 2+ views), Blade view exceeding 150 lines of actual markup with no splitting attempt and no documented exemption, a self-contained wire:model / wire:click / wire:submit interaction cluster left inline in the parent component, a @foreach body longer than ~10 lines of per-iteration markup, a region owning its own wire:loading / @empty / error state inline, or a clearly named UI concern (a noun phrase a designer would use) still inline. Each entry cites file:line, the matched trigger, the proposed concern name, the Component-type decision (Livewire iff state / lifecycle / server interaction; Blade otherwise — never a Livewire wrapper around a stateless presentational block), and the target folder (app/Livewire/<Domain>/ + resources/views/livewire/<domain>/ or resources/views/components/<domain>/). Severity is the refactoring-section default — these are tech-debt proposals, not Critical / Moderate blockers. The Critical / Moderate / Minor entries from the rule's CR Severity Rules (HTML Layout Splitting) subsection are picked up separately by the Strict rule compliance walk-through in Core Analysis and surface there at their declared severity, so this DRY entry does not duplicate them.->when() (Laravel projects). Flag every block on the diff that gates a query-builder / Eloquent clause on a nullable, optional, or boolean filter through an imperative if and then mutates $query inside the body — including but not limited to: if ($filters->x !== null) { $query->where*(...); }, if (! empty($filters->x)) { $query->where*(...); }, if (isset($filters->x)) { $query->where*(...); }, if ($filters->flag) { $query->orderBy(...); }, if ($search !== '') { $query->where('name', 'like', ...); }. The canonical Laravel form is $query->when($filters->x, fn ($q, $value) => $q->whereIn('campaign_type', $value)) — when() takes the filter value as the truth test, passes it as the second closure argument, and short-circuits when the value is falsy / null / empty. Raise one entry per matched if block with file:line, a one-sentence statement of the imperative conditional being rewritten, and a Suggested Fix that rewrites the if as a ->when(...) call while preserving the original predicate semantics byte-for-byte. Laravel's when() short-circuits on every falsy value (null, false, 0, '0', '', []); whenever the original if predicate would still fire for one of those values, the Suggested Fix must use the explicit truth-test form so behavior does not silently change. Concrete templates the reviewer must pick from:
if ($filters->x !== null) over a value the caller may legitimately set to 0 / '0' / false / '' / [] (collection / array filters, "clear all chips" → [], boolean false, numeric 0): rewrite as ->when($filters->x !== null, fn ($q) => $q->whereIn('campaign_type', $filters->x)). The truth test is the literal !== null and the closure references $filters->x directly — do not drop to ->when($filters->x, …), which would skip the clause for [] / 0 / false and change the result set (e.g. whereIn('campaign_type', []) produces 0 = 1 / empty result, while a skipped when() returns the unfiltered base set).if ($filters->x) (already truthy-gated — caller intentionally skips falsy values) or if (! empty($filters->x)): the short form ->when($filters->x, fn ($q, $value) => $q->whereIn('campaign_type', $value)) preserves semantics and the closure receives the value as $value.if (isset($filters->x)): rewrite as ->when($filters->x !== null, …) — isset rejects only null, matching !== null and not the broader falsy set when() short-circuits on.
Do not raise the entry when the body does more than one query mutation that genuinely cannot be expressed as a single closure (multi-statement orchestration, branching on the value, side effects outside $query); in that case the imperative if stays. Severity is the refactoring-section default — these are tech-debt proposals listed under Refactoring (DRY / tech debt), not Critical / Moderate blockers.@skills/refactor-entry-point-to-action/SKILL.md run with MODE=cr@rules/laravel/architecture.mdc)test:coverage / coverage script, or a direct vendor/bin/pest --coverage-clover=<file> / PHPUnit --coverage-clover invocation) and run it to produce a coverage report. Do not assume a default and do not add a new bespoke coverage script to the consuming project.--coverage-clover plus a path filter, PCOV pcov.directory scoped to the changed directories), narrow it so the run stays fast; otherwise generate the report and read off only the changed files.--coverage-clover output or any other auto-generated coverage artifact) as soon as it has been read, so it is never accidentally committed. Keep such artifacts in the project's .gitignore as a second line of defence.## Coverage section on the published comment only when there is something the reader must act on — uncovered changed lines (Critical findings) or unavailable / non-runnable coverage tooling (Critical finding with the reason). When every changed line is at 100% coverage and the tool ran successfully, omit the ## Coverage section entirely, omit the Coverage: header line, and omit the coverage … slot from the final summary line. The Counts line carries the clean signal; the omission is the report. Never emit 100% / clean / n/a placeholders for the section, the header line, or the summary slot.Run this step after every preceding analysis step has produced its findings (Core Analysis, Highest-Priority Fast Track, Named Arguments Review, Specialized Reviews, Refactoring & Tech Debt, Validation incl. the Coverage gate) and before the Output assembly. Walk every Critical finding aggregated within this skill's run through @skills/analyze-problem/SKILL.md to confirm the finding reflects a real problem in the diff before it blocks the PR.
@skills/analyze-problem/SKILL.md inline in this skill's context (do not dispatch as a subagent) and pass:
file:line, risk/impact line, Faulty Example, Expected Behavior, Test Hint, and Suggested Fix@skills/analyze-problem/templates/analysis-report.md):
Summary line are always rendered. The Coverage: header line, the ## Coverage section, and the coverage … slot in the summary line are all conditional — render them only when the coverage gate produced something to report (uncovered changed lines or unavailable / non-runnable tooling, both Critical findings). When every changed line is at 100% coverage and the tool ran successfully, drop all three coverage surfaces; the Counts line is the clean signal. Every other section — Findings (including each severity sub-heading), Refactoring (DRY / tech debt), Refactoring proposals, Database Analysis, and any specialized-review sub-section — appears only when it has at least one item. Never emit None. / Not applicable. / n/a / 100% placeholders for empty sections or omitted coverage surfaces; drop the whole heading and body instead. History across CR runs is preserved by the tracker's edit history on the upserted comment — never re-create a Previous CR Status section in the body.## Architecture section (issue #530). On Laravel projects the architecture walk-through described in Core Analysis runs on every CR run, but the ## Architecture heading is rendered only when the walk produces at least one finding. When findings exist, render them under the heading with the standard Critical / Moderate / Minor severity grouping and the reproducer fields. When the walk produces zero findings, omit the heading entirely — never render a walked, 0 findings status line, a clean placeholder, or any other "the check ran" confirmation. The principle is the same as for every other section: report only items that still need action; an empty section is dropped. On non-Laravel projects (laravel/framework not in composer.json require), the ## Architecture section is omitted entirely — the section is Laravel-only by design.@rules/php/core-standards.mdc and, for Laravel projects, @rules/laravel/architecture.mdc. Use n/a — <reason> only when a snippet adds no value over the one-line Fix description (e.g. naming-only changes, dead-code removal, pointers to an existing helper whose name already says enough).@skills/process-code-review/SKILL.md can convert each finding into a reproducer test and apply the fix without re-deriving context.@skills/code-review-github/SKILL.md, @skills/code-review-jira/SKILL.md) must delegate the single consolidated comment on every linked issue in the originating tracker (GitHub issue, JIRA ticket, or both) to @skills/pr-summary/SKILL.md — the CR wrappers never author their own non-technical template. pr-summary produces a uniform "Summary of changes + How to test" comment understandable by non-technical project managers, rendered as GitHub Markdown for GitHub issues and JIRA Wiki Markup for JIRA tickets per @rules/jira/general.mdc. When @skills/assignment-compliance-check/SKILL.md returns a markdown block (i.e. at least one Critical gap was detected), the CR wrapper passes it as an embedded block to pr-summary, which appends it after How to test so the linked-tracker audience reads exactly one comment per CR run (per issue #498). When the compliance check returns a skip status (clean or no linked tracker), the wrapper embeds nothing and the consolidated comment carries only the change summary — clean compliance is reported by the absence of the block. Technical findings still go directly on the PR comment.FormRequest::messages() / attributes(), custom validation messages on rules, exception messages surfaced to end users (abort(), throw new RuntimeException(...) reaching a JSON / Inertia / Blade response), Notification subject and body, Mailable bodies, flash messages, API error envelopes, Filament notifications, __() / trans() / Lang::get() / @lang / t() / i18next.t() calls in every locale shipped by the project (every key under lang/ / resources/lang/ / translations/ / *.json locale files), and every literal string rendered into Blade / Livewire / Filament / Vue / React templates — walk each string against @rules/security/backend.md Safe Validation & Error Messages and (for frontend / mobile surfaces) @rules/security/frontend.md / @rules/security/mobile.md Safe Validation & Error Messages, and raise one finding per match against any of these patterns:
404 Not Found envelope must cover both missing and forbidden)."Email '$email' is invalid") into the validation message or error body; the rule is to reference the field by name only.en.json says "Invalid credentials." but cs.json says "Účet neexistuje."). Walk every locale shipped by the project, not just the default. This is independent of the Translation completeness check — the wording finding fires even when the key is present in every locale.
Severity: Critical when the unsafe wording sits on an auth / password-reset / sign-up / authorization surface (the leak is directly exploitable for enumeration); Moderate on every other user-facing surface. The Suggested Fix rewrites the string to a generic, non-enumerating equivalent — "Invalid credentials.", "If the account exists, we sent the reset link.", "The email address is invalid." — routed through the project's translation function in every shipped locale (any locale the agent cannot translate confidently keeps the TODO(<locale>): translate "<source>" placeholder used by the Translation completeness check above). Specific, non-enumerating validation messages — "The age must be at least 18.", "Phone number is required.", "File must be a PDF under 5 MB." — are not findings; the gate fires on enumeration / authorization / introspection wording only.composer.json / package.json script hooks, and installer hooks against @rules/security/backend.md Malicious Code & Supply-Chain Indicators (and the @rules/security/frontend.md / @rules/security/mobile.md mirrors for client surfaces), and raise one finding per match against any of these patterns:
curl -s / wget -q fetching a payload, especially piped to an interpreter (curl … | sh / | bash / | php) or written to disk and executed.curl -k / --insecure, wget --no-check-certificate, CURLOPT_SSL_VERIFYPEER => false / CURLOPT_SSL_VERIFYHOST => 0, Guzzle 'verify' => false, NODE_TLS_REJECT_UNAUTHORIZED=0, or a trust-all certificate / hostname verifier.2>/dev/null / &>/dev/null on a security-relevant command, the PHP @ operator on a security-relevant call, error_reporting(0), or an empty catch {} that swallows the exception./tmp / /var/tmp / /dev/shm or a dot-prefixed filename combined with a detached / backgrounded process (&, nohup … &, setsid, disown, at / cron, detached proc_open).
Severity: Critical when the indicator maps to active RCE / MITM / persistence (silent fetch piped to a shell, TLS disabled on a credential-bearing request, both halves of the dropper pattern co-occurring); Moderate otherwise. The Suggested Fix replaces the unsafe construct with the allow-listed / checksum-verified / logged / queued equivalent from the rule. Documented benign uses (a -s curl in a doc example, a 2>/dev/null on a best-effort cleanup annotated inline) are not findings.lang/ directory (Laravel), a resources/lang/ directory (older Laravel layout), a translations/ directory (Symfony), *.json / *.po / *.mo locale files under a recognised locale directory, calls to __() / trans() / Lang::get() / @lang / t() / i18next.t() / useTranslation() / $t( / $tc( anywhere in the diff or in the wider project. If none of those signals match, skip this section entirely. When translations are in use, for every user-facing string introduced or modified by the diff — across all three surfaces named in @rules/laravel/laravel.mdc Localization and Translatable Strings: UI (labels in Blade / Livewire / Filament / Vue / React templates, validation messages, Notification subject and body, Mailable / Markdown views, FormRequest custom messages(), enum getLabel(), Filament resource / form field labels), Console (human-readable Artisan command output — $this->info() / error() / warn() / line() / comment(), ask() / confirm() / choice() prompts, table headers), and API (JSON message fields, abort() / abort_if() descriptions, human-readable strings in API Resources, exception messages surfaced to users), plus log messages exposed to humans — walk every detected locale and raise one finding per missing key — that is, the diff added a translation key in the default locale but did not add (or updated but did not synchronise) the same key in every other locale shipped with the project, or the diff hard-codes a literal string in a user-facing surface even though the surrounding code uses __() / t() for analogous strings. Severity: Moderate; escalate to Critical when the missing key is on a primary-flow surface (login, checkout, primary CRUD form, transactional email) or when the project's CI / build asserts translation parity and the diff would break that gate. Each finding cites the missing locale and key, and the Suggested Fix is the literal translation entry the locale file is missing (translated into the locale's language; for languages the agent cannot translate confidently, leave the value as TODO(<locale>): translate "<source>" and flag the gap explicitly so a human translator picks it up).*.php under the project's test root — tests/, *Test.php, Pest it() / test() files), scan for code paths that can reach the real network or spawn a real OS process, per @rules/laravel/laravel.mdc Testing. Two independent gates, both Critical (a test that escapes its sandbox is non-deterministic and a security risk — it can hit production endpoints, leak credentials, or execute arbitrary host commands during CI):
Http:: client, Guzzle, cURL, file_get_contents() against a URL, a vendor SDK that calls out) without a registered fake/mock: no Http::fake() (or Http::preventStrayRequests()), no Guzzle MockHandler, no injected fake client. A test that can reach a real endpoint is the finding even when the call happens to be stubbed at the assertion layer but the transport is still live.Process::run() / Process::start() (Laravel) without Process::fake(), Symfony\Component\Process actually executing, or any of exec(), shell_exec(), system(), passthru(), proc_open(), popen(), or backtick operators running a host command. A test must never invoke an external binary or script directly on the system — calling out to git, node, npm, composer, ffmpeg, docker, aws, a .sh / .py / .bin script, or any other host executable through a test is a finding on the spot; the process layer must be mocked / faked (Process::fake([...])) so the test asserts the intended command line without ever executing it. The only non-finding is the project's own Artisan command driven through Artisan::call() / $this->artisan(); every external binary or shell command is flagged.
For each match cite file:line, name which gate fired, and give a Suggested Fix that registers the missing fake (Http::fake([...]) / Process::fake([...])) or injects a test double so the test asserts the intended request / command without executing it. Documented, explicitly-isolated integration tests that the project marks as such (a dedicated tests/Integration group gated behind an env flag / CI stage and annotated in code) are exempt — cite the annotation in the finding instead of raising it. Http::fake() / Process::fake() / Artisan::call() are the Laravel-specific examples; on a non-Laravel PHP project apply the same two gates with the framework's equivalent isolation — Guzzle MockHandler or an injected fake HTTP client, a Symfony Process test double, or any injected double — and phrase the Suggested Fix in that project's vocabulary.@skills/mysql-problem-solver/SKILL.md trigger fires (see Specialized Reviews for the trigger pattern list), append a dedicated ## Database Analysis section to the published review before the ## Coverage section. The section reports only the findings — each with severity on the Critical / Moderate / Minor scale and the proposed query rewrite / index reuse / batching fix per @rules/sql/optimalize.mdc. Do not include the trigger decision, the inspected file:line list, or the EXPLAIN / static-analysis summary; those belong to the internal mysql-problem-solver investigation, not to the published review. When no DB operations are present in the diff, omit the section entirely; never replace it with a generic "no DB changes" placeholder absorbed into Coverage or the summary line.templates/review-output.md.@skills/test-like-human/SKILL.md. The user-perspective testing skill runs on demand only (via /test-like-human or an explicit follow-up); CR-track skills must never chain into it.