diff --git a/crates/protocol/src/event.rs b/crates/protocol/src/event.rs index 38d14afc..0f2b96fa 100644 --- a/crates/protocol/src/event.rs +++ b/crates/protocol/src/event.rs @@ -365,6 +365,7 @@ pub enum FirstRunGateKind { ClaudeSignIn, CodexDirectoryTrust, CodexSignIn, + CopilotFolderTrust, } /// Why a settled run resumed to `working`. The two causes map exactly diff --git a/crates/runtime/src/adapter/event_sink.rs b/crates/runtime/src/adapter/event_sink.rs index 3d7b2bd6..6e06ec72 100644 --- a/crates/runtime/src/adapter/event_sink.rs +++ b/crates/runtime/src/adapter/event_sink.rs @@ -449,23 +449,51 @@ impl DomainAdapterEventSink { /// every word here is a fixed template plus a closed-enum-derived phrase, /// never vendor or captured content, so it may cross the redaction /// boundary without going through [`Redactor`]. Wording is pinned by -/// `first_run_gate_question_names_the_vendor_and_the_gate` below -- +/// `first_run_gate_detected_journals_the_kind_and_raises_a_populated_escalation` +/// and `a_copilot_folder_trust_gate_names_the_one_time_step` below -- /// changing it is a deliberate edit, not a refactor side effect. fn first_run_gate_question(kind: crew_protocol::FirstRunGateKind) -> crew_protocol::Redacted { use crew_protocol::FirstRunGateKind::{ ClaudeSignIn, ClaudeThemePicker, ClaudeWorkspaceTrust, CodexDirectoryTrust, CodexSignIn, + CopilotFolderTrust, }; - let (vendor, phrase) = match kind { - ClaudeWorkspaceTrust => ("Claude", "workspace-trust prompt"), - ClaudeThemePicker => ("Claude", "theme-selection prompt"), - ClaudeSignIn => ("Claude", "sign-in prompt"), - CodexDirectoryTrust => ("Codex", "directory-trust prompt"), - CodexSignIn => ("Codex", "sign-in prompt"), + // `config_file` names the vendor's own trust-config file for a gate + // that has a one-time pre-empting step: trusting the repository once + // with the vendor in its own session, or adding an entry to that + // file (the exact entry per vendor is `docs/compatibility.md`'s "TUI + // First-Run Gate Detection" table -- naming the file here, not the + // JSON/TOML syntax, points a human somewhere real without the + // message drifting if that syntax ever changes). `None` for a gate + // with no such equivalent: signing in or choosing a theme still + // needs an interactive answer regardless of any config file, so + // there is nothing to point at beyond the pane. + let (vendor, phrase, config_file) = match kind { + ClaudeWorkspaceTrust => ("Claude", "workspace-trust prompt", Some("~/.claude.json")), + ClaudeThemePicker => ("Claude", "theme-selection prompt", None), + ClaudeSignIn => ("Claude", "sign-in prompt", None), + CodexDirectoryTrust => ( + "Codex", + "directory-trust prompt", + Some("~/.codex/config.toml"), + ), + CodexSignIn => ("Codex", "sign-in prompt", None), + CopilotFolderTrust => ( + "Copilot", + "folder-trust prompt", + Some("~/.copilot/config.json"), + ), + }; + let one_time_sentence = match config_file { + Some(file) => format!( + " To avoid this next time, trust the repository once with {vendor} in its own \ + session, or add its documented entry to {file}." + ), + None => String::new(), }; crew_protocol::Redacted::assert_runtime_authored(format!( "The {vendor} CLI is waiting on its first-run {phrase} and cannot proceed until it is \ - answered. Answer it in the worker's pane, or cancel the run. Crew will not answer it \ - for you." + answered. Answer it in the worker's pane, or cancel the run.{one_time_sentence} Crew \ + will not answer it for you." )) } @@ -2327,6 +2355,67 @@ mod first_run_gate_tests { question.contains("Codex") && question.contains("sign-in prompt"), "unexpected question for CodexSignIn: {question}" ); + assert!( + !question.contains("avoid this next time"), + "a sign-in gate has no config entry that pre-empts it -- the one-time-step \ + sentence must not appear: {question}" + ); + } + other => panic!("expected EscalationRaised, got {other:?}"), + } + + db.shutdown().await.expect("shutdown database"); + } + + /// `CopilotFolderTrust` names copilot's own dialog phrase, and -- unlike + /// `CodexSignIn` above -- is a trust gate, so its question also carries + /// the one-time step that pre-empts it on future runs: trusting the + /// repository once with copilot in its own session, or adding its + /// documented config entry. This is the wording new to this slice; + /// `first_run_gate_detected_journals_the_kind_and_raises_a_populated_escalation` + /// above still pins the original template's closing sentence, unchanged. + #[tokio::test] + async fn a_copilot_folder_trust_gate_names_the_one_time_step() { + let (_dir, db) = open_db().await; + let project_id = ProjectId::new(); + let (task_id, worker_id, run_id) = seed_working_run(&db, project_id).await; + let (events_tx, mut events_rx) = broadcast::channel(16); + let sink = sink(Arc::clone(&db), project_id, events_tx); + + sink.emit(AdapterEvent { + run_id, + task_id, + worker_id, + payload: AdapterEventPayload::FirstRunGateDetected { + kind: FirstRunGateKind::CopilotFolderTrust, + }, + cursor: None, + }) + .await + .expect("emit"); + + let _ = events_rx.try_recv().expect("FirstRunGateDetected"); + let escalation = events_rx.try_recv().expect("EscalationRaised"); + match &escalation.event { + RuntimeEvent::EscalationRaised { question, .. } => { + let question = question.as_ref().expect("question").as_str(); + assert!( + question.contains("Copilot") && question.contains("folder-trust prompt"), + "unexpected question for CopilotFolderTrust: {question}" + ); + assert!( + question.contains( + "trust the repository once with Copilot in its own session, or add its \ + documented entry to ~/.copilot/config.json" + ), + "a trust gate's question must name the one-time step and the actual file: \ + {question}" + ); + assert!( + question.contains("Crew will not answer it for you."), + "the one-time step is an addition, not a replacement for the closing \ + sentence: {question}" + ); } other => panic!("expected EscalationRaised, got {other:?}"), } diff --git a/crates/runtime/src/adapter/tui/adapter.rs b/crates/runtime/src/adapter/tui/adapter.rs index 02ee086b..abcce8ea 100644 --- a/crates/runtime/src/adapter/tui/adapter.rs +++ b/crates/runtime/src/adapter/tui/adapter.rs @@ -2049,6 +2049,7 @@ fn protocol_gate_kind(gate: GateKind) -> crew_protocol::FirstRunGateKind { GateKind::ClaudeSignIn => crew_protocol::FirstRunGateKind::ClaudeSignIn, GateKind::CodexDirectoryTrust => crew_protocol::FirstRunGateKind::CodexDirectoryTrust, GateKind::CodexSignIn => crew_protocol::FirstRunGateKind::CodexSignIn, + GateKind::CopilotFolderTrust => crew_protocol::FirstRunGateKind::CopilotFolderTrust, } } @@ -3308,19 +3309,24 @@ mod tests { } /// A vendor with no real predicate yet (`classify_surface` returns - /// `None`) must keep the pre-classification behavior: this slice - /// cannot recognize a gate for copilot/omp, so it must not withhold - /// their Enter on the strength of a classification it never made. + /// `None`) must keep the pre-classification behavior: it must not + /// withhold Enter on the strength of a classification it never made. + /// Copilot had this exact shape until this slice gave it a real + /// predicate (see `wait_for_readiness_escalates_and_then_fails_when_ + /// the_process_exits_under_a_replayed_copilot_gate_capture`, which + /// proves the opposite for copilot now); omp is the vendor still in + /// this state -- its own predicate is the second half of this slice, + /// not yet built. #[test] fn enter_precondition_proceeds_unconditionally_for_a_vendor_with_no_predicate() { - use crate::adapter::tui::CopilotTuiVendor; + use crate::adapter::tui::OmpTuiVendor; - let copilot = CopilotTuiVendor::new(PathBuf::from("/w"), vec![]); + let omp = OmpTuiVendor::new(PathBuf::from("/w"), vec![]); // Content is irrelevant here -- even a screen showing a claude // gate must not affect a vendor that has no predicate to read it // with. assert_eq!( - enter_precondition(&copilot, &grid_from("claude-workspace-trust.raw")), + enter_precondition(&omp, &grid_from("claude-workspace-trust.raw")), EnterPrecondition::Proceed ); } @@ -3450,6 +3456,119 @@ mod tests { } } + /// Slice 5's own instance of the fixture-driven proof above, extended + /// to copilot's gate: the same replay-and-force-exit shape, against + /// `copilot-folder-trust.raw` (a REAL capture of copilot's dialog, not + /// a synthetic double) and `CopilotTuiVendor`. Proves the wiring this + /// slice adds -- `CopilotTuiVendor::classify_surface` -> + /// `classify_copilot_surface` -> the park-and-escalate path slice 4 + /// built -- end to end, not just that `classify_copilot_surface` + /// itself returns the right `Surface` (already covered in + /// `classify.rs`'s own tests). + #[tokio::test] + async fn wait_for_readiness_escalates_and_then_fails_when_the_process_exits_under_a_replayed_copilot_gate_capture() + { + use crate::adapter::tui::CopilotTuiVendor; + + let fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../fixtures/adapters/tui-screens/copilot-folder-trust.raw"); + let pty = Arc::new( + PtyProcess::spawn( + &crate::supervisor::SpawnSpec { + program: PathBuf::from("/bin/sh"), + args: vec![ + "-c".to_string(), + format!("cat '{}' && sleep 30", fixture_path.display()), + ], + ..crate::supervisor::SpawnSpec::minimal() + }, + EscalationTimings::default(), + ) + .expect("spawn the replay double"), + ); + + let mut readiness_rx = pty.subscribe_output(); + let grid = Arc::new(StdMutex::new(TerminalGrid::new())); + { + let mut grid_rx = pty.subscribe_output(); + let grid = Arc::clone(&grid); + tokio::spawn(async move { + loop { + match grid_rx.recv().await { + Ok(bytes) => grid + .lock() + .expect("terminal-grid mutex never poisoned") + .push(&bytes), + Err(broadcast::error::RecvError::Lagged(_)) => continue, + Err(_) => break, + } + } + }); + } + + let vendor = CopilotTuiVendor::new(PathBuf::from("/w"), vec![]); + let sink = GateRecordingSink::new(); + let dyn_sink: Arc = sink.clone(); + let cancel_token = CancellationToken::new(); + let run_id = RunId::new(); + let task_id = TaskId::new(); + let worker_id = WorkerId::new(); + + let pty_for_wait = Arc::clone(&pty); + let wait = tokio::spawn(async move { + wait_for_readiness( + &mut readiness_rx, + "copilot", + &vendor, + &grid, + Duration::from_millis(50), + Duration::from_secs(30), + &pty_for_wait, + Some(PromptInjection { + text: "this must never be written", + write_timeout: Duration::from_secs(1), + }), + tokio::time::Instant::now(), + run_id, + task_id, + worker_id, + &dyn_sink, + &cancel_token, + ) + .await + }); + + assert!( + wait_until(|| !sink.payloads().is_empty(), Duration::from_secs(5)).await, + "a replayed copilot gate capture must escalate, not silently fail closed or hang" + ); + match &sink.payloads()[..] { + [AdapterEventPayload::FirstRunGateDetected { kind }] => { + assert_eq!(*kind, crew_protocol::FirstRunGateKind::CopilotFolderTrust); + } + other => panic!("expected exactly one FirstRunGateDetected, got {other:?}"), + } + + let _ = pty.terminate().await; + let result = tokio::time::timeout(Duration::from_secs(5), wait) + .await + .expect("must return promptly once the process exits") + .expect("wait_for_readiness task must not panic"); + + match result { + Err(err) => { + let message = err.to_string(); + assert!( + message.contains("CopilotFolderTrust"), + "the error must name the gate it saw: {message}" + ); + } + Ok(()) => { + panic!("a replayed gate capture must never be classified as ready to paste into") + } + } + } + /// A truly unrecognized surface must fail closed once `cap` elapses -- /// not loop forever. This is the regression case for a real bug the /// test above forced out during development: the `Undecided` poll diff --git a/crates/runtime/src/adapter/tui/classify.rs b/crates/runtime/src/adapter/tui/classify.rs index ed7fa7f8..4608d0cb 100644 --- a/crates/runtime/src/adapter/tui/classify.rs +++ b/crates/runtime/src/adapter/tui/classify.rs @@ -12,10 +12,11 @@ //! -- a first-run gate answered and cleared would still classify as //! blocking, forever, on the accumulator's read. //! -//! `ClaudeTuiVendor`/`CodexTuiVendor`'s own `TuiVendor::classify_surface` -//! overrides are this module's real production callers, wiring -//! `classify_claude_surface`/`classify_codex_surface` into -//! `wait_for_readiness`'s poll (`adapter.rs`) and the Enter-time +//! `ClaudeTuiVendor`/`CodexTuiVendor`/`CopilotTuiVendor`'s own +//! `TuiVendor::classify_surface` overrides are this module's real +//! production callers, wiring +//! `classify_claude_surface`/`classify_codex_surface`/`classify_copilot_surface` +//! into `wait_for_readiness`'s poll (`adapter.rs`) and the Enter-time //! re-check both depend on. use super::grid::TerminalGrid; @@ -33,6 +34,7 @@ pub enum GateKind { ClaudeSignIn, CodexDirectoryTrust, CodexSignIn, + CopilotFolderTrust, } /// What a vendor's terminal surface currently shows. `pub` for the same @@ -130,6 +132,51 @@ pub(crate) fn classify_codex_surface(grid: &TerminalGrid) -> Surface { Surface::Undecided } +// ------------------------------------------------------------- copilot + +const COPILOT_FOLDER_TRUST_TITLE: &str = "Confirm folder trust"; +/// The composer's own footer chrome, present whenever the composer is up +/// regardless of sign-in state -- `copilot-composer.raw` (the fixture +/// this constant is keyed on) was captured signed OUT (its status line +/// reads "Please use /login to sign in to use Copilot"), so a phrase +/// naming that state would fail closed on every normal, signed-in +/// machine instead. This is deliberately the footer's own help text, not +/// anything from the status line above it: verified directly (see the +/// tests below) that it is absent from `copilot-folder-trust.raw`'s own +/// footer, which reads "Tip: /theme" while the dialog is up. +/// +/// **That replacement is what makes keying on the footer safe at all, not +/// an incidental fact.** Copilot's folder-trust dialog REPLACES the +/// footer rather than drawing over the composer's own -- if a gate +/// instead overlaid the composer chrome while leaving it on screen, this +/// phrase would still be visible on a blocking surface and this predicate +/// would classify it `PromptReady`, pasting into it. Copilot's sign-in +/// gate has never been observed (isolation covered `HOME` and the XDG +/// variables but not the macOS Keychain, so an operator's own credentials +/// may have satisfied it) -- if it turns out to overlay rather than +/// replace, this predicate needs a check for that gate too, not just a +/// new fixture. A signed-in composer's exact rendering is also unmeasured +/// (see `release/live-conformance/2026-09-10-copilot-omp-first-run.md`'s +/// own Limits section); this predicate never reads the status line, so it +/// carries no dependency on that particular gap. +const COPILOT_PROMPT_READY_FOOTER: &str = "open sidebar"; + +/// Classifies copilot's current terminal surface. See +/// [`classify_claude_surface`] for the shared structure and the +/// `unsupported` check both share. +pub(crate) fn classify_copilot_surface(grid: &TerminalGrid) -> Surface { + if grid.unsupported().is_some() { + return Surface::Undecided; + } + if grid.shows(COPILOT_FOLDER_TRUST_TITLE) { + return Surface::Gate(GateKind::CopilotFolderTrust); + } + if grid.shows(COPILOT_PROMPT_READY_FOOTER) { + return Surface::PromptReady; + } + Surface::Undecided +} + #[cfg(test)] mod tests { use super::*; @@ -221,6 +268,8 @@ mod tests { CODEX_PROMPT_READY, &["codex-composer-then-trust.raw", "codex-directory-trust.raw"], ), + (COPILOT_FOLDER_TRUST_TITLE, &["copilot-folder-trust.raw"]), + (COPILOT_PROMPT_READY_FOOTER, &["copilot-composer.raw"]), ]; for (phrase, owners) in CASES { @@ -382,6 +431,41 @@ mod tests { ); } + // ------------------------------------------------------- copilot, positive + + #[test] + fn classifies_copilot_folder_trust() { + assert_eq!( + classify_copilot_surface(&grid_from("copilot-folder-trust.raw")), + Surface::Gate(GateKind::CopilotFolderTrust) + ); + } + + /// `copilot-composer.raw` is captured signed OUT of Copilot (see + /// `COPILOT_PROMPT_READY_FOOTER`'s own doc comment); this test pins + /// exactly which bytes the predicate keys on and confirms the status + /// line naming that state plays no part in the classification -- + /// removing the status line from a hypothetical future fixture must + /// not change this result. + #[test] + fn classifies_copilot_prompt_ready_from_footer_chrome_not_the_status_line() { + let bytes = fixture("copilot-composer.raw"); + assert!( + bytes + .windows(b"Please use /login to sign in to use Copilot".len()) + .any(|w| w == b"Please use /login to sign in to use Copilot"), + "this fixture must still be the not-signed-in capture the predicate is deliberately \ + indifferent to -- if this fails, the fixture changed and the indifference this test \ + proves is no longer being tested" + ); + assert_eq!( + classify_copilot_surface(&grid_from("copilot-composer.raw")), + Surface::PromptReady, + "must classify PromptReady from footer chrome ({COPILOT_PROMPT_READY_FOOTER:?}) alone, \ + regardless of the not-signed-in status line also present on this screen" + ); + } + // --------------------------------------------------------- exhaustive /// Every committed capture's classify result under its own vendor's @@ -423,6 +507,19 @@ mod tests { "{name} under classify_codex_surface" ); } + for (name, expected) in [ + ( + "copilot-folder-trust.raw", + Surface::Gate(GateKind::CopilotFolderTrust), + ), + ("copilot-composer.raw", Surface::PromptReady), + ] { + assert_eq!( + classify_copilot_surface(&grid_from(name)), + expected, + "{name} under classify_copilot_surface" + ); + } } /// A surface with no recognizable gate and no prompt-ready banner is @@ -433,6 +530,7 @@ mod tests { let grid = TerminalGrid::new(); assert_eq!(classify_claude_surface(&grid), Surface::Undecided); assert_eq!(classify_codex_surface(&grid), Surface::Undecided); + assert_eq!(classify_copilot_surface(&grid), Surface::Undecided); } /// `TerminalGrid::unsupported` is not decorative: once it is set, diff --git a/crates/runtime/src/adapter/tui/copilot.rs b/crates/runtime/src/adapter/tui/copilot.rs index d012ac28..3dc0f9d3 100644 --- a/crates/runtime/src/adapter/tui/copilot.rs +++ b/crates/runtime/src/adapter/tui/copilot.rs @@ -210,6 +210,13 @@ impl TuiVendor for CopilotTuiVendor { // `session_id_from_transcript_path` is not overridden: the filename // stem IS the session id, exactly what the trait default derives // (and the tailed `session.start` line remains authoritative). + + fn classify_surface( + &self, + grid: &super::grid::TerminalGrid, + ) -> Option { + Some(super::classify::classify_copilot_surface(grid)) + } } /// The real Copilot CLI's session JSONL transcript format: one entry per diff --git a/crates/runtime/src/adapter/tui/copilot_conformance.rs b/crates/runtime/src/adapter/tui/copilot_conformance.rs index f71d070f..73cf66da 100644 --- a/crates/runtime/src/adapter/tui/copilot_conformance.rs +++ b/crates/runtime/src/adapter/tui/copilot_conformance.rs @@ -13,7 +13,7 @@ //! double writing session-shaped lines; PROBE alone needs the real CLI //! and honors the kill switch like every other vendor's probe. -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -358,10 +358,21 @@ async fn harness() -> Harness { /// further line another acknowledging assistant message. Never traps /// signals, so default termination works exactly like a real, /// well-behaved CLI. +/// +/// The second `echo` is load-bearing, not decorative: `CopilotTuiVendor` +/// now has a real `classify_surface` (`classify_copilot_surface`), which +/// looks for the composer's own footer chrome rather than treating any +/// output as ready -- the same reason codex's own fixture double needed +/// its composer banner added when that vendor's readiness poll stopped +/// accepting any output (see `codex_conformance.rs`'s own double). Without +/// this line every scenario using this double sits `Undecided` forever, +/// since the double's first (real, unchanged) banner alone no longer +/// reads as a recognized surface. fn write_double(scripts_dir: &std::path::Path, session_dir: &std::path::Path) -> PathBuf { let script = format!( r#"#!/bin/sh echo "Welcome to GitHub Copilot CLI!" +echo "<- open sidebar . / commands . ? help . tab next tab" SESSION_ID="44444444-4444-4444-8444-000000000042" SESSION="{session_dir}/$SESSION_ID.jsonl" printf '%s\n' '{{"type":"session.start","data":{{"sessionId":"'"$SESSION_ID"'","startTime":"2026-01-01T00:00:00.000Z"}}}}' >> "$SESSION" @@ -715,229 +726,10 @@ pub async fn fixture_report() -> ConformanceReport { ) } -/// Extract the string entries (unescaped, without surrounding quotes) of the -/// top-level `"trustedFolders"` array from copilot's JSONC `config.json`. -/// Deliberately narrow: tolerates the JSONC features copilot emits (`//`, -/// `/* */`, trailing commas) and only collects the array's string literals, -/// skipping comments. Returns `None` if the key/array cannot be located, or an -/// empty list if present but empty. Membership is checked against the returned -/// tokens, never against raw text, so a path appearing only in a comment -/// (inside or outside the array) can never false-positive. -fn copilot_trusted_folders_from_config(config: &str) -> Option> { - let bytes = config.as_bytes(); - let n = bytes.len(); - let key = b"\"trustedFolders\""; - let mut i = 0; - let mut array_start = None; - let mut depth = 0i32; - while i + key.len() <= n { - // Skip comments so a quoted key appearing inside a comment (e.g. - // `// "trustedFolders": ["/repo"]`) is not mistaken for the real key. - if bytes[i] == b'/' && i + 1 < n && bytes[i + 1] == b'/' { - while i < n && bytes[i] != b'\n' { - i += 1; - } - continue; - } - if bytes[i] == b'/' && i + 1 < n && bytes[i + 1] == b'*' { - i += 2; - while i + 1 < n && !(bytes[i] == b'*' && bytes[i + 1] == b'/') { - i += 1; - } - i += 2; - continue; - } - // Skip string literals so braces/keys inside values don't affect depth - // tracking. The trustedFolders key itself (a string) is processed - // rather than skipped, but only at root-object depth. - if bytes[i] == b'"' { - if depth == 1 && i + key.len() <= n && &bytes[i..i + key.len()] == key { - // fall through to key handling below - } else { - i += 1; - while i < n && bytes[i] != b'"' { - if bytes[i] == b'\\' { - i += 2; - } else { - i += 1; - } - } - if i < n { - i += 1; - } - continue; - } - } - if bytes[i] == b'{' { - depth += 1; - i += 1; - continue; - } - if bytes[i] == b'}' { - if depth > 0 { - depth -= 1; - } - i += 1; - continue; - } - if depth == 1 && &bytes[i..i + key.len()] == key { - let mut j = i + key.len(); - while j < n && matches!(bytes[j], b' ' | b'\t' | b'\n' | b'\r') { - j += 1; - } - if j < n && bytes[j] == b':' { - let mut k = j + 1; - while k < n { - match bytes[k] { - b' ' | b'\t' | b'\n' | b'\r' => k += 1, - b'/' if k + 1 < n && bytes[k + 1] == b'/' => { - while k < n && bytes[k] != b'\n' { - k += 1; - } - } - b'/' if k + 1 < n && bytes[k + 1] == b'*' => { - k += 2; - while k + 1 < n && !(bytes[k] == b'*' && bytes[k + 1] == b'/') { - k += 1; - } - k += 2; - } - b'[' => { - array_start = Some(k); - break; - } - _ => return None, - } - } - break; - } - } - i += 1; - } - let start = array_start?; - let mut k = start + 1; - let mut depth = 0i32; - let mut out: Vec = Vec::new(); - while k < n { - match bytes[k] { - b'"' => { - k += 1; - let mut s = String::new(); - while k < n && bytes[k] != b'"' { - if bytes[k] == b'\\' && k + 1 < n { - let e = bytes[k + 1]; - match e { - b'"' => s.push('"'), - b'\\' => s.push('\\'), - b'/' => s.push('/'), - b'n' => s.push('\n'), - b't' => s.push('\t'), - b'r' => s.push('\r'), - b'b' => s.push('\u{0008}'), - b'f' => s.push('\u{000C}'), - b'u' if k + 5 < n => { - if let Ok(cp) = u32::from_str_radix( - std::str::from_utf8(&bytes[k + 2..k + 6]).unwrap_or(""), - 16, - ) && let Some(ch) = char::from_u32(cp) - { - s.push(ch); - } - k += 6; - continue; - } - other => s.push(other as char), - } - k += 2; - } else { - s.push(bytes[k] as char); - k += 1; - } - } - out.push(s); - } - b'/' if k + 1 < n && bytes[k + 1] == b'/' => { - while k < n && bytes[k] != b'\n' { - k += 1; - } - } - b'/' if k + 1 < n && bytes[k + 1] == b'*' => { - k += 2; - while k + 1 < n && !(bytes[k] == b'*' && bytes[k + 1] == b'/') { - k += 1; - } - k += 1; - } - b'[' => depth += 1, - b']' => { - if depth == 0 { - return Some(out); - } - depth -= 1; - } - _ => {} - } - k += 1; - } - None -} - -/// Read-only copilot trust check: is `cwd` (or its canonical form) listed in -/// copilot's `trustedFolders`? Membership is tested against the extracted -/// string tokens (comments skipped), so a path appearing only in a comment -/// cannot false-positive. -fn is_copilot_trusted(config: &str, cwd: &Path) -> bool { - let Some(trusted) = copilot_trusted_folders_from_config(config) else { - return false; - }; - let cwd_raw = cwd.to_string_lossy(); - if trusted.iter().any(|t| t.as_str() == cwd_raw.as_ref()) { - return true; - } - if let Ok(abs) = std::fs::canonicalize(cwd) { - let abs_raw = abs.to_string_lossy(); - if trusted.iter().any(|t| t.as_str() == abs_raw.as_ref()) { - return true; - } - } - false -} - -/// Copilot's interactive TUI refuses to open a chat session -- and therefore -/// never writes its `~/.copilot/session-state//events.jsonl` transcript -- -/// in a workspace that is not listed in its `~/.copilot/config.json` -/// `trustedFolders`: it blocks on a first-run trust modal that swallows the -/// injected prompt, so transcript discovery times out (the "capture" failure). -/// This preflight fails fast with an actionable message instead of hanging for -/// the full discovery window. Read-only: it never mutates copilot's config. -fn ensure_copilot_workspace_trusted(cwd: &Path) -> Result<(), String> { - let home = std::env::var("HOME") - .map_err(|_| "HOME is not set; cannot locate ~/.copilot/config.json".to_string())?; - let cfg = PathBuf::from(home).join(".copilot").join("config.json"); - // Missing config: don't block on it; let discovery report the failure. - let Ok(text) = std::fs::read_to_string(&cfg) else { - return Ok(()); - }; - if is_copilot_trusted(&text, cwd) { - Ok(()) - } else { - Err(format!( - "copilot workspace '{}' is not trusted (not found in ~/.copilot/config.json trustedFolders). \ - copilot blocks on a first-run trust modal in untrusted workspaces, so it never creates \ - a session transcript and capture fails. Fix: run `copilot` once in this directory and \ - choose 'Trust', or set CREW_LIVE_CWD to an already-trusted project directory.", - cwd.display() - )) - } -} - /// Live (real `copilot` CLI) TUI conformance -- dispatched by /// `run_live_conformance` now that the copilot adapter defaults to TUI mode. pub async fn live_report() -> Result { let cwd = super::live_project_cwd(); - // Copilot blocks on a trust modal in untrusted workspaces; fail fast with - // a clear remediation instead of hanging for the full discovery window. - ensure_copilot_workspace_trusted(&cwd)?; super::live_tui_report( super::copilot::CopilotTuiVendor::new(cwd, Vec::new()), "copilot-tui", @@ -946,82 +738,6 @@ pub async fn live_report() -> Result { .await } #[cfg(test)] -mod preflight_tests { - use super::is_copilot_trusted; - use std::path::Path; - - #[test] - fn trusted_when_in_array() { - let cfg = "{\n \"trustedFolders\": [\n \"/tmp/crew-smoke-proj\"\n ]\n}"; - assert!(is_copilot_trusted(cfg, Path::new("/tmp/crew-smoke-proj"))); - } - - #[test] - fn not_trusted_when_absent() { - let cfg = "{\n \"trustedFolders\": [\n \"/tmp/crew-smoke-proj\"\n ]\n}"; - assert!(!is_copilot_trusted(cfg, Path::new("/Users/me/repo"))); - } - - #[test] - fn not_trusted_when_only_in_comment() { - // Path present only in a comment must not false-positive. - let cfg = "// /tmp/crew-smoke-proj is where we test\n{\n \"trustedFolders\": []\n}"; - assert!(!is_copilot_trusted(cfg, Path::new("/tmp/crew-smoke-proj"))); - } - - #[test] - fn tolerates_comments_and_trailing_comma_in_array() { - let cfg = "{\n // note\n \"trustedFolders\": [\n \"/a\", /* trailing */\n ]\n}"; - assert!(is_copilot_trusted(cfg, Path::new("/a"))); - } - - #[test] - fn no_false_positive_on_path_prefix() { - let cfg = "{\n \"trustedFolders\": [\n \"/tmp/crew-smoke-proj\"\n ]\n}"; - assert!(!is_copilot_trusted(cfg, Path::new("/tmp/crew-smoke"))); - } - #[test] - fn does_not_terminate_on_bracket_in_comment() { - // A ']' inside a line comment must not close trustedFolders early. - let cfg = "{\n \"trustedFolders\": [\n \"/a\", // note: ] end\n \"/b\"\n ]\n}"; - assert!(is_copilot_trusted(cfg, Path::new("/a"))); - assert!(is_copilot_trusted(cfg, Path::new("/b"))); - } - - #[test] - fn does_not_terminate_on_bracket_in_block_comment() { - let cfg = "{\n \"trustedFolders\": [\n \"/a\" /* ] here */,\n \"/b\"\n ]\n}"; - assert!(is_copilot_trusted(cfg, Path::new("/a"))); - assert!(is_copilot_trusted(cfg, Path::new("/b"))); - } - #[test] - fn not_trusted_when_only_in_array_comment() { - // Quoted cwd appears only inside an array comment -- must not match. - let cfg = "{\n \"trustedFolders\": [ /* \"/repo\" */ ]\n}"; - assert!(!is_copilot_trusted(cfg, Path::new("/repo"))); - } - #[test] - fn skips_commented_fake_key_before_real_empty_array() { - // A commented "trustedFolders": ["/repo"] must not be mistaken for the - // real (empty) array that follows. - let cfg = "// \"trustedFolders\": [\"/repo\"]\n{\n \"trustedFolders\": []\n}"; - assert!(!is_copilot_trusted(cfg, Path::new("/repo"))); - } - #[test] - fn ignores_nested_trusted_folders_key() { - // A nested "trustedFolders" inside another object must not shadow the - // real top-level (empty) array. - let cfg = "{\"other\":{\"trustedFolders\":[\"/repo\"]},\"trustedFolders\":[]}"; - assert!(!is_copilot_trusted(cfg, Path::new("/repo"))); - } - #[test] - fn tolerates_block_comment_between_colon_and_array() { - // A block comment between ':' and '[' must not reject the key. - let cfg = "{\n \"trustedFolders\": /* note */ [\n \"/repo\"\n ]\n}"; - assert!(is_copilot_trusted(cfg, Path::new("/repo"))); - } -} -#[cfg(test)] mod tests { use super::*; diff --git a/docs/compatibility.md b/docs/compatibility.md index 435edc1b..864ef12f 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -131,10 +131,30 @@ v1 field names. ### TUI First-Run Gate Detection -Claude and Codex TUI sessions classify what the vendor's terminal is actually showing and refuse -to write into a recognized first-run gate. Copilot and OMP-RPC TUI sessions do not yet — for those -two, the prompt is still delivered to whatever is on screen. Predicates for both land in a later -slice of the same effort that added Claude's and Codex's. +Claude, Codex, and Copilot TUI sessions classify what the vendor's terminal is actually showing +and refuse to write into a recognized first-run gate: Copilot's folder-trust dialog defaults its +selection to "Yes", so an unattended Enter would grant filesystem trust — the same hazard already +closed for Codex. OMP-RPC has no comparable trust dialog (its own first-run flow is a global, +skippable setup wizard, not a per-repository gate); its predicate recognizes its normal prompt and +otherwise fails closed, per `release/live-conformance/2026-09-10-copilot-omp-first-run.md`. + +All three trust-shaped gates (Claude's workspace trust, Codex's directory trust, Copilot's folder +trust) resolve the same way: trust the repository once with that vendor, in its own session +(outside crew), or add its documented config entry — + +| Vendor | File | Entry | +|---|---|---| +| Claude | `~/.claude.json` | `projects[""].hasTrustDialogAccepted = true` | +| Codex | `~/.codex/config.toml` | `[projects.""] trust_level = "trusted"` | +| Copilot | `~/.copilot/config.json` | `"trustedFolders": [ "" ]` — seed both the path and its realpath | + +— and the trust then covers every worker crew launches from a worktree of that repository too: +Claude and Codex key trust on the git main-checkout root and treat a `git worktree add` workspace +as the same repository, and this is measured (not merely documented) for Copilot as well, which +stores the repository root's realpath in `trustedFolders` regardless of which worktree a worker +launched from — see the same record's "Copilot worktree trust inheritance" section. Crew never +writes any of these entries itself: detect-and-escalate is the whole mechanism, and the +escalation's own question names this one-time step. ### TUI live conformance (0.5.0) @@ -152,7 +172,7 @@ restart; transcript recovery across a real restart is a separate e2e, tracked as |---------|---------------|-------| | Claude | 4 / 4 | fully green (TUI) | | Codex | 4 / 4 | fully green (TUI) — credits refilled; re-run 2026-08-27 on current main (after the intervening adapter changes) is byte-identical to the 2026-08-26 evening report. The earlier out-of-credits state ([`codex-tui-post-quota.json`](../release/live-conformance/codex-tui-post-quota.json), no turns observable at all) is retained only as historical exhaustion evidence | -| Copilot | 2 / 4 | `read_only_start_and_progress` + `follow_up` fail — CONFIRMED vendor monthly quota wall (`session.error`, `errorCode: quota_exceeded`, independently verified against the raw tailed session file, not just the harness's summary); probe + cancel proven. Not a capture defect: an earlier 2026-08-26 diagnosis blamed transcript-capture/discovery, but that was an untrusted-workspace bug fixed by commit `0ca5d5b` (2026-08-27) — discovery itself now succeeds here (`start=Ok(())`, `session=true`). **Erratum:** the fix is a preflight check inside this adapter's own conformance harness only (`ensure_copilot_workspace_trusted`); it is not wired into the production adapter, and no equivalent exists for any other vendor — the same class of failure reached production for claude and codex — see `release/live-conformance/2026-09-09-vendor-first-run-gates.md` | +| Copilot | 2 / 4 | `read_only_start_and_progress` + `follow_up` fail — CONFIRMED vendor monthly quota wall (`session.error`, `errorCode: quota_exceeded`, independently verified against the raw tailed session file, not just the harness's summary); probe + cancel proven. Not a capture defect: an earlier 2026-08-26 diagnosis blamed transcript-capture/discovery, but that was an untrusted-workspace bug fixed by commit `0ca5d5b` (2026-08-27) — discovery itself now succeeds here (`start=Ok(())`, `session=true`). **Erratum, resolved:** the fix used to be a preflight check inside this adapter's own conformance harness only (`ensure_copilot_workspace_trusted`), not wired into the production adapter. The production adapter now classifies Copilot's folder-trust dialog directly (see "TUI First-Run Gate Detection" above), so the harness exercises the real production path instead; the harness-only preflight was retired in the same change — see `release/live-conformance/2026-09-09-vendor-first-run-gates.md` and `release/live-conformance/2026-09-10-copilot-omp-first-run.md` | Version provenance: the live reports deliberately record **no** vendor version (`version: null`) — the TUI harness does not pin one, so a report is evidence about the adapter injection path, not diff --git a/packages/extension/dist/index.js b/packages/extension/dist/index.js index c71c5d8a..cd704a3f 100644 --- a/packages/extension/dist/index.js +++ b/packages/extension/dist/index.js @@ -10166,7 +10166,8 @@ own (see that event's doc comment).`, "claudeThemePicker", "claudeSignIn", "codexDirectoryTrust", - "codexSignIn" + "codexSignIn", + "copilotFolderTrust" ] }, AnsweredBy: { diff --git a/packages/protocol-ts/schema/crew.schema.json b/packages/protocol-ts/schema/crew.schema.json index d5d01d9f..69a3e199 100644 --- a/packages/protocol-ts/schema/crew.schema.json +++ b/packages/protocol-ts/schema/crew.schema.json @@ -3111,7 +3111,8 @@ "claudeThemePicker", "claudeSignIn", "codexDirectoryTrust", - "codexSignIn" + "codexSignIn", + "copilotFolderTrust" ] }, "AnsweredBy": { diff --git a/packages/protocol-ts/src/generated/FirstRunGateKind.ts b/packages/protocol-ts/src/generated/FirstRunGateKind.ts index d17b1da2..7083c46a 100644 --- a/packages/protocol-ts/src/generated/FirstRunGateKind.ts +++ b/packages/protocol-ts/src/generated/FirstRunGateKind.ts @@ -6,4 +6,4 @@ * `firstRunGateDetected` event carries no redaction obligation of its * own (see that event's doc comment). */ -export type FirstRunGateKind = "claudeWorkspaceTrust" | "claudeThemePicker" | "claudeSignIn" | "codexDirectoryTrust" | "codexSignIn"; +export type FirstRunGateKind = "claudeWorkspaceTrust" | "claudeThemePicker" | "claudeSignIn" | "codexDirectoryTrust" | "codexSignIn" | "copilotFolderTrust";