diff --git a/crates/maestro-cli/src/cli.rs b/crates/maestro-cli/src/cli.rs index 90da608..882f5e4 100644 --- a/crates/maestro-cli/src/cli.rs +++ b/crates/maestro-cli/src/cli.rs @@ -92,6 +92,14 @@ pub struct SpawnArgs { /// there. #[arg(long)] pub existing_branch: bool, + /// Run a Claude Code or Codex session in a plain terminal instead of the + /// structured (ACP) view it otherwise gets by default. + #[arg(long, conflicts_with = "structured")] + pub terminal: bool, + /// Ask for the structured (ACP) view explicitly. Only Claude Code and + /// Codex commands have an adapter; the daemon refuses anything else. + #[arg(long)] + pub structured: bool, /// After spawning, attach immediately. #[arg(long, short = 'a')] pub attach: bool, diff --git a/crates/maestro-cli/src/sessions.rs b/crates/maestro-cli/src/sessions.rs index f98d9eb..5929d8a 100644 --- a/crates/maestro-cli/src/sessions.rs +++ b/crates/maestro-cli/src/sessions.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::io::Write; -use maestro_protocol::display::abbreviate_dir; +use maestro_protocol::display::{abbreviate_dir, supports_structured_view}; use maestro_protocol::{ ActivityState, ClientError, CreateSessionRequest, DaemonClient, DaemonInfo, SendInputRequest, Session, SessionStatus, SessionView, WorktreeSpec, @@ -51,7 +51,7 @@ pub async fn run_spawn(client: &DaemonClient, args: SpawnArgs) -> Result Result SessionView { + if args.terminal { + return SessionView::Terminal; + } + if args.structured || supports_structured_view(&args.command) { + SessionView::Structured + } else { + SessionView::Terminal + } +} + /// `maestro kill`. A forced kill (#81) retains the guarded worktree instead /// of removing it; the outcome names what was kept so dispatch can say where /// the work went. @@ -239,6 +255,62 @@ mod tests { use super::DaemonInfo; + use clap::Parser; + + use crate::cli::{Cli, Command}; + + fn spawn_args(extra: &[&str]) -> crate::cli::SpawnArgs { + let mut argv = vec!["maestro", "spawn", "--cwd", "/tmp", "--command"]; + argv.extend_from_slice(extra); + let cli = Cli::try_parse_from(argv).expect("parses"); + let Command::Spawn(args) = cli.command else { + panic!("expected spawn") + }; + args + } + + /// The CLI follows the two interactive clients: a command with an ACP + /// adapter spawns structured unless `--terminal` says otherwise, and + /// `--structured` still forces the request for a command this side does + /// not recognise. + #[test] + fn spawn_defaults_to_structured_for_supported_agents() { + use super::spawn_view; + assert_eq!( + spawn_view(&spawn_args(&["claude"])), + SessionView::Structured + ); + assert_eq!( + spawn_view(&spawn_args(&["npx codex exec"])), + SessionView::Structured + ); + assert_eq!(spawn_view(&spawn_args(&["bash"])), SessionView::Terminal); + assert_eq!( + spawn_view(&spawn_args(&["claude", "--terminal"])), + SessionView::Terminal + ); + assert_eq!( + spawn_view(&spawn_args(&["opencode", "--structured"])), + SessionView::Structured + ); + } + + #[test] + fn spawn_refuses_both_view_flags_at_once() { + let err = Cli::try_parse_from([ + "maestro", + "spawn", + "--cwd", + "/tmp", + "--command", + "claude", + "--structured", + "--terminal", + ]) + .unwrap_err(); + assert!(err.to_string().contains("--terminal"), "{err}"); + } + fn session(cwd: &str, command: &str, id: u128) -> Session { Session { view: Default::default(), diff --git a/crates/maestro-protocol/src/display.rs b/crates/maestro-protocol/src/display.rs index 8bf1ff3..c71ada8 100644 --- a/crates/maestro-protocol/src/display.rs +++ b/crates/maestro-protocol/src/display.rs @@ -226,6 +226,15 @@ pub fn command_tool(command: &str) -> Option<&'static str> { .map(|(_, tool)| *tool) } +/// Whether a command line resolves to an agent Maestro can run as a +/// structured (ACP) session — Claude Code or Codex, the two adapters +/// `docs/STRUCTURED-VIEW.md` lists. Every client gates its structured-view +/// control on this, and the daemon rejects a `Structured` request for +/// anything else, so the answer has to come from one place. +pub fn supports_structured_view(command: &str) -> bool { + matches!(command_tool(command), Some("claude" | "codex")) +} + /// [`resolve`]'s outcome: the tokenized command line, and the index within it /// of the word that is the actual program (past every assignment/wrapper it /// resolved through). @@ -1064,4 +1073,14 @@ mod tests { assert_eq!(tokenize("\"FOO=1\" claude").unwrap()[0].plain_eq, None); assert_eq!(tokenize("FOO\\=1 claude").unwrap()[0].plain_eq, None); } + + #[test] + fn structured_view_support_follows_the_resolved_program() { + assert!(supports_structured_view("claude --resume")); + assert!(supports_structured_view("FOO=1 /usr/local/bin/codex exec")); + assert!(supports_structured_view("npx claude-code")); + assert!(!supports_structured_view("opencode")); + assert!(!supports_structured_view("bash -l")); + assert!(!supports_structured_view("claude-code-acp")); + } } diff --git a/crates/maestro-tui/src/draft.rs b/crates/maestro-tui/src/draft.rs index b78b275..b233bca 100644 --- a/crates/maestro-tui/src/draft.rs +++ b/crates/maestro-tui/src/draft.rs @@ -41,7 +41,7 @@ use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; use maestro_config::{builtin_presets, custom_preset}; -use maestro_protocol::display::{abbreviate_dir, command_tool}; +use maestro_protocol::display::{abbreviate_dir, supports_structured_view}; use maestro_protocol::{ BranchInfo, CreateSessionRequest, Project, SessionView, SpawnPreset, WorktreeSpec, }; @@ -255,9 +255,14 @@ pub struct DraftPane { /// "Advanced options" disclosure (#93). Closed by default — most spawns /// take the default target. pub advanced_collapsed: bool, - /// ACP-backed structured transcript. Off by default and supported only - /// when the resolved command is Claude Code or Codex. + /// ACP-backed structured transcript. Supported only when the resolved + /// command is Claude Code or Codex, and on by default whenever it is — + /// until the operator works the toggle themselves. pub structured_view: bool, + /// Set once the operator toggles structured view by hand. From then on a + /// command change never re-enables what they turned off, the same guard + /// `name_touched` gives the name field. + pub structured_view_touched: bool, /// Enclosing repository of the chosen target — from the project scan for /// picked rows, `/api/gitroot` for raw paths. `None` until known. pub base_repo: Option, @@ -298,7 +303,7 @@ impl Default for DraftPane { // while its data loads, and stay usable when the daemon reports none. let mut presets = builtin_presets(); presets.push(custom_preset()); - Self { + let mut draft = Self { id: NEXT_DRAFT_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), presets, selected_preset: 0, @@ -319,6 +324,7 @@ impl Default for DraftPane { worktree_enabled: true, advanced_collapsed: true, structured_view: false, + structured_view_touched: false, base_repo: None, probed_cwd: None, probe_seq: 0, @@ -337,7 +343,11 @@ impl Default for DraftPane { // straight away, and Tab from here reaches the command tiles. focused_field: DraftField::DirPicker, home_dir: String::new(), - } + }; + // The built-in tile the form opens on decides whether structured view + // starts ticked, exactly as a later preset list would. + draft.sync_structured_view(); + draft } } @@ -356,7 +366,7 @@ impl DraftPane { .position(|p| !p.command.is_empty()) .unwrap_or(0); self.apply_seed_command(); - self.clear_unsupported_structured_view(); + self.sync_structured_view(); } /// Land a seeded command on the tile that runs it: its own preset when the @@ -673,18 +683,24 @@ impl DraftPane { } fn supports_structured_view(&self) -> bool { - matches!(command_tool(self.command()), Some("claude" | "codex")) + supports_structured_view(self.command()) } - fn clear_unsupported_structured_view(&mut self) { + /// Re-resolve the toggle after the command changed: an unsupported + /// command forces it off, a supported one turns it back on unless the + /// operator has already made the choice themselves. + fn sync_structured_view(&mut self) { if !self.supports_structured_view() { self.structured_view = false; + } else if !self.structured_view_touched { + self.structured_view = true; } } pub fn toggle_structured_view(&mut self) { if self.supports_structured_view() { self.structured_view = !self.structured_view; + self.structured_view_touched = true; } } @@ -827,7 +843,7 @@ impl DraftPane { } self.selected_preset = ((self.selected_preset as i32) + delta).rem_euclid(len as i32) as usize; - self.clear_unsupported_structured_view(); + self.sync_structured_view(); } /// Step-one boxes surviving the type-to-filter, in display order. A @@ -1130,7 +1146,7 @@ impl DraftPane { DraftField::Preset | DraftField::Submit => {} DraftField::CustomCommand => { self.custom_command.handle_key(other); - self.clear_unsupported_structured_view(); + self.sync_structured_view(); } // Typing filters the list; the highlight resets to the top // so a narrowed search always starts at its best match. @@ -1888,7 +1904,7 @@ mod tests { } #[test] - fn structured_view_is_default_off_and_visible_for_supported_agents() { + fn structured_view_is_default_on_and_visible_for_supported_agents() { let mut draft = DraftPane { advanced_collapsed: false, ..DraftPane::default() @@ -1910,13 +1926,11 @@ mod tests { command: "opencode".into(), }, ]); - assert!(!draft.structured_view); + assert!(draft.structured_view, "on by default for a supported agent"); assert!(draft.field_order().contains(&DraftField::StructuredToggle)); let rendered = render_to_string(100, 24, |f| draft.render(f, f.area(), true)); - assert!(rendered.contains("[ ] structured view"), "{rendered}"); + assert!(rendered.contains("[x] structured view"), "{rendered}"); - draft.toggle_structured_view(); - assert!(draft.structured_view); draft.cycle_preset(1); assert_eq!(draft.command(), "codex"); assert!( @@ -2006,16 +2020,16 @@ mod tests { }; draft.selected_preset = draft.presets.len() - 1; draft.custom_command = crate::text_input::TextInput::with_value(command); + draft.sync_structured_view(); assert_eq!( draft.field_order().contains(&DraftField::StructuredToggle), supported, "toggle visibility for {command:?}" ); - draft.toggle_structured_view(); assert_eq!( draft.structured_view, supported, - "toggle takes effect for {command:?}" + "default selection for {command:?}" ); let view = draft.build_request().unwrap().view; assert_eq!( @@ -2027,6 +2041,12 @@ mod tests { }, "request view for {command:?}" ); + + draft.toggle_structured_view(); + assert!( + !draft.structured_view, + "the toggle turns the default off for {command:?}" + ); } } @@ -2061,7 +2081,6 @@ mod tests { assert_eq!(draft.command(), "npx claude-code --resume"); assert!(draft.field_order().contains(&DraftField::StructuredToggle)); - draft.toggle_structured_view(); assert!(draft.structured_view); assert_eq!(draft.build_request().unwrap().view, SessionView::Structured); @@ -2080,6 +2099,43 @@ mod tests { assert_eq!(draft.build_request().unwrap().view, SessionView::Terminal); } + /// The default only applies until the operator has an opinion: once they + /// untick the box, moving between supported commands leaves it unticked + /// instead of turning it back on under them. + #[test] + fn an_unticked_structured_view_survives_a_command_change() { + let mut draft = DraftPane { + picked_cwd: Some(PathBuf::from("/srv/work")), + worktree_enabled: false, + advanced_collapsed: false, + ..DraftPane::default() + }; + draft.set_presets(vec![ + SpawnPreset { + id: "claude".into(), + label: "Claude".into(), + command: "claude".into(), + }, + SpawnPreset { + id: "codex".into(), + label: "Codex".into(), + command: "codex".into(), + }, + ]); + assert!(draft.structured_view); + + draft.toggle_structured_view(); + assert!(!draft.structured_view); + + draft.cycle_preset(1); + assert_eq!(draft.command(), "codex"); + assert!( + !draft.structured_view, + "the operator's choice outranks the default" + ); + assert_eq!(draft.build_request().unwrap().view, SessionView::Terminal); + } + /// Editing the custom-command text away from a supported agent clears an armed /// selection immediately, matching the preset-switch case above — an /// invalid `Structured` request can never reach `build_request`. @@ -2094,7 +2150,7 @@ mod tests { }; draft.selected_preset = draft.presets.len() - 1; draft.custom_command = crate::text_input::TextInput::with_value("codex"); - draft.toggle_structured_view(); + draft.sync_structured_view(); assert!(draft.structured_view); typed(&mut draft, " -p"); diff --git a/docs/STRUCTURED-VIEW.md b/docs/STRUCTURED-VIEW.md index 9eb9a2a..3dd5db7 100644 --- a/docs/STRUCTURED-VIEW.md +++ b/docs/STRUCTURED-VIEW.md @@ -35,6 +35,22 @@ session's worktree holds unsaved work — applies to a structured session exactly as it does to a terminal one; only what gets terminated differs (an ACP handle instead of a tmux pane). +## Choosing the view + +`view` is fixed at spawn and never changes. Every client picks it the same +way, from the resolved command alone +(`maestro_protocol::display::supports_structured_view`): a command that +resolves to Claude Code or Codex spawns **Structured** by default, anything +else spawns **Terminal**. The operator can override the default — the web and +TUI spawn forms show the checkbox ticked and let it be unticked, and +`maestro spawn` takes `--terminal` (or `--structured`, which sends the request +even for a command the CLI does not recognise and lets the daemon refuse it). +Once the operator works the toggle, a later command change never re-ticks it +under them; it can still be forced off when the new command has no adapter. + +The daemon is the last word: a `Structured` request for a command with no +adapter is refused there, whichever client sent it. + ## The adapter The resolved structured command selects a provider adapter. Its binary is diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index 24dd35f..b5e20fe 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -375,13 +375,23 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { // operator has already touched (#76). let commandTouched = false; + // Set once the operator works the checkbox themselves. Until then every + // command change re-derives the default (on for Claude Code and Codex, off + // for everything else); afterwards their choice outranks it, the same guard + // `commandTouched` gives the command field. + let structuredTouched = false; + function updateStructuredAvailability(): void { const supported = supportsStructuredView(command); structuredWrap.hidden = !supported; if (!supported) structuredView.checked = false; + else if (!structuredTouched) structuredView.checked = true; } - structuredView.addEventListener("change", emit); + structuredView.addEventListener("change", () => { + structuredTouched = true; + emit(); + }); cmdRaw.addEventListener("input", () => { if (selectedPresetId === "custom" || presets.find((p) => p.id === selectedPresetId)?.command === "") { diff --git a/web/src/spawnFormStructured.test.ts b/web/src/spawnFormStructured.test.ts index 0b84651..298fe40 100644 --- a/web/src/spawnFormStructured.test.ts +++ b/web/src/spawnFormStructured.test.ts @@ -44,7 +44,7 @@ describe("spawn form structured view", () => { vi.unstubAllGlobals(); }); - it("is default-off and shown for resolved Claude Code and Codex commands", async () => { + it("is default-on and shown for resolved Claude Code and Codex commands", async () => { stubApi(); handle = createSpawnForm({ onSuccess() {} }); document.body.appendChild(handle.el); @@ -57,7 +57,7 @@ describe("spawn form structured view", () => { handle.el.querySelector(".wt-toggle")!.click(); expect(disclosure.hidden).toBe(false); expect(wrap.hidden).toBe(false); - expect(checkbox.checked).toBe(false); + expect(checkbox.checked).toBe(true); const custom = handle.el.querySelector('[data-id="custom"]')!; custom.click(); @@ -65,13 +65,34 @@ describe("spawn form structured view", () => { command.value = "FOO=1 /usr/local/bin/codex"; command.dispatchEvent(new Event("input")); expect(wrap.hidden).toBe(false); + expect(checkbox.checked).toBe(true); command.value = "opencode"; command.dispatchEvent(new Event("input")); expect(wrap.hidden).toBe(true); + expect(checkbox.checked).toBe(false); }); - it("clears a checked value when Codex changes to an unsupported command", async () => { + it("keeps an unchecked box unchecked across a command change", async () => { + stubApi(); + handle = createSpawnForm({ onSuccess() {} }); + document.body.appendChild(handle.el); + await drain(); + + const checkbox = handle.el.querySelector("input[name=structured_view]")!; + expect(checkbox.checked).toBe(true); + checkbox.click(); + expect(checkbox.checked).toBe(false); + + handle.el.querySelector('[data-id="custom"]')!.click(); + const command = handle.el.querySelector(".cmd-raw")!; + command.value = "codex"; + command.dispatchEvent(new Event("input")); + + expect(checkbox.checked).toBe(false); + }); + + it("clears the checked default when Codex changes to an unsupported command", async () => { stubApi(); handle = createSpawnForm({ onSuccess() {} }); document.body.appendChild(handle.el); @@ -83,7 +104,6 @@ describe("spawn form structured view", () => { command.dispatchEvent(new Event("input")); const checkbox = handle.el.querySelector("input[name=structured_view]")!; - checkbox.click(); expect(checkbox.checked).toBe(true); command.value = "opencode"; @@ -93,7 +113,7 @@ describe("spawn form structured view", () => { expect(checkbox.checked).toBe(false); }); - it.each(["claude", "codex"])("emits Structured for a selected %s command", async (tool) => { + it.each(["claude", "codex"])("emits Structured for a %s command by default", async (tool) => { const requests = stubApi(); handle = createSpawnForm({ seed: { cwd: "/srv/work", cwdRawVisible: true, worktreeEnabled: false }, @@ -109,7 +129,6 @@ describe("spawn form structured view", () => { command.dispatchEvent(new Event("input")); } - handle.el.querySelector("input[name=structured_view]")!.click(); handle.el.requestSubmit(); await drain(); @@ -119,4 +138,21 @@ describe("spawn form structured view", () => { view: "Structured", }); }); + + it("emits no view once the operator unticks the box", async () => { + const requests = stubApi(); + handle = createSpawnForm({ + seed: { cwd: "/srv/work", cwdRawVisible: true, worktreeEnabled: false }, + onSuccess() {}, + }); + document.body.appendChild(handle.el); + await drain(); + + handle.el.querySelector("input[name=structured_view]")!.click(); + handle.el.requestSubmit(); + await drain(); + + expect(requests).toHaveLength(1); + expect(JSON.parse(String(requests[0].body)).view).toBeUndefined(); + }); });