Octopus/octopus-workflow
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
bb35e661b20b3048b724cebcecf275ebd42896a8
octopus-workflow/core/skills/review-code/SKILL.md
T

1064 lines
63 KiB
Markdown
Raw Normal View History

Initial publish v0.1.0: standalone workflow core (corpus + examples + guards)
2026-09-15 08:41:51 +08:00
---
name: review-code
description: >
Use ONLY after code has been written — in DAG task mode (a Kind/Task node
ticket under a DAG-routed parent) against the node spec + frozen
cross-session contracts of the frozen DAG copy (`{epic-slug}/dag`), or in
standalone-bugfix mode without a design baseline. (The legacy
approved-design pipeline mode was archived 2026-08-21, [org-internal #3072] phase 3.) Spawns up to 10 parallel Explorer reviewers (risk-tier scaled;
fewer in standalone-bugfix mode where DGN may be skipped) against core/checklists/code-review.md, synthesizes
findings with a Worker Synthesizer (the main-session Orchestrator aggregates), and iterates with a Worker developer until
convergence.
stage: review-code
triggers:
- review code
- code review
- 代码评审
- audit code
- check code
role: Reviewer
---
> Core 中立版（Increment 6a 改写，原 deferHard verbatimDir）。机制、结构与 frontmatter 保持；实例术语（工具名、路径、工单号）按 `core/adapters/TERMINOLOGY.md` 绑定到具体实例。
# Code Review
Multi-dimensional parallel review of code changes against the approved design
document and coding standards. Based on the checklist at
`core/checklists/code-review.md`.
## Greenfield vs. Brownfield
**Greenfield** (new project): Review code against the design document and
abstract best practices.
**Brownfield** (existing project + new feature):
- Review code against THREE baselines: design + existing code conventions +
existing codebase references.
- STY dimension uses the existing codebase as the authoritative style guide,
not abstract best practices. The reviewer MUST read 3+ existing files in
the same module to establish convention expectations. (This is the
canonical statement of the shared brownfield read-neighbors rule —
implement/frontend skills and checklists reference it here.)
- DGN dimension additionally checks dependencies: does the new code introduce
a dependency that duplicates an existing library's purpose? Does it conflict
with an existing dependency version?
- COR dimension additionally checks compatibility: does the change break any
existing public API? Is there a deprecation path? Are config changes
backward-compatible? Does the new code interfere with existing code paths?
(e.g. a new middleware that affects all routes, a shared state mutation)
- The Orchestrator assigns an additional file list to each reviewer: 3–5
neighboring existing files in the same module for convention comparison.
---
## Preconditions
Before launching this review, confirm:
> **Legacy pipeline preconditions retired ([org-internal #3072] phase 3, 2026-08-21)**: the
> design-document / design-space-convergence / iteration-plan-convergence
> checks that used to head this list belonged to the archived legacy pipeline
> (`design` / `plan-iterations` skills — `<instance-root>/archive/`). Live entry
> modes are DAG Task Mode (convergence precondition: `octopus review status
> --stage review-dag` = `success`) and standalone bugfix (below); historical
> runs' design pages stay readable via `_shared/gitea-read-patterns.md`.
- [ ] Code files to review are specified (list of file paths or use `git diff`).
- [ ] Mechanical-green gate PASSED (L0-enforced, [org-internal #2598]): run
`bun core/skills/review-code/scripts/precondition-gate.ts {slug}` —
CI-aligned checks (`bun oxlint --deny-warnings` + `bun turbo typecheck`
+ `test:changed` with `CI=true TEST_SHARDS=3`, mirroring ci.yml).
Exit 0 required BEFORE any reviewer dispatch; on exit 1 abort dispatch
and route the gate output to the Developer (Phase C). `test:parallel`
is NOT a gate check — it stays with verify. The gate caches results on
a tree fingerprint (HEAD + tracked diff + untracked contents): a re-run
on an unchanged tree is a cache hit, so repeated invocations within a
round do not re-pay the mechanical cost (`--no-cache` forces a re-run
after an environment change). Self-attested "tests pass" no longer
satisfies this item.
- [ ] `core/checklists/code-review.md` is accessible.
- [ ] `slug` matches the design slug.
- [ ] 跨阶段门控清单: `core/checklists/pipeline-gate.md` is accessible and
its Upstream Review Convergence section has been confirmed item by item.
**Kind/MVP graduation chokepoint ([org-internal #3061])**: when this skill is invoked ad hoc
on a `Kind/MVP` ticket's changes (no mode, no pipeline), the mechanical-green
gate applies, and the Orchestrator additionally checks the diff for
**graduation triggers** — changes crossing ≥2 modules, touching a shared
contract / public API, or introducing an external dependency. On a trigger,
complete the review as requested but flag prominently: "this work has
outgrown MVP mode — suggest relabeling `Kind/Feature` (analyze-dag backfill
reads the ticket's decision log)". Advise, do not block; the relabel is the
maintainer's call.
If any precondition is unmet, abort and inform the user. Refer to
`core/checklists/pipeline-gate.md` for the complete cross-stage gate
checklist. Refer to the Recovery Protocol in the same document to determine
which skill to run next to fix each missing item.
When the user requests a review without specifying files, run `git diff` /
`git status` and present the changed files for confirmation. Review only the
user-confirmed set.
### Standalone Bugfix Mode
Triggered by the `implement` skill (Mode: bugfix) Phase 5 Handoff when changes > 20 lines or ≥ 3 files.
The caller (typically the `implement` skill in bugfix mode) passes `mode: "standalone-bugfix"` AND
no design doc exists at `.artifacts/{slug}/design/index.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/03-design-index`（Size/L 独立页）或 `{slug}/02-03-req-design`（Size/M / Size/ML 合并页，design 为后半），`_shared/gitea-read-patterns.md` Pattern 1）.
**Weakened preconditions** for standalone mode:
- Design doc existence → NOT required (the legacy design pipeline is archived, [org-internal #3072] phase 3).
- **Instead require**: `.artifacts/{slug}/repro-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/repro-notes`，`_shared/gitea-read-patterns.md` Pattern 1）exists (bugfix Phase 1-2 output: reproduction steps + root cause) OR `.artifacts/{slug}/impl-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/impl-notes`，`_shared/gitea-read-patterns.md` Pattern 1）exists (post-fix behavior contract).
- All other Preconditions (code files specified, mechanical-green gate green ([org-internal #2598]), checklist accessible) STILL APPLY.
- The `core/checklists/pipeline-gate.md` cross-stage gate (Upstream Review Convergence section) → NOT required; the bugfix-specific Pre-Design Falsification Gate section still applies if relevant.
**DGN dimension handling**: In standalone-bugfix mode, the DGN (Design Compliance & Dependencies) reviewer is SKIPPED because there is no design to comply with. The Orchestrator dispatches 9 reviewers (all 10 dimensions except DGN).
**What standalone bugfix review focuses on**: code correctness, error handling & compatibility (COR), regression coverage (TST), security (SEC), performance (PERF), style & conventions (STY), database & data (DBT), accessibility & browser compatibility (A11Y), documentation (DOC), traceability (TRC) — everything that does not depend on a design baseline.
**Historical**: the legacy pipeline mode (design doc + design-space/plan
review convergence as preconditions) was archived 2026-08-21 ([org-internal #3072] phase 3).
Historical design pages remain readable via `_shared/gitea-read-patterns.md`.
### DAG Task Mode
Triggered for a `Kind/Task` ticket under a DAG-routed parent Epic — the
`dag.task_route` flow (`implement → review-code → verify`,
`<instance-root>/workflow-routing.yaml` `dag.task_route`), whose `keep_gates` includes
`review-code`. Applies when a frozen DAG copy exists at wiki page
`{epic-slug}/dag` and the ticket's `## 父级 / Parent` links to that Epic. The
caller (the main session / Builder, at implement Phase 6 handoff on a node
ticket) passes `mode: "dag-task"`.
Unlike standalone-bugfix, this mode is NOT threshold-gated by diff size:
`task_route.keep_gates` keeps the gate for EVERY task node ticket, and verify's
task mode consumes this PASS ("this node's ticket review-code PASS recorded" —
verify/SKILL.md §"DAG branch (milestone + task modes)").
**Precondition replacements** for DAG task mode (mirror verify's DAG task-mode
table and implement's §"DAG-mode input path"):
| Standard precondition | DAG-task-mode replacement |
| --- | --- |
| Design-space review converged (`octopus review status --stage design-space`) | review-dag single-gate converged: `octopus review status --stage review-dag` must show state `success` (this Epic already passed the single gate; do NOT re-run it). Not converged → abort: "review-dag 未收敛，代码评审无法开始" |
| Iteration-plan review converged (`--stage iteration-plan`) | REMOVED (DAG has no plan review; the single review-dag gate replaces both) |
| `slug` matches the design slug | N/A (review is scoped to the node ticket; run slug = `{epic-slug}-task-{node-id}` — the same node-task run namespace verify's task mode archives to — so review outputs are node-scoped and never clobber sibling nodes' reports. Note: this namespace covers review + verify runs only; the node's implement report below lives in the Epic run workspace `{epic-slug}/workers/` — implement worker runs keep the Epic run slug (implement §Preconditions: "DAG: `{epic-slug}`"), a different run from this review run) |
| `core/checklists/pipeline-gate.md` Upstream Review Convergence | NOT required — the review-dag convergence check above stands in its place |
| All other preconditions (code files specified, mechanical-green gate green ([org-internal #2598]), checklist accessible) | KEEP — unconditional; plus the implement report (AC → test traceability) present — Tier 1 at `<runs-root>/{epic-slug}/workers/{node-id}-worker-{seq}.md` (implement Phase 5 persistence; `{chunk-id}` = the node id) |
The `{BASELINE_SECTION}` for DAG-task mode uses the node spec + cross-session
edge contracts from `{epic-slug}/dag` instead of the design index; the 5-Check
Gate's legacy `{slug}/03-design-index` references resolve to the node's `req_refs` +
component field in `{epic-slug}/dag` (mirror `implement/SKILL.md` DAG-route
read map). The `{READ_FIRST_STEPS}` for DAG-task mode replaces step 1 with:
"Read the node spec + cross-session edge contracts in `{epic-slug}/dag` first
for orientation (plus the `{epic-slug}/dag-nodes/{node-id}` subpage when AC
detail is sunk)."
**Output naming (node-scoped)**: the Tier 1 run dir is
`<runs-root>/{epic-slug}-task-{node-id}/reviews/code/round{N}/` and the
final report wiki page is
`{epic-slug}/reviews/code/final/report-task-{node-id}` (suffix form,
mirroring verify's `{epic-slug}/05-verify-task-{node-id}`) — sibling node
reviews of one Epic run in parallel sessions and MUST NOT overwrite each
other's final reports or round-1 findings. Phase E steps 1/2.5/2.55/2.6/2.8
substitute these node-scoped paths (step 2.8 also emits the node-scoped
`REVIEW-code-task-{node-id}` 工件索引 row id per the Pattern 10 per-skill
row map, `_shared/gitea-write-patterns.md`).
**DGN dimension is NOT skipped** (unlike standalone-bugfix): a DAG task review
HAS a design baseline — the node spec. The DGN reviewer reads the node's AC +
its cross-session edge `contract_ref` targets (wiki `{epic-slug}/shared/` or
schema `$id`) from the frozen DAG copy instead of legacy `{slug}/03-design-*` pages,
and checks the diff against them. Reviewer count and risk-tier scaling are
unchanged (S=1 / M=3 / L=10).
**Frozen-contract conformance (COR + DGN)**: any change that widens, narrows,
or otherwise violates a `contract_state: frozen` cross-session edge contract
is a **BLOCKER** and fires the `frozen_contract_breaking` oversize signal
(`workflow-routing.yaml` `dag.size_derivation.oversize_signals`) — pause the
review; the signal mandates re-derivation + review-dag re-run, so do NOT
converge via a local fix. A `draft` cross-session contract turning `breaking`
fires `draft_contract_breaking` (depth escalation to D4) — flag as MAJOR and
surface the escalation, not a silent fix.
**Node scope conformance (`node_scope_drift`)**: before converging, compare
the actual changed-file set (`git diff --name-only`) against the node's
decomposition-time `size_attrs` and `component` field in `{epic-slug}/dag`.
Drift — the changeset lands in modules outside the node's component, or
clearly exceeds the `size_attrs` basis (e.g. a contained multi-file estimate
realized as a whole-module rewrite) WITHOUT any node_split / contract signal
firing — triggers the `node_scope_drift` oversize signal
(`workflow-routing.yaml` `dag.size_derivation.oversize_signals`): pause
convergence, surface the drift, and let the signal's re-derivation mandate
run (same posture as `frozen_contract_breaking` — no local-fix convergence).
A pure estimation miss (same scope, mildly larger surface) is recorded as
the revision rationale and does not block.
**Record the mode**: commit status context stays `pipeline/review-code`, with
a `mode: "dag-task"` field (as lightweight mode records `mode: "lightweight"`)
— verify's task mode keys off this ticket's review-code PASS.
### Prototype Promotion Mode
Triggered when the `prototype` skill's EVOLUTIONARY contract reaches its promotion gate. The caller passes `mode: "prototype-promotion"`.
**Required inputs**:
- `.artifacts/{slug}/prototype-debt.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/prototype-debt`，`_shared/gitea-read-patterns.md` Pattern 1）— the debt register maintained during the evolutionary prototype phase.
- The code files being promoted (same as standard mode).
**Promotion rule**: reviewers MUST treat every line item in the debt register as a review finding unless it has an explicit prior waiver. A debt item without a waiver blocks promotion with severity `MAJOR`. The promotion gate is passed only when the debt register is empty OR every remaining item carries a written waiver added to the register by the reviewer during this review.
**What reviewers focus on in addition to standard dimensions**: the debt register itself — is each promotion criterion concrete and testable, or is it vague ("improve later")? Vague criteria are flagged as DOC findings.
### Lightweight Review Mode (Test-Only Changes)
Triggered when the changeset consists **entirely** of test files (paths matching
`*.test.ts`, `*.test.tsx`, `*.spec.ts`, or `__tests__/**`) with zero source-code
changes outside test files. The Orchestrator MUST verify this condition before
entering lightweight mode: run `git diff --name-only` and confirm every changed
file is a test file.
**Weakened preconditions** for lightweight mode:
- Design doc existence → NOT required.
- Design-space review convergence → NOT required.
- Iteration-plan review convergence → NOT required.
- All Preconditions from the pipeline checklist are bypassed.
- Only requires: the mechanical-green gate ([org-internal #2598]) — same script as the full
mode; on a test-only changeset `test:changed` selects exactly the changed
tests, so the gate cost stays proportional to the diff.
**Reduced review surface**: The Orchestrator dispatches only **1–3 reviewers**
covering the minimum dimensions:
- COR (Correctness): are assertions testing the right thing? No tautological
asserts, no `.not.toThrow()` wrapping a call that never throws.
- TST (Test Quality): coverage completeness, boundary cases, no duplicate
coverage of already-tested paths.
- STY (Style): follows existing test file conventions (describe/it structure,
naming, setup/teardown patterns).
The Orchestrator assembles findings manually (no synthesis sub-agent; one round
only). Lightweight reviews do NOT iterate — if findings exist, the Developer
fixes them and the Orchestrator re-verifies, but does not spawn a second review
round. The review is recorded as round 1 with a `mode: "lightweight"` field in
the commit status (context: `pipeline/review-code`).
**Exclusion rule**: If ANY changed file is a source file (even a one-line fix),
fall back to the appropriate full mode (pipeline or standalone-bugfix).
| Property | Standard (Pipeline) | Standalone Bugfix | DAG Task | Lightweight (Test-Only) |
| -------- | ------------------- | ----------------- | -------- | ----------------------- |
| Design required | yes | no | node spec + frozen contracts | no |
| Reviewers | 10 (tier-scaled) | 9 (tier-scaled) | 10 (tier-scaled, DGN kept) | 1–3 |
| Iteration rounds | until convergence | until convergence | until convergence | 1 (no iteration) |
| Reviewer agent | Explorer | Explorer | Explorer | Orchestrator (manual) |
| Source changes allowed | yes | yes | yes | **no** (abort if any) |
---
## Agent Roles
> **Progressive disclosure ([org-internal #3722])**: when dispatching a Reviewer /
> Synthesizer / Developer scoped to one phase, pass
> `skills: ["review-code:phase-a"]` (preamble — incl. dimensions, JSON
> schema, prompt template — plus that phase only) instead of the full body;
> phases may union (`"review-code:phase-c,phase-d"`). Full-body remains the
> default when no selector is passed.
| Role | Agent Type | Count | Purpose |
| --------- | ---------- | ----- | --------------------------------------------- |
| Orchestrator | Worker | 1 | Dispatch, synthesize, control iteration loop |
| Reviewer | Explorer | 10 (risk-tier scaled) | 1 dimension per reviewer at L tier; grouped sweep at S/M tiers (see Risk Tier table; grouped dispatch fills the shared prompt's {DIMENSION_CODE}/{DIMENSION_NAME} with the joined group per `_shared/review-reviewer-prompt.md`). Read-only (9 in standalone-bugfix mode; DGN skipped) |
| Developer | Worker | 1 | Revise code based on synthesis (the same role |
| | | | that wrote the original code) |
All reviewers run in parallel. The orchestrator and developer run sequentially
(orchestrator → developer → orchestrator → ...).
## Risk Tier Assessment (Reviewer Count Scaling)
Before dispatching reviewers, the Orchestrator MUST assess the changeset's risk
tier to determine the optimal reviewer count. This replaces the fixed
1-reviewer-per-dimension model.
### Tier Determination
Run `git diff --stat` to measure the changeset, then classify:
| Tier | Criteria | Reviewers | Dimension Assignment |
|------|----------|-----------|---------------------|
| **S** (Small) | <5 changed files AND single package AND no public API/config change | 1 | All 10 dimensions in a single reviewer sweep |
| **M** (Medium) | 5–15 changed files OR multi-package OR public API change | 3 | Group A: COR+TST+DBT; Group B: DGN+SEC+PERF; Group C: STY+A11Y+DOC+TRC |
| **L** (Large) | >15 changed files OR config/infra/critical-path change | 10 | 1 reviewer per dimension (standard parallel dispatch) |
**Escalation rule**: If a Small or Medium tier review produces BLOCKER findings
in round 1, escalate to the next tier for round 2+.
**Lightweight mode override**: Test-only changes always use 1–3 reviewers
regardless of tier (see Lightweight Review Mode section).
**Standalone-bugfix mode**: In standalone-bugfix mode (no design doc), DGN is
skipped. Small=1 reviewer (9 dimensions), Medium=3 (Groups A-C minus DGN),
Large=9 (1 per non-DGN dimension).
**DAG task mode**: DGN is NOT skipped — the node spec + frozen cross-session
contracts in the frozen DAG copy `{epic-slug}/dag` are the design baseline
(all 10 dimensions intact; see DAG Task Mode).
<!-- inject: ../_shared/large-prompts.md -->
## Review Dimensions
Each reviewer is assigned exactly **one** dimension at **L tier** (1 reviewer
per dimension). At S/M tiers a single reviewer covers a grouped sweep over
multiple dimensions (see the Risk Tier table above) — the reviewer's scope is
the checklist items for each dimension in its group and nothing else.
See `reference/code-review-dimensions.md` for the 10-dimension mapping table
(dimension code, label, checklist ref, code files, design reference files),
the single-dimension rationale, and the design baseline rules (which reviewers
receive which design sub-files).
---
## JSON Schema
All schemas are defined in `core/schemas/` and are shared across the live
review consumers — code review, the review-dag single gate, and the process
audit (`review-artifact` targets):
| Schema File | Used For |
| ---------------------------------------------- | ------------------------------- |
| `core/schemas/reviewer-output.schema.json` | Per-dimension reviewer findings (Tier 1 review-code findings sink; validated in place at `<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`) |
| `core/schemas/synthesis.schema.json` | Orchestrator round synthesis (Tier 2 — synthesis comment) |
| `core/schemas/review-status.schema.json` | Review lifecycle tracking |
> **Findings sink: `tier1-local`** ([org-internal #1988]). `review-code` writes raw
> per-dimension findings to Tier 1 local files (`<runs-root>/{slug}/reviews/code/`),
> not Gitea issue comments. The synthesis comment + commit status stay Tier 2.
> See `_shared/review-pipeline-phases.md` "Output model — two findings sinks"
> and `core/rules/two-tier-artifacts.md`.
---
## Standardized Reviewer Prompt Template
The Orchestrator MUST use the standardized reviewer prompt template at
`core/skills/_shared/review-reviewer-prompt.md` when dispatching each
reviewer. The same template goes to every dispatched reviewer (10 at L tier,
3 at M tier, 1 at S tier); the Orchestrator fills
in skill-level constants (below) and per-dimension values (from the dimension
table).
**Skill-level constants** (same for all reviewers in this skill):
- Model tier: default session reviewer model (`Reviewer` Explorer role, `_shared/roles/reviewer.yaml`) — review-code sets no `reviewer-tier` override (unlike review-dag, which consumes `analyze-dag`'s reviewer-tier); record any assignment change here so sibling review skills stay comparable (TRC 10.5)
- `{REVIEWER_ROLE}`: code reviewer
- `{BASELINE_SECTION}`:
**Legacy pipeline mode** (archived 2026-08-21, [org-internal #3072] phase 3 — the design
doc at `{slug}/03-design-*` / `{slug}/02-03-req-design` it read is
historical; live modes use the two baselines below):
**Standalone-bugfix mode** (no design doc; caller passed `mode: "standalone-bugfix"`):
```
## Bugfix Baseline
The code you are reviewing is a standalone bugfix. There is no design doc;
the baseline is the bug's expected behavior documented in
`.artifacts/{slug}/repro-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/repro-notes`，`_shared/gitea-read-patterns.md` Pattern 1）reproduction steps + root cause) and/or
`.artifacts/{slug}/impl-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/impl-notes`，`_shared/gitea-read-patterns.md` Pattern 1）post-fix behavior contract). The code MUST
resolve the documented root cause without changing unrelated behavior.
```
**DAG-task mode** (caller passed `mode: "dag-task"`; see §"DAG Task Mode" above):
```
## DAG-Task Baseline
The code you are reviewing implements one task node of a DAG-routed Epic.
The baseline is the node spec + acceptance criteria + frozen cross-session
edge contracts read from the frozen DAG copy at wiki page `{epic-slug}/dag`
(+ `{epic-slug}/dag-nodes/{node-id}` subpages when AC detail is sunk). The
5-Check Gate's legacy `{slug}/03-design-index` references resolve to the
node's `req_refs` + component fields in `{epic-slug}/dag`. The code MUST
satisfy the node's acceptance criteria without breaking the frozen
cross-session contracts.
```
- `{READ_FIRST_STEPS}`:
```
1. Read the design `index.md` first for architecture and component orientation.
2. Read every code file listed above. Do NOT skip any file.
3. Read every design file listed above (if any). Use them as the authoritative
source against which to judge the code's correctness and completeness.
```
- `{ARTIFACT_NOUN}`: code
- `{FINDING_DETAILS}`: file path and line number, a summary, detailed
explanation, quoted code evidence
- `{FINDING_ID_EXAMPLE}`: COR-F001
- `{FINDING_ID_PREFIX}`: the finding's own dimension code — `{DIMENSION_CODE}`
in the standard single-dimension dispatch; in a grouped M/S-tier dispatch
each finding's id uses its own dimension code (e.g. `TST-F001` inside the
COR+TST+DBT group), never the joined group code
- `{CONTRADICTION_RULE}`:
```
6. When code contradicts the design, this is at minimum a MAJOR finding.
When a required component or interface from the design has no corresponding
code at all, this is a BLOCKER.
```
- `{SEVERITY_DEFINITIONS}`:
```
- BLOCKER: design-exposed security vulnerability (SQL injection, XSS, auth
bypass), data corruption risk, missing implementation of a required
design component, broken build or test suite, hardcoded secrets
- MAJOR: incorrect or missing interface implementation, unhandled error
paths that the design specifies, N+1 query or obvious performance trap,
missing test coverage for a design-specified behavior, code that
contradicts a design decision without documented rationale
- MINOR: naming deviation from convention, missing or misleading inline
comment, dead code, import ordering, minor style violation that does
not affect correctness or readability
- INFO: observation, suggestion, or refactoring opportunity that goes
beyond the checklist
```
- `{LOOKS_FINE_PHRASE}`: the code looks fine
**Per-dimension values** (filled from `reference/code-review-dimensions.md`):
- `{DIMENSION_CODE}`: the dimension code (e.g. COR, DGN, SEC, …); in a
grouped M/S-tier dispatch this is the JOINED GROUP (e.g. `COR+TST+DBT`) for
the heading and output filename, while finding ids use each finding's own
dimension code per `{FINDING_ID_PREFIX}` above
- `{DIMENSION_NAME}`: the dimension label (e.g. Correctness, Design Compliance)
- `{CHECKLIST_ITEMS}`: the checklist items for this dimension, inlined from
`core/checklists/code-review.md`
- `{FILE_SECTIONS}`: construct using these section headers with the dimension's
assigned files:
- `## Code Files to Review` — the dimension's code files
- `## Design Files for Cross-Reference` — the dimension's design reference
files, or `(none)`
- Append the note: `(If this section contains "(none)", your dimension does
not require cross-referencing against specific design sections. Rely on the
baseline index for orientation.)`
- `{OUTPUT_FILE}`: **Tier 1 local** — write the finding JSON to
`<worktrees-root>/<repo>-<slug>/<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`
(absolute worktree path — see the worktree-path rule below), validated by
`core/schemas/reviewer-output.schema.json`. `review-code` is the first
skill migrated to the Tier 1 findings sink (see `_shared/review-pipeline-phases.md`
"Output model — two findings sinks" and `core/rules/two-tier-artifacts.md`).
The synthesis (human gate) and commit status STAY Tier 2 (Gitea).
---
<!-- section:phase-a -->
## Phase A — Parallel Review
> **Canonical mechanics**: `core/skills/_shared/review-pipeline-phases.md` — keep this section in sync with the canonical Phase A–E flow. This section adds review-code-specific constants (10 dimensions, severity definitions, risk tiers, output mode, standalone-bugfix mode). Step 0 (move source issue to `review` column on Pipeline Stages board per Pattern 7.5) is inherited from the shared Phase A orchestrator actions; see `_shared/review-pipeline-phases.md`.
### Standalone Bugfix Mode — Orchestrator Dispatch
When invoked from the `implement` skill in bugfix mode (caller passes
`mode: "standalone-bugfix"` — the legacy "detect absence of a design doc"
fallback is retired with the design pipeline, [org-internal #3072] phase 3):
1. The Orchestrator dispatches **9 reviewers** (all 10 dimensions **except DGN** — Design Compliance & Dependencies), because there is no design baseline to audit compliance against.
2. For any dimension that would normally receive design reference files (COR, DGN, SEC, PERF, TST, DBT, A11Y, DOC, TRC), substitute the bugfix baseline docs:
- `.artifacts/{slug}/repro-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/repro-notes`，`_shared/gitea-read-patterns.md` Pattern 1）— reproduction steps + root cause analysis.
- `.artifacts/{slug}/impl-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/impl-notes`，`_shared/gitea-read-patterns.md` Pattern 1）— post-fix behavior contract.
- These replace the design sub-files in `{FILE_SECTIONS}` (the "Design Files for Cross-Reference" section).
3. The `{BASELINE_SECTION}` uses the **Bugfix Baseline** variant (from the constants above), not the Design Baseline.
4. The `{READ_FIRST_STEPS}` replace "Read the design `index.md`" with:
```
1. Read `.artifacts/{slug}/repro-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/repro-notes`，`_shared/gitea-read-patterns.md` Pattern 1）first (if it exists) for the bug's root cause and expected behavior.
2. Read every code file listed above. Do NOT skip any file.
3. Read `.artifacts/{slug}/impl-notes.md`（存量只读，不迁移；Gitea 等效：wiki page `{slug}/impl-notes`，`_shared/gitea-read-patterns.md` Pattern 1）(if it exists) for the post-fix behavior contract.
```
5. The `{CONTRADICTION_RULE}` is weakened: code that contradicts repro-notes/impl-notes is a MAJOR — there is no design to elevate to BLOCKER.
6. All other Phase A dispatch rules (reviewer comment posting, commit status, reviewer prompt template, temperature, permissions, worktree-path rule) apply unchanged.
7. Efficiency optimization (round 2+, [org-internal #2598]): follows the shared Phase D rule (`_shared/review-pipeline-phases.md` Phase D "Efficiency optimization") with no standalone-mode deviation — partial re-dispatch, full sweep every 2 rounds, delta-scoped reviewer input.
**Orchestrator actions**:
1. Prepare Tier 1 run workspace: ensure `<runs-root>/{slug}/` exists with a
`meta.json` (state `active`, schema: `core/schemas/runs-meta.schema.json`)
and an `index.json` (append to an existing one
if a prior stage already initialized the run — see `core/templates/runs-layout.md`;
schema: `core/schemas/runs-index.schema.json`).
Reviewer findings will be written to
`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`
(Tier 1 local, validated by `core/schemas/reviewer-output.schema.json`); the synthesis
prompt goes to `<runs-root>/{slug}/reviews/code/round{N}/task-synthesizer.md`.
Task prompts for the reviser are created as wiki pages via `wiki 读写 API（见 TERMINOLOGY）`
（`_shared/gitea-write-patterns.md` Pattern 1）with page_name `{slug}/reviews/code/round{N}/task-{role}`.
2. Initialize or update review status by posting a commit status via `POST /api/v1/repos/Octopus/octopus/statuses/{sha}` (`_shared/gitea-write-patterns.md` Pattern 8) with context `pipeline/review-code`. The commit-status payload carries `state` and `description`; the full review lifecycle state is documented in `core/schemas/review-status.schema.json` and tracked by the `octopus review status` CLI.
2.5. **Mechanical-green gate (mandatory, [org-internal #2598])** — before spawning ANY reviewer,
run `bun core/skills/review-code/scripts/precondition-gate.ts {slug} --round {N}`
from the worktree root. Exit 0 → continue to step 3. Exit 1 → the script has
appended a PRECONDITION-BLOCK record (Tier 1,
`<runs-root>/{slug}/reviews/code/precondition-gate.jsonl`); do NOT dispatch
reviewers — dispatch the Developer (Phase C) with the failing check name and
the first error lines, then re-enter this step. A blocked attempt does NOT
increment the review round. On the first gate run, append one `index.json`
row for the gate log (type `precondition-gate`, stage `review-code`,
produced_by `Orchestrator`, `schema: null` — the jsonl is self-versioned
and machine-regenerable, no backing `core/schemas/*.json`) following
the findings-row pattern.
The gate is fingerprint-cached: when the Developer already ran the
pre-handoff gate (Phase C step 4) on the unchanged tree, this step
cache-hits and costs ~nothing — do not skip it.
3. Spawn **all reviewers in parallel** (10 in pipeline mode at Large tier; fewer at Small/Medium tiers; DGN skipped in standalone-bugfix mode). Each reviewer receives:
- The standardized prompt (above) with dimension-specific values filled in.
- For round ≥ 2 ([org-internal #2598] delta scoping): `{FILE_SECTIONS}` lists ONLY the
files revised since the previous round plus their 3–5 convention-comparison
neighbors — not the round-1 full file set. Round 1 keeps the full set.
- Agent type: Explorer.
- Permission: read-only EXCEPT scoped write to the Tier 1 findings dir. Pass
`permissions` with `allow` rules for `read`, `glob`, `grep` (read-only
everywhere) PLUS an `edit` allow for the runs dir so the reviewer can write
its OWN finding file — do NOT pass deny rules (they are inherited). Example:
`[{ permission: "read", pattern: "*", action: "allow" }, { permission: "glob", pattern: "*", action: "allow" }, { permission: "grep", pattern: "*", action: "allow" }, { permission: "edit", pattern: "<runs-root>/**", action: "allow" }]`
(the `edit` permission governs the `write` tool too — see
`<harness-package>/src/permission/index.ts` `EDIT_TOOLS`. This scoped grant
emits the reviewer's output; it does NOT touch the code under review, so
rule #7 auditor independence holds.)
- Temperature: 0.1 (deterministic output).
**Worktree-path rule (mandatory)**: Every file path in the reviewer prompt
(`{FILE_SECTIONS}`, design reference paths, artifact paths) MUST be the
absolute path inside the current workflow worktree
(e.g. `<worktrees-root>/<repo>-<slug>/packages/app/src/...`), NOT a
bare repo-relative path. Explorer sub-agents resolve relative paths
against the main session's CWD (`/data/octopus`, a bare repo with no
working tree), which causes them to read stale or absent files and
produce hallucinated findings. Before dispatch, the Orchestrator MUST
compute the worktree root (e.g. via `git worktree list` or the known
`<worktrees-root>/<repo>-<slug>` convention) and prefix every path
with it. This is the single highest-source of false-positive findings
observed in the 2026-07 auto-approve retrospective (5-round convergence,
67.7% rework fraction).
4. Wait for all reviewers to complete. Each reviewer writes its finding JSON to
`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`
(Tier 1 local, validated by `core/schemas/reviewer-output.schema.json`). After all
reviewers complete, the Orchestrator appends one `index.json` row per finding
file to `<runs-root>/{slug}/index.json` (type `review-findings`, stage
`review-code`, produced_by `Reviewer`, schema = the reviewer-output `$id`,
tier2_ref = the synthesis comment once posted in Phase B; null until then).
No Gitea issue comment is created for raw findings — the synthesis comment
(Phase B) is the Tier 2 human-gate distillation.
5. If a reviewer fails (crash / timeout), retry once. If still failed,
mark that dimension as `UNRESOLVED` in the synthesis and continue
with the remaining dimensions. The unresolved dimension is recorded
for deferred retry — the Orchestrator may re-dispatch it at the start
of the next round or, if convergence is otherwise achieved, prompt
the user to decide whether to retry or accept the gap.
---
<!-- section:phase-b -->
## Phase B — Synthesis (Delegated)
> **Context optimization**: Phase B is delegated to a Synthesizer (Worker) to
> prevent the Orchestrator's context from accumulating N dimension JSON Read
> results (up to N × 50KB per round). The Synthesizer reads all JSONs in its
> own context, posts synthesis comment + commit status, and returns a compact
> summary (≤ 2KB). The Orchestrator never reads dimension JSONs directly.
**Orchestrator actions**:
1. Fill the synthesis task template
(`core/skills/_shared/synthesis_task.md`) with:
- Round number. A round-1 dispatch carries the template's step-8
"Round 1 only — findings digest" duty (the Synthesizer writes
`round1/findings-summary.md`, copied verbatim into the final report at
step 2.55 below); verify it is present in the filled template.
- List of dimension finding-file paths from Phase A dispatch
(`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`).
- List of UNRESOLVED dimensions (reviewers that crashed/timed out).
- Write the filled template to
`<runs-root>/{slug}/reviews/code/round{N}/task-synthesizer.md` (Tier 1
synthesis prompt — replaces the legacy `/tmp/octopus/` embryo + wiki page).
Append an `index.json` row (type `synthesis-prompt`, stage `review-code`,
produced_by `Orchestrator`).
2. Dispatch the **Synthesizer** (Worker) via the `task` tool:
- `role`: `Synthesizer`
- `subagent_type`: `worker`
- `prompt_file`: `<runs-root>/{slug}/reviews/code/round{N}/task-synthesizer.md` (Tier 1 local)
- `permissions`: read + write + edit + bash + glob + grep (all allow)
- `temperature`: 0.1
- Foreground (the Orchestrator waits for completion).
3. The Synthesizer:
- Reads all dimension finding JSONs from
`<runs-root>/{slug}/reviews/code/round{N}/findings-*.json` (in its OWN
context, not the Orchestrator's — no Gitea call).
- Computes synthesis following `core/schemas/synthesis.schema.json`.
- Posts synthesis as an issue comment via `工单评论 API（见 TERMINOLOGY）` （`_shared/gitea-write-patterns.md` Pattern 5）with a ```json code block on the ticket for `{slug}` (Tier 2 human gate — stays on Gitea).
- Updates review status by posting a commit status via `POST /api/v1/repos/Octopus/octopus/statuses/{sha}` （`_shared/gitea-write-patterns.md` Pattern 8）with context `pipeline/review-code`.
- Persists its return summary to
`<runs-root>/{slug}/reviews/code/round{N}/synthesis-return.md` (Tier 1,
worker-report persistence [org-internal #2847] — see
`_shared/worker-report-persistence.md`) BEFORE returning, and registers
the `index.json` row (type `other`, schema null, stage `review-code`,
produced_by `Synthesizer`). The persisted copy is the report of record —
if the completion notification is lost, the Orchestrator recovers the
round verdict from this file instead of re-running synthesis.
- **Round 1 only — findings digest**: writes
`<runs-root>/{slug}/reviews/code/round1/findings-summary.md` (DAG task
mode: `<runs-root>/{epic-slug}-task-{node-id}/reviews/code/round1/findings-summary.md`)
— the rendered `## Round-1 Findings Summary` table consumed verbatim by
Phase E step 2.55 (template step 8; shared Phase B step 3 "Round-1
findings digest"; `index.json` row type `review-synthesis`). Rounds ≥ 2
skip this.
- Returns a compact summary (≤ 2KB) with verdict, per-dimension results,
and re-audit targets.
4. The Orchestrator receives the summary and proceeds to Phase C or Phase E
based on `overall_verdict`. The Orchestrator does NOT read dimension JSONs
or the synthesis comment directly — all decision-relevant data is in the
Synthesizer's return summary.
5. Present the Synthesizer's return summary to the user in chat.
---
<!-- section:phase-c -->
## Phase C — Developer Revision
If `overall_verdict` is PASS, skip to Phase E.
Otherwise:
**Orchestrator actions**:
1. Prepare the Developer's input as a structured task file,
created as a wiki page via `wiki 读写 API（见 TERMINOLOGY）` （`_shared/gitea-write-patterns.md` Pattern 1）with page_name `{slug}/reviews/code/round{N}/task-developer`.
This file must contain:
- Paths to all code files that need revision.
- Reference to the synthesis comment on the ticket (posted via `工单评论 API（见 TERMINOLOGY）` in Phase B — Tier 2).
- References to the Tier 1 finding files for dimensions that have FAIL or WARN
verdicts (`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`).
- The Developer MUST NOT receive the full text of all reviewer findings
inline — use finding-file path references only to avoid context explosion.
- Convergence status: blockers/majors remaining vs. previous round.
- **Revision survival ([org-internal #2601]) — primary path**: the task tool's completion
output includes a `task_id`; passing it back resumes the SAME Developer
subagent session instead of creating a fresh one. Round 1: persist the
returned `task_id` with the round's working-notes entry (it must survive
round-boundary compaction). Round ≥ 2: dispatch with `task_id` set to the
previous revision round's — the Developer continues in-session with its
prior diff/context; the round's task page then carries only the findings
delta, not a full re-derivation. Reviewers are NEVER resumed — per-round
freshness is their independence.
- **Revision survival — fallback** (task_id lost to compaction before it
was persisted): prepend the current `<runs-root>/{slug}/working-notes.md`
content (run narrative memory — prior decisions, dead ends, reviewer
feedback from earlier rounds) and reference the previous round's revision
summary (`{slug}/reviews/code/round{N-1}/revision-summary`), stating
explicitly that this revises code a prior round already wrote — do NOT
re-read the full design/acceptance artifacts unless a finding requires
them.
2. Spawn the Developer (Worker) with the prompt
template at `core/skills/_shared/review-revision-prompt.md`, substituting:
- `{ROLE}`: Developer
- `{TASK_FILE}`: task-developer
- `{REVISION_SCOPE}`: Apply the Developer Revision Protocol (see Protocol
subsection below) — evaluate each finding against codebase reality:
Explorer reviewers are automated LLM sub-agents that can hallucinate
citations, misapply rules, or miss design context. Verify cited code
exists, the rule applies, and the fix does not break existing
functionality. WAIVE technically-incorrect findings with justification
(`WAIVED-{finding-id}` → one-line reason; canonical definition of the
`WAIVED-{id}` convention, incl. the round-2+ cosmetic-MINOR waiver
rule: `_shared/review-revision-prompt.md`). Revise code minimally and
surgically — do NOT refactor unrelated code or introduce new behavior.
- `{RESOLUTION_RULE}`: Do NOT silently deviate from the approved baseline
(DAG task mode: the node spec + frozen cross-session contracts in
`{epic-slug}/dag`; the legacy `{slug}/03-design-*` baseline is archived
historical, [org-internal #3072] phase 3). If a finding exposes a design flaw, mark it
as deferred with justification and suggest a design amendment — do not
change the code against the baseline.
- `{COSMETIC_MINOR_EXAMPLES}`: naming, comments, dead code, import
ordering, style
- `{WAIVED_EXAMPLE}`: `WAIVED-COR-R2-001`
- `{CHECKLIST_REF}`: `core/checklists/code-review.md`
- `{FILES_MODIFIED_LINE}`: {list}
- `{RETURN_FILE}`:
`<runs-root>/{slug}/reviews/code/round{N}/revision-return.md`
(Tier 1 return persistence — [org-internal #2847],
`_shared/worker-report-persistence.md`)
3. The Developer revises the code files in place. The Developer creates a
revision summary as a wiki page via `wiki 读写 API（见 TERMINOLOGY）` （`_shared/gitea-write-patterns.md` Pattern 1）with page_name `{slug}/reviews/code/round{N}/revision-summary`,
and persists its final return summary to
`<runs-root>/{slug}/reviews/code/round{N}/revision-return.md` (Tier 1,
worker-report persistence [org-internal #2847] — see
`_shared/worker-report-persistence.md`) BEFORE returning. If the
completion notification is lost, the Orchestrator recovers the revision
outcome from the persisted return (wiki page for the full summary,
Tier 1 file for the conclusion of record).
4. **Pre-handoff gate** — before reporting completion, the Developer MUST run
the same mechanical gate the Orchestrator will re-run at Phase D re-entry
(the tree-fingerprint cache turns the re-run into a cache hit — one real
execution per tree state, no double-paying):
- Run `bun core/skills/review-code/scripts/precondition-gate.ts {slug} --round {N}`
from the worktree root (oxlint + typecheck + `test:changed`, CI-aligned —
`bun run test:parallel` is verify's job, not a per-revision
requirement ([org-internal #2598])).
- Exit 0 required; a new test failure, lint error, or type error introduced
by the revision must be fixed before the Orchestrator proceeds to Phase D.
- Report the gate's per-check results in the revision summary.
---
### Phase C — Developer Revision: Protocol
The Developer applies this cognitive protocol before implementing any finding.
**Core principle:** reviewer findings are claims to evaluate, not orders to
execute. Verify before implementing. Technical correctness over performative
agreement.
#### Response Pattern
1. **READ** — Load every FAIL/WARN dimension finding file
(`<runs-root>/{slug}/reviews/code/round{N}/findings-{DIMENSION_CODE}.json`,
Tier 1 local). Do not react item-by-item.
2. **UNDERSTAND** — Restate each finding's requirement in your own words.
If you cannot, the finding is unclear — see STOP rule below.
3. **VERIFY** — Check the finding against codebase reality: does the cited
code exist at the given file:line? Does the cited rule apply?
4. **EVALUATE** — Is the finding technically correct for THIS codebase?
Does the proposed fix break existing functionality? Does it conflict
with the approved baseline (DAG task mode: the node spec + contracts in
`{epic-slug}/dag`)?
5. **RESPOND** — Fix it (correct), WAIVE it (`WAIVED-{finding-id}` →
one-line reason), or flag it (unclear / design-level question).
6. **IMPLEMENT** — One verified finding at a time, test each individually.
#### Evaluating Findings — 5-Check Gate
Before implementing any finding, run these checks:
1. **Cited code is real** — Open the file:line. Does the code exist and say
what the finding claims?
2. **Rule applies** — Does the cited checklist rule actually match this issue?
3. **Reason for current implementation** — Is the flagged behavior
intentional? Check the baseline (node spec + contracts in
`{epic-slug}/dag` for DAG task mode) for rationale.
4. **Fix does not break existing functionality** — Will the proposed change
cause a regression?
5. **Reviewer has full context** — Did the reviewer see the baseline (node
spec + acceptance criteria) and the diff context? An isolated diff review
can miss architectural intent.
```
IF a finding seems wrong:
WAIVE it: WAIVED-{finding-id} → <one-line technical reason>
IF you cannot easily verify a finding:
State the limitation in revision_summary.md and request direction.
IF a finding conflicts with the approved design:
Flag as a design-level question — do NOT silently change code against the design.
```
#### Unclear Findings — STOP Rule
```
IF any finding is unclear:
STOP — do not implement ANY finding yet.
Flag the unclear items in revision_summary.md and request clarification.
WHY: findings may be related. Partial understanding produces wrong
implementation across multiple items.
```
**Example:** The synthesis lists findings F1–F6. You understand F1, F2, F3, F6.
F4 and F5 are ambiguous about what change is expected.
- **WRONG:** implement F1, F2, F3, F6 now; ask about F4, F5 later.
- **RIGHT:** "Understand F1, F2, F3, F6. Need clarification on F4, F5 before
proceeding — they may interact with the same module."
#### YAGNI Check
```
IF a reviewer suggests "implementing this properly" (full abstraction,
generic API, configurable layer):
Check actual usage (codegraph callers / grep) BEFORE expanding scope.
IF unused: WAIVE — "No call sites found. Leaving as-is (YAGNI)."
IF used: Then implement properly.
```
#### Forbidden Responses in revision_summary.md
**NEVER** write:
- "You're absolutely right!" / "Great point!" / "Excellent catch!" — performative.
- "Thanks for catching that!" / any gratitude expression.
- "Let me implement that now" — before verification.
**INSTEAD:**
- Restate the technical requirement concisely.
- State the fix factually: `Fixed {finding-id}: <what changed> at <file:line>`.
- Just fix it and show the result in the diff.
#### Gracefully Correcting a WAIVE
If you WAIVEd a finding and were wrong:
```
"Verified {finding-id} — the reviewer is correct. My initial read missed
<reason>. Implementing now."
```
State the correction factually. No lengthy apology.
#### Implementation Order
For multi-finding revisions, after clarifying all unclear items:
1. **Blocking issues** — security, data loss, breaks existing functionality.
2. **Simple fixes** — typos, imports, naming, dead code.
3. **Complex fixes** — logic changes, refactoring within minimal-surgical scope.
Test each fix individually.
#### Rationalization-Anticipation Quick Reference
| Trap | Hold to |
|------|---------|
| "The reviewer flagged it as BLOCKER, so it must be real." | VERIFY cited code exists and rule applies before any edit. |
| "Just fix everything to avoid a re-review round." | EVALUATE each finding; WAIVE the wrong ones with reasoning. |
| "This finding looks plausible — I'll fix it without checking." | Open the file:line, confirm the behavior. |
| "I'll implement the clear findings and ask about unclear ones later." | STOP. Clarify ALL unclear items before implementing any. |
| "The reviewer wants it done 'properly' — full abstraction." | Check call sites first. If unused, WAIVE (YAGNI). |
| "I shouldn't push back — the reviewer is probably right." | Push back with technical reasoning. The diff is the acknowledgment. |
---
<!-- section:phase-d -->
## Phase D — Re-review Loop
After the Developer completes revision:
1. Increment the round counter (the Synthesizer updates the commit status in Phase B
with the new round's results).
2. Re-enter Phase A at step 2.5 — the mechanical-green gate re-runs on the
revised tree before any reviewer spawn (a red gate blocks dispatch exactly
as on round-1 entry) — then spawn the new round of reviewers against the
revised files.
**Convergence criteria**: see `core/skills/_shared/review-pipeline-phases.md` Phase D for the canonical table (stop conditions). `{MAX_ROUNDS}` is per-skill — code review binds the routing budget: **2** rounds default, **3** for high risk (a BLOCKER survived a round, or the diff touches a frozen cross-session contract) — `<instance-root>/workflow-routing.yaml` `review.max_rounds` ([org-internal #3567]). The code-review-specific addition is inheritance of the shared INFO-disposition rule. The shared Round-3 Escalation Guardrail is scoped to the `review-artifact` skill targets only and does not apply here.
**Round budget (mandatory, [org-internal #3567])**: when `current_round` reaches the budget (2; 3 for high risk) without convergence, do NOT enter another round. STOP: register every remaining BLOCKER/MAJOR finding as TD registry rows (`rules/ticket-lifecycle.md`), record the verdict (FAIL if any BLOCKER remains, else PASS — the debt is carried by the TD registry rows just registered), and close the loop. A review that cannot converge within budget is a design-level stall wearing review clothes (retro 2026-08-21 / [org-internal #3050]): in DAG task mode, map it to the DAG oversize signals (`<instance-root>/workflow-routing.yaml` `dag.size_derivation.oversize_signals`) and let the aggregator agent drive re-derivation + a `review-dag` re-run; otherwise record the design-level finding on the Epic, or open `{slug}/03-design-amendments` via `wiki 读写 API（见 TERMINOLOGY）` (`_shared/gitea-write-patterns.md` Pattern 1).
**Efficiency optimization**: see `core/skills/_shared/review-pipeline-phases.md` Phase D (same rule — from round 2 onward ([org-internal #2598]), only spawn reviewers for FAIL/WARN/UNRESOLVED dimensions; full sweep every 2 rounds). Round ≥2 reviewer input is delta-scoped to the revised files + their neighbors.
<!-- inject: ../_shared/compaction-round-boundary.md -->
---
<!-- section:phase-e -->
## Phase E — Final Report & Approval
**Orchestrator actions**:
1. Create the final report as a wiki page via `wiki 读写 API（见 TERMINOLOGY）` （`_shared/gitea-write-patterns.md` Pattern 1）with page_name `{slug}/reviews/code/final/report`
(DAG task mode: `{epic-slug}/reviews/code/final/report-task-{node-id}` —
node-scoped per the DAG Task Mode output-naming rule), using the template at
`core/skills/_shared/review-final-report-template.md`, substituting
`{REVIEW_TYPE}`: Code Review.
2. Update review status by posting a commit status via `POST /api/v1/repos/Octopus/octopus/statuses/{sha}` （`_shared/gitea-write-patterns.md` Pattern 8）with context `pipeline/review-code` and `converged: true/false`.
2.5. Ensure the final report wiki page `{slug}/reviews/code/final/report`
(DAG task mode: `{epic-slug}/reviews/code/final/report-task-{node-id}` —
node-scoped per the DAG Task Mode output-naming rule) is created with
the Accepted Risks table populated. Every ACCEPTED_RISK finding MUST
appear in this table with a Recommended Reactivation Trigger. If
the final report wiki page is not created, the review is NOT complete — verify's
tech debt registration step depends on this artifact (standard and
standalone-bugfix modes; DAG task mode is exempt — verify's task mode
sources ACCEPTED_RISK from the node's task-ticket review report instead,
see `verify/SKILL.md` §"DAG branch" Phase 5.5).
2.55. **Include a Round-1 Findings summary table in the final report.** The
final report MUST contain a `## Round-1 Findings Summary` section (table
columns `| Dim | Finding ID | Severity | Summary |` — one row per
round-1 finding; Summary = one-line 摘由/root cause). PRODUCER: the
round-1 Synthesizer writes the rendered table to
`<runs-root>/{slug}/reviews/code/round1/findings-summary.md` in
Phase B round 1 (wired as step 8 of
`_shared/synthesis_task.md` — "Round 1 only — findings digest"; shared
spec: `_shared/review-pipeline-phases.md` Phase B step 3
"Round-1 findings digest"; DAG task mode path:
`<runs-root>/{epic-slug}-task-{node-id}/reviews/code/round1/findings-summary.md`),
registering the `index.json` row (type `review-synthesis`, stage
`review-code`, produced_by `Synthesizer`; the round-verdict
`synthesis-return.md` keeps the worker-report type `other` per
`_shared/worker-report-persistence.md`).
The Orchestrator copies that file verbatim into the final report — it
does NOT read the findings JSONs (Phase B step 4). Rationale:
the findings JSONs are deleted at Tier 1 archive-at-close, so the final
report is the ONLY durable carrier of round-1 findings — retrospective
M7 (§2.9 first-round root-cause analysis) reads this table instead of
relying on commit messages (retro 2026-08-21 / [org-internal #3120]).
2.56. **Inline-ization — post the final round's findings as PR inline
comments** (consumer of frozen contract
`{epic-slug}/shared/review-inline-v1`, epic [org-internal #3533]; REQ-F-001/002/004).
After the final report page exists (step 1), post the FINAL round's
findings as Gitea PR inline comments so review conclusions land
line-level on the diff. Run the runner from the repo root:
```
bun <harness-package>/script/post-inline-comments.ts \
--pr {PR_INDEX} \
--findings "<worktree>/<runs-root>/{slug}/reviews/code/round{N}/findings-*.json" \
[--identity zcode-bot --sudo zcode-bot]
```
(DAG task mode: findings path
`<runs-root>/{epic-slug}-task-{node-id}/reviews/code/round{N}/findings-*.json`;
`{slug}` otherwise. Skip this step entirely when the review target has
no PR — issue-only standalone flows.)
- **Input scope**: ONLY the final (converged) round's findings files —
never re-feed superseded rounds. The runner normalizes Tier 1
`location` values (`path:line` and `path:start-end` ranges → the
first line; unparseable freeform locations are reported as
`unresolved` and never posted).
- **Noise**: INFO / DISCARDED / `WAIVED-{id}` findings never become
inline comments — enforced inside the tool (contract behavior 2);
the runner and this step add no filtering of their own.
- **Multi-round**: the tool deletes the identity's previous inline
comments before posting (contract behavior 5) — the PR diff page
always reflects only the latest round.
- **Accounting (mandatory)**: append the runner's last output line to
the review synthesis comment — the step 2.6 template below carries
the `Inline comments:` row — and mirror the same line in the final
report page's conclusions:
`inline: X posted / Y file-level / Z fallback / W failed`.
`fallback` = stale-line degradations (target line no longer in the
diff → re-anchored to the file's first diff hunk, contract behavior
4). Caveat: this server build returns HTTP 500 (not the contract's
422) for stale lines (TD-611), so stale items currently surface
under `failed` with `fallback` empty (walkthrough-n02 §差异);
`failed` = per-item failures (e.g. path not in diff with no
anchor). Both are ACCOUNTING ONLY — a partial failure
(some posted, some failed) NEVER blocks the review verdict (contract
behavior 6); the runner exits non-zero only when ALL comments fail,
and even then the review records the failure in the accounting line
and continues.
- **Identity**: production uses the orchestrator's admin token with
`--sudo zcode-bot` per the contract (runbook §2); the behavior-5
deletion target then defaults to the sudo login (the account the
comments post under). Without an admin token, omit both flags — the
runner resolves the token's own login as the behavior-5 identity.
- **Cleanup**: walkthrough/demo residue is removed with
`--cleanup-only` (deletes the identity's inline comments on that PR;
behavior-5 deletion as a standalone primitive). `--dry-run` gates it
to counting what would be deleted (`wouldDelete`), no deletion.
2.6. **Present the final report to the user and obtain approval** via the
`question` tool, passing `stage: "review-code"`:
> **Gitea approval timing (mandatory when the PR needs a human approval
> gate)**: register the reviewer FIRST via `gitea_pull__add_reviewer`,
> wait for the `REQUEST_REVIEW` registration to be recorded, and only
> THEN ask the human to approve. Approving before the reviewer is
> registered produces `official: false` on the approval — the gate
> treats the PR as unapproved and the approval must be re-done. See
> `_shared/review-pipeline-phases.md` Phase E step 3.5. ([org-internal #1931]: audit
> R2→R3 re-loop + re-approve, ~30min wasted on exactly this ordering.)
```
[Code Review Complete — {N} rounds]
Final verdict: {PASS | WARN | FAIL}
Converged: {true | false}
See full report: wiki page `{slug}/reviews/code/final/report` （DAG task mode: `{epic-slug}/reviews/code/final/report-task-{node-id}`）（via `gitea_wiki__get_page`，`_shared/gitea-read-patterns.md` Pattern 1）
All review artifacts: ticket #{issue_number} comments + wiki pages under `{slug}/reviews/code/`（DAG task mode: under `{epic-slug}/reviews/code/`, Tier 1 round artifacts under `<runs-root>/{epic-slug}-task-{node-id}/reviews/code/`）
Inline comments（only when step 2.56 ran；omit for no-PR flows）: `inline: X posted / Y file-level / Z fallback / W failed`
→ Approve and proceed? (yes / no / revise)
```
The `question` tool auto-approves per `core/rules/auto-approve.md` (stage `"review-code"`).
Do NOT advance without approval (auto or explicit).
2.7. **Issue checklist sync (mandatory on PASS).** Only once the verdict is
PASS, the report is finalized, AND the user has approved the final report
(step 2.6 above — if the user instead chose "revise", skip this sync: a new
round runs and the annotation happens on the eventual PASS round), sync the
source issue's checklist per the `issue-checklist-sync` L1 rule — follow
its "How to sync (each point)" procedure (identify source issue → fetch body
→ map → update, preserving non-checklist content) — do NOT wait for the
verify skill:
- Check off review-related items (e.g. "代码评审通过" / "reviewed") and
append `_(reviewed: round {N} PASS)_`.
- The `## 当前状态` review row is written by the `status-sync` poller
(`.gitea/scripts/status-sync-poll.ts`), NOT this skill — do NOT manually
`工单 API（见 TERMINOLOGY）update` the PR / 代码评审 / CI rows (per
`issue-checklist-sync.md` § Automated sync). If no `## 当前状态` section
exists and this is an incident / standalone-bugfix flow, create the empty
section (the poller never creates it); the poller then populates the PR
and review rows.
2.8. **Update the `## 工件索引` comment** (artifact index — Pattern 10). If
the source issue has an artifact index comment, update it in place per
`_shared/gitea-write-patterns.md` Pattern 10: add/update a `REVIEW-code`
row with 位置 = `{slug}/reviews/code/final/report`, 重读 = `CORE`
(DAG task mode: row id `REVIEW-code-task-{node-id}`, 位置 =
`{epic-slug}/reviews/code/final/report-task-{node-id}` — per the
Pattern 10 per-skill row map). If no index comment exists yet, create
one following Pattern 10's
creation + per-skill row map. Do NOT create a duplicate index comment
— check for the legacy prefix `## Pipeline 工件追踪表` first per
`core/rules/artifact-index-guards.md`.
---
## Orchestrator Rules (Mandatory)
See `core/skills/_shared/review-orchestrator-rules.md` for the mandatory
Orchestrator behavior rules. Substitute `{REVISION_ROLE}`: Developer.
---
## Output Artifacts Summary
See `reference/review-artifact-tree.md` for the `reviews/code/` directory tree.
To notify workflow completion, call the `signal_stage_done` tool.
---
## References
- `core/checklists/code-review.md` — The authoritative checklist for all 10
review dimensions.
- `core/schemas/` — Shared JSON schemas for reviewer output, synthesis,
and status.
- IEEE 1028-2008 — Software Reviews and Audits
- ISO/IEC/IEEE 24765:2017 — Systems and software engineering vocabulary
- Legacy design path (historical note, FLOW-F001 / audit-20260901): the
design-document precondition row formerly in the DAG task-mode table
(legacy `{slug}/03-design-index` / `{slug}/02-03-req-design` → node spec + frozen DAG
copy) referenced the legacy design pipeline, archived 2026-08-21 with [org-internal #3072]
phase 3 (see `<instance-root>/archive/skills/design/`); the live design baseline is
the node spec + cross-session edge contracts in the frozen DAG copy
`{epic-slug}/dag`.
Reference in New Issue Copy Permalink