feat(review): integrate OCR planning, CLI review gates and audit reports

CHANGELOG.md

@@ -4,13 +4,30 @@ All notable changes to junto are documented in this file. The project follows Se
 
 ## [Unreleased]
 
+- Keep selected CLI review filenames literal in Git diff, including paths with brackets, so excluded files cannot leak into review input.
+
 ### Added
 
+- Host CLI review gates (`provider: "cli"`) using installed OCR delegation rules and bounded stdin context; opt-in live semantic evaluation.
+- Shared version-1 finding schema/fixtures including source, nullable line and metadata.
+- Review event log and `junto__report` with escaped static HTML export under `.junto/`.
+
+- Review gates: a gate with `"type": "review"` runs OpenCodeReview (`ocr`, `npm install -g @alibaba-group/open-code-review`) through the same verdict evidence as command gates. A missing reviewer is `skipped`, a broken or timed-out reviewer is `fail`, and findings at `failOn` severities (default critical and high) are `fail`. The requirement context is passed as a bounded `review-background.md`; raw reviewer output is redacted and stored beside the normalized result.
+- `junto__plan` MCP tool: deterministic plan from git changes, config `rules`, and the skill registry, written to `plan.resolved.json`.
+- Config `rules` (glob to skills, gates, approval) and `skills.roots`; skills declare `appliesTo` and `tags` in frontmatter or in the collection's `skillset.json`.
+- `ocr delegate preview` support for deterministic review scope without an LLM.
 - Public GitHub Pages landing page with installation, lifecycle, status, and design-principle guidance.
 - Automatic Pages deployment from the self-contained `site/` directory.
 
 ### Changed
 
+- Review rules retain rename source paths and committed changes reversed by pending edits. Skill metadata follows the selected search root instead of merging shadowed definitions.
+- Review evidence redacts error messages and escaped JSON values without corrupting JSON, removes stale raw output after an unavailable reviewer, and rejects truncated delegation previews.
+- Added an opt-in real OCR preview smoke test (`OCR_SMOKE_BIN` points to an installed binary); no LLM or runtime download is required.
+- Review verification and delegation preview cover both committed task changes and pending workspace edits; evidence retains each scope and a skipped scope never counts as a completed review.
+- Rules now enforce gates and human approval at verification and phase transitions, including small or auto-approved tasks, omitted task gates, deleted files, and missing gate definitions.
+- Changed-file resolution reports git failures instead of returning an empty list, handles paths with spaces and renames, and includes untracked files.
+- Review findings share one severity/category vocabulary with governed-agent-sdlc; reviewer infrastructure failures are provider errors, never findings.
 - Improve README onboarding with install verification, first-run behavior, explicit deep-task review
   transition guidance, troubleshooting, current capabilities, and project links.
 

CLAUDE.md

@@ -33,7 +33,7 @@ corepack pnpm --filter @junto/core add <package>
   Never touch `~/.claude`, user settings, or the project's root `.gitignore`.
 - **R2 - No runtime downloads.** No downloaded binaries, `chmod`, CDN bootstrap, or startup-time `npx`.
 - **R3 - Never persist secrets.** Configuration stores environment variable names, never API key values.
-- **R5 - Deterministic gates, advisory panels.** Only a real command exit code may block a phase transition.
+- **R5 - Deterministic gates, advisory panels.** Command exit codes and explicit severity policy over validated review findings may block a phase transition. Advisory consult/panel opinions cannot satisfy or block gates. Missing, incomplete or broken required reviews never pass.
 - Always spawn commands with argv arrays; never concatenate shell command strings.
 - `SCHEMA_VERSION` remains 1 until an intentional migration is designed. Reject future schema versions clearly.
 - `canEnter()` must stay pure. Pass all filesystem facts in through `TransitionContext`.

README.md

@@ -72,8 +72,60 @@ When source files change, previous verdicts become stale and gates must run agai
 The `guard.js` hook blocks evidence writes through Edit/Write/MultiEdit, but it **cannot block Bash**.
 It prevents accidents and shortcuts; it is not a security boundary against a malicious actor.
 
