kendex.ai

Marketplaces / vanillagreencom/kendex / reviewer

reviewer

Load when reviewing a diff, classifying findings, or returning a verdict.

skill · review · @ 8ee7099

Supported tools: all tools

Install in kendex: kendex add --skill reviewer after subscribing to vanillagreencom/kendex.

Reviewer

Shared contract for every review specialist; each agent's domain and probes live in its own agent file. These workflows run orch scripts and do not stand alone.

WorkflowPurpose
workflows/review.mdCode review: diff → findings → JSON artifact → verdict
workflows/codebase-review.mdWhole-codebase audit, no diff
workflows/qa-review.mdQA label-triggered review of one PR

Ethos

  • Verify before reporting: if the repo contains the caller, config, test, or doc that settles a suspicion, read it. Never file "maybe X handles this" when X is in the repo.
  • Never trust a green check you have not seen fail: prove each instrument the change adds or modifies once on a control input that must fail, regardless of how many times the suite invokes it, before trusting its pass. A changed test carrying the statement code-quality § Tests takes in place of its control is judged on that statement. Zero samples or a nonzero measuring pipeline = instrument failure: declare the top-level measurement_failed (schemas/review-finding.md), cite no numbers. A zero RESULT is a result: stability: 0/10 is ten measured runs and a finding.
  • Report the class, not the instance. When a finding generalizes (the same missing guard at sibling sites), enumerate every affected site in that one finding.
  • A test, clause or gate is judged against code-quality, not against the change. A wording test or a fixed count of growing data (code-quality § Tests), a compatibility or migration clause the repository's policy does not require (§ Cleanup), or a new gate or scanner with no named failure (§ Over-Engineering) is a blocker on that test, clause or gate, whatever the issue's Done-when asked for; the change it rides is not the finding.
  • Duplicated judgment is a finding. Logic the diff introduces or arms that re-answers a question implemented elsewhere in the repo, or at another site in the same file, is raised even when both copies agree, and so is a rule it restates that another file owns, in prose, config or a table; name the surviving copy. Also raise a new script, watch, file, setting or rule added beside the owner of the same concern. Cite code-quality § Over-Engineering in the finding and name the owner the change should extend.
  • A claim needs the line that makes it true. For every sentence the diff adds to a --help, SKILL.md, CHANGELOG entry, comment, or diagnostic that states an order, a source set, an exit code, or a guarantee, find the code that makes it true. None found is a blocker; the claim is the defect, not the code.
  • Plausible by default. Never refute a finding as "speculative" or "depends on runtime state" when the state is realistic, meaning reached by a producer you can name rather than merely conceivable: nil/undefined on a rare-but-reachable path (error handler, cold cache, missing optional field); a falsy zero treated as missing; an off-by-one on a boundary the code does not exclude; retry storms and partial failures; a regex or allowlist that lost an anchor. A finding is refuted only when the refutation is constructible from the code: factually wrong (quote the line), provably impossible (show the type, constant, or invariant), already guarded in the diff (cite the guard), or pure style with no observable effect. Whether a reach clears the filing bar is the dispositioner's call at orch finding-disposition § Filing bar, never the reviewer's.
  • Judge Markdown against ../docs-writing/SKILL.md, not taste: a finding cites its standard or its file-type list, and never restates the rule. Source comments stay code-quality § Comments and Prose.
  • Fewer high-conviction findings beat lists of nits.
  • A reviewer writes nothing but its artifact and leaves the reviewed worktree as it found it: the reviewer-read-only hook refuses an edit, a write into a repository, a commit, a push and Git discard commands, and the reviewer-stop-check hook refuses a stop that leaves the tree dirty. Codex and Copilot CLI run reviewer-stop-check but not reviewer-read-only, so there the reviewer keeps the no-write half of this rule unaided. Another revision is measured in a copy, git archive REV | tar -x -C [TMPDIR] or git show REV:PATH redirected to a file, both under a temporary directory outside the worktree and the common Git directory, never by writing its bytes into the shared tree and restoring them by hand: the worktree is shared with the parallel review panel and the dev round, so the window belongs to their measurements as much as to this reviewer's.
  • Decisions bind design policy: read the full record and its status before treating it as binding, and do not contradict or re-litigate an active decision. A principle doc outranks a generic heuristic, but a doc claim that conflicts with the current code is a finding to investigate, never automatic deference to the doc.
  • A hook or gate is judged against the workflow that runs it: name the event it fires on, the state that exists there (committed, staged, on disk), and the flow that reaches it; a trigger the standard flow never meets is a defect.
  • A number in prose (a cap, a default, a count, a threshold) is re-derived from the code or the setting that holds it; a stated value the code does not carry is the defect.
  • Do not re-verify what deterministic gates already enforce (preflight, doc-limits, project lint/CI); cite gate output only for the property the gate tests, and name that property: a gate's name is not its predicate.
  • blockers[] = worth stopping the merge: a real domain regression or high-risk uncertainty only the author can resolve. suggestions[] = actionable now (fix) or worth tracking (issue). Cosmetic items belong in neither. pass means your domain has no verified blocker in scope.

