diff --git a/plugins/opencode/prompts/review.md b/plugins/opencode/prompts/review.md new file mode 100644 index 0000000..42e154b --- /dev/null +++ b/plugins/opencode/prompts/review.md @@ -0,0 +1,79 @@ + +You are OpenCode performing a balanced, high-signal software review. +Your job is to find real bugs, correctness issues, and regressions in the change. + + + +Review the provided repository context for bugs, correctness issues, regressions, and quality problems. +Target: {{TARGET_LABEL}} +User focus: {{USER_FOCUS}} + + + +Be honest, calibrated, and thorough. +Flag what matters. Acknowledge what looks correct. +Do not manufacture skepticism or invent problems where none exist. +If the change is solid, say so clearly. + + + +Prioritize findings that affect correctness, reliability, or security: +- logic errors, off-by-one mistakes, and incorrect control flow +- broken contracts, type mismatches, and violated invariants +- resource leaks, missing cleanup, and unhandled error paths +- race conditions, ordering assumptions, and stale-state bugs +- data loss, corruption, duplication, or irreversible state changes +- regressions that break existing behavior or compatibility +- missing tests or verification for critical paths +- unclear or misleading code that hides a real defect + + + +Read each changed file carefully. +Trace the data flow and control flow through the change. +Check edge cases: empty input, null values, timeouts, failures in dependencies. +If the user supplied a focus area, weight it, but still review the full change. +{{REVIEW_COLLECTION_GUIDANCE}} + + + +Report findings with severity and concrete evidence. +Each finding must include: +1. What is the defect? +2. Where is it (file and line range)? +3. What is the impact? +4. How can it be fixed? +Skip style nits, naming preferences, and opinion-only feedback. +Do not report issues you cannot support from the code. + + + +Return only valid JSON matching the provided schema. +Keep the output compact and specific. +Use `needs-attention` for any material finding. +Use `approve` only if the change looks correct and safe. +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 as a terse, actionable assessment. + + + +Report the strongest findings first. +Group related issues rather than splitting them. +If the change is clean, say so directly and return no findings. + + + +Before finalizing, check that each finding is: +- a real defect, not a style preference +- tied to a concrete code location +- supported by evidence in the provided context +- actionable for an engineer fixing the issue + + + +{{REVIEW_INPUT}} + diff --git a/plugins/opencode/scripts/lib/opencode.mjs b/plugins/opencode/scripts/lib/opencode.mjs index d6b898f..d2654f1 100644 --- a/plugins/opencode/scripts/lib/opencode.mjs +++ b/plugins/opencode/scripts/lib/opencode.mjs @@ -1220,6 +1220,29 @@ export function getSessionRuntimeStatus(env = process.env, cwd = process.cwd()) }; } +function extractConnectedProviderIds(provider) { + if (!provider || typeof provider !== "object") { + return []; + } + // The 1.17.15 /provider response carries connected provider ids in + // `connected`; tolerate id-object entries but no invented field aliases. + const connected = provider.connected ?? []; + if (!Array.isArray(connected)) { + return []; + } + return connected + .map((entry) => { + if (typeof entry === "string") { + return entry; + } + if (entry && typeof entry === "object" && typeof entry.id === "string") { + return entry.id; + } + return null; + }) + .filter(Boolean); +} + export async function getAuthStatus(cwd) { const availability = getAvailability(cwd); if (!availability.available) { @@ -1239,6 +1262,20 @@ export async function getAuthStatus(cwd) { if (config?.error && provider?.error) { throw config.error; } + + const connectedProviders = extractConnectedProviderIds(provider); + const isConnected = connectedProviders.length > 0; + + if (!isConnected) { + return buildAuthStatus({ + loggedIn: false, + detail: "No OpenCode provider is connected. Configure and connect an OpenCode provider, then rerun /opencode:setup.", + source: "server", + available: true, + provider: null + }); + } + const providerID = provider?.id ?? provider?.providerID ?? diff --git a/plugins/opencode/scripts/opencode-companion.mjs b/plugins/opencode/scripts/opencode-companion.mjs index fb4be68..f3e766a 100644 --- a/plugins/opencode/scripts/opencode-companion.mjs +++ b/plugins/opencode/scripts/opencode-companion.mjs @@ -238,10 +238,10 @@ async function handleSetup(argv) { outputResult(options.json ? finalReport : renderSetupReport(finalReport), options.json); } -function buildAdversarialReviewPrompt(context, focusText) { - const template = loadPromptTemplate(ROOT_DIR, "adversarial-review"); +function buildReviewPrompt(context, focusText, { templateName, reviewKind }) { + const template = loadPromptTemplate(ROOT_DIR, templateName); return interpolateTemplate(template, { - REVIEW_KIND: "Adversarial Review", + REVIEW_KIND: reviewKind, TARGET_LABEL: context.target.label, USER_FOCUS: focusText || "No extra focus provided.", REVIEW_COLLECTION_GUIDANCE: context.collectionGuidance, @@ -339,7 +339,10 @@ async function executeReviewRun(request) { const reviewName = request.reviewName ?? "Review"; const context = collectReviewContext(request.cwd, target); - const prompt = buildAdversarialReviewPrompt(context, focusText); + const prompt = buildReviewPrompt(context, focusText, { + templateName: request.promptTemplate ?? "adversarial-review", + reviewKind: reviewName + }); const result = await runServerReview(context.repoRoot, { prompt, model: request.model, @@ -746,6 +749,7 @@ async function handleReviewCommand(argv, config) { model: options.model, focusText, reviewName: config.reviewName, + promptTemplate: config.promptTemplate, onProgress: progress }), { json: options.json } @@ -754,7 +758,15 @@ async function handleReviewCommand(argv, config) { async function handleReview(argv) { return handleReviewCommand(argv, { - reviewName: "Review" + reviewName: "Review", + promptTemplate: "review", + validateRequest: (target, focusText) => { + if (focusText) { + throw new Error( + "/opencode:review does not accept positional focus text. Use /opencode:adversarial-review for a steerable challenge review with custom focus." + ); + } + } }); } @@ -1167,7 +1179,8 @@ async function main() { break; case "adversarial-review": await handleReviewCommand(argv, { - reviewName: "Adversarial Review" + reviewName: "Adversarial Review", + promptTemplate: "adversarial-review" }); break; case "task": diff --git a/tests/fake-opencode-fixture.mjs b/tests/fake-opencode-fixture.mjs index 17ea743..5e1cd73 100644 --- a/tests/fake-opencode-fixture.mjs +++ b/tests/fake-opencode-fixture.mjs @@ -508,7 +508,15 @@ const server = http.createServer(async (req, res) => { } if (req.method === "GET" && url.pathname === "/provider") { - sendJson(res, { id: "openai", name: "OpenAI" }); + if (process.env.FAKE_OPENCODE_PROVIDER_FAIL === "1") { + sendJson(res, { error: "provider endpoint failed" }, 500); + return; + } + if (process.env.FAKE_OPENCODE_NO_PROVIDER === "1") { + sendJson(res, { all: [], default: {}, connected: [] }); + } else { + sendJson(res, { all: [{ id: "openai" }], default: {}, connected: ["openai"] }); + } return; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 770d4b3..e0b512b 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1385,3 +1385,126 @@ test("runServerTurn deletes only sessions created by a failed captureTurn", asyn resumedFetch.restore(); } }); + +test("adversarial-review prompt uses the adversarial-review.md template", { skip: LOCAL_LISTEN_SKIP }, () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeOpencode(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "before\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "after\n"); + const env = buildTestEnv(binDir); + + try { + const result = run("node", [SCRIPT, "adversarial-review", "--json"], { + cwd: repo, + env + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = readFakeState(binDir); + const prompt = fakeState.lastMessage.prompt; + assert.match(prompt, /adversarial software review/); + assert.match(prompt, /break confidence/); + assert.match(prompt, /Default to skepticism/); + } finally { + cleanupServer(repo, env); + } +}); + +test("review prompt uses the neutral review.md template", { skip: LOCAL_LISTEN_SKIP }, () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeOpencode(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "before\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "after\n"); + const env = buildTestEnv(binDir); + + try { + const result = run("node", [SCRIPT, "review", "--json"], { + cwd: repo, + env + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = readFakeState(binDir); + const prompt = fakeState.lastMessage.prompt; + assert.doesNotMatch(prompt, /break confidence/); + assert.doesNotMatch(prompt, /Default to skepticism/); + assert.match(prompt, /balanced, high-signal software review/); + assert.match(prompt, /find real bugs, correctness issues/); + } finally { + cleanupServer(repo, env); + } +}); + +test("review rejects positional focus text with a clear error", () => { + const repo = makeTempDir(); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "test\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "review", "check auth paths"], { + cwd: repo, + env: process.env + }); + + assert.notEqual(result.status, 0, result.stdout); + assert.match(result.stderr, /does not accept positional focus text/); + assert.match(result.stderr, /adversarial-review/); +}); + +test("setup reports not ready when no provider is connected", { skip: LOCAL_LISTEN_SKIP }, () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeOpencode(binDir); + const env = buildTestEnv(binDir, { + FAKE_OPENCODE_NO_PROVIDER: "1" + }); + + try { + const result = run("node", [SCRIPT, "setup", "--json"], { + cwd: repo, + env + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.ready, false); + assert.equal(payload.opencode.available, true); + assert.equal(payload.auth.loggedIn, false); + assert.match(payload.auth.detail, /No OpenCode provider is connected/); + assert.ok(payload.nextSteps.some((step) => /provider/i.test(step)), "nextSteps should include a provider config step"); + } finally { + cleanupServer(repo, env); + } +}); + +test("setup reports loggedIn false when /provider endpoint fails", { skip: LOCAL_LISTEN_SKIP }, () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeOpencode(binDir); + const env = buildTestEnv(binDir, { + FAKE_OPENCODE_PROVIDER_FAIL: "1" + }); + + try { + const result = run("node", [SCRIPT, "setup", "--json"], { + cwd: repo, + env + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.auth.loggedIn, false); + assert.equal(payload.ready, false); + } finally { + cleanupServer(repo, env); + } +});