diff --git a/plugins/opencode/commands/adversarial-review.md b/plugins/opencode/commands/adversarial-review.md index bbd4586..9da2b9b 100644 --- a/plugins/opencode/commands/adversarial-review.md +++ b/plugins/opencode/commands/adversarial-review.md @@ -1,6 +1,6 @@ --- description: Run a steerable adversarial OpenCode review that challenges implementation and design decisions -argument-hint: '[--wait|--background] [--base ] [--model | --free] [--pr ] [--post] [--confidence-threshold ] [--path ] [--path ] [focus area or custom review instructions]' +argument-hint: '[--wait|--background] [--base ] [--model | --free] [--pr ] [--path ] [--path ] [focus area or custom review instructions]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), Bash(gh:*), AskUserQuestion --- @@ -40,8 +40,6 @@ Argument handling: - `--model ` overrides the saved setup default model and OpenCode's own default model for this single review (e.g. `--model openrouter/anthropic/claude-opus-4-6`). Pass it through verbatim if the user supplied it. - `--free` tells the companion script to shell out to `opencode models`, filter for first-party `opencode/*` free-tier models (those ending in `:free` or `-free`), and pick one at random for this review. Restricted to the `opencode/*` provider because OpenRouter free-tier models have inconsistent tool-use support, and the review agent needs `read`/`grep`/`glob`/`list`. Pass it through verbatim if the user supplied it. `--free` and `--model` are mutually exclusive — the companion will error if both are given. - `--pr ` reviews a GitHub pull request via `gh pr diff` instead of the local working tree. The cwd must be a git repo whose remote points at the PR's repository, and `gh` must be installed and authenticated. -- `--post` (opt-in, requires `--pr`) publishes the adversarial review back to the PR as a GitHub review comment. The summary (verdict + findings table + collapsible full findings) is posted as the review body, and any finding with confidence at or above the threshold whose line is part of the PR diff is posted as an inline review comment. Findings below the threshold or pointing at lines outside the diff stay in the summary only. The review is always published with `event: "COMMENT"` — never `REQUEST_CHANGES` — because this tool is advisory. Pass `--post` through verbatim if the user supplied it. -- `--confidence-threshold ` (optional, default `0.8`) sets the confidence bar for inline comments when `--post` is set. Accepts `0..1` floats or percentages (`80`, `80%`). Pass through verbatim. - `--path ` reviews a specific file or directory instead of git diff. Can be specified multiple times (`--path src --path lib`). When `--path` is set, the review is assembled from the actual file contents at those paths rather than from `git diff`. This is useful for reviewing specific directories, fixed sets of files, or large untracked/imported code drops. Mutually exclusive with `--pr` (paths take precedence over PR mode). PR reference extraction (REQUIRED — read this carefully): @@ -60,7 +58,7 @@ Focus text quoting (REQUIRED): - `/opencode:adversarial-review --background look for race conditions in $RUNTIME` → `node ... adversarial-review --background 'look for race conditions in $RUNTIME'` Foreground flow: -- First, transform `$ARGUMENTS` using the **PR reference extraction** and **Focus text quoting** rules above. Pass through `--wait`, `--background`, `--base`, `--scope`, `--model`, `--pr`, `--post`, and `--confidence-threshold` flags as-is; convert any `PR #N` reference in the user's text to `--pr N`; single-quote whatever free-form focus text remains. +- First, transform `$ARGUMENTS` using the **PR reference extraction** and **Focus text quoting** rules above. Pass through `--wait`, `--background`, `--base`, `--scope`, `--model`, and `--pr` flags as-is; convert any `PR #N` reference in the user's text to `--pr N`; single-quote whatever free-form focus text remains. - Then run the resulting command (illustrative shape — substitute the actual transformed args): ```bash node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" adversarial-review [--pr N] [''] @@ -68,7 +66,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" adversarial-review < - Return the command stdout verbatim, exactly as-is. - Do not paraphrase, summarize, or add commentary before or after it. - Do not fix any issues mentioned in the review output. -- If `--post` was set, ALSO follow the "Post-review publishing" steps below before your final turn output. +- If the user's original request asked for the review to be posted/commented/published to the PR, follow the "Optional: post the review to GitHub" section below after returning the review output. Background flow: - Apply the same `$ARGUMENTS` transformation as the foreground flow above (PR ref extraction + focus text single-quoting). @@ -82,25 +80,19 @@ Bash({ ``` - Do not call `BashOutput` or wait for completion in this turn. - After launching the command, tell the user: "OpenCode adversarial review started in the background. Check `/opencode:status` for progress." -- When the user later asks to collect the result and `--post` was part of the original arguments, read the completed `BashOutput` and then follow the "Post-review publishing" steps below on the captured stderr. +- When the user later asks to collect the result and the original request asked for a PR post, read the completed `BashOutput`, return the review output, then follow the "Optional: post the review to GitHub" section below. -Post-review publishing (only when `--post` is set): -- After the companion script exits, scan its **stderr** (not stdout) for a block shaped like: -``` - -412 -3 -2 -/tmp/opencode-plugin-cc/post-pr-412-…json -gh api -X POST "repos/{owner}/{repo}/pulls/412/reviews" --input '/tmp/…json' -rm -f '/tmp/…json' - -``` -- If the block is present: - 1. Run the exact string inside `` via a single `Bash` tool call. Do not edit the command, do not re-serialize the JSON, do not invent a different endpoint — the companion already quoted the path and picked the correct endpoint. - 2. Parse the response body; GitHub returns a JSON object with an `html_url` field pointing at the newly-created review. - 3. After the verbatim stdout from step "Return the command stdout verbatim" above, append a single-line status like: `Review posted to PR #: `. Include "(N inline comments)" only when `` is greater than zero. - 4. Run the exact string inside `` via a second `Bash` tool call to delete the temp payload file. If this fails, mention it once; do not retry or escalate. - 5. If step 1 fails (non-zero exit, or `gh` returns an error), append `Failed to post review to PR #: ` instead of the success line and still run ``. Do not retry. -- If the block is **not** present (either `--post` wasn't requested or the companion's preparation step failed — in which case stderr will contain a `[opencode-companion] Failed to prepare PR post …` line instead), do nothing extra. -- Never POST, comment, or otherwise mutate the PR unless you found an `` block in stderr on THIS run. Do not "help" by reposting or retrying a previous run's block. +Optional: post the review to GitHub (only when the user explicitly asked): +- Triggers: the user's slash-command invocation said something like "and post it to the PR", "comment it back", "publish the review", "review on GitHub". A bare `/opencode:adversarial-review --pr 412` is NOT a request to post. If in doubt, do not post. +- Preconditions: `--pr ` was in the arguments AND the user's request explicitly asked for publishing. Never post a review for a `--path` run (paths have no PR to post to). +- What to post: + - A single GitHub PR review (`event: COMMENT`) containing a clean markdown summary body written in your own voice based on the review output. Do not paste the raw review output verbatim into the body; rewrite it as a digest with a short ship/no-ship line, the handful of material findings, and a closing note. + - Optionally, for findings where you are confident about the exact file and line, include inline review comments anchored to those lines. Only include an inline comment if the line is part of the PR's unified diff (`gh api repos/{owner}/{repo}/pulls//files` → `patch` fields). Skip inline anchoring when the review is prose-only or the lines are unclear. +- How to post: + 1. Compose the payload as a single JSON object: `{ "commit_id": "", "event": "COMMENT", "body": "", "comments": [ { "path": "...", "line": N, "side": "RIGHT", "body": "..." }, ... ] }`. Get the head SHA via `gh pr view --json headRefOid --jq .headRefOid`. + 2. Write the payload to a temp file (`/tmp/opencode-post--.json`) using the `Write` tool so you control escaping — do NOT try to build a heredoc inside a Bash call. + 3. Run `gh api -X POST "repos/{owner}/{repo}/pulls//reviews" --input /tmp/...json` via a single `Bash` call. + 4. Extract `html_url` from the response and append a one-line status to your final turn output: `Review posted to PR #: `. + 5. Delete the temp file with `rm -f /tmp/...json`. + 6. On failure, say `Failed to post review to PR #: ` and do not retry. +- The review is always published with `event: "COMMENT"` — never `REQUEST_CHANGES`. This tool is advisory, not gatekeeping. diff --git a/plugins/opencode/commands/review.md b/plugins/opencode/commands/review.md index cfaf5a6..4e0a534 100644 --- a/plugins/opencode/commands/review.md +++ b/plugins/opencode/commands/review.md @@ -1,6 +1,6 @@ --- description: Run an OpenCode code review against local git state or a GitHub PR -argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model | --free] [--pr ] [--post] [--confidence-threshold ] [--path ] [--path ]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model | --free] [--pr ] [--path ] [--path ]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), Bash(gh:*), AskUserQuestion --- @@ -42,8 +42,6 @@ Argument handling: - `--model ` overrides the saved setup default model and OpenCode's own default model for this single review (e.g. `--model openrouter/anthropic/claude-opus-4-6`). Pass it through verbatim if the user supplied it. - `--free` tells the companion script to shell out to `opencode models`, filter for first-party `opencode/*` free-tier models (those ending in `:free` or `-free`), and pick one at random for this review. Restricted to the `opencode/*` provider because OpenRouter free-tier models have inconsistent tool-use support, and the review agent needs `read`/`grep`/`glob`/`list`. Pass it through verbatim if the user supplied it. `--free` and `--model` are mutually exclusive — the companion will error if both are given. - `--pr ` reviews a GitHub pull request via `gh pr diff` instead of the local working tree. The cwd must be a git repo whose remote points at the PR's repository, and `gh` must be installed and authenticated. Pass it through verbatim if the user supplied it. -- `--post` (opt-in, requires `--pr`) publishes the review as a PR review comment on GitHub. The summary (verdict + findings table) is posted as the review body, and any finding with confidence at or above the threshold whose line is part of the PR diff is posted as an inline review comment on that line. Findings below the threshold or pointing at lines outside the diff stay in the summary only. The review is always published with `event: "COMMENT"` — never `REQUEST_CHANGES` — because this tool is advisory. Pass `--post` through verbatim if the user supplied it. -- `--confidence-threshold ` (optional, default `0.8`) controls which findings become inline comments when `--post` is set. Accepts `0..1` floats or percentages (`80`, `80%`). Pass through verbatim if the user supplied it. - `--path ` reviews a specific file or directory instead of git diff. Can be specified multiple times (`--path src --path lib`). When `--path` is set, the review is assembled from the actual file contents at those paths rather than from `git diff`. This is useful for reviewing specific directories, fixed sets of files, or large untracked/imported code drops. Mutually exclusive with `--pr` (paths take precedence over PR mode). - **PR reference extraction (REQUIRED)**: if the user's input contains a PR reference like `PR #390`, `pr #390`, `PR 390`, or `pr 390` (e.g. `/opencode:review on PR #390`), you MUST extract the number yourself and pass it as `--pr 390`. Do not pass `PR #390` literally to bash — bash strips unquoted `#NNN` tokens as comments before they reach the companion script. Example: `node ... review --pr 390`, NOT `node ... review on PR #390`. @@ -55,7 +53,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" review $ARGUMENTS - Return the command stdout verbatim, exactly as-is. - Do not paraphrase, summarize, or add commentary before or after it. - Do not fix any issues mentioned in the review output. -- If `--post` was set, ALSO follow the "Post-review publishing" steps below before your final turn output. +- If the user's original request asked for the review to be posted/commented/published to the PR, follow the "Optional: post the review to GitHub" section below after returning the review output. Background flow: - Launch the review with `Bash` in the background: @@ -68,25 +66,19 @@ Bash({ ``` - Do not call `BashOutput` or wait for completion in this turn. - After launching the command, tell the user: "OpenCode review started in the background. Check `/opencode:status` for progress." -- When the user later asks to collect the result and `--post` was part of the original arguments, read the completed `BashOutput` and then follow the "Post-review publishing" steps below on the captured stderr. +- When the user later asks to collect the result and the original request asked for a PR post, read the completed `BashOutput`, return the review output, then follow the "Optional: post the review to GitHub" section below. -Post-review publishing (only when `--post` is set): -- After the companion script exits, scan its **stderr** (not stdout) for a block shaped like: -``` - -412 -3 -2 -/tmp/opencode-plugin-cc/post-pr-412-…json -gh api -X POST "repos/{owner}/{repo}/pulls/412/reviews" --input '/tmp/…json' -rm -f '/tmp/…json' - -``` -- If the block is present: - 1. Run the exact string inside `` via a single `Bash` tool call. Do not edit the command, do not re-serialize the JSON, do not invent a different endpoint — the companion already quoted the path and picked the correct endpoint. - 2. Parse the response body; GitHub returns a JSON object with an `html_url` field pointing at the newly-created review. - 3. After the verbatim stdout from step "Return the command stdout verbatim" above, append a single-line status like: `Review posted to PR #: `. Include "(N inline comments)" only when `` is greater than zero. - 4. Run the exact string inside `` via a second `Bash` tool call to delete the temp payload file. If this fails, mention it once; do not retry or escalate. - 5. If step 1 fails (non-zero exit, or `gh` returns an error), append `Failed to post review to PR #: ` instead of the success line and still run ``. Do not retry. -- If the block is **not** present (either `--post` wasn't requested or the companion's preparation step failed — in which case stderr will contain a `[opencode-companion] Failed to prepare PR post …` line instead), do nothing extra. -- Never POST, comment, or otherwise mutate the PR unless you found an `` block in stderr on THIS run. Do not "help" by reposting or retrying a previous run's block. +Optional: post the review to GitHub (only when the user explicitly asked): +- Triggers: the user's slash-command invocation said something like "and post it to the PR", "comment it back", "publish the review", "review on GitHub". A bare `/opencode:review --pr 412` is NOT a request to post. If in doubt, do not post. +- Preconditions: `--pr ` was in the arguments AND the user's request explicitly asked for publishing. Never post a review for a `--path` run (paths have no PR to post to). +- What to post: + - A single GitHub PR review (`event: COMMENT`) containing a clean markdown summary body written in your own voice based on the review output. Do not paste the raw review output verbatim into the body; rewrite it as a digest with a short ship/no-ship line, the handful of material findings, and a closing note. + - Optionally, for findings where you are confident about the exact file and line, include inline review comments anchored to those lines. Only include an inline comment if the line is part of the PR's unified diff (`gh api repos/{owner}/{repo}/pulls//files` → `patch` fields). Skip inline anchoring when the review is prose-only or the lines are unclear. +- How to post: + 1. Compose the payload as a single JSON object: `{ "commit_id": "", "event": "COMMENT", "body": "", "comments": [ { "path": "...", "line": N, "side": "RIGHT", "body": "..." }, ... ] }`. Get the head SHA via `gh pr view --json headRefOid --jq .headRefOid`. + 2. Write the payload to a temp file (`/tmp/opencode-post--.json`) using the `Write` tool so you control escaping — do NOT try to build a heredoc inside a Bash call. + 3. Run `gh api -X POST "repos/{owner}/{repo}/pulls//reviews" --input /tmp/...json` via a single `Bash` call. + 4. Extract `html_url` from the response and append a one-line status to your final turn output: `Review posted to PR #: `. + 5. Delete the temp file with `rm -f /tmp/...json`. + 6. On failure, say `Failed to post review to PR #: ` and do not retry. +- The review is always published with `event: "COMMENT"` — never `REQUEST_CHANGES`. This tool is advisory, not gatekeeping. diff --git a/plugins/opencode/prompts/adversarial-review.md b/plugins/opencode/prompts/adversarial-review.md index da4ef3f..7cf4d94 100644 --- a/plugins/opencode/prompts/adversarial-review.md +++ b/plugins/opencode/prompts/adversarial-review.md @@ -44,18 +44,29 @@ A finding should answer: 4. What concrete change would reduce the risk? - -Return only valid JSON matching the provided schema. -Keep the output compact and specific. -Use `needs-attention` if there is any material risk worth blocking on. -Use `approve` only if you cannot support any substantive adversarial finding from the provided context. -Every finding must include: -- the affected file -- `line_start` and `line_end` -- a confidence score from 0 to 1 -- a concrete recommendation -Write the summary like a terse ship/no-ship assessment, not a neutral recap. - + +Respond in plain markdown prose. Do not wrap the response in JSON, do +not emit a code-fenced schema, and do not prefix the review with a +labelled verdict token. + +Open with a one-line ship/no-ship assessment in your own words. + +Then, for every material finding, use the shape below (literal +headings, in order): + +### — +- **File:** `<path>`:<line_start>-<line_end> +- **Confidence:** <low | medium | high> + +<one or two paragraphs of adversarial analysis> + +**Recommendation:** <concrete change> + +Keep severity one of `LOW`, `MEDIUM`, `HIGH`, `CRITICAL`. Keep the file +path real — do not invent paths or lines. If you have no material +findings, say so directly after the opening line and stop; do not pad +with stylistic nits. +</output_format> <grounding_rules> Be aggressive, but stay grounded. diff --git a/plugins/opencode/scripts/lib/pr-comments.mjs b/plugins/opencode/scripts/lib/pr-comments.mjs deleted file mode 100644 index c255545..0000000 --- a/plugins/opencode/scripts/lib/pr-comments.mjs +++ /dev/null @@ -1,581 +0,0 @@ -// Prepare a GitHub PR review payload from an OpenCode review. -// -// Structured review findings from OpenCode have a `file`, `line_start`, -// `line_end`, `confidence`, and a recommendation. We turn them into: -// - a summary comment body for the PR review, and -// - inline review comments anchored to specific lines for findings -// whose confidence exceeds the user-supplied threshold (default 0.8) -// AND whose target line is addressable on GitHub's unified diff for -// that PR. -// -// GitHub rejects review comments on lines that are not part of the PR's -// diff, so we parse each file's `patch` returned by the pulls/files -// endpoint to learn which RIGHT-side line numbers are addressable. A -// high-confidence finding whose line is outside the diff silently -// degrades to summary-only; we never drop the finding. -// -// Execution model: this module does NOT call `gh api` to POST the -// review itself. It constructs a ready-to-POST JSON payload, writes it -// to a temp file, and returns a `gh api … --input <file>` command. The -// slash-command runner (Claude Code) is responsible for executing that -// command via its Bash tool. This keeps complex gh plumbing out of -// Node, lets Claude show/confirm the payload before it fires, and -// sidesteps the whole class of JSON-stream-reassembly bugs that come -// with `gh api --paginate`. - -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import crypto from "node:crypto"; -import { spawn } from "node:child_process"; - -/** - * Prepare a POST-ready GitHub review payload for `prNumber`. Never - * throws — returns `{ prepared: false, error }` on failure. Callers - * should emit the returned command as a structured trailer for Claude - * to execute. - * - * @param {string} workspace - cwd for the `gh` invocations - * @param {object} opts - * @param {number} opts.prNumber - * @param {object|null} opts.structured - parsed review JSON (or null) - * @param {string} [opts.rendered] - fallback raw review text (used - * when `structured` is null so the summary comment still has - * *something* to say) - * @param {{ providerID: string, modelID: string }|null} opts.model - * @param {boolean} opts.adversarial - * @param {number} [opts.confidenceThreshold=0.8] - * @param {object} [opts.prData] - pre-fetched `{ headSha, files }`, - * primarily for tests; production callers omit this and let the - * module fetch it via `gh`. - * @returns {Promise< - * | { prepared: true, command: string, cleanup: string, payloadPath: string, inlineCount: number, summaryOnlyCount: number, prNumber: number } - * | { prepared: false, error: string } - * >} - */ -export async function preparePostInstructions(workspace, opts) { - const { - prNumber, - structured, - rendered, - model, - adversarial, - confidenceThreshold = 0.8, - } = opts; - - try { - const prData = opts.prData ?? (await fetchPrData(workspace, prNumber)); - - const findings = Array.isArray(structured?.findings) - ? structured.findings - : []; - const addableByFile = buildAddableLineMap(prData.files); - const { inline, summaryOnly } = splitFindings( - findings, - addableByFile, - confidenceThreshold - ); - - const summaryBody = renderSummaryBody({ - structured, - rendered, - model, - adversarial, - inlineCount: inline.length, - summaryOnlyCount: summaryOnly.length, - confidenceThreshold, - }); - - const payload = { - commit_id: prData.headSha, - event: "COMMENT", - body: summaryBody, - comments: inline.map(findingToInlineComment), - }; - - const payloadPath = writePayloadFile(prNumber, payload); - const quotedPath = shQuote(payloadPath); - const command = `gh api -X POST "repos/{owner}/{repo}/pulls/${prNumber}/reviews" --input ${quotedPath}`; - const cleanup = `rm -f ${quotedPath}`; - - return { - prepared: true, - command, - cleanup, - payloadPath, - inlineCount: inline.length, - summaryOnlyCount: summaryOnly.length, - prNumber, - }; - } catch (err) { - return { prepared: false, error: err.message }; - } -} - -// --------------------------------------------------------------------- -// Payload file + shell quoting -// --------------------------------------------------------------------- - -/** - * Write `payload` to a unique temp file and return its absolute path. - * Exported so tests can call it directly and assert file contents. - */ -export function writePayloadFile(prNumber, payload) { - const dir = path.join(os.tmpdir(), "opencode-plugin-cc"); - fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); - const suffix = crypto.randomBytes(4).toString("hex"); - const filename = `post-pr-${prNumber}-${Date.now()}-${suffix}.json`; - const full = path.join(dir, filename); - fs.writeFileSync(full, JSON.stringify(payload, null, 2), { - encoding: "utf8", - mode: 0o600, - }); - return full; -} - -/** - * POSIX single-quote `s` so bash/zsh pass it through literally. The - * only escape needed inside single quotes is the closing quote itself, - * which is handled by the standard `'\''` trick. - */ -export function shQuote(s) { - return `'${String(s).replace(/'/g, "'\\''")}'`; -} - -// --------------------------------------------------------------------- -// Trailer emission (for Claude Code to act on) -// --------------------------------------------------------------------- - -/** - * Render the stderr trailer the slash command reads to know it should - * POST the review. Kept plain text with tagged XML-ish children so - * Claude can parse it with a single regex and extract the command - * verbatim. - * - * @param {{ prepared: true, command: string, cleanup: string, payloadPath: string, inlineCount: number, summaryOnlyCount: number, prNumber: number }} prepared - * @returns {string} - */ -export function formatPostTrailer(prepared) { - const lines = [ - "<opencode_post_instructions>", - `<pr>${prepared.prNumber}</pr>`, - `<inline_count>${prepared.inlineCount}</inline_count>`, - `<summary_only_count>${prepared.summaryOnlyCount}</summary_only_count>`, - `<payload_path>${prepared.payloadPath}</payload_path>`, - `<command>${prepared.command}</command>`, - `<cleanup>${prepared.cleanup}</cleanup>`, - "</opencode_post_instructions>", - "", - ]; - return lines.join("\n"); -} - -// --------------------------------------------------------------------- -// gh plumbing (read-only — no POSTs) -// --------------------------------------------------------------------- - -/** - * Run a `gh` subcommand and return stdout. `input` is piped to stdin. - * Rejects with a useful error on non-zero exit codes. - */ -function runGh(workspace, args, { input } = {}) { - return new Promise((resolve, reject) => { - const proc = spawn("gh", args, { - cwd: workspace, - stdio: ["pipe", "pipe", "pipe"], - env: process.env, - }); - let stdout = ""; - let stderr = ""; - proc.stdout.on("data", (d) => { - stdout += d.toString("utf8"); - }); - proc.stderr.on("data", (d) => { - stderr += d.toString("utf8"); - }); - proc.on("error", (err) => reject(err)); - proc.on("close", (code) => { - if (code !== 0) { - reject( - new Error( - `gh ${args.join(" ")} exited ${code}: ${stderr.trim() || "(no stderr)"}` - ) - ); - return; - } - resolve(stdout); - }); - if (input != null) { - proc.stdin.write(input); - } - proc.stdin.end(); - }); -} - -/** - * Fetch the PR head SHA + the file list (with unified-diff patches) so - * we can classify findings into inline vs summary-only before writing - * the payload. Both calls are single-shot — we deliberately do NOT use - * `gh api --paginate`, because its output is a concatenation of per-page - * JSON arrays (`][` at page boundaries) and string-splitting that apart - * corrupts any patch whose content legitimately contains `][` (common - * in JS/Go code). GitHub allows `per_page=100` here, which covers the - * vast majority of real PRs. On a 100+ file PR, findings in the tail - * files simply degrade to summary-only, which is better than crashing. - */ -async function fetchPrData(workspace, prNumber) { - const headJson = await runGh(workspace, [ - "pr", - "view", - String(prNumber), - "--json", - "headRefOid", - ]); - let headSha; - try { - headSha = JSON.parse(headJson).headRefOid; - } catch (err) { - throw new Error(`gh pr view returned invalid JSON: ${err.message}`); - } - if (typeof headSha !== "string" || headSha.length === 0) { - throw new Error( - `gh pr view ${prNumber} did not return a headRefOid; is the PR visible to this token?` - ); - } - - const filesJson = await runGh(workspace, [ - "api", - `repos/{owner}/{repo}/pulls/${prNumber}/files?per_page=100`, - ]); - let files; - try { - files = JSON.parse(filesJson); - } catch (err) { - throw new Error( - `gh api pulls/${prNumber}/files returned invalid JSON: ${err.message}` - ); - } - if (!Array.isArray(files)) files = []; - return { headSha, files }; -} - -// --------------------------------------------------------------------- -// Diff parsing -// --------------------------------------------------------------------- - -/** - * Parse the unified diff in a PR file's `patch` field and return the - * set of RIGHT-side line numbers that GitHub will accept as the `line` - * field of a review comment. Those are lines present in the diff as - * either additions (`+`) or unchanged context (` `). Deletions (`-`) - * only exist on the LEFT side and would need `side: "LEFT"`, which we - * don't support — our findings target the current state of the code. - * - * Exported for tests. - * - * @param {string} patch - * @returns {Set<number>} - */ -export function parseAddableLines(patch) { - const addable = new Set(); - if (typeof patch !== "string" || patch.length === 0) return addable; - const lines = patch.split("\n"); - let rightLine = 0; - for (const line of lines) { - const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); - if (hunk) { - rightLine = Number(hunk[1]); - continue; - } - if (rightLine === 0) continue; - // Skip the `\ No newline at end of file` marker without advancing. - if (line.startsWith("\\")) continue; - // Diff headers like `+++ b/path` — ignored, they never appear inside - // a hunk but defend anyway. - if (line.startsWith("+++") || line.startsWith("---")) continue; - if (line.startsWith("+")) { - addable.add(rightLine); - rightLine += 1; - } else if (line.startsWith("-")) { - // deletion — does not advance the right side - } else if (line.startsWith(" ") || line.length === 0) { - // context line OR a truly-empty line (`split("\n")` leaves empties - // where the patch had blank context). - addable.add(rightLine); - rightLine += 1; - } - } - return addable; -} - -/** - * Build `Map<filename, Set<lineNumber>>` for all files in a PR. - * @param {Array<{ filename: string, patch?: string }>} files - * @returns {Map<string, Set<number>>} - */ -export function buildAddableLineMap(files) { - const map = new Map(); - if (!Array.isArray(files)) return map; - for (const file of files) { - if (!file || typeof file.filename !== "string") continue; - const addable = parseAddableLines(file.patch); - if (addable.size > 0) map.set(file.filename, addable); - } - return map; -} - -// --------------------------------------------------------------------- -// Finding classification -// --------------------------------------------------------------------- - -/** - * Classify findings into inline-anchored vs summary-only. A finding is - * inline-eligible iff: - * - confidence >= threshold, AND - * - its file is present in the PR diff, AND - * - at least one of the lines in [line_start, line_end] is addable - * (present in the diff as context or addition). - * - * Exported for tests. - * - * @param {any[]} findings - * @param {Map<string, Set<number>>} addableByFile - * @param {number} threshold - */ -export function splitFindings(findings, addableByFile, threshold) { - const inline = []; - const summaryOnly = []; - if (!Array.isArray(findings)) return { inline, summaryOnly }; - - for (const f of findings) { - if (!f || typeof f !== "object") continue; - - const conf = typeof f.confidence === "number" ? f.confidence : null; - const hasConfidence = conf != null && conf >= threshold; - if (!hasConfidence) { - summaryOnly.push(f); - continue; - } - - const file = typeof f.file === "string" ? f.file : null; - const addable = file ? addableByFile.get(file) : null; - if (!addable) { - summaryOnly.push(f); - continue; - } - - const start = Number(f.line_start); - const end = Number.isFinite(Number(f.line_end)) ? Number(f.line_end) : start; - if (!Number.isFinite(start) || start <= 0) { - summaryOnly.push(f); - continue; - } - - let target = null; - const lo = Math.min(start, end); - const hi = Math.max(start, end); - for (let ln = lo; ln <= hi; ln += 1) { - if (addable.has(ln)) { - target = ln; - break; - } - } - if (target == null) { - summaryOnly.push(f); - continue; - } - - inline.push({ ...f, _targetLine: target }); - } - - return { inline, summaryOnly }; -} - -/** - * Build an inline review comment payload from a classified finding. - * Exported for tests. - */ -export function findingToInlineComment(finding) { - const sevRaw = typeof finding.severity === "string" ? finding.severity : null; - const sev = sevRaw ? sevRaw.toUpperCase() : null; - const confPct = - typeof finding.confidence === "number" - ? `${Math.round(finding.confidence * 100)}%` - : null; - - const header = [sev ? `**${sev}**` : null, finding.title ?? "Finding"] - .filter(Boolean) - .join(" · "); - const meta = confPct ? `_Confidence ${confPct}_` : ""; - - const body = []; - body.push(header); - if (meta) body.push(meta); - body.push(""); - if (finding.body) body.push(String(finding.body)); - if (finding.recommendation) { - body.push(""); - body.push(`**Recommendation:** ${finding.recommendation}`); - } - body.push(""); - body.push("_Posted by opencode-plugin-cc._"); - - return { - path: finding.file, - line: finding._targetLine, - side: "RIGHT", - body: body.join("\n"), - }; -} - -// --------------------------------------------------------------------- -// Summary body rendering -// --------------------------------------------------------------------- - -/** - * Build the top-level review comment body. Exported for tests. - * @param {object} opts - * @param {object|null} opts.structured - * @param {string} [opts.rendered] - * @param {{providerID: string, modelID: string}|null} opts.model - * @param {boolean} opts.adversarial - * @param {number} opts.inlineCount - * @param {number} opts.summaryOnlyCount - * @param {number} opts.confidenceThreshold - * @returns {string} - */ -export function renderSummaryBody(opts) { - const { - structured, - rendered, - model, - adversarial, - inlineCount, - summaryOnlyCount, - confidenceThreshold, - } = opts; - - const lines = []; - const title = adversarial - ? "OpenCode Adversarial Review" - : "OpenCode Review"; - - let verdictLabel; - if (structured?.verdict === "approve") verdictLabel = "Approve"; - else if (structured?.verdict === "needs-attention") - verdictLabel = "Needs attention"; - else verdictLabel = "Advisory"; - - lines.push(`## ${title} — ${verdictLabel}`); - lines.push(""); - - if (model?.providerID && model?.modelID) { - lines.push(`**Model:** \`${model.providerID}/${model.modelID}\``); - lines.push(""); - } - - if (structured?.summary) { - lines.push(String(structured.summary)); - lines.push(""); - } - - const findings = Array.isArray(structured?.findings) ? structured.findings : []; - - if (structured && findings.length === 0) { - lines.push("_No material findings._"); - lines.push(""); - } else if (findings.length > 0) { - lines.push(`### Findings (${findings.length})`); - lines.push(""); - lines.push("| # | Severity | Confidence | File | Lines | Title |"); - lines.push("|---|----------|------------|------|-------|-------|"); - findings.forEach((f, i) => { - const sev = typeof f.severity === "string" ? f.severity.toUpperCase() : "—"; - const conf = - typeof f.confidence === "number" - ? `${Math.round(f.confidence * 100)}%` - : "—"; - const file = typeof f.file === "string" ? `\`${escapeTableCell(f.file)}\`` : "—"; - const lo = Number(f.line_start); - const hi = Number(f.line_end); - const range = Number.isFinite(lo) - ? Number.isFinite(hi) && hi !== lo - ? `${lo}–${hi}` - : `${lo}` - : "—"; - const titleCell = escapeTableCell(f.title ?? ""); - lines.push(`| ${i + 1} | ${sev} | ${conf} | ${file} | ${range} | ${titleCell} |`); - }); - lines.push(""); - - lines.push("<details><summary>Full findings</summary>"); - lines.push(""); - findings.forEach((f, i) => { - const sev = typeof f.severity === "string" ? f.severity.toUpperCase() : ""; - const lo = Number(f.line_start); - const hi = Number(f.line_end); - const range = - Number.isFinite(lo) && Number.isFinite(hi) && hi !== lo - ? `${lo}–${hi}` - : Number.isFinite(lo) - ? `${lo}` - : "?"; - lines.push(`#### ${i + 1}. ${sev ? `${sev} — ` : ""}${f.title ?? "Finding"}`); - if (typeof f.file === "string") { - lines.push(`- **File:** \`${f.file}\`:${range}`); - } - if (typeof f.confidence === "number") { - lines.push(`- **Confidence:** ${Math.round(f.confidence * 100)}%`); - } - if (f.body) { - lines.push(""); - lines.push(String(f.body)); - } - if (f.recommendation) { - lines.push(""); - lines.push(`**Recommendation:** ${f.recommendation}`); - } - lines.push(""); - }); - lines.push("</details>"); - lines.push(""); - } else if (!structured && typeof rendered === "string" && rendered.trim()) { - // No structured output — fall back to posting the raw rendered review. - lines.push( - "_The model did not return structured JSON; raw review text below._" - ); - lines.push(""); - lines.push(rendered.trim()); - lines.push(""); - } - - const threshPct = Math.round(confidenceThreshold * 100); - const stats = []; - if (inlineCount > 0) { - stats.push( - `${inlineCount} finding${inlineCount === 1 ? "" : "s"} at or above ${threshPct}% confidence posted as inline comment${inlineCount === 1 ? "" : "s"}.` - ); - } - if (summaryOnlyCount > 0) { - stats.push( - `${summaryOnlyCount} finding${summaryOnlyCount === 1 ? "" : "s"} kept in the summary only (below threshold or outside the PR diff).` - ); - } - if (stats.length > 0) { - lines.push(stats.join(" ")); - lines.push(""); - } - - lines.push("---"); - lines.push( - "_Advisory review generated by [opencode-plugin-cc](https://github.com/JohnnyVicious/opencode-plugin-cc)._" - ); - - return lines.join("\n"); -} - -function escapeTableCell(text) { - return String(text) - .replace(/\|/g, "\\|") - .replace(/\r?\n/g, " "); -} diff --git a/plugins/opencode/scripts/lib/prompts.mjs b/plugins/opencode/scripts/lib/prompts.mjs index 7a2e0af..428b5a0 100644 --- a/plugins/opencode/scripts/lib/prompts.mjs +++ b/plugins/opencode/scripts/lib/prompts.mjs @@ -199,7 +199,22 @@ function buildStandardReviewPrompt(diff, status, changedFiles, opts) { return `You are performing a code review of ${targetLabel}. -Review the following changes and provide structured feedback in JSON format matching the review-output schema. +Respond in plain markdown prose. Do NOT wrap the review in JSON and do +NOT emit a code-fenced schema. Open with a one-line ship/no-ship +assessment in your own words. + +For every material finding, use the shape below (literal headings, in +order): + +### <SEVERITY> — <title> +- **File:** \`<path>\`:<line_start>-<line_end> +- **Confidence:** <low | medium | high> + +<one or two paragraphs of analysis> + +**Recommendation:** <concrete change> + +Severity must be one of \`LOW\`, \`MEDIUM\`, \`HIGH\`, \`CRITICAL\`. Focus on: - Correctness and logic errors @@ -208,7 +223,9 @@ Focus on: - Missing error handling - API contract violations -Be concise and actionable. Only report real issues, not style preferences. +Be concise and actionable. Only report real issues, not style +preferences. If you have no material findings, say so directly after +the opening line and stop. ${collectionGuidance} diff --git a/plugins/opencode/scripts/lib/render.mjs b/plugins/opencode/scripts/lib/render.mjs index 2cdd5f0..d643c63 100644 --- a/plugins/opencode/scripts/lib/render.mjs +++ b/plugins/opencode/scripts/lib/render.mjs @@ -101,40 +101,6 @@ export function renderResult(job, resultData) { return lines.join("\n"); } -/** - * Render a review result (structured JSON output). - * @param {object} review - * @returns {string} - */ -export function renderReview(review) { - const lines = []; - - if (review.verdict) { - const emoji = review.verdict === "approve" ? "PASS" : "NEEDS ATTENTION"; - lines.push(`## Review Verdict: ${emoji}\n`); - } - - if (review.summary) { - lines.push(`${review.summary}\n`); - } - - if (review.findings?.length > 0) { - lines.push(`### Findings (${review.findings.length})\n`); - for (const f of review.findings) { - lines.push(`#### ${f.severity?.toUpperCase()}: ${f.title}`); - lines.push(`- **File**: ${f.file}:${f.line_start}-${f.line_end}`); - lines.push(`- **Confidence**: ${(f.confidence * 100).toFixed(0)}%`); - lines.push(`- ${f.body}`); - lines.push(`- **Recommendation**: ${f.recommendation}`); - lines.push(""); - } - } else { - lines.push("No findings."); - } - - return lines.join("\n"); -} - /** * Extract text content from a message object. * @param {object} msg diff --git a/plugins/opencode/scripts/lib/review-parser.mjs b/plugins/opencode/scripts/lib/review-parser.mjs deleted file mode 100644 index 0e68507..0000000 --- a/plugins/opencode/scripts/lib/review-parser.mjs +++ /dev/null @@ -1,218 +0,0 @@ -// Schema-aware parser for OpenCode review output. -// -// The review system prompt asks the model for a JSON object shaped -// like `{ verdict, summary, findings: [...] }`. Models frequently -// produce JSON that is structurally valid *except* for a string -// field (usually `summary` or a finding's `body`) that contains a -// literal `"` that should have been escaped `\"`. -// -// The common failure mode looks like this: -// -// { -// "verdict": "approve", -// "summary": "... explicit {"success": false, "error": "..."} ...", -// "findings": [] -// } -// -// `JSON.parse` bails at the first unescaped `"` inside `summary`, so -// `structured` comes back null and the companion falls through to -// printing the raw text. We then render ugly JSON in the chat AND in -// the posted PR comment — see issue report on v1.0.10. -// -// Fix strategy: -// 1. Try strict `JSON.parse` first (fast path). -// 2. If that fails, extract each top-level field by anchoring on its -// key name. We slice `summary` between `"summary": "` and the -// `", "findings": [...]` anchor that comes after it, so embedded -// quotes never break the extraction. -// 3. For `findings`, do a depth-aware bracket walk that tracks -// string state, and attempt to parse the extracted array. If -// even that fails, return an empty findings array — we'd rather -// show verdict + summary than give up entirely. -// -// This is NOT a general-purpose JSON repair library. It is narrowly -// tailored to the `{verdict, summary, findings}` review schema and -// assumes the model emits the fields in that order (which our prompt -// template encourages). Anything outside that schema falls through to -// `null` and the caller treats the output as unstructured. - -/** - * @typedef {{ - * verdict: string, - * summary: string, - * findings: Array<object>, - * }} Review - */ - -/** - * Try to parse `text` as an OpenCode review. Returns `null` when even - * the lenient fallback can't recover the verdict, which is the minimum - * the caller needs to render anything useful. - * - * @param {string} text - * @returns {Review|null} - */ -export function tryParseReview(text) { - if (typeof text !== "string") return null; - const candidate = stripCodeFence(text).trim(); - if (!candidate) return null; - - // Fast path: strict JSON.parse. - try { - const parsed = JSON.parse(candidate); - if (parsed && typeof parsed === "object") { - return normalizeReview(parsed); - } - } catch { - // fall through to lenient extraction - } - - return lenientExtract(candidate); -} - -/** - * Strip a ```json … ``` code fence if present. Returns the inner - * content, or the original text when there is no fence. - */ -function stripCodeFence(text) { - const fenced = /```(?:json)?\s*\n?([\s\S]*?)\n?```/.exec(text); - return fenced ? fenced[1] : text; -} - -function normalizeReview(parsed) { - const verdict = typeof parsed.verdict === "string" ? parsed.verdict : null; - if (!verdict) return null; - const summary = typeof parsed.summary === "string" ? parsed.summary : ""; - const findings = Array.isArray(parsed.findings) ? parsed.findings : []; - return { verdict, summary, findings }; -} - -// --------------------------------------------------------------------- -// Lenient extraction -// --------------------------------------------------------------------- - -/** - * Schema-aware extractor used when strict JSON.parse fails. Walks - * `text` looking for the three known top-level keys by name. - * - * @param {string} text - * @returns {Review|null} - */ -function lenientExtract(text) { - const verdict = extractVerdict(text); - if (!verdict) return null; - const summary = extractSummary(text) ?? ""; - const findings = extractFindings(text); - return { verdict, summary, findings }; -} - -/** - * Verdict values are from a closed vocabulary (`approve` or - * `needs-attention`), so a plain regex is safe — there is no way for - * a verdict value to itself contain a `"`. - */ -function extractVerdict(text) { - const m = /"verdict"\s*:\s*"(approve|needs-attention)"/.exec(text); - return m ? m[1] : null; -} - -/** - * Slice `summary` between `"summary": "` and the next occurrence of - * the `", "findings"` anchor. Anything in between — including literal - * unescaped `"` characters — is treated as part of the summary. When - * the anchor isn't present we fall back to slicing to the last `"` - * before the closing `}`, so summaries in malformed responses that - * are missing the findings field still come through. - */ -function extractSummary(text) { - const startKey = /"summary"\s*:\s*"/.exec(text); - if (!startKey) return null; - const sliceStart = startKey.index + startKey[0].length; - - // Preferred anchor: the `", "findings"` transition. The regex allows - // whitespace / newlines between the closing quote and the next key. - const endAnchor = /"\s*,\s*"findings"\s*:/g; - endAnchor.lastIndex = sliceStart; - const endMatch = endAnchor.exec(text); - if (endMatch) { - return text.substring(sliceStart, endMatch.index); - } - - // Fallback: slice to the last `"` before the outermost closing `}`. - const lastBrace = text.lastIndexOf("}"); - const searchEnd = lastBrace > sliceStart ? lastBrace : text.length; - const lastQuote = text.lastIndexOf('"', searchEnd); - if (lastQuote > sliceStart) { - return text.substring(sliceStart, lastQuote); - } - return null; -} - -/** - * Extract the `findings` array. Uses a depth-aware walker that tracks - * JSON string state so brackets inside string literals don't confuse - * the bracket counter. If the extracted slice fails strict JSON.parse - * we return an empty array — we'd rather show the verdict + summary - * than nothing at all. - */ -function extractFindings(text) { - const startKey = /"findings"\s*:\s*\[/.exec(text); - if (!startKey) return []; - const arrayStart = startKey.index + startKey[0].length - 1; // points at `[` - const arrayText = sliceMatchingBracket(text, arrayStart); - if (!arrayText) return []; - try { - const parsed = JSON.parse(arrayText); - return Array.isArray(parsed) ? parsed : []; - } catch { - return []; - } -} - -/** - * Walk `text` starting at `openIdx` (which must point at a `[` or - * `{`) and return the substring up to and including the matching - * closing bracket, or `null` if no match is found. Tracks JSON string - * state so brackets inside strings don't affect the depth counter. - * - * Exported for tests. - * - * @param {string} text - * @param {number} openIdx - * @returns {string|null} - */ -export function sliceMatchingBracket(text, openIdx) { - const open = text[openIdx]; - if (open !== "[" && open !== "{") return null; - const close = open === "[" ? "]" : "}"; - let depth = 0; - let inString = false; - let escape = false; - for (let i = openIdx; i < text.length; i += 1) { - const ch = text[i]; - if (escape) { - escape = false; - continue; - } - if (ch === "\\") { - escape = true; - continue; - } - if (inString) { - if (ch === '"') inString = false; - continue; - } - if (ch === '"') { - inString = true; - continue; - } - if (ch === open) depth += 1; - if (ch === close) { - depth -= 1; - if (depth === 0) { - return text.substring(openIdx, i + 1); - } - } - } - return null; -} diff --git a/plugins/opencode/scripts/opencode-companion.mjs b/plugins/opencode/scripts/opencode-companion.mjs index d9b624a..ba3b68c 100644 --- a/plugins/opencode/scripts/opencode-companion.mjs +++ b/plugins/opencode/scripts/opencode-companion.mjs @@ -69,7 +69,6 @@ import { createJobRecord, runTrackedJob, getClaudeSessionId } from "./lib/tracke import { renderStatus, renderResult, - renderReview, renderSetup, extractResponseModel, formatModelHeader, @@ -83,8 +82,6 @@ import { } from "./lib/worktree.mjs"; import { readJson, readDenyRules } from "./lib/fs.mjs"; import { resolveReviewAgent } from "./lib/review-agent.mjs"; -import { preparePostInstructions, formatPostTrailer } from "./lib/pr-comments.mjs"; -import { tryParseReview } from "./lib/review-parser.mjs"; import { parseModelString, selectFreeModel } from "./lib/model.mjs"; import { applyDefaultModelOptions, @@ -302,9 +299,9 @@ async function resolveRequestedModel(options) { async function handleReview(argv) { const { options } = parseArgs(argv, { - valueOptions: ["base", "scope", "model", "pr", "path", "confidence-threshold"], + valueOptions: ["base", "scope", "model", "pr", "path"], multiValueOptions: ["path"], - booleanOptions: ["wait", "background", "free", "post"], + booleanOptions: ["wait", "background", "free"], }); const prNumber = options.pr ? Number(options.pr) : null; @@ -316,22 +313,6 @@ async function handleReview(argv) { const paths = normalizePathOption(options.path); const effectivePrNumber = paths?.length ? null : prNumber; - const postRequested = Boolean(options.post); - if (postRequested && !effectivePrNumber) { - console.error( - "--post requires --pr <number> (posting back to GitHub is PR-only; --path reviews have nowhere to post)." - ); - process.exit(1); - } - - let confidenceThreshold; - try { - confidenceThreshold = parseConfidenceThreshold(options["confidence-threshold"]); - } catch (err) { - console.error(err.message); - process.exit(1); - } - const workspace = await resolveWorkspace(); const state = loadState(workspace); const defaults = normalizeDefaults(state.config?.defaults); @@ -382,33 +363,22 @@ async function handleReview(argv) { report("finalizing", "Processing review output..."); - // Try to parse structured output. `tryParseReview` tolerates the - // common LLM failure mode where a string value contains an - // unescaped `"` (usually embedded code or JSON literals in the - // summary) and strict JSON.parse would otherwise give up. + // No structured-output contract anymore. OpenCode returns prose; + // the companion passes it through unchanged so the slash command + // runner (Claude Code) can present it however it likes and, if + // the user asked, post it to a PR with its own Bash tool. const text = extractResponseText(response); - let structured = tryParseReview(text); const usedModel = extractResponseModel(response); return { - rendered: formatModelHeader(usedModel) + (structured ? renderReview(structured) : text), + rendered: formatModelHeader(usedModel) + text, raw: response, - structured, model: usedModel, }; }); saveLastReview(workspace, result.rendered); console.log(result.rendered); - - if (postRequested) { - await emitPostTrailer(workspace, { - prNumber: effectivePrNumber, - result, - adversarial: false, - confidenceThreshold, - }); - } } catch (err) { console.error(`Review failed: ${err.message}`); process.exit(1); @@ -417,9 +387,9 @@ async function handleReview(argv) { async function handleAdversarialReview(argv) { const { options, positional } = parseArgs(argv, { - valueOptions: ["base", "scope", "model", "pr", "path", "confidence-threshold"], + valueOptions: ["base", "scope", "model", "pr", "path"], multiValueOptions: ["path"], - booleanOptions: ["wait", "background", "free", "post"], + booleanOptions: ["wait", "background", "free"], }); const workspace = await resolveWorkspace(); @@ -458,22 +428,6 @@ async function handleAdversarialReview(argv) { const paths = normalizePathOption(options.path); const effectivePrNumber = paths?.length ? null : prNumber; - const postRequested = Boolean(options.post); - if (postRequested && !effectivePrNumber) { - console.error( - "--post requires --pr <number> (posting back to GitHub is PR-only; --path reviews have nowhere to post)." - ); - process.exit(1); - } - - let confidenceThreshold; - try { - confidenceThreshold = parseConfidenceThreshold(options["confidence-threshold"]); - } catch (err) { - console.error(err.message); - process.exit(1); - } - const job = createJobRecord(workspace, "adversarial-review", { base: options.base, focus, @@ -513,98 +467,26 @@ async function handleAdversarialReview(argv) { report("finalizing", "Processing review output..."); - // See note on `tryParseReview` in handleReview — same reason. + // See note in handleReview — no structured-output contract + // anymore; prose is passed through unchanged. const text = extractResponseText(response); - let structured = tryParseReview(text); const usedModel = extractResponseModel(response); return { - rendered: formatModelHeader(usedModel) + (structured ? renderReview(structured) : text), + rendered: formatModelHeader(usedModel) + text, raw: response, - structured, model: usedModel, }; }); saveLastReview(workspace, result.rendered); console.log(result.rendered); - - if (postRequested) { - await emitPostTrailer(workspace, { - prNumber: effectivePrNumber, - result, - adversarial: true, - confidenceThreshold, - }); - } } catch (err) { console.error(`Adversarial review failed: ${err.message}`); process.exit(1); } } -/** - * Parse `--confidence-threshold` into a 0..1 float. Accepts raw floats - * (`0.8`) or percentage strings (`80`, `80%`). Returns the default 0.8 - * when the option is unset. - */ -function parseConfidenceThreshold(raw) { - if (raw == null || raw === "") return 0.8; - let text = String(raw).trim(); - const isPercent = text.endsWith("%"); - if (isPercent) text = text.slice(0, -1).trim(); - const n = Number(text); - if (!Number.isFinite(n)) { - throw new Error( - `Invalid --confidence-threshold value: ${raw} (expected a number between 0 and 1, or a percentage like 80)` - ); - } - const normalized = isPercent || n > 1 ? n / 100 : n; - if (normalized < 0 || normalized > 1) { - throw new Error( - `Invalid --confidence-threshold value: ${raw} (must be between 0 and 1, or 0 and 100 if expressed as a percentage)` - ); - } - return normalized; -} - -/** - * Prepare a GitHub review payload and emit a structured trailer on - * stderr describing the `gh api` POST command the slash-command runner - * (Claude Code) should execute to actually publish the review. - * - * The companion deliberately does NOT run the POST itself. Keeping the - * network call in Claude's Bash tool: - * - lets Claude show/confirm the payload before it fires, - * - avoids re-implementing gh plumbing (pagination, JSON stream - * reassembly, auth) in Node, and - * - makes failures debuggable from the chat instead of buried in a - * stderr line the user may never see. - * - * Never throws — on failure the trailer is replaced with a human- - * readable `[opencode-companion] Failed to prepare PR post: …` line so - * the review output (already on stdout) is not disrupted. - */ -async function emitPostTrailer(workspace, { prNumber, result, adversarial, confidenceThreshold }) { - const prepared = await preparePostInstructions(workspace, { - prNumber, - structured: result.structured, - rendered: result.rendered, - model: result.model, - adversarial, - confidenceThreshold, - }); - - if (!prepared.prepared) { - process.stderr.write( - `[opencode-companion] Failed to prepare PR post for #${prNumber}: ${prepared.error}\n` - ); - return; - } - - process.stderr.write(formatPostTrailer(prepared)); -} - // ------------------------------------------------------------------ // Task (rescue delegation) // ------------------------------------------------------------------ diff --git a/tests/pr-comments.test.mjs b/tests/pr-comments.test.mjs deleted file mode 100644 index 736167b..0000000 --- a/tests/pr-comments.test.mjs +++ /dev/null @@ -1,580 +0,0 @@ -// Unit tests for pr-comments.mjs — the module that prepares an -// OpenCode review for posting back to a GitHub PR. Since the refactor, -// the module does NOT execute `gh api` to POST anything; it constructs -// the payload, writes it to a temp file, and emits a structured -// stderr trailer for Claude Code to act on. Tests therefore cover: -// - diff parsing (addable-line classification), -// - finding classification (inline vs summary-only), -// - summary body rendering, -// - payload-file + trailer construction via preparePostInstructions -// with an injected `prData` so no real `gh` calls happen. - -import { describe, it, after } from "node:test"; -import assert from "node:assert/strict"; -import fs from "node:fs"; -import { - parseAddableLines, - buildAddableLineMap, - splitFindings, - findingToInlineComment, - renderSummaryBody, - writePayloadFile, - shQuote, - formatPostTrailer, - preparePostInstructions, -} from "../plugins/opencode/scripts/lib/pr-comments.mjs"; - -describe("parseAddableLines", () => { - it("returns an empty set for empty or missing input", () => { - assert.equal(parseAddableLines("").size, 0); - assert.equal(parseAddableLines(null).size, 0); - assert.equal(parseAddableLines(undefined).size, 0); - }); - - it("collects addition and context lines as addable RIGHT-side lines", () => { - // Hunk starting at right line 10: - // 10: context "a" - // 11: addition "b" <- addable - // 12: addition "c" <- addable - // 13: context "d" <- addable - // (deletion "e" stays on left, does not advance right) - // 14: addition "f" <- addable - const patch = [ - "@@ -5,4 +10,5 @@", - " a", - "+b", - "+c", - " d", - "-e", - "+f", - ].join("\n"); - const addable = parseAddableLines(patch); - assert.deepEqual([...addable].sort((x, y) => x - y), [10, 11, 12, 13, 14]); - }); - - it("does not mark LEFT-side deletions as addable", () => { - const patch = ["@@ -1,2 +1,1 @@", " a", "-b"].join("\n"); - const addable = parseAddableLines(patch); - assert.deepEqual([...addable], [1]); - }); - - it("handles multi-hunk patches", () => { - const patch = [ - "@@ -1,1 +1,1 @@", - "+first", - "@@ -10,1 +20,1 @@", - "+second", - ].join("\n"); - const addable = parseAddableLines(patch); - assert.deepEqual([...addable].sort((x, y) => x - y), [1, 20]); - }); - - it("skips the '\\ No newline at end of file' marker without advancing", () => { - const patch = [ - "@@ -1,1 +1,2 @@", - "+a", - "\\ No newline at end of file", - "+b", - ].join("\n"); - const addable = parseAddableLines(patch); - assert.deepEqual([...addable].sort((x, y) => x - y), [1, 2]); - }); -}); - -describe("buildAddableLineMap", () => { - it("returns an empty map for missing input", () => { - assert.equal(buildAddableLineMap(undefined).size, 0); - assert.equal(buildAddableLineMap([]).size, 0); - }); - - it("keys by filename and drops files with no patch or no addable lines", () => { - const map = buildAddableLineMap([ - { filename: "a.js", patch: "@@ -1,1 +1,1 @@\n+a" }, - { filename: "b.js" }, // no patch - { filename: "c.js", patch: "" }, // empty patch - ]); - assert.equal(map.size, 1); - assert.ok(map.get("a.js").has(1)); - }); -}); - -describe("splitFindings", () => { - const addable = new Map([["src/foo.js", new Set([10, 11, 12])]]); - - it("routes below-threshold findings to summary-only", () => { - const { inline, summaryOnly } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 10, - line_end: 10, - confidence: 0.5, - title: "weak", - }, - ], - addable, - 0.8 - ); - assert.equal(inline.length, 0); - assert.equal(summaryOnly.length, 1); - }); - - it("routes high-confidence findings with addable line to inline", () => { - const { inline, summaryOnly } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 11, - line_end: 11, - confidence: 0.9, - title: "strong", - }, - ], - addable, - 0.8 - ); - assert.equal(inline.length, 1); - assert.equal(summaryOnly.length, 0); - assert.equal(inline[0]._targetLine, 11); - }); - - it("degrades a high-confidence finding whose file is not in the diff", () => { - const { inline, summaryOnly } = splitFindings( - [ - { - file: "src/not-in-diff.js", - line_start: 1, - line_end: 1, - confidence: 0.95, - title: "orphan", - }, - ], - addable, - 0.8 - ); - assert.equal(inline.length, 0); - assert.equal(summaryOnly.length, 1); - }); - - it("degrades a high-confidence finding whose range misses the diff", () => { - const { inline, summaryOnly } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 50, - line_end: 60, - confidence: 0.9, - title: "wrong region", - }, - ], - addable, - 0.8 - ); - assert.equal(inline.length, 0); - assert.equal(summaryOnly.length, 1); - }); - - it("anchors to the first addable line in the finding's range", () => { - const map = new Map([["src/foo.js", new Set([12, 14])]]); - const { inline } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 10, - line_end: 15, - confidence: 0.9, - title: "range", - }, - ], - map, - 0.8 - ); - assert.equal(inline.length, 1); - assert.equal(inline[0]._targetLine, 12); - }); - - it("handles missing line_end by treating it as line_start", () => { - const { inline } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 11, - confidence: 0.9, - title: "single line", - }, - ], - addable, - 0.8 - ); - assert.equal(inline.length, 1); - assert.equal(inline[0]._targetLine, 11); - }); - - it("respects a higher threshold", () => { - const { inline } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 10, - confidence: 0.85, - title: "medium-high", - }, - ], - addable, - 0.95 - ); - assert.equal(inline.length, 0); - }); - - it("skips findings without a numeric confidence", () => { - const { inline, summaryOnly } = splitFindings( - [ - { - file: "src/foo.js", - line_start: 10, - title: "no confidence", - }, - ], - addable, - 0.8 - ); - assert.equal(inline.length, 0); - assert.equal(summaryOnly.length, 1); - }); -}); - -describe("findingToInlineComment", () => { - it("builds a GitHub-review comment payload with path/line/side/body", () => { - const comment = findingToInlineComment({ - file: "src/foo.js", - _targetLine: 42, - severity: "high", - confidence: 0.92, - title: "Race condition", - body: "Two writers can observe stale state.", - recommendation: "Hold the lock around the read-modify-write.", - }); - assert.equal(comment.path, "src/foo.js"); - assert.equal(comment.line, 42); - assert.equal(comment.side, "RIGHT"); - assert.match(comment.body, /HIGH/); - assert.match(comment.body, /Race condition/); - assert.match(comment.body, /92%/); - assert.match(comment.body, /Recommendation/); - assert.match(comment.body, /opencode-plugin-cc/); - }); - - it("survives a minimal finding with no severity or recommendation", () => { - const comment = findingToInlineComment({ - file: "src/foo.js", - _targetLine: 1, - title: "Minimal", - }); - assert.equal(comment.path, "src/foo.js"); - assert.equal(comment.line, 1); - assert.match(comment.body, /Minimal/); - }); -}); - -describe("renderSummaryBody", () => { - const baseStructured = { - verdict: "needs-attention", - summary: "Two issues worth addressing before merge.", - findings: [ - { - severity: "high", - confidence: 0.9, - title: "Tenant isolation", - file: "src/foo.js", - line_start: 10, - line_end: 12, - body: "Reqs from tenant A can see tenant B's data.", - recommendation: "Scope the query by tenant_id.", - }, - { - severity: "medium", - confidence: 0.5, - title: "Missing retries", - file: "src/bar.js", - line_start: 22, - line_end: 22, - body: "Downstream 5xx is not retried.", - recommendation: "Wrap in exponential backoff.", - }, - ], - }; - - it("renders the title, verdict, model, summary, findings table, and footer", () => { - const body = renderSummaryBody({ - structured: baseStructured, - model: { providerID: "openrouter", modelID: "anthropic/claude-opus-4-6" }, - adversarial: true, - inlineCount: 1, - summaryOnlyCount: 1, - confidenceThreshold: 0.8, - }); - - assert.match(body, /OpenCode Adversarial Review/); - assert.match(body, /Needs attention/); - assert.match(body, /openrouter\/anthropic\/claude-opus-4-6/); - assert.match(body, /Two issues worth addressing/); - assert.match(body, /### Findings \(2\)/); - assert.match(body, /\| # \| Severity \| Confidence \|/); - assert.match(body, /HIGH/); - assert.match(body, /MEDIUM/); - assert.match(body, /src\/foo\.js/); - assert.match(body, /10.12/); - assert.match(body, /1 finding at or above 80% confidence posted as inline comment/); - assert.match(body, /1 finding kept in the summary only/); - assert.match(body, /opencode-plugin-cc/); - }); - - it("shows an approve verdict label when the review approves", () => { - const body = renderSummaryBody({ - structured: { verdict: "approve", findings: [] }, - model: null, - adversarial: false, - inlineCount: 0, - summaryOnlyCount: 0, - confidenceThreshold: 0.8, - }); - assert.match(body, /OpenCode Review — Approve/); - assert.match(body, /No material findings/); - }); - - it("falls back to the rendered text when structured is null", () => { - const body = renderSummaryBody({ - structured: null, - rendered: "Raw review text the model produced.", - model: null, - adversarial: false, - inlineCount: 0, - summaryOnlyCount: 0, - confidenceThreshold: 0.8, - }); - assert.match(body, /did not return structured JSON/); - assert.match(body, /Raw review text the model produced/); - }); - - it("escapes pipe characters and newlines in table cells", () => { - const body = renderSummaryBody({ - structured: { - verdict: "needs-attention", - findings: [ - { - severity: "high", - confidence: 0.9, - title: "Has | pipe and\nnewline", - file: "src/foo.js", - line_start: 1, - line_end: 1, - body: "", - recommendation: "", - }, - ], - }, - model: null, - adversarial: false, - inlineCount: 0, - summaryOnlyCount: 1, - confidenceThreshold: 0.8, - }); - // Pipes must be escaped so they don't split a markdown table cell. - assert.match(body, /Has \\\| pipe and newline/); - }); -}); - -describe("shQuote", () => { - it("wraps simple paths in single quotes", () => { - assert.equal(shQuote("/tmp/foo.json"), "'/tmp/foo.json'"); - }); - - it("escapes embedded single quotes via the standard '\\'' trick", () => { - assert.equal(shQuote("/tmp/wat's this.json"), "'/tmp/wat'\\''s this.json'"); - }); - - it("treats non-string input as string", () => { - assert.equal(shQuote(42), "'42'"); - }); -}); - -describe("writePayloadFile", () => { - const written = []; - after(() => { - for (const p of written) { - try { - fs.unlinkSync(p); - } catch { - // best-effort cleanup - } - } - }); - - it("writes a JSON file with the exact payload and returns its path", () => { - const payload = { - commit_id: "deadbeef", - event: "COMMENT", - body: "hello", - comments: [{ path: "a.js", line: 1, side: "RIGHT", body: "x" }], - }; - const p = writePayloadFile(42, payload); - written.push(p); - assert.ok(fs.existsSync(p)); - assert.ok(p.includes("post-pr-42-")); - const roundtripped = JSON.parse(fs.readFileSync(p, "utf8")); - assert.deepEqual(roundtripped, payload); - }); - - it("produces a unique path per call", () => { - const a = writePayloadFile(1, {}); - const b = writePayloadFile(1, {}); - written.push(a, b); - assert.notEqual(a, b); - }); -}); - -describe("formatPostTrailer", () => { - it("renders an XML-ish block that Claude can parse with one regex", () => { - const trailer = formatPostTrailer({ - prepared: true, - prNumber: 412, - inlineCount: 3, - summaryOnlyCount: 2, - payloadPath: "/tmp/opencode-plugin-cc/post-pr-412-xyz.json", - command: `gh api -X POST "repos/{owner}/{repo}/pulls/412/reviews" --input '/tmp/opencode-plugin-cc/post-pr-412-xyz.json'`, - cleanup: `rm -f '/tmp/opencode-plugin-cc/post-pr-412-xyz.json'`, - }); - assert.match(trailer, /<opencode_post_instructions>/); - assert.match(trailer, /<\/opencode_post_instructions>/); - assert.match(trailer, /<pr>412<\/pr>/); - assert.match(trailer, /<inline_count>3<\/inline_count>/); - assert.match(trailer, /<summary_only_count>2<\/summary_only_count>/); - assert.match(trailer, /<payload_path>[^<]*post-pr-412-xyz\.json<\/payload_path>/); - assert.match(trailer, /<command>gh api -X POST[^<]*<\/command>/); - assert.match(trailer, /<cleanup>rm -f[^<]*<\/cleanup>/); - }); -}); - -describe("preparePostInstructions (with injected prData)", () => { - const written = []; - after(() => { - for (const p of written) { - try { - fs.unlinkSync(p); - } catch { - // best-effort cleanup - } - } - }); - - const structured = { - verdict: "needs-attention", - summary: "Two issues.", - findings: [ - { - severity: "high", - confidence: 0.92, - title: "Race condition", - file: "src/foo.js", - line_start: 11, - line_end: 11, - body: "Two writers can observe stale state.", - recommendation: "Hold the lock around the read-modify-write.", - }, - { - severity: "medium", - confidence: 0.55, - title: "Missing retries", - file: "src/bar.js", - line_start: 22, - line_end: 22, - body: "Downstream 5xx is not retried.", - recommendation: "Wrap in exponential backoff.", - }, - ], - }; - - const prData = { - headSha: "cafef00d", - files: [ - { - filename: "src/foo.js", - patch: "@@ -10,1 +10,2 @@\n a\n+b", - }, - ], - }; - - it("builds a payload file and command, and bypasses gh entirely", async () => { - const out = await preparePostInstructions("/nowhere", { - prNumber: 412, - structured, - model: { providerID: "openrouter", modelID: "anthropic/claude-opus-4-6" }, - adversarial: true, - confidenceThreshold: 0.8, - prData, - }); - assert.equal(out.prepared, true); - written.push(out.payloadPath); - - // The command must be a `gh api -X POST ...` with the payload path - // quoted. It must also contain the {owner}/{repo} placeholders so - // gh resolves them from the current repo at execution time. - assert.match(out.command, /^gh api -X POST "repos\/\{owner\}\/\{repo\}\/pulls\/412\/reviews" --input '/); - assert.match(out.command, /\.json'$/); - assert.match(out.cleanup, /^rm -f '/); - - // Classification: the high-confidence finding targets line 11 - // which is addressable (it's in the diff), so it becomes inline; - // the medium-confidence finding has no diff coverage and stays - // summary-only. - assert.equal(out.inlineCount, 1); - assert.equal(out.summaryOnlyCount, 1); - - const payload = JSON.parse(fs.readFileSync(out.payloadPath, "utf8")); - assert.equal(payload.commit_id, "cafef00d"); - assert.equal(payload.event, "COMMENT"); - assert.ok(payload.body.includes("OpenCode Adversarial Review")); - assert.ok(payload.body.includes("Needs attention")); - assert.equal(payload.comments.length, 1); - assert.equal(payload.comments[0].path, "src/foo.js"); - assert.equal(payload.comments[0].line, 11); - assert.equal(payload.comments[0].side, "RIGHT"); - assert.match(payload.comments[0].body, /Race condition/); - assert.match(payload.comments[0].body, /92%/); - }); - - it("returns { prepared: false, error } if prData fetch would fail", async () => { - // We don't inject prData, so the module would try to call `gh pr - // view` in `/definitely/not/a/repo` and fail. We only want to - // verify the failure path wraps the error without throwing. - const out = await preparePostInstructions("/definitely/not/a/repo", { - prNumber: 1, - structured, - adversarial: false, - confidenceThreshold: 0.8, - // NOTE: `prData` intentionally omitted - }); - // We can't assert the exact error message because it depends on - // whether `gh` is installed on the host, but we know `prepared` - // must be false. - assert.equal(out.prepared, false); - assert.ok(typeof out.error === "string" && out.error.length > 0); - }); - - it("handles structured=null by falling back to a rendered-text body", async () => { - const out = await preparePostInstructions("/nowhere", { - prNumber: 99, - structured: null, - rendered: "Raw review the model produced.", - adversarial: false, - confidenceThreshold: 0.8, - prData: { headSha: "cafef00d", files: [] }, - }); - assert.equal(out.prepared, true); - written.push(out.payloadPath); - const payload = JSON.parse(fs.readFileSync(out.payloadPath, "utf8")); - assert.ok(payload.body.includes("did not return structured JSON")); - assert.ok(payload.body.includes("Raw review the model produced")); - assert.deepEqual(payload.comments, []); - }); -}); diff --git a/tests/render.test.mjs b/tests/render.test.mjs index 3f66ac6..d2558e4 100644 --- a/tests/render.test.mjs +++ b/tests/render.test.mjs @@ -3,7 +3,6 @@ import assert from "node:assert/strict"; import { renderStatus, renderResult, - renderReview, renderSetup, extractResponseModel, formatModelHeader, @@ -26,35 +25,6 @@ describe("renderStatus", () => { }); }); -describe("renderReview", () => { - it("renders approve verdict", () => { - const output = renderReview({ verdict: "approve", summary: "Looks good", findings: [] }); - assert.ok(output.includes("PASS")); - assert.ok(output.includes("No findings")); - }); - - it("renders findings", () => { - const output = renderReview({ - verdict: "needs-attention", - summary: "Issues found", - findings: [{ - file: "src/api.ts", - line_start: 10, - line_end: 15, - severity: "high", - title: "SQL injection", - body: "User input not sanitized", - confidence: 0.9, - recommendation: "Use parameterized queries", - }], - }); - assert.ok(output.includes("NEEDS ATTENTION")); - assert.ok(output.includes("SQL injection")); - assert.ok(output.includes("src/api.ts")); - assert.ok(output.includes("90%")); - }); -}); - describe("renderSetup", () => { it("renders installed status", () => { const output = renderSetup({ diff --git a/tests/review-parser.test.mjs b/tests/review-parser.test.mjs deleted file mode 100644 index 662f147..0000000 --- a/tests/review-parser.test.mjs +++ /dev/null @@ -1,196 +0,0 @@ -// Tests for the schema-aware review parser. -// -// The regression test below is pinned to the exact failure shape a -// real user hit on v1.0.10: an OpenCode review response that is -// structurally almost-valid JSON except for a `summary` field whose -// content contains an unescaped `"` (it embeds literal `{"success": -// false, "error": "..."}`). Strict JSON.parse bails, the companion -// used to fall through to printing raw text in the chat AND in the -// posted PR comment body, and the review looked like garbage. -// -// The fix is a lenient fallback that slices fields by schema anchors -// instead of re-tokenising the whole JSON. These tests lock in both -// the fast path (strict valid JSON) and the repair path. - -import { describe, it } from "node:test"; -import assert from "node:assert/strict"; -import { - tryParseReview, - sliceMatchingBracket, -} from "../plugins/opencode/scripts/lib/review-parser.mjs"; - -describe("tryParseReview — fast path (strict JSON)", () => { - it("parses a well-formed review", () => { - const out = tryParseReview( - JSON.stringify({ - verdict: "needs-attention", - summary: "Two issues to address.", - findings: [ - { - severity: "high", - title: "Race", - file: "a.js", - line_start: 10, - line_end: 10, - confidence: 0.9, - body: "b", - recommendation: "r", - }, - ], - }) - ); - assert.ok(out); - assert.equal(out.verdict, "needs-attention"); - assert.equal(out.summary, "Two issues to address."); - assert.equal(out.findings.length, 1); - assert.equal(out.findings[0].title, "Race"); - }); - - it("parses JSON wrapped in a ```json fence", () => { - const out = tryParseReview( - [ - "Here is the review:", - "```json", - JSON.stringify({ verdict: "approve", summary: "ok", findings: [] }), - "```", - "", - ].join("\n") - ); - assert.ok(out); - assert.equal(out.verdict, "approve"); - assert.equal(out.summary, "ok"); - assert.deepEqual(out.findings, []); - }); - - it("normalizes a valid object missing findings to an empty array", () => { - const out = tryParseReview( - JSON.stringify({ verdict: "approve", summary: "ok" }) - ); - assert.ok(out); - assert.deepEqual(out.findings, []); - }); - - it("returns null for non-review JSON", () => { - const out = tryParseReview(JSON.stringify({ random: "thing" })); - assert.equal(out, null); - }); - - it("returns null for non-string input", () => { - assert.equal(tryParseReview(null), null); - assert.equal(tryParseReview(undefined), null); - assert.equal(tryParseReview({}), null); - }); - - it("returns null for empty text", () => { - assert.equal(tryParseReview(""), null); - assert.equal(tryParseReview(" \n "), null); - }); -}); - -describe("tryParseReview — lenient fallback (the regression)", () => { - it("recovers a review whose summary contains unescaped inner quotes", () => { - // This is almost verbatim what the user's OpenCode model produced - // on PR #412: a `summary` that contains `{"success": false, ...}` - // as literal text with unescaped `"` characters. Strict JSON.parse - // dies at the first `"success"` because it thinks the string ended - // there. - const broken = [ - "{", - ' "verdict": "approve",', - ' "summary": "Change correctly closes the silent-failure gap for', - " entity-ID field omission in execute_sequence. The 8 validated", - " actions (whisper, kill_pebble, harvest_bush, gather_resource,", - " destroy_structure, advance_construction, add_town_hall_xp,", - " grow_settlement_bedrock) all now return explicit", - ' {"success": false, "error": "..."} instead of dispatching with', - " ID 0 into downstream validators. The validation pattern is", - " consistent and the stopping behavior under stop_on_failure is", - ' verified. No material findings from this review.",', - ' "findings": []', - "}", - ].join("\n"); - - // Sanity check: the strict path must actually fail on this input. - // If upstream Node ever made JSON.parse lenient, this whole test - // category would be a no-op. - assert.throws(() => JSON.parse(broken)); - - const out = tryParseReview(broken); - assert.ok(out, "lenient fallback must recover a review object"); - assert.equal(out.verdict, "approve"); - assert.match(out.summary, /silent-failure gap/); - assert.match(out.summary, /\{"success": false/); - assert.match(out.summary, /no material findings from this review/i); - assert.deepEqual(out.findings, []); - }); - - it("recovers verdict + summary even when findings is malformed", () => { - const broken = [ - "{", - ' "verdict": "needs-attention",', - ' "summary": "Problem",', - ' "findings": [ {this is not valid json} ]', - "}", - ].join("\n"); - - assert.throws(() => JSON.parse(broken)); - const out = tryParseReview(broken); - assert.ok(out); - assert.equal(out.verdict, "needs-attention"); - assert.equal(out.summary, "Problem"); - // Findings failed to parse — better to show an empty array than - // abandon the whole review. - assert.deepEqual(out.findings, []); - }); - - it("slices summary up to the last '\"' before '}' when findings is absent", () => { - const broken = [ - "{", - ' "verdict": "approve",', - ' "summary": "Summary with {"nested": "quotes"} and that is fine."', - "}", - ].join("\n"); - assert.throws(() => JSON.parse(broken)); - const out = tryParseReview(broken); - assert.ok(out); - assert.equal(out.verdict, "approve"); - assert.match(out.summary, /Summary with/); - assert.match(out.summary, /\{"nested": "quotes"\}/); - }); - - it("returns null when verdict cannot be located", () => { - const broken = '{ "totally": "unrelated" }'; - assert.equal(tryParseReview(broken), null); - }); -}); - -describe("sliceMatchingBracket", () => { - it("returns the full array including outer brackets", () => { - const text = 'noise [1, 2, 3] more noise'; - const openIdx = text.indexOf("["); - assert.equal(sliceMatchingBracket(text, openIdx), "[1, 2, 3]"); - }); - - it("respects nested brackets", () => { - const text = "[[1, 2], [3, 4]]"; - assert.equal(sliceMatchingBracket(text, 0), "[[1, 2], [3, 4]]"); - }); - - it("respects brackets inside string literals", () => { - const text = '["a[b]c", "d]"]'; - assert.equal(sliceMatchingBracket(text, 0), '["a[b]c", "d]"]'); - }); - - it("handles escaped quotes inside strings", () => { - const text = '["a\\"b", "c"]'; - assert.equal(sliceMatchingBracket(text, 0), '["a\\"b", "c"]'); - }); - - it("returns null when the opening bracket has no match", () => { - assert.equal(sliceMatchingBracket("[1, 2, 3", 0), null); - }); - - it("returns null when given a non-bracket start char", () => { - assert.equal(sliceMatchingBracket("abc", 0), null); - }); -});