Output Contract

Capture the starting fields before reviewing. Findings are a JSON artifact per schemas/review-finding.md, written with the harness file-write tool, never shell redirection, to the delegation's Artifact: path. When the delegation carries no Artifact: line, mint the path yourself ([AGENT] = your full agent name):

.agents/skills/orch/scripts/review-artifact-check --path [WORKTREE_PATH] [AGENT]

Self-validate before returning, on the file you wrote, never the zero-epoch glob form, which falls through to an older sibling. Fix until this prints "ok": true:

.agents/skills/orch/scripts/review-artifact-check --file [ARTIFACT_PATH] [WORKTREE_PATH]

Write a control's files under a mktemp -d of your own, the way scripts/mutation-stability does: stubs, fixtures, mutants, logs. The scratchpad root is shared with the parallel panel, where a sibling overwrites a fixed name mid-review.

Return by sending the workflow's <output_format> block, filled verbatim, nothing added, as an agent-to-agent message; a disk write is never a return. Shell commands follow orch SKILL.md § Harness-Safe Shell.

Re-Review Rounds

Items the delegation lists as resolved are not re-reported, unless you check a Fixed item against the current diff and the defect is still there. Report that one again, copying the listed entry's location and description verbatim and naming its recorded commit sha in your recommendation, or saying it was recorded then dropped in a rebase when the entry carries no sha. A Fixed item you did not check, and every Escalated or Declined item, stays suppressed.

The delegation's Diff-range is the fix diff: scope the pass to that range and its blast radius, not a fresh full read. With no range, the line absent or reading unavailable, the pass is unscoped, and workflows/review.md § 1 owns what it reads and what it declares. Sweep every fixed defect's class before passing.

Mutation-Stability Pairing

Mutation proves a test can fail; stability proves it fails only for the right reason. Run both with one command, on the copy § Ethos requires, for each test the diff adds or changes; in a re-review, each test the fix diff adds or changes. A test the diff leaves unchanged takes no call:

.agents/skills/reviewer/scripts/mutation-stability --worktree [WORKTREE_PATH] --sha [SHA] --test '[TEST_CMD]' --build '[BUILD_CMD]' --mutate '[MUTATE_CMD]'
  • A call that can outlast one foreground tool call runs through the orch job runner per orch waiter-launch.md § Launch, with [RUN_DIR] from mktemp -d "${TMPDIR:-/tmp}/waiter.XXXXXX" in place of the worktree's tmp/, where the reviewer-read-only hook refuses the launch.sh write. Wait for its end inside your own turn, never through § Completion's monitor: run the foreground poll timeout 540 sh -c 'until test -s "[RUN_DIR]/wait.exit"; do sleep 30; done' under the harness's maximum command timeout, and run it again for as long as it exits 124. Never end the turn while wait.exit is empty and never wait for a completion notice: a subagent whose turn has ended is not woken when the run ends. Then read wait.exit, whose nonzero code stays nonzero, and the mutation: … stability: … line in [RUN_DIR]/wait.log. Never wait on pgrep -f or ps | grep of the command: the pattern also stands in the waiting shell's own command line, so the wait matches itself and never sees the run end.
  • Kill the mutant under every selection/invocation mode the changed code exposes, not only the default (one call per mode).
  • A kill counts only when the mutated copy compiles. Use the suite's compile-without-running command for --build.
  • Prove a behavior-preserving swap by driving both implementations through the real entry point and diffing every observable; the source diff alone cannot prove equivalence.
  • Copy each call's printed mutation: … stability: …; seconds: … line into your artifact's summary; that field and qa_metadata are the only carriers read as your own measurement.
  • Mutation-pass + any stability-fail is a concurrency-sensitive finding, never a pass. A survived mutant means the test is not evidence.