agentleFS
SentryKnown publishergetsentry/sentry-dart/.agents/skills/review/SKILL.md

review

Skill878 starsChanged 33 days agoMITView on GitHub

Three-axis review of the branch diff — Standards (this repo's documented standards + public API surface), Spec (the originating Linear issue / PR), and Correctness (runtime bugs + the SDK threat model — PII, native leaks, buffers, serialization, concurrency). Runs the axes as parallel sub-agents and reports them side by side. Use when reviewing a branch, a PR, or work-in-progress changes before opening or merging, or "review since X".

192 other projects use a version of this, 4 of them edited.

Adopted by

Most recently taken up by

Used with

Projects with it also use close-spec, design-with-images and 10 more.

Other files people use alongside this one.

Files found in the same public projects as a version of this one, more often than they turn up anywhere else. Each project counts once, large collections count for less, and this file's own repository is left out.
  • dzhngSkill

    close-spec

    When a spec under specs/<feature>/ is done shipping (a write-spec build, or any planned task that produced a spec), archive it to specs/done/ and rewrite it from a build-plan into a durable rationale — the why, the principles, the invariants — pointing back to the real code for the how. Use when implementation has landed and the plan no longer matches what shipped, or the user says a feature/spec is finished. Pairs with [write-spec](../write-spec/SKILL.md) (the plan this closes) and [review](../review/SKILL.md) (run before closing).

    Also in 5 projects that use this

  • dzhngSkill

    design-with-images

    Explore UI and visual design with image generation, then iterate the real implementation against the user-selected concept. Use when brainstorming visual directions, offering mockup variants, or implementing an approved generated design; do not stop at the mockups or the first code approximation.

    Also in 5 projects that use this

  • LambenthanSkill

    daily-arxiv

    每日从 arXiv 拉取新论文,过滤相关性,auto-ingest 高优先级论文,检测 SOTA 更新

    Also in 5 projects that use this

  • LambenthanSkill

    exp-eval

    实验判决门:Review LLM 独立评判实验结果 → 4 种判决路径 → 自动更新 claims confidence、ideas status、graph edges

    Also in 5 projects that use this

  • +3 more from dzhng
  • +5 more from Lambenthan

The file

We split this skill into the rules it gives an agent

4 rules. 0 are common, 3 are distinctive, and 1 is very specific to this project.

  • 0

    Common

    Most of the 157 edited versions we read keep these.

  • 3

    Distinctive

    This author's own choices. Worth a look.

  • 1

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  1. Distinctive:

    Run the axes as parallel sub-agents so none pollutes another's context, then aggregate.

    line 12
  2. Very specific:

    Use whatever base the user named — a commit, branch, tag, or HEAD~N.

    line 18
  3. Distinctive:

    Look for the originating spec, in order:

    line 27
  4. Distinctive:

    If none is found, ask. If there genuinely is no spec, the Spec sub-agent skips and reports "no spec available".

    line 33

“Common” is measured: kept by at least 6 in 10 of the 157 edited versions of this file we could read. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

Review the diff between HEAD and a base, on three axes:

  • Standards — does the diff conform to this repo's documented standards?
  • Spec — does it faithfully implement the originating issue / PR?
  • Correctness — will it actually work, and is it safe?

Run the axes as parallel sub-agents so none pollutes another's context, then aggregate. Report them side by side, and never rerank across axes — keeping them separate is the whole point (see Why separate axes).

This is the every-PR pass. It complements security-review (deeper security audit) and span-convention-review (tracing-span conventions) — invoke those for depth. It is not a debugger: when a bug is already known and reproducing, use diagnosing-bugs instead.

1. Pin the base

Use whatever base the user named — a commit, branch, tag, or HEAD~N. If they named none, default to the merge-base with the default branch (git merge-base HEAD origin/main). Capture once:

  • git diff <base>...HEAD (three-dot, so it compares against the merge-base)
  • git log <base>..HEAD --oneline

Confirm the ref resolves (git rev-parse <base>) and the diff is non-empty. A bad ref or empty diff fails here — not inside parallel sub-agents.

2. Identify the spec source

Look for the originating spec, in order:

  1. Linear issue references in commit messages or the PR (fetch the issue via the Linear tools).
  2. The PR description (motivation / what it does).
  3. A path the user passed.

If none is found, ask. If there genuinely is no spec, the Spec sub-agent skips and reports "no spec available".

3. Spawn the sub-agents in parallel

Send one message with three Agent tool calls (general-purpose subagent for each). Give each the diff command and the commit list, plus its brief:

Standards sub-agent:

Read .agents/skills/code-guidelines/SKILL.md and .agents/skills/test-guidelines/SKILL.md, then review the diff against them. Top priority — public API surface: for any change to a barrel file (lib/sentry.dart, lib/sentry_flutter.dart, or an integration package's barrel), classify each exported symbol as added / removed / signature-changed / behavior-changed. Flag every removal or incompatible change as a breaking change (semver-major) and check it carries a deprecation path (per code-guidelines). Then report, per file/hunk, every place the diff violates a documented standard — cite the rule. For areas the docs don't cover, apply the smell baseline in .agents/skills/review/references/smell-baseline.md as judgement calls. Distinguish hard violations from judgement calls; a documented standard overrides the baseline. Skip anything dart analyze / dart format / lints enforce. Under 400 words.

Spec sub-agent (skip and note if no spec):

Report: (a) requirements the spec asked for that are missing or partial; (b) behavior in the diff that wasn't asked for (scope creep); (c) requirements that look implemented but wrong. Quote the spec line for each finding. Under 400 words.

Correctness sub-agent:

Find bugs and unsafe code the diff introduces — not style, which Standards owns. Check, per file/hunk:

  • Runtime bugs — unawaited / fire-and-forget futures, uncancelled stream subscriptions, late used before init, null/empty edge cases, swallowed errors, missing cleanup in close() / dispose(), races / TOCTOU.
  • SDK threat model — PII collected without gating on options.sendDefaultPii; sensitive data leaking into breadcrumbs, logs, or event payloads; native memory not released (JNI local refs, malloc) or a native exception that could crash the host app; unbounded buffers / queues / caches (memory growth); serialization correctness (toJson/fromJson round-trip, null handling); isolate / thread safety; rate-limiter or retry correctness. For each finding give file:line, a one-line severity (critical/high/medium/low), why it's real (not already handled, no covering test), and a concrete fix. Skip anything the analyzer or existing tests already catch. Under 400 words.

4. Aggregate

Present the reports under ## Standards, ## Spec, and ## Correctness headings, verbatim or lightly cleaned. Do not merge or rerank findings across axes.

End with a one-line summary per axis: total findings, and the worst issue within that axis. Don't pick a single winner across axes — that's the reranking the separation exists to prevent.

Why separate axes

A change can pass on one axis and fail on another, and a merged list lets one mask the others:

  • Follows every standard but implements the wrong thing → Standards pass, Spec fail.
  • Does what the issue asked but breaks conventions → Spec pass, Standards fail.
  • Clean and on-spec but leaks a native ref or drops a sendDefaultPii check → Standards & Spec pass, Correctness fail.

Reporting them separately stops any one axis from hiding another.

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.