Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/commands/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
161 changes: 160 additions & 1 deletion src/commands/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand Down Expand Up @@ -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 <config-id>')
.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 {
Expand Down
29 changes: 29 additions & 0 deletions src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
BudgetSummary,
CliAuthPoll,
CliAuthStart,
CodeReviewDefaultView,
CreateAgentConfigRequest,
CreateAssetRequest,
CreateAssetResponse,
Expand All @@ -32,6 +33,7 @@ import type {
ListAgentTemplatesResponse,
ListAssetsQuery,
ListAssetsResponse,
ListCodeReviewDefaultsResponse,
ListGithubMembersResponse,
ListGithubRepositoriesResponse,
ListLinearTeamsResponse,
Expand All @@ -43,6 +45,7 @@ import type {
ListSlackChannelsResponse,
ListSlackMembersResponse,
PutAgentDefaultRequest,
PutCodeReviewDefaultRequest,
ReplayAgentSessionRequest,
Review,
SendSessionMessageRequest,
Expand Down Expand Up @@ -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<CodeReviewDefaultView[]> {
const res = await this.request<ListCodeReviewDefaultsResponse>(
'GET',
'/v1/reviews/defaults',
)
return res.defaults
}

putReviewDefault(req: PutCodeReviewDefaultRequest): Promise<CodeReviewDefaultView> {
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<void> {
return this.request('DELETE', '/v1/reviews/defaults', undefined, { repository })
}

// ----------------------------- agent configs ----------------------------

async listAgentConfigs(): Promise<SavedAgentConfig[]> {
Expand Down
28 changes: 28 additions & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
61 changes: 61 additions & 0 deletions test/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down