diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..b38fa25 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,20 @@ +name: CLI verification + +on: + pull_request: + +permissions: + contents: read + +jobs: + verify: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v7 + with: + node-version: 24 + cache: npm + - run: npm ci + - run: npm run verify diff --git a/README.md b/README.md index 50284e7..55b9e07 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,44 @@ heyditto search "typescript" "language choices" heyditto search "launch notes" --include-public --filter-username peyton ``` +### `review` + +Manage Ditto Review and follow reviews from either `ditto` or `heyditto`. +Commands use your selected organization (`orgs use`) or `--org`; without one, +they use your personal workspace. Repositories must already be set up for +Review. Workspace membership and manager permissions are enforced by the API. + +```bash +heyditto review repos --org omniaura +heyditto review set ditto-assistant/console --max-minutes 20 --org omniaura +heyditto review pulls ditto-assistant/console --org omniaura +heyditto review start ditto-assistant/console 88 --org omniaura --watch +heyditto review runs ditto-assistant/console --org omniaura --output json +heyditto review status --org omniaura --output json +heyditto review watch --org omniaura --interval 10 --timeout 3600 +heyditto review retry --org omniaura +heyditto review cancel --org omniaura +``` + +`start` reviews the PR's current head using the repository's saved budget and +settings. Starting a previously completed head requests another paid attempt. +`retry` can reuse a stored result after publication failure; other eligible +retries can spend a new per-run budget. It does not raise any budget or enable +a paused repository. `cancel` requests cancellation; a running review stops +asynchronously, and a review already being published may refuse cancellation. + +`runs` lists the API's recent workspace runs; `status --output json` includes findings and +prior attempts. `watch` polls without starting, retrying, or cancelling the run. +It stops on completion, partial coverage, failure, cancellation, skip, or +supersession. Failed runs and timeouts return a nonzero exit code. A partial +review remains partial even though the command succeeds. A timeout or Ctrl-C +only stops the local watcher; use `cancel` to stop the server review. +`--output json` prints one final JSON document, with watch progress on stderr. + +The lifecycle commands require a backend with CLI-key access to Review run +and PR routes. On an older backend they return an authentication refusal; +upgrading the CLI alone cannot enable those routes. + ### `fetch` Fetch memory content for private pair ids or public share ids. The default diff --git a/src/api.ts b/src/api.ts index 1e09f45..35a19ee 100644 --- a/src/api.ts +++ b/src/api.ts @@ -37,7 +37,7 @@ async function requireKey(): Promise { export async function apiFetch( path: string, - init: { method?: string; body?: unknown; auth?: boolean } = {}, + init: { method?: string; body?: unknown; auth?: boolean; signal?: AbortSignal } = {}, ): Promise { const headers: Record = { Accept: "application/json", @@ -49,6 +49,7 @@ export async function apiFetch( method: init.method ?? "GET", headers, body: init.body === undefined ? undefined : JSON.stringify(init.body), + signal: init.signal, }); if (!response.ok) { const text = await response.text().catch(() => ""); diff --git a/src/review-commands.ts b/src/review-commands.ts index 42343be..edf7ee6 100644 --- a/src/review-commands.ts +++ b/src/review-commands.ts @@ -1,3 +1,4 @@ +import { setTimeout as delay } from "node:timers/promises"; import type { Command, Option } from "commander"; import { apiFetch, type Company, resolveCompany } from "./api.js"; import { readStoredAuth } from "./store.js"; @@ -78,6 +79,175 @@ export async function listReviewRepositories(company: Company | undefined): Prom return out.repositories ?? []; } +export interface ReviewRun { + id: string; + repositoryId: string; + prNumber: number; + status: string; + headSha: string; + attempts: number; + reviewUrl?: string; + error?: string; + cancelRequested?: boolean; + chargedCredits?: number; + creditsPerDollar?: number; + summary?: unknown; +} + +interface ReviewList { + repositories: ReviewRepository[]; + runs: ReviewRun[]; +} + +interface ReviewDetail { + run: ReviewRun; + repository: ReviewRepository; + result?: unknown; + attempts?: unknown[]; +} + +interface WatchOptions extends ScopeOptions { + interval?: string; + timeout?: string; + watch?: boolean; +} + +const TERMINAL_REVIEW_STATUSES = new Set(["completed", "partial", "failed", "cancelled", "skipped", "superseded"]); +const ACTIVE_REVIEW_STATUSES = new Set(["queued", "running", "publishing"]); + +function runPath(id: string): string { + if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(id)) { + throw new Error("run id must be a UUID (see `heyditto review runs`)"); + } + return `/api/v5/review/runs/${encodeURIComponent(id)}`; +} + +async function findReviewRepository(fullName: string, company: Company | undefined): Promise { + const repos = await listReviewRepositories(company); + const repo = repos.find((r) => r.fullName.toLowerCase() === fullName.trim().toLowerCase()); + if (!repo) throw new Error(`${fullName} is not set up in this workspace (see \`heyditto review repos --org \`).`); + return repo; +} + +function writeJSON(value: unknown): void { + process.stdout.write(`${JSON.stringify(value, null, 2)}\n`); +} + +function printRun(detail: ReviewDetail, options: ScopeOptions): void { + if (isJSON(options)) return writeJSON(detail); + const r = detail.run; + process.stdout.write(`${detail.repository.fullName} #${r.prNumber}: ${r.status}${r.cancelRequested ? " (cancellation requested)" : ""}\n`); + process.stdout.write(`Run: ${r.id}\nHead: ${r.headSha}\n`); + if (r.reviewUrl) process.stdout.write(`Review: ${r.reviewUrl}\n`); + if (r.summary) process.stdout.write(`Summary: ${JSON.stringify(r.summary)}\n`); + if (r.error) process.stdout.write(`Error: ${r.error}\n`); +} + +export async function cmdReviewRuns(fullName: string | undefined, options: ScopeOptions): Promise { + const company = await resolveScope(options); + const out = await apiFetch(`/api/v5/review${scopeQuery(company)}`); + let runs = out.runs ?? []; + if (fullName) { + const repo = out.repositories.find((r) => r.fullName.toLowerCase() === fullName.trim().toLowerCase()); + if (!repo) throw new Error(`${fullName} is not set up in this workspace`); + runs = runs.filter((r) => r.repositoryId === repo.id); + } + if (isJSON(options)) return writeJSON({ runs }); + const names = new Map(out.repositories.map((r) => [r.id, r.fullName])); + if (!runs.length) process.stdout.write("No recent review runs in this workspace.\n"); + for (const r of runs) process.stdout.write(`${r.id} ${r.status} ${names.get(r.repositoryId) ?? r.repositoryId} #${r.prNumber} ${r.headSha.slice(0, 7)}\n`); +} + +export async function cmdReviewPulls(fullName: string, options: ScopeOptions): Promise { + const company = await resolveScope(options); + const repo = await findReviewRepository(fullName, company); + const out = await apiFetch<{ pulls: { number: number; title: string; draft: boolean; url: string }[] }>( + `/api/v5/review/repositories/${encodeURIComponent(repo.id)}/pulls${scopeQuery(company)}`, + ); + if (isJSON(options)) return writeJSON(out); + if (!out.pulls.length) process.stdout.write(`No open pull requests in ${repo.fullName}.\n`); + for (const p of out.pulls) process.stdout.write(`#${p.number}${p.draft ? " (draft)" : ""} ${p.title}\n ${p.url}\n`); +} + +export async function cmdReviewStatus(id: string, options: ScopeOptions): Promise { + const route = runPath(id); + const company = await resolveScope(options); + printRun(await apiFetch(`${route}${scopeQuery(company)}`), options); +} + +function watchLimits(options: WatchOptions): { intervalMs: number; timeoutMs: number } { + return { + intervalMs: intFlag("--interval", options.interval ?? "10", 1, 60) * 1000, + timeoutMs: intFlag("--timeout", options.timeout ?? "3600", 1, 86400) * 1000, + }; +} + +async function watchReview(id: string, company: Company | undefined, options: WatchOptions): Promise { + const route = runPath(id); + const { intervalMs, timeoutMs } = watchLimits(options); + const deadline = performance.now() + timeoutMs; + let prior = ""; + for (;;) { + const remaining = deadline - performance.now(); + if (remaining <= 0) throw new Error(`timed out waiting for review ${id}; the review continues on the server`); + let detail: ReviewDetail; + try { + detail = await apiFetch(`${route}${scopeQuery(company)}`, { signal: AbortSignal.timeout(Math.ceil(remaining)) }); + } catch (error) { + if (performance.now() >= deadline) throw new Error(`timed out waiting for review ${id}; the review continues on the server`); + throw error; + } + const status = detail.run.status; + if (status !== prior) { + process.stderr.write(`Review ${id}: ${status}\n`); + prior = status; + } + if (TERMINAL_REVIEW_STATUSES.has(status)) { + printRun(detail, options); + if (status === "failed") process.exitCode = 1; + return; + } + if (!ACTIVE_REVIEW_STATUSES.has(status)) throw new Error(`unknown review status: ${status}`); + await delay(Math.max(1, Math.min(intervalMs, deadline - performance.now()))); + } +} + +export async function cmdReviewWatch(id: string, options: WatchOptions): Promise { + runPath(id); + watchLimits(options); + const company = await resolveScope(options); + await watchReview(id, company, options); +} + +export async function cmdReviewStart(fullName: string, prNumber: string, options: WatchOptions): Promise { + const number = intFlag("pr-number", prNumber, 1, 2_147_483_647); + if (options.watch) watchLimits(options); + const company = await resolveScope(options); + const repo = await findReviewRepository(fullName, company); + const out = await apiFetch<{ runId: string; status: string; alreadyReviewed?: unknown }>( + `/api/v5/review/repositories/${encodeURIComponent(repo.id)}/pulls/${number}/review${scopeQuery(company)}`, + { method: "POST" }, + ); + if (options.watch) { + process.stderr.write(`Review requested for ${repo.fullName} #${number}: ${out.runId}\n`); + if (out.alreadyReviewed) process.stderr.write("This head was already reviewed; this request starts a new attempt within the repository's budget.\n"); + return watchReview(out.runId, company, options); + } + if (isJSON(options)) return writeJSON(out); + process.stdout.write(`${repo.fullName} #${number}: ${out.status}\nRun: ${out.runId}\n`); + if (out.alreadyReviewed) process.stdout.write("This head was already reviewed; a new attempt was requested.\n"); +} + +export async function cmdReviewAction(id: string, action: "retry" | "cancel", options: ScopeOptions): Promise { + const route = runPath(id); + const company = await resolveScope(options); + const out = await apiFetch<{ status: string; cancelRequested?: boolean; reusedResult?: boolean; watchCancelled?: boolean }>( + `${route}/${action}${scopeQuery(company)}`, { method: "POST" }, + ); + if (isJSON(options)) return writeJSON(out); + process.stdout.write(`${id}: ${out.status}${out.cancelRequested ? " (cancellation requested)" : ""}${out.reusedResult ? " (reusing stored result)" : ""}${out.watchCancelled ? " (CI watch cancelled)" : ""}\n`); +} + function money(cents: number): string { return `$${(cents / 100).toFixed(2)}`; } @@ -189,8 +359,8 @@ export function registerReviewCommands( ): void { const review = program .command("review") - .description("Ditto Review repositories and their settings") - .summary("Ditto Review settings"); + .description("Ditto Review repositories, settings, and review runs") + .summary("manage and follow Ditto Review"); addExamples( review .command("repos", { isDefault: true }) @@ -201,6 +371,30 @@ export function registerReviewCommands( ` heyditto review repos --org omni-aura heyditto review repos --output json`, ); + const scoped = (command: Command): Command => command.addOption(orgOption()).addOption(outputOption()); + const watched = (command: Command): Command => command + .option("--interval ", "poll interval (1-60 seconds)", "10") + .option("--timeout ", "stop waiting after this many seconds (review continues)", "3600"); + scoped(review.command("runs").description("list recent review runs in this workspace") + .argument("[repository]", "filter by owner/name")).action(cmdReviewRuns); + scoped(review.command("pulls").description("list open pull requests available to review") + .argument("", "owner/name")).action(cmdReviewPulls); + addExamples( + watched(scoped(review.command("start").description("request a review of the current PR head (uses the repository's budget; re-reviews cost a new attempt)") + .argument("", "owner/name").argument("", "open pull request number") + .option("--watch", "follow the requested run until it finishes"))).action(cmdReviewStart), + ` heyditto review start ditto-assistant/console 88 --org omniaura --watch`, + ); + scoped(review.command("status").description("show run status; JSON includes results and prior attempts") + .argument("", "review run UUID")).action(cmdReviewStatus); + watched(scoped(review.command("watch").description("follow a review until it finishes; does not start or retry it") + .argument("", "review run UUID"))).action(cmdReviewWatch); + for (const action of ["retry", "cancel"] as const) { + scoped(review.command(action).description(action === "retry" + ? "retry an eligible run (may spend a new budget; publication retries reuse stored results)" + : "request cancellation of a queued/running review or its CI watch") + .argument("", "review run UUID")).action((id: string, options: ScopeOptions) => cmdReviewAction(id, action, options)); + } addExamples( review .command("set") diff --git a/test/funnel.test.mjs b/test/funnel.test.mjs index 9bf6bf5..14d584f 100644 --- a/test/funnel.test.mjs +++ b/test/funnel.test.mjs @@ -468,14 +468,25 @@ test("a pending_plan endpoint blocks launch and prints the activation notice wit test("claude/codex without a key and without a TTY still fail fast (no device flow)", async () => { const stub = await startStub(); + const binDir = mkdtempSync(path.join(os.tmpdir(), "heyditto-auth-harness-")); + const launched = path.join(binDir, "launched"); + for (const harness of ["claude", "codex"]) { + const executable = path.join(binDir, harness); + writeFileSync(executable, `#!/bin/sh\ntouch "${launched}"\nexit 99\n`); + chmodSync(executable, 0o755); + } try { for (const harness of ["claude", "codex"]) { - const result = await runAsync([harness, "--endpoint", "alpha"], { DITTO_API_BASE: stub.base }); + const result = await runAsync([harness, "--endpoint", "alpha"], { + DITTO_API_BASE: stub.base, + PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}`, + }); assert.equal(result.status, 1, harness); assert.match(result.stderr, /no Ditto API key configured/); assert.match(result.stderr, /heyditto login/); } assert.ok(!stub.calls.some((c) => c.url === "/api/v2/mcp/device-code"), "non-interactive runs must not start a device flow"); + assert.ok(!existsSync(launched), "authentication must fail before launching either harness"); } finally { stub.close(); } diff --git a/test/review-commands.test.mjs b/test/review-commands.test.mjs index 50e05b3..3d06ca4 100644 --- a/test/review-commands.test.mjs +++ b/test/review-commands.test.mjs @@ -18,7 +18,12 @@ const REPO = { minConfidence: 70, maxComments: 30, autofixMaxPushes: 2, monthlyBudgetUsd: null, lastPolledAt: "x", spend: { cents: 1 }, }; -function startStub() { +const RUN = { id: "55555555-5555-5555-5555-555555555555", repositoryId: REPO.id, prNumber: 88, + status: "completed", headSha: "a".repeat(40), attempts: 0, summary: { posted: 1 }, + reviewUrl: "https://github.com/ditto-assistant/console/pull/88#pullrequestreview-1" }; + +function startStub({ statuses = ["completed"], refusal, detailDelay = 0 } = {}) { + let detailReads = 0; const calls = []; const server = http.createServer((req, res) => { let body = ""; @@ -28,8 +33,24 @@ function startStub() { res.setHeader("content-type", "application/json"); const json = (status, payload) => { res.statusCode = status; res.end(JSON.stringify(payload)); }; if (req.url === "/api/v5/companies") return json(200, { companies: [COMPANY] }); - if (req.url === `/api/v5/review?company=${COMPANY.id}` && req.method === "GET") return json(200, { repositories: [REPO], runs: [] }); + if (req.url === `/api/v5/review?company=${COMPANY.id}` && req.method === "GET") return json(200, { repositories: [REPO], runs: [RUN] }); if (req.url === `/api/v5/review/repositories?company=${COMPANY.id}` && req.method === "PUT") return json(200, { ...REPO, ...JSON.parse(body), revision: 8 }); + const query = `?company=${COMPANY.id}`; + const pullsRoute = `/api/v5/review/repositories/${REPO.id}/pulls`; + if (req.url === pullsRoute + query && req.method === "GET") return json(200, { pulls: [{ number: 88, title: "review picker", draft: false, url: "https://github.com/ditto-assistant/console/pull/88" }] }); + if (req.url === pullsRoute + "/88/review" + query && req.method === "POST") { + if (refusal) return json(refusal, { message: "only managers may start reviews" }); + return json(202, { runId: RUN.id, status: "queued", alreadyReviewed: { attempt: 1 } }); + } + const runRoute = `/api/v5/review/runs/${RUN.id}`; + if (req.url === runRoute + query && req.method === "GET") { + const status = statuses[Math.min(detailReads++, statuses.length - 1)]; + const reply = () => json(200, { run: { ...RUN, status }, repository: REPO, result: { findings: [{ title: "finding" }] }, attempts: [] }); + if (detailDelay) setTimeout(reply, detailDelay); else reply(); + return; + } + 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 }); json(404, { message: "no route" }); }); }); @@ -97,3 +118,120 @@ test("review set refuses an out-of-range time limit before any request", async ( stub.close(); } }); + + +test("review help exposes lifecycle commands", async () => { + const out = await run("http://127.0.0.1:1", ["review", "--help"]); + assert.equal(out.status, 0, out.stderr); + for (const command of ["repos", "set", "runs", "pulls", "start", "status", "watch", "retry", "cancel"]) assert.match(out.stdout, new RegExp(`\\b${command}\\b`)); +}); + +test("review runs filters by repository and returns JSON", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "runs", "Ditto-Assistant/Console", "--org", COMPANY.slug, "--output", "json"]); + assert.equal(out.status, 0, out.stderr); + assert.deepEqual(JSON.parse(out.stdout), { runs: [RUN] }); + assert.ok(stub.calls.every(c => c.method === "GET")); + } finally { stub.close(); } +}); + +test("review pulls uses the repository id and organization scope", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "pulls", REPO.fullName, "--org", COMPANY.slug]); + assert.equal(out.status, 0, out.stderr); + assert.match(out.stdout, /#88.*review picker/); + assert.ok(stub.calls.some(c => c.url === `/api/v5/review/repositories/${REPO.id}/pulls?company=${COMPANY.id}`)); + } finally { stub.close(); } +}); + +test("review start queues exactly once and preserves the re-review notice", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "start", REPO.fullName, "88", "--org", COMPANY.slug, "--output", "json"]); + assert.equal(out.status, 0, out.stderr); + assert.equal(JSON.parse(out.stdout).alreadyReviewed.attempt, 1); + const writes = stub.calls.filter(c => c.method === "POST"); + assert.equal(writes.length, 1); + assert.equal(writes[0].url, `/api/v5/review/repositories/${REPO.id}/pulls/88/review?company=${COMPANY.id}`); + } finally { stub.close(); } +}); + +test("review start --watch follows the returned run without queuing it again and emits one JSON document", async () => { + const stub = await startStub({ statuses: ["running", "publishing", "partial"] }); + try { + const out = await run(stub.base, ["review", "start", REPO.fullName, "88", "--org", COMPANY.slug, "--watch", "--interval", "1", "--timeout", "10", "--output", "json"]); + assert.equal(out.status, 0, out.stderr); + assert.equal(JSON.parse(out.stdout).run.status, "partial"); + assert.match(out.stderr, /running[\s\S]*publishing[\s\S]*partial/); + assert.match(out.stderr, /already reviewed/); + assert.equal(stub.calls.filter(c => c.method === "POST").length, 1); + } finally { stub.close(); } +}); + +test("review status exposes results and uses the scoped run detail", async () => { + const stub = await startStub(); + try { + const out = await run(stub.base, ["review", "status", RUN.id, "--org", COMPANY.slug, "--output", "json"]); + assert.equal(out.status, 0, out.stderr); + assert.equal(JSON.parse(out.stdout).result.findings[0].title, "finding"); + assert.ok(stub.calls.some(c => c.url === `/api/v5/review/runs/${RUN.id}?company=${COMPANY.id}`)); + } finally { stub.close(); } +}); + +test("review watch exits on failed, cancelled, skipped, and superseded states without a write", async () => { + for (const status of ["failed", "cancelled", "skipped", "superseded"]) { + const stub = await startStub({ statuses: [status] }); + try { + const out = await run(stub.base, ["review", "watch", RUN.id, "--org", COMPANY.slug, "--output", "json"]); + assert.equal(out.status, status === "failed" ? 1 : 0, out.stderr); + assert.equal(JSON.parse(out.stdout).run.status, status); + assert.ok(stub.calls.every(c => c.method === "GET")); + } finally { stub.close(); } + } +}); + +test("review watch times out without cancelling the server run, including a hung response", async () => { + for (const detailDelay of [0, 2000]) { + const stub = await startStub({ statuses: ["running"], detailDelay }); + try { + const out = await run(stub.base, ["review", "watch", RUN.id, "--org", COMPANY.slug, "--timeout", "1", "--interval", "1"]); + assert.notEqual(out.status, 0); + assert.match(out.stderr, /timed out.*review continues on the server/); + assert.ok(stub.calls.every(c => c.method === "GET")); + } finally { stub.close(); } + } +}); + +test("review lifecycle validates identifiers and watch flags before any request", async () => { + const stub = await startStub(); + try { + for (const args of [ + ["start", REPO.fullName, "0"], ["start", REPO.fullName, "1.5"], + ["start", REPO.fullName, "88", "--watch", "--interval", "0"], + ["watch", RUN.id, "--timeout", "0"], ["status", "../other"], ["cancel", "invalid"], + ]) { + const out = await run(stub.base, ["review", ...args, "--org", COMPANY.slug]); + assert.notEqual(out.status, 0, JSON.stringify(args)); + } + assert.equal(stub.calls.length, 0); + } finally { stub.close(); } +}); + +test("review actions keep server semantics; start propagates manager refusal without a retry", async () => { + const stub = await startStub({ refusal: 403 }); + try { + for (const action of ["retry", "cancel"]) { + const out = await run(stub.base, ["review", action, RUN.id, "--org", COMPANY.slug, "--output", "json"]); + assert.equal(out.status, 0, out.stderr); + assert.equal(JSON.parse(out.stdout)[action === "retry" ? "reusedResult" : "cancelRequested"], true); + assert.ok(stub.calls.some(c => c.method === "POST" && c.url === `/api/v5/review/runs/${RUN.id}/${action}?company=${COMPANY.id}`)); + } + const before = stub.calls.filter(c => c.method === "POST").length; + const out = await run(stub.base, ["review", "start", REPO.fullName, "88", "--org", COMPANY.slug]); + assert.notEqual(out.status, 0); + assert.match(out.stderr, /HTTP 403.*only managers/); + assert.equal(stub.calls.filter(c => c.method === "POST").length, before + 1); + } finally { stub.close(); } +}); diff --git a/test/teleport.test.mjs b/test/teleport.test.mjs index c161a86..bcbb641 100644 --- a/test/teleport.test.mjs +++ b/test/teleport.test.mjs @@ -626,7 +626,7 @@ test("pull restores per-branch upstreams tracking different remotes", async () = /** A repo whose branch is `ahead` commits past a real bare origin, with `behind` commits only on origin. */ function makeTrackedRepo({ ahead = 0, behind = 0 } = {}) { const origin = tmp("teleport-origin-"); - git(["init", "--bare", "-q", origin], os.tmpdir()); + git(["init", "--bare", "-q", "-b", "main", origin], os.tmpdir()); const src = tmp("teleport-src-"); git(["init", "-q", "-b", "main"], src); git(["config", "user.email", "t@example.test"], src);