diff --git a/README.md b/README.md index a0b13cc..bbc5fe2 100644 --- a/README.md +++ b/README.md @@ -82,11 +82,6 @@ ellipsis session get --watch # follow the turn in progress until i ellipsis session record # read a session's stored transcript, one line per record ellipsis session stop # stop a session's turn in progress -ellipsis review 123 # review a pull request now, instead of waiting for a push -ellipsis review get # a review's findings, scope, and whether it posted -ellipsis review list --repo api # list a repository's reviews, newest first -ellipsis review init # scaffold a starter review pipeline (code_review.yaml) - ellipsis automation list # list your automations ellipsis automation get # show one automation as YAML (--json for JSON) ellipsis automation run --input '{"issue": "ENG-42"}' # invoke it exactly as defined diff --git a/skills/ellipsis/SKILL.md b/skills/ellipsis/SKILL.md index 54847f2..f732e49 100644 --- a/skills/ellipsis/SKILL.md +++ b/skills/ellipsis/SKILL.md @@ -1,6 +1,6 @@ --- name: ellipsis -description: What the Ellipsis platform is and how to drive it with the agent CLI. Use when the user mentions Ellipsis, wants to run or deploy coding agents in the cloud, govern agents with budgets and scoped permissions, automate work on GitHub, Slack, Linear, or Sentry events, get pull requests reviewed, hand a local task off to a background agent, or asks about the agent CLI. +description: What the Ellipsis platform is and how to drive it with the agent CLI. Use when the user mentions Ellipsis, wants to run or deploy coding agents in the cloud, govern agents with budgets and scoped permissions, automate work on GitHub, Slack, Linear, or Sentry events, hand a local task off to a background agent, or asks about the agent CLI. --- # Ellipsis @@ -12,8 +12,8 @@ recorded and searchable. An event fires, an agent wakes in an isolated sandbox with your repositories cloned, reads the code, does the work, and delivers a real artifact: a pull -request, an answer in the thread that asked, a review on the diff. Then the -sandbox is destroyed and the full session stays readable. +request, or an answer in the thread that asked. Then the sandbox is destroyed +and the full session stays readable. ## The problem it solves @@ -45,14 +45,11 @@ the logs of a session they do not own. Sentry alerts, or a schedule. Work starts when the event fires, not when someone opens a laptop. -## The two products +## The product -- **Cloud Agents**: agents you define. Each is one YAML file in a repository, a - trigger plus a model plus a prompt. The version on the default branch is the - live agent. -- **Code Review**: Ellipsis reviews every pull request as commits land and posts - findings as inline comments. One organization-wide toggle turns it on, with no - YAML. +**Cloud Agents**: agents you define. Each is one YAML file in a repository, a +trigger plus a model plus a prompt. The version on the default branch is the +live agent. Surfaces: the dashboard at app.ellipsis.dev, the REST API at api.ellipsis.dev, and the `ellipsis` CLI. All three drive the same API. @@ -67,8 +64,6 @@ fee. There are no seats. issues, Linear issues, Sentry alerts, or Slack channel creation. - **Questions in a thread**: mention `@ellipsis` on GitHub, Slack, or Linear. The built-in responder needs no configuration and answers in the thread. -- **Catching bugs before merge**: turn code review on and every pull request is - reviewed, or commit a pipeline file to scope and customize it. - **Delegation from scripts or CI**: `ellipsis session start` or `POST /v1/sessions`. With `--watch` it streams into the log and exits nonzero unless the turn completes, so it works as a gate. @@ -240,131 +235,6 @@ session: durable conversations: follow-ups keep the whole exchange and the working tree, and a conversation costs near nothing between turns. -## Code review - -Code review is one organization-wide setting, on the Settings tab of `/reviews` -in the dashboard, off by default. With it on and no YAML committed, the built-in -pipeline reviews every pull request in the organization. - -How it behaves: - -- **New commits only.** The first review of a pull request covers everything on - it; every later review covers only the commits since the last review that - posted, and never re-comments a line it already covered. -- **Only confirmed defects.** The reviewer files a finding only when it can - point at the wrong line and name the input or state that breaks it, so a - review is a short list of real problems rather than a page of "consider - whether". It reads the surrounding code, not the diff alone, because most real - findings depend on a caller or a guard the diff does not show. -- **Comment-only.** Ellipsis posts one pull request review as the Ellipsis bot, - anchored to the commit it reviewed. It never approves, requests changes, - pushes commits, or merges, so it cannot satisfy a required-review rule. A - review that finds nothing posts a one-line summary instead of invented - nitpicks. -- **Bots are reviewed too**, minus dependabot and renovate. A dependency bump is - exactly the change nobody reads closely. -- The built-in pipeline is two agents: a Haiku `pr-description` agent that keeps - the pull request description's summary current, and one Opus reviewer named - `bugs`. The stages are `pre_review`, `description`, `review`, deduplication, - `filter`, `post_review`; only `description` and `review` are populated by - default. -- Reviewers never post. Each writes findings to a file, and the platform posts. - A reviewer prompt is the reviewing brain only: Ellipsis supplies the commit - range, so never restate the scope and never tell a reviewer to post to GitHub. - -### The pipeline file - -One optional file customizes the review. **There is one filename, -`code_review.yaml`, and where you commit it decides what it governs:** - -- At the root of a repository: governs that repository. `.ellipsis/code_review.yaml` - also works for its own repository, and the root path wins if both exist. -- At the root of the repository literally named `.ellipsis`: governs every - repository in the organization. That repository is never itself reviewed. -- Anywhere else: a configuration error, not a file that silently reviews - nothing. - -First hit wins, and a repository's own file **replaces** the organization-wide -one rather than merging with it, so copy across whatever you meant to keep. That -also means an organization file's filters are not a ceiling: a repository the -organization file excluded can review itself by committing its own file. - -The file is an overlay on the built-in pipeline, so it declares only what it -changes: - -```yaml -# code_review.yaml at the root of the .ellipsis repository -ellipsis: - version: v1 - kind: code_review - name: Backend review - -pull_requests: - repositories: [api] # valid only in the .ellipsis repository's copy - base: [main, "release/*"] - draft: false - -review: - - name: migration-safety - harness: - type: claude_code - instructions: | - Review database migrations for production safety. Check for - locking that blocks writes on large tables, missing backfills for - new non-null columns, and rollout ordering that breaks if the - migration and the code deploy out of order. - pull_requests: - paths: ["**/migrations/**"] - -filter: - name: strict-gate - harness: - type: claude_code - instructions: | - Approve only findings a staff engineer would raise in review. - Reject style opinions and anything a linter catches. - -budget: - run: 15.00 - day: 100.00 -``` - -Merge rules that catch people out: - -- **`ellipsis.kind: code_review` marks the file**, and the path decides its - scope. A pipeline file is not an agent config and is not synced from `agents/`. -- **Declaring `pull_requests:` makes it authoritative.** A pull request the - governing file does not match gets no review at all, rather than falling back - to the built-in pipeline. It also narrows the audience to humans unless the - block writes `for` back out, because the block replaces the default wholesale. -- **Declaring a stage list replaces that stage wholesale.** There is no - "append to the built-in reviewers" knob. A file wanting a specialist beside a - general pass declares both reviewers itself. -- **Unset and empty differ where the built-in ships a stage.** No `review:` key - inherits the built-in reviewer. No `description:` key inherits the built-in - description agent, and `description: []` is the only way to stop it. For - `filter:` both unset and `[]` mean no gatekeeper, since nothing gates findings - unless you declare one. -- **`enabled: false` does not suppress review.** It marks the file inactive, so - Ellipsis reads it as no policy and continues to the organization file, then the - built-in pipeline. To stop reviews in one repository, commit a file whose - `pull_requests:` matches nothing, such as `for: {users: false, bots: false}`. -- `description` and `filter` are each exactly one agent, never a list with - entries. At most 8 reviewers, each with a unique name. Reviewers run in - parallel, and a reviewer whose own `pull_requests` filters exclude a pull - request costs nothing. -- `environment:` and `budget:` merge field by field. -- `budget.run` (default $10) caps one whole review across every stage, divided - among its agents. `budget.day` and `budget.week` are trailing caps checked - before a review starts, which is the guard against a push storm. - -An optional `filter` gatekeeper judges every finding before it posts and rejects -claims that do not hold against the code, are handled elsewhere, are style -preferences, or are speculative. Rejected findings stay visible on the reviews -dashboard with the reason. It is off by default because one careful reviewer -leaves a second pass little to arbitrate, and that pass doubles every review's -cost and latency. - ## The agent CLI One open-source binary named `ellipsis` (`el` for short), a terminal client for the same API @@ -418,22 +288,6 @@ Search covers transcripts, recaps, and pull request references, with embedding similarity alongside full text, so one agent's investigation compounds into team knowledge. Facets cover repository, author, agent, status, source, and date. -Review pull requests on demand, without waiting for a push: - -```sh -ellipsis review 519 # review a pull request by number -ellipsis review 519 --full # re-review the whole PR, ignoring earlier reviews -ellipsis review 519 --no-post # print findings instead of posting to GitHub -ellipsis review list --repo api # a repository's reviews, newest first -ellipsis review get # one review's findings, scope, and whether it posted -ellipsis review init # scaffold code_review.yaml for this repository -``` - -Which pipeline runs is not a parameter. An explicit review resolves the same -pipeline by location that the webhook does, so the two entry points can never -disagree. A review with nothing new to cover returns a `skipped` review rather -than an error. - Author and deploy agents: ```sh @@ -469,9 +323,8 @@ ellipsis github repos # also github members, slack cha # linear teams, sentry orgs ``` -Most singular commands accept the plural spelling as a hidden alias, and -`review` also answers to `cr`. `ellipsis --help` and `ellipsis --help` are -authoritative for flags. +Most singular commands accept the plural spelling as a hidden alias. +`ellipsis --help` and `ellipsis --help` are authoritative for flags. ## Writing a config @@ -664,9 +517,6 @@ is every page in one file. - Permissions: https://www.ellipsis.dev/docs/cloud-agents/permissions - Conversations: https://www.ellipsis.dev/docs/cloud-agents/conversations - Skills: https://www.ellipsis.dev/docs/cloud-agents/skills -- Code review: https://www.ellipsis.dev/docs/code-review -- Review pipeline reference: https://www.ellipsis.dev/docs/code-review/configuration-yaml -- Which PRs get reviewed: https://www.ellipsis.dev/docs/code-review/which-prs-get-reviewed - CLI reference: https://www.ellipsis.dev/docs/cli - REST API reference: https://www.ellipsis.dev/docs/api - Models: https://www.ellipsis.dev/docs/models diff --git a/src/cli.ts b/src/cli.ts index 7e5c8e7..d4d3e57 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -2,7 +2,6 @@ import { Command } from 'commander' import { registerAuth } from './commands/auth' import { registerHost } from './commands/host' import { registerSession } from './commands/session' -import { registerReview } from './commands/review' import { registerAutomation } from './commands/automation' import { registerEnvironment } from './commands/environment' import { registerVariable } from './commands/variable' @@ -37,7 +36,6 @@ configureCliHelp(program) registerAuth(program) registerHost(program) registerSession(program) -registerReview(program) registerAutomation(program) registerEnvironment(program) registerVariable(program) diff --git a/src/commands/review.ts b/src/commands/review.ts deleted file mode 100644 index 7c4da0e..0000000 --- a/src/commands/review.ts +++ /dev/null @@ -1,437 +0,0 @@ -import { type Command } from 'commander' -import { existsSync, mkdirSync, writeFileSync } from 'node:fs' -import { basename, dirname, extname } from 'node:path' -import { api, APIError } from '../lib/api' -import { alsoKnownAs, apiRoutes } from '../lib/help' -import { repoFromCwd } from '../lib/git' -import { formatTs, printJson, printTable, relativeAge, runAction, usdFromMillicents } from '../lib/output' -import { followConversation } from './session' -import type { Ellipsis } from '@ellipsis-dev/sdk' -import type { - CodeReviewRunStatus, - CreateReviewRequest, - Finding, - Review, - ReviewScope, -} from '../lib/types' - -// `ellipsis review`: ask for a code review now, instead of waiting for a push to -// trigger one. -// -// ellipsis review 5975 review that pull request -// -// 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. -// -// Which pipeline runs is not a parameter. It is resolved from the repository's -// committed `code_review.yaml` (see `ellipsis 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[]`. - -// 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 on demand'), - 'reviews', - 'code-review', - 'cr', - ) - - apiRoutes( - review - .command('start ', { isDefault: true }) - .description('Review a pull request by number'), - 'POST /v1/reviews', - 'WS /v1/sessions/{id}/stream', - 'GET /v1/reviews/{id}', - ) - .option('--repo ', 'repository to review (default: this git remote)') - .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('--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, opts: StartOptions) => { - await runAction(async () => { - const client = api() - const request = buildCreateRequest(pullRequest, opts) - const started = await client.reviews.create(request) - - // Nothing new since the last review of this PR. Not an error — you - // asked, and the honest answer is "already covered". - if (started.status === 'skipped') { - if (opts.json) printJson(started) - else { - console.log( - `already reviewed at ${(started.scope.watermark ?? '').slice(0, 7)} — ` + - 'nothing new to review (use --full to re-review the whole PR)', - ) - } - return - } - - if (!opts.wait) { - if (opts.json) printJson(started) - else { - console.log(`✓ started review ${started.id}`) - console.log(` follow with: ellipsis review get ${started.id}`) - } - return - } - - // Follow the conversation to its close, then re-read: the findings - // are collected when the review's environment is torn down, after - // its turn ended, so they only exist once the conversation closes. - // Same two-step `ellipsis file get` uses. - if (!opts.json) { - console.log( - `✓ reviewing ${request.owner}/${request.repo}#${started.pull_request.number} ` + - `(${started.id})`, - ) - } - await followConversation(client, started.id, false) - const finished = await client.reviews.get(started.id) - if (opts.json) printJson(finished) - else renderReview(finished) - }) - }) - - apiRoutes( - review - .command('get ') - .description("Print a review's findings, scope, and whether it posted"), - 'GET /v1/reviews/{id}', - ) - .option('--json', 'output raw JSON') - .action(async (reviewId: string, opts: { json?: boolean }) => { - await runAction(async () => { - const found = await getReviewOrExplain(api(), reviewId) - if (opts.json) printJson(found) - else renderReview(found) - }) - }) - - apiRoutes( - alsoKnownAs( - review.command('list').description("List a pull request's reviews, newest first"), - 'ls', - ), - 'GET /v1/reviews', - ) - .option('--repo ', 'only reviews of this repository') - .option('--pr ', 'only reviews of this pull request number', parsePositiveInt) - .option('-s, --status ', 'only reviews in this status') - .option('-l, --limit ', 'max results (server cap: 200)', parsePositiveInt) - .option('--json', 'output raw JSON') - .action(async (opts: ListOptions) => { - await runAction(async () => { - // Every filter is a set on the wire; the flags take one value each. - if (opts.repo) splitRepo(opts.repo) - const reviews = ( - await api().reviews.list({ - repository: opts.repo ? [opts.repo] : undefined, - pull_request_number: opts.pr !== undefined ? [opts.pr] : undefined, - status: opts.status ? [opts.status] : undefined, - limit: opts.limit, - }) - ).items - if (opts.json) { - printJson(reviews) - return - } - if (reviews.length === 0) { - console.log('No reviews.') - return - } - printTable( - ['ID', 'PR', 'STATUS', 'SCOPE', 'FINDINGS', 'POSTED', 'COST', 'AGE'], - reviews.map((r) => [ - r.id, - `${r.repository.owner}/${r.repository.name}#${r.pull_request.number}`, - r.status, - scopeWord(r), - r.counters ? String(r.counters.n_parsed) : '-', - r.posted_review_id ? String(r.posted_review_id) : r.post_error ? 'failed' : '-', - usdFromMillicents(r.cost_millicents), - r.created_at ? relativeAge(r.created_at) : '-', - ]), - ) - }) - }) - - registerReviewInit(review) -} - -// `ellipsis review init`: the code review twin of `ellipsis automation init`. Scaffolds a -// starter pipeline YAML locally; you commit it and Ellipsis syncs it from -// GitHub. No API call and no pull request, because `ellipsis automation create` posts -// an automation and a pipeline is a different kind of file. -function registerReviewInit(review: Command): void { - review - .command('init [path]') - .description( - `Scaffold a starter code review pipeline YAML locally (default: ${DEFAULT_PIPELINE_PATH})`, - ) - // No `-f` short: CLI-wide, `-f` means an input file. - .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 - } - mkdirSync(dirname(target), { recursive: true }) - writeFileSync(target, starterPipeline(pipelineName())) - console.log(`✓ wrote ${target}`) - console.log( - 'Commit it to your default branch. Ellipsis syncs code review pipelines from GitHub.', - ) - }) -} - -// 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]} 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. -// -// 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. 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 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: a pull request description writer plus one bug reviewer. -# -# review: -# - name: migration-safety -# harness: -# type: claude_code -# instructions: | -# Review SQL migrations for locks that block writes on a large table. -# pull_requests: -# paths: ["sql/migrations/**"] -# -# Declaring a filter stage adds a gatekeeper that judges what the reviewers -# found. There is no gatekeeper unless you declare one. -# -# filter: -# name: gatekeeper -# harness: -# type: claude_code -# instructions: | -# Drop any finding that is not worth the author's time. - -budget: - run: 2.00 - day: 20.00 - week: 75.00 -` -} - -interface StartOptions { - repo?: string - full?: boolean - watermark?: string - head?: string - // commander sets these false for --no-* flags. - post: boolean - wait: boolean - cwd?: string - json?: boolean -} - -interface ListOptions { - repo?: string - pr?: number - status?: CodeReviewRunStatus - limit?: number - json?: boolean -} - -// Assemble the request. Exported for tests. -export function buildCreateRequest( - pullRequest: string, - opts: StartOptions, -): CreateReviewRequest { - const cwd = opts.cwd ?? process.cwd() - const repo = splitRepo(opts.repo ?? repoFromCwdOrThrow(cwd)) - return { - owner: repo.owner, - repo: repo.name, - scope: buildScope(opts), - pull_request_number: parsePullRequest(pullRequest), - post: opts.post, - } -} - -// `--full`/`--watermark`/`--head` → the scope model. Default is incremental: -// only the commits since the last review, which is what the webhook does. -function buildScope(opts: StartOptions): ReviewScope { - return { - kind: opts.full ? 'full' : 'incremental', - ...(opts.watermark ? { watermark: opts.watermark } : {}), - ...(opts.head ? { head: opts.head } : {}), - } -} - -async function getReviewOrExplain(client: Ellipsis, reviewId: string): Promise { - try { - return await client.reviews.get(reviewId) - } catch (err) { - // 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 crun_…)`) - } - throw err - } -} - -function renderReview(review: Review): void { - console.log(`review: ${review.id}`) - console.log( - `pr: ${review.repository.owner}/${review.repository.name}` + - `#${review.pull_request.number} ${review.pull_request.url}`, - ) - console.log(`status: ${review.status}`) - console.log(`scope: ${scopeWord(review)}`) - if (review.posted_review_id) console.log(`posted: ${review.posted_review_id}`) - if (review.post_error) console.log(`post: failed — ${review.post_error}`) - console.log(`cost: ${usdFromMillicents(review.cost_millicents)}`) - if (review.completed_at) console.log(`completed: ${formatTs(review.completed_at)}`) - - if (review.review_body) console.log(`\n${review.review_body.trim()}`) - - const findings = review.findings ?? [] - if (findings.length === 0) { - // Distinguish "clean" from "not collected yet": the outbox row only exists - // once the review finalizes. - console.log(review.counters ? '\nNo findings.' : '\nStill running — no findings yet.') - return - } - console.log('') - // Highest severity first, the order the platform posts them in. - for (const finding of [...findings].sort((a, b) => b.severity - a.severity)) { - console.log(formatFinding(finding)) - } - const counters = review.counters - if (counters && counters.n_dropped > 0) { - console.log(`(${counters.n_dropped} finding(s) could not be parsed)`) - } -} - -// One finding as a `path:line severity category` header plus its claim, so the -// output greps like a compiler's. Exported for tests. -export function formatFinding(finding: Finding): string { - const lines = - finding.end_line > finding.start_line - ? `${finding.start_line}-${finding.end_line}` - : String(finding.start_line) - const head = `${finding.path}:${lines} [${finding.severity}/5 ${finding.category}]` - const body = [finding.claim, finding.evidence, finding.suggested_fix] - .filter((part): part is string => Boolean(part && part.trim())) - .map((part) => indent(part.trim())) - .join('\n') - // Anchored off the diff means it was recorded but never posted inline. - const note = - finding.anchor === 'not_commentable' - ? indent('(outside the diff — recorded, not posted)') - : '' - return [head, body, note].filter(Boolean).join('\n') + '\n' -} - -function indent(text: string): string { - return text - .split('\n') - .map((line) => ` ${line}`) - .join('\n') -} - -function scopeWord(review: Review): string { - const { watermark, head } = review.scope - return `${(watermark ?? 'base').slice(0, 7)}...${(head ?? '').slice(0, 7)}` -} - -function repoFromCwdOrThrow(cwd: string): string { - const repo = repoFromCwd(cwd) - if (!repo) { - throw new Error( - 'not inside a git repository with an origin remote — pass --repo ', - ) - } - return repo -} - -export function splitRepo(value: string): { owner: string; name: string } { - const [owner, name, ...rest] = value.split('/') - if (!owner || !name || rest.length > 0) { - throw new Error(`--repo must be owner/name (got '${value}')`) - } - return { owner, name } -} - -// Accept `123` and `#123`, and a full PR URL, since all three get pasted. -export function parsePullRequest(raw: string): number { - const match = /^#?(\d+)$/.exec(raw.trim()) ?? /\/pull\/(\d+)/.exec(raw.trim()) - if (!match) { - // `review` reserves the word, so `ellipsis 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 (ellipsis review 123). ` + - 'To run an agent with a prompt that starts with "review", quote it: ' + - `ellipsis "review ${raw} …"`, - ) - } - return Number.parseInt(match[1], 10) -} - -function parsePositiveInt(raw: string): number { - const n = Number.parseInt(raw, 10) - if (!Number.isFinite(n) || n <= 0) throw new Error(`invalid count '${raw}'`) - return n -} diff --git a/src/commands/session.ts b/src/commands/session.ts index bcd6689..f3ff673 100644 --- a/src/commands/session.ts +++ b/src/commands/session.ts @@ -594,18 +594,6 @@ export async function watchTurn( } } -// Follow a session's whole conversation live until it closes. A session that -// runs once closes after its turn ended and the platform's teardown work is -// done: a review's findings are collected then, so `ellipsis review` waits -// for the close rather than the turn's end. -export async function followConversation( - client: Ellipsis, - sessionId: string, - json?: boolean, -): Promise { - await streamFrames(client, sessionId, null, FALLBACK_POLL_INTERVAL_SECONDS, json) -} - // The watch's last word: one line naming how the turn ended, and the exit // code that goes with it. `--json` callers have already printed the turn. function endWatch(sessionId: string, turn: TurnEnd, json?: boolean): void { diff --git a/src/lib/help.ts b/src/lib/help.ts index c9ff2e6..07125a8 100644 --- a/src/lib/help.ts +++ b/src/lib/help.ts @@ -18,7 +18,7 @@ function withoutAliases(term: string, cmd: Command): string { // missing from every group still renders (under "Other") rather than silently // vanishing from help. const TOP_LEVEL_GROUPS: ReadonlyArray<{ title: string; commands: readonly string[] }> = [ - { title: 'Sessions', commands: ['session', 'review'] }, + { title: 'Sessions', commands: ['session'] }, { title: 'Automations', commands: ['automation', 'model', 'template'] }, { title: 'Platform', commands: ['variable'] }, { title: 'Integrations', commands: ['integration', 'github', 'slack', 'linear', 'sentry'] }, diff --git a/src/lib/types.ts b/src/lib/types.ts index 6a85746..a2a0226 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -62,19 +62,6 @@ export type ListAgentTemplatesResponse = S['AgentTemplatesListResponse'] export type GetSupportedModelsResponse = S['ModelsListResponse'] -// -------------------------------- reviews --------------------------------- -// A review's `id` IS a session id, so the session types above apply to it -// unchanged — hence no review-specific status, stream, or cost type. - -export type Review = S['Review'] -export type ReviewScope = S['ReviewScope'] -export type ResolvedReviewScope = S['ResolvedReviewScope'] -export type ReviewCounters = S['ReviewCounters'] -export type Finding = S['ReviewFinding'] -export type CreateReviewRequest = S['CreateReviewRequest'] -export type ListReviewsResponse = S['ReviewsListResponse'] -export type CodeReviewRunStatus = S['CodeReviewRunStatus'] - // ------------------------------- secrets ---------------------------------- // Customer-scoped environment variables injected into a sandbox when an agent // config names them. Values are write-only: the API accepts them but never @@ -166,14 +153,6 @@ export interface ListFilesQuery { limit?: number } -export interface ListReviewsQuery { - owner?: string - repo?: string - pull_request_number?: number - status?: S['CodeReviewRunStatus'] - limit?: number -} - // Shared analytics window: explicit start/end (ISO timestamps) or a `days` // look-back (mutually exclusive with start; server default: last 30 days). export interface AnalyticsWindowQuery { diff --git a/test/args.test.ts b/test/args.test.ts index 1206618..d74b553 100644 --- a/test/args.test.ts +++ b/test/args.test.ts @@ -122,11 +122,11 @@ describe('looksLikeCommandTypo', () => { }) describe('similarCommands', () => { - const commands = ['session', 'review', 'automation', 'install', 'model', 'host'] + const commands = ['session', 'variable', 'automation', 'install', 'model', 'host'] it('finds the intended command behind a typo', () => { expect(similarCommands('sesion', commands)).toEqual(['session']) - expect(similarCommands('reveiw', commands)).toEqual(['review']) + expect(similarCommands('varible', commands)).toEqual(['variable']) expect(similarCommands('instal', commands)).toEqual(['install']) }) @@ -141,7 +141,7 @@ describe('similarCommands', () => { }) describe('commandTypoMessage', () => { - const commands = ['session', 'review', 'install'] + const commands = ['session', 'variable', 'install'] it('names the likely command and how to force a prompt', () => { const msg = commandTypoMessage('sesion', commands) diff --git a/test/review.test.ts b/test/review.test.ts deleted file mode 100644 index 0c0598d..0000000 --- a/test/review.test.ts +++ /dev/null @@ -1,189 +0,0 @@ -import { execFileSync } from 'node:child_process' -import { mkdtempSync, writeFileSync } from 'node:fs' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { describe, expect, it } from 'vitest' -import { parse } from 'yaml' -import { - buildCreateRequest, - formatFinding, - parsePullRequest, - splitRepo, - starterPipeline, -} from '../src/commands/review' -import type { Finding } from '../src/lib/types' - -// A throwaway repo with an origin remote, so the local path's git work runs for -// real instead of being mocked. The "remote" is a bare repo on disk — good -// enough for `push` and for repoFromCwd's remote parsing. -function scratchRepo(branch = 'feature/thing'): { work: string; remote: string } { - const root = mkdtempSync(join(tmpdir(), 'agent-review-')) - const remote = join(root, 'remote.git') - const work = join(root, 'work') - execFileSync('git', ['init', '--bare', '-b', 'main', remote]) - execFileSync('git', ['init', '-b', 'main', work]) - const git = (...args: string[]) => execFileSync('git', ['-C', work, ...args]) - git('config', 'user.email', 'ci@example.com') - git('config', 'user.name', 'ci') - // An https URL so repoFromCwd resolves owner/name; the push target is set - // separately below, since that URL isn't reachable. - git('remote', 'add', 'origin', 'https://github.com/ellipsis-dev/scratch.git') - git('remote', 'set-url', '--push', 'origin', remote) - writeFileSync(join(work, 'a.txt'), 'one\n') - git('add', '.') - git('commit', '-m', 'first') - git('checkout', '-b', branch) - return { work, remote } -} - -const START_DEFAULTS = { post: true, wait: true, json: false } - -describe('buildCreateRequest — an existing pull request', () => { - it('sends owner and repo as separate fields, never owner/name', () => { - const req = buildCreateRequest('5975', { - ...START_DEFAULTS, - repo: 'ellipsis-dev/ellipsis', - }) - expect(req.owner).toBe('ellipsis-dev') - expect(req.repo).toBe('ellipsis') - expect(req.pull_request_number).toBe(5975) - expect(req.branch).toBeUndefined() - }) - - it('defaults to incremental — only the commits since the last review', () => { - const req = buildCreateRequest('1', { ...START_DEFAULTS, repo: 'o/r' }) - expect(req.scope.kind).toBe('incremental') - }) - - it('--full re-reviews the whole pull request', () => { - const req = buildCreateRequest('1', { ...START_DEFAULTS, repo: 'o/r', full: true }) - expect(req.scope.kind).toBe('full') - }) - - it('passes a pinned range through as SHAs', () => { - const req = buildCreateRequest('1', { - ...START_DEFAULTS, - repo: 'o/r', - watermark: 'aaaa111', - head: 'bbbb222', - }) - expect(req.scope.watermark).toBe('aaaa111') - expect(req.scope.head).toBe('bbbb222') - }) - - it('posts by default, and --no-post turns it off', () => { - expect(buildCreateRequest('1', { ...START_DEFAULTS, repo: 'o/r' }).post).toBe(true) - expect( - buildCreateRequest('1', { ...START_DEFAULTS, repo: 'o/r', post: false }).post, - ).toBe(false) - }) -}) - -describe('buildCreateRequest — repository resolution', () => { - it('needs a repo when there is no git remote to infer one from', () => { - expect(() => - buildCreateRequest('123', { ...START_DEFAULTS, cwd: mkdtempSync(join(tmpdir(), 'bare-')) }), - ).toThrow(/--repo/) - }) -}) - -describe('parsePullRequest', () => { - it('accepts the three spellings people paste', () => { - expect(parsePullRequest('5975')).toBe(5975) - expect(parsePullRequest('#5975')).toBe(5975) - expect(parsePullRequest('https://github.com/ellipsis-dev/ellipsis/pull/5975')).toBe(5975) - }) - - it('teaches the fix when `review` swallowed a bare prompt', () => { - // `ellipsis review the auth changes` reaches here because the verb reserves - // the word — the error has to name the quoted form. - expect(() => parsePullRequest('the')).toThrow(/quote it/) - }) -}) - -describe('splitRepo', () => { - it('rejects anything that is not exactly owner/name', () => { - expect(splitRepo('o/r')).toEqual({ owner: 'o', name: 'r' }) - expect(() => splitRepo('just-a-name')).toThrow(/owner\/name/) - expect(() => splitRepo('a/b/c')).toThrow(/owner\/name/) - }) -}) - -describe('formatFinding', () => { - const base: Finding = { - path: 'src/auth.py', - start_line: 42, - end_line: 42, - side: 'RIGHT', - severity: 4, - category: 'security', - claim: 'Missing authz check.', - evidence: '', - suggested_fix: null, - confidence: null, - extra: {}, - anchor: 'valid', - snapped_from: null, - in_scope: true, - } - - it('leads with a greppable path:line and the severity', () => { - expect(formatFinding(base)).toContain('src/auth.py:42 [4/5 security]') - }) - - it('renders a multi-line anchor as a range', () => { - expect(formatFinding({ ...base, end_line: 48 })).toContain('src/auth.py:42-48') - }) - - it('includes the evidence and the suggested fix', () => { - const out = formatFinding({ - ...base, - evidence: 'no membership assert', - suggested_fix: 'assert_membership(user)', - }) - expect(out).toContain('no membership assert') - expect(out).toContain('assert_membership(user)') - }) - - it('flags a finding that was recorded but never posted inline', () => { - const out = formatFinding({ ...base, anchor: 'not_commentable' }) - expect(out).toContain('outside the diff') - }) -}) - -describe('starterPipeline', () => { - it('marks the file as a pipeline, not an agent', () => { - expect(starterPipeline('cli code review')).toContain('kind: code_review') - }) - - it('parses as YAML and only sets keys the schema allows', () => { - 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' }) - }) - - // 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('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/') - }) -})