From 1559912793a6cf1a4f368127f371ec91a4a40d45 Mon Sep 17 00:00:00 2001 From: "ellipsis-dev[bot]" <65095814+ellipsis-dev[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 13:59:21 +0000 Subject: [PATCH] review: add 'agent review default' to set the default code review pipeline A two-rung ladder (account default + per-repo defaults, repo wins) mirroring 'agent config default', over the new GET/PUT/DELETE /v1/reviews/defaults endpoints (ellipsis-dev/ellipsis#6032). Only explicit reviews read it; webhook reviews keep matching the pipelines' own pull_requests filters. Co-Authored-By: Claude Fable 5 --- src/commands/config.ts | 5 +- src/commands/review.ts | 161 ++++++++++++++++++++++++++++++++++++++++- src/lib/api.ts | 29 ++++++++ src/lib/types.ts | 28 +++++++ test/api.test.ts | 61 ++++++++++++++++ 5 files changed, 281 insertions(+), 3 deletions(-) diff --git a/src/commands/config.ts b/src/commands/config.ts index eafc731..7215d86 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -340,8 +340,9 @@ const COMMIT_HINT = // --repo semantics on defaults mutations: absent -> the account rung; bare // --repo -> the repo you're standing in (from the origin remote, an error -// when there isn't one); --repo owner/name -> that repo. -function resolveRepoFlag(repo: string | boolean | undefined): string | undefined { +// when there isn't one); --repo owner/name -> that repo. Shared with +// `agent review default`, whose rungs are addressed identically. +export function resolveRepoFlag(repo: string | boolean | undefined): string | undefined { if (repo === undefined || repo === false) return undefined if (repo === true) { const detected = repoFromCwd(process.cwd()) diff --git a/src/commands/review.ts b/src/commands/review.ts index a50fb11..a7161e5 100644 --- a/src/commands/review.ts +++ b/src/commands/review.ts @@ -3,8 +3,15 @@ import { ApiClient, ApiError } from '../lib/api' import { alsoKnownAs, apiRoutes } from '../lib/help' import { createWipCommit, currentBranch, pushReviewBranch, repoFromCwd } from '../lib/laptop' import { formatTs, printJson, printTable, relativeAge, runAction, usdFromMillicents } from '../lib/output' +import { resolveRepoFlag } from './config' import { watchSessionStreaming } from './session' -import type { CreateReviewRequest, Finding, Review, ReviewScope } from '../lib/types' +import type { + CodeReviewDefaultView, + CreateReviewRequest, + Finding, + Review, + ReviewScope, +} from '../lib/types' // `agent review`: ask for a code review now, instead of waiting for a push to // trigger one. Two targets — @@ -168,6 +175,158 @@ export function registerReview(program: Command): void { ) }) }) + + registerReviewDefaults(review) +} + +// `agent review default`: which code review pipeline runs when an explicit +// review names none — a two-rung ladder (account default + per-repo defaults, +// repo wins) mirroring `agent config default`, with the same --repo +// semantics. Only explicit reviews read it: webhook reviews keep matching the +// pipelines' own `pull_requests:` filters. +function registerReviewDefaults(review: Command): void { + const defaults = apiRoutes( + alsoKnownAs( + review + .command('default') + .description('Show or set which code review pipeline runs when a review names none'), + 'defaults', + ), + 'GET /v1/reviews/defaults', + ) + .option('--json', 'output raw JSON') + // Bare `agent review default`: the effective default for the repo you're + // standing in, computed locally from GET /v1/reviews/defaults + the origin + // remote (the same ladder the server resolves at review start). + .action(async (opts: { json?: boolean }) => { + await runAction(async () => { + const rungs = await new ApiClient().listReviewDefaults() + const repo = repoFromCwd(process.cwd()) + const repoRung = repo + ? rungs.find((d) => d.repository?.toLowerCase() === repo.toLowerCase()) + : undefined + const accountRung = rungs.find((d) => d.repository === null) + const effective = repoRung ?? accountRung + if (opts.json) { + printJson({ repository: repo ?? null, effective: effective ?? null }) + return + } + if (!effective) { + console.log( + repo + ? `no default set for ${repo} or the account (reviews run the synced pipeline, or the platform defaults)` + : 'no account default set (reviews run the synced pipeline, or the platform defaults)', + ) + return + } + const rung = effective.repository + ? `repo default for ${effective.repository}` + : 'account default' + console.log( + `using pipeline "${defaultName(effective)}" (${rung})${brokenSuffix(effective)}`, + ) + }) + }) + + apiRoutes( + alsoKnownAs( + defaults + .command('list') + .description('List every default that is set, account rung and per-repo rungs'), + 'ls', + ), + 'GET /v1/reviews/defaults', + ) + .option('--json', 'output raw JSON') + // The group also defines --json (for the bare view), and commander parses + // parent options even when they follow the subcommand name — so read the + // merged view, not just this command's own opts. + .action(async (_opts: { json?: boolean }, cmd: Command) => { + await runAction(async () => { + const rungs = await new ApiClient().listReviewDefaults() + if (cmd.optsWithGlobals().json) { + printJson(rungs) + return + } + if (rungs.length === 0) { + console.log( + 'No defaults set. Reviews run the synced pipeline, or the platform defaults.', + ) + return + } + printTable( + ['RUNG', 'PIPELINE', 'CONFIG ID', 'STATUS', 'UPDATED'], + rungs.map((d) => [ + d.repository ?? 'account', + d.config_name ?? '—', + d.config_id, + d.broken ? `broken: ${d.broken}` : 'ok', + formatTs(d.updated_at), + ]), + ) + }) + }) + + apiRoutes( + defaults + .command('set ') + .description('Set the account default code review pipeline, or a repo default with --repo'), + 'PUT /v1/reviews/defaults', + ) + .option( + '-r, --repo [repository]', + 'target a repo rung: "owner/name", or no value for the repo you are standing in', + ) + .option('--json', 'output raw JSON') + .action( + async (configId: string, opts: { repo?: string | boolean; json?: boolean }, cmd: Command) => { + await runAction(async () => { + const repository = resolveRepoFlag(opts.repo) + const set = await new ApiClient().putReviewDefault({ + config_id: configId, + ...(repository ? { repository } : {}), + }) + if (cmd.optsWithGlobals().json) { + printJson(set) + return + } + const rung = set.repository ? `default for ${set.repository}` : 'account default' + console.log(`✓ set ${rung} to "${defaultName(set)}" (${set.config_id})`) + }) + }, + ) + + apiRoutes( + alsoKnownAs( + defaults + .command('clear') + .description('Clear the account default code review pipeline, or a repo default with --repo'), + 'rm', + 'delete', + ), + 'DELETE /v1/reviews/defaults', + ) + .option( + '-r, --repo [repository]', + 'target a repo rung: "owner/name", or no value for the repo you are standing in', + ) + .action(async (opts: { repo?: string | boolean }) => { + await runAction(async () => { + const repository = resolveRepoFlag(opts.repo) + await new ApiClient().deleteReviewDefault(repository) + console.log(`✓ cleared ${repository ? `default for ${repository}` : 'account default'}`) + }) + }) +} + +function defaultName(d: CodeReviewDefaultView): string { + return d.config_name ?? d.config_id +} + +// A set-but-broken rung fails explicit reviews closed (never a silent run of +// a different pipeline), so surface it wherever the rung is shown. +function brokenSuffix(d: CodeReviewDefaultView): string { + return d.broken ? ` (broken: ${d.broken})` : '' } interface StartOptions { diff --git a/src/lib/api.ts b/src/lib/api.ts index 5e79d61..ef82e24 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -11,6 +11,7 @@ import type { BudgetSummary, CliAuthPoll, CliAuthStart, + CodeReviewDefaultView, CreateAgentConfigRequest, CreateAssetRequest, CreateAssetResponse, @@ -32,6 +33,7 @@ import type { ListAgentTemplatesResponse, ListAssetsQuery, ListAssetsResponse, + ListCodeReviewDefaultsResponse, ListGithubMembersResponse, ListGithubRepositoriesResponse, ListLinearTeamsResponse, @@ -43,6 +45,7 @@ import type { ListSlackChannelsResponse, ListSlackMembersResponse, PutAgentDefaultRequest, + PutCodeReviewDefaultRequest, ReplayAgentSessionRequest, Review, SendSessionMessageRequest, @@ -362,6 +365,32 @@ export class ApiClient { return res.reviews } + // --------------------------- review defaults ----------------------------- + // The default code review pipeline ladder (repo default -> account default), + // the code_review twin of the /v1/defaults methods below and addressed the + // same way: `repository` is "owner/name" for a repo rung and null/omitted + // for the account rung — never a row id. Read by POST /v1/reviews when no + // config is named; webhook reviews are unaffected. Mutations are refused + // for sandbox tokens (403). + + async listReviewDefaults(): Promise { + const res = await this.request( + 'GET', + '/v1/reviews/defaults', + ) + return res.defaults + } + + putReviewDefault(req: PutCodeReviewDefaultRequest): Promise { + return this.request('PUT', '/v1/reviews/defaults', req) + } + + // Clears a rung: the account default when `repository` is omitted, that + // repo's default otherwise. 404 when the rung isn't set. + deleteReviewDefault(repository?: string): Promise { + return this.request('DELETE', '/v1/reviews/defaults', undefined, { repository }) + } + // ----------------------------- agent configs ---------------------------- async listAgentConfigs(): Promise { diff --git a/src/lib/types.ts b/src/lib/types.ts index 5c5a548..54c9a89 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -305,6 +305,34 @@ export interface PutAgentDefaultRequest { config_id: string } +// One rung of the default code review pipeline ladder (GET +// /v1/reviews/defaults) — the code_review twin of AgentDefaultView, pointing +// at a synced `kind: code_review` pipeline (crcfg_…) instead of an agent +// config. Read by POST /v1/reviews when no config is named: repo default -> +// account default -> the oldest synced pipeline -> the platform defaults. +export interface CodeReviewDefaultView { + id: string + repository: string | null + config_id: string + // The pointed-at pipeline's name; null when the pipeline is gone (see broken). + config_name: string | null + // Why this rung can't serve reviews (config_deleted | config_disabled | + // config_sync_error | repo_inaccessible); null when healthy. + broken: string | null + updated_at: string +} + +export interface ListCodeReviewDefaultsResponse { + defaults: CodeReviewDefaultView[] +} + +// Body of PUT /v1/reviews/defaults: point a rung at a pipeline. `repository` +// omitted sets the account default; "owner/name" sets that repo's default. +export interface PutCodeReviewDefaultRequest { + repository?: string + config_id: string +} + // Create-config payload for POST /v1/configs. Exactly one of `config` (inline) // or `template_id` (a gallery template slug). `repository` is a bare repo name // in the caller's account — the owner is always the account. diff --git a/test/api.test.ts b/test/api.test.ts index 1741e13..da0e8d4 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -192,6 +192,67 @@ describe('ApiClient sandbox variables', () => { }) }) +describe('ApiClient review defaults', () => { + afterEach(() => vi.unstubAllGlobals()) + + const rung = { + id: 'crdef_1', + repository: null, + config_id: 'crcfg_1', + config_name: 'Team reviewer', + broken: null, + updated_at: '', + } + + it('lists rungs and unwraps the response envelope', async () => { + const fetchMock = vi.fn( + async () => new Response(JSON.stringify({ defaults: [rung] }), { status: 200 }), + ) + vi.stubGlobal('fetch', fetchMock) + + const out = await new ApiClient('http://api.test', 't').listReviewDefaults() + expect(out).toEqual([rung]) + expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/v1/reviews/defaults') + expect((fetchMock.mock.calls[0][1] as RequestInit).method).toBe('GET') + }) + + it('PUTs the rung: account when repository is omitted, repo when named', async () => { + const fetchMock = vi.fn(async () => new Response(JSON.stringify(rung), { status: 200 })) + vi.stubGlobal('fetch', fetchMock) + + await new ApiClient('http://api.test', 't').putReviewDefault({ config_id: 'crcfg_1' }) + let [url, init] = fetchMock.mock.calls[0] + expect(url).toBe('http://api.test/v1/reviews/defaults') + expect((init as RequestInit).method).toBe('PUT') + expect(JSON.parse((init as RequestInit).body as string)).toEqual({ config_id: 'crcfg_1' }) + + await new ApiClient('http://api.test', 't').putReviewDefault({ + config_id: 'crcfg_1', + repository: 'acme/api', + }) + ;[url, init] = fetchMock.mock.calls[1] + expect(JSON.parse((init as RequestInit).body as string)).toEqual({ + config_id: 'crcfg_1', + repository: 'acme/api', + }) + }) + + it('clears a rung via query param, omitted for the account rung', async () => { + const fetchMock = vi.fn(async () => new Response(null, { status: 204 })) + vi.stubGlobal('fetch', fetchMock) + + const api = new ApiClient('http://api.test', 't') + await api.deleteReviewDefault() + expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/v1/reviews/defaults') + expect((fetchMock.mock.calls[0][1] as RequestInit).method).toBe('DELETE') + + await api.deleteReviewDefault('acme/api') + expect(fetchMock.mock.calls[1][0]).toBe( + 'http://api.test/v1/reviews/defaults?repository=acme%2Fapi', + ) + }) +}) + describe('getSessionLog', () => { afterEach(() => vi.unstubAllGlobals())