Skip to content

feat: review locally with your own agent as the engine - #51

Merged
akf66 merged 4 commits into
mainfrom
akf66/local-review-harness
Sep 3, 2026
Merged

feat: review locally with your own agent as the engine#51
akf66 merged 4 commits into
mainfrom
akf66/local-review-harness

Conversation

@akf66

@akf66 akf66 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Orca-Code-Review — push 2

Severity Count Δ vs previous push
P0 0 0
P1 1 +1
P2 0 0
P3 0 0

❌ 1 finding blocks merge

What this adds

Local review. npx @orcarouter/code-review review plan prints a complete review request; the user's own coding agent reviews; review submit verifies positions, dedupes, applies the P0-P3 gate, and reports as text, markdown, or JSON. No OrcaRouter account, no API key — the thinking is the agent's. --pr <n> reviews a pull request by number without checking it out.

Same file selection as CI, no binary. Open Code Review's exclusion rules and per-language checklists are vendored as data under vendor/open-code-review/ (Apache-2.0, unmodified, with LICENSE and provenance) and read by a JavaScript port of its matcher in bin/selection.mjs. ocr is no longer spawned anywhere.

Per-repo settings. .orcacode-review.json holds block_on, language, exclude, rules. review config shows what applies and where each value came from; review config init writes the template; the plan offers to create it once, after the first review. An invalid file exits 2 with the offending key named.

Language follows the user. --lang (or the file, or the locale) drives the plan's instructions, the findings, and the report's own strings, in en/zh/ja/ko.

Exit codes. A review that ran exits 0; the verdict is in the report. --fail-on-block gives hooks and CI a 1. An unusable result exits 2 and is never suppressed.

Skills renamed to orca-review and orca-review-action. Installing the new names removes our old run-orca-code-review / setup-orca-code-review beside them (only ours — a same-named directory with a different name: is left alone). The installer now asks how you will use it — local, Action, or both — and --mode answers that for scripts.

Rubric. rules/output-shape.md now shows a wrong/right title pair; a real PR had produced a sixteen-word imperative title with a semicolon.

Demos

Three recordings under docs/, each answered with its recommended option, embedded in the README: install (both modes), Action setup via orca-review-action, and a local review of a pull request via orca-review that finds three planted P1s. docs/tapes/ has the vhs tapes and how to re-render.

Not changed

action.yml, scripts/postfilter.mjs, scripts/severity.mjs, scripts/judge.mjs — the CI path is untouched; the local harness shares the rubric and the position check with it, byte for byte.

Tests

536 passing (npm test), including end-to-end runs of the CLI against scratch repositories.

`review plan` prints everything a reviewer needs — files in scope with the
reasons for exclusions, per-language checklists, the P0-P3 rubric, the output
shape, the project's conventions — and `review submit` verifies positions,
dedupes, applies the gate and reports, in text, markdown or JSON. Nothing in
this path talks to OrcaRouter or needs a key; the thinking is the agent's.

- `--pr <n>` reviews a pull request by number without checking it out
- `--lang` follows the user's language through the plan, findings and report
- file selection is Open Code Review's own rules, vendored as data under
  vendor/open-code-review (Apache-2.0) with a JS port of its matcher — no
  `ocr` binary to install
- `.orcacode-review.json` holds a repo's block_on / language / exclude / rules;
  `review config` shows what applies and `review config init` writes it; the
  plan offers to create it once, after the first review
- exit 0 for a review that ran; `--fail-on-block` for hooks that want a 1;
  2 for an unusable result is never suppressed

Skills are renamed to orca-review and orca-review-action; installing the new
names retires our old ones beside them. The installer now asks how you will
use it — local, Action, or both — and `--mode` answers that for scripts.
rules/output-shape.md shows a wrong/right title pair, because a real PR got a
sixteen-word imperative title with a semicolon.
…E that shows each

Recorded with vhs against a throwaway repo (docs/tapes/README.md says how).
Every prompt takes its recommended answer. The old combined demo is retired;
the two features get their own GIF, with the 3x-speed mp4 linked beside each.

@orcacode-review orcacode-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐳 OrcaCode Review

No findings — nothing to flag in this PR. Great work!

OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 360 calls · 26.1M tokens · 98% cached

❤️ Share · Install OrcaCode Review

Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.

Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter

Drop the '3x · mp4' captions under the GIFs, and the menus of npx subcommands
and flags. The product is the two skills; you tell your agent what to review
and it drives the CLI. The one command that stays is the install. Flags and
the review contract are documented where a scripter looks, in --help and
skills/orca-review/references/contract.md.
package.json, plugin.json and marketplace.json move together, as the publish
workflow insists. The tarball check now also names the local-review files —
harness, selection, config, the vendored rules, both skills — so a files:
regression in package.json fails the release instead of shipping a plan with
no rubric.
@akf66
akf66 merged commit 5dae0d6 into main Sep 3, 2026
2 checks passed

@orcacode-review orcacode-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Outdated review — the PR head changed from 0ed4dea to 44c4fef during the review; these findings describe a diff the PR no longer has.
These findings describe 0ed4dea66723. Re-run the review to check the PR as it stands now.

🐳 OrcaCode Review

No findings — nothing to flag in this PR. Great work!

OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 321 calls · 22.9M tokens · 98% cached

❤️ Share · Install OrcaCode Review

Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.

Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter

@orcacode-review orcacode-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐳 OrcaCode Review

Found 1 issue in this PR: 🟠 1 P1.

OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 339 calls · 26M tokens · 98% cached

❤️ Share · Install OrcaCode Review

Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.

Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter

Comment thread bin/localconfig.mjs
const abs = path.resolve(root, r.rule_file);
if (!abs.startsWith(`${path.resolve(root)}${path.sep}`)) return fail(`${at} "rule_file" must stay inside the repository`);
try {
text = fs.readFileSync(abs, "utf8");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 P1 Resolve rule_file through realpath before trusting it stays inside the repo

The confinement check for "rule_file" is purely lexical: abs = path.resolve(root, r.rule_file) then a startsWith(root + sep) prefix test, then fs.readFileSync(abs). path.resolve does not resolve symlinks, so a rule_file that names a symlink inside the repo (git tracks symlinks, and a clone reproduces them, absolute targets and all) passes the check while readFileSync follows the link and reads an arbitrary file outside the checkout. Proven with a repo containing .orcacode-review.json {"rules":[{"path":"/*.js","rule_file":"hook.js"}]} where hook.js -> /outside/file: loadLocalConfig returns ok:true with the outside file's content as the rule text. That text is then embedded by buildPlan/ruleGroups into the plan prompt (review plan prints it to stdout; the agent forwards it to the model provider). The comment right above the check states the intent — "a config that points outside the checkout is a config that reads someone else's files into a prompt" — so a malicious or merely unusual repo (branch being locally reviewed, or a fork's tree containing a symlink to a well-known local path such as a credentials file) causes arbitrary local file contents to be read and shipped in the prompt. Fix: compare fs.realpathSync of the resolved file (and, to be safe, realpath of root) instead of the lexical prefix, e.g. const real = fs.realpathSync(abs); if (!real.startsWith(realRoot + path.sep)) fail(...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant