code-review

Reading workflow files (TOC protocol)

When you Read any file under .claude/workflow/ or .claude/skills/, follow the protocol in conventions.md §1.8:

1. Read the TOC region: from <!--Document index start--> to <!--Document index end--> (read to the closing delimiter, not a fixed line count). If the file has no TOC region (a file whose only ## heading is this bootstrap block carries none, per §1.8(d)), read the file in full.
2. Match TOC rows where Roles contains any of your roles (or your role is any, or the row's Roles is any) AND Phases contains any of your phases (or your phase is any, or the row's Phases is any).
3. Use Read(offset, limit) to read only matched sections; if no row matches your role/phase, the file holds nothing for you — do not read further.

Your role: orchestrator. Your phase: 3B or 3C (the review phase that invoked this skill).

Inline refs you find inside workflow files carry the same name:roles:phases suffix; apply file-level filtering before opening: a ref matches when any of your roles is in its roles and any of your phases is in its phases, your own any on either axis matches every ref on that axis, and a ref whose own roles or phases is any matches you. Backtick-wrapped refs carry no suffix; open or skip them at your discretion.

Section	Roles	Phases	Summary
§Step 1: Determine what to review	orchestrator	3B,3C	Resolve the review target from the argument or, when empty, from the branch's PR or its commits ahead of develop.
§If $ARGUMENTS is provided:	orchestrator	3B,3C	Six argument shapes: branch, commit range, last-N-commits, uncommitted, PR ref, or an unparseable input.
§If $ARGUMENTS is empty:	orchestrator	3B,3C	Fall back to the branch's open PR, then to commits ahead of develop, then ask the user when neither resolves.
§Step 2: Detect the base branch	orchestrator	3B,3C	Pick the comparison base: a PR's baseRefName, develop for a bare branch, or none for a range or uncommitted review.
§Step 3: Gather the review context	orchestrator	3B,3C	Collect changed files and commit log, and write the full diff to a unique /tmp file each agent reads on demand.
§For branch or PR review:	orchestrator	3B,3C	The git diff and log commands that build the changed-file list, diff file, and commit log for a branch or PR target.
§For commit range:	orchestrator	3B,3C	The git diff and log commands for a commit-range review target.
§For uncommitted changes:	orchestrator	3B,3C	The git diff commands for an uncommitted-changes target, with the no-commit-history sentinel for the log.
§PR description (if available):	orchestrator	3B,3C	The gh command that reads the PR body for context, when a PR is associated with the target.
§Step 4: Filter non-reviewable files	orchestrator	3B,3C	List the generated-code paths to skip and pass the filter note to every dispatched agent.
§Step 5: Triage — categorize changes and select relevant agents	orchestrator	3B,3C	Categorize each changed file, map categories to review agents, and log the triage decision before dispatch.
§5a: Categorize each changed file	orchestrator	3B,3C	The category table mapping file signals to domains; a file may carry several categories.
§5b: Map categories to agents	orchestrator	3B,3C	Launch rules for the 16 review agents across code, test, and workflow groups, keyed on detected categories.
§5c: Log your triage decision	orchestrator	3B,3C	Print the triage summary naming detected categories, selected agents, and skipped agents before launching.
§5d: Edge cases	orchestrator	3B,3C	Last-matching-row resolution for category combinations: workflow-only, docs-only, build-config, tests, mixed.
§Step 6: Dispatch selected review agents	orchestrator	3B,3C	Launch selected agents in parallel with a scope-filtered file list and the group-specific prompt template.
§Prompt template — code-review and test-review groups	orchestrator	3B,3C	The prompt body for the code and test review groups, including the PSI-over-grep tooling rule for Java symbols.
§Prompt template — workflow-review group	orchestrator	3B,3C	The prompt body for the workflow-review group, noting PSI does not apply to markdown, shell, and JSON files.
§Handling missing fields	orchestrator	3B,3C	Treat the missing-PR and uncommitted sentinels as no signal; do not infer requirements from their absence.
§Step 7: Synthesize the results	orchestrator	3B,3C	Map sub-agent severities, deduplicate, attribute, and summarize into one unified report, not a concatenation.
§Handling agent failures and empty output	orchestrator	3B,3C	Record failed reviewers, propagate preface notes, emit All clear when empty, and remove the temp diff file.
§Output format	orchestrator	3B,3C	The synthesized report layout: failed reviewers, assessment, blockers, should-fix, suggestions, notes, and questions.
§Important rules	orchestrator	3B,3C	Standing rules: gh CLI for GitHub, parallel dispatch, no self-added findings, no severity softening, large-diff abort.

Review code, test, and workflow-machinery changes across multiple dimensions by dispatching to specialized review agents and synthesizing their findings. Production code, test code, and workflow files (skills, agents, hooks, settings, prompts, CLAUDE.md, plan/design artifacts) are reviewed in one pass with triage-driven agent selection.

Use $ARGUMENTS as the review target if provided (branch name, commit range, or "uncommitted").

Step 1: Determine what to review

If $ARGUMENTS is provided:

1. Branch name (e.g., ytdb-605-unified-edges): Review all changes on that branch that are absent from the base branch.
2. Commit range (e.g., abc123..def456): Review that specific range.
3. "last N commits" (e.g., last 3 commits): Review HEAD~N...HEAD.
4. "uncommitted" or "working tree": Review uncommitted changes (git diff HEAD).
5. PR number or URL (e.g., #42 or https://github.com/.../pull/42): Fetch PR details and review its diff.
6. None of the above (malformed input, non-existent branch, unresolvable PR, free-form phrase): Report what you tried to parse, then ask the user for a valid target. Do not guess.

If $ARGUMENTS is empty:

1. Check if the current branch has an open PR:

   gh pr list --head $(git branch --show-current) --json number,title,body,baseRefName,url --limit 1
   

2. If a PR exists, use it as the review target (the PR's base branch becomes the comparison base).
3. If no PR exists, check if the current branch differs from develop:

   git log develop..HEAD --oneline
   

4. If there are commits ahead of develop, review those.
5. If the branch IS develop or has no commits ahead, ask the user what to review. Treat the user's reply as if it were $ARGUMENTS and restart Step 1.

Step 2: Detect the base branch

The base branch determines what "new changes" means:

1. If reviewing a PR: use the PR's baseRefName (fetched via gh pr view).
2. If reviewing a branch (no PR): default to develop.
3. If reviewing a commit range or uncommitted changes: no base branch needed.

Step 3: Gather the review context

Based on the review mode, collect the changed-file list and commit log inline, but write the full diff to a temp file so each agent can Read it on demand instead of receiving it interpolated into its prompt. This keeps per-agent prompt size bounded and removes the diff-size ceiling that inline interpolation would impose.

Use a unique /tmp filename to avoid collisions with concurrent Claude Code agents on the same host (per the user-global rule). Generate the suffix once and reuse it for the whole dispatch:

# Generate a unique suffix for this review's temp file
DIFF_FILE=/tmp/claude-code-review-diff-$$.txt   # or use $(uuidgen)

For branch or PR review:

git diff {base}...HEAD --name-only           # → CHANGED_FILES
git diff {base}...HEAD > "$DIFF_FILE"        # → DIFF_FILE
git log {base}..HEAD --oneline               # → COMMIT_LOG

For commit range:

git diff {start}..{end} --name-only          # → CHANGED_FILES
git diff {start}..{end} > "$DIFF_FILE"       # → DIFF_FILE
git log {start}..{end} --oneline             # → COMMIT_LOG

For uncommitted changes:

git diff HEAD --name-only                    # → CHANGED_FILES
git diff HEAD > "$DIFF_FILE"                 # → DIFF_FILE

For uncommitted changes there is no commit log; set COMMIT_LOG to the literal sentinel "(uncommitted changes — no commit history)".

PR description (if available):

gh pr view {number} --json body --jq '.body'

Store the collected context:

• DIFF_FILE — absolute path to the temp file containing the full diff (e.g., /tmp/claude-code-review-diff-12345.txt). Each agent reads this file via the Read tool.
• CHANGED_FILES — the list of changed file paths
• COMMIT_LOG — the commit history, or the uncommitted-changes sentinel above
• PR_DESCRIPTION — the PR body text, or the literal string "No PR associated with these changes." if absent
• REVIEW_SCOPE — human-readable description of what's being reviewed (e.g., "Branch ytdb-605-unified-edges vs develop (15 commits, 23 files)")

The temp file is ephemeral — remove it at the end of Step 7 (synthesis) so orphans don't accumulate. The unique suffix prevents collision with concurrent agents while the review is in flight.

Step 4: Filter non-reviewable files

Before dispatching, note files that should be skipped:

• Files under core/src/main/java/com/jetbrains/youtrackdb/internal/core/sql/parser/
• Generated Gremlin DSL classes
• Files under generated-sources/ or generated-test-sources/

Include this filter note in the context passed to agents.

Step 5: Triage — categorize changes and select relevant agents

Before dispatching agents, perform a quick triage pass over the entire diff (both production and test code) to determine which review dimensions are actually relevant. This avoids wasting time on agents that have nothing meaningful to review.

5a: Categorize each changed file

Scan the diff and assign one or more categories to every changed file — production code, test code, and other files alike:

Category	Signals
storage-engine	Files in storage/, cache/, wal/, StorageComponent subclasses, page read/write logic, DiskStorage, WriteCache, ReadCache, LogSequenceNumber, double-write log
concurrency	synchronized, Lock, Atomic*, volatile, StampedLock, ReentrantLock, thread pools, ConcurrentHashMap, CompletableFuture, shared mutable state, @GuardedBy, ConcurrentTestHelper, CountDownLatch, CyclicBarrier
index-data-structures	Files in index/, B-tree, hash index, SBTree, CellBTree, histogram, IndexEngine
network-server	Files in server/, driver/, Gremlin Server, protocol handling, TLS/SSL, authentication, session management
sql-query	Files in sql/ (excluding parser/), query execution, command handlers, SELECT/INSERT/UPDATE/DELETE logic
gremlin	Files in gremlin/, traversal steps, YTDBGraph* classes, TinkerPop integration
public-api	Files in com.jetbrains.youtrackdb.api, YourTracks, YouTrackDB interface
serialization	Record serializers, binary format, property map encoding/decoding
crash-durability	WAL operations, crash simulation, durable StorageComponent recovery, page corruption handling, transaction atomicity under failure, LogSequenceNumber manipulation, double-write log, Java assert statements in production code
configuration	GlobalConfiguration, config parameters, system properties
tests-only	Changes exclusively in test files with no production code changes
build-config	pom.xml, CI workflows, Maven profiles, Docker configs
workflow-machinery	Files under .claude/ (skills, agents, hooks, scripts, settings, workflow rules, workflow prompts, output styles, docs), project root CLAUDE.md, all files under docs/adr/<dir>/ (plan/design artifacts in _workflow/ and the durable design-final.md / adr.md)
docs-only	Markdown documentation under docs/ excluding docs/adr/<dir>/, plus comments-only changes
other	Files matching no category above (e.g., .gitattributes, miscellaneous root files). Triaged as a no-op — no agents dispatch on this category.

A file can belong to multiple categories (e.g., a lock change in storage code is both storage-engine and concurrency). Production and test files in the same domain should share the same categories. workflow-machinery is exclusive with docs-only: any file under .claude/ or docs/adr/<dir>/ is workflow-machinery; anything else under docs/ is docs-only.

5b: Map categories to agents

There are 16 specialized review agents in three groups:

Code-review agents (review production code):

Agent	Launch when ANY of these categories are present
review-code-quality	Always launched (unless docs-only is the ONLY category)
review-bugs	Always launched (unless docs-only or build-config are the ONLY categories)
review-concurrency	concurrency is present on any changed file
review-crash-safety	crash-durability
review-security	network-server, public-api, sql-query, serialization, configuration, OR when new dependencies are added in pom.xml
review-performance	storage-engine, index-data-structures, concurrency, serialization, sql-query, gremlin

review-bugs owns every defect findable by single-threaded sequential reasoning (logic, null safety, resource leaks, RID handling, state-machine / lifecycle); review-concurrency owns every defect whose detection needs reasoning about two or more threads interleaving (races, visibility / publication, lock-ordering / deadlock, compound-op atomicity). When review-bugs, reasoning sequentially, meets concurrent-looking code that review-concurrency was not triaged onto, it emits a one-line "concurrency triage gap" note so the orchestrator can launch review-concurrency.

Test-review agents (review test quality and coverage gaps):

Agent	Launch when
review-test-quality	Always launched (unless docs-only or build-config are the ONLY categories)
review-test-structure	Any test files are changed (reviews isolation, readability, setup/teardown of test code itself)
review-test-concurrency	concurrency is present on any changed file (production or test)
review-test-crash-safety	crash-durability

review-test-quality carries both the behavior sub-protocol (whether tests verify real behavior, assertion depth) and the completeness sub-protocol (corner cases, boundary conditions); it keeps both the TB and TC finding prefixes verbatim.

Categories from both production and test code count for the test-review side — for example, if production code adds a new synchronized block but tests don't exercise threading, review-test-concurrency should still launch to flag the gap.

Workflow-review agents (review changes to the workflow machinery itself):

Agent	Launch when
review-workflow-consistency	workflow-machinery is present — always launched for this group
review-workflow-prompt-design	workflow-machinery AND any changed file matches .claude/skills/*/SKILL.md, .claude/agents/*.md, or .claude/workflow/prompts/*.md
review-workflow-instruction-completeness	workflow-machinery AND any changed file matches .claude/skills/*/SKILL.md, .claude/agents/*.md, .claude/workflow/*.md, or .claude/workflow/prompts/*.md
review-workflow-hook-safety	workflow-machinery AND any changed file matches .claude/hooks/*.sh, .claude/scripts/**, or .claude/settings*.json
review-workflow-context-budget	workflow-machinery is present — always launched for this group. The agent decides whether the diff affects any of three axes (always-loaded surface, load-on-demand discipline, or instant per-operation consumption) and emits an empty findings list when none are affected.
review-workflow-writing-style	workflow-machinery AND any changed file matches .claude/**/*.md, root CLAUDE.md, or docs/adr/**/*.md

The workflow-review agents focus on .claude/, root CLAUDE.md, and plan artifacts under docs/adr/<dir>/_workflow/. They ignore Java code changes — the code-review and test-review agents handle those.

Complexity never changes which agents this step selects. Selection is domain-only: a category is present → its agent launches, identically at every per-track complexity level. The per-track complexity tag (read by the workflow's Phase-C callers, not by this standalone skill) moves only the rigor dial — what terminates the Phase-C review-iteration loop. Blockers loop until clear at every complexity level. The should-fix depth scales with the tag: low never lets should-fix drive iteration, medium allows up to three iterations, high is uncapped. The uncapped loops terminate by no-progress detection rather than a fixed cap (see review-iteration.md § Limits and track-code-review.md § Review loop). The floor plus the domain-matched set is never suppressed by a low complexity; complexity only shortens or lengthens iteration, never drops a selected reviewer. The standalone /code-review skill takes no complexity input and always runs the domain-selected set once.

5c: Log your triage decision

Before launching agents, output a brief triage summary so the user can see the reasoning:

### Triage summary
- **Categories detected**: storage-engine, concurrency, index-data-structures
- **Code agents selected**: review-code-quality, review-bugs, review-concurrency, review-crash-safety, review-performance
- **Test agents selected**: review-test-quality, review-test-structure, review-test-concurrency, review-test-crash-safety
- **Workflow agents selected**: (none — no workflow-machinery changes)
- **Agents skipped**: review-security (no network/API/SQL/config/dependency changes)

5d: Edge cases

The rules below cover combinations not handled by the per-row tables above. Where a combination matches more than one row, the last matching row wins. Workflow-machinery cases are listed first because workflow-only diffs are common and short-circuit the entire code/test agent dispatch.

Staged-path normalization (run first). On a workflow-modifying plan the authored .claude/... edits live under docs/adr/<dir>/_workflow/staged-workflow/.claude/... (per §1.7) while the live tree stays at develop's state. The Step 5b workflow-review globs name live .claude/... paths, so a staged path matches none of them. Three glob-gated reviewers therefore miss and fail to launch: review-workflow-prompt-design, review-workflow-instruction-completeness, and review-workflow-hook-safety. (review-workflow-consistency and review-workflow-context-budget always run for this group; review-workflow-writing-style already fires via its docs/adr/**/*.md glob, and since its .claude/**/*.md glob also matches the normalized path, it fires regardless of which form this row is evaluated against.) Before evaluating the Step 5b per-agent triggers against the workflow-machinery subset of the diff, normalize each changed path: a path matching the anchored prefix docs/adr/<any-dir>/_workflow/staged-workflow/(\.claude/…) is replaced by its captured .claude/… remainder; the match is anchored after the docs/adr/<dir>/ head (the <dir> segment is variable). A path that does not match this exact anchored prefix passes through unchanged, including one that merely contains .claude/ lower down. A staged file then evaluates exactly as its live counterpart would, and the three glob-gated reviewers launch on the staged edit. Normalization runs ahead of the Step 5b glob match only; it does not edit the globs themselves and does not change the Step 5a file-set categorization (a staged file is already workflow-machinery by the docs/adr/<dir>/ rule).