Skip to content

v0.11.0

Choose a tag to compare

@guimatheus92 guimatheus92 released this 05 Sep 17:49
· 8 commits to main since this release
9fcdea0

Added

  • A truncated file list is never reviewed. PrMetadata.changedFileCount / changedFileListTruncated carry the provider's own count (GitHub changed_files; GitLab changes_count, where "N+" means the stored diff overflowed and /diffs serves exactly the capped set). runGather refuses a provider list of any other length, or one declared truncated, and completes it from the reviewer's checkout when that checkout is the PR's repository with base and head already present: git diff-tree -r -M -z from the single merge base (plumbing, so reviewer diff config and the PR's own .gitattributes cannot reshape the list; -z, so non-ASCII paths arrive raw), one hunks-only patch per missing file, provider entries winning. It never fetches: when a commit is absent, the history has two merge bases, or the clone is shallow, the run fails BEFORE anything is cached — exit 2, error.txt in detached mode — naming the counts and the exact git fetch to run (git fetch origin <base> refs/pull/N/head, refs/merge-requests/N/head, or the ADO branches). A completion that still falls short of an exact count is refused as well, and a list LONGER than the count is reported as a disagreement rather than a truncation. The fetch command in the message quotes every ref. An incomplete file list is unknown, never empty: the rule-trust, config-trust and MCP gates it feeds would otherwise pass a rule the PR changed. Fixes #23.
  • The plugin slash command (commands/pr-review.md) locates the checkout whose git origin matches the PR URL before starting the CLI — the current directory, then its subdirectories, then its siblings, preferring a primary worktree over a linked one — and prints a project-skill count computed with the same rule as the loader.
  • --force-skill <file|dir> is documented as the only bypass: the file, or every .md under the directory, is injected whole into every pass with no scope, relevance or rule-trust check (the directory form has always been accepted). It is per run and CLI-only by design — there is deliberately no yaml or env key for forcing, so a committed .pr-review.yaml can never pre-authorize branch-authored content. Docs that told you to point extra_skills_dirs / --skills-dir / PR_REVIEW_SKILLS_DIR at a directory to force it now say --force-skill <dir>; those keys are trust-checked, and the warning not to aim a forced directory at rules the PR can edit applies to --force-skill alone.
  • Brand icon (assets/icon.svg) and a branded README: centered header, badges, highlights, a mermaid pipeline diagram, a table of contents, and the verbose reference material folded into collapsible sections. Docs only — nothing ships differently.

Changed

  • GitHub no longer fetches the whole-PR text diff: pulls.get with the diff media type returns 406 above 300 files (undocumented), and nothing reads fullDiff — the per-file patches are the diff. GitHub PRs of 300–500 files are now reviewable; fullDiff stays in pr-review-gather.json, empty for GitHub.
  • Gather cache entries carry changedFilesComplete, set only once the file list passed the completeness gate. An entry without it was written before this release and is refetched once, then rewritten in place; no cache clear needed.
  • BREAKING: --skills-dir / extra_skills_dirs / PR_REVIEW_SKILLS_DIR are now selected like repo skill dirs, not injected whole. In 0.10 every file under a configured dir was force-fed into every pass regardless of scope, relevance or rule trust — observed live: 1.4 MB × 8 passes on a 5-file PR. Now a targeted file (applies_to / applyTo / paths) becomes a scoped rule, an untargeted one goes through the name+description relevance heuristic, an unmatched one lands in the on-demand index, and a file the PR changed inside a configured dir is skipped (also when the dir comes from an unchanged .pr-review.yaml). Configured dirs still apply when the cwd is not the PR's repository, their origin is configured, and passes.json / matchedBy are unchanged (glob / repo / index). The stderr line names the bypass: --force-skill <dir> injects a directory whole.
  • Linked skill directories are followed, and trust is by authorship rather than location. Discovery follows a directory link (symlink or NTFS junction) one hop, in every discovery dir (.claude/skills, .claude/rules, .copilot/skills, .github/skills, .github/instructions, .agents/skills) and in configured dirs — 0.9/0.10 rejected every link and failed closed on anything resolving outside the checkout. A link the PR added or changed (its path, or any parent directory of it, is in the diff) is refused before anything behind it is read and named as degraded coverage; a link met inside a linked directory is not followed. Content whose real path is outside the checkout is trusted only when the PR did not author the link reaching it AND the file is committed and clean in its home git repository (git ls-files + git status; a SKILL.md needs its whole directory clean) — the same gate applies to every rule outside the checkout (linked, configured or personal), and a repository git cannot read is skipped, never trusted: on Windows git checkout of a PR branch writes through a junction into the shared directory, so a planted file would otherwise become a trusted rule for every sibling repo's review. Untracked or modified files there are skipped and named; a directory under no git repository at all is trusted as the reviewer's local configuration, with one stderr note per directory reached through a link. Nothing depends on one company's layout — any link, any discovery root, any OS, git or not. The aggregate stderr line now reads [skills] skipped N project rule(s) — changed by this PR, reached through a link it changed, or not committed in their home repository. Fixes #20.
  • Trust comparisons fold letter case and Unicode (NFC) on every platform. They folded case on win32 only, so a PR committing .Agents/skills could bypass the rule-trust check on a macOS reviewer's machine.
  • The posting guarantee now states exactly what each provider does: GitLab findings post as inline discussions; re-anchoring of unanchorable findings applies on GitHub and GitLab, while Azure DevOps threads post at the reported file:line as-is and a location-less finding lands as a resolvable PR-level thread. README and AGENTS.md said "re-anchored" universally; the code (reanchor in src/commands/post.ts) never did that on ADO.
  • Reference docs agree on the pass ceiling (6 stack + up to 2 installed-plugin + every baseline under the 16-pass materialization ceiling; 10 only when no pack passes exist), the ## Skills totals line, and GitLab in SECURITY.md's scope.
  • Reference docs now correctly describe post-gather setup, parallel Codex review, conditional verification, post-selection shared context (skills-project.md vs the budgeted skills-all.md fallback), and direct-agent versus slash-command companions. Materialized pass files now label skill source paths as provenance instead of claiming inaccessible sibling references resolve inside the confined runtime.

