From 6f8cbd2054e31646b362278b7a1bbcfe88985c34 Mon Sep 17 00:00:00 2001 From: hbrooks Date: Mon, 10 Aug 2026 18:55:44 -0400 Subject: [PATCH] composer: let the repository picker check out zero, one, or many repos The Repository row's toggle was already multi-select, but its resting state lied: an empty selection MEANT "let the server resolve it" while RENDERING as [x] on the detected repo. Checking a second repo therefore produced a set of one, silently dropping the repo that looked selected, and unchecking everything was indistinguishable from the default. The selection is now null when untouched, and the first toggle seeds from the detected repo, so the checkboxes on screen are the checkout you get. The row reads "none" for the no-checkout sandbox. Two related bugs, either of which would have made that fix invisible: The override key was dead. The backend renamed sandbox: to environment: and the config model forbids extra keys, so every repo pick was rejected at validation. The same stale key sat behind --repo/--cpu/--memory/--timeout on 'session start'. The context repo overrode the selection. The server merges the request's `repository` into the checkout unconditionally, even under an explicit config, so an empty set could never hold; it is dropped when the selection excludes it. Request building moves to applyComposerChoices so the three cases are testable without mounting the pane. The public skill documented the old sandbox: schema (and github: under it, now top-level permissions:), which would have generated configs the API rejects. --- skills/ellipsis/SKILL.md | 25 +++++++----- src/commands/session.tsx | 10 ++--- src/lib/sessions.ts | 45 ++++++++++++++++++++- src/ui/SessionsApp.tsx | 84 +++++++++++++++++++--------------------- test/session.test.ts | 2 +- test/sessions.test.ts | 78 +++++++++++++++++++++++++++++++++++++ 6 files changed, 183 insertions(+), 61 deletions(-) diff --git a/skills/ellipsis/SKILL.md b/skills/ellipsis/SKILL.md index c3446c3..686e130 100644 --- a/skills/ellipsis/SKILL.md +++ b/skills/ellipsis/SKILL.md @@ -115,10 +115,12 @@ trigger: type: cron schedule: "0 9 * * 1" -sandbox: +environment: repositories: - name: api - name: web + +permissions: github: permissions: read_only @@ -200,9 +202,11 @@ trigger: base: [default] paths: ["migrations/**"] -sandbox: +environment: repositories: - name: api + +permissions: github: permissions: contents: read @@ -224,7 +228,7 @@ budget: - `for` gates the author. The default is `users: true, bots: false`, so bot-authored events never trigger an agent unless you opt in. - Trigger `repositories` is the watch set and is independent of - `sandbox.repositories`, the clone set. The triggering repository is always + `environment.repositories`, the clone set. The triggering repository is always cloned. - Sentry re-fires inside a 6 hour per-issue window append to the existing conversation, so an alert storm produces one investigation, not dozens of @@ -345,7 +349,7 @@ Merge rules that catch people out: 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. -- `sandbox:` and `budget:` merge field by field. +- `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. @@ -489,7 +493,8 @@ Top-level keys, all optional except `ellipsis`: | `claude` | `system`, `model`, `effort`, `fallback_model`, `max_turns`, `settings`. | | `codex` | Run on OpenAI's Codex CLI instead. Declaring the block selects the harness. | | `trigger` | One trigger, or omit for a manual-only agent. | -| `sandbox` | `repositories`, `variables`, `ports`, `compute`, `image`, `hooks`, `github`. | +| `environment` | Where the agent runs: `repositories`, `variables`, `ports`, `compute`, `image`, `hooks`. | +| `permissions` | What it may do: `github` scopes its GitHub token, `ellipsis` its API token. | | `skills` | Claude Code skills beyond what the cloned repositories provide. | | `structured_output` | A JSON Schema contract, so downstream automation gets typed data. | | `budget` | `session`, `day`, `week`, `month`, in US dollars. | @@ -532,7 +537,7 @@ repositories already cloned and destroyed when the session ends. The base image carries Python 3.13, Node.js 22, `git`, the `gh` CLI, `curl`, and a C/C++ toolchain. Your agents can build and test your product, not just read it. -Three `sandbox` fields define the environment, each with a different lifetime: +Three `environment` fields define the sandbox, each with a different lifetime: - `image.dockerfile_append`: `RUN` layers on the managed base image, before any repository exists. Use it for toolchain installs. Only `RUN` is accepted. @@ -551,7 +556,7 @@ dependencies. `agent session start --config-file --rebuild --watch` provisions through a fresh full build and streams every phase, which is how you prove an environment before merging. -`sandbox.compute` sizes the machine: `cpu` 0.125 to 16, `memory` 512MB to 64GB, +`environment.compute` sizes the machine: `cpu` 0.125 to 16, `memory` 512MB to 64GB, `timeout` 60s to 1h. Defaults are 1 vCPU, 4GB, and 1h. One hour is also the maximum, because a sandbox never outlives its GitHub token. Compute bills on the requested allocation over the sandbox's lifetime, so size up only when the @@ -561,15 +566,15 @@ Credentials are scoped and short-lived: - Each sandbox gets its own `GH_TOKEN`, minted from the GitHub App installation, living one hour, covering only the sandbox's repositories, and dying with the - sandbox. `sandbox.github.permissions` narrows it further, either the string + sandbox. `permissions.github.permissions` narrows it further, either the string `read_only` (read on contents, issues, metadata, pull requests) or a map such as `{contents: read, pull_requests: write}`. GitHub mints the reduced token, so nothing in the sandbox can exceed it, not a misbehaving tool and not a prompt - injection in a pull request description. `sandbox.github.repositories` narrows + injection in a pull request description. `permissions.github.repositories` narrows which repositories the token may touch, independently of what is cloned. Because permissions are YAML in git, every agent's blast radius is explicit and reviewed. -- Other credentials enter as `sandbox.variables`. Store the value once with +- Other credentials enter as `environment.variables`. Store the value once with `agent variable set`, then name it in the config. The name list is the scope, so only agents that name a variable receive it, and a compromised agent never sees the inventory. Stored values are write-only and never readable back diff --git a/src/commands/session.tsx b/src/commands/session.tsx index a4c1136..c863e29 100644 --- a/src/commands/session.tsx +++ b/src/commands/session.tsx @@ -1111,10 +1111,10 @@ export function buildStartOverride(opts: { if (opts.cpu !== undefined) compute.cpu = opts.cpu if (opts.memory !== undefined) compute.memory = opts.memory if (opts.timeout !== undefined) compute.timeout = opts.timeout - const sandbox: Record = {} - if (Object.keys(compute).length) sandbox.compute = compute - if (opts.repo && opts.repo.length) sandbox.repositories = opts.repo.map(parseRepo) - if (Object.keys(sandbox).length) sugar.sandbox = sandbox + const environment: Record = {} + if (Object.keys(compute).length) environment.compute = compute + if (opts.repo && opts.repo.length) environment.repositories = opts.repo.map(parseRepo) + if (Object.keys(environment).length) sugar.environment = environment if (opts.budget !== undefined) sugar.budget = { session: opts.budget } @@ -1122,7 +1122,7 @@ export function buildStartOverride(opts: { return Object.keys(merged).length ? merged : undefined } -// Parse a --repo value into a sandbox.repositories entry. "owner/name" sets +// Parse a --repo value into an environment.repositories entry. "owner/name" sets // both; a bare "name" omits owner so the server defaults it to the account. function parseRepo(value: string): { name: string; owner?: string } { const parts = value.split('/') diff --git a/src/lib/sessions.ts b/src/lib/sessions.ts index 767b53b..659eaf6 100644 --- a/src/lib/sessions.ts +++ b/src/lib/sessions.ts @@ -1,7 +1,7 @@ import { sessionStatusWord } from '@ellipsis-dev/sdk/stream' import type { AgentSessionWire } from '@ellipsis-dev/sdk' import { theme } from './theme' -import type { AgentSession, SupportedModel } from './types' +import type { AgentSession, StartAgentSessionRequest, SupportedModel } from './types' // Pure session-model helpers shared by the connect command and the // multi-session UI (SessionsApp). No I/O here — everything is testable. @@ -284,6 +284,49 @@ export function repoOverrideEntry(fullName: string): { owner: string; name: stri return { owner, name } } +// The composer's picks, as the new-session pane reports them. `repos` null = +// the Repository row was never touched, so the server's own resolution stands; +// an array is an explicit checkout set, and [] is the legitimate "no repository +// at all" sandbox. +export interface ComposerChoices { + configId: string | null + model: string | null + repos: string[] | null +} + +// The entry point's base request with the composer's picks layered on: a saved +// config as the source, the model + repositories as a per-run config override +// (the dashboard composer's shape). +export function applyComposerChoices( + base: StartAgentSessionRequest, + choices: ComposerChoices, +): StartAgentSessionRequest { + const req: StartAgentSessionRequest = { ...base } + if (choices.configId) req.config_id = choices.configId + const override: Record = {} + if (choices.model) override.claude = { model: choices.model } + if (choices.repos !== null) { + // Lists replace wholesale in a config override, so this set becomes the + // run's entire checkout — including the empty set, which a sandbox + // supports (zero, one, or many repositories are all valid). + override.environment = { + repositories: choices.repos + .map(repoOverrideEntry) + .filter((e): e is { owner: string; name: string } => e !== null), + } + // The server merges the request's `repository` context into the checkout + // unconditionally, even under an explicit config, so leaving it on would + // re-add a repo the user just unchecked. Dropping it also moves default- + // config resolution off that repo's rung, which is the honest reading of + // "not this one". + if (req.repository !== undefined && !choices.repos.includes(req.repository)) { + delete req.repository + } + } + if (Object.keys(override).length > 0) req.config_override = override + return req +} + // ------------------------------- layout --------------------------------- // Which slice of the session cells renders when the list overflows the diff --git a/src/ui/SessionsApp.tsx b/src/ui/SessionsApp.tsx index 59391f4..4605da4 100644 --- a/src/ui/SessionsApp.tsx +++ b/src/ui/SessionsApp.tsx @@ -23,12 +23,13 @@ import { applyEditShortcut } from '../lib/editing' import { hyperlink, sessionUrl } from '../lib/urls' import { usdNumberFromMillicents } from '../lib/output' import { + applyComposerChoices, attentionFlip, compactTokens, composerModelOptions, configDisplayName, connectability, - repoOverrideEntry, + type ComposerChoices, rowDescription, rowGlyph, rowMeta, @@ -318,27 +319,11 @@ export function SessionsApp(props: SessionsAppProps): React.ReactElement { }, [mainPane.type, api]) const startSession = useCallback( - async ( - prompt: string, - choices: { configId: string | null; model: string | null; repos: string[] }, - ): Promise => { + async (prompt: string, choices: ComposerChoices): Promise => { setStarting(true) setStartError(null) try { - // The entry point's base request (prompt + detected repository), - // with the composer's picks layered on: a saved config as the - // source, the model + repositories as a per-run override (the - // dashboard composer's shape — lists replace wholesale, so the - // checked repos become the run's whole checkout set). - const req = props.buildStartRequest(prompt) - if (choices.configId) req.config_id = choices.configId - const override: Record = {} - if (choices.model) override.claude = { model: choices.model } - const repoEntries = choices.repos - .map(repoOverrideEntry) - .filter((e): e is { owner: string; name: string } => e !== null) - if (repoEntries.length > 0) override.sandbox = { repositories: repoEntries } - if (Object.keys(override).length > 0) req.config_override = override + const req = applyComposerChoices(props.buildStartRequest(prompt), choices) const session = await api.startAgentSession(req) lastWords.current.set(session.id, rowStatusWord(session)) setLocalSessions((prev) => [session, ...prev]) @@ -771,8 +756,10 @@ const PICKER_ROWS: readonly PickerRow[] = [ // enter/space) activates the // highlighted option, ← (or esc) backs out unchanged. "Default" everywhere // means the server resolves it (defaults ladder, DEFAULT_AGENT_MODEL, the -// detected repo). Esc — or ↓ / ← at the prompt's left edge — hands focus to -// the session nav. +// detected repo). Repository is the one multi-select, and a sandbox takes any +// number of repos: check several to clone them all, or uncheck every one for a +// sandbox with no checkout (the row then reads "none"). Esc — or ↓ / ← at the +// prompt's left edge — hands focus to the session nav. function NewSessionPane({ width, height, @@ -799,10 +786,7 @@ function NewSessionPane({ // The cwd's repo ("owner/name") — what the server's default resolution // checks out; named on the Repository row instead of a bare "Default". detectedRepo: string | null - onSubmit: ( - text: string, - choices: { configId: string | null; model: string | null; repos: string[] }, - ) => void + onSubmit: (text: string, choices: ComposerChoices) => void onLeave: () => void rawMode: boolean }): React.ReactElement { @@ -817,9 +801,12 @@ function NewSessionPane({ // Single-pick indices; 0 is always "Default" (server-resolved). const [configIdx, setConfigIdx] = useState(0) const [modelIdx, setModelIdx] = useState(0) - // The multi-select repository set ("owner/name" full names). Empty = - // Default (the detected repo, server-resolved). - const [repoSel, setRepoSel] = useState>(new Set()) + // The multi-select repository set ("owner/name" full names). null = the + // picker is untouched: the run inherits the server's resolution (the + // detected repo + whatever the resolved config declares). The first toggle + // materializes an explicit set — zero, one, or many repos are all valid — + // seeded with the detected repo, since that is what the [x] showed. + const [repoSel, setRepoSel] = useState | null>(null) // The open row's dropdown state: which picker is open and where its // highlight sits. null = no subtree open. const [openPicker, setOpenPicker] = useState<{ key: PickerRow['key']; hover: number } | null>( @@ -838,10 +825,10 @@ function NewSessionPane({ const modelOptions = useMemo(() => composerModelOptions(models ?? []), [models]) // When the cwd names a repo there is no "Default" row: the detected repo // heads the list as a normal checkable entry — it reads [x] while the - // selection is empty (the server checks it out by default) and can be - // checked alongside any others (repositories multi-select). Only with no - // detection does the null Default row appear (the server still resolves - // one, but there's no name to show). + // selection is untouched (the server checks it out by default) and can be + // unchecked, or checked alongside any others (repositories multi-select). + // Only with no detection does the null Default row appear (the server still + // resolves the checkout, but there's no name to show). const repoOptions = useMemo(() => { const listed = (repos ?? []).filter((r) => r !== detectedRepo) return detectedRepo @@ -853,22 +840,28 @@ function NewSessionPane({ }, [repos, detectedRepo]) const optionsFor = (key: PickerRow['key']) => key === 'config' ? configOptions : key === 'model' ? modelOptions : repoOptions - // Whether an option is currently picked. Repo is a multi-select: an empty - // selection means the server's default checkout, so the detected repo (or - // the null Default row) reads [x] while nothing is explicitly checked; the + // The set a toggle starts from: an explicit selection once one exists, else + // what the untouched row was already showing as checked (the detected repo). + // Without this seed, checking a SECOND repo would silently drop the first. + const repoBaseSet = (prev: ReadonlySet | null): Set => + new Set(prev ?? (detectedRepo ? [detectedRepo] : [])) + // Whether an option is currently picked. Repo is a multi-select: an + // untouched selection is the server's default checkout, so the detected repo + // (or the null Default row) reads [x] until you touch the list; the // single-pickers match their index. const isPicked = (key: PickerRow['key'], at: number): boolean => { if (key === 'repo') { const id = repoOptions[at]?.id - if (repoSel.size === 0) return id === null || id === detectedRepo + if (repoSel === null) return id === null || id === detectedRepo return id !== null && repoSel.has(id) } const idx = key === 'config' ? configIdx : modelIdx return at === Math.min(idx, optionsFor(key).length - 1) } // Activating an option: single-pickers pick and close; the repo list - // TOGGLES the entry ([x]↔[ ]) and stays open so several can be checked - // (the null Default row, shown only with no detected repo, clears the set). + // TOGGLES the entry ([x]↔[ ]) and stays open so several can be checked, or + // all of them unchecked for a sandbox with no checkout. The null Default row + // (shown only with no detected repo) hands resolution back to the server. const activate = (key: PickerRow['key'], at: number): void => { if (key === 'config') { setConfigIdx(at) @@ -878,10 +871,10 @@ function NewSessionPane({ setOpenPicker(null) } else { const id = repoOptions[at]?.id - if (id == null) setRepoSel(new Set()) + if (id == null) setRepoSel(null) else { setRepoSel((prev) => { - const next = new Set(prev) + const next = repoBaseSet(prev) if (next.has(id)) next.delete(id) else next.add(id) return next @@ -897,7 +890,7 @@ function NewSessionPane({ onSubmit(text.trim(), { configId: configOptions[Math.min(configIdx, configOptions.length - 1)]?.id ?? null, model: modelOptions[Math.min(modelIdx, modelOptions.length - 1)]?.id ?? null, - repos: [...repoSel], + repos: repoSel === null ? null : [...repoSel], }) } @@ -1012,12 +1005,15 @@ function NewSessionPane({ { isActive: focused && rawMode }, ) - // The summary shown on a row: the single pick's label, or the checked - // repo set joined (the detected repo when nothing is checked). + // The summary shown on a row: the single pick's label, or the checked repo + // set joined (the detected repo while the list is untouched, "none" once you + // have explicitly unchecked everything — a sandbox with no checkout). const rowValue = (key: PickerRow['key']): string => { if (key === 'repo') { if (repos === null) return 'loading…' - return repoSel.size === 0 ? (detectedRepo ?? 'Default') : [...repoSel].join(', ') + if (repoSel === null) return detectedRepo ?? 'Default' + if (repoSel.size === 0) return 'none' + return [...repoSel].join(', ') } if (key === 'config' && configs === null) return 'loading…' const options = optionsFor(key) diff --git a/test/session.test.ts b/test/session.test.ts index a00f0b8..46a601b 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -210,7 +210,7 @@ describe('buildStartOverride', () => { }), ).toEqual({ claude: { model: 'claude-opus-4-8', system: 'do the thing' }, - sandbox: { + environment: { compute: { cpu: 2, memory: '8GB', timeout: '30m' }, repositories: [{ owner: 'ellipsis-dev', name: 'ellipsis' }, { name: 'solo' }], }, diff --git a/test/sessions.test.ts b/test/sessions.test.ts index 376c91a..1b62301 100644 --- a/test/sessions.test.ts +++ b/test/sessions.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + applyComposerChoices, attentionFlip, compactTokens, COMPOSER_MODELS, @@ -384,3 +385,80 @@ describe('composerModelOptions', () => { expect(options[1].label).toBe('claude-opus-5') }) }) + +describe('applyComposerChoices', () => { + const untouched = { configId: null, model: null, repos: null } + + it('leaves the base request alone when nothing was picked', () => { + expect(applyComposerChoices({ prompt: 'hi', repository: 'acme/api' }, untouched)).toEqual({ + prompt: 'hi', + repository: 'acme/api', + }) + }) + + it('sends no repository override while the picker is untouched', () => { + const req = applyComposerChoices({ repository: 'acme/api' }, { ...untouched, model: 'claude-opus-5' }) + expect(req.config_override).toEqual({ claude: { model: 'claude-opus-5' } }) + expect(req.repository).toBe('acme/api') + }) + + // The whole point of the fix: a sandbox takes zero, one, or many repos. + it('checks out many repositories at once', () => { + const req = applyComposerChoices( + { repository: 'acme/api' }, + { ...untouched, repos: ['acme/api', 'acme/web', 'acme/infra'] }, + ) + expect(req.config_override).toEqual({ + environment: { + repositories: [ + { owner: 'acme', name: 'api' }, + { owner: 'acme', name: 'web' }, + { owner: 'acme', name: 'infra' }, + ], + }, + }) + // Still in the set, so the context repo stays (it also picks the repo rung + // of the defaults ladder). + expect(req.repository).toBe('acme/api') + }) + + it('checks out no repository at all when every box is unchecked', () => { + const req = applyComposerChoices({ repository: 'acme/api' }, { ...untouched, repos: [] }) + expect(req.config_override).toEqual({ environment: { repositories: [] } }) + // The server merges `repository` into the checkout unconditionally, so an + // empty set only holds if the context repo goes too. + expect(req.repository).toBeUndefined() + }) + + it('drops the context repo when the selection excludes it', () => { + const req = applyComposerChoices({ repository: 'acme/api' }, { ...untouched, repos: ['acme/web'] }) + expect(req.config_override).toEqual({ + environment: { repositories: [{ owner: 'acme', name: 'web' }] }, + }) + expect(req.repository).toBeUndefined() + }) + + it('overrides under the environment key the config schema uses, not the old sandbox one', () => { + const req = applyComposerChoices({}, { ...untouched, repos: ['acme/web'] }) + expect(req.config_override).not.toHaveProperty('sandbox') + expect(req.config_override).toHaveProperty('environment') + }) + + it('carries a chosen agent config and model through', () => { + const req = applyComposerChoices( + { prompt: 'ship it' }, + { configId: 'cfg_1', model: 'claude-fable-5', repos: null }, + ) + expect(req).toEqual({ + prompt: 'ship it', + config_id: 'cfg_1', + config_override: { claude: { model: 'claude-fable-5' } }, + }) + }) + + it('does not mutate the request it was given', () => { + const base = { repository: 'acme/api' } + applyComposerChoices(base, { ...untouched, repos: [] }) + expect(base).toEqual({ repository: 'acme/api' }) + }) +})