+## OpenCodeReview gates and rules
+
+See [`examples/config.review.json`](examples/config.review.json) for a review gate and skill registry.
+Set the gate's `required` field to `true` when a completed review must block task completion.
+
+For a real CLI integration smoke test, set `OCR_SMOKE_BIN` to an installed OCR executable and run
+`corepack pnpm test`. It checks delegation preview in a temporary Git repository without an LLM;
+the test is skipped when the variable is unset. Full semantic reviews still require OCR LLM configuration.
+
+OpenCodeReview must be installed and configured separately; Junto uses `OPEN_CODE_REVIEW_BIN` or
+`ocr` on `PATH` and does not download a reviewer at runtime.
+
+The plan preview and verification cover the same task scope. Commits after the task's base are
+reviewed with `--from <base> --to <head-sha>`, and pending workspace edits get a separate review.
+Empty scopes are omitted unless the whole task has no changes. Each invocation uses the gate timeout.
+The normalized evidence records both scopes and their commands. Every selected scope must complete:
+a skipped review cannot satisfy a required gate, and one successful scope cannot hide another's failure.
+Since range mode reviews committed content, commit pending fixes before re-running when they resolve
+findings in the committed range. Start a new task after a rebase that removes the original task base.
+Create tasks after the repository's initial commit if you intend to make commits during the task;
+Junto refuses to guess a missing base after history has been created.
+
+Rules are re-evaluated when planning, verifying, showing status, and advancing phases. Matching gates
+are added even if the task started with a narrower gate list, and missing gate definitions block progress.
+Deleted files also trigger their rules. A rule with `approvalRequired: true` overrides `autoApprove`;
+write `plan.md` and have the user run `/junto:approve`. This also works for small tasks and rules first
+matched during implementation. Status remains read-only.
+
 ## Advisory consult and panel
 
+For review gates, `"provider": "cli"` selects a logged-in host CLI. Set `JUNTO_REVIEW_COMMAND`
+in the launching environment to a JSON argv array, for example
+`["codex","exec","--sandbox","read-only","--ephemeral","--color","never","-"]`.
+Use `OPEN_CODE_REVIEW_BIN` for a locally installed OCR binary when it is not on PATH. OCR selects
+files and version-1 delegation rules; the host receives the diff, new files, rules and background
+on stdin and returns OCR-shaped JSON (`status`, `comments`). Claude and other CLIs can use the same
+contract; for Claude use `--safe-mode --print --tools= --no-session-persistence --output-format text`.
+On Windows, use `node` plus the installed CLI's absolute JavaScript entry point if only an npm shim
+is available. Repository configuration cannot select the host executable; nothing is downloaded.
+Choose read-only/tool-disabled commands: arbitrary host CLIs are not sandboxed by Junto itself.
+Input is limited to 512 KiB, output to 8 MiB per stream and calls to the gate timeout. CLI billing
+cannot be measured by the harness. Missing, incomplete or out-of-scope output never passes.
+
+`junto__report` reads the active task's review findings, verdicts and append-only review events.
+Pass `{"html":true}` to export an escaped static report to
+`.junto/tasks/<id>/review-report.html`. Reports show stored evidence; use `junto__status` for current
+transition eligibility. Review findings use the version-1 contract in `packages/core/contracts/`,
+mirrored from governed-agent-sdlc with shared behavioral fixtures.
+
+To evaluate a real host CLI, set `REVIEW_LIVE=1`, `JUNTO_REVIEW_COMMAND` and `OPEN_CODE_REVIEW_BIN`,
+then run `corepack pnpm vitest run packages/core/test/cli-review.test.ts`. Two calls (120 seconds
+each) check a known division-by-zero defect and zero high/critical false positives on a clean
+control. Regular CI uses deterministic fixtures and does not call an LLM.
+
 `junto__consult` asks one advisory role (`architect`, `adversary`, `pragmatist`, `reviewer`, or a
 project-defined role) about the active task's brief and plan; `junto__panel` (via `/junto:panel`)
 asks several roles in sequence. Both are strictly advisory: their output never blocks a phase

docs/superpowers/specs/2026-08-30-junto-design.md

@@ -32,8 +32,11 @@ binary, runs `chmod`, contacts a CDN, or invokes `npx`.
 
 **R4 - No bypass mechanism.** No skill or hook edits transcripts, configuration, or model refusals.
 
-**R5 - Deterministic gates, advisory models.** Only the exit code of an executable command can block a
-phase transition. Model opinions never become gates.
+**R5 - Deterministic gates, advisory panels.** Command gates use executable exit codes. Review gates
+apply the configured `failOn` severity policy to validated findings from an explicitly selected review
+provider. Missing, incomplete or failed reviews never satisfy a required gate. Consult and panel
+opinions remain advisory and cannot satisfy or block gates. The policy mapping is deterministic;
+semantic findings themselves depend on the reviewer and are not a correctness proof.
 
 ## 3. Architecture
 

examples/config.review.json

@@ -0,0 +1,29 @@
+{
+  "schemaVersion": 1,
+  "gates": {
+    "tests": {
+      "argv": ["npm", "test"],
+      "required": true,
+      "timeoutMs": 120000
+    },
+    "typecheck": {
+      "argv": ["npm", "run", "typecheck"],
+      "required": true,
+      "timeoutMs": 120000
+    },
+    "code-review": {
+      "type": "review",
+      "provider": "open-code-review",
+      "failOn": ["critical", "high"],
+      "required": false,
+      "timeoutMs": 600000
+    }
+  },
+  "rules": [
+    { "id": "auth", "match": ["**/auth/**"], "skills": ["review-code"], "approvalRequired": true },
+    { "id": "typescript", "match": ["**/*.ts", "**/*.tsx"] }
+  ],
+  "skills": {
+    "roots": ["../ai-engineering-skills"]
+  }
+}

packages/core/contracts/review-contract.fixture.json

@@ -0,0 +1,14 @@
+{
+  "version": 1,
+  "input": {
+    "status": "complete",
+    "comments": [
+      {"path": "src\\a.py", "content": "Broken condition", "severity": "error", "category": "bug", "start_line": 4, "end_line": 5},
+      {"path": "src/b.ts", "content": "Check this", "severity": "unknown", "category": "test", "start_line": 0}
+    ]
+  },
+  "findings": [
+    {"id": "ocr-1", "source": "open-code-review", "severity": "high", "category": "correctness", "file": "src/a.py", "line": 4, "message": "Broken condition", "metadata": {"end_line": 5}},
+    {"id": "ocr-2", "source": "open-code-review", "severity": "medium", "category": "testing", "file": "src/b.ts", "line": null, "message": "Check this", "metadata": {}}
+  ]
+}

packages/core/contracts/review-finding.schema.json

@@ -0,0 +1,30 @@
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "$id": "https://github.com/vannt-dev/governed-agent-sdlc/contracts/v1/review-finding.schema.json",
+  "x-contract-version": 1,
+  "title": "Normalized review finding",
+  "description": "Shared contract between governed-agent-sdlc (agentkit.review) and junto (packages/core/src/review.ts). Provider-specific schemas stop at each adapter; policies and gates only see this shape.",
+  "type": "object",
+  "required": ["id", "source", "severity", "category", "file", "line", "message", "metadata"],
+  "additionalProperties": false,
+  "properties": {
+    "id": { "type": "string", "minLength": 1 },
+    "source": { "type": "string", "minLength": 1 },
+    "severity": { "enum": ["critical", "high", "medium", "low", "info"] },
+    "category": {
+      "enum": [
+        "security",
+        "correctness",
+        "performance",
+        "maintainability",
+        "testing",
+        "architecture",
+        "other"
+      ]
+    },
+    "file": { "type": "string", "minLength": 1 },
+    "line": { "type": ["integer", "null"], "minimum": 1 },
+    "message": { "type": "string" },
+    "metadata": { "type": "object" }
+  }
+}

packages/core/src/changes.ts

