From 8a6f2ba088ee0f2db0a0281d523ca20617723ac6 Mon Sep 17 00:00:00 2001 From: Peyton-Spencer Date: Sat, 3 Oct 2026 10:42:39 -0400 Subject: [PATCH] feat: review findings, dismiss, undismiss and feedback commands heyditto review findings lists a pull request's newest Ditto Review run; review dismiss|undismiss|feedback act on a finding named by its GitHub review-comment URL, :, or a finding id with --pr, through the same routes the console uses, so the action is recorded as learning data and the dismissal replies on and resolves the GitHub thread. DITTO-178. Co-Authored-By: Claude Fable 5.1 --- README.md | 35 +++ src/review-commands.ts | 2 + src/review-findings.ts | 426 ++++++++++++++++++++++++++++++++++ test/review-commands.test.mjs | 132 +++++++++++ 4 files changed, 595 insertions(+) create mode 100644 src/review-findings.ts diff --git a/README.md b/README.md index 55b9e07..33b3117 100644 --- a/README.md +++ b/README.md @@ -686,6 +686,41 @@ List your Ditto agents (`GET /api/v5/chat-agents`): id, kind (`main`, `chat`, `inference_endpoint`, `mcp`, `connector`), name, thread count, last activity and the live connections (API keys, OAuth grants, endpoints) writing into each. +### `review` — Ditto Review from the shell + +The developer console's Review pages, as commands. Repository settings: + +```bash +heyditto review repos --org omniaura # repositories set up for Ditto Review +heyditto review set ditto-assistant/console --max-minutes 20 --org omniaura +``` + +Findings on a pull request (its newest run), and acting on one. A finding is +named the way you meet it: by the GitHub review comment's URL +(`…/pull/12#discussion_r`), by `:`, or by a bare +finding id with `--pr`. Without `--org` the PR's repository is looked up in +your personal workspace and then in each organization you belong to. + +```bash +heyditto review findings https://github.com/ditto-assistant/backend/pull/3158 +heyditto review findings ditto-assistant/ditto-app#3163 --all --output json # include dismissed + +# Managers: dismiss with a reason. Ditto replies on the GitHub thread, resolves +# it, and keeps the same issue off this PR on later reviews. +heyditto review dismiss "https://github.com/ditto-assistant/backend/pull/3112#discussion_r4171976544" \ + --reason "Retrying with different arguments is the documented design (trace_test.go:106-115)." +heyditto review undismiss : + +# Any member: say whether a finding was useful, wrong or not useful. +heyditto review feedback "" wrong --note "The retry test covers different arguments." +``` + +Every action is recorded as the same signal the console records +(`review_finding_feedback`, `review_finding_dismissals`), so it feeds +false-positive learning; a plain GitHub reply does not. The key reaches the run +detail, feedback and dismiss routes only — not cancel, retry, the event stream +or posting a withheld finding. + ## Remote Control Every `heyditto claude` / `heyditto codex` session is reachable from the Ditto diff --git a/src/review-commands.ts b/src/review-commands.ts index edf7ee6..8e37c50 100644 --- a/src/review-commands.ts +++ b/src/review-commands.ts @@ -1,6 +1,7 @@ import { setTimeout as delay } from "node:timers/promises"; import type { Command, Option } from "commander"; import { apiFetch, type Company, resolveCompany } from "./api.js"; +import { registerReviewFindingCommands } from "./review-findings.js"; import { readStoredAuth } from "./store.js"; /** @@ -409,4 +410,5 @@ export function registerReviewCommands( ` heyditto review set ditto-assistant/console --max-minutes 20 --org omni-aura heyditto review set acme/api --budget-cents 300 --enabled on`, ); + registerReviewFindingCommands(review, addExamples, outputOption, orgOption); } diff --git a/src/review-findings.ts b/src/review-findings.ts new file mode 100644 index 0000000..03bc0c0 --- /dev/null +++ b/src/review-findings.ts @@ -0,0 +1,426 @@ +import type { Command, Option } from "commander"; +import { ApiError, apiFetch, type Company, listCompanies, resolveCompany } from "./api.js"; +import { readStoredAuth } from "./store.js"; + +/** + * Ditto Review findings from the shell (DITTO-178): list a pull request's + * findings, dismiss or undismiss one, and give feedback on one. Every action + * goes through the same backend routes the developer console uses, so it is + * recorded as the same learning signal (review_finding_feedback, + * review_finding_dismissals) and a dismissal replies on the GitHub thread and + * resolves it exactly as a console dismissal does. + * + * A finding is addressed the way a person meets it: by the GitHub + * review-comment URL (`…/pull/12#discussion_r4167148147`), by `:`, + * or by a bare finding id together with `--pr`. + */ + +/** One finding as `GET /api/v5/review/runs/{id}` returns it (the fields the CLI shows). */ +export interface ReviewFinding { + id: string; + path: string; + line: number; + startLine?: number; + severity: string; + category?: string; + title?: string; + status?: string; + withheldReason?: string; + confidence?: number; + commentUrl?: string; + dismissed?: boolean; + dismissReason?: string; + feedback?: { mine?: string | null }; + canFeedback?: boolean; + canDismiss?: boolean; +} + +export interface ReviewRunDetail { + run: { id: string; prNumber: number; headSha: string; status: string; reviewUrl?: string; prTitle?: string; createdAt?: string }; + result: { findings: ReviewFinding[] }; + repository: { id: string; fullName: string }; +} + +export interface ReviewDismissResponse { + runId: string; + findingId: string; + githubStatus: string; + githubReason?: string; + replyUrl?: string; + alreadyDismissed: boolean; + githubUpdated: boolean; + threadResolved: boolean; + markerConflict?: { message: string; findingIds: string[] }; +} + +export interface ReviewUndismissResponse { + runId: string; + findingId: string; + wasDismissed: boolean; + dismissed: boolean; + githubNote: string; + githubReplyUrl?: string; +} + +export interface ReviewFeedbackResponse { + runId: string; + findingId: string; + attempt: number; + feedback?: { mine?: string | null; counts?: Record }; +} + +export const FEEDBACK_VERDICTS = ["useful", "wrong", "not_useful"] as const; +export type FeedbackVerdict = (typeof FEEDBACK_VERDICTS)[number]; + +interface ScopeOptions { + org?: string; + output?: string; +} + +interface FindingsOptions extends ScopeOptions { + all?: boolean; +} + +interface FindingOptions extends ScopeOptions { + pr?: string; +} + +interface DismissOptions extends FindingOptions { + reason?: string; +} + +interface FeedbackOptions extends FindingOptions { + note?: string; +} + +/** A pull request named by URL or `owner/name#N`. */ +export interface PullRef { + fullName: string; + number: number; + /** The GitHub review-comment id from a `#discussion_r` fragment. */ + commentId?: string; +} + +/** What a `` argument named. */ +export type FindingRef = + | { kind: "comment"; pull: PullRef; commentId: string } + | { kind: "direct"; runId: string; findingId: string } + | { kind: "id"; findingId: string }; + +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** Parses a pull request reference: a GitHub PR URL (with or without a review-comment fragment) or `owner/name#N`. */ +export function parsePullRef(text: string): PullRef | undefined { + const value = text.trim(); + const url = value.match(/^https?:\/\/github\.com\/([^/\s]+)\/([^/\s#?]+)\/pull\/(\d+)(?:\/[^#?\s]*)?(?:\?[^#\s]*)?(?:#(.*))?$/i); + if (url) { + const fragment = url[4] ?? ""; + const comment = fragment.match(/^discussion_r(\d+)$/) ?? fragment.match(/^r(\d+)$/); + return { fullName: `${url[1]}/${url[2]}`, number: Number(url[3]), commentId: comment?.[1] }; + } + const short = value.match(/^([\w.-]+\/[\w.-]+)#(\d+)$/); + if (short) return { fullName: short[1]!, number: Number(short[2]) }; + return undefined; +} + +/** Parses a `` argument. */ +export function parseFindingRef(text: string): FindingRef { + const value = text.trim(); + const pull = parsePullRef(value); + if (pull) { + if (!pull.commentId) { + throw new Error( + `${value} names a pull request, not a finding. Pass the review comment's URL (…#discussion_r), or a finding id with --pr ${pull.fullName}#${pull.number}.`, + ); + } + return { kind: "comment", pull, commentId: pull.commentId }; + } + const direct = value.match(/^([0-9a-f-]{36})[:/](\S+)$/i); + if (direct && UUID.test(direct[1]!)) return { kind: "direct", runId: direct[1]!.toLowerCase(), findingId: direct[2]! }; + if (!value || /\s/.test(value)) throw new Error(`"${value}" is not a finding: pass a review comment URL, :, or a finding id with --pr`); + return { kind: "id", findingId: value }; +} + +function isJSON(options: { output?: string }): boolean { + return options.output === "json" || options.output === "raw"; +} + +function scopeQuery(company: Company | undefined): string { + return company ? `?company=${encodeURIComponent(company.id)}` : ""; +} + +function scopeSuffix(company: Company | undefined): string { + return company ? `&company=${encodeURIComponent(company.id)}` : ""; +} + +/** The workspace named by --org or the stored default; undefined means "not chosen" (not "personal"). */ +async function chosenScope(options: ScopeOptions): Promise { + const wanted = options.org?.trim() || (await readStoredAuth())?.defaultCompany; + if (!wanted) return undefined; + return resolveCompany(wanted); +} + +/** The run detail of a PR's newest run in one workspace; undefined on 404. */ +async function latestRunIn(company: Company | undefined, pull: PullRef): Promise { + try { + return await apiFetch( + `/api/v5/review/runs/latest?repository=${encodeURIComponent(pull.fullName)}&pr=${pull.number}${scopeSuffix(company)}`, + ); + } catch (error) { + if (error instanceof ApiError && error.status === 404) return undefined; + throw error; + } +} + +/** + * The PR's newest run and the workspace it was found in. With --org (or a + * stored default) only that workspace is asked; otherwise the personal + * workspace and then every organization the key can see, because a GitHub + * URL already names the repository and nobody wants to look up which + * organization reviews it. + */ +export async function findLatestRun(options: ScopeOptions, pull: PullRef): Promise<{ company: Company | undefined; detail: ReviewRunDetail }> { + const chosen = await chosenScope(options); + if (chosen || options.org) { + const detail = await latestRunIn(chosen, pull); + if (!detail) throw new Error(`Ditto Review has no run for ${pull.fullName}#${pull.number} in ${chosen?.slug ?? "your personal workspace"}.`); + return { company: chosen, detail }; + } + const tried: string[] = []; + const personal = await latestRunIn(undefined, pull); + if (personal) return { company: undefined, detail: personal }; + tried.push("your personal workspace"); + for (const company of await listCompanies()) { + const detail = await latestRunIn(company, pull); + if (detail) return { company, detail }; + tried.push(company.slug); + } + throw new Error( + `Ditto Review has no run for ${pull.fullName}#${pull.number} in ${tried.join(", ")}. Is the repository set up for Ditto Review (heyditto review repos --org )?`, + ); +} + +/** The run and finding a `` argument names, with the workspace to act in. */ +export async function resolveFinding( + text: string, + options: FindingOptions, +): Promise<{ company: Company | undefined; runId: string; finding: ReviewFinding; detail?: ReviewRunDetail }> { + const ref = parseFindingRef(text); + if (ref.kind === "direct") { + const company = await chosenScope(options); + const detail = await apiFetch(`/api/v5/review/runs/${encodeURIComponent(ref.runId)}${scopeQuery(company)}`); + const finding = detail.result.findings.find((f) => f.id === ref.findingId); + if (!finding) throw new Error(`run ${ref.runId} has no finding ${ref.findingId}. Findings: ${detail.result.findings.map((f) => f.id).join(", ") || "none"}`); + return { company, runId: detail.run.id, finding, detail }; + } + let pull: PullRef | undefined; + if (ref.kind === "comment") pull = ref.pull; + else { + if (!options.pr) throw new Error(`a bare finding id needs --pr , or pass the review comment's URL instead`); + pull = parsePullRef(options.pr); + if (!pull) throw new Error(`--pr ${options.pr} is not a pull request URL or owner/name#N`); + } + const { company, detail } = await findLatestRun(options, pull); + const finding = + ref.kind === "comment" + ? detail.result.findings.find((f) => f.commentUrl?.endsWith(`discussion_r${ref.commentId}`)) + : detail.result.findings.find((f) => f.id === ref.findingId); + if (!finding) { + const what = ref.kind === "comment" ? `the review comment discussion_r${ref.commentId}` : `finding ${ref.findingId}`; + throw new Error( + `${what} is not in the newest Ditto Review run of ${pull.fullName}#${pull.number} (run ${detail.run.id}, head ${detail.run.headSha.slice(0, 8)}). ` + + `List it with: heyditto review findings ${pull.fullName}#${pull.number}`, + ); + } + return { company, runId: detail.run.id, finding, detail }; +} + +function short(value: string | undefined, width: number): string { + if (!value) return ""; + return value.length > width ? `${value.slice(0, width - 1)}…` : value; +} + +function findingState(f: ReviewFinding): string { + if (f.dismissed) return "dismissed"; + if (f.status === "withheld") return f.withheldReason ? `held (${f.withheldReason})` : "held"; + return f.status || ""; +} + +function table(header: string[], rows: string[][]): void { + const widths = header.map((h, i) => Math.max(h.length, ...rows.map((row) => row[i]!.length))); + for (const row of [header, ...rows]) { + process.stdout.write(`${row.map((c, i) => (i === row.length - 1 ? c : c.padEnd(widths[i]!))).join(" ")}\n`); + } +} + +export async function cmdReviewFindings(target: string, options: FindingsOptions): Promise { + const value = target.trim(); + let detail: ReviewRunDetail; + let company: Company | undefined; + if (UUID.test(value)) { + company = await chosenScope(options); + detail = await apiFetch(`/api/v5/review/runs/${encodeURIComponent(value.toLowerCase())}${scopeQuery(company)}`); + } else { + const pull = parsePullRef(value); + if (!pull) throw new Error(`${value} is not a pull request URL, owner/name#N, or a run id`); + ({ company, detail } = await findLatestRun(options, pull)); + } + const findings = options.all ? detail.result.findings : detail.result.findings.filter((f) => !f.dismissed); + if (isJSON(options)) { + process.stdout.write(`${JSON.stringify({ company: company?.slug, run: detail.run, repository: detail.repository, findings }, null, 2)}\n`); + return; + } + const run = detail.run; + process.stdout.write( + `${detail.repository.fullName}#${run.prNumber}${run.prTitle ? ` ${run.prTitle}` : ""}\nrun ${run.id} · head ${run.headSha.slice(0, 8)} · ${run.status}${run.reviewUrl ? ` · ${run.reviewUrl}` : ""}\n\n`, + ); + if (findings.length === 0) { + process.stdout.write(options.all ? "No findings.\n" : "No open findings (pass --all to include dismissed ones).\n"); + return; + } + table( + ["FINDING", "SEV", "CONF", "STATE", "MINE", "WHERE", "TITLE"], + findings.map((f) => [ + f.id, + f.severity, + f.confidence ? `${f.confidence}%` : "", + findingState(f), + f.feedback?.mine ?? "", + `${f.path}:${f.line}`, + short(f.title, 70), + ]), + ); + const withComments = findings.filter((f) => f.commentUrl); + if (withComments.length > 0) { + process.stdout.write("\nOn GitHub:\n"); + for (const f of withComments) process.stdout.write(` ${f.id} ${f.commentUrl}\n`); + } + process.stdout.write( + `\nAct on one: heyditto review dismiss > --reason "…" · heyditto review feedback useful|wrong|not_useful\n`, + ); +} + +export async function cmdReviewDismiss(target: string, options: DismissOptions): Promise { + const reason = options.reason?.trim() ?? ""; + if (!reason) throw new Error('--reason is required: say why this finding does not apply (it is posted on the GitHub thread and kept as a team decision)'); + if (reason.length > 1000) throw new Error("--reason must be at most 1000 characters"); + const { company, runId, finding } = await resolveFinding(target, options); + const out = await apiFetch( + `/api/v5/review/runs/${encodeURIComponent(runId)}/findings/${encodeURIComponent(finding.id)}/dismiss${scopeQuery(company)}`, + { method: "POST", body: { reason } }, + ); + if (isJSON(options)) { + process.stdout.write(`${JSON.stringify(out, null, 2)}\n`); + return; + } + const lines = [`${out.alreadyDismissed ? "Already dismissed" : "Dismissed"} ${finding.id} (${finding.severity}, ${finding.path}:${finding.line})${finding.title ? `: ${finding.title}` : ""}`]; + switch (out.githubStatus) { + case "updated": + lines.push(` GitHub: replied and resolved the thread${out.replyUrl ? ` — ${out.replyUrl}` : ""}`); + break; + case "no_thread": + lines.push(" GitHub: nothing to update (the finding has no review thread)"); + break; + case "pending": + lines.push(" GitHub: reply and resolve still running; check the thread in a minute"); + break; + case "marker_conflict": + lines.push(" GitHub: thread left open because another, different issue shares it"); + break; + default: + lines.push(` GitHub: ${out.githubStatus}${out.githubReason ? ` — ${out.githubReason}` : ""} (run the command again to retry)`); + } + if (out.markerConflict) lines.push(` Note: ${out.markerConflict.message}`); + lines.push(` Undo: heyditto review undismiss ${runId}:${finding.id}${company ? ` --org ${company.slug}` : ""}`); + process.stdout.write(`${lines.join("\n")}\n`); +} + +export async function cmdReviewUndismiss(target: string, options: FindingOptions): Promise { + const { company, runId, finding } = await resolveFinding(target, options); + const out = await apiFetch( + `/api/v5/review/runs/${encodeURIComponent(runId)}/findings/${encodeURIComponent(finding.id)}/dismiss${scopeQuery(company)}`, + { method: "DELETE" }, + ); + if (isJSON(options)) { + process.stdout.write(`${JSON.stringify(out, null, 2)}\n`); + return; + } + process.stdout.write( + `${out.wasDismissed ? "Undismissed" : "Not dismissed"} ${finding.id} (${finding.path}:${finding.line}).\n ${out.githubNote}\n`, + ); +} + +export async function cmdReviewFeedback(target: string, verdict: string, options: FeedbackOptions): Promise { + const v = verdict.trim().toLowerCase().replace("-", "_") as FeedbackVerdict; + if (!FEEDBACK_VERDICTS.includes(v)) throw new Error(`verdict must be one of ${FEEDBACK_VERDICTS.join(", ")}`); + const note = options.note?.trim() ?? ""; + if (note.length > 1000) throw new Error("--note must be at most 1000 characters"); + const { company, runId, finding } = await resolveFinding(target, options); + const out = await apiFetch( + `/api/v5/review/runs/${encodeURIComponent(runId)}/findings/${encodeURIComponent(finding.id)}/feedback${scopeQuery(company)}`, + { method: "POST", body: { verdict: v, note } }, + ); + if (isJSON(options)) { + process.stdout.write(`${JSON.stringify(out, null, 2)}\n`); + return; + } + process.stdout.write(`Recorded "${v}" on ${finding.id} (${finding.path}:${finding.line})${finding.title ? `: ${finding.title}` : ""}${note ? `\n note: ${note}` : ""}\n`); +} + +export function registerReviewFindingCommands( + review: Command, + addExamples: (c: Command, ex: string) => Command, + outputOption: () => Option, + orgOption: () => Option, +): void { + addExamples( + review + .command("findings") + .description("list a pull request's Ditto Review findings (its newest run), or a run's by id") + .argument("", "GitHub PR URL, owner/name#N, or a run id") + .option("--all", "include dismissed findings") + .addOption(orgOption()) + .addOption(outputOption()) + .action(cmdReviewFindings), + ` heyditto review findings https://github.com/ditto-assistant/backend/pull/3158 + heyditto review findings ditto-assistant/ditto-app#3163 --all --output json`, + ); + addExamples( + review + .command("dismiss") + .description("dismiss a finding with a reason (managers): replies on its GitHub thread, resolves it, and keeps the same issue off this PR") + .argument("", "review comment URL (…#discussion_r), :, or a finding id with --pr") + .requiredOption("--reason ", "why it does not apply; posted on GitHub and kept as a team decision") + .option("--pr ", "the PR a bare finding id belongs to (URL or owner/name#N)") + .addOption(orgOption()) + .addOption(outputOption()) + .action(cmdReviewDismiss), + ` heyditto review dismiss "https://github.com/ditto-assistant/backend/pull/3112#discussion_r4171976544" --reason "Retrying with different arguments is the documented design (trace_test.go:106-115)." + heyditto review dismiss f_6eb378e97faaaf48 --pr ditto-assistant/backend#3112 --reason "Theoretical: tool arguments never carry integers above 2^53."`, + ); + addExamples( + review + .command("undismiss") + .description("undo a dismissal (managers); the GitHub thread stays as it is") + .argument("", "review comment URL, :, or a finding id with --pr") + .option("--pr ", "the PR a bare finding id belongs to (URL or owner/name#N)") + .addOption(orgOption()) + .addOption(outputOption()) + .action(cmdReviewUndismiss), + ` heyditto review undismiss "https://github.com/ditto-assistant/backend/pull/3112#discussion_r4171976544"`, + ); + addExamples( + review + .command("feedback") + .description("say whether a finding was useful, wrong or not useful (any workspace member)") + .argument("", "review comment URL, :, or a finding id with --pr") + .argument("", FEEDBACK_VERDICTS.join("|")) + .option("--note ", "what was wrong or missing (up to 1000 characters)") + .option("--pr ", "the PR a bare finding id belongs to (URL or owner/name#N)") + .addOption(orgOption()) + .addOption(outputOption()) + .action(cmdReviewFeedback), + ` heyditto review feedback "https://github.com/ditto-assistant/ditto-app/pull/3161#discussion_r4167262944" useful + heyditto review feedback f_0a073e36dfb26698 --pr ditto-assistant/backend#3112 wrong --note "The retry test covers different arguments."`, + ); +} diff --git a/test/review-commands.test.mjs b/test/review-commands.test.mjs index 3d06ca4..11d5a97 100644 --- a/test/review-commands.test.mjs +++ b/test/review-commands.test.mjs @@ -22,6 +22,17 @@ const RUN = { id: "55555555-5555-5555-5555-555555555555", repositoryId: REPO.id, status: "completed", headSha: "a".repeat(40), attempts: 0, summary: { posted: 1 }, reviewUrl: "https://github.com/ditto-assistant/console/pull/88#pullrequestreview-1" }; + +const FRUN = { id: "66666666-6666-6666-6666-666666666666", prNumber: 12, headSha: "abcdef1234567890", status: "completed", reviewUrl: "https://github.com/ditto-assistant/console/pull/12#pullrequestreview-1", prTitle: "feat: thing" }; +const FINDING_OPEN = { + id: "f_6eb378e97faaaf48", path: "pkg/a.go", line: 358, severity: "medium", category: "bug", title: "Preserve precision when canonicalizing numeric arguments", + status: "posted", confidence: 96, commentUrl: "https://github.com/ditto-assistant/console/pull/12#discussion_r4171976534", feedback: { mine: null }, canFeedback: true, canDismiss: true, +}; +const FINDING_DISMISSED = { id: "f_0a073e36dfb26698", path: "pkg/b.go", line: 7, severity: "low", title: "Old one", status: "withheld", withheldReason: "dismissed", dismissed: true, dismissReason: "no", feedback: { mine: "wrong" } }; +const DETAIL = { run: FRUN, result: { findings: [FINDING_OPEN, FINDING_DISMISSED] }, repository: { id: REPO.id, fullName: REPO.fullName } }; +const LATEST = `/api/v5/review/runs/latest?repository=${encodeURIComponent(REPO.fullName)}&pr=12`; +const FINDING_BASE = `/api/v5/review/runs/${FRUN.id}/findings/${FINDING_OPEN.id}`; + function startStub({ statuses = ["completed"], refusal, detailDelay = 0 } = {}) { let detailReads = 0; const calls = []; @@ -51,6 +62,19 @@ function startStub({ statuses = ["completed"], refusal, detailDelay = 0 } = {}) } if (req.url === runRoute + "/retry" + query && req.method === "POST") return json(202, { status: "queued", reusedResult: true }); if (req.url === runRoute + "/cancel" + query && req.method === "POST") return json(200, { runId: RUN.id, status: "running", cancelRequested: true }); + // The PR's newest run: 404 in the personal workspace, found in the organization. + if (req.url === LATEST && req.method === "GET") return json(404, { message: "not found" }); + if (req.url === `${LATEST}&company=${COMPANY.id}` && req.method === "GET") return json(200, DETAIL); + if (req.url === `/api/v5/review/runs/${FRUN.id}?company=${COMPANY.id}` && req.method === "GET") return json(200, DETAIL); + if (req.url === `${FINDING_BASE}/dismiss?company=${COMPANY.id}` && req.method === "POST") { + return json(200, { runId: FRUN.id, findingId: FINDING_OPEN.id, githubStatus: "updated", githubUpdated: true, threadResolved: true, alreadyDismissed: false, replyUrl: `${FINDING_OPEN.commentUrl}0`, dismissal: { id: "d1", reason: JSON.parse(body).reason } }); + } + if (req.url === `${FINDING_BASE}/dismiss?company=${COMPANY.id}` && req.method === "DELETE") { + return json(200, { runId: FRUN.id, findingId: FINDING_OPEN.id, wasDismissed: true, dismissed: false, githubNote: "Ditto does not reopen the review thread on GitHub." }); + } + if (req.url === `${FINDING_BASE}/feedback?company=${COMPANY.id}` && req.method === "POST") { + return json(200, { runId: FRUN.id, findingId: FINDING_OPEN.id, attempt: 1, feedback: { mine: JSON.parse(body).verdict, counts: {} } }); + } json(404, { message: "no route" }); }); }); @@ -235,3 +259,111 @@ test("review actions keep server semantics; start propagates manager refusal wit assert.equal(stub.calls.filter(c => c.method === "POST").length, before + 1); } finally { stub.close(); } }); + +test("review findings finds the PR's newest run across workspaces and lists open findings with their GitHub comments", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "findings", "https://github.com/ditto-assistant/console/pull/12"]); + assert.equal(out.status, 0, out.stderr); + // Personal workspace first (404), then the organization. + assert.deepEqual(stub.calls.filter((c) => c.url.startsWith("/api/v5/review/runs/latest")).map((c) => c.url), [LATEST, `${LATEST}&company=${COMPANY.id}`]); + assert.match(out.stdout, /ditto-assistant\/console#12 feat: thing/); + assert.match(out.stdout, /f_6eb378e97faaaf48\s+medium\s+96%\s+posted\s+pkg\/a\.go:358\s+Preserve precision/); + assert.doesNotMatch(out.stdout, /f_0a073e36dfb26698/, "dismissed findings are hidden without --all"); + assert.match(out.stdout, /f_6eb378e97faaaf48\s+https:\/\/github\.com\/ditto-assistant\/console\/pull\/12#discussion_r4171976534/); + } finally { + stub.close(); + } +}); + +test("review findings --all --output json includes dismissed findings and the workspace", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "findings", "ditto-assistant/console#12", "--all", "--org", "omni-aura", "--output", "json"]); + assert.equal(out.status, 0, out.stderr); + const parsed = JSON.parse(out.stdout); + assert.equal(parsed.company, "omni-aura"); + assert.equal(parsed.run.id, FRUN.id); + assert.deepEqual(parsed.findings.map((f) => f.id), [FINDING_OPEN.id, FINDING_DISMISSED.id]); + // --org asks only that workspace. + assert.deepEqual(stub.calls.map((c) => c.url), ["/api/v5/companies", `${LATEST}&company=${COMPANY.id}`]); + } finally { + stub.close(); + } +}); + +test("review dismiss by review-comment URL posts the reason and reports the GitHub outcome", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "dismiss", FINDING_OPEN.commentUrl, "--reason", "Theoretical: tool arguments never carry integers above 2^53."]); + assert.equal(out.status, 0, out.stderr); + const post = stub.calls.find((c) => c.method === "POST"); + assert.equal(post.url, `${FINDING_BASE}/dismiss?company=${COMPANY.id}`); + assert.deepEqual(post.body, { reason: "Theoretical: tool arguments never carry integers above 2^53." }); + assert.match(out.stdout, /Dismissed f_6eb378e97faaaf48 \(medium, pkg\/a\.go:358\): Preserve precision/); + assert.match(out.stdout, /GitHub: replied and resolved the thread/); + assert.match(out.stdout, new RegExp(`Undo: heyditto review undismiss ${FRUN.id}:f_6eb378e97faaaf48 --org omni-aura`)); + } finally { + stub.close(); + } +}); + +test("review dismiss needs a reason and a finding, before any request", async () => { + const stub = await startStub(); + try { + const noReason = await run(stub.base, ["review", "dismiss", FINDING_OPEN.commentUrl]); + assert.notEqual(noReason.status, 0); + assert.match(noReason.stderr, /--reason/); + const prOnly = await run(stub.base, ["review", "dismiss", "https://github.com/ditto-assistant/console/pull/12", "--reason", "x"]); + assert.notEqual(prOnly.status, 0); + assert.match(prOnly.stderr, /names a pull request, not a finding/); + const bareId = await run(stub.base, ["review", "dismiss", "f_6eb378e97faaaf48", "--reason", "x"]); + assert.notEqual(bareId.status, 0); + assert.match(bareId.stderr, /needs --pr/); + assert.equal(stub.calls.length, 0); + } finally { + stub.close(); + } +}); + +test("review dismiss reports a comment that is not in the newest run", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "dismiss", "https://github.com/ditto-assistant/console/pull/12#discussion_r999", "--reason", "x", "--org", "omni-aura"]); + assert.notEqual(out.status, 0); + assert.match(out.stderr, /discussion_r999 is not in the newest Ditto Review run of ditto-assistant\/console#12/); + assert.equal(stub.calls.some((c) => c.method === "POST"), false); + } finally { + stub.close(); + } +}); + +test("review feedback with a bare finding id and --pr posts the verdict and note", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "feedback", FINDING_OPEN.id, "not-useful", "--pr", "ditto-assistant/console#12", "--note", "Cosmetic.", "--org", "omni-aura"]); + assert.equal(out.status, 0, out.stderr); + const post = stub.calls.find((c) => c.method === "POST"); + assert.equal(post.url, `${FINDING_BASE}/feedback?company=${COMPANY.id}`); + assert.deepEqual(post.body, { verdict: "not_useful", note: "Cosmetic." }); + assert.match(out.stdout, /Recorded "not_useful" on f_6eb378e97faaaf48/); + const bad = await run(stub.base, ["review", "feedback", FINDING_OPEN.commentUrl, "meh"]); + assert.notEqual(bad.status, 0); + assert.match(bad.stderr, /verdict must be one of useful, wrong, not_useful/); + } finally { + stub.close(); + } +}); + +test("review undismiss by run:finding sends the DELETE and relays the GitHub note", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "undismiss", `${FRUN.id}:${FINDING_OPEN.id}`, "--org", "omni-aura"]); + assert.equal(out.status, 0, out.stderr); + const del = stub.calls.find((c) => c.method === "DELETE"); + assert.equal(del.url, `${FINDING_BASE}/dismiss?company=${COMPANY.id}`); + assert.match(out.stdout, /Undismissed f_6eb378e97faaaf48 \(pkg\/a\.go:358\)\.\n Ditto does not reopen the review thread on GitHub\./); + } finally { + stub.close(); + } +});