Code review jira

Code Review (JIRA)

Purpose

Perform code review for JIRA issues by analyzing related pull requests and publishing results to:

• GitHub (technical findings)
• JIRA (human-readable summary)

---

Constraints

• Apply @rules/jira/general.mdc
• Apply @rules/git/general.mdc
• Apply @rules/reports/general.mdc. The GitHub PR technical comment this skill posts (Status / Counts / Findings / Refactoring / Database Analysis / Coverage / Summary) stays in canonical English per the rule's Exception — technical CR findings on the GitHub PR. The JIRA comment delegated to @skills/pr-summary/SKILL.md and the mirrored linked-GitHub-issue summary follow the language of the source JIRA assignment. Never mix languages inside the same comment; never use bilingual Kritické (Critical) style parentheses.
• Read-only skill — never modify code, never stage / commit / push changes, and never run any git write operation (git add, git commit, git push, git reset, git checkout -- …, etc.). Checking out the relevant branch and git pull to read the latest code are required (the mandatory Branch checkout gate below); mutating the working tree or pushing to the remote is not. Publishing is limited to PR / linked-issue comments via gh and to JIRA ticket comments via acli.
• JIRA output must be understandable for non-developers: only how to test the change, plus — when they exist — clarifying questions, assignment discrepancies, and Critical items. Nothing else. It is rendered in JIRA Wiki Markup with no leaked Markdown control characters (no raw **, #, `, - ).
• Output findings only (no praise)

---

Execution

1. Load Context

• Load JIRA context by running skills/code-review-jira/scripts/load-issue.sh <KEY|URL> — the single deterministic entry point. Never call acli directly. Read issue header, description, comments, attachments, subtasks, issue links, custom fields, devSummary, and pullRequests off the resulting JSON document.
• The script accepts a bare key (ECOMAIL-1234), a /browse/<KEY> URL, or any URL containing ?selectedIssue=<KEY>.
• For a single ready-to-read context brief — the issue plus its comments, attachments, recursively-loaded linked issues, and an inventory of external URLs, rendered as Markdown — run skills/code-review-jira/scripts/gather-issue-context.sh <KEY|URL> instead of hand-assembling the JSON. To read only the comments as a structured array, use skills/code-review-jira/scripts/parse-comments.sh <KEY|URL>. Both build on load-issue.sh, so the same exit codes and MCP fallback apply. Attachment content and the inventoried URLs are not fetched by the scripts (acli cannot download them) — read them with your own tools when a finding depends on them.
• If the script is unavailable (missing tool, exit code 2/3) fall back to the JIRA MCP server. Always prefer the MCP fallback for data the script cannot cover: changelog (expand=changelog), available next transitions, and friendly custom-field names (expand=names).
• Identify all open PRs linked to the issue from the script's pullRequests array
• Branch checkout gate (mandatory, always). Before running any review step, check out the PR branch (the head branch of the linked GitHub PR) and pull the latest commits — git fetch origin, git checkout <branch>, git pull — so the review always runs against the actual current codebase on disk (the checked-out working tree), never against the remote diff in isolation. Confirm local HEAD equals the PR head SHA. If the checkout fails (missing ref, detached HEAD, or local changes that would be overwritten), stop and report it instead of reviewing from the diff. Every sub-review then reads the checked-out files.

Issue Context Analysis

Before reviewing code, load and analyze the full JIRA issue:

1. Fetch the complete JIRA issue — description, all comments, and all attachments (screenshots, files, embedded data).
2. Extract from the issue:
   • Requirements and acceptance criteria — what the code must do
   • Expected behavior — how the feature or fix should work
   • Edge cases and constraints — mentioned by the reporter or in comments
   • Test data — any sample inputs, payloads, or scenarios provided in the issue
3. Use this context to evaluate whether the implementation fully satisfies the issue — not just whether the code is technically correct.
4. If the issue contains test data or test scenarios, verify they are covered by existing or new tests. Flag missing test coverage as a finding.

Reviewer Comment Fulfillment Gate (mandatory)

Run the Reviewer Comment Fulfillment Gate defined canonically in @skills/code-review-github/SKILL.md against the GitHub PR linked to this JIRA issue — that is where this skill publishes technical CR findings and where reviewer comments and line-anchored review threads live. After loading all PR comments, verify each actionable reviewer instruction is satisfied by the current checked-out diff (the applied change corresponds to what the reviewer asked for), raise one Critical finding per not-fulfilled instruction on the GitHub PR comment with the four reproducer fields, and record the reviewer comments: M/N fulfilled verdict on the GitHub PR comment summary line. The JIRA non-technical comment never carries this gate's findings.

2. Pre-checks

• CI coverage of checks. Load the GitHub PR JSON via skills/code-review-github/scripts/load-issue.sh <PR-URL> (if not already loaded) and read statusCheckRollup[]. Identify which checks ran on the PR head commit (headRefOid) and their result. Pass this CI check map to the Coverage gate decision in @skills/code-review/SKILL.md (Validation → Coverage gate; the Reuse-CI-results detail now lives in @rules/code-review/general.mdc Validation & Coverage Gate) so only missing or non-green checks are run locally.
• If PR has conflicts → skip review for that PR

3. Run Reviews

Inline dispatch. Each sub-review below runs inline in this wrapper's context — invoke each skill directly (@skills/<name>/SKILL.md with any MODE=cr flag), passing the PR URL / number and the branch already checked out, and declare the publishing contract for this CR run (quiet vs publish; see step 4). Each invoked skill must return its findings as the canonical markdown block (## Assignment Compliance block, Critical / Moderate / Minor lists with reproducer fields, refactoring proposals). The CR wrapper then assembles the outputs into the final GitHub PR comment + JIRA / linked-issue summary. Run the sub-reviews one at a time — do not dispatch them as parallel subagents.

The mysql-problem-solver / race-condition-review / refactor-entry-point-to-action conditionals follow the same rule: when their trigger fires, invoke them inline after the always-run set, still one at a time.

• For each PR (sub-reviews invoked inline, one at a time):

  • run @skills/assignment-compliance-check/SKILL.md — builds the Functional review (@rules/code-review/general.mdc Two-part CR output): non-technical, full acceptance-criteria checklist against the assignment. The skill does not publish anywhere itself — it returns either the assembled ## Assignment Compliance markdown block, rendered on every run that has a linked tracker (including the affirmative Goal met: Yes checklist on a clean run; the wrapper converts it to JIRA Wiki Markup before passing it to pr-summary for the JIRA target, and keeps GitHub Markdown for the linked-GitHub-issue mirror), 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 tracker (JIRA ticket, linked GitHub issue) receives one consolidated comment per CR run (per issue #498); on the skip status the wrapper embeds nothing. Do not embed the block into the GitHub PR comment — the PR comment carries the Technical review only.
  • run @skills/code-review/SKILL.md
  • run @skills/analyze-problem/SKILL.md — always run, scoped to assignment conformance, invoked inline and read-only (analysis-only — no plan artifact, no code / git writes). Compares the loaded JIRA requirements / acceptance criteria / expected behavior against what the PR diff actually implements and raises every unmet requirement as a Critical finding with reproducer fields on the GitHub PR comment. Canonical definition lives in @skills/code-review/SKILL.md Specialized Reviews → Always run; it is distinct from the per-Critical-finding verification (issue #537) and must not duplicate gaps already raised by assignment-compliance-check.
  • run @skills/security-review/SKILL.md
  • run @skills/class-refactoring/SKILL.md with MODE=cr — read-only refactoring lens scoped to the PR diff, run on every CR. The lens walks the skill's complete guideline set; every item it returns is rendered in the published GitHub PR comment, routed and de-duplicated per @rules/code-review/general.mdc Refactoring & Tech Debt (DRY) Analysis — diff-scoped detail. The JIRA non-technical comment never carries these technical sections. Canonical definition of the lens invocation lives in @skills/code-review/SKILL.md Specialized Reviews → Always run. MODE=cr guarantees no code changes, commits, fixers, or review chaining.

• Run conditionally:

  • Diff is a refactoring (behavior-preserving structural change per @rules/refactoring/general.mdc) → run the full refactoring skill set read-only. When the PR restructures existing code without adding a feature or changing observable behavior, additionally invoke @skills/refactor-entry-point-to-action/SKILL.md with MODE=cr to surface the entry-point → Action proposals. Both refactoring 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 Reduction) section (in-scope) and Refactoring Proposals section (out-of-scope) of the GitHub PR comment. The JIRA non-technical comment never carries these technical sections.
  • Database operations detected in the diff → @skills/mysql-problem-solver/SKILL.md is mandatory. Trigger pattern list is owned by @skills/code-review/SKILL.md Specialized Reviews (raw SQL, Eloquent / query-builder calls, eager loads, model scopes, ModelManager / Repository methods, migrations, seeders, DynamoDB / NoSQL access). Capture its findings and surface them on the GitHub PR comment under the dedicated ## Database Analysis section (see Output Rules) — never silently fold them into the Critical / Moderate / Minor buckets. The JIRA non-technical comment does not carry this section (it stays plain-language via pr-summary).
  • Shared state → @skills/race-condition-review/SKILL.md
  • Third-party API or service changes → ensure the Third-Party API & Service Analysis step from @skills/code-review/SKILL.md is executed for the diff, including its mandatory documentation verification (locate the docs or request the link — never assess from memory)

Refactoring & Tech Debt (DRY) Analysis (PR diff only)

1. Restrict the analysis to lines added or modified in the PR — never review untouched code.
2. For each changed block, apply @skills/class-refactoring/SKILL.md (run with MODE=cr — read-only), walking that skill's complete guideline set. The walked guidelines, the per-item routing, and the de-duplication + no-drop contract are owned by @rules/code-review/general.mdc Refactoring & Tech Debt (DRY) Analysis — diff-scoped detail — apply it as written; do not narrow the walk to a subset and do not restate the guideline list here. The duplicated-logic half of the walk runs through the reuse-first gate in @rules/code-review/general.mdc Reuse Existing Logic — first decide whether the new logic is necessary at all, then whether an existing implementation must be reused instead of a parallel one.
3. Each finding must include the file path, the affected line range, a concrete refactoring that reduces tech debt, and the guideline it matched.
4. In-scope refactorings go into the Refactoring (DRY / Tech Debt Reduction) section of the GitHub PR comment template. Out-of-scope structural problems still belong in Refactoring Proposals; an item whose underlying rule declares a severity goes to that Critical / Moderate / Minor bucket instead, with the four reproducer fields.

4. Publish Results

Quiet mode (loop iterations from @skills/process-code-review/SKILL.md): when the caller explicitly requests "do not publish; return findings as in-memory markdown for this loop iteration only", skip all publishing below — no GitHub PR comment, no JIRA comment, no linked-GitHub-issue mirror. Return the assembled review markdown to the caller and stop. Only the final (publishing) call from process-code-review after convergence runs Publish Results in full.

GitHub (technical findings only — always-new comment per CR run)

• Every CR run posts a fresh PR comment. The helper never edits a prior comment in place — each run produces its own self-contained entry so reviewers see one comment per run, in chronological order. The hidden marker <!-- cr-comment:actor=<gh-login> --> is still appended to the body for traceability (auto-appended by the helper) but no longer drives an upsert lookup. History across runs lives in the comment sequence itself; never re-create a Previous CR Status section in the body.
• Publish via skills/code-review-github/scripts/upsert-comment.sh <PR-NUMBER|URL> - (body on stdin). The helper detects the current actor (gh api user --jq .login), appends the marker, and POSTs a new comment. The published URL is emitted on stdout; the action (created) on stderr — log it in the PR comment summary line.
• If the helper exits with code 2 (missing tool) or 3 (API failure), fall back to the GitHub MCP server's addIssueComment — also as a fresh post. Never call updateIssueComment to edit a prior CR comment and never quote / reply to one; the always-new-comment convention replaces the previous in-place edit flow.
• Format inside the comment body:
  • Critical → Moderate → Minor → Refactoring (DRY / Tech Debt Reduction)
  • file + line
  • actionable fix
• Post all technical findings inside the single PR comment — never as line-anchored review comments. Include the file:line reference in the body of each finding instead.
• This is the only place where technical details appear.

JIRA (consolidated non-technical comment — fresh comment per CR run)

• Delegate the JIRA comment to @skills/pr-summary/SKILL.md. This CR skill must not author its own JIRA summary. The JIRA non-technical comment carries only How to test plus, when they exist, two conditional blocks: a Clarifying questions block and an Assignment Compliance block (assignment discrepancies + Critical items). No Authors line, no Summary of changes section, no severity counts, no file paths — pr-summary renders this reduced JIRA shape from @skills/pr-summary/templates/pr-summary-jira.md.
• Clarifying questions block (conditional). While running the sub-reviews, collect every genuine open question the reviewer needs answered before the work can be accepted — an ambiguity in the assignment that the issue description, comments, and code could not resolve (a missing acceptance criterion, an undefined edge case, a value the assignment never specified, a contradiction between the ticket and a comment). When at least one exists, assemble a h2. Clarifying questions block in JIRA Wiki Markup (one * bullet per question, each a single plain-language sentence) and pass it as an embedded block to pr-summary so it renders after How to test. When there are none, pass nothing — never emit an empty "no questions" block. Do not invent questions to fill the section; ask only what genuinely blocks acceptance.
• Consolidation contract (issue #498): invoke pr-summary exactly once for the JIRA ticket, passing the conditional blocks together (clarifying questions first, then assignment compliance) so the reader gets one comment per CR run. @skills/assignment-compliance-check/SKILL.md returns the Functional review block — the full acceptance-criteria checklist with a Goal met: Yes/No verdict — on every run that has a linked tracker, including the affirmative report when every criterion is Met; convert that block to JIRA Wiki Markup per @rules/jira/general.mdc (## → h2. , **bold** → *bold*, `code` → {{code}}, - → * , Markdown link [label] + (url) → [label|url]) and pass it as an embedded block — pr-summary appends it verbatim after How to test. Only when assignment-compliance-check returns the no linked issue — assignment compliance skipped status does the wrapper skip passing that block. pr-summary then POSTs a new comment via skills/code-review-jira/scripts/upsert-comment.sh (JIRA MCP server fallback on exit code 2/3). Every CR run creates a fresh JIRA comment — the always-new comment convention means the chronological sequence of JIRA comments is the audit trail across runs, consistent with GitHub. Never post a separate JIRA comment for clarifying questions or assignment compliance on top of it.
• When invoking pr-summary, pass through any test-parameter gating detected in the diff (feature flag, ENV switch, query-string parameter, request header, admin toggle, allow-list) so the first How to test step enables the toggle before the tester proceeds. The JIRA comment does not carry a separate Available behind line — the gating lives inside the test steps.
• Invoke @skills/pr-summary/SKILL.md with the JIRA tracker target so it renders @skills/pr-summary/templates/pr-summary-jira.md in JIRA Wiki Markup and upserts the comment on the originating JIRA ticket through the helper above (no direct acli jira workitem comment add calls that bypass the marker).
• Verify the published JIRA body contains no leaked Markdown. Before / when publishing, confirm the body uses only Wiki Markup — no ** / __, no #/## ATX headings, no `/```, no - bullets, no Markdown [label] + (url) links. Any such artifact must be converted per @rules/jira/general.mdc so the JIRA UI never shows raw markup characters.
• Never post file paths, line numbers, code snippets, technical severity levels, or finding counts to JIRA — pr-summary already enforces this by design, and the embedded blocks obey the same rule. Technical content stays exclusively on the GitHub PR comment.
• When the CR run yields Critical / Moderate findings that block merge, surface that signal in the GitHub PR comment summary line; the consolidated JIRA comment stays focused on how to test the change, plus any clarifying questions and assignment discrepancies / Critical items.

Linked GitHub issues (consolidated mirror — always-new comment per CR run)

• If the reviewed PR also references a GitHub issue (i.e. closingIssues[] of the GitHub PR JSON is non-empty), delegate the linked-GitHub-issue comment to @skills/pr-summary/SKILL.md (GitHub tracker target). The skill renders @skills/pr-summary/templates/pr-summary-github.md in GitHub Markdown and posts it via skills/code-review-github/scripts/upsert-comment.sh on each entry in closingIssues[] (one fresh comment per linked issue per CR run — marker <!-- cr-comment:actor=<gh-login> --> appended for traceability, no in-place edit). Pass the same author + test-parameter-gating context as the JIRA invocation above — the mirrored comment must carry the same Authors and Available behind lines.
• Consolidation contract (issue #498): @skills/assignment-compliance-check/SKILL.md returns the Functional review block on every run that has a linked tracker (including the affirmative report on a clean run); pass its GitHub-Markdown version as an embedded block so the linked-GitHub-issue audience also sees one consolidated comment per CR run instead of two separate posts. Only when the compliance check returns the no linked issue — assignment compliance skipped status does the wrapper skip passing an embedded block. Follow-up runs add a new comment rather than editing the previous one — the chronological sequence is the audit trail.
• The JIRA-side summary is the primary tracker comment; the GitHub-issue comment is a courtesy mirror so reviewers reading the GitHub issue see the same "Summary of changes + How to test + Assignment Compliance" output without opening JIRA. Both comments come from pr-summary, so they are guaranteed to match.
• If closingIssues[] is empty, skip this block and note "no linked GitHub issue — mirror skipped" in the PR comment summary line.
• If the upsert helper or the GitHub MCP fallback returns a permission error (cross-repo issue, lacking write access), log the failure in the PR comment summary line and continue — do not abort the review.

---

Output Rules

GitHub (technical report — only here)

• All technical findings go exclusively to GitHub PR comments
• Include: file paths, line numbers, code references, severity levels, concrete fixes
• Findings only — no praise, no explanations of what was checked
• Omit empty sections entirely. Only the header block (Status / Counts / Last updated / Linked-tracker mirror) and the final Summary line are always rendered in the GitHub PR comment. 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, and Database Analysis — 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 chronological sequence of always-new GitHub PR comments — never re-create a Previous CR Status section in the body.
• ## Architecture section (issue #530). On Laravel projects (laravel/framework is in composer.json require), the architecture walk-through defined in @skills/code-review/SKILL.md Core Analysis runs on every CR run, but the ## Architecture heading is rendered on the GitHub PR comment only when the walk produces at least one finding. When findings exist, render the heading and list them. When the walk is clean, omit the heading entirely — never render a walked, 0 findings status line, a clean placeholder, or any other confirmation that the check ran. On non-Laravel projects, omit the ## Architecture section entirely. The JIRA non-technical comment (produced by pr-summary) never includes this section.
• Use severity levels: Critical, Moderate, Minor
• Each Critical and Moderate finding must include:
  • Faulty Example — minimal code snippet or input payload reproducing the issue (redact secrets/PII)
  • Expected Behavior — single assertable statement (return value, exception, persisted state, emitted event)
  • Test Hint — one sentence pointing at the test layer (unit, integration, feature) and entry point
  • Suggested Fix — minimal corrected code snippet that resolves the finding. Must comply with @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).
• These four fields exist so @skills/process-code-review/SKILL.md can convert each finding into a reproducer test and apply the fix directly from the PR comment.
• Minor findings may omit these fields when no behavior change is implied.
• When the diff touches database operations (per the trigger list in @skills/code-review/SKILL.md Specialized Reviews), the posted GitHub PR comment must include a dedicated ## Database Analysis section before ## Coverage. The section reports only the mysql-problem-solver findings (with severity mirroring Critical / Moderate / Minor), each carrying a Suggested Fix that renders the concrete optimization artifact — the rewritten query in full, the exact index DDL, or the rewritten batching code in a fenced code block — or, when the slowdown is the justified branch, the three-part documentation block that is the artifact there. Naming the fix category in prose does not satisfy the requirement on its own, per @rules/code-review/general.mdc Database Analysis section and @rules/sql/optimalize.mdc. Each DB-performance defect appears here exactly once and is never duplicated into the Critical / Moderate / Minor buckets; a security finding on the same file:line is a different defect and always keeps its own entry in the severity buckets. Do not include the queries / migrations inspected list or any EXPLAIN / static-analysis summary — those stay inside the internal investigation. When no DB operations are present, omit the section entirely. The JIRA non-technical comment (produced by pr-summary) never includes this section.
• The posted PR comment includes a ## Coverage section before the summary line only when the coverage gate has something to report — uncovered changed lines (Critical findings) or unavailable / non-runnable coverage tooling (Critical finding). When every changed line is at 100% coverage and the tool ran successfully, omit the ## Coverage section, the Coverage: header line, and the coverage … slot from the summary line per @skills/code-review/SKILL.md Output Rules. The coverage gate itself (per the Coverage gate in @skills/code-review/SKILL.md) still runs on every review; only the user-visible section is short-circuited.
• Use the template defined in templates/github-output.md

JIRA (non-technical summary — only here)

• The non-technical JIRA comment is produced and posted by @skills/pr-summary/SKILL.md, not by this skill. Invoke pr-summary with the JIRA tracker target; do not author or embed a custom template here.
• The JIRA comment carries only How to test, plus the two conditional embedded blocks when they apply — Clarifying questions (open questions the reviewer needs answered) and Assignment Compliance (assignment discrepancies + Critical items). It carries no Authors line, no Summary of changes section, no severity counts, no file paths, no line numbers, no code snippets — pr-summary enforces this reduced shape by design. Plain language understandable by non-developers.
• The JIRA Wiki Markup conversion (h2. / h3. headings, *bold*, _italic_, {{inline}}, {code:php} ... {code}, * / # bullets, [label|url], {quote}) is handled by @skills/pr-summary/templates/pr-summary-jira.md per @rules/jira/general.mdc. Do not "translate" the output back to GitHub Markdown when posting via acli / JIRA MCP server, and never let a raw Markdown control character (**, #, `, - ) reach the published comment — the JIRA UI would show it literally.

---

Principles

• Focus on risks, not style
• Prefer impact over quantity
• Avoid duplication of findings
• Prioritize regression detection
• Be precise and actionable

---

After Completion

• Do not auto-invoke @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.