diff --git a/README.md b/README.md index ac92c91..3a83ef9 100644 --- a/README.md +++ b/README.md @@ -68,11 +68,9 @@ agent session connect # inside an Ellipsis sandbox: connects to agent session stop # stop an in-flight session agent review 123 # review a pull request now, instead of waiting for a push -agent review # review the work in your tree; findings print here +agent review get # a review's findings, scope, and whether it posted agent review list --repo api # list a repository's reviews, newest first -agent review init [path] # scaffold a starter review pipeline (default: agents/code_review.yaml) -agent review default # the effective review pipeline for the repo you are standing in -agent review default set # set the account default pipeline (--repo [owner/name] for one repo) +agent review init # scaffold a starter review pipeline (code_review.yaml) agent config list # list saved agent configs agent config get # show one config as YAML (--json for JSON) @@ -92,10 +90,10 @@ agent slack members # workspace members, with linked GitHub identi agent linear teams # teams in the connected Linear organization agent sentry orgs # connected Sentry organizations -agent asset upload shot.png # store a PNG; prints an org-gated link to paste into a PR comment -agent asset list # list stored assets (--session scopes to one run's uploads) -agent asset get -o shot.png # show one asset, or download its bytes with -o -agent asset delete # delete an asset (it disappears from list/get and its link stops resolving) +agent file upload shot.png # store a PNG; prints an org-gated link to paste into a PR comment +agent file list # list stored files (--session scopes to one run's uploads) +agent file get -o shot.png # show one file, or download its bytes with -o +agent file delete # delete a file (it disappears from list/get and its link stops resolving) agent variable list # list sandbox env variable names (values are write-only) agent variable set A=1 B=2 # create/update variables (or --from-file .env/.json) @@ -110,7 +108,7 @@ agent analytics review --repo my-service # review totals + top reviewers agent ping # check authenticated API connectivity ``` -Every command shown is singular. The plural spelling of each (`agent assets`, +Every command shown is singular. The plural spelling of each (`agent files`, `agent sessions`, `agent analytics prs`) is a hidden alias that works but is left out of `--help`. See [`skills/cli-conventions`](skills/cli-conventions/SKILL.md) for the full diff --git a/bun.lock b/bun.lock index daaa242..480cb52 100644 --- a/bun.lock +++ b/bun.lock @@ -5,7 +5,7 @@ "": { "name": "@ellipsis/cli", "dependencies": { - "@ellipsis-dev/sdk": "^0.5.0", + "@ellipsis-dev/sdk": "^0.6.0", "chalk": "^5.6.2", "cli-table3": "^0.6.5", "commander": "^12.1.0", @@ -35,7 +35,7 @@ "@colors/colors": ["@colors/colors@1.5.0", "", {}, "sha512-ooWCrlZP11i8GImSjTHYHLkvFDP48nS4+204nGb1RiX/WXYHmJA2III9/e2DWVabCESdW7hBAEzHRqUn9OUVvQ=="], - "@ellipsis-dev/sdk": ["@ellipsis-dev/sdk@0.5.0", "", {}, "sha512-14g9g9Mrt10hfoPpaqeIqZsr8vsDYkBtIwdkd6Rz/3Izs5pvtVm4zys5znRL9UH8dC+R02B1Ljz2AGKGrtEEsQ=="], + "@ellipsis-dev/sdk": ["@ellipsis-dev/sdk@0.6.0", "", {}, "sha512-FEZVXzM+TJ7YrDnJaQu4G+cxpX/49wFbtQelldB/FowX7uwi9X13rY0Crv6Oxox+uJiise7TeK9AIAyfZbYv0Q=="], "@esbuild/aix-ppc64": ["@esbuild/aix-ppc64@0.27.7", "", { "os": "aix", "cpu": "ppc64" }, "sha512-EKX3Qwmhz1eMdEJokhALr0YiD0lhQNwDqkPYyPhiSwKrh7/4KRjQc04sZ8db+5DVVnZ1LmbNDI1uAMPEUBnQPg=="], diff --git a/package.json b/package.json index 1984a10..ddece9f 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "test:watch": "vitest" }, "dependencies": { - "@ellipsis-dev/sdk": "^0.5.0", + "@ellipsis-dev/sdk": "^0.6.0", "chalk": "^5.6.2", "cli-table3": "^0.6.5", "commander": "^12.1.0", diff --git a/src/cli.tsx b/src/cli.tsx index 7b4200e..be79329 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -7,7 +7,7 @@ import { registerSession } from './commands/session' import { registerReview } from './commands/review' import { registerConfig } from './commands/config' import { registerVariable } from './commands/variable' -import { registerAsset } from './commands/asset' +import { registerFile } from './commands/file' import { registerHook } from './commands/hooks' import { registerTemplate } from './commands/template' import { registerModel } from './commands/model' @@ -44,7 +44,7 @@ registerSession(program) registerReview(program) registerConfig(program) registerVariable(program) -registerAsset(program) +registerFile(program) registerHook(program) registerTemplate(program) registerModel(program) diff --git a/src/commands/config.ts b/src/commands/config.ts index 01f2529..dadd318 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -29,7 +29,7 @@ export function registerConfig(program: Command): void { config.command('list').description('List your saved agent configs'), 'ls', ), - 'GET /configs', + 'GET /agents/configs', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { @@ -59,7 +59,7 @@ export function registerConfig(program: Command): void { config .command('get ') .description('Print one agent config as YAML, or as JSON with --json'), - 'GET /configs/{id}', + 'GET /agents/configs/{id}', ) .option('--json', 'output raw JSON') .action(async (configId: string, opts: { json?: boolean }) => { @@ -85,7 +85,7 @@ export function registerConfig(program: Command): void { config .command('create') .description('Create an agent config by opening a pull request that adds it to a repo'), - 'POST /configs', + 'POST /agents/configs', ) .requiredOption( '-r, --repo ', @@ -148,7 +148,7 @@ export function registerConfig(program: Command): void { .description('Show or set which agent config runs when a session names none'), 'defaults', ), - 'GET /defaults', + 'GET /agents/defaults', ) .option('--json', 'output raw JSON') // Bare `agent config default`: the effective default for the repo you're @@ -189,7 +189,7 @@ export function registerConfig(program: Command): void { .description('List every default that is set, account rung and per-repo rungs'), 'ls', ), - 'GET /defaults', + 'GET /agents/defaults', ) .option('--json', 'output raw JSON') // The group also defines --json (for the bare view), and commander parses @@ -223,7 +223,7 @@ export function registerConfig(program: Command): void { defaults .command('set ') .description('Set the account default agent config, or a repo default with --repo'), - 'PUT /defaults', + 'PUT /agents/defaults', ) .option( '-r, --repo [repository]', @@ -256,7 +256,7 @@ export function registerConfig(program: Command): void { 'rm', 'delete', ), - 'DELETE /defaults', + 'DELETE /agents/defaults', ) .option( '-r, --repo [repository]', @@ -278,7 +278,7 @@ export function registerConfig(program: Command): void { .description( `Scaffold a starter agent config YAML locally (default: ${DEFAULT_CONFIG_PATH})`, ), - 'POST /configs with --template', + 'POST /agents/configs with --template', ) // No `-f` short: CLI-wide, `-f` means an input file (see `config create`). .option('--force', 'overwrite the file if it already exists') diff --git a/src/commands/asset.ts b/src/commands/file.ts similarity index 68% rename from src/commands/asset.ts rename to src/commands/file.ts index 2da9ff2..fa16300 100644 --- a/src/commands/asset.ts +++ b/src/commands/file.ts @@ -4,20 +4,19 @@ import { basename } from 'node:path' import { ApiClient, ApiError } from '../lib/api' import { alsoKnownAs, apiRoutes } from '../lib/help' import { formatTs, printJson, printTable, runAction } from '../lib/output' -import type { AssetView, CreateAssetRequest, GetAssetResponse } from '../lib/types' +import type { CreateFileRequest, FileView, GetFileResponse } from '../lib/types' -// `agent asset `: persist files to Ellipsis platform storage and get -// back an org-membership-gated link (documents/eng/AGENT_ASSET_STORAGE.md in -// the ellipsis repo). The primary caller is an agent inside a sandbox that -// took a screenshot of a UI change and wants a link to paste into a PR -// comment — the injected sandbox token authenticates it with zero setup, and -// the same commands work on a laptop with a device-login token. +// `agent file `: persist files to Ellipsis platform storage and get back +// an org-membership-gated link. The primary caller is an agent inside a sandbox +// that took a screenshot of a UI change and wants a link to paste into a PR +// comment: the injected sandbox token authenticates it with zero setup, and the +// same commands work on a laptop with a device-login token. -// Client-side mirrors of the server limits (assets_service.py), so an +// Client-side mirrors of the server limits (files_service.py), so an // oversized or non-PNG file fails fast with a clear message instead of a // base64-inflated round trip to a 400. The server re-validates; these are // UX, not enforcement. -export const MAX_ASSET_SIZE_BYTES = 10 * 1024 * 1024 +export const MAX_FILE_SIZE_BYTES = 10 * 1024 * 1024 const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) // Well-known magic bytes we can name in the "not a PNG" error, so an agent @@ -45,12 +44,12 @@ function sniffFormat(bytes: Buffer): string | null { // Build the upload request from a file's bytes, throwing the fast client-side // errors (empty, oversized, not a PNG). Exported for tests. -export function buildUploadRequest(path: string, bytes: Buffer): CreateAssetRequest { +export function buildUploadRequest(path: string, bytes: Buffer): CreateFileRequest { if (bytes.length === 0) throw new Error(`${path} is empty`) - if (bytes.length > MAX_ASSET_SIZE_BYTES) { + if (bytes.length > MAX_FILE_SIZE_BYTES) { throw new Error( `${path} is ${formatSize(bytes.length)}; the limit is ` + - `${formatSize(MAX_ASSET_SIZE_BYTES)} per asset`, + `${formatSize(MAX_FILE_SIZE_BYTES)} per file`, ) } if (!bytes.subarray(0, PNG_MAGIC.length).equals(PNG_MAGIC)) { @@ -75,25 +74,27 @@ export function formatSize(bytes: number): string { return `${(bytes / (1024 * 1024)).toFixed(1)} MiB` } -export function registerAsset(program: Command): void { - const asset = alsoKnownAs( +export function registerFile(program: Command): void { + const file = alsoKnownAs( program - .command('asset') + .command('file') .description('Store files on the platform and share them as org-gated links'), + 'files', + 'asset', 'assets', ) apiRoutes( - asset + file .command('upload ') .description('Upload a PNG and print its org-gated URL, ready to paste into a PR comment'), - 'POST /assets', + 'POST /files', ) .option('--json', 'output raw JSON') .action(async (path: string, opts: { json?: boolean }) => { await runAction(async () => { const req = buildUploadRequest(path, readFileSync(path)) - const res = await new ApiClient().uploadAsset(req) + const res = await new ApiClient().uploadFile(req) // The URL is the whole point — keep it the bare primary output so an // agent (or $(...) in a script) can capture it directly. if (opts.json) printJson(res) @@ -102,87 +103,82 @@ export function registerAsset(program: Command): void { }) apiRoutes( - alsoKnownAs( - asset.command('list').description('List your stored assets, newest first'), - 'ls', - ), - 'GET /assets', + alsoKnownAs(file.command('list').description('List your stored files, newest first'), 'ls'), + 'GET /files', ) - .option('--session ', 'only assets uploaded by this agent session') + .option('--session ', 'only files uploaded by this agent session') .option('-l, --limit ', 'max results (server cap: 250)', parsePositiveInt) .option('--json', 'output raw JSON') .action(async (opts: { session?: string; limit?: number; json?: boolean }) => { await runAction(async () => { - const assets = await new ApiClient().listAssets({ + const files = await new ApiClient().listFiles({ agent_session_id: opts.session, limit: opts.limit, }) if (opts.json) { - printJson(assets) + printJson(files) return } - if (assets.length === 0) { - console.log('No assets.') + if (files.length === 0) { + console.log('No files.') return } printTable( ['ID', 'FILENAME', 'SIZE', 'CREATED', 'SESSION'], - assets.map((a) => [ - a.id, - a.filename, - formatSize(a.size_bytes), - formatTs(a.created_at), - a.agent_session_id ?? '-', + files.map((f) => [ + f.id, + f.filename, + formatSize(f.size_bytes), + formatTs(f.created_at), + f.agent_session_id ?? '-', ]), ) }) }) apiRoutes( - asset - .command('get ') - .description("Print one asset's metadata, or download its bytes with -o"), - 'GET /assets/{id}', + file + .command('get ') + .description("Print one file's metadata, or download its bytes with -o"), + 'GET /files/{id}', 'presigned S3 GET', ) .option('-o, --output ', 'write the file contents to this path') .option('--json', 'output raw JSON (includes the short-lived download_url)') - .action(async (assetId: string, opts: { output?: string; json?: boolean }) => { + .action(async (fileId: string, opts: { output?: string; json?: boolean }) => { await runAction(async () => { - const res = await new ApiClient().getAsset(assetId) + const res = await new ApiClient().getFile(fileId) if (opts.output) { // download_url is a ~60s presigned S3 GET — fetch it immediately, // while it's fresh. The JSON API never carries the bytes itself. await downloadTo(res.download_url, opts.output) if (!opts.json) { - console.log(`✓ wrote ${opts.output} (${formatSize(res.asset.size_bytes)})`) + console.log(`✓ wrote ${opts.output} (${formatSize(res.file.size_bytes)})`) } } if (opts.json) printJson(res) - else if (!opts.output) renderAsset(res) + else if (!opts.output) renderFile(res) }) }) apiRoutes( alsoKnownAs( - asset - .command('delete ') - .description('Delete an asset, so its link stops resolving'), + file.command('delete ').description('Delete a file, so its link stops resolving'), 'rm', ), - 'DELETE /assets/{id}', + 'DELETE /files/{id}', ) .option('--json', 'output raw JSON') - .action(async (assetId: string, opts: { json?: boolean }) => { + .action(async (fileId: string, opts: { json?: boolean }) => { await runAction(async () => { try { - await new ApiClient().deleteAsset(assetId) + await new ApiClient().deleteFile(fileId) } catch (err) { // A 404 covers "never existed", "someone else's", and "already // deleted" — all the same "there's nothing here to delete" to the // caller, so give one clear message instead of the raw HTTP error. if (err instanceof ApiError && err.status === 404) { - throw new Error(`asset not found: ${assetId}`) + throw new Error(`file not found: ${fileId}`) } // A 403 is a real policy decision (e.g. sandbox tokens can't delete); // surface the server's own explanation rather than masking it. @@ -192,26 +188,26 @@ export function registerAsset(program: Command): void { throw err } // 204 No Content — nothing to echo, so confirm with the id. - if (opts.json) printJson({ id: assetId, deleted: true }) - else console.log(`✓ deleted ${assetId}`) + if (opts.json) printJson({ id: fileId, deleted: true }) + else console.log(`✓ deleted ${fileId}`) }) }) } -function renderAsset(res: GetAssetResponse): void { - const a: AssetView = res.asset - console.log(`id: ${a.id}`) - console.log(`filename: ${a.filename}`) - console.log(`type: ${a.content_type}`) - console.log(`size: ${formatSize(a.size_bytes)}`) - console.log(`created: ${formatTs(a.created_at)}`) - if (a.agent_session_id) console.log(`session: ${a.agent_session_id}`) +function renderFile(res: GetFileResponse): void { + const f: FileView = res.file + console.log(`id: ${f.id}`) + console.log(`filename: ${f.filename}`) + console.log(`type: ${f.content_type}`) + console.log(`size: ${formatSize(f.size_bytes)}`) + console.log(`created: ${formatTs(f.created_at)}`) + if (f.agent_session_id) console.log(`session: ${f.agent_session_id}`) console.log(`url: ${res.url}`) - console.log(`\ndownload the file with: agent asset get ${a.id} -o ${a.filename}`) + console.log(`\ndownload the file with: agent file get ${f.id} -o ${f.filename}`) } // Pull the bytes from the presigned S3 URL. Deliberately bare fetch (no -// bearer header — the signature in the URL is the credential). Assets are +// bearer header — the signature in the URL is the credential). Files are // ≤10 MiB, so buffering in memory is fine. async function downloadTo(url: string, path: string): Promise { const res = await fetch(url) diff --git a/src/commands/github.ts b/src/commands/github.ts index d121378..3536427 100644 --- a/src/commands/github.ts +++ b/src/commands/github.ts @@ -18,7 +18,7 @@ export function registerGithub(program: Command): void { .description('List the repositories the GitHub installation can reach'), 'repo', ), - 'GET /github/repos', + 'GET /integrations/github/repos', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { @@ -53,7 +53,7 @@ export function registerGithub(program: Command): void { ), 'member', ), - 'GET /github/members', + 'GET /integrations/github/members', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { diff --git a/src/commands/linear.ts b/src/commands/linear.ts index 842cf15..49af586 100644 --- a/src/commands/linear.ts +++ b/src/commands/linear.ts @@ -15,7 +15,7 @@ export function registerLinear(program: Command): void { .description('List the Linear teams, marking which have Ellipsis enabled'), 'team', ), - 'GET /linear/teams', + 'GET /integrations/linear/teams', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { diff --git a/src/commands/review.ts b/src/commands/review.ts index bd314e1..d62c415 100644 --- a/src/commands/review.ts +++ b/src/commands/review.ts @@ -3,44 +3,38 @@ import { existsSync, mkdirSync, writeFileSync } from 'node:fs' import { basename, dirname, extname } from 'node:path' import { ApiClient, ApiError } from '../lib/api' import { alsoKnownAs, apiRoutes } from '../lib/help' -import { createWipCommit, currentBranch, pushReviewBranch, repoFromCwd } from '../lib/laptop' +import { repoFromCwd } from '../lib/laptop' import { formatTs, printJson, printTable, relativeAge, runAction, usdFromMillicents } from '../lib/output' -import { resolveRepoFlag } from './config' import { watchSessionStreaming } from './session' -import type { - CodeReviewDefaultView, - CreateReviewRequest, - Finding, - Review, - ReviewScope, -} from '../lib/types' +import type { 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 — +// trigger one. // -// agent review 5975 an existing pull request -// agent review the work in your tree, right now +// agent review 5975 review that pull request // -// The second is the interesting one: it snapshots your working tree, pushes it -// to a sidecar branch (never your own), and the platform finds-or-creates a -// draft PR to review it against — because a code review is structurally a PR -// review (the range, the checkout, and the delivery all read PR state). Those -// reviews are terminal-only: nothing is posted to GitHub, findings print here. +// A review is always of an existing pull request: the range, the checkout, and +// the delivery all read PR state, so there is nothing to review without one. // -// A review IS a session under the hood, and a review id IS a session id — so -// `agent session get `, `--watch`, records, and stop all work on it. +// Which pipeline runs is not a parameter. It is resolved from the repository's +// committed `code_review.yaml` (see `agent review init`), the same way an +// automatic review resolves it. +// +// A review is a pipeline of stage sessions, not a single session: its id is a +// `crun_…`, and each stage's session id lives in `stages[]`. // How long to wait between REST polls when the stream isn't available. const FALLBACK_POLL_INTERVAL_SECONDS = 3 -// The conventional path for a pipeline file. Convention only: `kind:` decides. -const DEFAULT_PIPELINE_PATH = 'agents/code_review.yaml' +// The only paths a committed pipeline may live at, in the precedence order the +// server resolves them (CODE_REVIEW_CONFIG_PATHS). A file anywhere else is a +// hard sync error, never a silently-unused config. +const DEFAULT_PIPELINE_PATH = 'code_review.yaml' +const NESTED_PIPELINE_PATH = '.ellipsis/code_review.yaml' export function registerReview(program: Command): void { const review = alsoKnownAs( - program - .command('review') - .description('Review a pull request, or the work in your tree, on demand'), + program.command('review').description('Review a pull request on demand'), 'reviews', 'code-review', 'cr', @@ -48,35 +42,24 @@ export function registerReview(program: Command): void { apiRoutes( review - .command('start [pull-request]', { isDefault: true }) - .description('Review a pull request by number, or your working tree if you omit one'), + .command('start ', { isDefault: true }) + .description('Review a pull request by number'), 'POST /reviews', 'WS /sessions/{id}/stream', 'GET /reviews/{id}', ) .option('--repo ', 'repository to review (default: this git remote)') - .option( - '--branch ', - 'review an already-pushed branch instead of snapshotting your tree', - ) .option('--full', 're-review the whole pull request, not just the new commits') .option('--watermark ', 'pin the range start (a commit SHA)') .option('--head ', 'pin the range end (a commit SHA)') - .option('-c, --config ', 'run a saved agent config instead of the built-in reviewer') - .option('-m, --model ', 'override the reviewer model (see `agent model list`)') - .option('-b, --budget ', 'override the budget for this review', parseUsd) .option('--no-post', 'do not post to GitHub; print the findings here instead') .option('--no-wait', 'print the review id and exit instead of waiting for findings') .option('--cwd ', 'repository directory (default: current directory)') .option('--json', 'output raw JSON') - .action(async (pullRequest: string | undefined, opts: StartOptions) => { + .action(async (pullRequest: string, opts: StartOptions) => { await runAction(async () => { const api = new ApiClient() const request = buildCreateRequest(pullRequest, opts) - const local = request.branch !== undefined - if (!opts.json && local) { - console.log(`✓ pushed ${request.sha?.slice(0, 12)} to ${request.branch}`) - } const started = await api.createReview(request) // Nothing new since the last review of this PR. Not an error — you @@ -103,7 +86,7 @@ export function registerReview(program: Command): void { // Block-and-stream, then re-read: the findings are collected from the // sandbox at teardown, so they only exist once the review finalizes. - // Same two-step `agent asset get` uses. + // Same two-step `agent file get` uses. if (!opts.json) { console.log( `✓ reviewing ${request.owner}/${request.repo}#${started.pull_request.number} ` + @@ -182,7 +165,6 @@ export function registerReview(program: Command): void { }) registerReviewInit(review) - registerReviewDefaults(review) } // `agent review init`: the code review twin of `agent config init`. Scaffolds a @@ -199,14 +181,23 @@ function registerReviewInit(review: Command): void { .option('--force', 'overwrite the file if it already exists') .action((path: string | undefined, opts: { force?: boolean }) => { const target = path ?? DEFAULT_PIPELINE_PATH + // Refuse a path the server would reject at sync: writing a file that can + // never run is worse than not writing one, because it looks like it works. + if (target !== DEFAULT_PIPELINE_PATH && target !== NESTED_PIPELINE_PATH) { + console.error( + `error: a code review pipeline must live at '${DEFAULT_PIPELINE_PATH}' or ` + + `'${NESTED_PIPELINE_PATH}'; '${target}' is never used`, + ) + process.exitCode = 1 + return + } if (existsSync(target) && !opts.force) { console.error(`error: ${target} already exists (use --force to overwrite)`) process.exitCode = 1 return } - const name = basename(target, extname(target)) mkdirSync(dirname(target), { recursive: true }) - writeFileSync(target, starterPipeline(name, repoNameFromCwd())) + writeFileSync(target, starterPipeline(pipelineName())) console.log(`✓ wrote ${target}`) console.log( 'Commit it to your default branch. Ellipsis syncs code review pipelines from GitHub.', @@ -214,40 +205,42 @@ function registerReviewInit(review: Command): void { }) } -// The repository this pipeline should watch, as the bare name the schema takes -// (`repositories:` is scoped to your account, so it never carries the owner). -function repoNameFromCwd(): string | undefined { +// Both legal paths share one filename, so the file can't name the pipeline. +// Use the repository instead, falling back to the filename outside a checkout. +function pipelineName(): string { const repo = repoFromCwd(process.cwd()) - return repo ? repo.split('/')[1] : undefined + return repo ? `${repo.split('/')[1]} code review` : 'code review' } // A minimal valid pipeline. `ellipsis.kind` is the only field the schema // requires; every stage left unset runs the platform's default reviewers. -// Exported for tests. -export function starterPipeline(name: string, repository: string | undefined): string { +// +// Deliberately omits `pull_requests.repositories`: a file's location IS its +// scope now, so naming repositories is a sync error everywhere except the +// org-wide copy in the `.ellipsis` repository. Exported for tests. +export function starterPipeline(name: string): string { return `# Ellipsis code review pipeline. Commit this to your default branch; Ellipsis -# syncs it from GitHub. Valid locations: agents/, .agents/, ellipsis/, .ellipsis/ -# (any depth). The kind line is what makes this a review pipeline, not an agent. +# syncs it from GitHub. It must live at '${DEFAULT_PIPELINE_PATH}' or +# '${NESTED_PIPELINE_PATH}'; a pipeline anywhere else is never used. +# +# Where it sits decides what it reviews: in a normal repository it reviews that +# repository, and in your organization's '.ellipsis' repository it reviews every +# repository. The kind line is what makes this a review pipeline, not an agent. ellipsis: version: v1 kind: code_review name: ${name} description: What this pipeline reviews. -# Which pull requests this pipeline watches. Only one enabled pipeline may watch -# a given pull request, so name your repositories here. Leaving this out watches -# every repository in the account. -pull_requests: - repositories: - - ${repository ?? 'my-repo'} - # base: [main] - # draft: false - # paths: ["src/**"] - # for: { bots: false } +# Which pull requests to review. Omit this to review every pull request. +# pull_requests: +# base: [main] +# draft: false +# paths: ["src/**"] +# for: { bots: false } # Every stage is optional. With all of them unset, reviews run the platform -# default pipeline: three reviewer lenses (correctness, security, regression) -# and a gatekeeper that judges what they found. +# default pipeline: a pull request description writer plus one bug reviewer. # # review: # - name: migration-safety @@ -257,7 +250,8 @@ pull_requests: # pull_requests: # paths: ["sql/migrations/**"] # -# include_default_reviewers: true # run the built-in lenses as well +# Declaring a filter stage adds a gatekeeper that judges what the reviewers +# found. There is no gatekeeper unless you declare one. # # filter: # name: gatekeeper @@ -272,165 +266,11 @@ budget: ` } -// `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 /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 /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 /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 /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 /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 { repo?: string - branch?: string full?: boolean watermark?: string head?: string - config?: string - model?: string - budget?: number // commander sets these false for --no-* flags. post: boolean wait: boolean @@ -446,41 +286,20 @@ interface ListOptions { json?: boolean } -// Assemble the request, doing the local path's git work when no pull request -// was named. Exported for tests. +// Assemble the request. Exported for tests. export function buildCreateRequest( - pullRequest: string | undefined, + pullRequest: string, opts: StartOptions, ): CreateReviewRequest { const cwd = opts.cwd ?? process.cwd() const repo = splitRepo(opts.repo ?? repoFromCwdOrThrow(cwd)) - const scope = buildScope(opts) - - const common = { + return { owner: repo.owner, repo: repo.name, - scope, - // The generated type requires every field the server defaults, so send the - // empty default rather than omitting it. - metadata: {}, - ...(opts.config ? { config_id: opts.config } : {}), - ...(opts.model ? { model: opts.model } : {}), - ...(opts.budget !== undefined ? { budget: opts.budget } : {}), + scope: buildScope(opts), + pull_request_number: parsePullRequest(pullRequest), + post: opts.post, } - - if (pullRequest !== undefined) { - return { ...common, pull_request_number: parsePullRequest(pullRequest), post: opts.post } - } - - // The local path. Snapshot the tree and push it to a sidecar branch, so the - // branch you are working on is never touched. ALWAYS terminal-only — the - // pull request being reviewed is one the platform manufactured for the - // purpose, so commenting on it would be talking to itself. Findings print - // here instead. (`post` has no opt-in flag; --no-post only turns it off for - // a real pull request.) - const branch = opts.branch ?? sidecarFor(cwd) - const sha = opts.branch ? undefined : pushSnapshot(cwd, branch) - return { ...common, branch, ...(sha ? { sha } : {}), post: false } } // `--full`/`--watermark`/`--head` → the scope model. Default is incremental: @@ -493,35 +312,15 @@ function buildScope(opts: StartOptions): ReviewScope { } } -function sidecarFor(cwd: string): string { - const branch = currentBranch(cwd) - if (!branch) { - throw new Error( - 'detached HEAD: check out a branch, or name a pull request (`agent review 123`)', - ) - } - return `ellipsis/review/${branch}` -} - -// Snapshot the working tree and force-push it. Returns the pushed SHA, which -// pins the review's range end — GitHub's reported PR head lags a force-push. -function pushSnapshot(cwd: string, branch: string): string { - const { sha } = createWipCommit(cwd) - const realBranch = currentBranch(cwd) - if (!realBranch) throw new Error('detached HEAD: check out a branch first') - pushReviewBranch(cwd, sha, realBranch) - return sha -} - async function getReviewOrExplain(api: ApiClient, reviewId: string): Promise { try { return await api.getReview(reviewId) } catch (err) { - // A review id IS a session id, so the most likely mistake is handing this - // the id of a session that isn't a review — indistinguishable from an + // The likeliest mistake is handing this a stage session id (or any other + // session id) instead of the review's own — indistinguishable from an // unknown id server-side, on purpose. if (err instanceof ApiError && err.status === 404) { - throw new Error(`no review with id ${reviewId} (a review id looks like session_…)`) + throw new Error(`no review with id ${reviewId} (a review id looks like crun_…)`) } throw err } @@ -617,9 +416,9 @@ export function parsePullRequest(raw: string): number { // `review` reserves the word, so `agent review the auth changes` lands // here rather than starting a session with that prompt. Name the fix. throw new Error( - `'${raw}' is not a pull request number. Pass a number (agent review 123), ` + - 'or omit it to review your working tree. To run an agent with a prompt ' + - `that starts with "review", quote it: agent "review ${raw} …"`, + `'${raw}' is not a pull request number. Pass a number (agent review 123). ` + + 'To run an agent with a prompt that starts with "review", quote it: ' + + `agent "review ${raw} …"`, ) } return Number.parseInt(match[1], 10) @@ -630,9 +429,3 @@ function parsePositiveInt(raw: string): number { if (!Number.isFinite(n) || n <= 0) throw new Error(`invalid count '${raw}'`) return n } - -function parseUsd(raw: string): number { - const n = Number.parseFloat(raw) - if (!Number.isFinite(n) || n < 0) throw new Error(`invalid budget '${raw}'`) - return n -} diff --git a/src/commands/sentry.ts b/src/commands/sentry.ts index 31af405..f182269 100644 --- a/src/commands/sentry.ts +++ b/src/commands/sentry.ts @@ -14,7 +14,7 @@ export function registerSentry(program: Command): void { 'org', 'organizations', ), - 'GET /sentry/organizations', + 'GET /integrations/sentry/organizations', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { diff --git a/src/commands/session.tsx b/src/commands/session.tsx index c863e29..c187bbe 100644 --- a/src/commands/session.tsx +++ b/src/commands/session.tsx @@ -326,7 +326,7 @@ export function registerSession(program: Command): void { '--since/--until accept ISO 8601 or "today", "yesterday", "N days ago".', ), 'GET /sessions', - 'GET /github/members to resolve --author', + 'GET /integrations/github/members to resolve --author', ) .option('-c, --config ', 'only sessions run by this saved agent config') .option( @@ -402,7 +402,7 @@ export function registerSession(program: Command): void { '--since/--until accept ISO 8601 or "today", "yesterday", "N days ago".', ), 'GET /sessions/search', - 'GET /github/members to resolve --author', + 'GET /integrations/github/members to resolve --author', ) .option( '-a, --author ', diff --git a/src/commands/slack.ts b/src/commands/slack.ts index 4d289ff..6eb0526 100644 --- a/src/commands/slack.ts +++ b/src/commands/slack.ts @@ -13,7 +13,7 @@ export function registerSlack(program: Command): void { slack.command('channels').description('List the channels in the Slack workspace'), 'channel', ), - 'GET /slack/channels', + 'GET /integrations/slack/channels', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { @@ -46,7 +46,7 @@ export function registerSlack(program: Command): void { .description('List the workspace members, with linked GitHub identities'), 'member', ), - 'GET /slack/members', + 'GET /integrations/slack/members', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { diff --git a/src/commands/template.ts b/src/commands/template.ts index 65b7748..b541eb9 100644 --- a/src/commands/template.ts +++ b/src/commands/template.ts @@ -16,7 +16,7 @@ export function registerTemplate(program: Command): void { .description('List the built-in agent templates and the slugs --template takes'), 'ls', ), - 'GET /templates', + 'GET /agents/templates', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { diff --git a/src/commands/variable.ts b/src/commands/variable.ts index 4d65aca..09de7ee 100644 --- a/src/commands/variable.ts +++ b/src/commands/variable.ts @@ -20,7 +20,7 @@ export function registerVariable(program: Command): void { variable.command('list').description('List the variable names (never their values)'), 'ls', ), - 'GET /variables', + 'GET /secrets', ) .option('--json', 'output raw JSON') .action(async (opts: { json?: boolean }) => { @@ -34,7 +34,7 @@ export function registerVariable(program: Command): void { variable .command('set [assignments...]') .description('Create or update variables, e.g. `set A=1 B=2`'), - 'PUT /variables', + 'PUT /secrets', ) .option('-f, --from-file ', 'load variables from a .env or .json file') .option('--json', 'output raw JSON') @@ -52,7 +52,7 @@ export function registerVariable(program: Command): void { apiRoutes( alsoKnownAs(variable.command('delete ').description('Delete a variable'), 'rm'), - 'DELETE /variables/{name}', + 'DELETE /secrets/{name}', ) .option('--json', 'output raw JSON') .action(async (name: string, opts: { json?: boolean }) => { diff --git a/src/lib/api.ts b/src/lib/api.ts index 3657065..9cb3e67 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -7,17 +7,16 @@ import type { AnalyticsMetricsQuery, AnalyticsPullRequestsQuery, AnalyticsReviewsQuery, - AssetView, BudgetSummary, CliAuthPoll, CliAuthStart, - CodeReviewDefaultView, CreateAgentConfigRequest, - CreateAssetRequest, - CreateAssetResponse, + CreateFileRequest, + CreateFileResponse, CreateReviewRequest, CreatedAgentConfig, - GetAssetResponse, + FileView, + GetFileResponse, GetAnalyticsMetricsResponse, GetAnalyticsPullRequestsResponse, GetAnalyticsReviewsResponse, @@ -31,9 +30,8 @@ import type { ListAgentSessionsQuery, ListAgentSessionsResponse, ListAgentTemplatesResponse, - ListAssetsQuery, - ListAssetsResponse, - ListCodeReviewDefaultsResponse, + ListFilesQuery, + ListFilesResponse, ListGithubMembersResponse, ListGithubRepositoriesResponse, ListLinearTeamsResponse, @@ -45,7 +43,6 @@ import type { ListSlackChannelsResponse, ListSlackMembersResponse, PutAgentDefaultRequest, - PutCodeReviewDefaultRequest, ReplayAgentSessionRequest, Review, SendSessionMessageRequest, @@ -298,42 +295,42 @@ export class ApiClient { ) } - // --------------------------------- assets -------------------------------- - // Agent asset storage: persist a file to the platform and get back an - // org-membership-gated link (documents/eng/AGENT_ASSET_STORAGE.md in the - // ellipsis repo). v1 is PNG-only with a 10 MiB cap, enforced server-side. + // --------------------------------- files --------------------------------- + // Agent file storage: persist a file to the platform and get back an + // org-membership-gated link. v1 is PNG-only with a 10 MiB cap, enforced + // server-side. - uploadAsset(req: CreateAssetRequest): Promise { - return this.request('POST', '/assets', req) + uploadFile(req: CreateFileRequest): Promise { + return this.request('POST', '/files', req) } - // Newest-first metadata for the credential's customer's assets. Metadata - // only — presigned download URLs are minted per explicit getAsset. - async listAssets(query?: ListAssetsQuery): Promise { - const res = await this.request( + // Newest-first metadata for the credential's customer's files. Metadata + // only — presigned download URLs are minted per explicit getFile. + async listFiles(query?: ListFilesQuery): Promise { + const res = await this.request( 'GET', - '/assets', + '/files', undefined, query as Record | undefined, ) - return res.assets + return res.files } // Metadata + the gated URL + a short-lived presigned `download_url`. To pull // the bytes locally, GET download_url immediately (it expires in ~60s; if it // lapses, just call this again for a fresh one). - getAsset(assetId: string): Promise { - return this.request('GET', `/assets/${encodeURIComponent(assetId)}`) + getFile(fileId: string): Promise { + return this.request('GET', `/files/${encodeURIComponent(fileId)}`) } - // Delete an asset: it disappears from every read path and its gated link + // Delete a file: it disappears from every read path and its gated link // stops resolving (the server soft-deletes; storage accounting keeps // charging for everything ever written). The // server returns 204 with an empty body on success; 404 when the id is // unknown to the credential's customer, 403 when the token isn't allowed to // delete (e.g. a sandbox token). - deleteAsset(assetId: string): Promise { - return this.request('DELETE', `/assets/${encodeURIComponent(assetId)}`) + deleteFile(fileId: string): Promise { + return this.request('DELETE', `/files/${encodeURIComponent(fileId)}`) } // -------------------------------- reviews -------------------------------- @@ -365,47 +362,21 @@ export class ApiClient { return res.reviews } - // --------------------------- review defaults ----------------------------- - // The default code review pipeline ladder (repo default -> account default), - // the code_review twin of the /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 /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', - '/reviews/defaults', - ) - return res.defaults - } - - putReviewDefault(req: PutCodeReviewDefaultRequest): Promise { - return this.request('PUT', '/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', '/reviews/defaults', undefined, { repository }) - } - // ----------------------------- agent configs ---------------------------- async listAgentConfigs(): Promise { - const res = await this.request('GET', '/configs') + const res = await this.request('GET', '/agents/configs') return res.configs } // Opens a pull request that adds the config's YAML to the repo's agents/ // directory; the agent goes live once it merges and syncs. createAgentConfig(req: CreateAgentConfigRequest): Promise { - return this.request('POST', '/configs', req) + return this.request('POST', '/agents/configs', req) } getAgentConfig(configId: string): Promise { - return this.request('GET', `/configs/${encodeURIComponent(configId)}`) + return this.request('GET', `/agents/configs/${encodeURIComponent(configId)}`) } // ------------------------------ defaults -------------------------------- @@ -415,18 +386,18 @@ export class ApiClient { // refused for sandbox tokens (403). async listAgentDefaults(): Promise { - const res = await this.request('GET', '/defaults') + const res = await this.request('GET', '/agents/defaults') return res.defaults } putAgentDefault(req: PutAgentDefaultRequest): Promise { - return this.request('PUT', '/defaults', req) + return this.request('PUT', '/agents/defaults', req) } // Clears a rung: the account default when `repository` is omitted, that // repo's default otherwise. 404 when the rung isn't set. deleteAgentDefault(repository?: string): Promise { - return this.request('DELETE', '/defaults', undefined, { repository }) + return this.request('DELETE', '/agents/defaults', undefined, { repository }) } // ------------------------------- variables -------------------------------- @@ -434,14 +405,14 @@ export class ApiClient { // mutation), so callers can render the resulting state. async listSandboxVariables(): Promise { - const res = await this.request('GET', '/variables') + const res = await this.request('GET', '/secrets') return res.variables } async putSandboxVariables( variables: SandboxVariableInput[], ): Promise { - const res = await this.request('PUT', '/variables', { + const res = await this.request('PUT', '/secrets', { variables, }) return res.variables @@ -450,7 +421,7 @@ export class ApiClient { async deleteSandboxVariable(name: string): Promise { const res = await this.request( 'DELETE', - `/variables/${encodeURIComponent(name)}`, + `/secrets/${encodeURIComponent(name)}`, ) return res.variables } @@ -467,12 +438,12 @@ export class ApiClient { // ---------------------------- agent templates --------------------------- async listAgentTemplates(): Promise { - const res = await this.request('GET', '/templates') + const res = await this.request('GET', '/agents/templates') return res.templates } getAgentTemplate(slug: string): Promise { - return this.request('GET', `/templates/${encodeURIComponent(slug)}`) + return this.request('GET', `/agents/templates/${encodeURIComponent(slug)}`) } // ------------------------ integration discovery ------------------------- @@ -485,38 +456,38 @@ export class ApiClient { } listGithubRepositories(): Promise { - return this.request('GET', '/github/repos') + return this.request('GET', '/integrations/github/repos') } listGithubMembers(): Promise { - return this.request('GET', '/github/members') + return this.request('GET', '/integrations/github/members') } listSlackChannels(): Promise { - return this.request('GET', '/slack/channels') + return this.request('GET', '/integrations/slack/channels') } listSlackMembers(): Promise { - return this.request('GET', '/slack/members') + return this.request('GET', '/integrations/slack/members') } listLinearTeams(): Promise { - return this.request('GET', '/linear/teams') + return this.request('GET', '/integrations/linear/teams') } listSentryOrganizations(): Promise { - return this.request('GET', '/sentry/organizations') + return this.request('GET', '/integrations/sentry/organizations') } // --------------------------- device-code auth --------------------------- // Unauthenticated: the CLI has no credential yet — that's what it's obtaining. startCliAuth(): Promise { - return this.request('POST', '/cli-auth/start') + return this.request('POST', '/auth/cli/start') } pollCliAuth(deviceCode: string): Promise { - return this.request('POST', '/cli-auth/poll', { device_code: deviceCode }) + return this.request('POST', '/auth/cli/poll', { device_code: deviceCode }) } } @@ -552,20 +523,30 @@ export function buildQuery(query?: Record): string { return qs ? `?${qs}` : '' } -// Pull the server's `{"detail": ...}` message and request id off a non-2xx -// response. The detail keeps `agent` error output actionable instead of bare -// codes; the id comes from the `X-Request-ID` header (set on every API response) -// and falls back to the `request_id` field the server's 500 handler includes in -// its body, so an error carries something we can grep our logs for. +// Pull the server's message and request id off a non-2xx response. The message +// keeps `agent` error output actionable instead of bare codes; the id comes from +// the `X-Request-ID` header (set on every API response) and falls back to the +// body, so an error carries something we can grep our logs for. +// +// The public API answers with `{"error": {code, message, request_id}}`. FastAPI's +// own validation and auth rejections still use `{"detail": ...}`, so both shapes +// are read: an unrecognized body would otherwise degrade to a bare status line. export async function parseErrorResponse( res: Response, ): Promise<{ detail: string; requestId?: string }> { const headerRequestId = res.headers.get('x-request-id') ?? undefined try { - const body = (await res.json()) as { detail?: unknown; request_id?: unknown } + const body = (await res.json()) as { + error?: { code?: unknown; message?: unknown; request_id?: unknown } + detail?: unknown + request_id?: unknown + } + const bodyRequestId = body.error?.request_id ?? body.request_id const requestId = - headerRequestId ?? - (typeof body.request_id === 'string' ? body.request_id : undefined) + headerRequestId ?? (typeof bodyRequestId === 'string' ? bodyRequestId : undefined) + if (typeof body.error?.message === 'string') { + return { detail: body.error.message, requestId } + } if (typeof body.detail === 'string') return { detail: body.detail, requestId } if (body.detail) return { detail: JSON.stringify(body.detail), requestId } return { detail: res.statusText, requestId } diff --git a/src/lib/help.ts b/src/lib/help.ts index 6c9ed27..7c07203 100644 --- a/src/lib/help.ts +++ b/src/lib/help.ts @@ -20,7 +20,7 @@ function withoutAliases(term: string, cmd: Command): string { const TOP_LEVEL_GROUPS: ReadonlyArray<{ title: string; commands: readonly string[] }> = [ { title: 'Sessions', commands: ['session', 'review'] }, { title: 'Agents', commands: ['config', 'model', 'template'] }, - { title: 'Platform', commands: ['variable', 'asset', 'hook'] }, + { title: 'Platform', commands: ['variable', 'file', 'hook'] }, { title: 'Integrations', commands: ['integration', 'github', 'slack', 'linear', 'sentry'] }, { title: 'Spend', commands: ['budget', 'usage', 'analytics'] }, { title: 'Account', commands: ['install', 'login', 'logout', 'me', 'host', 'ping'] }, diff --git a/src/lib/laptop.ts b/src/lib/laptop.ts index 907a7dd..2e9a3d0 100644 --- a/src/lib/laptop.ts +++ b/src/lib/laptop.ts @@ -443,28 +443,3 @@ export function pushHandoffRef(cwd: string, sha: string): string { return ref } -// --------------------------------------------------------------------------- -// Local review: the same snapshot trick, but pushed to a real BRANCH. -// --------------------------------------------------------------------------- - -export function currentBranch(cwd: string): string | null { - const branch = gitOrThrow(cwd, 'rev-parse', '--abbrev-ref', 'HEAD') - // Detached HEAD has no branch to derive a sidecar name from. - return branch && branch !== 'HEAD' ? branch : null -} - -// The sidecar branch a local review is pushed to. Your real branch is never -// touched, so a stash-quality WIP commit never lands on the work you're doing. -export function reviewBranchName(branch: string): string { - return `ellipsis/review/${branch}` -} - -// Force-push the WIP snapshot to the sidecar branch. A BRANCH, not the hidden -// ref handoff uses: GitHub cannot open a pull request from a non-branch ref, -// and the platform needs a PR to review (it finds-or-creates a draft one). -// Force because each review replaces the previous snapshot of the same work. -export function pushReviewBranch(cwd: string, sha: string, branch: string): string { - const target = reviewBranchName(branch) - gitOrThrow(cwd, 'push', '--force', 'origin', `${sha}:refs/heads/${target}`) - return target -} diff --git a/src/lib/output.ts b/src/lib/output.ts index 8230214..c6e4631 100644 --- a/src/lib/output.ts +++ b/src/lib/output.ts @@ -106,7 +106,7 @@ const UPGRADE_HINT_EXEMPT = new Set([ 404, // missing resource / integration not connected (mapped per-command) 408, // transient timeout 409, // documented conflicts (closed session, sandbox not running) - 413, // payload over a server-enforced cap (e.g. the 10 MiB asset limit) + 413, // payload over a server-enforced cap (e.g. the 10 MiB file limit) 429, // handled above: rate limit detail printed bare 502, // gateway trouble 503, // API down or overloaded diff --git a/src/lib/types.ts b/src/lib/types.ts index da884d0..c8a4b01 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -306,34 +306,6 @@ export interface PutAgentDefaultRequest { config_id: string } -// One rung of the default code review pipeline ladder (GET -// /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 /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 /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 /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. @@ -856,13 +828,13 @@ export interface GetAnalyticsReviewsResponse { } } -// --------------------------------- assets -------------------------------- -// Agent asset storage (ellipsis: documents/eng/AGENT_ASSET_STORAGE.md): files -// an agent persists beyond its sandbox's lifetime — v1 is PNG screenshots -// posted as org-gated links on PRs. Mirrors assets_service.py. +// --------------------------------- files --------------------------------- +// Agent file storage: files an agent persists beyond its sandbox's lifetime — +// v1 is PNG screenshots posted as org-gated links on PRs. Mirrors +// files_service.py. -// Caller-facing asset metadata — no storage internals (S3 key, sha, owner). -export interface AssetView { +// Caller-facing file metadata — no storage internals (S3 key, sha, owner). +export interface FileView { id: string filename: string content_type: string @@ -872,9 +844,9 @@ export interface AssetView { agent_session_id: string | null } -export interface CreateAssetRequest { +export interface CreateFileRequest { // Original basename, display only (the S3 key derives from the server-side - // asset id, never from this). + // file id, never from this). filename: string // v1: must be image/png; the server magic-byte-checks the decoded bytes. content_type: string @@ -883,25 +855,25 @@ export interface CreateAssetRequest { data_b64: string } -export interface CreateAssetResponse { - asset: AssetView - // The fully-formed org-gated dashboard URL (app.ellipsis.dev/assets/{id}) — +export interface CreateFileResponse { + file: FileView + // The fully-formed org-gated dashboard URL (app.ellipsis.dev/files/{id}) — // the link an agent pastes into a PR comment. url: string } -export interface ListAssetsQuery { +export interface ListFilesQuery { // Scope to one run's uploads. agent_session_id?: string limit?: number } -export interface ListAssetsResponse { - assets: AssetView[] +export interface ListFilesResponse { + files: FileView[] } -export interface GetAssetResponse { - asset: AssetView +export interface GetFileResponse { + file: FileView // The gated dashboard URL (same link the upload returned). url: string // Short-lived (60s) presigned S3 GET for the actual bytes — fetch it diff --git a/src/ui/SessionsApp.tsx b/src/ui/SessionsApp.tsx index b7c2ef1..6cd321a 100644 --- a/src/ui/SessionsApp.tsx +++ b/src/ui/SessionsApp.tsx @@ -174,6 +174,15 @@ export function SessionsApp(props: SessionsAppProps): React.ReactElement { // lag, or attributed differently); merged into the list until it does. const [localSessions, setLocalSessions] = useState([]) + // The last API failure from any background call (the poll, the composer's + // pickers). Those calls have no output of their own, so without this a broken + // route or a dead token just shows an empty list. Rendered in the nav hint + // row, which is the one line always on screen. + const [apiError, setApiError] = useState(null) + const reportApiError = useCallback((label: string, err: unknown): void => { + setApiError(`${label}: ${err instanceof ApiError ? err.detail : (err as Error).message}`) + }, []) + const poll = useCallback(async (): Promise => { try { const listed = await api.listAgentSessions({ @@ -192,10 +201,13 @@ export function SessionsApp(props: SessionsAppProps): React.ReactElement { setSessions(listed) setLocalSessions((prev) => prev.filter((l) => !listed.some((s) => s.id === l.id))) setPolledOnce(true) - } catch { - // Transient poll failure — keep the previous list; the next tick retries. + setApiError(null) + } catch (err) { + // Keep the previous list (the next tick retries), but say so: a poll that + // fails every tick is a broken session list, not a blip. + reportApiError('sessions', err) } - }, [api, authorId]) + }, [api, authorId, reportApiError]) // The poll only feeds the nav's rows and attention dots; with the bar // hidden there is nothing on screen it could update. @@ -315,16 +327,25 @@ export function SessionsApp(props: SessionsAppProps): React.ReactElement { void api .listAgentConfigs() .then((rows) => setConfigs(rows.filter((c) => !c.deleted))) - .catch(() => setConfigs([])) + .catch((err) => { + setConfigs([]) + reportApiError('agent configs', err) + }) void api .listGithubRepositories() .then((r) => setRepos(r.repositories.map((repo) => repo.full_name))) - .catch(() => setRepos([])) + .catch((err) => { + setRepos([]) + reportApiError('repositories', err) + }) void api .listSupportedModels() .then(setModels) - .catch(() => setModels([])) - }, [mainPane.type, api]) + .catch((err) => { + setModels([]) + reportApiError('models', err) + }) + }, [mainPane.type, api, reportApiError]) const startSession = useCallback( async (prompt: string, choices: ComposerChoices): Promise => { @@ -643,13 +664,22 @@ export function SessionsApp(props: SessionsAppProps): React.ReactElement { {/* Absorbs the rows a short list leaves empty, keeping the hint on the band's bottom edge. */} - - {navFocused - ? `↑↓ move · enter open · n new · esc chat · q quit${ - win.end < rows.length ? ` · ${rows.length - win.end} more below` : '' - }` - : '↓/esc: sessions'} - + {/* An API failure replaces the key hints rather than sharing the line: + the hints are always recoverable from muscle memory, a swallowed error + is not. */} + {apiError ? ( + + {`✗ ${apiError}`} + + ) : ( + + {navFocused + ? `↑↓ move · enter open · n new · esc chat · q quit${ + win.end < rows.length ? ` · ${rows.length - win.end} more below` : '' + }` + : '↓/esc: sessions'} + + )} ) diff --git a/test/api.test.ts b/test/api.test.ts index 9f63806..bf511f1 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -160,7 +160,7 @@ describe('ApiClient sandbox variables', () => { const out = await new ApiClient('http://api.test', 't').listSandboxVariables() expect(out).toEqual([{ name: 'A', created_at: '', updated_at: '' }]) - expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/variables') + expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/secrets') expect((fetchMock.mock.calls[0][1] as RequestInit).method).toBe('GET') }) @@ -172,7 +172,7 @@ describe('ApiClient sandbox variables', () => { await new ApiClient('http://api.test', 't').putSandboxVariables([{ name: 'TOKEN', value: 'x' }]) const [url, init] = fetchMock.mock.calls[0] - expect(url).toBe('http://api.test/variables') + expect(url).toBe('http://api.test/secrets') expect((init as RequestInit).method).toBe('PUT') expect(JSON.parse((init as RequestInit).body as string)).toEqual({ variables: [{ name: 'TOKEN', value: 'x' }], @@ -187,72 +187,11 @@ describe('ApiClient sandbox variables', () => { await new ApiClient('http://api.test', 't').deleteSandboxVariable('MY/VAR') const [url, init] = fetchMock.mock.calls[0] - expect(url).toBe('http://api.test/variables/MY%2FVAR') + expect(url).toBe('http://api.test/secrets/MY%2FVAR') expect((init as RequestInit).method).toBe('DELETE') }) }) -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/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/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/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/reviews/defaults?repository=acme%2Fapi', - ) - }) -}) - describe('getSessionLog', () => { afterEach(() => vi.unstubAllGlobals()) @@ -328,7 +267,7 @@ describe('agent templates', () => { const out = await new ApiClient('http://api.test', 't').listAgentTemplates() expect(out.map((t) => t.slug)).toEqual(['a', 'b']) - expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/templates') + expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/agents/templates') }) it('fetches a single template by slug (encoded)', async () => { @@ -340,7 +279,7 @@ describe('agent templates', () => { const out = await new ApiClient('http://api.test', 't').getAgentTemplate('ci-failure-triager') expect(out.yaml).toBe('x') - expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/templates/ci-failure-triager') + expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/agents/templates/ci-failure-triager') }) }) @@ -390,7 +329,7 @@ describe('createAgentConfig', () => { }) expect(out.pull_request_url).toBe('https://github.com/octocat/api/pull/7') const [url, init] = fetchMock.mock.calls[0] - expect(url).toBe('http://api.test/configs') + expect(url).toBe('http://api.test/agents/configs') expect((init as RequestInit).method).toBe('POST') expect(JSON.parse((init as RequestInit).body as string)).toEqual({ template_id: 'ci-failure-triager', @@ -399,7 +338,7 @@ describe('createAgentConfig', () => { }) }) -describe('ApiClient assets', () => { +describe('ApiClient files', () => { afterEach(() => vi.unstubAllGlobals()) it('POSTs the upload payload and returns the gated URL', async () => { @@ -407,22 +346,22 @@ describe('ApiClient assets', () => { async () => new Response( JSON.stringify({ - asset: { id: 'a1', filename: 'shot.png' }, - url: 'https://app.ellipsis.dev/assets/a1', + file: { id: 'a1', filename: 'shot.png' }, + url: 'https://app.ellipsis.dev/files/a1', }), { status: 201 }, ), ) vi.stubGlobal('fetch', fetchMock) - const out = await new ApiClient('http://api.test', 't').uploadAsset({ + const out = await new ApiClient('http://api.test', 't').uploadFile({ filename: 'shot.png', content_type: 'image/png', data_b64: 'aGk=', }) - expect(out.url).toBe('https://app.ellipsis.dev/assets/a1') + expect(out.url).toBe('https://app.ellipsis.dev/files/a1') const [url, init] = fetchMock.mock.calls[0] - expect(url).toBe('http://api.test/assets') + expect(url).toBe('http://api.test/files') expect((init as RequestInit).method).toBe('POST') expect(JSON.parse((init as RequestInit).body as string)).toEqual({ filename: 'shot.png', @@ -431,45 +370,45 @@ describe('ApiClient assets', () => { }) }) - it('lists assets, unwrapping the envelope and passing filters as query', async () => { + it('lists files, unwrapping the envelope and passing filters as query', async () => { const fetchMock = vi.fn( - async () => new Response(JSON.stringify({ assets: [{ id: 'a1' }] }), { status: 200 }), + async () => new Response(JSON.stringify({ files: [{ id: 'a1' }] }), { status: 200 }), ) vi.stubGlobal('fetch', fetchMock) - const out = await new ApiClient('http://api.test', 't').listAssets({ + const out = await new ApiClient('http://api.test', 't').listFiles({ agent_session_id: 'session_1', limit: 5, }) expect(out).toEqual([{ id: 'a1' }]) expect(fetchMock.mock.calls[0][0]).toBe( - 'http://api.test/assets?agent_session_id=session_1&limit=5', + 'http://api.test/files?agent_session_id=session_1&limit=5', ) }) - it('URL-encodes the asset id on get', async () => { + it('URL-encodes the file id on get', async () => { const fetchMock = vi.fn( async () => new Response( - JSON.stringify({ asset: { id: 'a/1' }, url: 'u', download_url: 'd' }), + JSON.stringify({ file: { id: 'a/1' }, url: 'u', download_url: 'd' }), { status: 200 }, ), ) vi.stubGlobal('fetch', fetchMock) - const out = await new ApiClient('http://api.test', 't').getAsset('a/1') + const out = await new ApiClient('http://api.test', 't').getFile('a/1') expect(out.download_url).toBe('d') - expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/assets/a%2F1') + expect(fetchMock.mock.calls[0][0]).toBe('http://api.test/files/a%2F1') }) - it('DELETEs the asset id (encoded) and tolerates a 204 empty body', async () => { + it('DELETEs the file id (encoded) and tolerates a 204 empty body', async () => { const fetchMock = vi.fn(async () => new Response(null, { status: 204 })) vi.stubGlobal('fetch', fetchMock) - const out = await new ApiClient('http://api.test', 't').deleteAsset('a/1') + const out = await new ApiClient('http://api.test', 't').deleteFile('a/1') expect(out).toBeUndefined() const [url, init] = fetchMock.mock.calls[0] - expect(url).toBe('http://api.test/assets/a%2F1') + expect(url).toBe('http://api.test/files/a%2F1') expect((init as RequestInit).method).toBe('DELETE') }) }) diff --git a/test/discovery.test.ts b/test/discovery.test.ts index 94428df..4b7f6d2 100644 --- a/test/discovery.test.ts +++ b/test/discovery.test.ts @@ -15,12 +15,20 @@ describe('integration discovery endpoints', () => { it('hits the provider-namespaced paths', async () => { const cases: Array<[(api: ApiClient) => Promise, string, unknown]> = [ [(api) => api.getIntegrations(), '/integrations', { sentry: [] }], - [(api) => api.listGithubRepositories(), '/github/repos', { repositories: [] }], - [(api) => api.listGithubMembers(), '/github/members', { members: [] }], - [(api) => api.listSlackChannels(), '/slack/channels', { channels: [] }], - [(api) => api.listSlackMembers(), '/slack/members', { members: [] }], - [(api) => api.listLinearTeams(), '/linear/teams', { teams: [] }], - [(api) => api.listSentryOrganizations(), '/sentry/organizations', { organizations: [] }], + [ + (api) => api.listGithubRepositories(), + '/integrations/github/repos', + { repositories: [] }, + ], + [(api) => api.listGithubMembers(), '/integrations/github/members', { members: [] }], + [(api) => api.listSlackChannels(), '/integrations/slack/channels', { channels: [] }], + [(api) => api.listSlackMembers(), '/integrations/slack/members', { members: [] }], + [(api) => api.listLinearTeams(), '/integrations/linear/teams', { teams: [] }], + [ + (api) => api.listSentryOrganizations(), + '/integrations/sentry/organizations', + { organizations: [] }, + ], ] for (const [call, path, body] of cases) { const fetchMock = stub(body) diff --git a/test/asset.test.ts b/test/file.test.ts similarity index 92% rename from test/asset.test.ts rename to test/file.test.ts index c44b8fb..a7761e9 100644 --- a/test/asset.test.ts +++ b/test/file.test.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from 'vitest' import { - MAX_ASSET_SIZE_BYTES, + MAX_FILE_SIZE_BYTES, buildUploadRequest, formatSize, -} from '../src/commands/asset' +} from '../src/commands/file' const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) @@ -31,14 +31,14 @@ describe('buildUploadRequest', () => { }) it('rejects files over the 10 MiB cap, with sizes in the message', () => { - const big = Buffer.concat([PNG_MAGIC, Buffer.alloc(MAX_ASSET_SIZE_BYTES)]) - expect(() => buildUploadRequest('big.png', big)).toThrow(/10\.0 MiB per asset/) + const big = Buffer.concat([PNG_MAGIC, Buffer.alloc(MAX_FILE_SIZE_BYTES)]) + expect(() => buildUploadRequest('big.png', big)).toThrow(/10\.0 MiB per file/) }) it('accepts a PNG exactly at the cap', () => { const atCap = Buffer.concat([ PNG_MAGIC, - Buffer.alloc(MAX_ASSET_SIZE_BYTES - PNG_MAGIC.length), + Buffer.alloc(MAX_FILE_SIZE_BYTES - PNG_MAGIC.length), ]) expect(buildUploadRequest('cap.png', atCap).content_type).toBe('image/png') }) diff --git a/test/output.test.ts b/test/output.test.ts index d8f31c2..a5b14a0 100644 --- a/test/output.test.ts +++ b/test/output.test.ts @@ -74,7 +74,7 @@ describe('friendlyErrorMessage', () => { const err = new ApiError( 429, 'POST', - '/assets', + '/files', 'Asset limit reached: your organization is storing 50 of 50 assets. ' + 'Delete assets you no longer need, or email team@ellipsis.dev to raise the limit.', ) diff --git a/test/review.test.ts b/test/review.test.ts index 9ce8669..fa6ae96 100644 --- a/test/review.test.ts +++ b/test/review.test.ts @@ -11,7 +11,6 @@ import { splitRepo, starterPipeline, } from '../src/commands/review' -import { currentBranch, reviewBranchName } from '../src/lib/laptop' import type { Finding } from '../src/lib/types' // A throwaway repo with an origin remote, so the local path's git work runs for @@ -72,19 +71,6 @@ describe('buildCreateRequest — an existing pull request', () => { expect(req.scope.head).toBe('bbbb222') }) - it('forwards the config, model, and budget overrides', () => { - const req = buildCreateRequest('1', { - ...START_DEFAULTS, - repo: 'o/r', - config: 'agent_abc', - model: 'claude-opus-4-8', - budget: 5, - }) - expect(req.config_id).toBe('agent_abc') - expect(req.model).toBe('claude-opus-4-8') - expect(req.budget).toBe(5) - }) - it('posts by default, and --no-post turns it off', () => { expect(buildCreateRequest('1', { ...START_DEFAULTS, repo: 'o/r' }).post).toBe(true) expect( @@ -93,76 +79,14 @@ describe('buildCreateRequest — an existing pull request', () => { }) }) -describe('buildCreateRequest — the local path', () => { - it('pushes a sidecar branch and pins the range end to the pushed commit', () => { - const { work: cwd, remote } = scratchRepo('feature/thing') - writeFileSync(join(cwd, 'a.txt'), 'dirty\n') - - const req = buildCreateRequest(undefined, { ...START_DEFAULTS, cwd }) - - // The sidecar, never the branch you're working on. - expect(req.branch).toBe('ellipsis/review/feature/thing') - expect(currentBranch(cwd)).toBe('feature/thing') - // The pushed snapshot pins the head: GitHub's reported PR head lags a - // force-push to the sidecar. - expect(req.sha).toMatch(/^[0-9a-f]{40}$/) - expect(req.pull_request_number).toBeUndefined() - // The commit really landed on the remote (read the bare repo directly — - // the fetch URL is a real GitHub URL this test can't reach). - const pushed = execFileSync('git', ['-C', remote, 'show-ref'], { - encoding: 'utf8', - }) - expect(pushed).toContain('refs/heads/ellipsis/review/feature/thing') - }) - - it('never posts a local review, even without --no-post', () => { - const { work: cwd } = scratchRepo() - // Reviewing unfinished work must not leave comments on a pull request, so - // the terminal-only default does not depend on the caller remembering. - expect(buildCreateRequest(undefined, { ...START_DEFAULTS, cwd }).post).toBe(false) - }) - - it('snapshots a clean tree too (HEAD), so a review needs no dirty edit', () => { - const { work: cwd } = scratchRepo() - const head = execFileSync('git', ['-C', cwd, 'rev-parse', 'HEAD'], { - encoding: 'utf8', - }).trim() - expect(buildCreateRequest(undefined, { ...START_DEFAULTS, cwd }).sha).toBe(head) - }) - - it('reviews an already-pushed branch without touching git', () => { - const { work: cwd } = scratchRepo() - const req = buildCreateRequest(undefined, { - ...START_DEFAULTS, - cwd, - branch: 'someone/elses-branch', - }) - expect(req.branch).toBe('someone/elses-branch') - // Nothing was pushed, so there is no snapshot SHA to pin. - expect(req.sha).toBeUndefined() - }) - - it('explains itself on a detached HEAD instead of guessing a branch', () => { - const { work: cwd } = scratchRepo() - execFileSync('git', ['-C', cwd, 'checkout', '--detach'], { stdio: 'ignore' }) - expect(() => buildCreateRequest(undefined, { ...START_DEFAULTS, cwd })).toThrow( - /detached HEAD/, - ) - }) - +describe('buildCreateRequest — repository resolution', () => { it('needs a repo when there is no git remote to infer one from', () => { expect(() => - buildCreateRequest(undefined, { ...START_DEFAULTS, cwd: mkdtempSync(join(tmpdir(), 'bare-')) }), + buildCreateRequest('123', { ...START_DEFAULTS, cwd: mkdtempSync(join(tmpdir(), 'bare-')) }), ).toThrow(/--repo/) }) }) -describe('reviewBranchName', () => { - it('prefixes the sidecar so it is obvious what it is in a branch list', () => { - expect(reviewBranchName('hunter/my-feature')).toBe('ellipsis/review/hunter/my-feature') - }) -}) - describe('parsePullRequest', () => { it('accepts the three spellings people paste', () => { expect(parsePullRequest('5975')).toBe(5975) @@ -229,22 +153,37 @@ describe('formatFinding', () => { describe('starterPipeline', () => { it('marks the file as a pipeline, not an agent', () => { - expect(starterPipeline('code_review', 'cli')).toContain('kind: code_review') + expect(starterPipeline('cli code review')).toContain('kind: code_review') }) it('parses as YAML and only sets keys the schema allows', () => { - const parsed = parse(starterPipeline('code_review', 'cli')) as Record - expect(Object.keys(parsed).sort()).toEqual(['budget', 'ellipsis', 'pull_requests']) + const parsed = parse(starterPipeline('cli code review')) as Record + expect(Object.keys(parsed).sort()).toEqual(['budget', 'ellipsis']) expect(parsed.ellipsis).toMatchObject({ version: 'v1', kind: 'code_review' }) - expect(parsed.pull_requests).toEqual({ repositories: ['cli'] }) }) - it('names the pipeline after the file, so a second file does not collide', () => { - const parsed = parse(starterPipeline('backend', 'cli')) as { ellipsis: { name: string } } - expect(parsed.ellipsis.name).toBe('backend') + // Location is the scope now, so naming repositories is a sync error anywhere + // but the org-wide copy — the scaffold must never emit the key. + it('omits pull_requests.repositories, which would be a sync error', () => { + expect(starterPipeline('cli code review')).not.toContain('repositories:') + }) + + // Deleted from the schema, which forbids unknown keys. + it('omits include_default_reviewers', () => { + expect(starterPipeline('cli code review')).not.toContain('include_default_reviewers') + }) + + it('names the pipeline so a reader knows what it covers', () => { + const parsed = parse(starterPipeline('backend code review')) as { + ellipsis: { name: string } + } + expect(parsed.ellipsis.name).toBe('backend code review') }) - it('falls back to a placeholder repository outside a git checkout', () => { - expect(starterPipeline('code_review', undefined)).toContain('- my-repo') + it('documents both legal paths and no others', () => { + const text = starterPipeline('cli code review') + expect(text).toContain('code_review.yaml') + expect(text).toContain('.ellipsis/code_review.yaml') + expect(text).not.toContain('agents/') }) })