Skip to content

Commit 9d479f1

Browse files
review: add 'agent review default' to set the default code review pipeline (#92)
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: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent be2094d commit 9d479f1

5 files changed

Lines changed: 281 additions & 3 deletions

File tree

src/commands/config.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,9 @@ const COMMIT_HINT =
340340

341341
// --repo semantics on defaults mutations: absent -> the account rung; bare
342342
// --repo -> the repo you're standing in (from the origin remote, an error
343-
// when there isn't one); --repo owner/name -> that repo.
344-
function resolveRepoFlag(repo: string | boolean | undefined): string | undefined {
343+
// when there isn't one); --repo owner/name -> that repo. Shared with
344+
// `agent review default`, whose rungs are addressed identically.
345+
export function resolveRepoFlag(repo: string | boolean | undefined): string | undefined {
345346
if (repo === undefined || repo === false) return undefined
346347
if (repo === true) {
347348
const detected = repoFromCwd(process.cwd())

src/commands/review.ts

Lines changed: 160 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,15 @@ import { ApiClient, ApiError } from '../lib/api'
33
import { alsoKnownAs, apiRoutes } from '../lib/help'
44
import { createWipCommit, currentBranch, pushReviewBranch, repoFromCwd } from '../lib/laptop'
55
import { formatTs, printJson, printTable, relativeAge, runAction, usdFromMillicents } from '../lib/output'
6+
import { resolveRepoFlag } from './config'
67
import { watchSessionStreaming } from './session'
7-
import type { CreateReviewRequest, Finding, Review, ReviewScope } from '../lib/types'
8+
import type {
9+
CodeReviewDefaultView,
10+
CreateReviewRequest,
11+
Finding,
12+
Review,
13+
ReviewScope,
14+
} from '../lib/types'
815

916
// `agent review`: ask for a code review now, instead of waiting for a push to
1017
// trigger one. Two targets —
@@ -168,6 +175,158 @@ export function registerReview(program: Command): void {
168175
)
169176
})
170177
})
178+
179+
registerReviewDefaults(review)
180+
}
181+
182+
// `agent review default`: which code review pipeline runs when an explicit
183+
// review names none — a two-rung ladder (account default + per-repo defaults,
184+
// repo wins) mirroring `agent config default`, with the same --repo
185+
// semantics. Only explicit reviews read it: webhook reviews keep matching the
186+
// pipelines' own `pull_requests:` filters.
187+
function registerReviewDefaults(review: Command): void {
188+
const defaults = apiRoutes(
189+
alsoKnownAs(
190+
review
191+
.command('default')
192+
.description('Show or set which code review pipeline runs when a review names none'),
193+
'defaults',
194+
),
195+
'GET /v1/reviews/defaults',
196+
)
197+
.option('--json', 'output raw JSON')
198+
// Bare `agent review default`: the effective default for the repo you're
199+
// standing in, computed locally from GET /v1/reviews/defaults + the origin
200+
// remote (the same ladder the server resolves at review start).
201+
.action(async (opts: { json?: boolean }) => {
202+
await runAction(async () => {
203+
const rungs = await new ApiClient().listReviewDefaults()
204+
const repo = repoFromCwd(process.cwd())
205+
const repoRung = repo
206+
? rungs.find((d) => d.repository?.toLowerCase() === repo.toLowerCase())
207+
: undefined
208+
const accountRung = rungs.find((d) => d.repository === null)
209+
const effective = repoRung ?? accountRung
210+
if (opts.json) {
211+
printJson({ repository: repo ?? null, effective: effective ?? null })
212+
return
213+
}
214+
if (!effective) {
215+
console.log(
216+
repo
217+
? `no default set for ${repo} or the account (reviews run the synced pipeline, or the platform defaults)`
218+
: 'no account default set (reviews run the synced pipeline, or the platform defaults)',
219+
)
220+
return
221+
}
222+
const rung = effective.repository
223+
? `repo default for ${effective.repository}`
224+
: 'account default'
225+
console.log(
226+
`using pipeline "${defaultName(effective)}" (${rung})${brokenSuffix(effective)}`,
227+
)
228+
})
229+
})
230+
231+
apiRoutes(
232+
alsoKnownAs(
233+
defaults
234+
.command('list')
235+
.description('List every default that is set, account rung and per-repo rungs'),
236+
'ls',
237+
),
238+
'GET /v1/reviews/defaults',
239+
)
240+
.option('--json', 'output raw JSON')
241+
// The group also defines --json (for the bare view), and commander parses
242+
// parent options even when they follow the subcommand name — so read the
243+
// merged view, not just this command's own opts.
244+
.action(async (_opts: { json?: boolean }, cmd: Command) => {
245+
await runAction(async () => {
246+
const rungs = await new ApiClient().listReviewDefaults()
247+
if (cmd.optsWithGlobals().json) {
248+
printJson(rungs)
249+
return
250+
}
251+
if (rungs.length === 0) {
252+
console.log(
253+
'No defaults set. Reviews run the synced pipeline, or the platform defaults.',
254+
)
255+
return
256+
}
257+
printTable(
258+
['RUNG', 'PIPELINE', 'CONFIG ID', 'STATUS', 'UPDATED'],
259+
rungs.map((d) => [
260+
d.repository ?? 'account',
261+
d.config_name ?? '—',
262+
d.config_id,
263+
d.broken ? `broken: ${d.broken}` : 'ok',
264+
formatTs(d.updated_at),
265+
]),
266+
)
267+
})
268+
})
269+
270+
apiRoutes(
271+
defaults
272+
.command('set <config-id>')
273+
.description('Set the account default code review pipeline, or a repo default with --repo'),
274+
'PUT /v1/reviews/defaults',
275+
)
276+
.option(
277+
'-r, --repo [repository]',
278+
'target a repo rung: "owner/name", or no value for the repo you are standing in',
279+
)
280+
.option('--json', 'output raw JSON')
281+
.action(
282+
async (configId: string, opts: { repo?: string | boolean; json?: boolean }, cmd: Command) => {
283+
await runAction(async () => {
284+
const repository = resolveRepoFlag(opts.repo)
285+
const set = await new ApiClient().putReviewDefault({
286+
config_id: configId,
287+
...(repository ? { repository } : {}),
288+
})
289+
if (cmd.optsWithGlobals().json) {
290+
printJson(set)
291+
return
292+
}
293+
const rung = set.repository ? `default for ${set.repository}` : 'account default'
294+
console.log(`✓ set ${rung} to "${defaultName(set)}" (${set.config_id})`)
295+
})
296+
},
297+
)
298+
299+
apiRoutes(
300+
alsoKnownAs(
301+
defaults
302+
.command('clear')
303+
.description('Clear the account default code review pipeline, or a repo default with --repo'),
304+
'rm',
305+
'delete',
306+
),
307+
'DELETE /v1/reviews/defaults',
308+
)
309+
.option(
310+
'-r, --repo [repository]',
311+
'target a repo rung: "owner/name", or no value for the repo you are standing in',
312+
)
313+
.action(async (opts: { repo?: string | boolean }) => {
314+
await runAction(async () => {
315+
const repository = resolveRepoFlag(opts.repo)
316+
await new ApiClient().deleteReviewDefault(repository)
317+
console.log(`✓ cleared ${repository ? `default for ${repository}` : 'account default'}`)
318+
})
319+
})
320+
}
321+
322+
function defaultName(d: CodeReviewDefaultView): string {
323+
return d.config_name ?? d.config_id
324+
}
325+
326+
// A set-but-broken rung fails explicit reviews closed (never a silent run of
327+
// a different pipeline), so surface it wherever the rung is shown.
328+
function brokenSuffix(d: CodeReviewDefaultView): string {
329+
return d.broken ? ` (broken: ${d.broken})` : ''
171330
}
172331

