diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fc72ee9..146ba146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.59.0] - 2026-09-30 + +### Added + +- **`aid --model` and `aid --effort`** (#662). Each agent is told them in its own spelling: + `claude --model/--effort`, `codex --model` and `-c model_reasoning_effort=`, and + `gemini --model`. The values are passed on as typed, since the agents add models + faster than `aid` is released. `--effort` beside `--gemini` is refused, as gemini has + no such setting. Both work with `aid resume` too. +- **`aid` asks for the agent, the model and the effort before the prompt** (#662). Each is a + picker that lists the recent choices first, so one Enter repeats the last launch. The + agent picker has one row per Claude login that `dl --claude-profiles` lists, then + `codex` and `gemini`. A name that is not listed can be typed. A flag on the line + skips its picker, and Esc stops the background boot. +- **A bare `aid` picks a workspace** (#662), as a bare `dl` does. So does a line of flags with + no workspace, such as `aid --codex`. + +### Fixed + +- **A pasted prompt arrives whole** (#662). The prompt editor read the terminal in line mode, + which cut a paste off at 4096 bytes, submitted at its first line break, and left + lines that arrived late for the agent to read as keystrokes. It is now a raw-mode + editor with bracketed paste: a paste keeps its line breaks, Alt-Enter or Ctrl-J adds + a line, and Enter submits. +- **The agent, model and effort pickers no longer panic** (#662) on a terminal with a size of + zero, and take the workspace picker's `TERM` fallback. With no terminal they can draw + on, they are skipped, and the prompt editor still opens. + ## [0.58.0] - 2026-09-30 ### Added diff --git a/README.md b/README.md index 9a13088e..24abda66 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ one argument instead of a clone, a config file and a build command. [![GitHub pull-requests merged](https://badgen.net/github/merged-prs/blooop/devlaunch)](https://github.com/blooop/devlaunch/pulls?q=is%3Amerged) [![GitHub release](https://img.shields.io/github/release/blooop/devlaunch.svg)](https://GitHub.com/blooop/devlaunch/releases/) [![PyPI](https://img.shields.io/pypi/v/devlaunch)](https://pypi.org/project/devlaunch/) -[![Conda](https://img.shields.io/badge/conda-v0.58.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) +[![Conda](https://img.shields.io/badge/conda-v0.59.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) [![License](https://img.shields.io/github/license/blooop/devlaunch)](https://opensource.org/license/mit/) [![Platform](https://img.shields.io/badge/platform-linux--64-blue)](https://github.com/blooop/devlaunch/releases) [![Pixi Badge](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/prefix-dev/pixi/main/assets/badge/v0.json)](https://pixi.sh) @@ -341,7 +341,7 @@ clone, and [docs/cleanup.md](docs/cleanup.md) says what it carries one past and ```bash $ dl --version -dl 0.58.0 +dl 0.59.0 ``` `--devcontainer ` picks a non-default `devcontainer.json`. A bare name means @@ -407,15 +407,23 @@ once for the whole run: aid https://github.com/blooop/devlaunch/pull/579 address the review comments ``` -With no prompt on the line, `aid` starts the container booting and asks for the prompt while it -does. Type it free of shell quoting, with no escaping and no history expansion eating a `!`. An -empty Enter starts the agent's plain session. Piping stdin or setting `DEVLAUNCH_NO_TTY=1` skips -the question and launches one-shot, so scripts behave as they always have. +With no workspace on the line, `aid` on a terminal lets you pick one of your workspaces, as `dl` +does. With no prompt on the line, `aid` starts the container booting and asks for the prompt while it +does. Type it free of shell quoting, with no escaping and no history expansion eating a `!`. A +paste keeps its line breaks and can be any length, Alt-Enter or Ctrl-J adds a line, and an empty +Enter starts the agent's plain session. Before the prompt it asks for the agent (one row per Claude +login, then `codex` and `gemini`), the model and the effort, each in a picker that lists your +recent choices first, so one Enter repeats the last launch. A flag on the line skips its picker, +and Esc stops the boot. See [docs/cli.md](docs/cli.md#the-pickers-ahead-of-the-prompt). Piping +stdin or setting `DEVLAUNCH_NO_TTY=1` skips the question and launches one-shot, so scripts behave +as they always have. | Option | What it does | |---|---| | `--claude`, `--codex`, `--gemini` | Pick the agent. Default `claude` | | `--rm` | Delete the workspace when the agent is done. Appendable to a recalled line | +| `--model ` | The model the agent runs, in that agent's own spelling. Passed on as typed and not checked. See [docs/cli.md](docs/cli.md#model-and-effort-in-each-agents-spelling) | +| `--effort ` | How hard the agent thinks: claude's `--effort`, codex's `model_reasoning_effort`. gemini has none, so beside `--gemini` it stops | | `--no-remote-control`, `--no-remote` | Start a plain local session. Remote Control is on by default for `claude`: the session is named after the workspace and can be read and steered from claude.ai/code or the Claude app. It needs a claude.ai login in the container | | `--remote-control`, `--remote` | Ask for Remote Control by name. `claude` has it already; beside `--codex` or `--gemini` this says they have not got it and stops | | `--devcontainer ` | Passed through to `dl` | diff --git a/docs/cli.md b/docs/cli.md index eb6076d0..b6ce99b2 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -730,6 +730,113 @@ claude all take it offline immediately. The entry can sit in the claude.ai list roughly 4 hours after that before it clears, which is the web side timing out rather than anything still running on your machine. +## Model and effort, in each agent's spelling + +`--model` and `--effort` are `aid`'s own words. Each agent is told them in the +spelling its own CLI takes: + +```bash +aid --model opus --effort max blooop/devlaunch # claude --model opus --effort max +aid --codex --model gpt-5.5 --effort high blooop/devlaunch + # codex --model gpt-5.5 -c model_reasoning_effort=high +aid --gemini --model gemini-3-pro blooop/devlaunch # gemini --model gemini-3-pro +``` + +Both take their value as the next word or joined with `=`, and both go ahead of the +workspace like the agent flags do. After the workspace they are prompt text. + +**`aid` does not check the values.** Each agent adds models and effort levels on its +own release schedule, and codex already takes effort values it does not know by +name. A list in `aid` would be wrong a few weeks after each release. So `aid` holds +only the spelling of each flag, in its agent table, and passes the value on as you +typed it. A value the agent does not know is the agent's to answer for, in its own words: `claude` warns about an unknown effort and uses its default. +Leave a flag off and no flag is passed, so the agent starts on its own default. + +**gemini has no effort setting.** `--effort` beside `--gemini` is refused before +anything boots, the same way `--remote-control` is refused beside an agent without +Remote Control. A value that is missing, empty or starts with `-` is refused too: +`aid --model --codex ` is a typo, not a model called `--codex`. + +## The pickers ahead of the prompt + +`aid ` with no prompt on a terminal asks for up to three settings before +it opens the prompt editor. The workspace boots in the background the whole time. + +1. **The agent**, with one row per Claude login and one row for each other agent: + + ``` + AGENT NAME STATE ACCOUNT + claude default authed me@example.com + claude work authed me@acme.example · team + codex + gemini + ``` + + The Claude rows are the ones `dl --claude-profiles` lists, less the named + profiles with no credential, since a launch naming one of those refuses. A + named row becomes `--claude --claude-profile `, and `default` passes no + profile. codex and gemini have one row each, because `dl` forwards one login + for each of them. + + Choosing a row puts its flags in front of your line and parses the line again, + so every rule of the line still holds. A row the line would refuse is not + shown: `--remote-control` on the line leaves only claude, and `--effort` leaves + out gemini. A typed `--codex` or `--claude` shows only that agent's rows, and a + typed `--claude-profile` skips this picker. So does a list of one row. +2. **The model.** +3. **The effort**, for the agents that have one. + +Each picker lists your recent choices first, newest at the top. For the model and +the effort that is per agent, followed by `default`, then a few suggestions for a +first run. The cursor starts on the +first row, so one Enter repeats the last launch. Type to filter the rows. Type a +name that no row holds and Enter uses it as typed, which is how a model that came +out yesterday is chosen. It is listed from then on. Matching is exact rather than +fuzzy for that reason: a fuzzy match finds a listed row for almost any query. +Where the name is part of a listed row, such as `gpt-5.5` beside `gpt-5.5-codex`, +Alt-Enter uses the text as typed instead of the row. + +A setting that a flag on the line already gave is not asked for, so +`aid --model opus --effort max ` goes straight to the prompt. A line with a +prompt on it asks for nothing, as before. + +**Esc cancels the launch.** In a picker Esc and Ctrl-C are keys, not signals, so +`aid` stops the background boot itself, the same way a Ctrl-C at the prompt editor +does: its `devpod up` is killed and its staged token file is removed. The exit +status is 130. + +The recent choices live in `aid-recent.tsv` in the devlaunch cache, so +`XDG_CACHE_HOME` scopes them with everything else. A file that is missing or +broken reads as no history, and never stops a launch. + +With no `TERM`, with `TERM=dumb`, or on a terminal whose size reads as zero, +there are no pickers. The prompt editor still opens and every setting stays at +its default. + +## The prompt editor + +After the pickers, `aid` draws a `> ` prompt under one line that says what is +booting. It is a small editor of its own, not the terminal's line mode, because +the line mode could not take a paste: it holds 4096 bytes of a line, it +submitted at the first line break of a paste, and the lines of a paste that came +a moment late went on to the agent as keystrokes. + +- **A paste is text, line breaks included.** The editor turns on the terminal's + bracketed paste, so it knows where a paste starts and ends, and a line break + inside one never submits. A terminal without bracketed paste still sends a + paste faster than anybody types, so an Enter with more input right behind it + is read as a line break too. Windows line ends are one break. +- **Enter submits. Alt-Enter or Ctrl-J adds a line.** +- **Backspace, Ctrl-U and Ctrl-W** delete a character, the line and a word. + There is no cursor to move: the arrow keys do nothing, rather than printing + `^[[D`. +- **An empty Enter, or Ctrl-D on nothing,** starts the agent's plain session. +- **Ctrl-C stops the boot** and exits with 130, as Esc does in a picker. + +A prompt taller than the screen shows its last lines under a line that counts the +ones not shown. All of it is sent. Keys typed after the Enter are not read, so +they reach the agent. + ## `aid resume`: back into a session after a restart ```bash diff --git a/rust/Cargo.lock b/rust/Cargo.lock index c9436736..e03578d1 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "aid" -version = "0.58.0" +version = "0.59.0" dependencies = [ "devlaunch-test-support", "dl", @@ -438,7 +438,7 @@ dependencies = [ [[package]] name = "devlaunch-core" -version = "0.58.0" +version = "0.59.0" dependencies = [ "devlaunch-runner", "devlaunch-test-support", @@ -457,7 +457,7 @@ dependencies = [ [[package]] name = "devlaunch-runner" -version = "0.58.0" +version = "0.59.0" dependencies = [ "libc", "portable-pty", @@ -466,7 +466,7 @@ dependencies = [ [[package]] name = "devlaunch-test-support" -version = "0.58.0" +version = "0.59.0" dependencies = [ "devlaunch-runner", "serde", @@ -508,7 +508,7 @@ dependencies = [ [[package]] name = "dl" -version = "0.58.0" +version = "0.59.0" dependencies = [ "clap", "devlaunch-core", @@ -521,6 +521,7 @@ dependencies = [ "skim", "tempfile", "term", + "unicode-width", ] [[package]] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 8bb93e27..b8166321 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,7 @@ members = [ # The single source of the version (docs/rust-rewrite-plan.md: cutover ships # 0.1.0, version read from Cargo.toml). [workspace.package] -version = "0.58.0" +version = "0.59.0" edition = "2024" license = "MIT" repository = "https://github.com/blooop/devlaunch" @@ -36,6 +36,8 @@ toml = "0.9" toml_edit = "0.23" clap = { version = "4", features = ["derive"] } skim = "0.20" +# The prompt editor's column widths. skim already brings this version in. +unicode-width = "0.2" # The terminfo lookup skim's terminal setup makes and unwraps, made first so a # `TERM` it would panic on, or draw nothing with, can be replaced (dl's # `select.rs`). The version skim resolves, so the answer is the one skim would diff --git a/rust/aid/src/interactive.rs b/rust/aid/src/interactive.rs index 4dcb981d..ac807701 100644 --- a/rust/aid/src/interactive.rs +++ b/rust/aid/src/interactive.rs @@ -20,11 +20,15 @@ //! argv (aid's one dependency is `dl`, and the one binary aid can find without //! guessing at PATH is itself). Its output goes to a log file and is replayed to //! stderr after the prompt is submitted, so the build's progress is seen — just -//! not interleaved with the typing. The child is deliberately left in aid's -//! process group: a terminal Ctrl-C mid-editing reaches both processes, and the -//! child's own interrupt handler (the shared `dl::install_signal_handlers` -//! disposition) kills its `devpod up` group and unlinks its staged token file, -//! so abandoning the editor tears the whole boot down with no new machinery. +//! not interleaved with the typing. The prompt editor and the pickers hold the +//! terminal in raw mode, so a Ctrl-C there raises no signal: it is a byte aid +//! reads, and aid answers it with `BootChild::cancel`, which sends the child a +//! SIGINT of its own. The child's interrupt handler (the shared +//! `dl::install_signal_handlers` disposition) then kills its `devpod up` group +//! and unlinks its staged token file, so abandoning the editor tears the whole +//! boot down. The child is still left in aid's process group, so a Ctrl-C typed +//! outside raw mode, while the boot is waited on after the prompt is submitted, +//! reaches both processes as an ordinary terminal SIGINT. //! //! Every failure in here is a fallback, never an ending: a boot that could not //! be spawned means the launch runs serially, exactly as it did before this @@ -36,7 +40,8 @@ use std::path::PathBuf; use std::process::{Child, Command, Stdio}; use std::time::Duration; -use crate::rewrite::AidArgs; +use crate::recent::{self, Recent}; +use crate::rewrite::{self, AidArgs, Environment, Knob, Launcher, Tuning}; /// The internal argv word the boot child is started with. Undocumented on /// purpose: it is aid talking to itself, not a flag anyone types. @@ -92,6 +97,17 @@ impl BootChild { }) } + /// Stop the boot the way a terminal Ctrl-C would, and wait for it to end. + /// + /// SIGINT, so the boot's own handler kills its `devpod up` group and unlinks + /// its staged token file. Its output is not relayed: it is the noise of a boot + /// nobody wants any more. + pub(crate) fn cancel(mut self) { + dl::interrupt(self.child.id()); + let _ = self.child.wait(); + let _ = std::fs::remove_file(&self.log); + } + /// Wait for the boot to end, relaying its output to stderr as it lands, and /// say whether it succeeded. /// @@ -146,6 +162,15 @@ impl BootChild { } } +/// How the interactive flow ended. +pub(crate) enum Collected { + /// Launch this line, once the boot beside it, if any, has finished. + Launch(Box, Option), + /// A picker was cancelled, or Ctrl-C was typed at the prompt editor. The + /// boot was stopped and nothing launches. + Cancelled, +} + /// The interactive default, as one decision: boot in the background and collect /// the prompt from the terminal, or hand the line back untouched. /// @@ -159,16 +184,23 @@ impl BootChild { /// An empty submission — a bare Enter, or Ctrl-D — leaves the prompt empty, /// which is the agent's plain session: the old bare-`aid` behaviour is one /// keystroke away, not gone. -pub(crate) fn collect_prompt(parsed: AidArgs) -> (AidArgs, Option) { +/// +/// `argv` and `environment` are what `parsed` was parsed from: the agent picker +/// builds its rows by parsing the line again with each row's flags in front. +pub(crate) fn collect_prompt( + parsed: AidArgs, + argv: &[String], + environment: Environment<'_>, +) -> Collected { let promptless_agent = matches!( &parsed.task, crate::rewrite::Task::Agent { prompt, .. } if prompt.is_empty() ); if !promptless_agent || !dl::interactive_terminal() { - return (parsed, None); + return Collected::Launch(Box::new(parsed), None); } let Some(boot) = BootChild::spawn(&crate::rewrite::build_boot_args(&parsed)) else { - return (parsed, None); + return Collected::Launch(Box::new(parsed), None); }; // Name the pane and the tab now, because the launch that would name them is // behind the editor and the editor is where the waiting happens. Deliberately @@ -181,9 +213,217 @@ pub(crate) fn collect_prompt(parsed: AidArgs) -> (AidArgs, Option) { // log is not a title. That gate is also why this is a foreground call rather // than something handed to `BootChild`. dl::name_before_launch(&parsed.spec); + // The pickers come after the boot has started, so the minute they take is + // also spent booting. Nothing they choose reaches the boot: `up` takes no + // model, and `--claude-profile` is read when the session starts, not at `up`. + let Some(parsed) = settle(parsed, argv, environment) else { + boot.cancel(); + return Collected::Cancelled; + }; banner(&parsed); - let typed = dl::read_terminal_submission(); - (parsed.with_prompt(typed), Some(boot)) + match dl::read_prompt() { + dl::Submission::Text(typed) => { + Collected::Launch(Box::new(parsed.with_prompt(typed)), Some(boot)) + } + // The editor holds the terminal in raw mode, so Ctrl-C is a key there as + // it is in a picker, and ends the run the same way. + dl::Submission::Cancelled => { + boot.cancel(); + Collected::Cancelled + } + } +} + +/// What one picker settled. +enum Asked { + /// This value, or `None` for the default, which passes no flag. + Chose(Option), + /// Nothing to ask about, or no terminal to ask on. The line is left as it was. + Skipped, + /// Esc or Ctrl-C: the launch is off. + Cancelled, +} + +/// Ask for each setting the line left open, in one fixed order: the agent (and +/// for claude its login), the model, the effort. `None` is a cancel. +/// +/// A setting a flag already gave is not asked for, and neither is one the agent +/// does not have. Every choice is remembered, so the next launch lists it first. +fn settle(parsed: AidArgs, argv: &[String], environment: Environment<'_>) -> Option { + let file = recent::path(); + let mut recent = file.as_deref().map(Recent::read).unwrap_or_default(); + let parsed = match ask_agent(&parsed, argv, environment, &mut recent) { + Picked::Cancelled => return None, + Picked::Kept => parsed, + // The spec `parsed` holds may be a pull request link already resolved to a + // branch, which a fresh parse of argv would undo. + Picked::Line(line) => line.with_spec(parsed.spec.clone()), + }; + let (Some(agent), Some(tuning)) = (parsed.agent(), parsed.tuning()) else { + return Some(parsed); + }; + let agent = agent.to_owned(); + let mut tuning = tuning.clone(); + for knob in Knob::ALL { + if !rewrite::takes(&agent, knob) { + continue; + } + let slot = match knob { + Knob::Model => &mut tuning.model, + Knob::Effort => &mut tuning.effort, + }; + if slot.is_none() { + match ask_value(&agent, knob, &recent) { + Asked::Cancelled => return None, + Asked::Skipped => continue, + Asked::Chose(value) => *slot = value, + } + } + recent.record(&agent, knob, slot.as_deref()); + } + if let Some(file) = &file { + recent.write(file); + } + Some(parsed.with_tuning(tuning)) +} + +/// What the agent picker settled. +enum Picked { + /// The line as it was: one row or none to choose from, or no terminal. + Kept, + /// The line again, with the chosen row's flags in front. + Line(Box), + Cancelled, +} + +/// The agent picker: one row per Claude login, then one for each other agent. +/// +/// Not asked when there is at most one row, such as a line that typed `--codex`. +/// The first run lists the default agent's rows first; after that the rows chosen +/// most recently lead. Typed text that is no row is not an agent, so the picker +/// asks again. +fn ask_agent( + parsed: &AidArgs, + argv: &[String], + environment: Environment<'_>, + recent: &mut Recent, +) -> Picked { + let (heading, offers) = dl::claude_profile_offers(); + let named: Vec = offers + .iter() + .map(|offer| offer.name.clone()) + .filter(|name| name != dl::DEFAULT_CLAUDE_PROFILE) + .collect(); + let mut rows = rewrite::launchers(argv, environment, parsed, &named); + if rows.len() < 2 { + return Picked::Kept; + } + // A stable sort, so rows with the same rank keep the table's order. + let remembered = recent.launchers(); + let rank = |row: &Launcher| { + remembered + .iter() + .position(|key| *key == row.key()) + .unwrap_or(remembered.len() + usize::from(parsed.agent() != Some(row.agent))) + }; + rows.sort_by_key(|row| rank(row)); + + let width = rows.iter().map(|row| row.agent.len()).max().unwrap_or(0); + let login = |row: &Launcher| { + let name = row.profile.as_deref().unwrap_or(dl::DEFAULT_CLAUDE_PROFILE); + offers + .iter() + .find(|offer| rewrite::takes_claude_login(row.agent) && offer.name == name) + .map(|offer| offer.label.clone()) + }; + let labels: Vec = rows + .iter() + .map(|row| match login(row) { + Some(label) => format!("{: row.agent.to_owned(), + }) + .collect(); + let columns = if rows.iter().any(|row| login(row).is_some()) { + format!("{: { + let row = &rows[index]; + let Ok(line) = rewrite::relaunched(argv, environment, row) else { + return Picked::Kept; + }; + recent.record_launcher(&row.key()); + return Picked::Line(Box::new(line)); + } + dl::Choice::Typed(_) => {} + dl::Choice::Cancelled => return Picked::Cancelled, + dl::Choice::NoTerminal => return Picked::Kept, + } + } +} + +/// One picker over `rows`, drawn with `label`. Asked again after typed text that +/// cannot be a value, such as a flag. +fn ask(header: &str, rows: &[Option], label: impl Fn(&Option) -> String) -> Asked { + let labels: Vec = rows.iter().map(label).collect(); + loop { + match dl::choose(header, &labels) { + dl::Choice::Row(index) => return Asked::Chose(rows[index].clone()), + dl::Choice::Typed(typed) if rewrite::usable_value(&typed) => { + return Asked::Chose(Some(typed)); + } + dl::Choice::Typed(_) => {} + dl::Choice::Cancelled => return Asked::Cancelled, + dl::Choice::NoTerminal => return Asked::Skipped, + } + } +} + +/// The row that stands for "pass no flag". +const DEFAULT_ROW: &str = "default (the agent's own)"; + +/// The model or effort picker. +fn ask_value(agent: &str, knob: Knob, recent: &Recent) -> Asked { + let rows = recent::ordered( + recent.values(agent, knob), + rewrite::suggestions(agent, knob), + ); + let header = format!( + "{} for {agent}. Type to filter, or type a name that is not listed.\n\ + Alt-Enter uses the text as typed. Esc cancels the launch.", + match knob { + Knob::Model => "Model", + Knob::Effort => "Effort", + } + ); + ask(&header, &rows, |row| { + row.clone().unwrap_or_else(|| DEFAULT_ROW.to_owned()) + }) +} + +/// What the line settled beyond the agent, for the banner: ` (account work, model +/// opus)`, or nothing when every setting is the default. +fn chosen(parsed: &AidArgs) -> String { + let default = Tuning::default(); + let tuning = parsed.tuning().unwrap_or(&default); + let parts: Vec = [ + ("account", parsed.claude_profile()), + ("model", tuning.model.as_deref()), + ("effort", tuning.effort.as_deref()), + ] + .into_iter() + .filter_map(|(name, value)| value.map(|value| format!("{name} {value}"))) + .collect(); + if parts.is_empty() { + String::new() + } else { + format!(" ({})", parts.join(", ")) + } } /// The one line the editor shows before the read, naming what is booting, which @@ -191,10 +431,10 @@ pub(crate) fn collect_prompt(parsed: AidArgs) -> (AidArgs, Option) { pub(crate) fn banner(parsed: &AidArgs) { let agent = parsed.agent().unwrap_or_default(); eprintln!( - "Booting {} in the background. Type the prompt for {agent} and press Enter to \ - launch; an empty Enter starts a plain session.", - parsed.spec + "Booting {} in the background. Type the prompt for {agent}{} and press Enter to \ + launch; an empty Enter starts a plain session. Alt-Enter or Ctrl-J adds a line, \ + and a paste keeps its line breaks.", + parsed.spec, + chosen(parsed) ); - eprint!("> "); - let _ = std::io::stderr().flush(); } diff --git a/rust/aid/src/main.rs b/rust/aid/src/main.rs index 374e1921..fc7ac574 100644 --- a/rust/aid/src/main.rs +++ b/rust/aid/src/main.rs @@ -27,6 +27,7 @@ //! through an absolute path. mod interactive; +mod recent; mod rewrite; use std::io::Write as _; @@ -112,11 +113,14 @@ fn run(argv: &[String]) -> i32 { [first, ..] => is_help(first), [] => false, }; - if argv.is_empty() || asked_for_help { + // A bare `aid` on a terminal picks a workspace below, as a bare `dl` does. The + // help is for a run with nobody at a terminal to pick. + let picks = argv.is_empty() && dl::interactive_terminal(); + if (argv.is_empty() && !picks) || asked_for_help { print!("{}", help()); return if argv.is_empty() { 1 } else { 0 }; } - if argv[0] == "--version" { + if argv.first().is_some_and(|word| word == "--version") { // `aid `, the version dl prints under aid's name, and the same // build marker: `dl::BUILD_MARKER` is empty in a released build and `-dev` // in a working-tree one, so `aid-next` says which build it is exactly as @@ -141,15 +145,42 @@ fn run(argv: &[String]) -> i32 { agent: agent.as_deref(), remote_control: remote_control.as_deref(), }; - let parsed = match rewrite::parse_aid_args(argv, environment) { - Ok(rewrite::Line::Ready(parsed)) => parsed, + // A line with no workspace, on a terminal, gets dl's workspace picker. For + // `aid resume` the pick completes the line it parsed to. For any other line the + // id is added to the end of the line, after the leading flags, which is where a + // typed spec goes, and from here on the line is that longer one, because the + // agent picker parses it again. + let with_pick: Vec; + let (argv, parsed) = match rewrite::parse_aid_args(argv, environment) { + Ok(rewrite::Line::Ready(parsed)) => (argv, parsed), // `aid resume` with no workspace. The pick comes before everything below, // which is all about one named workspace, so from here on this line is an // `aid resume ` like any other. Ok(rewrite::Line::Unpicked(unpicked)) => match dl::pick_workspace() { - Ok(workspace_id) => unpicked.picked(workspace_id), + Ok(workspace_id) => (argv, unpicked.picked(workspace_id)), Err(code) => return code, }, + Err(UsageError::NoWorkspace) + if dl::interactive_terminal() && !rewrite::spells_a_retired_flag(argv) => + { + let spec = match dl::pick_workspace() { + Ok(spec) => spec, + Err(code) => return code, + }; + with_pick = argv.iter().cloned().chain([spec]).collect(); + match rewrite::parse_aid_args(&with_pick, environment) { + Ok(rewrite::Line::Ready(parsed)) => (with_pick.as_slice(), parsed), + // A line that names its workspace is never unpicked. + Ok(rewrite::Line::Unpicked(_)) => { + eprintln!("{}", refusal(&UsageError::NoWorkspace)); + return 1; + } + Err(refused) => { + eprintln!("{}", refusal(&refused)); + return 1; + } + } + } Err(refused) => { eprintln!("{}", refusal(&refused)); return 1; @@ -168,7 +199,15 @@ fn run(argv: &[String]) -> i32 { Ok(spec) => parsed.with_spec(spec), Err(code) => return code, }; - let (parsed, boot) = interactive::collect_prompt(parsed); + let (parsed, boot) = match interactive::collect_prompt(parsed, argv, environment) { + interactive::Collected::Launch(parsed, boot) => (*parsed, boot), + // 130, the code a Ctrl-C at the prompt editor ends with, because a cancel + // in a picker is the same request made with a different key. + interactive::Collected::Cancelled => { + eprintln!("aid: cancelled; the background boot was stopped and nothing launched."); + return 130; + } + }; let session = new_session_id(); let Some(launch) = rewrite::build_launch(&parsed, &dl::workspace_id_of, session.as_deref()) else { @@ -305,6 +344,21 @@ fn refusal(refused: &UsageError) -> String { dl::python_repr(value), rewrite::remote_control_values().join(", ") ), + UsageError::MissingValue { flag } => { + format!("{flag} needs a value: aid {flag} [prompt]") + } + // The agents that can take one are listed, because the fix is either to + // drop the flag or to pick one of them. + UsageError::EffortUnsupported { agent } => format!( + "{} sets a reasoning effort, which {agent} has no setting for. \ + Drop the flag or pick one of: {}.", + rewrite::EFFORT_FLAG, + rewrite::effort_agent_names() + .iter() + .map(|name| format!("--{name}")) + .collect::>() + .join(", ") + ), UsageError::ResumeTakesNoPrompt { words } => format!( "aid resume takes a workspace and nothing after it, not {}: the agent's own \ picker chooses the session. Use aid resume [].", @@ -341,6 +395,7 @@ opened by dl itself, so it is the same workspace, container and clone that already running, and never rebuilt just because aid asked for it. Usage: + aid Pick a workspace, then start the agent aid [@branch] [prompt...] Open the workspace and start the agent aid [prompt...] Same, for an existing workspace or ./path aid resume [] Reopen an earlier agent session in the @@ -352,15 +407,34 @@ full-auto, Remote Control named after the workspace) and hands it its own resume words: claude and codex open their session picker, and gemini reopens its latest session. +With no workspace on a terminal, aid lets you pick one of your workspaces, as +dl does. + With no prompt on a terminal, aid boots the workspace in the background and -asks for the prompt while it does: type it free of shell quoting and press -Enter to launch. An empty Enter (or Ctrl-D) starts the agent's plain session. +asks for the prompt while it does. First it asks for each setting the line +left open: the agent (one row per Claude login, then each other agent), the +model and the effort. Each picker lists your recent choices first, so one +Enter repeats the last launch. Type a name that is not listed to use it. +Alt-Enter uses the text as typed where it is part of a listed name. + +Then type the prompt free of shell quoting and press Enter to launch. A paste +keeps its line breaks, and Alt-Enter or Ctrl-J adds a line. An empty Enter +(or Ctrl-D) starts the agent's plain session. Esc in a picker, or Ctrl-C at +the prompt, stops the boot and launches nothing. + Piping stdin or setting DEVLAUNCH_NO_TTY=1 skips the question and launches one-shot, as a prompt on the command line always has. Options: {agents} Pick the agent (default: {default}) + --model The model the agent runs, passed on as typed + in the agent's own spelling. Any name the + agent takes works; aid does not check it. + Default: the agent's own default + --effort How hard the agent thinks, the same way: + claude's --effort, codex's + model_reasoning_effort. gemini has none --devcontainer Passed through to dl --rm Delete the workspace once the agent's session ends, the way docker run --rm does. Appendable: @@ -406,6 +480,8 @@ Examples: aid blooop/devlaunch@fix/42 fix the bug # Open the branch, hand over the prompt aid --gemini ./my-project explain this # Pick a different agent aid --no-remote blooop/devlaunch # Nothing but the session in front of you + aid --model opus --effort max blooop/devlaunch + # Pick the model and how hard it thinks aid resume # Pick a workspace, then a session in it aid resume blooop/devlaunch@fix/42 # Pick a session in that workspace aid blooop/devlaunch@fix/42 fix the bug --rm @@ -666,6 +742,25 @@ mod tests { ); } + #[test] + fn a_flag_with_no_value_says_where_the_value_goes() { + assert_eq!( + refusal(&UsageError::MissingValue { flag: "--model" }), + "--model needs a value: aid --model [prompt]" + ); + } + + #[test] + fn an_effort_beside_an_agent_that_has_none_names_the_agents_that_do() { + assert_eq!( + refusal(&UsageError::EffortUnsupported { + agent: "gemini".to_owned() + }), + "--effort sets a reasoning effort, which gemini has no setting for. \ + Drop the flag or pick one of: --claude, --codex." + ); + } + #[test] fn a_command_line_with_no_workspace_says_what_one_looks_like() { assert_eq!( diff --git a/rust/aid/src/recent.rs b/rust/aid/src/recent.rs new file mode 100644 index 00000000..6638cdcb --- /dev/null +++ b/rust/aid/src/recent.rs @@ -0,0 +1,293 @@ +//! The choices aid's pickers list first: the recent ones, per agent and setting. +//! +//! This is what keeps the pickers current without a release. The table in +//! `rewrite` suggests a few values for a first run, and from then on the rows a +//! person sees are the ones they chose, newest first, so the cursor starts on the +//! last choice and one Enter repeats it. A model that came out yesterday is typed +//! once and listed from then on. +//! +//! The file is `/aid-recent.tsv`, one choice per line: +//! ` TAB TAB `, newest first, with an empty value for the +//! default. Plain text rather than JSON because aid's one dependency is `dl`, and +//! the format needs nothing a `split` cannot read. +//! +//! **Nothing in here can stop a launch.** A file that is missing, unreadable, +//! half-written or full of junk reads as no history, and a write that fails is +//! dropped. The worst a broken file costs is the suggestions in place of the recent +//! choices. + +use std::path::{Path, PathBuf}; + +use crate::rewrite::Knob; + +/// The file's name inside the devlaunch cache. +const FILE_NAME: &str = "aid-recent.tsv"; + +/// How many choices are kept per agent and setting. +const KEPT: usize = 8; + +/// What a remembered choice is kept under. +#[derive(Clone, Copy)] +enum Key<'a> { + /// The launcher picker's row: which agent, and for claude which login. It is + /// no one agent's setting, because it is what chooses the agent. + Launcher, + Knob(&'a str, Knob), +} + +impl<'a> Key<'a> { + /// The agent and setting columns of the file. + fn columns(self) -> (&'a str, &'static str) { + match self { + Key::Launcher => ("*", "agent"), + Key::Knob(agent, Knob::Model) => (agent, "model"), + Key::Knob(agent, Knob::Effort) => (agent, "effort"), + } + } +} + +/// Where the file lives, or `None` with no cache directory to put it in. +pub(crate) fn path() -> Option { + dl::cache_dir().map(|dir| dir.join(FILE_NAME)) +} + +/// One remembered choice. `value` is `None` for the default, which passes no flag. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Entry { + agent: String, + setting: String, + value: Option, +} + +/// Every remembered choice, newest first. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(crate) struct Recent { + entries: Vec, +} + +impl Recent { + /// The file at `path`, or no history if it cannot be read. + pub(crate) fn read(path: &Path) -> Self { + let text = std::fs::read_to_string(path).unwrap_or_default(); + let entries = text + .lines() + .filter_map(|line| { + let mut fields = line.split('\t'); + let (agent, setting, value) = (fields.next()?, fields.next()?, fields.next()?); + if fields.next().is_some() || agent.is_empty() || setting.is_empty() { + return None; + } + Some(Entry { + agent: agent.to_owned(), + setting: setting.to_owned(), + value: (!value.is_empty()).then(|| value.to_owned()), + }) + }) + .collect(); + Recent { entries } + } + + /// The launcher rows remembered, by [`Launcher::key`](crate::rewrite::Launcher::key), + /// newest first. + pub(crate) fn launchers(&self) -> Vec { + self.lookup(Key::Launcher).into_iter().flatten().collect() + } + + /// Remember a launcher row as the newest. + pub(crate) fn record_launcher(&mut self, key: &str) { + self.remember(Key::Launcher, Some(key)); + } + + /// The choices remembered for this agent's knob, newest first. + pub(crate) fn values(&self, agent: &str, knob: Knob) -> Vec> { + self.lookup(Key::Knob(agent, knob)) + } + + /// Remember a choice for this agent's knob as the newest. + pub(crate) fn record(&mut self, agent: &str, knob: Knob, value: Option<&str>) { + self.remember(Key::Knob(agent, knob), value); + } + + fn lookup(&self, key: Key<'_>) -> Vec> { + let (agent, setting) = key.columns(); + self.entries + .iter() + .filter(|entry| entry.agent == agent && entry.setting == setting) + .map(|entry| entry.value.clone()) + .collect() + } + + /// Remember a choice as the newest, keeping at most [`KEPT`] under its key. + /// + /// A value holding a tab or a line break is not remembered, because the file + /// could not read it back as one value. + fn remember(&mut self, key: Key<'_>, value: Option<&str>) { + if value.is_some_and(|value| value.contains(['\t', '\n', '\r'])) { + return; + } + let (agent, setting) = key.columns(); + let entry = Entry { + agent: agent.to_owned(), + setting: setting.to_owned(), + value: value.map(str::to_owned), + }; + self.entries.retain(|kept| *kept != entry); + self.entries.insert(0, entry); + let mut seen = 0; + self.entries.retain(|kept| { + if kept.agent != agent || kept.setting != setting { + return true; + } + seen += 1; + seen <= KEPT + }); + } + + /// Write the file, replacing it whole. Failures are dropped. + /// + /// Through a temporary file and a rename, so a launch that reads it while + /// another writes it sees the old file or the new one, never half of one. + pub(crate) fn write(&self, path: &Path) { + let text: String = self + .entries + .iter() + .map(|entry| { + format!( + "{}\t{}\t{}\n", + entry.agent, + entry.setting, + entry.value.as_deref().unwrap_or("") + ) + }) + .collect(); + let Some(directory) = path.parent() else { + return; + }; + let staged = directory.join(format!("{FILE_NAME}.{}", std::process::id())); + let written = std::fs::create_dir_all(directory) + .and_then(|()| std::fs::write(&staged, text)) + .and_then(|()| std::fs::rename(&staged, path)); + if written.is_err() { + let _ = std::fs::remove_file(&staged); + } + } +} + +/// The rows a picker lists, in order: the recent choices, then the default if it +/// was not among them, then each suggestion not already listed. +/// +/// The default is always a row, because it is the one choice that needs no name. +pub(crate) fn ordered(recent: Vec>, offered: &[&str]) -> Vec> { + let mut rows = recent; + if !rows.contains(&None) { + rows.push(None); + } + for value in offered { + let value = Some((*value).to_owned()); + if !rows.contains(&value) { + rows.push(value); + } + } + rows +} + +#[cfg(test)] +mod tests { + use super::*; + + fn some(value: &str) -> Option { + Some(value.to_owned()) + } + + #[test] + fn the_rows_are_the_recent_ones_then_the_default_then_the_suggestions() { + assert_eq!( + ordered(vec![some("max"), some("low")], &["low", "medium", "max"]), + [some("max"), some("low"), None, some("medium")] + ); + } + + #[test] + fn a_default_chosen_last_time_is_listed_where_it_was_chosen() { + assert_eq!( + ordered(vec![None, some("opus")], &["sonnet"]), + [None, some("opus"), some("sonnet")] + ); + } + + #[test] + fn a_first_run_lists_the_default_ahead_of_the_suggestions() { + assert_eq!(ordered(Vec::new(), &["opus"]), [None, some("opus")]); + } + + #[test] + fn a_choice_is_moved_to_the_front_and_not_listed_twice() { + let mut recent = Recent::default(); + recent.record("claude", Knob::Model, Some("opus")); + recent.record("claude", Knob::Model, Some("sonnet")); + recent.record("claude", Knob::Model, Some("opus")); + assert_eq!( + recent.values("claude", Knob::Model), + [some("opus"), some("sonnet")] + ); + } + + #[test] + fn choices_are_kept_apart_per_agent_and_setting() { + let mut recent = Recent::default(); + recent.record("claude", Knob::Model, Some("opus")); + recent.record("codex", Knob::Model, Some("gpt-5.5")); + recent.record("claude", Knob::Effort, Some("max")); + assert_eq!(recent.values("claude", Knob::Model), [some("opus")]); + assert_eq!(recent.values("codex", Knob::Model), [some("gpt-5.5")]); + assert_eq!(recent.values("claude", Knob::Effort), [some("max")]); + } + + #[test] + fn only_the_newest_few_are_kept() { + let mut recent = Recent::default(); + recent.record("codex", Knob::Effort, Some("low")); + for index in 0..KEPT + 3 { + recent.record("claude", Knob::Model, Some(&format!("model-{index}"))); + } + let kept = recent.values("claude", Knob::Model); + assert_eq!(kept.len(), KEPT); + assert_eq!(kept[0], Some(format!("model-{}", KEPT + 2))); + // Another agent's history is not what pays for this one's. + assert_eq!(recent.values("codex", Knob::Effort), [some("low")]); + } + + #[test] + fn the_file_reads_back_what_was_written() { + let scratch = tempfile::tempdir().expect("a scratch directory"); + let file = scratch.path().join("deeper").join(FILE_NAME); + let mut recent = Recent::default(); + recent.record("claude", Knob::Model, None); + recent.record_launcher("claude/work"); + recent.write(&file); + assert_eq!(Recent::read(&file), recent); + } + + #[test] + fn a_value_the_file_could_not_hold_is_not_remembered() { + let mut recent = Recent::default(); + recent.record("claude", Knob::Model, Some("a\tb")); + assert_eq!(recent, Recent::default()); + } + + #[test] + fn a_missing_or_broken_file_is_no_history() { + let scratch = tempfile::tempdir().expect("a scratch directory"); + let file = scratch.path().join(FILE_NAME); + assert_eq!(Recent::read(&file), Recent::default()); + std::fs::write( + &file, + "junk\n\tmodel\topus\nclaude\tmodel\topus\textra\nclaude\tmodel\tsonnet\n", + ) + .expect("a file"); + assert_eq!( + Recent::read(&file).values("claude", Knob::Model), + [some("sonnet")] + ); + } +} diff --git a/rust/aid/src/rewrite.rs b/rust/aid/src/rewrite.rs index 2a87974d..620aacbe 100644 --- a/rust/aid/src/rewrite.rs +++ b/rust/aid/src/rewrite.rs @@ -34,6 +34,17 @@ struct Agent { /// line asking for Remote Control is a launch or a refusal, so the capability is /// stated once and consulted from both ends. remote_control: Option<&'static str>, + /// How this agent is told which model to run. + model: Spelling, + /// How this agent is told how hard to think, if it can be. + /// + /// `None` is gemini, which has no such setting. Read the way + /// [`Self::remote_control`] is read: a typed `--effort` beside `None` is a + /// refusal, and one beside `Some` reaches the agent. + effort: Option, + /// Whether this agent signs in with the host's Claude login, which is what + /// `--claude-profile` chooses. The account picker is offered for these only. + claude_login: bool, /// The words that reopen one of this agent's earlier sessions instead of /// starting a new one: what `aid resume` appends in place of a prompt. /// @@ -65,6 +76,42 @@ struct Agent { latest: &'static [&'static str], } +/// How one agent is told one setting: a flag word, then the value behind a prefix. +/// +/// Two words always, so the value can never be read as the prompt or glued onto +/// the flag before it. The prefix is what codex needs: it has no effort flag, only +/// a config key, so its spelling is `-c` then `model_reasoning_effort=`. +/// +/// **The spelling is a fact, the values are only suggestions.** The models and +/// effort levels each agent takes change with every release of that agent, and the +/// agent checks them itself, so aid passes a value on as it was typed and never +/// refuses one. `offered` is what the picker lists on a first run, before there are +/// recent choices to list. A stale entry costs a row nobody picks, and a missing one +/// costs typing the name once. +struct Spelling { + flag: &'static str, + prefix: &'static str, + offered: &'static [&'static str], +} + +/// claude's model aliases, from `claude --help` (2.1.280). An alias follows the +/// newest model of its family, so these age slower than full names. +const CLAUDE_MODELS: &[&str] = &["opus", "sonnet", "fable"]; + +/// claude's `--effort` levels, from `claude --help` (2.1.280). +const CLAUDE_EFFORTS: &[&str] = &["low", "medium", "high", "xhigh", "max"]; + +/// codex's common reasoning efforts. Its `ReasoningEffort` has more, and takes +/// names it does not know (codex-rs/protocol/src/openai_models.rs). +const CODEX_EFFORTS: &[&str] = &["low", "medium", "high", "xhigh"]; + +impl Spelling { + /// The two words that tell the agent `value`. + fn words(&self, value: &str) -> [String; 2] { + [self.flag.to_owned(), format!("{}{value}", self.prefix)] + } +} + /// The word that makes a line reopen an earlier session: `aid resume [workspace]`. /// /// A verb and not a flag, for dl's reason: `dl stop` is the verb with no @@ -160,6 +207,17 @@ const AGENTS: &[(&str, Agent)] = &[ ("IS_SANDBOX", "1"), ], remote_control: Some("--remote-control"), + model: Spelling { + flag: "--model", + prefix: "", + offered: CLAUDE_MODELS, + }, + effort: Some(Spelling { + flag: "--effort", + prefix: "", + offered: CLAUDE_EFFORTS, + }), + claude_login: true, resume: &["--resume"], session_id: Some("--session-id"), latest: &["--continue"], @@ -172,6 +230,18 @@ const AGENTS: &[(&str, Agent)] = &[ prompt_flags: &[], env: &[], remote_control: None, + // No model names are offered: codex's change too often to seed. + model: Spelling { + flag: "--model", + prefix: "", + offered: &[], + }, + effort: Some(Spelling { + flag: "-c", + prefix: "model_reasoning_effort=", + offered: CODEX_EFFORTS, + }), + claude_login: false, resume: &["resume"], session_id: None, latest: &["resume", "--last"], @@ -184,6 +254,13 @@ const AGENTS: &[(&str, Agent)] = &[ prompt_flags: &["--prompt-interactive"], env: &[], remote_control: None, + model: Spelling { + flag: "--model", + prefix: "", + offered: &[], + }, + effort: None, + claude_login: false, resume: &["--resume"], session_id: None, latest: &["--resume"], @@ -191,6 +268,19 @@ const AGENTS: &[(&str, Agent)] = &[ ), ]; +/// aid's own flag for the model the agent runs. aid's word, not the agent's: each +/// row of [`AGENTS`] says how its own CLI spells it. +pub(crate) const MODEL_FLAG: &str = "--model"; + +/// aid's own flag for how hard the agent thinks, spelled per agent the same way. +pub(crate) const EFFORT_FLAG: &str = "--effort"; + +/// aid's flags that take a value, read before the spec and never passed to dl. +/// +/// Each takes its value as the next word or joined by `=`. They are read only +/// ahead of the spec, like the agent flags: after it, they are prompt text. +const AGENT_VALUE_OPTIONS: &[&str] = &[MODEL_FLAG, EFFORT_FLAG]; + /// aid's own flag for Claude Code's Remote Control, which is not dl's and must not /// reach it. /// @@ -287,6 +377,147 @@ pub(crate) fn agent_names() -> Vec<&'static str> { names } +/// The agents that take an `--effort`, sorted, for the refusal that lists them. +pub(crate) fn effort_agent_names() -> Vec<&'static str> { + let mut names: Vec<&'static str> = AGENTS + .iter() + .filter(|(_, row)| row.effort.is_some()) + .map(|(name, _)| *name) + .collect(); + names.sort_unstable(); + names +} + +/// One value the interactive flow asks for once the agent is settled. The agent +/// is not one of these: the launcher picker chooses it, as a [`Launcher`] row. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Knob { + Model, + Effort, +} + +impl Knob { + /// Every knob, in the order the pickers ask them. + pub(crate) const ALL: [Knob; 2] = [Knob::Model, Knob::Effort]; +} + +/// Whether `agent` has this knob at all. +pub(crate) fn takes(agent: &str, knob: Knob) -> bool { + agent_row(agent).is_some_and(|row| match knob { + Knob::Model => true, + Knob::Effort => row.effort.is_some(), + }) +} + +/// The values the table suggests for this knob. Accounts are never suggested +/// here: they are read off the disk by `dl`. +pub(crate) fn suggestions(agent: &str, knob: Knob) -> &'static [&'static str] { + let Some(row) = agent_row(agent) else { + return &[]; + }; + match knob { + Knob::Model => row.model.offered, + Knob::Effort => row.effort.as_ref().map_or(&[], |spelling| spelling.offered), + } +} + +/// One row of the agent picker: an agent, and for claude which login. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Launcher { + pub(crate) agent: &'static str, + /// A named Claude login, or `None` for the one the host uses anyway. Always + /// `None` for an agent without a Claude login. + pub(crate) profile: Option, +} + +impl Launcher { + /// The word the recent-choices file keeps this row by: `claude`, or + /// `claude/work` for a named login. A profile name is a directory name, so it + /// cannot hold a `/`. + pub(crate) fn key(&self) -> String { + match &self.profile { + Some(profile) => format!("{}/{profile}", self.agent), + None => self.agent.to_owned(), + } + } + + /// The aid words that choose this row, for the front of a command line. + fn words(&self) -> Vec { + let mut words = vec![format!("--{}", self.agent)]; + if let Some(profile) = &self.profile { + words.push("--claude-profile".to_owned()); + words.push(profile.clone()); + } + words + } +} + +/// The rows the agent picker offers for this line, in table order: one per Claude +/// login for an agent that signs in with one, one for each other agent. +/// +/// `profiles` is the named Claude logins that can launch; the default login is +/// always a row. A line that already names a login offers nothing, and a line +/// that typed an agent offers only that agent's rows. +/// +/// **A row is offered only if the line it makes parses**, which is what keeps the +/// rules in one place: `--remote-control` on the line leaves out codex and gemini, +/// and `--effort` leaves out gemini, because [`parse_aid_args`] refuses those +/// lines. Nothing here restates why. +pub(crate) fn launchers( + argv: &[String], + environment: Environment<'_>, + parsed: &AidArgs, + profiles: &[String], +) -> Vec { + if parsed.claude_profile().is_some() { + return Vec::new(); + } + AGENTS + .iter() + .filter(|(name, _)| !parsed.agent_typed || parsed.agent() == Some(*name)) + .flat_map(|(name, row)| { + let named = profiles.iter().filter(|_| row.claude_login).cloned(); + std::iter::once(None) + .chain(named.map(Some)) + .map(|profile| Launcher { + agent: name, + profile, + }) + }) + .filter(|launcher| relaunched(argv, environment, launcher).is_ok()) + .collect() +} + +/// The command line with `launcher`'s words in front, parsed again. +/// +/// In front, so a word the person typed still comes later and wins, the way the +/// last of two agent flags does. +pub(crate) fn relaunched( + argv: &[String], + environment: Environment<'_>, + launcher: &Launcher, +) -> Result { + let mut line = launcher.words(); + line.extend(argv.iter().cloned()); + match parse_aid_args(&line, environment)? { + Line::Ready(parsed) => Ok(parsed), + // Only a line with no workspace is unpicked, and the picker only runs for a + // line that named one. + Line::Unpicked(_) => Err(UsageError::NoWorkspace), + } +} + +/// Whether `agent` signs in with the host's Claude login. +pub(crate) fn takes_claude_login(agent: &str) -> bool { + agent_row(agent).is_some_and(|row| row.claude_login) +} + +/// Whether a word can be a value for `--model`, `--effort` or `--claude-profile`: +/// not empty, and not a flag. +pub(crate) fn usable_value(value: &str) -> bool { + !value.is_empty() && !value.starts_with('-') +} + /// The values [`REMOTE_CONTROL_ENV_VAR`] takes, yes before no — for the refusal that /// lists them. Read off the same two lists the parse reads, so the sentence cannot /// offer a value the parse would reject. @@ -328,6 +559,14 @@ pub(crate) enum UsageError { /// who thinks they turned something off, and silently meaning *off* is a person /// who thinks they turned something on. UnknownRemoteControlInEnvironment { value: String }, + /// `--model` or `--effort` with nothing after it, or with another flag where + /// its value goes. + MissingValue { flag: &'static str }, + /// `--effort` on a line that starts an agent with no effort setting. + /// + /// Carries the agent for the reason [`Self::RemoteControlUnsupported`] does: + /// `DEVLAUNCH_AID_AGENT` can be what chose it. + EffortUnsupported { agent: String }, /// `aid resume ` with words after the workspace. /// /// Refused rather than handed on, because the words have no one place to go: @@ -438,6 +677,10 @@ pub(crate) struct AidArgs { /// beats two rules for one list. pub(crate) spec_options: Vec, pub(crate) task: Task, + /// Whether an agent flag on the line chose the agent, rather than the build's + /// default or `DEVLAUNCH_AID_AGENT`. The agent picker is not asked across + /// agents when one was typed. + pub(crate) agent_typed: bool, } /// What the line asks dl to do with the workspace. @@ -464,6 +707,9 @@ pub(crate) enum Task { /// line cannot describe codex being started with a feature codex has never /// had, and no later stage has to ask again. remote_control: RemoteControl, + /// The model and the effort the line asked for, already checked against the + /// agent: an effort is only ever here beside an agent that can take one. + tuning: Tuning, }, /// Reopen one of the agent's earlier sessions in the workspace: `aid resume`. /// @@ -475,6 +721,8 @@ pub(crate) enum Task { agent: String, /// As on [`Task::Agent`]: settled against the agent's table row. remote_control: RemoteControl, + /// As on [`Task::Agent`]: a resumed session can run another model. + tuning: Tuning, }, /// A line spelling a flag this build has retired ([`SUFFIX_RETIRED`]). /// @@ -485,6 +733,18 @@ pub(crate) enum Task { Retired, } +/// The model and the effort an agent line asked for. `None` passes no flag, so +/// the agent starts on its own default. +/// +/// A struct rather than two more `Option<&str>` parameters on +/// [`build_agent_command`], because two adjacent optional strings are two +/// arguments a call site can swap with nothing to say so. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(crate) struct Tuning { + pub(crate) model: Option, + pub(crate) effort: Option, +} + impl AidArgs { /// The agent this line starts, when it starts one. /// @@ -509,6 +769,37 @@ impl AidArgs { self } + /// The model and effort an agent line asked for. + pub(crate) fn tuning(&self) -> Option<&Tuning> { + match &self.task { + Task::Agent { tuning, .. } | Task::Resume { tuning, .. } => Some(tuning), + Task::Retired => None, + } + } + + /// The same line with the model and effort the pickers settled. A line that + /// starts no agent is returned unchanged, as [`Self::with_prompt`] does. + pub(crate) fn with_tuning(mut self, settled: Tuning) -> Self { + if let Task::Agent { tuning, .. } | Task::Resume { tuning, .. } = &mut self.task { + *tuning = settled; + } + self + } + + /// The Claude login the line names for dl, in either spelling dl takes. The + /// last one wins, as it does for dl. + pub(crate) fn claude_profile(&self) -> Option<&str> { + let mut named = None; + for (at, word) in self.dl_options.iter().enumerate() { + if word == "--claude-profile" { + named = self.dl_options.get(at + 1).map(String::as_str); + } else if let Some(joined) = word.strip_prefix("--claude-profile=") { + named = Some(joined); + } + } + named + } + /// The same line with the spec a pull request reference resolved to. /// /// aid holds the spec in three places `dl` never sees — the banner, the early @@ -550,6 +841,8 @@ pub(crate) struct Unpicked { spec_options: Vec, agent: String, remote_control: RemoteControl, + tuning: Tuning, + agent_typed: bool, } impl Unpicked { @@ -559,9 +852,11 @@ impl Unpicked { spec, dl_options: self.dl_options, spec_options: self.spec_options, + agent_typed: self.agent_typed, task: Task::Resume { agent: self.agent, remote_control: self.remote_control, + tuning: self.tuning, }, } } @@ -631,6 +926,15 @@ fn names_a_retired_spelling(options: &[String]) -> bool { .any(|word| SUFFIX_RETIRED.contains(&word.as_str())) } +/// Whether `argv` ends on a run that spells a retired flag. +/// +/// For the caller of a [`UsageError::NoWorkspace`]: that refusal on such a line is +/// final, because a picked spec appended after the run would stop it being the end +/// of the line, and the retired word would no longer be peeled. +pub(crate) fn spells_a_retired_flag(argv: &[String]) -> bool { + peel_suffix(argv).is_some_and(|suffix| names_a_retired_spelling(&suffix.options)) +} + /// A peeled trailing run, split by whose flag each word is. /// /// The split is the point. [`Self::options`] rides on to dl, and the remote-control @@ -753,6 +1057,8 @@ pub(crate) fn parse_aid_args( // `DEVLAUNCH_AID_REMOTE_CONTROL` that is neither a yes nor a no. let mut agent = default_agent(environment.agent)?; let mut remote_control = default_remote_control(environment.remote_control)?; + let mut tuning = Tuning::default(); + let mut agent_typed = false; let (line, trailing, trailing_remote_control) = match peel_suffix(argv) { Some(suffix) => (suffix.line, suffix.options, suffix.remote_control), None => (argv, Vec::new(), None), @@ -765,6 +1071,7 @@ pub(crate) fn parse_aid_args( let word = line[at].as_str(); if let Some(named) = agent_flag(word) { agent = named.to_owned(); + agent_typed = true; at += 1; continue; } @@ -783,6 +1090,16 @@ pub(crate) fn parse_aid_args( at += 1; continue; } + if let Some((flag, value, width)) = agent_value(line, at)? { + let slot = if flag == MODEL_FLAG { + &mut tuning.model + } else { + &mut tuning.effort + }; + *slot = Some(value.to_owned()); + at += width; + continue; + } if DL_VALUE_OPTIONS.contains(&word) { // Take the value with it; dl reports a missing one. dl_options.extend(line[at..line.len().min(at + 2)].iter().cloned()); @@ -830,12 +1147,18 @@ pub(crate) fn parse_aid_args( // looks like. Settled before the task is built, so the `RemoteControl` the task // carries is one an agent row supplied the flag for. let remote_control = remote_control.settle(&agent)?; + // Settled at the same point and for the same reason as Remote Control. + if tuning.effort.is_some() && agent_row(&agent).is_some_and(|row| row.effort.is_none()) { + return Err(UsageError::EffortUnsupported { agent }); + } let Some(spec) = spec else { return Ok(Line::Unpicked(Unpicked { dl_options, spec_options: trailing, agent, remote_control, + tuning, + agent_typed, })); }; let task = if retired { @@ -844,22 +1167,63 @@ pub(crate) fn parse_aid_args( Task::Resume { agent, remote_control, + tuning, } } else { Task::Agent { agent, prompt: rest.join(" "), remote_control, + tuning, } }; Ok(Line::Ready(AidArgs { spec, dl_options, spec_options: trailing, + agent_typed, task, })) } +/// The table row for `agent`, when it has one. +fn agent_row(agent: &str) -> Option<&'static Agent> { + AGENTS + .iter() + .find(|(name, _)| *name == agent) + .map(|(_, row)| row) +} + +/// One of [`AGENT_VALUE_OPTIONS`] at `line[at]`, with its value and how many words +/// the pair took: two for `--model opus`, one for `--model=opus`. +/// +/// `Ok(None)` is a word that is not one of them. A missing value, an empty one or +/// one that starts with `-` is refused: `aid --model --codex ` is a typo, not a +/// model called `--codex`. +fn agent_value( + line: &[String], + at: usize, +) -> Result, UsageError> { + let word = line[at].as_str(); + for &flag in AGENT_VALUE_OPTIONS { + let (value, width) = if word == flag { + (line.get(at + 1).map(String::as_str), 2) + } else if let Some(joined) = word + .strip_prefix(flag) + .and_then(|rest| rest.strip_prefix('=')) + { + (Some(joined), 1) + } else { + continue; + }; + return match value { + Some(value) if usable_value(value) => Ok(Some((flag, value, width))), + _ => Err(UsageError::MissingValue { flag }), + }; + } + Ok(None) +} + /// Which agent `--gemini` and friends name. fn agent_flag(word: &str) -> Option<&'static str> { let named = word.strip_prefix("--")?; @@ -897,8 +1261,9 @@ pub(crate) fn build_agent_command( agent: &str, prompt: &str, remote_control: Option<&str>, + tuning: &Tuning, ) -> Option> { - agent_line(agent, Opening::Prompt(prompt), remote_control, None) + agent_line(agent, Opening::Prompt(prompt), remote_control, tuning, None) } /// The argv that reopens one of the agent's earlier sessions inside the workspace. @@ -910,8 +1275,9 @@ pub(crate) fn build_agent_command( pub(crate) fn build_resume_command( agent: &str, remote_control: Option<&str>, + tuning: &Tuning, ) -> Option> { - agent_line(agent, Opening::Resume, remote_control, None) + agent_line(agent, Opening::Resume, remote_control, tuning, None) } /// How a session begins: with a prompt (empty for none), or by reopening one. @@ -930,12 +1296,32 @@ fn agent_line( agent: &str, opening: Opening<'_>, remote_control: Option<&str>, + tuning: &Tuning, session: Option<&str>, ) -> Option> { - let (_, started) = AGENTS.iter().find(|(name, _)| *name == agent)?; + let started = agent_row(agent)?; // No prompt to be interactive about: start the agent's plain session, without // the flags that only make sense alongside one. let mut words: Vec<&str> = started.command.to_vec(); + // The model before the effort, in one fixed order whatever order they were + // typed in. An effort beside an agent with no spelling for one is dropped here, + // which only a caller that skipped the parse can reach. + let settings: Vec = [ + tuning + .model + .as_deref() + .map(|model| started.model.words(model)), + started + .effort + .as_ref() + .zip(tuning.effort.as_deref()) + .map(|(spelling, effort)| spelling.words(effort)), + ] + .into_iter() + .flatten() + .flatten() + .collect(); + words.extend(settings.iter().map(String::as_str)); let named_session = started .remote_control .zip(remote_control) @@ -1060,26 +1446,35 @@ pub(crate) fn build_launch( } RemoteControl::Off => None, }; - let (command, agent, restored, remote) = match &parsed.task { + let (command, agent, restored, remote, tuning) = match &parsed.task { Task::Agent { agent, prompt, remote_control, + tuning, } => { let remote = named(remote_control); - let command = agent_line(agent, Opening::Prompt(prompt), remote.as_deref(), session)?; - (command, agent, session, remote) + let command = agent_line( + agent, + Opening::Prompt(prompt), + remote.as_deref(), + tuning, + session, + )?; + (command, agent, session, remote, tuning) } Task::Resume { agent, remote_control, + tuning, } => { let remote = named(remote_control); ( - build_resume_command(agent, remote.as_deref())?, + build_resume_command(agent, remote.as_deref(), tuning)?, agent, None, remote, + tuning, ) } Task::Retired => { @@ -1091,12 +1486,20 @@ pub(crate) fn build_launch( }; let removed = args.iter().any(|word| word == "--rm"); let resume = (!removed).then(|| { - let line = agent_line(agent, Opening::Restore(restored), remote.as_deref(), None)?; - let by_id = agent_line(agent, Opening::Resume, remote.as_deref(), None).filter(|_| { - AGENTS - .iter() - .any(|(name, it)| name == agent && it.session_id.is_some()) - }); + // The same model and effort as the line it reopens, as `aid resume` has. + let line = agent_line( + agent, + Opening::Restore(restored), + remote.as_deref(), + tuning, + None, + )?; + let by_id = + agent_line(agent, Opening::Resume, remote.as_deref(), tuning, None).filter(|_| { + AGENTS + .iter() + .any(|(name, it)| name == agent && it.session_id.is_some()) + }); dl::AgentResume::new( line.iter().cloned().collect(), by_id.map(|by_id| by_id.iter().cloned().collect()), @@ -1424,7 +1827,7 @@ mod tests { /// The agent's argv, for a name the table has. fn agent_argv(agent: &str, prompt: &str, remote_control: Option<&str>) -> Vec { - build_agent_command(agent, prompt, remote_control) + build_agent_command(agent, prompt, remote_control, &Tuning::default()) .expect("a known agent") .iter() .cloned() @@ -1555,7 +1958,10 @@ mod tests { #[test] fn an_agent_nothing_knows_has_no_command() { - assert_eq!(build_agent_command("clippy", "hi", None), None); + assert_eq!( + build_agent_command("clippy", "hi", None, &Tuning::default()), + None + ); } // ------------------------------------------- the dl command line @@ -1634,7 +2040,10 @@ mod tests { .expect("a usable command line"); assert_eq!(chosen.agent(), Some(name)); - assert!(build_agent_command(name, "hi", None).is_some(), "{name}"); + assert!( + build_agent_command(name, "hi", None, &Tuning::default()).is_some(), + "{name}" + ); } } @@ -2427,6 +2836,326 @@ mod tests { ); } + // --- the model and the effort -------------------------------------------- + + /// The words the agent is started with, off the dl line: everything after `--`. + fn agent_words(argv: &[&str]) -> Vec { + let dl = build_dl_args(&parsed(argv), &id_of).expect("an agent line"); + let at = dl + .iter() + .position(|word| word == "--") + .expect("a `--` tail"); + dl[at + 1..].to_vec() + } + + /// What `parse_aid_args` refused the line with. + fn refused(argv: &[&str]) -> UsageError { + parse_aid_args(&words(argv), Environment::default()).expect_err("a refusal") + } + + #[test] + fn a_model_reaches_each_agent_as_its_own_flag() { + // Ahead of the Remote Control flag and the prompt, so neither can be read as + // the model's value. All three CLIs happen to spell it `--model `. + assert_eq!( + agent_words(&["--model", "opus", "owner/repo", "fix", "it"]), + [ + "CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1", + "IS_SANDBOX=1", + "claude", + "--dangerously-skip-permissions", + "--model", + "opus", + "--remote-control=ws-id", + "fix it", + ] + ); + assert_eq!( + agent_words(&["--codex", "--model", "gpt-5.5", "owner/repo", "hi"]), + [ + "codex", + "--dangerously-bypass-approvals-and-sandbox", + "--model", + "gpt-5.5", + "hi", + ] + ); + assert_eq!( + agent_words(&["--gemini", "--model", "gemini-3-pro", "owner/repo", "hi"]), + [ + "gemini", + "--yolo", + "--model", + "gemini-3-pro", + "--prompt-interactive", + "hi", + ] + ); + } + + #[test] + fn an_effort_reaches_each_agent_that_has_one_in_that_agents_spelling() { + // claude has a flag of its own. codex has none: effort is a config key, and + // `-c key=value` is how codex overrides one from the command line. A value + // that does not parse as TOML is taken as a literal string, so the bare + // `high` needs no quotes (codex-rs/utils/cli/src/config_override.rs). + assert_eq!( + agent_words(&["--effort", "high", "owner/repo"]), + [ + "CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1", + "IS_SANDBOX=1", + "claude", + "--dangerously-skip-permissions", + "--effort", + "high", + "--remote-control=ws-id", + ] + ); + assert_eq!( + agent_words(&["--codex", "--effort", "high", "owner/repo"]), + [ + "codex", + "--dangerously-bypass-approvals-and-sandbox", + "-c", + "model_reasoning_effort=high", + ] + ); + } + + #[test] + fn the_model_goes_ahead_of_the_effort_whichever_was_typed_first() { + assert_eq!( + agent_words(&["--effort", "max", "--model", "opus", "owner/repo"]), + agent_words(&["--model", "opus", "--effort", "max", "owner/repo"]), + ); + } + + #[test] + fn a_value_is_passed_on_as_typed_and_never_checked() { + // The agents add models and effort levels faster than aid is released, and + // codex already takes effort values it does not know by name. A value the + // agent does not have is the agent's to answer for: claude warns and moves on. + assert_eq!( + agent_words(&["--codex", "--effort", "ultra-new", "owner/repo"]), + [ + "codex", + "--dangerously-bypass-approvals-and-sandbox", + "-c", + "model_reasoning_effort=ultra-new", + ] + ); + } + + #[test] + fn a_value_can_be_joined_to_its_flag() { + assert_eq!( + parsed(&["--model=opus", "--effort=high", "owner/repo"]), + parsed(&["--model", "opus", "--effort", "high", "owner/repo"]), + ); + } + + #[test] + fn the_last_value_typed_for_a_flag_wins() { + assert_eq!( + parsed(&["--model", "sonnet", "--model", "opus", "owner/repo"]), + parsed(&["--model", "opus", "owner/repo"]), + ); + } + + #[test] + fn a_flag_with_no_value_is_refused_by_name() { + // A flag in the value's place is the typo this catches: `aid --model --codex + // ` would otherwise start claude with a model called `--codex`. + for (argv, flag) in [ + (vec!["--model"], MODEL_FLAG), + (vec!["--model="], MODEL_FLAG), + (vec!["--model", "--codex", "owner/repo"], MODEL_FLAG), + (vec!["--effort", "", "owner/repo"], EFFORT_FLAG), + ] { + assert_eq!( + refused(&argv), + UsageError::MissingValue { flag }, + "{argv:?}" + ); + } + } + + #[test] + fn an_effort_beside_an_agent_that_has_none_is_refused() { + // gemini has no effort setting, so a typed `--effort` has nothing to reach. + // Refused rather than dropped, for the reason `--remote-control` is: the + // person asked for something by name. + assert_eq!( + refused(&["--gemini", "--effort", "high", "owner/repo"]), + UsageError::EffortUnsupported { + agent: "gemini".to_owned() + } + ); + } + + #[test] + fn the_flags_after_the_spec_are_prompt_like_any_other_flag() { + let chosen = parsed(&["owner/repo", "use", "--model", "opus"]); + assert_eq!(prompt(&chosen), "use --model opus"); + assert_eq!( + chosen, + parsed(&["owner/repo"]).with_prompt("use --model opus".to_owned()) + ); + } + + #[test] + fn the_background_boot_carries_neither_flag() { + // The boot is `dl up`, and dl has never heard of either word. + assert_eq!( + build_boot_args(&parsed(&[ + "--model", + "opus", + "--effort", + "high", + "owner/repo" + ])), + ["owner/repo", "up"] + ); + } + + #[test] + fn a_login_the_picker_chose_reaches_dl_ahead_of_the_spec() { + let chosen = relaunched( + &words(&["owner/repo"]), + Environment::default(), + &Launcher { + agent: "claude", + profile: Some("work".to_owned()), + }, + ) + .expect("a usable command line"); + let dl = build_dl_args(&chosen, &id_of).expect("an agent line"); + assert_eq!(dl[..3], ["--claude-profile", "work", "owner/repo"]); + } + + #[test] + fn a_profile_on_the_line_is_seen_in_either_spelling() { + assert_eq!( + parsed(&["--claude-profile", "work", "owner/repo"]).claude_profile(), + Some("work") + ); + assert_eq!( + parsed(&["--claude-profile=work", "owner/repo"]).claude_profile(), + Some("work") + ); + assert_eq!(parsed(&["owner/repo"]).claude_profile(), None); + } + + #[test] + fn the_settings_each_agent_takes_are_read_off_the_table() { + assert!(takes("gemini", Knob::Model)); + assert!(!takes("gemini", Knob::Effort)); + assert!(takes("codex", Knob::Effort)); + assert!(suggestions("claude", Knob::Effort).contains(&"max")); + assert!(suggestions("gemini", Knob::Effort).is_empty()); + } + + #[test] + fn a_tuning_the_pickers_settled_replaces_the_lines_own() { + let settled = Tuning { + model: Some("opus".to_owned()), + effort: None, + }; + let chosen = parsed(&["owner/repo"]).with_tuning(settled.clone()); + assert_eq!(chosen.tuning(), Some(&settled)); + assert_eq!(chosen, parsed(&["--model", "opus", "owner/repo"])); + } + + fn launcher(agent: &'static str, profile: Option<&str>) -> Launcher { + Launcher { + agent, + profile: profile.map(str::to_owned), + } + } + + fn launchers_for(argv: &[&str], profiles: &[&str]) -> Vec { + let argv = words(argv); + let profiles: Vec = profiles.iter().map(|name| (*name).to_owned()).collect(); + launchers( + &argv, + Environment::default(), + &parse_aid_args(&argv, Environment::default()).expect("a usable command line"), + &profiles, + ) + } + + #[test] + fn the_agent_picker_offers_each_claude_login_then_each_other_agent() { + assert_eq!( + launchers_for(&["owner/repo"], &["personal", "work"]), + [ + launcher("claude", None), + launcher("claude", Some("personal")), + launcher("claude", Some("work")), + launcher("codex", None), + launcher("gemini", None), + ] + ); + } + + #[test] + fn a_typed_agent_offers_only_its_own_logins() { + assert_eq!( + launchers_for(&["--claude", "owner/repo"], &["work"]), + [launcher("claude", None), launcher("claude", Some("work"))] + ); + assert_eq!( + launchers_for(&["--codex", "owner/repo"], &["work"]), + [launcher("codex", None)] + ); + } + + #[test] + fn a_named_login_on_the_line_offers_nothing() { + assert!(launchers_for(&["--claude-profile", "work", "owner/repo"], &["work"]).is_empty()); + } + + #[test] + fn a_row_the_line_would_refuse_is_not_offered() { + // The refusals are parse_aid_args's, read by trying each row. + assert_eq!( + launchers_for(&["--remote-control", "owner/repo"], &[]), + [launcher("claude", None)] + ); + assert_eq!( + launchers_for(&["--effort", "high", "owner/repo"], &[]), + [launcher("claude", None), launcher("codex", None)] + ); + } + + #[test] + fn a_chosen_row_is_the_line_with_its_flags_in_front() { + let argv = words(&["--model", "opus", "owner/repo"]); + let chosen = relaunched( + &argv, + Environment::default(), + &launcher("claude", Some("work")), + ) + .expect("a usable command line"); + assert_eq!(chosen.claude_profile(), Some("work")); + assert_eq!( + chosen.tuning().and_then(|t| t.model.as_deref()), + Some("opus") + ); + + let codex = relaunched(&argv, Environment::default(), &launcher("codex", None)) + .expect("a usable command line"); + assert_eq!(codex.agent(), Some("codex")); + // Remote Control was only the default, so codex is quietly without it. + assert_eq!(remote_control(&codex), RemoteControl::Off); + } + + #[test] + fn a_row_is_kept_by_its_agent_and_login() { + assert_eq!(launcher("claude", None).key(), "claude"); + assert_eq!(launcher("claude", Some("work")).key(), "claude/work"); + } + #[test] fn a_remote_control_session_is_named_after_the_workspace_id_not_the_spec() { // Claude Code's `SendMessage` refuses any `to` with a `/` in it ("to must be @@ -2534,6 +3263,7 @@ mod tests { Task::Resume { agent: "claude".to_owned(), remote_control: RemoteControl::On, + tuning: Tuning::default(), } ); } @@ -2629,6 +3359,46 @@ mod tests { assert_eq!(parsed(&["resume", "ws", "--autorm"]).task, Task::Retired); } + #[test] + fn a_resumed_session_takes_the_model_and_effort_too() { + // Ahead of Remote Control, and the resume words stay last, where claude's + // `--resume [value]` needs them. + let dl = build_dl_args( + &parsed(&["--model", "opus", "--effort", "max", "resume", "ws"]), + &id_of, + ) + .expect("a resume line"); + let at = dl + .iter() + .position(|word| word == "--") + .expect("a `--` tail"); + assert_eq!( + dl[at + 1..], + [ + "CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1", + "IS_SANDBOX=1", + "claude", + "--dangerously-skip-permissions", + "--model", + "opus", + "--effort", + "max", + "--remote-control=ws-id", + "--resume", + ] + ); + } + + #[test] + fn an_effort_on_a_resume_line_for_gemini_is_refused_like_any_other() { + assert_eq!( + refused(&["--gemini", "--effort", "high", "resume", "ws"]), + UsageError::EffortUnsupported { + agent: "gemini".to_owned() + } + ); + } + // ------------------------------------------ starting again after a restart const SESSION: &str = "0f3c9a4e-8d1b-4c2a-9e7f-5b6a1d2c3e4f"; @@ -2780,4 +3550,34 @@ mod tests { .expect("a retired line"); assert_eq!(launch.resume, None); } + + /// A restore after herdr restarts is the same session, so it keeps the model + /// and effort the launch asked for, as `aid resume` does. + #[test] + fn a_restored_session_keeps_the_model_and_effort() { + let launch = build_launch( + &parsed(&["--model", "opus", "--effort", "max", "owner/repo", "hi"]), + &id_of, + Some(SESSION), + ) + .expect("an agent line"); + let tuned = &[ + "CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1", + "IS_SANDBOX=1", + "claude", + "--dangerously-skip-permissions", + "--model", + "opus", + "--effort", + "max", + "--remote-control=ws-id", + ][..]; + assert_eq!( + launch.resume, + dl::AgentResume::new( + line(&with(tuned, &["--resume", SESSION])), + Some(line(&with(tuned, &["--resume"]))), + ) + ); + } } diff --git a/rust/aid/tests/interactive.rs b/rust/aid/tests/interactive.rs index e60976e4..0467d877 100644 --- a/rust/aid/tests/interactive.rs +++ b/rust/aid/tests/interactive.rs @@ -130,6 +130,10 @@ impl PtyAid { command.env("GIT_SSH_COMMAND", "false"); command.env("GIT_CONFIG_GLOBAL", "/dev/null"); command.env("GIT_CONFIG_SYSTEM", "/dev/null"); + // A terminal type, as every real terminal sets: skim reads terminfo by it + // and draws no picker without one. `aid`'s answer to no `TERM` is its own + // test below, which removes it again. + command.env("TERM", "xterm-256color"); if !world.root.join("gh-bin/gh").exists() { command.env("DEVLAUNCH_NO_GH_TOKEN", "1"); } @@ -178,25 +182,24 @@ impl PtyAid { /// Type a line and press Enter, the way a person at the terminal would. /// - /// The line and its Enter go out in **one** `write_all`, and that is - /// load-bearing rather than tidy. A pty master write is copied into the line - /// discipline in one go, so a single write makes every completed line of a - /// paste readable at the same instant. Two writes are two of those, and the - /// gap between them is a window: the first one completes `fix this`, wakes - /// the read in `aid`, and `read_terminal_submission` drains what the terminal - /// holds *right now* — which, if the Enter completing `and then that` has not - /// been written yet, is line one alone. That is the flake in #401, and it is - /// the test lying about its own premise rather than a defect in what it - /// tests: a terminal delivering a paste writes it whole. + /// Enter is `\r`, which is what a terminal sends for it. The prompt editor + /// holds the terminal in raw mode, where nothing turns it into `\n`, and a + /// `\n` is Ctrl-J, which adds a line rather than submitting. So a `\n` inside + /// `line` is a line of the prompt. + /// + /// One `write_all`, so the Enter comes in the same burst as the text. The + /// editor reads an Enter with more input right behind it as a pasted line + /// break; nothing follows this one, so it submits. fn send_line(&mut self, line: &str) { self.writer - .write_all(format!("{line}\n").as_bytes()) + .write_all(format!("{line}\r").as_bytes()) .and_then(|()| self.writer.flush()) .expect("typing into the pty"); } - /// A terminal Ctrl-C: the byte the line discipline turns into SIGINT for the - /// whole foreground process group. + /// A terminal Ctrl-C. In cooked mode the line discipline turns it into SIGINT + /// for the whole foreground process group. The prompt editor and the pickers + /// hold the terminal in raw mode, where it is a byte they read and act on. fn interrupt(&mut self) { self.writer .write_all(b"\x03") @@ -204,6 +207,53 @@ impl PtyAid { .expect("interrupting the pty"); } + /// Press keys in a picker. Raw bytes, because skim holds the terminal in raw + /// mode: Enter is `\r` there, not a line. + fn press(&mut self, keys: &str) { + self.writer + .write_all(keys.as_bytes()) + .and_then(|()| self.writer.flush()) + .expect("typing into the pty"); + } + + /// Wait for the picker whose header holds `header` and answer it with `keys`. + /// + /// Counted by occurrences, so a second picker with the same words is waited + /// for rather than answered from the first one's screen. + fn answer(&mut self, header: &str, keys: &str) { + let before = self.text().matches(header).count(); + assert!( + wait_for(|| self.text().matches(header).count() > before), + "the picker {header:?} never appeared; the pty said:\n{}", + self.text() + ); + // skim draws its header before it reads keys; a short pause keeps a key + // from landing while it is still setting up the terminal. + std::thread::sleep(Duration::from_millis(150)); + self.press(keys); + } + + /// Answer a picker by typing `query`, then pressing Enter once skim has matched. + /// + /// Two writes with a pause between, because skim takes Enter against the last + /// match it finished: a query and its Enter in one write reach it before the + /// match does, and Enter takes the old first row. + fn answer_typed(&mut self, header: &str, query: &str) { + self.answer(header, query); + std::thread::sleep(Duration::from_millis(400)); + self.press("\r"); + } + + /// Take the first row of the agent, model and effort pickers, then wait for + /// the prompt editor. On a first run that is claude with every default, which + /// is every launch's way in when nothing is chosen. + fn reach_the_editor(&mut self) { + self.answer(AGENT_PICKER, "\r"); + self.answer(MODEL_PICKER, "\r"); + self.answer(EFFORT_PICKER, "\r"); + self.expect(BANNER); + } + fn wait(mut self) -> u32 { self.child.wait().expect("aid exits").exit_code() } @@ -224,6 +274,15 @@ fn wait_for(mut ready: impl FnMut() -> bool) -> bool { /// the one the e2e suite keys on too. const BANNER: &str = "press Enter"; +/// Words in the header of claude's model picker. +const MODEL_PICKER: &str = "Model for claude"; + +/// Words in the header of claude's effort picker. +const EFFORT_PICKER: &str = "Effort for claude"; + +/// Words in the header of the agent picker, which also chooses claude's login. +const AGENT_PICKER: &str = "Agent for this launch"; + /// The name [`MAIN`] is put on a terminal under. /// /// The label and not the id, although the id is what is typed: the scenario records @@ -255,7 +314,7 @@ fn the_terminal_really_is_named_on_a_real_pty() { let by_id = osc_title(MAIN); let world = World::with(&["--warm"]); let mut session = PtyAid::spawn(&world, &[MAIN], &[]); - session.expect(BANNER); + session.reach_the_editor(); // Exact bytes and from the very first one: nothing precedes the name on this // terminal, so a prefix check is the whole claim about what the editor window // is titled. @@ -324,7 +383,7 @@ fn a_falsey_no_tty_leaves_the_terminal_alone_on_a_real_pty() { let world = World::with(&["--warm"]); for value in ["FALSE", " no "] { let mut session = PtyAid::spawn(&world, &[MAIN], &[("DEVLAUNCH_NO_TTY", value)]); - session.expect(BANNER); + session.reach_the_editor(); session.send_line("fix the bug"); session.expect("aid -> dl"); assert_eq!(session.wait(), 0, "DEVLAUNCH_NO_TTY={value:?}"); @@ -358,7 +417,7 @@ fn a_typed_prompt_reaches_the_agent_with_no_shell_in_the_way() { // editor exists to end. let world = World::with(&["--warm"]); let mut session = PtyAid::spawn(&world, &[MAIN], &[]); - session.expect(BANNER); + session.reach_the_editor(); session.send_line("fix the \"flaky\" test"); session.expect("aid -> dl"); assert_eq!(session.wait(), 0); @@ -372,6 +431,24 @@ fn a_typed_prompt_reaches_the_agent_with_no_shell_in_the_way() { ); } +#[test] +fn a_lone_esc_at_the_editor_does_not_swallow_the_next_key() { + // The pickers teach that Esc cancels, so a person presses it here too. On its + // own, with a pause after it, it is not the Alt of the next key. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.reach_the_editor(); + session.press("\x1b"); + std::thread::sleep(Duration::from_millis(100)); + session.press("go"); + std::thread::sleep(Duration::from_millis(100)); + session.press("\r"); + session.expect("aid -> dl"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!(last.ends_with(" go'"), "{last}"); +} + #[test] fn a_pasted_multi_line_prompt_arrives_whole_rather_than_leaking() { // A paste delivers its newlines with it, and the terminal holds the later @@ -381,11 +458,12 @@ fn a_pasted_multi_line_prompt_arrives_whole_rather_than_leaking() { // keystrokes. let world = World::with(&["--warm"]); let mut session = PtyAid::spawn(&world, &[MAIN], &[]); - session.expect(BANNER); - // One write, as a terminal delivers a paste: both lines arrive together, so - // the second is already queued when the first's Enter is read. `send_line` - // issuing exactly one write is what keeps that sentence true. - session.send_line("fix this\nand then that"); + session.reach_the_editor(); + // One write, as a terminal without bracketed paste delivers one. Each line + // ends in `\r`, the same byte as Enter, and the second line is already queued + // when the first `\r` is read: that queued input is all that makes the first + // `\r` a line break. The last one has nothing behind it and submits. + session.press("fix this\rand then that\r"); assert_eq!(session.wait(), 0); assert_eq!( &without_session_id(world.devpod_calls().last().expect("a session")), @@ -401,7 +479,7 @@ fn a_pasted_multi_line_prompt_arrives_whole_rather_than_leaking() { fn an_empty_enter_is_the_plain_session_it_always_was() { let world = World::with(&["--warm"]); let mut session = PtyAid::spawn(&world, &[MAIN], &[]); - session.expect(BANNER); + session.reach_the_editor(); session.send_line(""); assert_eq!(session.wait(), 0); assert_eq!( @@ -533,7 +611,7 @@ fn the_boot_runs_while_the_prompt_is_still_being_typed() { // attach that follows the Enter finds it running. let world = World::with(&["--stopped"]); let mut session = PtyAid::spawn(&world, &[MAIN], &[]); - session.expect(BANNER); + session.reach_the_editor(); assert!( wait_for(|| { world @@ -557,9 +635,10 @@ fn the_boot_runs_while_the_prompt_is_still_being_typed() { #[test] fn a_ctrl_c_at_the_editor_tears_the_whole_boot_down() { - // `tests/interrupt.rs` on the pty: the boot child is in aid's process group, - // so the terminal's SIGINT reaches it, and its own handler kills the blocked - // `devpod up` — now in a group of its own — and unlinks the staged token. + // `tests/interrupt.rs` on the pty. The editor holds the terminal in raw mode, + // so the Ctrl-C is a byte and no SIGINT is sent: aid itself interrupts the + // boot child, whose own handler kills the blocked `devpod up` (in a group of + // its own) and unlinks the staged token. let world = World::with(&["--gh"]); let devpod = world.root.join("bin/devpod"); let original = std::fs::read_to_string(&devpod).expect("the scenario's devpod"); @@ -594,7 +673,7 @@ fn a_ctrl_c_at_the_editor_tears_the_whole_boot_down() { ("DL_UP_STARTED", &up_started.display().to_string()), ], ); - session.expect(BANNER); + session.reach_the_editor(); // Interrupt only once the boot is mid-`up` with the token staged — the exact // state the interrupt handler exists to clean. assert!( @@ -631,6 +710,568 @@ fn token_file(dir: &Path) -> Option { }) } +// =========================================================================== +// the pickers ahead of the editor +// =========================================================================== + +/// The claude payload a launch sends, with the words the pickers added. +fn claude_session(settings: &str, prompt: &str) -> String { + format!( + "devpod ssh {MAIN} --log-output json --command bash -lc 'CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 \ + IS_SANDBOX=1 claude --dangerously-skip-permissions {settings}--remote-control={MAIN} {prompt}'" + ) +} + +#[test] +fn a_model_that_is_not_listed_is_typed_and_reaches_the_agent() { + // Nothing lists `claude-opus-5-5`, so the query matches no row and Enter takes + // the query itself. The effort is picked from the list by filtering to it. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.answer(AGENT_PICKER, "\r"); + session.answer_typed(MODEL_PICKER, "claude-opus-5-5"); + session.answer_typed(EFFORT_PICKER, "max"); + session.expect("(model claude-opus-5-5, effort max)"); + session.expect(BANNER); + session.send_line("go"); + assert_eq!(session.wait(), 0); + assert_eq!( + &without_session_id(world.devpod_calls().last().expect("a session")), + &claude_session("--model claude-opus-5-5 --effort max ", "go") + ); +} + +#[test] +fn the_last_choice_is_the_first_row_of_the_next_launch() { + // One world, two launches: the second takes the first row of each picker and + // gets what the first launch chose, because the cache remembered it. + let world = World::with(&["--warm"]); + let mut first = PtyAid::spawn(&world, &[MAIN], &[]); + first.answer(AGENT_PICKER, "\r"); + first.answer_typed(MODEL_PICKER, "sonnet"); + first.answer_typed(EFFORT_PICKER, "low"); + first.expect(BANNER); + first.send_line("one"); + assert_eq!(first.wait(), 0); + + let mut second = PtyAid::spawn(&world, &[MAIN], &[]); + second.reach_the_editor(); + second.send_line("two"); + assert_eq!(second.wait(), 0); + assert_eq!( + &without_session_id(world.devpod_calls().last().expect("a session")), + &claude_session("--model sonnet --effort low ", "two") + ); +} + +#[test] +fn a_setting_a_flag_gave_is_not_asked_for() { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &["--model", "opus", MAIN], &[]); + session.answer(AGENT_PICKER, "\r"); + session.answer(EFFORT_PICKER, "\r"); + session.expect(BANNER); + assert!( + !session.text().contains(MODEL_PICKER), + "the model picker was drawn although --model gave the model" + ); + session.send_line("go"); + assert_eq!(session.wait(), 0); + assert_eq!( + &without_session_id(world.devpod_calls().last().expect("a session")), + &claude_session("--model opus ", "go") + ); +} + +#[test] +fn with_no_terminal_type_the_pickers_are_drawn_as_the_fallback_terminal() { + // skim cannot draw without a `TERM`, and used to panic on one that was not + // set. The pickers take the workspace picker's fallback and say so, and each + // one is still answered with Enter. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[("TERM", "")]); + session.answer(AGENT_PICKER, "\r"); + session.answer(MODEL_PICKER, "\r"); + session.answer(EFFORT_PICKER, "\r"); + session.expect(BANNER); + assert!( + session.text().contains("drawn as xterm-256color"), + "{}", + session.text() + ); + session.send_line("go"); + assert_eq!(session.wait(), 0); + assert_eq!( + &without_session_id(world.devpod_calls().last().expect("a session")), + &claude_session("", "go") + ); +} + +#[test] +fn a_named_claude_login_is_a_row_of_the_agent_picker_and_reaches_dl() { + // A host with one named profile holding a credential gets a claude row for it + // in the agent picker. The row is found by filtering to its name, as a person + // would. + let world = World::with(&["--warm"]); + let profiles = world.root.join("profiles"); + std::fs::create_dir_all(profiles.join("work")).expect("a profile directory"); + std::fs::write(profiles.join("work/.credentials.json"), "{}").expect("a credential"); + let mut session = PtyAid::spawn( + &world, + &[MAIN], + &[( + "DEVLAUNCH_CLAUDE_PROFILES_DIR", + &profiles.display().to_string(), + )], + ); + session.answer_typed(AGENT_PICKER, "work"); + session.answer(MODEL_PICKER, "\r"); + session.answer(EFFORT_PICKER, "\r"); + session.expect(BANNER); + session.expect("(account work)"); + session.send_line("go"); + session.expect(&format!("aid -> dl --claude-profile work {MAIN} --")); + assert_eq!(session.wait(), 0); +} + +#[test] +fn another_agent_is_a_row_of_the_same_picker() { + // codex is chosen by name, and its own pickers follow. Remote Control was only + // claude's default, so codex starts without it and nothing refuses. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.answer_typed(AGENT_PICKER, "codex"); + session.answer(MODEL_PICKER.replace("claude", "codex").as_str(), "\r"); + session.answer_typed(EFFORT_PICKER.replace("claude", "codex").as_str(), "high"); + session.expect(BANNER); + session.send_line("hi"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!( + last.contains( + "codex --dangerously-bypass-approvals-and-sandbox -c model_reasoning_effort=high hi" + ), + "{last}" + ); +} + +#[test] +fn alt_enter_takes_a_model_that_is_a_prefix_of_a_listed_one() { + // Matching is by substring, so `gpt-5.5` still matches the remembered + // `gpt-5.5-codex` and Enter takes that row. Alt-Enter (ESC CR on a terminal) + // takes the query as typed. + let world = World::with(&["--warm"]); + let codex_model = MODEL_PICKER.replace("claude", "codex"); + let codex_effort = EFFORT_PICKER.replace("claude", "codex"); + let mut first = PtyAid::spawn(&world, &["--codex", MAIN], &[]); + first.answer_typed(&codex_model, "gpt-5.5-codex"); + first.answer(&codex_effort, "\r"); + first.expect(BANNER); + first.send_line("one"); + assert_eq!(first.wait(), 0); + + let mut second = PtyAid::spawn(&world, &["--codex", MAIN], &[]); + second.answer(&codex_model, "gpt-5.5"); + std::thread::sleep(Duration::from_millis(400)); + second.press("\x1b\r"); + second.answer(&codex_effort, "\r"); + second.expect("(model gpt-5.5)"); + second.expect(BANNER); + second.send_line("two"); + assert_eq!(second.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!(last.contains("--model gpt-5.5 "), "{last}"); +} + +#[test] +fn a_typed_agent_with_one_login_is_not_asked_which_agent() { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &["--claude", MAIN], &[]); + session.answer(MODEL_PICKER, "\r"); + session.answer(EFFORT_PICKER, "\r"); + session.expect(BANNER); + assert!(!session.text().contains(AGENT_PICKER), "{}", session.text()); + session.send_line(""); + assert_eq!(session.wait(), 0); +} + +#[test] +fn esc_in_a_picker_stops_the_boot_and_launches_nothing() { + // The same world as the Ctrl-C test above: an `up` that blocks with the token + // staged. In a picker Ctrl-C and Esc are keys, not signals, so nothing reaches + // the boot unless aid sends it. The cleanup is the proof that it did. + let world = World::with(&["--gh"]); + let devpod = world.root.join("bin/devpod"); + let original = std::fs::read_to_string(&devpod).expect("the scenario's devpod"); + let delegate = original + .lines() + .find(|line| line.starts_with("exec ")) + .expect("the delegate exec line"); + let script = format!( + "#!/bin/sh\n\ + if [ \"$1\" = \"up\" ]; then\n\ + \x20 echo \"$$\" > \"$DL_UP_PID\"\n\ + \x20 : > \"$DL_UP_STARTED\"\n\ + \x20 exec sleep 30\n\ + fi\n\ + {delegate}\n" + ); + std::fs::write(&devpod, script).expect("rewrite devpod"); + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(&devpod, std::fs::Permissions::from_mode(0o755)) + .expect("keep devpod executable"); + let tmpdir = world.root.join("tmp"); + std::fs::create_dir_all(&tmpdir).expect("a scratch TMPDIR"); + let up_pid = world.root.join("up.pid"); + let up_started = world.root.join("up.started"); + + let mut session = PtyAid::spawn( + &world, + &["blooop/devlaunch@cold"], + &[ + ("TMPDIR", &tmpdir.display().to_string()), + ("DL_UP_PID", &up_pid.display().to_string()), + ("DL_UP_STARTED", &up_started.display().to_string()), + ], + ); + session.answer(AGENT_PICKER, "\r"); + session.answer(MODEL_PICKER, ""); + assert!( + wait_for(|| up_started.exists() && token_file(&tmpdir).is_some()), + "devpod up never blocked with a token staged" + ); + let up = std::fs::read_to_string(&up_pid).expect("the up pid"); + let up = up.trim().to_owned(); + + session.press("\x1b"); + session.expect("cancelled"); + let seen = Arc::clone(&session.seen); + assert_eq!(session.wait(), 130); + assert!( + !String::from_utf8_lossy(&seen.lock().expect("the pty buffer")).contains(BANNER), + "the editor opened after a cancel" + ); + assert!( + token_file(&tmpdir).is_none(), + "the token file must be gone once aid has waited the boot out" + ); + assert!( + wait_for(|| !Command::new("kill") + .args(["-0", &up]) + .output() + .expect("kill is installed") + .status + .success()), + "the devpod up (pid {up}) must have been killed" + ); + assert!( + !world + .devpod_calls() + .iter() + .any(|call| call.starts_with("devpod ssh")), + "a session was opened after a cancel: {:?}", + world.devpod_calls() + ); +} + +/// The agent sessions devpod was asked for so far. The boot's own `devpod ssh` +/// calls provision the workspace and are left out: only a session asks for +/// `--log-output json`. +fn sessions(world: &World) -> Vec { + world + .devpod_calls() + .into_iter() + .filter(|call| call.starts_with(&format!("devpod ssh {MAIN} --log-output json "))) + .collect() +} + +#[test] +fn the_agent_chosen_last_is_the_first_row_of_the_next_launch() { + // The first run lists claude first, so an Enter on the second launch's agent + // picker takes codex only if the first launch's choice moved it up. + let world = World::with(&["--warm"]); + let mut first = PtyAid::spawn(&world, &[MAIN], &[]); + first.answer_typed(AGENT_PICKER, "codex"); + first.answer("Model for codex", "\r"); + first.answer("Effort for codex", "\r"); + first.expect(BANNER); + first.send_line("one"); + assert_eq!(first.wait(), 0); + + let mut second = PtyAid::spawn(&world, &[MAIN], &[]); + second.answer(AGENT_PICKER, "\r"); + // Waited for by the words every agent's model picker shares, so a claude row + // fails here at once rather than at the deadline. + second.answer("Model for ", "\r"); + assert!( + second.text().contains("Model for codex"), + "the first row was not codex; the pty said:\n{}", + second.text() + ); + second.answer("Effort for codex", "\r"); + second.expect(BANNER); + second.send_line("two"); + assert_eq!(second.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!( + last.contains("codex --dangerously-bypass-approvals-and-sandbox two"), + "{last}" + ); +} + +#[test] +fn a_typed_agent_that_is_no_row_is_asked_for_again() { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.answer_typed(AGENT_PICKER, "nonesuch"); + // `answer` counts the header, so it waits for the picker drawn a second time. + session.answer(AGENT_PICKER, "\x1b"); + session.expect("cancelled"); + let seen = Arc::clone(&session.seen); + assert_eq!(session.wait(), 130); + let whole = String::from_utf8_lossy(&seen.lock().expect("the pty buffer")).into_owned(); + assert!( + !whole.contains(MODEL_PICKER), + "a typed name that is no agent went on to the model picker; the pty said:\n{whole}" + ); + assert!( + sessions(&world).is_empty(), + "a session was opened: {:?}", + sessions(&world) + ); +} + +#[test] +fn esc_at_the_agent_picker_launches_nothing() { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.answer(AGENT_PICKER, "\x1b"); + session.expect("cancelled"); + let seen = Arc::clone(&session.seen); + assert_eq!(session.wait(), 130); + let whole = String::from_utf8_lossy(&seen.lock().expect("the pty buffer")).into_owned(); + assert!( + !whole.contains(MODEL_PICKER) && !whole.contains(BANNER), + "the launch went on after the Esc; the pty said:\n{whole}" + ); + assert!( + sessions(&world).is_empty(), + "a session was opened after a cancel: {:?}", + sessions(&world) + ); +} + +#[test] +fn a_setting_a_flag_gave_is_the_first_row_of_the_next_launch() { + // Remembered as a choice although no picker asked for it. + let world = World::with(&["--warm"]); + let mut first = PtyAid::spawn(&world, &["--model", "opus", MAIN], &[]); + first.answer(AGENT_PICKER, "\r"); + first.answer(EFFORT_PICKER, "\r"); + first.expect(BANNER); + first.send_line("one"); + assert_eq!(first.wait(), 0); + + let mut second = PtyAid::spawn(&world, &[MAIN], &[]); + second.reach_the_editor(); + second.send_line("two"); + assert_eq!(second.wait(), 0); + assert_eq!( + &without_session_id(world.devpod_calls().last().expect("a session")), + &claude_session("--model opus ", "two") + ); +} + +// =========================================================================== +// the prompt editor +// =========================================================================== + +#[test] +fn a_pasted_prompt_keeps_its_line_breaks_and_waits_for_enter() { + // The paste ends in a line break, as a copied block of text often does. Under + // the cooked read that break submitted the prompt; here it is text, and the + // prompt is sent only on the Enter after it. Windows line ends are one break. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.reach_the_editor(); + // Two writes, so one read ends on a `\r` with nothing queued behind it. The + // open paste is then the only thing that keeps that `\r` a line break. + session.press("\x1b[200~first line\r"); + std::thread::sleep(Duration::from_millis(200)); + session.press("second line\r\n\x1b[201~"); + std::thread::sleep(Duration::from_millis(300)); + assert!( + !session.text().contains("aid -> dl"), + "the paste's own line break submitted the prompt; devpod was asked for {:?}", + world.devpod_calls() + ); + session.press("\r"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!(last.contains("first line\nsecond line'"), "{last}"); +} + +#[test] +fn a_paste_longer_than_the_kernels_line_limit_arrives_whole() { + // The cooked read went through the line discipline, which holds 4096 bytes of + // one line and drops the rest. Twice that, in pieces, to be sure. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.reach_the_editor(); + let long = "y".repeat(9000); + session.press("\x1b[200~"); + for chunk in long.as_bytes().chunks(3000) { + session.press(std::str::from_utf8(chunk).expect("ascii")); + std::thread::sleep(Duration::from_millis(50)); + } + session.press("\x1b[201~"); + std::thread::sleep(Duration::from_millis(200)); + session.press("\r"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!( + last.contains(&format!(" {long}'")), + "{} bytes long", + last.len() + ); +} + +#[test] +fn a_paste_that_arrives_late_does_not_leak_into_the_agent() { + // The second half of the paste comes after a pause. The cooked read took what + // was queued at the first line break and left the rest for the agent session + // as keystrokes; the editor waits for the end of the paste. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.reach_the_editor(); + session.press("\x1b[200~part one\r"); + std::thread::sleep(Duration::from_millis(300)); + session.press("part two\x1b[201~"); + std::thread::sleep(Duration::from_millis(200)); + session.press("\r"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!(last.contains("part one\npart two'"), "{last}"); +} + +#[test] +fn a_line_is_added_by_typing_alt_enter_or_ctrl_j() { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[MAIN], &[]); + session.reach_the_editor(); + session.press("one\x1b\r"); + std::thread::sleep(Duration::from_millis(100)); + session.press("two\n"); + std::thread::sleep(Duration::from_millis(100)); + session.press("three"); + std::thread::sleep(Duration::from_millis(100)); + session.press("\r"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!(last.contains("one\ntwo\nthree'"), "{last}"); +} + +// =========================================================================== +// no workspace named +// =========================================================================== + +/// Words in the header of dl's workspace picker. +const WORKSPACE_PICKER: &str = "Select workspace"; + +#[test] +fn a_bare_aid_picks_a_workspace_as_a_bare_dl_does() { + // The scenario's one workspace is the only row, so Enter takes it, and the + // launch that follows is the ordinary one for that workspace. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[], &[]); + session.answer(WORKSPACE_PICKER, "\r"); + session.expect(&format!("-> {MAIN}")); + session.reach_the_editor(); + session.send_line("go"); + assert_eq!(session.wait(), 0); + assert_eq!( + &without_session_id(world.devpod_calls().last().expect("a session")), + &claude_session("", "go") + ); +} + +#[test] +fn leading_flags_with_no_workspace_pick_one_and_keep_the_flags() { + // `--codex` still chooses the agent, so there is no agent picker after the + // workspace one: the typed flag already said. + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &["--codex"], &[]); + session.answer(WORKSPACE_PICKER, "\r"); + session.answer("Model for codex", "\r"); + session.answer("Effort for codex", "\r"); + session.expect(BANNER); + assert!(!session.text().contains(AGENT_PICKER), "{}", session.text()); + session.send_line("hi"); + assert_eq!(session.wait(), 0); + let last = world.devpod_calls().last().expect("a session").clone(); + assert!( + last.contains("codex --dangerously-bypass-approvals-and-sandbox hi"), + "{last}" + ); +} + +#[test] +fn a_quit_workspace_picker_launches_nothing() { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, &[], &[]); + session.answer(WORKSPACE_PICKER, "\x1b"); + assert_eq!(session.wait(), 1); + assert!( + world.devpod_calls().is_empty(), + "{:?}", + world.devpod_calls() + ); +} + +#[test] +fn a_retired_spelling_with_no_workspace_is_refused_before_any_picker() { + // dl is what refuses a retired spelling, and it needs a workspace to be asked + // about. A picked spec would land after the retired word, which then no longer + // ends the line, and the line would become an agent launch. + for line in [&["--stop"][..], &["resume", "--autorm"]] { + let world = World::with(&["--warm"]); + let mut session = PtyAid::spawn(&world, line, &[]); + let seen = Arc::clone(&session.seen); + // Bounded: a picker that opened anyway would wait on this pty for ever. + let mut code = None; + wait_for(|| { + code = session.child.try_wait().expect("aid's status"); + code.is_some() + }); + let _ = session.child.kill(); + let said = || String::from_utf8_lossy(&seen.lock().expect("the pty buffer")).into_owned(); + assert_eq!( + code.map(|status| status.exit_code()), + Some(1), + "{line:?}: the pty said:\n{:?}", + said() + ); + assert!( + wait_for(|| said().contains("aid needs a workspace")), + "{line:?}: the refusal was not given; the pty said:\n{:?}", + said() + ); + assert!( + !said().contains(WORKSPACE_PICKER), + "{line:?}: the workspace picker opened: {:?}", + said() + ); + assert!( + !world + .devpod_calls() + .iter() + .any(|call| call.starts_with("devpod ssh")), + "{line:?}: a session was opened: {:?}", + world.devpod_calls() + ); + } +} + /// A session call with aid's `--session-id ` taken out, after checking it is /// there: the id is random per launch, and every other byte of the line is the /// assertion. diff --git a/rust/aid/tests/rewrite.rs b/rust/aid/tests/rewrite.rs index 47fdcf10..a51ab742 100644 --- a/rust/aid/tests/rewrite.rs +++ b/rust/aid/tests/rewrite.rs @@ -616,6 +616,50 @@ fn remote_control_beside_an_agent_that_has_none_is_refused_before_anything_opens assert!(world.devpod_calls().is_empty()); } +#[test] +fn a_model_and_an_effort_reach_the_agent_and_dl_never_sees_either() { + // dl has never heard of `--model` or `--effort`, so a version that passed + // either through as an unknown leading option would exit 2 here. + let world = World::with(&["--warm"]); + world + .aid(&["--model", "opus", "--effort=max", MAIN, "fix", "it"]) + .exited(0); + assert_eq!( + world.devpod_calls().last().expect("a session"), + &format!( + "devpod ssh {MAIN} --log-output json --command bash -lc \ + 'CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 IS_SANDBOX=1 claude \ + --dangerously-skip-permissions --model opus --effort max \ + --remote-control={MAIN} '\"'\"'fix it'\"'\"''" + ) + ); + + let codex = World::with(&["--warm"]); + codex + .aid(&["--codex", "--effort", "high", MAIN, "hi"]) + .exited(0); + let session = codex.devpod_calls().last().expect("a session").clone(); + assert!( + session.contains( + "codex --dangerously-bypass-approvals-and-sandbox -c model_reasoning_effort=high hi" + ), + "{session}" + ); +} + +#[test] +fn an_effort_beside_an_agent_that_has_none_is_refused_before_anything_opens() { + let world = World::with(&["--warm"]); + let run = world.aid(&["--gemini", "--effort", "high", MAIN, "hi"]); + run.exited(1); + assert_eq!( + run.err, + "--effort sets a reasoning effort, which gemini has no setting for. \ + Drop the flag or pick one of: --claude, --codex.\n" + ); + assert!(world.devpod_calls().is_empty()); +} + #[test] fn a_dl_option_is_passed_through_and_a_flag_after_the_spec_is_prompt() { // `--devcontainer` reaches dl, which says what it thinks of it for a workspace diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 5344e781..49516065 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -71,7 +71,7 @@ _dl_completion() { # flag, so a spelling this build only still answers for is never offered. local global_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --herdr-setup --herdr-env --herdr-workspace --rm --devcontainer --claude-profile --claude-profiles --help -h --version" if [[ "$cmd" == aid ]]; then - global_opts="--claude --codex --gemini --devcontainer --claude-profile --help -h --version" + global_opts="--claude --codex --gemini --model --effort --devcontainer --claude-profile --help -h --version" fi # Workspace subcommands @@ -84,6 +84,15 @@ _dl_completion() { # Options that take a value; a variant name, a profile name or a path follows. local value_opts="--devcontainer --claude-profile --herdr-workspace" + # aid's own value-taking flags, from `AGENT_VALUE_OPTIONS`. Nothing completes + # their values: the models and efforts are each agent's to list, they change + # with every agent release, and aid holds no copy of them on purpose. Empty + # for dl, which has neither flag. + local aid_value_opts="" + if [[ "$cmd" == aid ]]; then + aid_value_opts="--model --effort" + fi + # The flags a workspace spec may still follow: they modify a launch instead # of being one. Every other flag ends the line, and that direction is the # load-bearing half -- listing the flags that *end* it instead put ten flags @@ -115,7 +124,7 @@ _dl_completion() { # takes a value, so on `aid --unknown-taking-a-value foo owner/repo` it # calls `foo` the spec, and completing a slot aid itself cannot place is # worse than completing nothing. - spec_follows="--claude --codex --gemini --remote-control --remote --no-remote-control --no-remote --devcontainer --claude-profile" + spec_follows="--claude --codex --gemini --remote-control --remote --no-remote-control --no-remote --model --effort --devcontainer --claude-profile" fi if [[ "${prev}" == "--herdr-workspace" ]]; then @@ -172,6 +181,10 @@ _dl_completion() { return 0 fi + if [[ -n "${aid_value_opts}" && " ${aid_value_opts} " == *" ${prev} "* ]]; then + return 0 + fi + # After --devcontainer, offer the repo's variant directories (and paths). if [[ " ${value_opts} " == *" ${prev} "* ]]; then local variants="" @@ -232,7 +245,7 @@ _dl_completion() { else ends_here=1 fi - if [[ " ${value_opts} " == *" ${scanned} "* ]]; then + if [[ " ${value_opts} ${aid_value_opts} " == *" ${scanned} "* ]]; then # Its value is not a positional word, so step over the pair. (( scan += 2 )) else diff --git a/rust/dl/Cargo.toml b/rust/dl/Cargo.toml index c7cc31b4..565fc5a9 100644 --- a/rust/dl/Cargo.toml +++ b/rust/dl/Cargo.toml @@ -15,6 +15,9 @@ clap = { workspace = true } # `signal(2)`, so Ctrl-C exits 130 the way Python's KeyboardInterrupt did rather # than killing this process by signal (which has no exit code at all). libc = { workspace = true } +# The prompt editor's column count, so a line of wide characters wraps where the +# terminal wraps it. Already in the tree through skim. +unicode-width = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } skim = { workspace = true } diff --git a/rust/dl/src/lib.rs b/rust/dl/src/lib.rs index 2b0feb22..f40e2f81 100644 --- a/rust/dl/src/lib.rs +++ b/rust/dl/src/lib.rs @@ -30,6 +30,7 @@ mod herdr_editor; mod herdr_environment; mod launch; mod pane_shell; +mod prompt_editor; mod render; mod select; mod session; @@ -63,6 +64,10 @@ pub use devlaunch_core::shell; /// `dl` quotes what a tool or an environment said. pub use render::python_repr; +/// The one-row chooser `aid`'s model and effort pickers are drawn with. See +/// [`select::choose`]. +pub use select::{Choice, choose}; + /// `os.environ.get`, for the entry point that has environment variables of its /// own: `aid` reads `DEVLAUNCH_AID_AGENT` through here rather than through /// `std::env::var(..).ok()`, which reports a value that is not valid UTF-8 as @@ -407,6 +412,10 @@ pub fn workspace_id_of(spec: &str) -> Option { /// asks for the pick first and builds a line for a named workspace, which is the /// path every other aid launch takes. /// +/// A bare `aid`, and a line of flags with no workspace, take the same pick for the +/// same reason: the agent, model and effort pickers, the banner and the background +/// boot all come between the pick and the launch. +/// /// A pick that never came (Esc, an empty list, no terminal) is `Err(1)`, with the /// reason on stderr where there is one. pub fn pick_workspace() -> Result { @@ -499,86 +508,45 @@ fn early_name(spec: &str, cache_dir: &Path) -> Option { } } -/// Read one submission from a cooked-mode terminal: the line the user ends with -/// Enter, plus whatever input was already buffered at that moment — a multi-line -/// paste — joined as the newlines it arrived with. Empty on a bare Enter or an -/// immediate Ctrl-D. -/// -/// The read is byte-by-byte from descriptor 0 rather than through -/// `std::io::stdin()`, and that is load-bearing twice over. First, `Stdin`'s -/// buffer would swallow the rest of a paste where nothing can see it — a -/// zero-timeout `poll` on the descriptor answers for the kernel's queue, not for -/// bytes a `BufRead` already took. Second, whatever this function does not -/// consume stays in the terminal's queue for the *next* process to inherit — the -/// agent session `aid` goes on to attach — so pasted lines that were not drained -/// here would land inside the agent as keystrokes. +/// Send SIGINT to one process, as a terminal Ctrl-C would. /// -/// Cooked mode is also why this is safe to call and abandon: no raw mode is -/// entered, so a Ctrl-C mid-read leaves the terminal exactly as it found it. -pub fn read_terminal_submission() -> String { - let mut bytes: Vec = Vec::new(); - // The line itself: up to Enter, or EOF (Ctrl-D on an empty line reads 0). - let mut ended_with_newline = false; - loop { - match read_stdin_byte() { - None => break, - Some(b'\n') => { - ended_with_newline = true; - break; - } - Some(byte) => bytes.push(byte), - } - } - // The paste tail: the newline-terminated lines the terminal already holds. - // Only what is *already* queued — the zero timeout is what keeps a person - // who typed one line from being waited on for a second — and in cooked mode - // that is only *completed* lines: a final fragment a paste left unterminated - // is not yet readable, stays queued, and reaches the agent's session as - // typed-ahead input. The Enter that ended the first line was consumed above, - // so it is put back before the tail or the first two lines would be glued - // into one word. - if ended_with_newline && stdin_readable_now() { - bytes.push(b'\n'); - while stdin_readable_now() { - match read_stdin_byte() { - None => break, - Some(byte) => bytes.push(byte), - } - } +/// Exported for `aid`. A cancel in its picker is a key, not a signal, so its +/// background boot never sees it. SIGINT is what makes that boot run the handler +/// [`install_signal_handlers`] installs: kill its `devpod up` group and unlink its +/// staged token file. `Child::kill` sends SIGKILL, which does neither. +pub fn interrupt(pid: u32) { + let Ok(pid) = libc::pid_t::try_from(pid) else { + return; + }; + // SAFETY: `kill` on one pid this process spawned; the result is ignored + // because a child that already ended is the outcome being asked for. + unsafe { + libc::kill(pid, libc::SIGINT); } - String::from_utf8_lossy(&bytes).trim_end().to_owned() } -/// One byte from descriptor 0, or `None` on EOF or an unreadable stdin. -fn read_stdin_byte() -> Option { - let mut byte: u8 = 0; - loop { - // SAFETY: reading one byte into a stack buffer of that size. - let read = unsafe { libc::read(0, std::ptr::from_mut(&mut byte).cast(), 1) }; - match read { - 1 => return Some(byte), - 0 => return None, - // A signal that did not kill the process (SIGWINCH, a stopped and - // resumed job) interrupts the read without ending the input. - _ if std::io::Error::last_os_error().kind() == std::io::ErrorKind::Interrupted => {} - _ => return None, - } - } +pub use render::ClaudeProfileOffer; + +/// The Claude logins this host can launch with, for `aid`'s account picker: the +/// heading of the `dl --claude-profiles` table, and one row per login that can +/// launch. See [`render::claude_profile_offers`]. +pub fn claude_profile_offers() -> (String, Vec) { + render::claude_profile_offers(&devlaunch_core::flows::claude_profiles::from_process()) } -/// Whether descriptor 0 has bytes to read right now, without waiting for any. -fn stdin_readable_now() -> bool { - let mut asked = libc::pollfd { - fd: 0, - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: polling one descriptor with a zero timeout; the struct outlives the - // call. - let ready = unsafe { libc::poll(&mut asked, 1, 0) }; - ready > 0 && (asked.revents & libc::POLLIN) != 0 +/// The name that means "the login this host uses anyway", which passes no flag. +pub const DEFAULT_CLAUDE_PROFILE: &str = devlaunch_core::flows::claude_profiles::DEFAULT_PROFILE; + +/// devlaunch's cache directory, or `None` with no home directory to find it by. +/// +/// Exported for `aid`, which keeps its recent model and effort choices there, so +/// `XDG_CACHE_HOME` scopes them with everything else devlaunch stores. +pub fn cache_dir() -> Option { + devlaunch_core::domain::xdg::devlaunch_cache().ok() } +pub use prompt_editor::{Submission, read_prompt}; + /// Run one `dl` command line — the words after the program name — and say how it /// ended. /// diff --git a/rust/dl/src/prompt_editor.rs b/rust/dl/src/prompt_editor.rs new file mode 100644 index 00000000..a9558aae --- /dev/null +++ b/rust/dl/src/prompt_editor.rs @@ -0,0 +1,685 @@ +//! The prompt editor `aid` opens while a workspace boots: a raw-mode line editor +//! that takes a pasted prompt whole, line breaks and all. +//! +//! It replaced a cooked-mode read, and the reasons are the ones that read had no +//! answer for. The kernel's line discipline holds at most 4096 bytes of one line, +//! so a long paste was cut off. A paste is only whole if it arrives in one piece, +//! so the lines that came a moment late were left in the terminal's queue and +//! reached the agent as keystrokes. A last line with no line break was never read +//! at all. And no line break could be typed, because Enter always submitted. +//! +//! **Bracketed paste is what tells a paste from typing.** The editor asks the +//! terminal for it (`ESC [ ? 2004 h`), and the terminal then wraps every paste in +//! `ESC [ 200 ~` and `ESC [ 201 ~`. A line break between those is text. Outside +//! them, Enter submits, and Alt-Enter or Ctrl-J adds a line. A terminal without +//! bracketed paste still sends a paste faster than anybody types, so an Enter with +//! more input right behind it is read as a line break too. +//! +//! **One byte at a time from descriptor 0**, as the cooked read did, and for the +//! same reason: whatever this does not read stays in the terminal's queue for the +//! agent session `aid` attaches next. Reading stops at the Enter that submits, so +//! keys typed after it are the agent's. +//! +//! The editor only ever appends. There is no cursor to move, so the arrow keys +//! and the other escape sequences are read and dropped rather than printed as +//! `^[[D`. Backspace, Ctrl-U and Ctrl-W edit the end of the text. +//! +//! [`Editor`] and [`render`] are pure, so the tests below read them without a +//! terminal. [`read_prompt`] is the thin loop around them. + +use std::io::Write as _; +use std::time::Duration; + +use unicode_width::UnicodeWidthStr as _; + +/// What the person at the prompt did. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Submission { + /// Enter: this text, with trailing whitespace trimmed. Empty is the agent's + /// plain session. + Text(String), + /// Ctrl-C. In raw mode that is a key rather than a signal, so the caller gets + /// it as an answer and ends the run itself. + Cancelled, +} + +/// What one byte did to the editor. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Step { + Continue, + Submit, + Cancel, +} + +/// Where the byte parser is inside an escape sequence or a UTF-8 character. +#[derive(Clone, Debug, PartialEq, Eq)] +enum Parse { + Ground, + /// After `ESC`. + Escape, + /// After `ESC [`, with the parameter bytes so far. + Control(Vec), + /// After `ESC O`: one more byte, then back to ground. + Shift, + /// Inside a UTF-8 character: the bytes so far, and how many it has in all. + Character(Vec, usize), +} + +/// The text being edited, and where the parser is. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Editor { + text: String, + parse: Parse, + /// Between `ESC [ 200 ~` and `ESC [ 201 ~`. + pasting: bool, + /// The last byte was a carriage return, so a line feed next is the second half + /// of one line break rather than a second one. + after_return: bool, +} + +impl Default for Editor { + fn default() -> Self { + Editor { + text: String::new(), + parse: Parse::Ground, + pasting: false, + after_return: false, + } + } +} + +impl Editor { + pub(crate) fn text(&self) -> &str { + &self.text + } + + /// Take one byte. `more_follows` says whether more input was already waiting + /// behind it, which is how an unbracketed paste's Enter is told from a typed + /// one. It only matters for a carriage return. + pub(crate) fn feed(&mut self, byte: u8, more_follows: bool) -> Step { + let after_return = std::mem::replace(&mut self.after_return, false); + match std::mem::replace(&mut self.parse, Parse::Ground) { + Parse::Ground => self.ground(byte, more_follows, after_return), + Parse::Escape => { + match byte { + b'[' => self.parse = Parse::Control(Vec::new()), + b'O' => self.parse = Parse::Shift, + // Alt-Enter: a terminal sends ESC before the key. + b'\r' | b'\n' => self.text.push('\n'), + // Alt-Backspace: the word, as Ctrl-W. + 0x7f | 0x08 => self.delete_word(), + // ESC ESC, or Alt with any other key: nothing to do. + _ => {} + } + Step::Continue + } + Parse::Control(mut parameters) => { + if (0x40..=0x7e).contains(&byte) { + match (parameters.as_slice(), byte) { + (b"200", b'~') => self.pasting = true, + (b"201", b'~') => self.pasting = false, + // Arrows, Home, End, function keys: there is no cursor to + // move, so they are dropped. + _ => {} + } + } else if parameters.len() < 16 { + parameters.push(byte); + self.parse = Parse::Control(parameters); + } + Step::Continue + } + Parse::Shift => Step::Continue, + Parse::Character(mut bytes, length) => { + if byte & 0xc0 == 0x80 { + bytes.push(byte); + if bytes.len() == length { + self.text.push_str(&String::from_utf8_lossy(&bytes)); + } else { + self.parse = Parse::Character(bytes, length); + } + Step::Continue + } else { + // A broken character: say so once, and read this byte afresh. + self.text.push(char::REPLACEMENT_CHARACTER); + self.ground(byte, more_follows, after_return) + } + } + } + } + + /// Nothing followed an `ESC` within the gap a terminal leaves inside one + /// sequence, so it was the Esc key on its own. It is dropped, and the next + /// byte is read as a key of its own rather than as Alt with it. + pub(crate) fn escape_timed_out(&mut self) { + if self.parse == Parse::Escape { + self.parse = Parse::Ground; + } + } + + fn ground(&mut self, byte: u8, more_follows: bool, after_return: bool) -> Step { + match byte { + 0x1b => self.parse = Parse::Escape, + b'\r' => { + self.after_return = true; + if self.pasting || more_follows { + self.text.push('\n'); + } else { + return Step::Submit; + } + } + // The second half of a CR LF, or a line break on its own: in a paste, + // or Ctrl-J typed. + b'\n' => { + if !after_return { + self.text.push('\n'); + } + } + b'\t' => self.text.push('\t'), + _ if self.pasting && byte < 0x20 => {} + 0x03 => return Step::Cancel, + 0x04 if self.text.is_empty() => return Step::Submit, + 0x7f | 0x08 => { + self.text.pop(); + } + 0x15 => { + let start = self.text.rfind('\n').map_or(0, |at| at + 1); + self.text.truncate(start); + } + 0x17 => self.delete_word(), + 0x00..=0x1f => {} + 0x20..=0x7e => self.text.push(char::from(byte)), + 0xc0..=0xdf => self.parse = Parse::Character(vec![byte], 2), + 0xe0..=0xef => self.parse = Parse::Character(vec![byte], 3), + 0xf0..=0xf7 => self.parse = Parse::Character(vec![byte], 4), + _ => self.text.push(char::REPLACEMENT_CHARACTER), + } + Step::Continue + } + + /// The last word on the current line and the spaces after it, as a shell's + /// Ctrl-W does. Never crosses a line break. + fn delete_word(&mut self) { + let start = self.text.rfind('\n').map_or(0, |at| at + 1); + let line = &self.text[start..]; + let kept = line.trim_end_matches([' ', '\t']); + let kept = kept.trim_end_matches(|c: char| c != ' ' && c != '\t'); + let cut = start + kept.len(); + self.text.truncate(cut); + } +} + +/// The prefix of the first line, and of every line after it. +const PROMPT: &str = "> "; +const MORE: &str = " "; + +/// One drawing of the editor: the text to write, and how many rows above the +/// last one it takes, which is how far up the next drawing has to start. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Frame { + pub(crate) text: String, + pub(crate) rows_above: usize, +} + +/// Draw `text` for a terminal `columns` wide with `height` rows to spare. +/// +/// **Never taller than the space it has.** The next drawing starts by moving the +/// cursor up over this one, and a cursor cannot move above the top of the screen, +/// so a drawing that scrolled would be redrawn in the wrong place, line after +/// line. A prompt too tall for the screen shows its last lines under a line that +/// counts the ones not shown. The text is all still there, and all of it is sent. +/// +/// A tab is drawn as four spaces and sent as a tab. +pub(crate) fn render(text: &str, columns: usize, height: usize) -> Frame { + let columns = columns.max(1); + let budget = height.max(1); + let lines: Vec = text + .split('\n') + .enumerate() + .map(|(index, line)| { + let prefix = if index == 0 { PROMPT } else { MORE }; + format!("{prefix}{}", line.replace('\t', " ")) + }) + .collect(); + let rows_of = |line: &str| line.width().div_ceil(columns).max(1); + + let mut shown: Vec = Vec::new(); + let mut used = 0; + let mut hidden = lines.len(); + for line in lines.iter().rev() { + // One row is held back for the count, unless this is the first line and + // nothing would be hidden. + let room = if hidden == 1 { budget } else { budget - 1 }; + let rows = rows_of(line); + if used + rows <= room { + shown.push(line.clone()); + used += rows; + hidden -= 1; + continue; + } + if shown.is_empty() { + // One line taller than the screen: its end, which is where typing is. + let keep = (room.max(1) * columns).saturating_sub(2).max(1); + shown.push(format!("{MORE}{}", tail(line, keep))); + used += rows_of(&shown[0]); + hidden -= 1; + } + break; + } + shown.reverse(); + if hidden > 0 { + shown.insert(0, count_line(hidden, columns)); + used += 1; + } + Frame { + text: shown.join("\r\n"), + rows_above: used - 1, + } +} + +/// The line that counts `hidden` lines, in the longest wording that fits one row +/// of `columns`: it is held to one row, because `render` reserves one row for it. +fn count_line(hidden: usize, columns: usize) -> String { + let word = if hidden == 1 { "line" } else { "lines" }; + let wordings = [ + format!("{MORE}({hidden} earlier {word} not shown)"), + format!("{MORE}({hidden} not shown)"), + format!("{MORE}(+{hidden})"), + format!("(+{hidden})"), + ]; + match wordings.iter().find(|line| line.width() <= columns) { + Some(line) => line.clone(), + None => wordings[3].chars().take(columns).collect(), + } +} + +/// The end of `line` that fits in `width` columns. +fn tail(line: &str, width: usize) -> String { + let mut kept: Vec = Vec::new(); + let mut used = 0; + for c in line.chars().rev() { + let w = unicode_width::UnicodeWidthChar::width(c).unwrap_or(0); + if used + w > width { + break; + } + used += w; + kept.push(c); + } + kept.into_iter().rev().collect() +} + +/// The terminal's settings as they were, put back when this is dropped. +/// +/// A guard rather than a call at the end, so an early return or a panic still +/// leaves the terminal the way the person's shell expects it. `TCSANOW` rather +/// than `TCSAFLUSH` on the way out: flushing would throw away keys typed after +/// the Enter, which belong to the agent. +struct Raw { + before: libc::termios, +} + +impl Raw { + /// Raw input on descriptor 0, or `None` if its settings cannot be read. + /// + /// Output keeps its processing (`OPOST`), so anything else that writes to the + /// terminal meanwhile still gets its carriage returns. `ISIG` is off, so + /// Ctrl-C arrives as a byte and the editor restores the terminal before the + /// run ends. + fn enter() -> Option { + // SAFETY: `tcgetattr` fills one `termios` this scope owns. + let mut before: libc::termios = unsafe { std::mem::zeroed() }; + if unsafe { libc::tcgetattr(0, &mut before) } != 0 { + return None; + } + let mut raw = before; + raw.c_lflag &= !(libc::ICANON | libc::ECHO | libc::ISIG | libc::IEXTEN); + raw.c_iflag &= !(libc::ICRNL | libc::INLCR | libc::IXON); + raw.c_cc[libc::VMIN] = 1; + raw.c_cc[libc::VTIME] = 0; + // SAFETY: as above, setting what was read and changed. + if unsafe { libc::tcsetattr(0, libc::TCSANOW, &raw) } != 0 { + return None; + } + Some(Raw { before }) + } +} + +impl Drop for Raw { + fn drop(&mut self) { + write_out(BRACKETED_PASTE_OFF); + // SAFETY: restoring the settings `enter` read. + unsafe { + libc::tcsetattr(0, libc::TCSANOW, &self.before); + } + } +} + +const BRACKETED_PASTE_ON: &str = "\x1b[?2004h"; +const BRACKETED_PASTE_OFF: &str = "\x1b[?2004l"; + +/// How long an Enter waits to see whether more input follows it. Far under the +/// gap between two keys a person types, far over the gap inside one paste. +const PASTE_GAP: Duration = Duration::from_millis(15); + +/// Read one prompt from the terminal. See the module's documentation. +/// +/// Falls back to the cooked read it replaced when descriptor 0's settings cannot +/// be changed, which is nothing a person at a terminal meets. +pub fn read_prompt() -> Submission { + let Some(raw) = Raw::enter() else { + return Submission::Text(read_cooked()); + }; + write_out(BRACKETED_PASTE_ON); + let mut editor = Editor::default(); + let mut rows_above = draw(&editor, 0); + let ended = loop { + let Some(byte) = read_stdin_byte() else { + break Step::Submit; + }; + let more_follows = byte == b'\r' && stdin_readable_within(PASTE_GAP); + let step = editor.feed(byte, more_follows); + if byte == 0x1b && !stdin_readable_within(PASTE_GAP) { + editor.escape_timed_out(); + } + match step { + // Drawn once the input so far is read, so a paste of a thousand lines + // is one drawing and not a thousand. + Step::Continue if !stdin_readable_within(Duration::ZERO) => { + rows_above = draw(&editor, rows_above); + } + Step::Continue => {} + ended => break ended, + } + }; + draw(&editor, rows_above); + write_out("\r\n"); + drop(raw); + match ended { + Step::Cancel => Submission::Cancelled, + _ => Submission::Text(editor.text().trim_end().to_owned()), + } +} + +/// Draw over the last drawing, which ended `rows_above` rows below its top, and +/// say how far this one reaches. +fn draw(editor: &Editor, rows_above: usize) -> usize { + let (height, columns) = terminal_size().unwrap_or((24, 80)); + // Two rows kept free: one for the banner line above, one for the terminal's + // own last row. + let frame = render( + editor.text(), + usize::from(columns), + usize::from(height).saturating_sub(2), + ); + let mut out = String::from("\r"); + if rows_above > 0 { + out.push_str(&format!("\x1b[{rows_above}A")); + } + out.push_str("\x1b[J"); + out.push_str(&frame.text); + write_out(&out); + frame.rows_above +} + +fn write_out(text: &str) { + let mut out = std::io::stdout().lock(); + let _ = out.write_all(text.as_bytes()); + let _ = out.flush(); +} + +/// Rows and columns of the terminal on descriptor 1, which is what is drawn on. +fn terminal_size() -> Option<(u16, u16)> { + let mut size = libc::winsize { + ws_row: 0, + ws_col: 0, + ws_xpixel: 0, + ws_ypixel: 0, + }; + // SAFETY: TIOCGWINSZ fills one `winsize` this scope owns. + let read = unsafe { libc::ioctl(1, libc::TIOCGWINSZ, &mut size) }; + (read == 0 && size.ws_row > 0 && size.ws_col > 0).then_some((size.ws_row, size.ws_col)) +} + +/// The cooked-mode read this module replaced, kept as its fallback: the line up +/// to Enter, plus whatever complete lines were already queued behind it. +fn read_cooked() -> String { + let mut bytes: Vec = Vec::new(); + let mut ended_with_newline = false; + loop { + match read_stdin_byte() { + None => break, + Some(b'\n') => { + ended_with_newline = true; + break; + } + Some(byte) => bytes.push(byte), + } + } + if ended_with_newline && stdin_readable_within(Duration::ZERO) { + bytes.push(b'\n'); + while stdin_readable_within(Duration::ZERO) { + match read_stdin_byte() { + None => break, + Some(byte) => bytes.push(byte), + } + } + } + String::from_utf8_lossy(&bytes).trim_end().to_owned() +} + +/// One byte from descriptor 0, or `None` on EOF or an unreadable stdin. +fn read_stdin_byte() -> Option { + let mut byte: u8 = 0; + loop { + // SAFETY: reading one byte into a stack buffer of that size. + let read = unsafe { libc::read(0, std::ptr::from_mut(&mut byte).cast(), 1) }; + match read { + 1 => return Some(byte), + 0 => return None, + // A signal that did not kill the process (SIGWINCH, a stopped and + // resumed job) interrupts the read without ending the input. + _ if std::io::Error::last_os_error().kind() == std::io::ErrorKind::Interrupted => {} + _ => return None, + } + } +} + +/// Whether descriptor 0 has bytes to read within `wait`. +fn stdin_readable_within(wait: Duration) -> bool { + let mut asked = libc::pollfd { + fd: 0, + events: libc::POLLIN, + revents: 0, + }; + let timeout = libc::c_int::try_from(wait.as_millis()).unwrap_or(libc::c_int::MAX); + // SAFETY: polling one descriptor; the struct outlives the call. + let ready = unsafe { libc::poll(&mut asked, 1, timeout) }; + ready > 0 && (asked.revents & libc::POLLIN) != 0 +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Feed `bytes`, each with nothing following it, and say how it ended. + fn typed(bytes: &[u8]) -> (Editor, Step) { + let mut editor = Editor::default(); + let mut last = Step::Continue; + for byte in bytes { + last = editor.feed(*byte, false); + if last != Step::Continue { + break; + } + } + (editor, last) + } + + /// Feed `bytes` as one burst: every byte but the last has more behind it. + fn burst(bytes: &[u8]) -> (Editor, Step) { + let mut editor = Editor::default(); + let mut last = Step::Continue; + for (at, byte) in bytes.iter().enumerate() { + last = editor.feed(*byte, at + 1 < bytes.len()); + if last != Step::Continue { + break; + } + } + (editor, last) + } + + #[test] + fn enter_submits_what_was_typed() { + let (editor, ended) = typed(b"fix the bug\r"); + assert_eq!(ended, Step::Submit); + assert_eq!(editor.text(), "fix the bug"); + } + + #[test] + fn a_bracketed_paste_keeps_its_line_breaks_and_does_not_submit() { + let (editor, ended) = typed(b"\x1b[200~line one\r\nline two\r\n\x1b[201~"); + assert_eq!(ended, Step::Continue); + assert_eq!(editor.text(), "line one\nline two\n"); + let (editor, ended) = typed(b"\x1b[200~a\rb\nc\x1b[201~\r"); + assert_eq!(ended, Step::Submit); + assert_eq!(editor.text(), "a\nb\nc"); + } + + #[test] + fn a_paste_longer_than_the_kernels_line_limit_arrives_whole() { + let long = "x".repeat(10_000); + let mut bytes = b"\x1b[200~".to_vec(); + bytes.extend_from_slice(long.as_bytes()); + bytes.extend_from_slice(b"\x1b[201~\r"); + let (editor, ended) = typed(&bytes); + assert_eq!(ended, Step::Submit); + assert_eq!(editor.text(), long); + } + + #[test] + fn an_unbracketed_paste_is_told_apart_by_the_input_behind_each_enter() { + // A terminal without bracketed paste: the Enters inside the paste have more + // input right behind them, and the last one has none. + let (editor, ended) = burst(b"one\rtwo\r"); + assert_eq!(ended, Step::Submit); + assert_eq!(editor.text(), "one\ntwo"); + } + + #[test] + fn alt_enter_and_ctrl_j_add_a_line_when_typing() { + let (editor, ended) = typed(b"first\x1b\rsecond\nthird\r"); + assert_eq!(ended, Step::Submit); + assert_eq!(editor.text(), "first\nsecond\nthird"); + } + + #[test] + fn a_lone_esc_is_dropped_and_the_next_key_is_read_afresh() { + let mut editor = Editor::default(); + assert_eq!(editor.feed(0x1b, false), Step::Continue); + editor.escape_timed_out(); + assert_eq!(editor.feed(b'f', false), Step::Continue); + assert_eq!(editor.feed(b'\r', false), Step::Submit); + assert_eq!(editor.text(), "f"); + + let mut editor = Editor::default(); + editor.feed(b'x', false); + editor.feed(0x1b, false); + editor.escape_timed_out(); + assert_eq!(editor.feed(b'\r', false), Step::Submit); + assert_eq!(editor.text(), "x"); + + // The timeout only ends a bare ESC: Alt-Enter in one burst still adds a + // line. + let (editor, ended) = burst(b"a\x1b\rb"); + assert_eq!(ended, Step::Continue); + assert_eq!(editor.text(), "a\nb"); + } + + #[test] + fn the_editing_keys_edit_the_end_of_the_text() { + assert_eq!(typed(b"abc\x7f").0.text(), "ab"); + assert_eq!(typed(b"one\ntwo three\x15").0.text(), "one\n"); + assert_eq!(typed(b"one two \x17").0.text(), "one "); + assert_eq!(typed(b"one\ntwo\x17\x17").0.text(), "one\n"); + // Backspace at the start of a line joins it to the line above. + assert_eq!(typed(b"one\n\x7f").0.text(), "one"); + } + + #[test] + fn arrow_and_other_keys_are_dropped_rather_than_printed() { + let (editor, _) = typed(b"a\x1b[D\x1b[1;5C\x1bOA\x1b[3~b"); + assert_eq!(editor.text(), "ab"); + } + + #[test] + fn ctrl_c_cancels_and_ctrl_d_on_nothing_submits_nothing() { + assert_eq!(typed(b"half a prompt\x03").1, Step::Cancel); + let (editor, ended) = typed(b"\x04"); + assert_eq!((editor.text(), ended), ("", Step::Submit)); + // With text there, Ctrl-D is not an answer. + assert_eq!(typed(b"text\x04").1, Step::Continue); + } + + #[test] + fn a_ctrl_c_inside_a_paste_is_text_not_a_cancel() { + let (editor, ended) = typed(b"\x1b[200~a\x03b\x1b[201~"); + assert_eq!(ended, Step::Continue); + assert_eq!(editor.text(), "ab"); + } + + #[test] + fn utf8_is_read_whole_and_a_broken_character_is_marked() { + assert_eq!(typed("héllo ✓ 日本".as_bytes()).0.text(), "héllo ✓ 日本"); + assert_eq!(typed(b"a\xc3b").0.text(), "a\u{fffd}b"); + } + + #[test] + fn a_drawing_counts_its_rows_so_the_next_can_start_over_it() { + let frame = render("one\ntwo", 80, 20); + assert_eq!(frame.text, "> one\r\n two"); + assert_eq!(frame.rows_above, 1); + // A line wider than the terminal takes the rows it wraps onto. + assert_eq!(render(&"x".repeat(100), 80, 20).rows_above, 1); + // Wide characters take two columns each. + assert_eq!(render(&"日".repeat(40), 80, 20).rows_above, 1); + assert_eq!( + render("", 80, 20), + Frame { + text: "> ".to_owned(), + rows_above: 0 + } + ); + } + + #[test] + fn a_prompt_taller_than_the_screen_shows_its_end_under_a_count() { + let text: Vec = (1..=30).map(|n| format!("line {n}")).collect(); + let frame = render(&text.join("\n"), 80, 5); + assert_eq!( + frame.text, + " (26 earlier lines not shown)\r\n line 27\r\n line 28\r\n line 29\r\n line 30" + ); + assert_eq!(frame.rows_above, 4); + } + + #[test] + fn the_count_line_is_counted_at_the_rows_it_really_takes() { + let text: Vec = (1..=30).map(|n| format!("line {n}")).collect(); + let text = text.join("\n"); + for (columns, height) in [(20, 5), (5, 3)] { + let frame = render(&text, columns, height); + let rows: usize = frame + .text + .split("\r\n") + .map(|line| line.width().div_ceil(columns).max(1)) + .sum(); + assert_eq!(rows, frame.rows_above + 1, "{columns}x{height}: {frame:?}"); + assert!(rows <= height, "{columns}x{height}: {frame:?}"); + } + } + + #[test] + fn one_line_taller_than_the_screen_shows_its_end() { + let frame = render(&"x".repeat(1000), 10, 3); + assert!(frame.rows_above < 3, "{frame:?}"); + assert!(frame.text.ends_with("xxxx"), "{frame:?}"); + } +} diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index c86a485e..83db1efe 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -191,6 +191,41 @@ pub(crate) fn claude_profile_lines(rows: &[claude_profiles::ProfileSummary]) -> lines } +/// One Claude login a picker can offer: the name `--claude-profile` takes, and its +/// row of the `dl --claude-profiles` table. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ClaudeProfileOffer { + pub name: String, + pub label: String, +} + +/// The logins a launch can use, as picker rows, with the table heading above them. +/// +/// The rows `dl --claude-profiles` draws, in its order and its columns, less the +/// named profiles with no credential: a launch naming one of those refuses, so a +/// picker that offered one would offer a refusal. The unnamed `default` login stays +/// whatever its state, because choosing it passes no flag at all. The shared-account +/// notes are left off, since a picker has no place for a footnote. +pub fn claude_profile_offers( + rows: &[claude_profiles::ProfileSummary], +) -> (String, Vec) { + let mut lines = claude_profile_lines(rows).into_iter(); + let heading = lines.next().unwrap_or_default(); + let offers = rows + .iter() + .zip(lines) + .filter(|(row, _)| { + row.name == claude_profiles::DEFAULT_PROFILE + || row.state == claude_profiles::ProfileState::Authed + }) + .map(|(row, label)| ClaudeProfileOffer { + name: row.name.clone(), + label, + }) + .collect(); + (heading, offers) +} + /// The sentence for a listing with no rows at all. /// /// A real state rather than a defensive branch: no home directory and no @@ -4047,6 +4082,49 @@ mod tests { } } + #[test] + fn a_picker_offers_the_logins_that_can_launch_in_the_listings_own_rows() { + let rows = [ + profile( + "default", + claude_profiles::ProfileState::NoCredential, + None, + &[], + ), + profile( + "fresh", + claude_profiles::ProfileState::NoCredential, + None, + &[], + ), + profile( + "work", + claude_profiles::ProfileState::Authed, + Some(account(Some("me@acme.example"), None, None)), + &["default"], + ), + ]; + let lines = claude_profile_lines(&rows); + let (heading, offers) = claude_profile_offers(&rows); + + assert_eq!(heading, lines[0]); + // `fresh` would refuse the launch, so it is not offered. `default` passes no + // flag, so it is offered whatever its state. + assert_eq!( + offers, + [ + ClaudeProfileOffer { + name: "default".to_owned(), + label: lines[1].clone(), + }, + ClaudeProfileOffer { + name: "work".to_owned(), + label: lines[3].clone(), + }, + ] + ); + } + #[test] fn the_profile_listing_has_a_header_and_a_row_each() { let lines = claude_profile_lines(&[ diff --git a/rust/dl/src/select.rs b/rust/dl/src/select.rs index 36bba513..37b799ad 100644 --- a/rust/dl/src/select.rs +++ b/rust/dl/src/select.rs @@ -985,6 +985,160 @@ impl SkimItem for Row { } } +/// What [`choose`] settled. +/// +/// Four arms, because a caller does a different thing for each: a row maps back +/// to the value the caller offered, typed text is a value nobody offered, a +/// cancel ends the caller's run, and no terminal means there was nobody to ask. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Choice { + /// The row at this position in the slice [`choose`] was given. + Row(usize), + /// Enter on a query that matched no row: the query, trimmed. Never empty. + Typed(String), + /// Esc or Ctrl-C. In skim these are keys, not signals, so the caller gets + /// the cancel as an answer and has to act on it itself. + Cancelled, + /// No terminal to draw on. skim would abort the process here, so it is not + /// entered. + NoTerminal, +} + +/// Offer `rows` under `header` and wait for one, or for text that is none of them. +/// +/// Exported for `aid`, whose model and effort pickers are this with rows `aid` +/// builds. The same skim and the same layout as the workspace picker, with two +/// differences. Matching is **exact** (substring), not fuzzy: a query is often a +/// name that is not in the list yet, and fuzzy matching finds a listed row for +/// almost any query, so Enter would take that row instead of what was typed. And +/// a query that matches no row is an answer, [`Choice::Typed`]. +/// +/// Exact matching still leaves a query that is a substring of a row, such as +/// `gpt-5.5` beside a listed `gpt-5.5-codex`, and Enter takes that row. Alt-Enter +/// ([`AS_TYPED_KEY`]) takes the query as typed whatever it matches, and the +/// caller's header is what says so. +pub fn choose(header: &str, rows: &[String]) -> Choice { + if !a_terminal_exists() || !a_drawable_size(terminal_size()) { + return Choice::NoTerminal; + } + // The workspace picker's `TERM` check, for the same panic: a `TERM` skim cannot + // draw with is swapped for [`FALLBACK_TERM`] while the chooser is up, and with + // no usable entry at all the chooser is skipped. The caller then goes on + // without it, as it does with no terminal. + let swapped = match plan( + TermInfo::from_env(), + TermInfo::from_name(FALLBACK_TERM), + Was::read(), + ) { + Plan::Keep => None, + Plan::Swap { was, reason } => Some(DrawableTerm::swap(was, reason)), + Plan::Refuse { .. } => return Choice::NoTerminal, + }; + let (tx, rx): (SkimItemSender, SkimItemReceiver) = unbounded(); + for (index, label) in rows.iter().enumerate() { + let row: Arc = Arc::new(Row { + label: label.clone(), + index, + }); + if tx.send(row).is_err() { + break; + } + } + drop(tx); + let output = Skim::run_with(&choose_options(header), Some(rx)); + drop(swapped); + let Some(output) = output else { + return Choice::Cancelled; + }; + let picked = output.selected_items.first().map(|item| item.get_index()); + let accepted = match &output.final_event { + Event::EvActAccept(Some(key)) if key == AS_TYPED_KEY => Accepted::AsTyped, + _ => Accepted::Enter, + }; + choice_of(output.is_abort, &output.query, picked, rows.len(), accepted) +} + +/// The key that takes a [`choose`] query as typed, in skim's key names. +const AS_TYPED_KEY: &str = "alt-enter"; + +/// Which key accepted the chooser. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Accepted { + /// Enter: the row under the cursor, or the query where no row matched. + Enter, + /// [`AS_TYPED_KEY`]: the query, whatever row is under the cursor. + AsTyped, +} + +/// The terminal's size as rows and columns, or `None` if it cannot be read. +fn terminal_size() -> Option<(u16, u16)> { + use std::os::fd::AsRawFd as _; + let tty = std::fs::OpenOptions::new() + .read(true) + .write(true) + .open("/dev/tty") + .ok()?; + let mut size = libc::winsize { + ws_row: 0, + ws_col: 0, + ws_xpixel: 0, + ws_ypixel: 0, + }; + // SAFETY: TIOCGWINSZ fills one `winsize` this scope owns, on a descriptor it + // owns; the file is closed at the end of the scope either way. + let read = unsafe { libc::ioctl(tty.as_raw_fd(), libc::TIOCGWINSZ, &mut size) }; + (read == 0).then_some((size.ws_row, size.ws_col)) +} + +/// Whether skim can draw in a terminal of this size. +/// +/// A pty a program opened without setting a size is 0 by 0, and skim subtracts +/// from the size with no check and panics (`attempt to subtract with overflow`, +/// skim-tuikit 0.6.6). A size that cannot be read is let through: skim reads it +/// its own way, and the panic is only known for the zero size. +fn a_drawable_size(size: Option<(u16, u16)>) -> bool { + size.is_none_or(|(rows, columns)| rows > 0 && columns > 0) +} + +/// The options [`choose`] draws with. A function so a test can read them. +fn choose_options(header: &str) -> SkimOptions { + SkimOptions { + exact: true, + no_sort: true, + // See `skim_options`: the layout by name, because `reverse: true` is only + // expanded by a `build` that `run_with` never calls. + layout: String::from("reverse"), + header: Some(header.to_owned()), + expect: vec![AS_TYPED_KEY.to_owned()], + ..Default::default() + } +} + +/// What skim's answer means. Split from [`choose`] because it is the part a test +/// can reach without a terminal. +/// +/// An index outside `rows` is read as no row, and a query of only spaces as no +/// query, so neither can turn into a value nobody chose. +fn choice_of( + aborted: bool, + query: &str, + picked: Option, + rows: usize, + accepted: Accepted, +) -> Choice { + if aborted { + return Choice::Cancelled; + } + let picked = picked.filter(|_| accepted == Accepted::Enter); + if let Some(index) = picked.filter(|index| *index < rows) { + return Choice::Row(index); + } + match query.trim() { + "" => Choice::Cancelled, + typed => Choice::Typed(typed.to_owned()), + } +} + #[cfg(test)] mod tests { //! The Python `test_workspace_source::TestTheFuzzyPickerOffersEverySource` @@ -2142,6 +2296,74 @@ mod tests { ); } + #[test] + fn a_choice_is_the_row_taken_or_else_the_text_typed() { + assert_eq!( + choice_of(false, "op", Some(1), 3, Accepted::Enter), + Choice::Row(1) + ); + assert_eq!( + choice_of(false, " claude-opus-5-5 ", None, 3, Accepted::Enter), + Choice::Typed("claude-opus-5-5".to_owned()) + ); + // Esc wins over whatever was under the cursor or in the query. + assert_eq!( + choice_of(true, "op", Some(1), 3, Accepted::Enter), + Choice::Cancelled + ); + // Enter on nothing at all is no answer, not an empty value. + assert_eq!( + choice_of(false, " ", None, 3, Accepted::Enter), + Choice::Cancelled + ); + // A row skim invented is not one of ours. + assert_eq!( + choice_of(false, "x", Some(7), 3, Accepted::Enter), + Choice::Typed("x".to_owned()) + ); + } + + #[test] + fn alt_enter_takes_the_query_even_where_it_matches_a_row() { + // `gpt-5.5` is a substring of the row `gpt-5.5-codex`, so the row is under + // the cursor. Enter takes the row; Alt-Enter takes what was typed. + assert_eq!( + choice_of(false, "gpt-5.5", Some(0), 1, Accepted::Enter), + Choice::Row(0) + ); + assert_eq!( + choice_of(false, " gpt-5.5 ", Some(0), 1, Accepted::AsTyped), + Choice::Typed("gpt-5.5".to_owned()) + ); + // Alt-Enter on no text is no answer, and Esc still wins. + assert_eq!( + choice_of(false, " ", Some(0), 1, Accepted::AsTyped), + Choice::Cancelled + ); + assert_eq!( + choice_of(true, "gpt-5.5", Some(0), 1, Accepted::AsTyped), + Choice::Cancelled + ); + assert_eq!(choose_options("h").expect, ["alt-enter"]); + } + + #[test] + fn the_chooser_matches_exactly_so_a_new_name_is_not_taken_for_an_old_one() { + let options = choose_options("Model for claude:"); + assert!(options.exact); + assert!(!options.multi); + assert_eq!(options.layout, "reverse"); + assert_eq!(options.header.as_deref(), Some("Model for claude:")); + } + + #[test] + fn a_terminal_with_no_size_is_no_terminal_to_draw_a_chooser_on() { + assert!(a_drawable_size(Some((24, 80)))); + assert!(a_drawable_size(None)); + assert!(!a_drawable_size(Some((0, 0)))); + assert!(!a_drawable_size(Some((24, 0)))); + } + /// A terminfo entry holding exactly these string capabilities. fn entry(capabilities: &[&'static str]) -> TermInfo { TermInfo { diff --git a/rust/dl/tests/completion_tables.rs b/rust/dl/tests/completion_tables.rs index e51811b5..58bab09c 100644 --- a/rust/dl/tests/completion_tables.rs +++ b/rust/dl/tests/completion_tables.rs @@ -591,6 +591,17 @@ const AID_FLAGS_BESIDE_THE_AGENTS: [(&str, &str); 3] = [ ("--version", "aid's own, likewise"), ]; +#[test] +fn the_aid_flags_whose_value_is_stepped_over_are_aids_value_options() { + // A second copy of `AGENT_VALUE_OPTIONS`, so it is diffed here: the scan steps + // over these flags' values, and a flag missed here would make its value read + // as the workspace spec. + assert_eq!( + assigned(&completion_script(), "aid_value_opts="), + aid_flag_list(&aid_rewrite(), "AGENT_VALUE_OPTIONS") + ); +} + #[test] fn aid_offers_one_flag_per_agent_it_can_start() { let script = completion_script(); @@ -601,6 +612,7 @@ fn aid_offers_one_flag_per_agent_it_can_start() { .map(|agent| format!("--{agent}")) .collect(); expected.extend(aid_flag_list(&rewrite, "DL_VALUE_OPTIONS")); + expected.extend(aid_flag_list(&rewrite, "AGENT_VALUE_OPTIONS")); expected.extend( AID_FLAGS_BESIDE_THE_AGENTS .iter() @@ -636,6 +648,7 @@ fn the_aid_flags_a_spec_may_follow_are_the_ones_parse_aid_args_reads_past() { for table in [ "REMOTE_CONTROL_FLAGS", "NO_REMOTE_CONTROL_FLAGS", + "AGENT_VALUE_OPTIONS", "DL_VALUE_OPTIONS", ] { expected.extend(aid_flag_list(&rewrite, table)); diff --git a/test/e2e/test_interactive_session.py b/test/e2e/test_interactive_session.py index 5b9c8ddd..b34cc7c1 100644 --- a/test/e2e/test_interactive_session.py +++ b/test/e2e/test_interactive_session.py @@ -33,6 +33,7 @@ import re import shlex import subprocess +import time from dataclasses import dataclass from pathlib import Path from typing import Dict, Iterator @@ -391,11 +392,28 @@ def test_aid_leaves_an_interactive_agent_running(self, workspace): session = workspace.aid() with session: - # On a terminal a promptless `aid` asks for the prompt while the - # workspace boots; an empty Enter is the plain session this test has - # always been about. + # On a terminal a promptless `aid` asks for the agent (one row per + # Claude login, then the other agents), the model and the effort, then + # the prompt, while the workspace boots. The agent picker is filtered + # to claude, since a recent choice could have put another agent first; + # Enter takes the first row of the rest. `\r`, not a newline: skim + # holds the terminal in raw mode, where a newline is Ctrl-J and moves + # the cursor down a row. With TERM=dumb there are no pickers. + session.expect(r"Agent for this launch|press Enter") + if "Agent for this launch" in session.text: + time.sleep(0.2) + session.send("claude", newline=False) + time.sleep(0.5) + session.send("\r", newline=False) + for picker in ("Model for claude", "Effort for claude"): + session.expect(picker) + time.sleep(0.2) + session.send("\r", newline=False) + # An empty Enter is the plain session this test has always been about. session.expect(r"press Enter") - session.send("") + # The prompt editor is raw mode too: Enter is `\r`, and a newline + # would add a line to the prompt instead of submitting it. + session.send("\r", newline=False) # Claude Code prints its banner once the TUI is up; without a # terminal it exits before ever getting there. session.expect(r"Claude Code|Welcome to Claude") diff --git a/test/test_bash_completion.py b/test/test_bash_completion.py index 3efd1e5f..e58bc853 100644 --- a/test/test_bash_completion.py +++ b/test/test_bash_completion.py @@ -361,6 +361,24 @@ def test_a_value_option_and_its_value_do_not_move_the_workspace_spec(self): assert self.run_completion("dl --devcontainer robot my-") == bare assert self.run_completion("dl --claude-profile work my-") == bare + def test_an_aid_model_or_effort_and_its_value_do_not_move_the_spec(self): + """`aid --model opus ` completes its spec as `aid ` does. + + Both flags take a value, so the scan has to step over two words. A + script that stepped over one would call `opus` the spec and complete + nothing after it. + """ + bare = self.run_completion("aid my-") + assert bare + assert self.run_completion("aid --model opus my-") == bare + assert self.run_completion("aid --effort high my-") == bare + assert self.run_completion("aid --model opus --effort high my-") == bare + + def test_nothing_completes_an_aid_model_or_effort_value(self): + """The values are each agent's, and aid holds no list of them.""" + assert self.run_completion("aid --model ") == [] + assert self.run_completion("aid --effort ") == [] + def test_a_leading_modifier_leaves_the_verb_where_it_was(self): """`dl --rm `: a flag ahead of the spec shifts nothing.