@@ -0,0 +1,160 @@
+import { execa } from "execa"
+import { shouldStale } from "./stale.js"
+
+export interface ChangedFile {
+  path: string
+  status: "added" | "modified" | "deleted" | "renamed"
+}
+
+export interface ResolveChangesOptions {
+  base?: string
+  head?: string
+  ignore?: string[]
+}
+
+/** Git could not tell us what changed. Callers must not treat this as "nothing changed". */
+export class ChangedFilesError extends Error {
+  constructor(message: string) {
+    super(message)
+    this.name = "ChangedFilesError"
+  }
+}
+
+const DEFAULT_IGNORE = [
+  "**/node_modules/**",
+  "**/.git/**",
+  "**/.junto/**",
+  "**/dist/**",
+  "**/.temp/**",
+]
+
+type Status = ChangedFile["status"]
+
+/** Parse a git status or diff status code into a standard status string. */
+function parseStatus(code: string): Status {
+  const c = code.trim().toUpperCase()[0]
+  switch (c) {
+    case "A":
+    case "C":
+    case "?":
+      return "added"
+    case "D":
+      return "deleted"
+    case "R":
+      return "renamed"
+    default:
+      return "modified"
+  }
+}
+
+/** Normalize file path to forward slashes without leading ./ */
+function normalizePath(p: string): string {
+  return p.replace(/\\/g, "/").replace(/^\.\//, "")
+}
+
+export class ChangedFileResolver {
+  constructor(private readonly defaultIgnore: string[] = DEFAULT_IGNORE) {}
+
+  private collect(entries: Array<{ path: string; status: Status }>, ignorePatterns: string[]): ChangedFile[] {
+    const combinedIgnore = [...this.defaultIgnore, ...ignorePatterns]
+    const results: ChangedFile[] = []
+    const seen = new Set<string>()
+    for (const entry of entries) {
+      const path = normalizePath(entry.path)
+      if (path === "" || seen.has(path)) continue
+      if (!shouldStale(path, combinedIgnore)) continue
+      seen.add(path)
+      results.push({ path, status: entry.status })
+    }
+    return results
+  }
+
+  /**
+   * Parse text `git diff --name-status` output (tab separated). Paths that contain spaces survive
+   * because tabs, not whitespace, delimit fields. Prefer `parseNameStatusZ` for real git output.
+   */
+  parseNameStatusOutput(output: string, ignorePatterns: string[] = []): ChangedFile[] {
+    const entries: Array<{ path: string; status: Status }> = []
+    for (const line of output.split(/\r?\n/)) {
+      if (line.trim().length === 0) continue
+      const parts = line.includes("\t") ? line.split("\t") : line.trim().split(/\s+/)
+      if (parts.length < 2) continue
+      const [statusCode, ...paths] = parts
+      // For renames (R100 old new) the new path is the last field.
+      const path = paths[paths.length - 1]
+      if (statusCode === undefined || path === undefined) continue
+      // Moving a file out of a protected directory still changes that directory.
+      if (statusCode.startsWith("R") && paths.length > 1 && paths[0]) {
+        entries.push({ path: paths[0], status: "deleted" })
+      }
+      entries.push({ path, status: parseStatus(statusCode) })
+    }
+    return this.collect(entries, ignorePatterns)
+  }
+
+  /** Parse `git diff --name-status -z`: NUL separated, with two paths for renames and copies. */
+  parseNameStatusZ(output: string, ignorePatterns: string[] = []): ChangedFile[] {
+    const tokens = output.split("\0")
+    const entries: Array<{ path: string; status: Status }> = []
+    for (let i = 0; i < tokens.length;) {
+      const code = tokens[i]
+      if (code === undefined || code === "") { i += 1; continue }
+      const pathCount = /^[RC]/.test(code) ? 2 : 1
+      const path = tokens[i + pathCount]
+      const oldPath = tokens[i + 1]
+      if (code.startsWith("R") && oldPath) entries.push({ path: oldPath, status: "deleted" })
+      if (path !== undefined && path !== "") entries.push({ path, status: parseStatus(code) })
+      i += 1 + pathCount
+    }
+    return this.collect(entries, ignorePatterns)
+  }
+
+  /** Untracked files from `git ls-files --others -z`; they are new to the working tree. */
+  parsePathList(output: string, status: Status, ignorePatterns: string[] = []): ChangedFile[] {
+    const entries = output.split("\0").filter(p => p !== "").map(path => ({ path, status }))
+    return this.collect(entries, ignorePatterns)
+  }
+
+  private async git(root: string, args: string[]): Promise<string> {
+    const res = await execa("git", args, { cwd: root, reject: false })
+    if (res.exitCode !== 0) {
+      const detail = typeof res.stderr === "string" && res.stderr.trim() !== "" ? res.stderr.trim() : `exit ${res.exitCode}`
+      throw new ChangedFilesError(`git ${args.join(" ")} failed: ${detail}`)
+    }
+    return typeof res.stdout === "string" ? res.stdout : ""
+  }
+
+  /**
+   * Resolve changed files from git in the specified repository root. Throws
+   * `ChangedFilesError` when git fails: an empty list would look like "nothing to review".
+   */
+  async resolve(root: string, options: ResolveChangesOptions = {}): Promise<ChangedFile[]> {
+    const ignore = options.ignore ?? []
+
+    if (options.base && options.head) {
+      const out = await this.git(root, ["diff", "--name-status", "-z", "--find-renames", options.base, options.head])
+      return this.parseNameStatusZ(out, ignore)
+    }
+
+    // Working tree against `base` (default HEAD) covers committed, staged and unstaged edits.
+    let against = options.base
+    if (against === undefined) {
+      const head = await execa("git", ["rev-parse", "--verify", "HEAD"], { cwd: root, reject: false })
+      against = head.exitCode === 0 ? "HEAD" : undefined
+    }
+
+    const tracked = against === undefined
+      ? this.parsePathList(await this.git(root, ["ls-files", "-z", "--cached"]), "added", ignore)
+      : this.parseNameStatusZ(await this.git(root, ["diff", "--name-status", "-z", "--find-renames", against]), ignore)
+    const untracked = this.parsePathList(
+      await this.git(root, ["ls-files", "-z", "--others", "--exclude-standard"]),
+      "added",
+      ignore,
+    )
+
+    const map = new Map<string, ChangedFile>()
+    for (const item of tracked) map.set(item.path, item)
+    for (const item of untracked) if (!map.has(item.path)) map.set(item.path, item)
+    return Array.from(map.values())
+  }
+}