From 2aba3219e605051bbeca4eb6b1a1f3dce404cfd3 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 13 Aug 2026 18:16:30 +0000 Subject: [PATCH] Recognize PowerShell `Get-Content` file reads (#38415) ## What changed - Classify simple PowerShell `Get-Content` commands, including `gc` and `type` aliases, as file reads while preserving Windows paths. - Reuse the shared classification for implicit skill invocation detection on Windows and render recognized commands as `Read ` in the TUI. - Leave commands with unsupported flags, multiple operands, wildcards, or expressions unclassified. ## Testing - Cover supported and rejected PowerShell forms, Windows executor skill detection, and the TUI read summary. GitOrigin-RevId: 4e8f5470f2ae31c08d74091f9634c2926e516ccf --- codex-rs/core/tests/suite/skills_extension.rs | 9 +- codex-rs/shell-command/src/parse_command.rs | 172 +++++++++++++++++- codex-rs/skills/src/invocation.rs | 60 +----- codex-rs/skills/src/invocation_tests.rs | 26 ++- codex-rs/tui/src/exec_cell/render.rs | 28 +++ 5 files changed, 231 insertions(+), 64 deletions(-) diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index 9e356e18e5..3e83163e5b 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -53,7 +53,6 @@ use core_test_support::responses::mount_sse_once; use core_test_support::responses::sse; use core_test_support::skip_if_no_network; use core_test_support::skip_if_remote; -use core_test_support::skip_if_target_windows; use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_mcp_server; @@ -1241,7 +1240,6 @@ async fn production_turn_aliases_executor_skill_roots() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn executor_skill_invocation_is_environment_scoped_and_deduplicated() -> Result<()> { - skip_if_target_windows!(Ok(()), "executes a POSIX cat command"); skip_if_remote!(Ok(()), "executor fixture uses a host-local skill path"); skip_if_no_network!(Ok(())); @@ -1280,8 +1278,13 @@ async fn executor_skill_invocation_is_environment_scoped_and_deduplicated() -> R ], warnings: Vec::new(), }; + let read_command = if cfg!(windows) { + format!("Get-Content -LiteralPath \"{}\"", skill_path.display()) + } else { + format!("cat {}", skill_path.display()) + }; let command = json!({ - "cmd": format!("cat {}", skill_path.display()), + "cmd": read_command, "login": false, }) .to_string(); diff --git a/codex-rs/shell-command/src/parse_command.rs b/codex-rs/shell-command/src/parse_command.rs index 7f5e8942d7..e91fa3c472 100644 --- a/codex-rs/shell-command/src/parse_command.rs +++ b/codex-rs/shell-command/src/parse_command.rs @@ -12,6 +12,30 @@ pub fn shlex_join(tokens: &[String]) -> String { .unwrap_or_else(|_| "".to_string()) } +/// Tokenizes a PowerShell command while preserving Windows paths and reader aliases. +pub fn tokenize_powershell_command(command: &str) -> Vec { + let normalized = command.replace('\\', "/"); + let mut tokens = shlex_split(&normalized) + .unwrap_or_else(|| normalized.split_whitespace().map(str::to_string).collect()); + if let Some(executable) = tokens.first_mut() + && matches!( + executable.to_ascii_lowercase().as_str(), + "get-content" | "gc" | "type" + ) + { + *executable = "Get-Content".to_owned(); + // POSIX shlex must not silently rewrite a PowerShell file path. + if tokens + .iter() + .skip(1) + .any(|argument| !normalized.contains(argument)) + { + return Vec::new(); + } + } + tokens +} + /// Extracts the shell and script from a command, regardless of platform pub fn extract_shell_command(command: &[String]) -> Option<(&str, &str)> { extract_bash_command(command).or_else(|| extract_powershell_command(command)) @@ -1245,6 +1269,102 @@ mod tests { ); } + #[test] + fn powershell_file_reads_are_classified() { + for (shell, script, path) in [ + ( + "powershell", + r"Get-Content C:\skills\demo\SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ( + "powershell", + r#"get-content -Raw "C:\skills and plugins\SKILL.md""#, + "C:/skills and plugins/SKILL.md", + ), + ( + "powershell", + r"Get-Content 'C:\skills and plugins\SKILL.md'", + "C:/skills and plugins/SKILL.md", + ), + ( + "powershell", + r"Get-Content -Path C:\skills\demo\SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ( + "powershell", + r"Get-Content -LiteralPath C:\skills\demo\SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ( + "powershell", + r"Get-Content C:\skills\demo\SKILL.md -Raw", + "C:/skills/demo/SKILL.md", + ), + ( + "powershell", + r"Get-Content -Raw -LiteralPath C:\skills\demo\SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ( + "powershell", + r"Get-Content C:\workspace\README.md", + "C:/workspace/README.md", + ), + ( + "powershell", + "gc C:/skills/demo/SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ( + "powershell", + "type C:/skills/demo/SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ( + r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe", + "Get-Content C:/skills/demo/SKILL.md", + "C:/skills/demo/SKILL.md", + ), + ] { + assert_parsed( + &vec_str(&[shell, "-NoProfile", "-Command", script]), + vec![ParsedCommand::Read { + cmd: script.to_string(), + name: PathBuf::from(path) + .file_name() + .expect("file path") + .to_string_lossy() + .into_owned(), + path: PathBuf::from(path), + }], + ); + } + } + + #[test] + fn complex_powershell_file_reads_are_intentionally_not_classified() { + for script in [ + r"Get-Content 'C:\Users\O''Brien\skill\SKILL.md'", + r#"Get-Content "$(Remove-Item C:/important)/skills/demo/SKILL.md""#, + "Get-Content -ReadCount:([IO.File]::Delete('C:/important')) C:/skills/demo/SKILL.md", + "Get-Content C:/Users/Alice/.ssh/id_rsa,C:/skills/demo/SKILL.md", + "Get-Content C:/skills/demo/SKILL.md -Raw; Remove-Item C:/important", + "Get-Content C:/skills/demo/SKILL.md C:/important", + "Get-Content C:/skills/*/SKILL.md", + "Get-Content -Encoding UTF8 C:/skills/demo/SKILL.md", + "Get-Content -Raw", + ] { + assert_parsed( + &vec_str(&["powershell", "-Command", script]), + vec![ParsedCommand::Unknown { + cmd: script.to_string(), + }], + ); + } + } + #[test] fn pwsh_with_noprofile_and_c_alias_is_stripped() { assert_parsed( @@ -1277,7 +1397,29 @@ pub fn parse_command_impl(command: &[String]) -> Vec { return commands; } - if let Some((_, script)) = extract_powershell_command(command) { + let powershell_command = command + .first() + .filter(|shell| shell.contains('\\')) + .map(|shell| { + let mut normalized = command.to_vec(); + normalized[0] = shell.rsplit(['/', '\\']).next().unwrap_or(shell).to_owned(); + normalized + }); + if let Some((_, script)) = + extract_powershell_command(powershell_command.as_deref().unwrap_or(command)) + { + let tokens = tokenize_powershell_command(script); + if tokens + .first() + .is_some_and(|executable| executable == "Get-Content") + && let [ParsedCommand::Read { name, path, .. }] = parse_command_impl(&tokens).as_slice() + { + return vec![ParsedCommand::Read { + cmd: script.to_string(), + name: name.clone(), + path: path.clone(), + }]; + } return vec![ParsedCommand::Unknown { cmd: script.to_string(), }]; @@ -2253,8 +2395,32 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand { path, } } - Some((head, tail)) if head == "cat" => { - if let Some(path) = single_non_flag_operand(tail, &[]) { + Some((head, tail)) if head == "cat" || head.eq_ignore_ascii_case("Get-Content") => { + let path = if head == "cat" { + single_non_flag_operand(tail, &[]) + } else { + // Intentionally miss complex reads: conservative presentation should not + // require implementing PowerShell's full expression and argument grammar. + if tail.iter().all(|argument| { + !argument.starts_with('-') + || ["-Raw", "-Path", "-LiteralPath"] + .iter() + .any(|flag| argument.eq_ignore_ascii_case(flag)) + }) { + single_non_flag_operand(tail, &[]) + } else { + None + } + .filter(|path| { + !path.is_empty() + && !path.starts_with('-') + && path.chars().all(|character| { + character.is_alphanumeric() + || matches!(character, ' ' | '/' | '\\' | '.' | '-' | '_' | ':') + }) + }) + }; + if let Some(path) = path { let name = short_display_path(&path); ParsedCommand::Read { cmd: shlex_join(main_cmd), diff --git a/codex-rs/skills/src/invocation.rs b/codex-rs/skills/src/invocation.rs index dbfa9a8332..e7427efef5 100644 --- a/codex-rs/skills/src/invocation.rs +++ b/codex-rs/skills/src/invocation.rs @@ -2,6 +2,7 @@ use std::path::Path; use codex_protocol::parse_command::ParsedCommand; use codex_shell_command::parse_command::parse_command_impl; +use codex_shell_command::parse_command::tokenize_powershell_command; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; @@ -28,14 +29,17 @@ pub fn detect_implicit_skill_invocation_for_command( workdir: &AbsolutePathBuf, ) -> Option { let workdir = canonicalize_if_exists(workdir); - let tokens = tokenize_command(command); + let tokens = if PathConvention::native() == PathConvention::Windows { + tokenize_powershell_command(command) + } else { + tokenize_command(command) + }; if let Some(candidate) = detect_skill_script_run(outcome, tokens.as_slice(), &workdir) { return Some(candidate); } detect_skill_doc_read(outcome, tokens.as_slice(), &workdir) - .or_else(|| detect_powershell_skill_doc_read(outcome, command, &workdir)) } /// Resolves statically recognizable skill accesses without consulting the host filesystem. @@ -43,20 +47,8 @@ pub fn implicit_skill_accesses_for_command( command: &str, workdir: &PathUri, ) -> Vec { - // Normalize Windows paths and recognize PowerShell reads using existing cat parsing. let tokens = if workdir.infer_path_convention() == Some(PathConvention::Windows) { - let mut tokens = tokenize_command(&command.replace('\\', "/")); - - if let Some(executable) = tokens.first_mut() - && matches!( - executable.to_ascii_lowercase().as_str(), - "get-content" | "gc" | "type" - ) - { - *executable = "cat".to_owned(); - } - - tokens + tokenize_powershell_command(command) } else { tokenize_command(command) }; @@ -151,44 +143,6 @@ fn detect_skill_doc_read( None } -fn detect_powershell_skill_doc_read( - outcome: &impl ImplicitSkillLookup, - command: &str, - workdir: &AbsolutePathBuf, -) -> Option { - let path = powershell_get_content_path(command)?; - let candidate_path = canonicalize_if_exists(&workdir.join(Path::new(path))); - outcome - .implicit_skill_for_doc_path(&candidate_path) - .cloned() -} - -fn powershell_get_content_path(command: &str) -> Option<&str> { - let mut arguments = command.trim().strip_prefix("Get-Content ")?; - if let Some(remaining_arguments) = arguments.strip_prefix("-Raw ") { - arguments = remaining_arguments; - } - - let (path, trailing) = if let Some(quoted_path) = arguments.strip_prefix('"') { - let closing_quote = quoted_path.find('"')?; - ( - "ed_path[..closing_quote], - "ed_path[closing_quote + 1..], - ) - } else { - let path_end = arguments - .char_indices() - .find_map(|(index, character)| character.is_whitespace().then_some(index)) - .unwrap_or(arguments.len()); - (&arguments[..path_end], &arguments[path_end..]) - }; - - if path.is_empty() || path.starts_with('-') || !trailing.trim().is_empty() { - return None; - } - Some(path) -} - fn command_basename(command: &str) -> String { Path::new(command) .file_name() diff --git a/codex-rs/skills/src/invocation_tests.rs b/codex-rs/skills/src/invocation_tests.rs index 02109f092a..6d946d893c 100644 --- a/codex-rs/skills/src/invocation_tests.rs +++ b/codex-rs/skills/src/invocation_tests.rs @@ -85,6 +85,7 @@ fn powershell_skill_doc_read_matches_common_forms() { format!("Get-Content -Raw {path}"), format!("Get-Content \"{spaced_path}\""), format!("Get-Content -Raw \"{spaced_path}\""), + format!("get-content -raw '{spaced_path}'"), ] { let found = detect_implicit_skill_invocation_for_command( &outcome, @@ -101,11 +102,26 @@ fn powershell_skill_doc_read_matches_common_forms() { } #[test] -fn powershell_get_content_path_preserves_windows_backslashes() { - assert_eq!( - powershell_get_content_path(r#"Get-Content C:\skills\example\SKILL.md"#), - Some(r#"C:\skills\example\SKILL.md"#) - ); +fn windows_executor_skill_reads_share_powershell_classification() { + let workdir = PathUri::parse("file:///C:/skills").expect("Windows workdir URI"); + let document = PathUri::parse("file:///C:/skills/demo/SKILL.md").expect("skill URI"); + + for command in [ + r"Get-Content C:\skills\demo\SKILL.md", + r"get-content -Raw C:\skills\demo\SKILL.md", + r"Get-Content -Path C:\skills\demo\SKILL.md", + r"Get-Content -LiteralPath C:\skills\demo\SKILL.md", + r"Get-Content C:\skills\demo\SKILL.md -Raw", + r"Get-Content -Raw -LiteralPath C:\skills\demo\SKILL.md", + r"gc C:\skills\demo\SKILL.md", + r"type C:\skills\demo\SKILL.md", + ] { + assert_eq!( + implicit_skill_accesses_for_command(command, &workdir), + vec![ImplicitSkillAccess::Document(document.clone())], + "command: {command}" + ); + } } #[test] diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index 55135de9ad..ed091515d3 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -964,6 +964,34 @@ mod tests { ); } + #[test] + fn powershell_skill_read_snapshot() { + let command = vec![ + "powershell.exe".to_string(), + "-Command".to_string(), + r"Get-Content C:\skills\demo\SKILL.md".to_string(), + ]; + let parsed = codex_shell_command::parse_command::parse_command(&command); + let cell = new_active_exec_command( + "call-id".to_string(), + command, + parsed, + ExecCommandSource::Agent, + /*interaction_input*/ None, + /*animations_enabled*/ false, + ); + let rendered = cell + .display_lines(/*width*/ 80) + .iter() + .map(render_line_text) + .join("\n"); + + insta::assert_snapshot!(rendered, @r" + • Exploring + └ Read SKILL.md + "); + } + #[test] fn command_truncation_ellipsis_does_not_include_transcript_hint() { let truncated = ExecCell::limit_lines_from_start(