Review

Code Review

Evidence-based and blunt. Every finding references a specific file:line, function, or component. NEVER sugarcoat, NEVER pad with praise, NEVER report a concern that is not grounded in the diff or the codebase. Style preferences are not defects — but a violation of AGENTS.md or CODESTYLE.md is a convention finding, not a preference.

If you are the diff's author

If you wrote the diff you are reviewing now, say so explicitly in the report's Verdict and treat this pass as necessary but insufficient. challenge-review/SKILL.md covers why self-review inherits your own design assumptions and what to recommend instead — read it, don't re-derive it here.

Before reviewing

1. Ask the user for numbered acceptance criteria (DoD). If there are none, derive them from the PR description or the linked issue, mark them (inferred), and proceed — do not stall, and do not invent criteria silently.
2. Resolve the base first: git fetch, then diff against the branch the PR actually targets. werf maintains release branches (1.2, 2.63, 3, …), so main is the wrong base for a backport. State the resolved base in the report. For uncommitted work, review git diff / git diff --cached instead.
3. If the PR head is not checked out, take every file:line from that commit's blob (git show <head>:<path>), never from the worktree. The worktree sits on the base, so any file the PR itself changed has different line numbers there, and a comment anchored on a worktree line number lands on the wrong line — or is rejected outright.
4. Read every changed file. Then trace callers of the changed exported symbols whose signature or behavior changed, and of anything crossing a persistence boundary — via LSP call hierarchy and references, not grep. LSP indexes the worktree, so when the head is not checked out it answers about the base: add a worktree at the head first, or read blobs and say in the report that call tracing was limited.
5. For 10+ changed files, split the reading by area (e.g. new files, storage/cleanup, build pipeline) across subagents if your harness has them, and synthesize the findings yourself.
6. If the worktree holds the branch and task works, run task build and task test:unit — a review that never compiled the change is an opinion. NEVER run task format: it would rewrite the diff under review.

Technical perspective

Code structure and correctness only — user impact belongs to the product perspective.

• Conventions: AGENTS.md and CODESTYLE.md are the standard. This project prefers a bit of duplication over abstraction and minimizes interfaces and generics — flag deviations in either direction, and NEVER report duplication as a DRY defect on its own.
• Correctness: error wrapping and discarded errors, context propagation and cancellation, goroutine and errgroup ownership, nil map writes, typed-nil interfaces.
• Security: least privilege, input validation, secret handling, container security.
• Observability: when deploy or registry operations fail, is the cause visible in the logs?
• Testability: can the change be exercised without a cluster or a registry?
• Consistency with the werf, nelm, Docker, and Container Registry patterns already in the project.

Cover the ones the diff actually touches; stay silent about the rest.

Tests as evidence

Passing tests, high coverage, and the author's confidence are not evidence of correctness, whoever wrote the diff. Read test-the-tests/SKILL.md and run its mutation loop against every load-bearing test: mutate the implementation and confirm the test fails, rather than reading the assertions and trusting they'd catch a regression. This is not optional — skipping it because the tests "look thorough" is exactly the failure mode it exists to catch.

For a non-trivial or high-risk diff, or a diff that touches tests or verification infrastructure, also read challenge-review/SKILL.md as an independent challenge pass. It covers check-gaming detection (weakened assertions, quietly skipped tests, mocked-out critical behavior, and more) in one place, so this list doesn't drift from it again.