Fixed

  • A dispatched claude session no longer boots the MCP servers it is forbidden to call. 0.10 denied the mcp__* tools but left --setting-sources user loading every user-level MCP server, so each review started them anyway — on Windows a cmd.exe + conhost.exe + npx + node per server, each console window leaking permanently because the terminal never reclaims it. Measured on one live run: 20 processes and 4 consoles, of which 16 processes and 3 consoles existed only to start three servers the session could not reach. --strict-mcp-config is now passed alongside the tool denial, and the per-runtime switch lives in a typed MCP_PROCESS_DENIAL: Record<Runtime, string> so a runtime added later fails to compile until it declares one. The two are not symmetric and the docs now say so: claude's is categorical, copilot's --disable-builtin-mcps is completed per name by --disable-mcp-server and so is bounded by discoverMcpCapabilities. Because no --mcp-config is passed, the run-dir .mcp.json is now provenance only — no runtime loads it, and the artifact docs say so. This also corrects the 0.10.0 note below, which claimed "built-in/ambient MCP servers are denied" when that held at the tool level only under claude. No capability is lost: the tools were never in a pass's callable surface — run artifacts recorded passes reporting exactly that.
  • The shared PR context no longer advertises ## Available MCP Capabilities, and the installed-plugin capability brief no longer tells a pass to call tools it cannot have. Both runtimes deny MCP, so the advertisement only bought a paragraph of each plugin pass explaining why the call it was told to make was impossible. capabilities.json and the capability-<pass>.json sidecars are unchanged in shape; the brief now states that available, attempted and used are all empty under a runtime that denies MCP, which the previous wording left undefined for available.
  • Azure DevOps PRs with more than 100 changed files were reviewed on their first 100. getPullRequestIterationChanges defaults to $top=100 and the response cursor was never read; every release from 0.6 to 0.10 issued that one unpaged call and cached the result. Iteration changes are now paged at 2000 per call until a short page, a full page without a cursor is probed once more, and a cursor that does not advance throws instead of looping.
  • Azure DevOps folder entries (a directory add, or an ancestor of an edited file) no longer become "changed files" with an empty synthesized patch: they counted against the 500-file guard, cost a content fetch each, and would have multiplied under full pagination.
  • Foreground reviews now pass the provider resolved from pre-gather trusted configuration into gather, and detached preflight uses the same trusted host map. A changed repository .pr-review.yaml can no longer remap a self-hosted host before that branch-authored configuration is rejected as untrusted. hosts: is now honoured from the global config only, on every command: the detectProvider fallback no longer reads a checkout-local .pr-review.yaml, so a repo-level hosts: map that used to resolve a self-hosted URL must move to ~/.pr-review/config.yaml. loadConfig strips a checkout-local hosts: at load (so config show never advertises a mapping nothing honours) and says so once on stderr.
  • Phantom rules from folders without a SKILL.md. Under a skills root (any discovery or configured dir whose basename is skills) a subdirectory is a skill only through <dir>/SKILL.md: a subdirectory without one is skipped, with a stderr warning naming it when it holds .md files; flat .md files at the root are still skills; a README.md met while walking is never a skill (an explicit --skill README.md still loads); rules/ and instructions/ roots recurse as before. Observed live: 8 phantom rules — 465 KB, 31% of the injected payload — came from two SKILL.md-less folders and a README.
  • Dead code: walkSkillDirs / loadFromDir in src/plugins/builtin.ts — a second directory walker with a link policy of its own — are gone.

Removed

  • Stray patch5.py at the repo root — a one-off patch script whose edits had already landed in src/commands/packs.ts.