asterinas/.agents/skills/aster-code-review/spec/interface.md

Interface & Output

How the skill is invoked

aster-code-review is a skill, not a binary. You trigger it from inside a Claude Code or Codex session; the host agent then runs the orchestration in execution_model.md. There is no command on your PATH — though the skill also ships a headless CLI (below) for running it from a plain shell or from CI. However it is triggered, the skill understands one small argument string, and that string begins with a mode word:

diff   <base>              <output> [--overwrite] [--per-persona-context=auto|yes|no]
files  <path[:lines] ...>  <output> [--overwrite] [--per-persona-context=auto|yes|no]

• diff — review a Git change: the defects in what the working tree adds over a <base>.
• files — review target code: the defects in a set of files, optionally narrowed to line ranges.

Both modes write the same kind of review file (below) and run the same persona fan-out (see execution_model.md); they differ only in what code they put under review.

Running it headless — the aster_code_review.sh CLI

Usually the skill runs inside an agent session. It also ships a headless CLI at the skill root, aster_code_review.sh, that runs the whole skill from a plain shell by driving an agent for you:

ACR_AGENT_PROFILE=<name> aster_code_review.sh <mode> <args…> <output> [flags]

The arguments are the argument string above, so the CLI is identical to an in-session invocation; the only addition is the agent, chosen by the environment rather than an argument (a given environment exposes only a few agents):

• ACR_AGENT_PROFILE (required) — the agent profile to run under (see benchmark.md for the profile format and the shipped claude / codex / codex_workflow profiles).
• ACR_PROFILE_VARIANT (optional) — smoke for the low-effort overlay.

It reviews the current working tree, so cd into the repo (or a scratch worktree) first. Under the hood it builds the invocation prompt and hands it to the shared launcher scripts/run_agent.sh, which is the one thing that turns a profile into a running agent. The benchmark and the PR-review CI both drive the skill through this one CLI.

The rule: HEAD is the head — there is no head argument

Whichever mode you use, the skill reviews relative to the current checkout (HEAD); there is never a "head" or "right endpoint" argument. diff <base> reviews the commits the branch added to reach HEAD — the series merge-base(<base>, HEAD)..HEAD; files reviews the working-tree contents of the named files at HEAD. To review at a specific commit, branch, or tag, make it HEAD — git checkout <ref>, or git worktree add a scratch tree and cd into it — then run the skill.

This keeps the interface small and gives one mental model across PR review, periodic quality scans, and the self-review loop:

• no head/range bookkeeping — you name a base (or some files), never two endpoints;
• diff reviews committed commits, so commit your work first — the self-review loop commits each round, making its work part of the series; uncommitted edits are not in a diff review (review them with files, or commit them);
• files reviews the working tree as it stands, so it does cover uncommitted edits to the files you name.

The earlier <base..head> range is gone: an explicit head endpoint contradicts "HEAD is the head." Reviewing an arbitrary historical range means checking that range's head out first.

Mode 1 — diff <base>: review a Git change

diff <base> reviews the commit series the branch added over its merge-base with <base> — the commits merge-base(<base>, HEAD)..HEAD, oldest first, each presented as its message and its diff:

git log -p --reverse $(git merge-base <base> HEAD)..HEAD

Reviewing the series rather than one flattened diff is what lets the skill judge per-commit intent (does each commit do what its message claims?) and commit hygiene (imperative subject, atomic commits — a Maintainability concern), on top of the code itself.

• On a clean branch this is what the branch added since it diverged from <base> — the pull request. <base> may be any ref or SHA (main, origin/main, a tag, a short SHA).
• Using the merge-base (not <base> directly) means a <base> that advanced after you branched does not inject its upstream commits as spurious reversals — only the branch's own commits are reviewed.
• Committed commits only. Uncommitted edits, and brand-new untracked files, are not in a diff review — commit them first (the self-review loop commits each round), or review a specific file with files <path>.

Mode 2 — files <path[:lines] ...>: review target code

files reviews the working-tree contents of the files you name, optionally narrowed to line ranges. It is for auditing existing code rather than a change — periodically scanning select files to raise quality, or focusing a review on one area.

Each target is a path, optionally followed by line ranges:

TARGET := PATH | PATH ":" RANGES | '"' QPATH '"' | '"' QPATH '"' ":" RANGES
RANGES := RANGE ("," RANGE)*
RANGE  := N | N "-" M           # 1-based, inclusive, N ≤ M

• kernel/src/sched/fair.rs — review the whole file.
• kernel/src/sched/fair.rs:120-180 — review only lines 120–180.
• kernel/src/sched/fair.rs:120-180,240-260 — two ranges in one file.
• Repeating a path is allowed; its ranges are unioned, and overlapping ranges are merged.

The named lines are the review scope; the rest of the file and the wider repository remain readable as context — exactly as in diff mode, where the diff is the unit of review but a pass may read around it.

Quoting paths

A path that contains a space or a colon — or whose final segment looks like a line range — would confuse the bare grammar. Wrap it in double quotes:

