From d3dd44be00efe6713af5047ffa5ebb560ce2eaef Mon Sep 17 00:00:00 2001 From: Nikolas Demiridis Date: Thu, 10 Sep 2026 13:00:24 +0300 Subject: [PATCH 1/2] feat(runtime): recognize copilot's folder-trust gate Copilot's TUI session now gets a real classify_surface predicate, closing the gap left when Claude and Codex got theirs: a copilot first-run session shows a folder-trust dialog whose selection defaults to "Yes", so an unattended Enter grants filesystem trust before a human ever answers it -- the same hazard already closed for codex. classify_copilot_surface recognizes the dialog by its own title ("Confirm folder trust", a new CopilotFolderTrust gate kind) and recognizes the composer as ready by footer chrome ("open sidebar") rather than anything in the status line. That distinction matters because the only committed composer capture is unauthenticated -- its status line reads "Please use /login to sign in to use Copilot" -- so a predicate keyed on that line would fail closed on every normal, signed-in machine instead of the unconfigured one it is meant to catch. A dedicated test pins exactly which bytes the predicate keys on and confirms the status line plays no part in the result. Both phrases are proven exclusive against every committed fixture, and the fixture-driven park-and-escalate path (journal, escalate, resume or fail on exit) is proven end to end against a real replayed capture of the dialog, mirroring the equivalent claude/codex proof from the slice that built that path. The copilot conformance harness's own fixture double never printed anything the new predicate recognizes as ready -- the same shape codex's fake double hit when its own readiness poll stopped accepting any output as proof of life. Fixed the same way: added the real footer chrome to the double's banner. The copilot conformance harness's own workaround for the untrusted-workspace case (`ensure_copilot_workspace_trusted`, a preflight check reachable only from the live conformance path, never wired into the production adapter) is retired now that the production adapter recognizes the same condition directly; the harness exercises the real path instead. docs/compatibility.md records both: Copilot's gate as covered rather than pending, and the erratum this closes. The escalation question a recognized trust gate raises now also names the one-time step that pre-empts it on every future run -- trusting the repository once with that vendor in its own session, or adding its documented config entry -- alongside the existing instruction to answer it in the pane or cancel the run. This applies to all three trust-shaped gates (claude/codex/copilot); sign-in and theme-selection gates are unchanged, since there is no config-file equivalent for them to point at. Tests: the new predicate's positive/negative cases and its exclusivity proof; the fixture-driven park-and-escalate proof against copilot's real dialog capture; the escalation question's new sentence for a trust gate and its deliberate absence for a non-trust gate. Fixed two tests that assumed copilot had no real predicate, which stopped being true here. cargo fmt --all, cargo clippy --all-targets --all-features -D warnings, CREW_DISABLE_VENDOR_CLI=1 cargo test --workspace (69 test binaries, all passing), bun test, bun run typecheck, and bun run generate --check all verified clean. --- crates/protocol/src/event.rs | 1 + crates/runtime/src/adapter/event_sink.rs | 107 +++++- crates/runtime/src/adapter/tui/adapter.rs | 131 +++++++- crates/runtime/src/adapter/tui/classify.rs | 106 +++++- crates/runtime/src/adapter/tui/copilot.rs | 7 + .../src/adapter/tui/copilot_conformance.rs | 308 +----------------- docs/compatibility.md | 30 +- packages/protocol-ts/schema/crew.schema.json | 3 +- .../src/generated/FirstRunGateKind.ts | 2 +- 9 files changed, 373 insertions(+), 322 deletions(-) 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/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"; From 4577681015e8ed00d25a3a1c4f181176c1e08372 Mon Sep 17 00:00:00 2001 From: "crew-bot[bot]" <322634903+crew-bot[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 10:26:33 +0000 Subject: [PATCH 2/2] chore(extension): refresh bundle from linux-x64 CI Build: https://github.com/nikolasd/crew/actions/runs/34466037263 Source: bd1567660cd2404fe1f2ca54a25b7f0138dbdf2c --- packages/extension/dist/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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: {