173332
interface StartOptions {

src/lib/api.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
BudgetSummary,
1212
CliAuthPoll,
1313
CliAuthStart,
14+
CodeReviewDefaultView,
1415
CreateAgentConfigRequest,
1516
CreateAssetRequest,
1617
CreateAssetResponse,
@@ -32,6 +33,7 @@ import type {
3233
ListAgentTemplatesResponse,
3334
ListAssetsQuery,
3435
ListAssetsResponse,
36+
ListCodeReviewDefaultsResponse,
3537
ListGithubMembersResponse,
3638
ListGithubRepositoriesResponse,
3739
ListLinearTeamsResponse,
@@ -43,6 +45,7 @@ import type {
4345
ListSlackChannelsResponse,
4446
ListSlackMembersResponse,
4547
PutAgentDefaultRequest,
48+
PutCodeReviewDefaultRequest,
4649
ReplayAgentSessionRequest,
4750
Review,
4851
SendSessionMessageRequest,
@@ -362,6 +365,32 @@ export class ApiClient {
362365
return res.reviews
363366
}
364367

368+
// --------------------------- review defaults -----------------------------
369+
// The default code review pipeline ladder (repo default -> account default),
370+
// the code_review twin of the /v1/defaults methods below and addressed the
371+
// same way: `repository` is "owner/name" for a repo rung and null/omitted
372+
// for the account rung — never a row id. Read by POST /v1/reviews when no
373+
// config is named; webhook reviews are unaffected. Mutations are refused
374+
// for sandbox tokens (403).
375+
376+
async listReviewDefaults(): Promise<CodeReviewDefaultView[]> {
377+
const res = await this.request<ListCodeReviewDefaultsResponse>(
378+
'GET',
379+
'/v1/reviews/defaults',
380+
)
381+
return res.defaults
382+
}
383+
384+
putReviewDefault(req: PutCodeReviewDefaultRequest): Promise<CodeReviewDefaultView> {
385+
return this.request('PUT', '/v1/reviews/defaults', req)
386+
}
387+
388+
// Clears a rung: the account default when `repository` is omitted, that
389+
// repo's default otherwise. 404 when the rung isn't set.
390+
deleteReviewDefault(repository?: string): Promise<void> {
391+
return this.request('DELETE', '/v1/reviews/defaults', undefined, { repository })
392+
}
393+
365394
// ----------------------------- agent configs ----------------------------
366395

367396
async listAgentConfigs(): Promise<SavedAgentConfig[]> {

src/lib/types.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,34 @@ export interface PutAgentDefaultRequest {
305305
config_id: string
306306
}
307307

308+
// One rung of the default code review pipeline ladder (GET
309+
// /v1/reviews/defaults) — the code_review twin of AgentDefaultView, pointing
310+
// at a synced `kind: code_review` pipeline (crcfg_…) instead of an agent
311+
// config. Read by POST /v1/reviews when no config is named: repo default ->
312+
// account default -> the oldest synced pipeline -> the platform defaults.
313+
export interface CodeReviewDefaultView {
314+
id: string
315+
repository: string | null
316+
config_id: string
317+
// The pointed-at pipeline's name; null when the pipeline is gone (see broken).
318+
config_name: string | null
319+
// Why this rung can't serve reviews (config_deleted | config_disabled |
320+
// config_sync_error | repo_inaccessible); null when healthy.
321+
broken: string | null
322+
updated_at: string
323+
}
324+
325+
export interface ListCodeReviewDefaultsResponse {
326+
defaults: CodeReviewDefaultView[]
327+
}
328+
329+
// Body of PUT /v1/reviews/defaults: point a rung at a pipeline. `repository`
330+
// omitted sets the account default; "owner/name" sets that repo's default.
331+
export interface PutCodeReviewDefaultRequest {
332+
repository?: string
333+
config_id: string
334+
}
335+
308336
// Create-config payload for POST /v1/configs. Exactly one of `config` (inline)
309337
// or `template_id` (a gallery template slug). `repository` is a bare repo name
310338
// in the caller's account — the owner is always the account.

test/api.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,67 @@ describe('ApiClient sandbox variables', () => {
192192
})
193193
})
194194

195+
describe('ApiClient review defaults', () => {
196+
afterEach(() => vi.unstubAllGlobals())
197+
198+
const rung = {
199+
id: 'crdef_1',
200+
repository: null,
201+
config_id: 'crcfg_1',
202+
config_name: 'Team reviewer',
203+
broken: null,
204+
updated_at: '',
205+
}
206+
207+
it('lists rungs and unwraps the response envelope', async () => {
208+
const fetchMock = vi.fn(
209+
async () => new Response(JSON.stringify({ defaults: [rung] }), { status: 200 }),
210+
)
211+
vi.stubGlobal('fetch', fetchMock)
212+
213+
const out = await new ApiClient('http://api.test', 't').listReviewDefaults()
214+
expect(out).toEqual([rung])
215+
expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/v1/reviews/defaults')
216+
expect((fetchMock.mock.calls[0][1] as RequestInit).method).toBe('GET')
217+
})
218+
219+
it('PUTs the rung: account when repository is omitted, repo when named', async () => {
220+
const fetchMock = vi.fn(async () => new Response(JSON.stringify(rung), { status: 200 }))
221+
vi.stubGlobal('fetch', fetchMock)
222+
223+
await new ApiClient('http://api.test', 't').putReviewDefault({ config_id: 'crcfg_1' })
224+
let [url, init] = fetchMock.mock.calls[0]
225+
expect(url).toBe('http://api.test/v1/reviews/defaults')
226+
expect((init as RequestInit).method).toBe('PUT')
227+
expect(JSON.parse((init as RequestInit).body as string)).toEqual({ config_id: 'crcfg_1' })
228+
229+
await new ApiClient('http://api.test', 't').putReviewDefault({
230+
config_id: 'crcfg_1',
231+
repository: 'acme/api',
232+
})
233+
;[url, init] = fetchMock.mock.calls[1]
234+
expect(JSON.parse((init as RequestInit).body as string)).toEqual({
235+
config_id: 'crcfg_1',
236+
repository: 'acme/api',
237+
})
238+
})
239+
240+
it('clears a rung via query param, omitted for the account rung', async () => {
241+
const fetchMock = vi.fn(async () => new Response(null, { status: 204 }))
242+
vi.stubGlobal('fetch', fetchMock)
243+
244+
const api = new ApiClient('http://api.test', 't')
245+
await api.deleteReviewDefault()
246+
expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/v1/reviews/defaults')
247+
expect((fetchMock.mock.calls[0][1] as RequestInit).method).toBe('DELETE')
248+
249+
await api.deleteReviewDefault('acme/api')
250+
expect(fetchMock.mock.calls[1][0]).toBe(
251+
'http://api.test/v1/reviews/defaults?repository=acme%2Fapi',
252+
)
253+
})
254+
})
255+
195256
describe('getSessionLog', () => {
196257
afterEach(() => vi.unstubAllGlobals())
197258

0 commit comments

Comments
 (0)