• "a path with spaces.rs" — whole file, spaces and all.
• "weird: dir/mod.rs":30-60 — colon in the path, plus a range.
• "gen/12-20" — a file literally named 12-20, reviewed whole (quotes stop it being read as path:range).

Unquoted, the parser splits the line spec from the right: it peels a trailing :RANGES only if the suffix fully matches the range grammar, otherwise the whole token is the path. So weird:dir/mod.rs — whose tail is not a range — is already a valid bare path; quotes are the explicit escape for the genuinely ambiguous rest.

How the argument string is parsed

The skill receives its arguments as one raw string and tokenizes it itself, deterministically — it does not rely on a shell to do so. When you type /aster-code-review files "a b.rs" out.md in Claude Code, the prompt is not a POSIX shell: the quotes arrive intact, so the skill must own the parse. The grammar:

1. The first token is the mode word, diff or files.
2. The remainder is split into tokens on whitespace, except inside double quotes — a quoted span is one token, spaces and colons included.
3. Flags (--overwrite) are pulled out.
4. The last positional is <output>; everything before it is the target list — one <base> for diff, one or more targets for files.
5. In files mode each target is parsed by the grammar above: quotes strip to the bare path, and an unquoted trailing :RANGES is split from the right.

Owning the parse — rather than leaning on the agent or a shell to pre-split the string — makes it identical no matter how the skill was triggered, and keeps it a deterministic step the benchmark exercises directly.

The argument table

Argument	Meaning
diff | files	Required, first. The review mode.
<base>	(diff) Required. Any ref or SHA; the review covers the commit series merge-base(<base>, HEAD)..HEAD — each commit's message + diff.
<path[:lines] ...>	(files) Required, one or more. The files (and optional line ranges) to review, in the working tree.
<output>	Required, last. Where to write the review; refuses to clobber an existing file unless --overwrite is given.
--overwrite	Replace the output file if it already exists.
--per-persona-context	yes fans out one isolated agent per persona (best recall); no runs one combined agent over all personas (cheaper, lower recall); auto (default) currently resolves to yes. Affects orchestration only — see execution_model.md.

Rationale for the argument shape

• A leading mode word, not a guessed argument. The mode is named explicitly because the alternative — detecting whether the first token is a ref or a path — is genuinely ambiguous (main vs main.rs, a ref that is also a filename, a path absent at HEAD) and against this interface's no-magic ethos. One extra word buys an unambiguous, self-documenting parse and room for a third mode later.
• Output is the last positional, cp-style. With a variadic target list in files mode, the output cannot live in a fixed second slot. "Last positional is the destination" is the familiar cp src... dest convention and keeps both modes' grammars uniform. It stays mandatory — there is no silent default review file, which keeps the benchmark deterministic and never surprises a human with a file they did not name.
• HEAD is the head, so no head argument. There is never a second endpoint: diff reviews the commit series up to HEAD, files the working tree at HEAD. This replaces the old <base..head> range and the removed --worktree flag. The cost is that reviewing an arbitrary historical range means checking its head out first; the gain is one consistent model across PR review, scans, and the self-review loop (which commits each round).
• Quoting is the skill's, not the shell's. Because the trigger is not a shell, the skill self-tokenizes and treats double quotes as its own grammar — the only robust way to let a path hold spaces or colons given that the quotes survive into the raw argument string.
• An existing review is never clobbered by accident. Overwriting is opt-in via --overwrite: a loop that means to replace its previous review passes the flag; a hand-annotated review stays safe by default.
• Cost is an explicit, opt-in knob. --per-persona-context=no collapses the five-way persona fan-out into one combined pass — cheaper, but with measurably lower recall (a single context dilutes focus; see execution_model.md). It is never the silent default: auto resolves to the recall-first fan-out today and will graduate to a benchmark-driven heuristic. Recall-first means the cheap path is something a caller asks for, not something the skill does behind your back.

The review file

The output is one Markdown file: YAML frontmatter, a constructive # Summary, then a ## <Persona> section per activated persona, each holding one ### <location> subsection per comment.

---
date: 2026-06-28
mode: diff
base: a1b2c3d
head: 9f3a1c2
branch: my-feature
title: "Report real RSP before the trap in TrapFrame"
---

# Summary

<What the change does well; the top issues that need attention, ranked by
severity; any structural recommendations.>

## Maintainability

### `ostd/src/arch/x86/trap/mod.rs` line 12

> ```diff
> +    let n = frame.size();
> ```

`descriptive-names` (nit): `n` is opaque at the point of use.

**Fix.** Rename it to `frame_size` so the call site reads on its own.

## Hardware

### `ostd/src/arch/x86/trap/mod.rs` line 30

> ```diff
> -    _pad: usize,
> ```

`16b-align-rsp-before-call` (critical): removing `_pad` makes
`size_of::<TrapFrame>()` no longer a multiple of 16, but the CPU aligns `%rsp`
to 16 bytes on trap entry (System V AMD64 ABI). An odd-sized frame misaligns the
stack and is unsound for SSE.

**Fix.** Restore the padding and add a
`const_assert!(size_of::<TrapFrame>().is_multiple_of(16))`.

The frontmatter records the mode and what was reviewed:

• diff records the base (the merge-base) and the head (HEAD's short SHA). It reviews committed commits, so head is never -dirty.
• files records the reviewed files (with any ranges) instead of a base; head is HEAD's short SHA, suffixed -dirty when the working tree has uncommitted edits (files mode reviews the working tree).

The rest — date, branch, optional title — is common to both modes.

The comment model

Comments are grouped under their persona's ## section, headed by that persona's review concern — Maintainability, Correctness, Security, Hardware, Documentation — rather than carrying a per-comment tag. (The Development persona's section is titled Correctness: a bare ## Development would read oddly next to the others, since all of this is development.) Each comment is a ### subsection that carries:

• a location heading — `path` line N (or lines N-M), or a commit locus like `commit abc1234 message` for a finding about a commit message;
• a quoted snippet of the relevant lines — a diff hunk (diff mode), the source lines (files mode), or the offending part of a commit message;
• a prose body opening with the grounding and severity — `<short-name>` (<severity>): for a guideline, <description> (<severity>): for a bug — then a statement of the problem, constructive and specific;
• a separate Fix. paragraph (see below).

The grounding is one of two kinds — and this split is the heart of the subjective-plus-objective stance from motivation.md:

• guideline-backed — the rule short-name it rests on (a lowercase kebab identifier, e.g. validate-at-boundaries), rendered as code — making a subjective call traceable to the shared standard rather than an opinion;
• bug-described — for an undeniable defect (logic error, UB, data race) that no rule covers, a short plain-language description of the defect (e.g. "Off by one", "Use after free"), rendered as prose. A description, not a coined short-name — so it never reads as a rule — and never the bare word bug. This is a first-class, encouraged category: a pass must hunt bugs by reasoning, never suppress one because "no rule covers it". Requiring a citation must never silence a real defect that no rule happens to name.

The severity is critical, major, minor, or nit (critical: must fix; major: should fix; minor: worth fixing; nit: optional or stylistic). It is the persona's judgement, used to rank the Summary's top issues — and is not used to gate benchmark recall.

Every comment must propose a remedy. Pointing out a problem is not enough — a comment that flags an issue with no path to resolving it is incomplete. So each comment closes with a separate paragraph led by Fix., giving a concrete change or snippet a human reviewer or the Ralph loop can apply directly.

Two later orchestrator passes may revise a comment after it is written: verification can flag it (unverified) or retract it, and consolidation can repoint its Fix. at a shared remedy. Both are described in execution_model.md.

Two consumers

The review file is deliberately a plain Markdown artifact so it serves both audiences without adaptation:

1. PR comments. Each comment is file+line anchored, so a thin adapter posts the comments as inline review comments on a pull request: scripts/post_reviews_to_github.sh parses a review file and posts it (dropping any comment whose line is not on the PR diff, which GitHub would silently discard). The workflow .github/workflows/review_pr_with_codex.yml, triggered by an /aster-code-review PR comment, drives exactly this: run the skill via the CLI, then post with the adapter.
2. The Ralph loop. The agent reads the review file and fixes the flagged issues, then re-runs — the review is the loop's feedback signal. The per-comment Fix. paragraph is what makes this directly actionable. The loop commits each round and runs diff <base>, so the review covers its commits (per-commit intent and hygiene included); a quick edit it has not committed can still be reviewed with files.

CI commands

On GitHub, a member triggers the skill on a PR with a /aster-code-review comment; the CI mirrors the CLI/skill interface:

Comment	Runs	Posts
/aster-code-review (or … diff)	review the PR diff	inline review comments
/aster-code-review files <p1> … <pN>	review those files (tracked; no spaces in a path, v1)	inline review comments
/aster-code-review smoke [--problems="0002 0006"]	the smoke test — does the skill run?	a ✅/❌ status comment
/aster-code-review benchmark [--problems="0002 0006"]	the benchmark — recall (informational)	a recall X/Y status comment

smoke/benchmark run the PR's skill + guidelines against the PR's problems.yaml, so a PR that adds a guideline and a fixture problem is verified end-to-end. The comment is parsed by scripts/parse_pr_command.sh (strict allowlist), and the workflow re-validates every value before it reaches a command.

The CI contract

The PR-review workflow lives on the repository's default branch but runs the PR's copy of the skill — a PR ships its own guidelines, so the review evolves with the code. For that to stay robust as the skill changes, the workflow depends on a small, fixed surface and nothing else:

• the profile name codex_workflow;
• aster_code_review.sh <mode> <args…> <output> — the argument grammar above, with ACR_AGENT_PROFILE in the environment;
• post_reviews_to_github.sh --repo <r> --pr <n> --head-sha <sha> [--finalize] <review-file>;
• scripts/parse_pr_command.sh (a /aster-code-review … comment → a validated plan) and the make smoke / make benchmark targets.

Everything else — the prompt wording, the --per-persona-context policy, the persona set, the review-file format, and the coding guidelines — is internal to the skill and free to change without touching the workflow.