open/voyage
1
0
Fork 0
Code Issues Releases 13 Activity

merge: main (7d, 7c) into 7e

Browse source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-17 18:20:08 +02:00
commit 0db3fe4be3
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
49 changed files with 1206 additions and 120 deletions
Download patch file Download diff file Expand all files Collapse all files

168
skills/reviewing-adversarially/SKILL.md Normal file
View file

@ -0,0 +1,168 @@
---
name: reviewing-adversarially
description: "The stance of a reviewer who judges someone else's work. No praise, no finding without a source (file:line or a quoted line), no downgrade without a reason, an honest account of what was read in full and what only as a summary, a Declined to Judge list so nothing set aside disappears, every claim of done or green checked against the diff and the output rather than the report, and the review axes kept separate. Use when judging a diff, an implementation plan, a task brief or a helper's report before it is merged, approved or acted on, when deciding whether delivered work matches what was promised, or when an author asks for a quick approval."
---
# Reviewing adversarially
## Overview
How a reviewer in Voyage judges. It holds for every agent whose job is a
verdict on someone else's work: the two `/trekreview` reviewers and their
coordinator, `scope-guardian`, `plan-critic` and `brief-reviewer`. The agent's
own role (its dimensions, its finding schema, its verdict rule) stays in the
agent; this skill is what all of them share.
Being useful to the author is not the same as agreeing with the author. A
review the author enjoys reading but that lets a defect through has failed.
## When to use
- A diff, a plan, a brief or a helper's report is judged before it is merged,
approved or acted on.
- Someone asks whether delivered work matches what was promised, or whether a
step is done.
- The author asks for a quick thumbs-up, says "looks good, right?", or says a
problem is cosmetic.
- Not for: improving the wording or style of a text (that is editing, not a
verdict), or running Claude Code's own `/code-review` engine, which brings
its own method.
## Process
### 1. No praise
- Write no sentence whose job is to commend the work or the author: "clean",
"tidy", "nice", "readable", "solid", "well done", "looks good". A part
with no finding gets no sentence; silence is the verdict.
- Do not open with agreement ("Absolutely", "Great point", "You're right").
Do not restate the author's claim in stronger words than they used, and do
not add enthusiasm they did not express.
- "Right?" and "don't you think?" are questions to evaluate, not cues to
confirm. Before you endorse anything, look for at least one real risk; if
there is none, say what you checked.
- Measured: without this skill, 2 of 6 review runs praised the code ("tidy",
"small and readable"), both where the author asked "looks good, right?".
### 2. No finding without a source
- Every finding points at its evidence: **`file:line`** in the file as it
stands after the change (not the line in the diff), or a **verbatim quote**
of the plan, brief or report line it concerns.
- A function name, a file name alone or "the date handling" is not a source.
If you cannot point, the finding is either dropped or moved to Declined to
Judge (step 6) with what would settle it.
- A finding about deleted code cites the old line as `<rev>:<path>:<line>`.
- Measured: without this skill, 6 of 6 runs raised at least one finding with
neither a line nor a quote (8 findings in all).
### 3. A claim is checked against evidence
A report is a claim. You judge the work, not the report.
| Claim | Requires | Not sufficient |
|---|---|---|
| "Step done" / "helper finished" | the diff shows the change | the helper's report |
| "Tests pass" | output you saw: 0 failed, and the tests that matter not skipped | "npm test is green" in a report |
| "Test added for X" | the test, in the diff, asserting X | a test file that exists |
| "Requirement met" | each requirement traced to a line | the suite passing |
| "Bug fixed" | the original symptom, now checked | code changed near it |
"The helper said done" requires the diff, not the report. When a reviewer
cannot run anything, it says that the run was not seen and judges from the
code alone.
### 4. Say what you read, and how
- State what you read in full, and what you saw only as a summary, a
signature list or a line count.
- A file seen as summary only gets **no verdict**: it is "not reviewed",
never "probably fine". Summary-only coverage covers declared signatures,
nothing more.
- Do not judge what you did not measure. "Probably fine", "looks correct",
"believable", "should be fast enough" are verdicts with no evidence: cite
the evidence, or move the item to Declined to Judge.
- Measured: without this skill, 4 of 6 runs judged something they had not
measured ("probably fine because the code is linear", "look correct as
written", "npm test is green is believable").
### 5. No downgrade without a reason
- Severity comes from the agent's own scale (the rule catalogue for
`/trekreview`, the plan review's tiers for the plan critics). An author, a
lead or a deadline calling something "a nit" or "cosmetic" is not a reason.
- When a rule lets you lower a finding, name the rule and what catches the
defect instead (`lowered_from`, `caught_by`). With no named rule, keep the
tier or drop the finding as wrong; never soften it in between.
### 6. Declined to Judge: nothing disappears silently
Before the verdict, write a separate list headed **`## Declined to Judge`**
(in `review.md` it is the required section of that name; in other outputs,
a section with that title before the verdict and the JSON block). One line
per item you considered and did not rule on:
```
src/receipt.mjs - summary only in the bundle - needs the full diff
plan "handles 10 000 plants in under 50 ms" - no benchmark in the change - needs a timed run
```
An item that is also a finding (a file nobody could review) is listed here
too. An empty list is written `None.`, which says you set nothing aside.
Measured: without this skill, 0 of 6 runs had such a list; the set-aside
items were mixed into the findings at made-up severities.
### 7. The axes stay separate
- Brief conformance, code correctness, scope, security and design are
different axes. Report each on its own; do not merge them into one score
or rerank findings across them.
- Name the worst issue within each axis, never a single winner across axes.
- The verdict comes from the agent's own rule (a threshold table, a gate),
not from an overall impression.
## Red flags
| Thought | What it means |
|---|---|
| "The rest is clean, I'll say so to be fair" | That sentence is praise. A part with no finding gets no sentence (step 1). |
| "They asked 'right?', a short yes will do" | It is a question. Evaluate, then answer with findings or with what you checked (step 1). |
| "Everyone can see which function I mean" | Then cite its line. A name is not a source (step 2). |
| "The report says the tests pass" | A report is a claim. What did the diff and the output show (step 3)? |
| "I only saw a summary, but it's probably fine" | Summary only means not reviewed. Put it under Declined to Judge (steps 4 and 6). |
| "The lead says it's cosmetic" | Not a reason. Keep the tier, or name the rule that lowers it (step 5). |
| "I'll leave out what I couldn't check; it would only clutter the review" | Leaving it out is how it disappears. List it (step 6). |
| "The style issue is the most visible, I'll lead with it" | Axes stay separate; the worst issue is named per axis (step 7). |
## Enforced by
- `lib/review/findings-schema.mjs` rejects a `/trekreview` reviewer finding
with no `file`, a non-integer `line`, or a line past the end of the file,
and `/trekreview` re-asks the reviewer. It is **partial**: a file-level
finding with line 0 passes, and nothing checks that the cited line
supports the claim.
- `lib/review/coordinator-contract.mjs` drops a finding with no citation
(`no-citation`) in the deterministic mirror of the coordinator. Partial in
the same way, and it covers `/trekreview` only, not the plan critics.
- `lib/validators/review-validator.mjs` fails a `review.md` (review_version
1.1 or later) without a `## Declined to Judge` section. It checks that the
section exists, not what it lists.
- Praise and flattery are **prose only**: no check reads the reviewer's
prose for them. So are downgrades without a reason, the account of what was
read and how, a Declined to Judge list in any output other than
`review.md`, claims checked against evidence, and the separate axes.
## Credits
Ideas adapted, text rewritten; no code or prose copied.
- obra/superpowers @8ca22db, `skills/requesting-code-review/code-reviewer.md:33-48`: the
"declined to judge" list before the verdict, so nothing set aside is dropped silently; and
`skills/verification-before-completion/SKILL.md:38-48`: the claim / requires / not sufficient
table, including "agent completed" requiring the diff, not the agent's report. MIT, Copyright (c)
2025 Jesse Vincent.
- mattpocock/skills @c55ee46, `skills/engineering/code-review/SKILL.md:76-78`: two review axes
are reported separately and never reranked into one winner. MIT, Copyright (c) 2026 Matt Pocock.
- The measured lines come from Voyage's own baseline review runs without this skill; see
`docs/skills-authoring.md`. Step 1 keeps the substance of the earlier per-agent
"Interaction Awareness" block, which this skill replaces.

119
skills/writing-criteria/SKILL.md Normal file
View file

@ -0,0 +1,119 @@
---
name: writing-criteria
description: "Criteria discipline for the checks Voyage runs itself: each brief success criterion, each plan Verify, Regression and End-to-end line and each Verification bullet is one command the criteria runner accepts, written as its first backticked span (a known test runner, plain words, no pipe, no &&, no shell), and a check that needs more becomes a script declared as bash tests/<name>.sh. Each criterion must be able to go red: a command that exits 0 whatever the code does is no criterion. Use when writing or reviewing success criteria, a Verify, Regression or End-to-end line or a Verification section, or when a validator reports a criterion as not runnable (BRIEF_SC_NOT_RUNNABLE, PLAN_STEP_VERIFY_NOT_RUNNABLE)."
---
# Writing criteria
## Overview
Voyage runs the checks a brief or a plan declares, and it decides "done" from their exit codes.
`lib/verification/criteria-runner.mjs` runs each one with no shell, as its words, and only when it is a
known test-runner invocation. Anything else is NOT RUN: never executed, never green. In `/trekexecute`
Phase 7 an unrunnable plan criterion stops the run; in `/trekreview` an unrunnable success criterion is
an absent measurement. A criterion that runs but cannot fail measures nothing either. This skill is the
one place that says how to write both right.
## When to use
- Writing a brief's `## Success Criteria`.
- Writing a plan step's `Verify:`, `Regression:` or `End-to-end:` line, or the plan's `## Verification`.
- Reviewing them: `brief-reviewer` and `plan-critic` preload this skill.
- A validator reports `BRIEF_SC_NOT_RUNNABLE`, `PLAN_VERIFICATION_NOT_RUNNABLE`,
`PLAN_STEP_VERIFY_NOT_RUNNABLE`, `PLAN_STEP_REGRESSION_NOT_RUNNABLE` or `PLAN_END_TO_END_NOT_RUNNABLE`.
- Not for: writing the test itself and seeing it fail (that is `voyage:test-first`), or starting the app
to look at a change by hand (Claude Code's own `run`).
## Process
1. **One criterion, one command.** No `&&`, `;`, `|`, `>`, `<`, `$(` or backtick inside it. Two things to
check are two criteria, or one script (step 5). A line that chains the suite, a setup and three
checks is refused whole, and then none of it is measured.
2. **The command is the first backticked span on the line.** The runner reads the first span as the
command. Name the feature in plain words, not in backticks before the command:
``- shelf count prints the number of books: `bash tests/count.sh` exits 0``. A line that opens with
the feature in backticks (`` `shelf count` prints ... ``) declares `shelf count` as its command.
3. **Use a runner the runner knows.** These forms run:
| Form | Note |
|---|---|
| `npm test`, `npm run <script>` | the script must be in `package.json` when the check runs; a plan may add it |
| `node --test <files>` | a glob only on Node 21 and later; elsewhere name the files |
| `vitest`, `jest`, `pytest`, `python -m pytest`, `uv run pytest` | with a short list of read-only options |
| `cargo test`, `go test`, `make test` | nothing may follow `make test` |
| `bash tests/<name>.sh`, `/bin/bash tests/<name>.sh` | a script under `tests/`; the second pins the system bash |
| `git status`, `log`, `diff`, `show`, `ls-files` | read-only only; `git grep` is not on the list |
4. **Plain words.** No shell reads the line, so a word that means something in a shell is refused: a
quote, a backslash, `$`, a brace, a leading `~`, a word starting with `@` (a git revision such as
`@~1` is the one exception), a path that climbs out with `..` or starts at `/`, an environment prefix
(`CI=1 npm test`), and any option not on the runner's read-only list. A trailing `# comment` is
dropped, unless it holds a quote, `$`, backtick or backslash; then the line is refused.
5. **A check that needs more is a script.** The product's own binary, a temporary folder, an
environment variable, comparing output, `jq`, `grep`, `curl`: put it in a script under `tests/` that
exits non-zero with a message when the check fails (`set -eu`, an explicit `exit 1`), and declare
`bash tests/<name>.sh`. The script goes in the `Files:` of the step that builds the behaviour. This is
how an `End-to-end:` line runs the product the way a user does.
6. **It must be able to go red.** Before writing a criterion, name the wrong result that would make it
fail. The runner decides on the exit code alone: the `→ expected:` text after the command is for the
reader and is never compared. So a command that exits 0 whatever the code does is no criterion:
`git status --porcelain` "expected: no output" exits 0 on a dirty tree too, `git log` and
`git diff --stat` report and exit 0, and `npm test` alone proves no new behaviour unless a test for
that behaviour is in it. A check on output goes in a script that compares it and exits 1. A step's `Verify:` is ONE command, the step's own new test, seen red before the
step; a `Verify:` that runs a test file which already passes can never be red. The suite the step must
not break goes in `Regression:`.
7. **Check it before handing it on.** `node ${CLAUDE_PLUGIN_ROOT}/lib/validators/plan-validator.mjs
--strict --json <plan>` and `lib/validators/brief-validator.mjs` name every unrunnable criterion.
A refused criterion is rewritten (steps 1 to 5), never kept with a note.
### Examples
Each line marked `Runs:` is accepted by the criteria runner, each `Refused:` is refused, and each
`Never red:` is accepted but exits 0 whatever the code does (a pin in `npm test` holds all three).
- Runs: `npm test`
- Runs: `npm run typecheck`
- Runs: `node --test tests/count.test.mjs`
- Runs: `bash tests/count.sh`
- Runs: `/bin/bash tests/e2e.sh`
- Runs: `pytest -q tests/test_count.py`
- Runs: `cargo test --workspace`
- Refused: `npm test && npm run lint` (two commands: write two criteria)
- Refused: `node bin/shelf.mjs count` (the product's binary is no test runner: `bash tests/count.sh`)
- Refused: `node bin/shelf.mjs list --json | jq length` (a pipe: move the check into a script)
- Refused: `SHELF_FILE=tmp/s.json node --test tests/cli.test.mjs` (an environment prefix: set it in the test)
- Refused: `node -e "process.exit(0)"` (node, but not `node --test`)
- Refused: `npx tsc --noEmit` (not a runner: `npm run typecheck`)
- Refused: `curl -s localhost:8080/health` (not a runner: a script under `tests/` that calls it)
- Refused: `git grep -q count -- src` (git grep is not a read-only subcommand on the list)
- Refused: `make test lint` (nothing may follow `make test`)
- Never red: `git status --porcelain`
- Never red: `git log --oneline -2`
## Red flags
| Thought | What it means |
|---|---|
| "One line with `&&` checks it all" | The runner refuses the whole line; none of it is measured. Two criteria, or one script. |
| "Name the feature in backticks, then the command" | The first backticked span is the command. The feature goes in plain words. |
| "A user runs `node bin/app.mjs`, so that is the check" | It is still no test runner. A script under `tests/` runs it. |
| "A quick `node -e` one-liner checks the JSON" | Refused. The same code in a test file runs as `node --test`. |
| "`npm test` exits 0, so the feature is done" | It exited 0 before the change too, unless a test for the feature is in it. |
| "`git status --porcelain` → expected: no output fails on a dirty tree" | The runner reads the exit code, never the expected text. It exits 0 either way: it can never go red. |
| "`Verify:` runs the step's test plus a CLI check" | `Verify:` is one command, the step's new test. The rest is a script or `Regression:`. |
| "NOT RUN is not a failure, so it can stay" | NOT RUN is never green: an unrunnable plan criterion stops `/trekexecute` Phase 7. |
## Enforced by
- `lib/verification/criteria-runner.mjs`: deterministic. It runs only what its allowlist accepts, with no
shell, and reports everything else NOT RUN, never green.
- `lib/validators/plan-validator.mjs` and `lib/validators/brief-validator.mjs`: the same allowlist when
the plan or brief is written (`PLAN_*_NOT_RUNNABLE` are errors in a plan, `BRIEF_SC_NOT_RUNNABLE` a
warning in a brief).
- prose only: that a criterion can go red and that it tests what the brief asks for. `brief-reviewer`
and `plan-critic` judge that by reading; no check runs it.
## Credits
- Voyage's own content; nothing is borrowed. The rules restate `lib/verification/criteria-runner.mjs`.
The red flags come from Voyage's own baseline runs without this skill (`docs/skills-authoring.md`).