mirror of
https://github.com/openai/codex.git
synced 2026-10-21 14:23:14 +00:00
Harden TUI parsing of assistant markup (#42640)
## What changed - Add a shared parser for assistant directives that handles quoted and unquoted attributes, embedded braces, escaped quotes, and malformed input consistently. - Use the parser for Git action receipts and code comments, including Git actions that follow a rendered code comment on the same line. - Parse `file://` links with `PathUri` so Windows drive and UNC paths render correctly regardless of the TUI host while preserving line and column anchors. ## Testing - Add unit coverage for directive grammar and snapshot coverage for code comments, Git actions, and foreign Windows file links. GitOrigin-RevId: 8252b3254b92ec6943096026e74c19160c4e24cc
This commit is contained in:
@@ -0,0 +1,119 @@
|
||||
//! Parse structured annotations embedded in assistant-authored Markdown.
|
||||
//!
|
||||
//! For example, `::git-create-pr{cwd="/repo" isDraft=true}` becomes the name
|
||||
//! `git-create-pr` and the attributes `cwd` = `/repo` and `isDraft` = `true`.
|
||||
//! A review annotation such as `::code-comment{body="Keep \"x}\" literal."}`
|
||||
//! can contain quotes and braces within its quoted value. Consumers choose
|
||||
//! the quote-escaping rules and interpret the attributes; this module also
|
||||
//! retains the exact directive source, excluding any trailing Markdown.
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
/// An assistant annotation and its complete, unmodified source representation.
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
pub(crate) struct AssistantDirective<'source> {
|
||||
pub(crate) name: &'source str,
|
||||
pub(crate) attributes: BTreeMap<&'source str, Cow<'source, str>>,
|
||||
pub(crate) raw: &'source str,
|
||||
}
|
||||
|
||||
/// Git receipts preserve backslashes; review comments allow escaped quote delimiters.
|
||||
#[derive(Clone, Copy, Eq, PartialEq)]
|
||||
pub(crate) enum QuoteEscaping {
|
||||
Literal,
|
||||
Backslash,
|
||||
}
|
||||
|
||||
/// Parse one inline, leaf, or container marker from the beginning of `source`.
|
||||
pub(crate) fn parse_assistant_directive(
|
||||
source: &str,
|
||||
escaping: QuoteEscaping,
|
||||
) -> Option<AssistantDirective<'_>> {
|
||||
// `::git-create-pr{...}` starts with a one-to-three-colon marker and a name;
|
||||
// require `{` immediately after the name so `::git-create-pr prose` is not parsed.
|
||||
let rest = source.trim_start_matches(':');
|
||||
if !(1..=3).contains(&(source.len() - rest.len())) {
|
||||
return None;
|
||||
}
|
||||
let name_len = rest.bytes().take_while(|byte| is_name_byte(*byte)).count();
|
||||
let (name, suffix) = rest.split_at(name_len);
|
||||
let mut rest = suffix.strip_prefix('{')?;
|
||||
if !name.as_bytes().first().is_some_and(u8::is_ascii_alphabetic) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut attributes = BTreeMap::new();
|
||||
loop {
|
||||
rest = rest.trim_start_matches([' ', '\t']);
|
||||
if let Some(suffix) = rest.strip_prefix('}') {
|
||||
// In `::git-push{cwd="/repo"} done`, retain the directive but not ` done`.
|
||||
return Some(AssistantDirective {
|
||||
name,
|
||||
attributes,
|
||||
raw: &source[..source.len() - suffix.len()],
|
||||
});
|
||||
}
|
||||
// For `cwd = "/repo" isDraft=true}`, split off `cwd` and leave the next
|
||||
// attribute for the next iteration after consuming this value.
|
||||
let key_len = rest.bytes().take_while(|byte| is_name_byte(*byte)).count();
|
||||
let (key, suffix) = rest.split_at(key_len);
|
||||
// Reject malformed keys and duplicates before scanning a potentially long value.
|
||||
if key.is_empty() || attributes.contains_key(key) {
|
||||
return None;
|
||||
}
|
||||
let value = suffix.trim_start_matches([' ', '\t']).strip_prefix('=')?;
|
||||
rest = value.trim_start_matches([' ', '\t']);
|
||||
let value = if let Some(delimiter @ (b'"' | b'\'')) = rest.as_bytes().first().copied() {
|
||||
// In `body="Keep \"x}\" literal."`, only the matching unescaped quote
|
||||
// ends the value, not the embedded `}`. Single-quoted values work too.
|
||||
let quoted = &rest[1..];
|
||||
let mut characters = quoted.char_indices().peekable();
|
||||
let end = loop {
|
||||
let (index, character) = characters.next()?;
|
||||
match character {
|
||||
c if c == char::from(delimiter) => break index,
|
||||
'\n' | '\r' => return None,
|
||||
'\\' if escaping == QuoteEscaping::Backslash
|
||||
&& characters
|
||||
.peek()
|
||||
.is_some_and(|(_, next)| *next == char::from(delimiter)) =>
|
||||
{
|
||||
// Backslash mode consumes `\"` as a literal quote. Literal
|
||||
// mode instead lets the quote close `cwd="/repo\"`.
|
||||
characters.next();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
};
|
||||
let value = "ed[..end];
|
||||
rest = "ed[end + 1..];
|
||||
let escaped = format!("\\{}", char::from(delimiter));
|
||||
if escaping == QuoteEscaping::Backslash && value.contains(&escaped) {
|
||||
Cow::Owned(value.replace(&escaped, &char::from(delimiter).to_string()))
|
||||
} else {
|
||||
Cow::Borrowed(value)
|
||||
}
|
||||
} else {
|
||||
// Unquoted `isDraft=true` ends at whitespace or `}`, not at a quote.
|
||||
let end = rest
|
||||
.find([' ', '\t', '}', '\n', '\r'])
|
||||
.unwrap_or(rest.len());
|
||||
if end == 0 {
|
||||
return None;
|
||||
}
|
||||
let value = &rest[..end];
|
||||
rest = &rest[end..];
|
||||
Cow::Borrowed(value)
|
||||
};
|
||||
attributes.insert(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
fn is_name_byte(byte: u8) -> bool {
|
||||
byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-')
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "assistant_directives_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,86 @@
|
||||
//! Shared directive grammar and caller-specific quote compatibility.
|
||||
|
||||
use super::AssistantDirective;
|
||||
use super::QuoteEscaping;
|
||||
use super::parse_assistant_directive;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::borrow::Cow;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
#[test]
|
||||
fn parses_git_receipts_with_unquoted_attributes() {
|
||||
let raw = r#"::git-create-pr{cwd="/repo\" branch="feature/report" isDraft=true}"#;
|
||||
|
||||
assert_eq!(
|
||||
parse_assistant_directive(raw, QuoteEscaping::Literal),
|
||||
Some(AssistantDirective {
|
||||
name: "git-create-pr",
|
||||
attributes: BTreeMap::from([
|
||||
("branch", Cow::Borrowed("feature/report")),
|
||||
("cwd", Cow::Borrowed(r"/repo\")),
|
||||
("isDraft", Cow::Borrowed("true")),
|
||||
]),
|
||||
raw,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_triple_colon_comments_with_escaped_quotes_and_closing_braces() {
|
||||
let raw = r#":::code-comment{title="Fix class" body="Keep \"px-${size}\" literal." file="C:\Users\me\app.ts" start=10 priority=2}"#;
|
||||
let source = format!("{raw} remaining text");
|
||||
|
||||
assert_eq!(
|
||||
parse_assistant_directive(&source, QuoteEscaping::Backslash),
|
||||
Some(AssistantDirective {
|
||||
name: "code-comment",
|
||||
attributes: BTreeMap::from([
|
||||
(
|
||||
"body",
|
||||
Cow::Owned(r#"Keep "px-${size}" literal."#.to_string())
|
||||
),
|
||||
("file", Cow::Borrowed(r"C:\Users\me\app.ts")),
|
||||
("priority", Cow::Borrowed("2")),
|
||||
("start", Cow::Borrowed("10")),
|
||||
("title", Cow::Borrowed("Fix class")),
|
||||
]),
|
||||
raw,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_artifact_metadata_and_single_quoted_values() {
|
||||
let raw = ":artifact{path='Quarterly Report.xlsx' label='team\\'s report' sheet='Revenue' range='A1:D8'}";
|
||||
|
||||
assert_eq!(
|
||||
parse_assistant_directive(raw, QuoteEscaping::Backslash),
|
||||
Some(AssistantDirective {
|
||||
name: "artifact",
|
||||
attributes: BTreeMap::from([
|
||||
("label", Cow::Owned("team's report".to_string())),
|
||||
("path", Cow::Borrowed("Quarterly Report.xlsx")),
|
||||
("range", Cow::Borrowed("A1:D8")),
|
||||
("sheet", Cow::Borrowed("Revenue")),
|
||||
]),
|
||||
raw,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_ambiguous_or_incomplete_directives() {
|
||||
for source in [
|
||||
r#"::git-push{cwd="/repo" cwd="/other"}"#,
|
||||
r#"::git-push{cwd="/repo" branch="feature""#,
|
||||
"::::code-comment{title=comment}",
|
||||
":artifact{path=/tmp/a path=:artifact{path=/tmp/b}}",
|
||||
":artifact{invalid:key=:artifact{path=/tmp/a}}",
|
||||
":artifact{path=/tmp/a\nnext=value}",
|
||||
] {
|
||||
assert_eq!(
|
||||
parse_assistant_directive(source, QuoteEscaping::Backslash),
|
||||
None
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
//! Codex App directives embedded in assistant markdown.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use crate::assistant_directives::AssistantDirective;
|
||||
use crate::assistant_directives::QuoteEscaping;
|
||||
use crate::assistant_directives::parse_assistant_directive;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
|
||||
@@ -58,11 +60,8 @@ pub(crate) fn parse_assistant_markdown(markdown: &str, cwd: &Path) -> ParsedAssi
|
||||
let mut visible_lines = Vec::new();
|
||||
|
||||
for line in markdown.lines() {
|
||||
if let Some(rewritten) = rewrite_code_comment_line(line, cwd) {
|
||||
visible_lines.push(rewritten.trim_end().to_string());
|
||||
continue;
|
||||
}
|
||||
let (visible_line, line_actions) = strip_line_directives(line);
|
||||
let (visible_line, line_actions) =
|
||||
rewrite_code_comment_line(line, cwd).unwrap_or_else(|| strip_line_directives(line));
|
||||
for action in line_actions {
|
||||
if seen.insert(action.clone()) {
|
||||
git_actions.push(action);
|
||||
@@ -84,32 +83,29 @@ pub(crate) fn parse_assistant_markdown(markdown: &str, cwd: &Path) -> ParsedAssi
|
||||
}
|
||||
}
|
||||
|
||||
fn rewrite_code_comment_line(line: &str, cwd: &Path) -> Option<String> {
|
||||
fn rewrite_code_comment_line(line: &str, cwd: &Path) -> Option<(String, Vec<GitActionDirective>)> {
|
||||
let content = line.trim_start_matches([' ', '\t']);
|
||||
let indent = &line[..line.len() - content.len()];
|
||||
let marker_length = content.bytes().take_while(|byte| *byte == b':').count();
|
||||
if !(1..=3).contains(&marker_length) {
|
||||
let directive = parse_assistant_directive(content, QuoteEscaping::Backslash)?;
|
||||
if directive.name != "code-comment" {
|
||||
return None;
|
||||
}
|
||||
|
||||
let directive = content[marker_length..].strip_prefix("code-comment{")?;
|
||||
let (attributes, suffix) = directive.rsplit_once('}')?;
|
||||
let attributes = parse_code_comment_attributes(attributes)?;
|
||||
let title = attributes.get("title")?;
|
||||
let body = attributes.get("body")?;
|
||||
let file = attributes.get("file")?;
|
||||
let suffix = &content[directive.raw.len()..];
|
||||
let title = directive.attributes.get("title")?;
|
||||
let body = directive.attributes.get("body")?;
|
||||
let file = directive.attributes.get("file")?;
|
||||
let title = title.trim();
|
||||
let body = body.trim();
|
||||
let file = file.trim();
|
||||
(!title.is_empty() && !body.is_empty() && !file.is_empty()).then_some(())?;
|
||||
|
||||
let start = directive_integer(&attributes, "start").unwrap_or(1).max(1);
|
||||
let end = directive_integer(&attributes, "end")
|
||||
let start = directive_integer(&directive, "start").unwrap_or(1).max(1);
|
||||
let end = directive_integer(&directive, "end")
|
||||
.unwrap_or(start)
|
||||
.max(start);
|
||||
let title = if title_has_priority(title) {
|
||||
title.to_string()
|
||||
} else if let Some(priority @ 0..=3) = directive_integer(&attributes, "priority") {
|
||||
} else if let Some(priority @ 0..=3) = directive_integer(&directive, "priority") {
|
||||
format!("[P{priority}] {title}")
|
||||
} else {
|
||||
title.to_string()
|
||||
@@ -126,8 +122,10 @@ fn rewrite_code_comment_line(line: &str, cwd: &Path) -> Option<String> {
|
||||
format!("{file}:{start}-{end}")
|
||||
};
|
||||
|
||||
Some(format!(
|
||||
"{indent}- {title} — {location}\n{indent} {body}{suffix}"
|
||||
let (suffix, actions) = strip_line_directives(suffix);
|
||||
Some((
|
||||
format!("{indent}- {title} — {location}\n{indent} {body}{suffix}"),
|
||||
actions,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -138,29 +136,28 @@ pub(crate) fn strip_line_directives(line: &str) -> (String, Vec<GitActionDirecti
|
||||
|
||||
while let Some(start) = remaining.find("::git-") {
|
||||
visible.push_str(&remaining[..start]);
|
||||
let directive = &remaining[start + 2..];
|
||||
let Some(open_brace) = directive.find('{') else {
|
||||
visible.push_str(&remaining[start..]);
|
||||
let source = &remaining[start..];
|
||||
if let Some(directive) = parse_assistant_directive(source, QuoteEscaping::Literal) {
|
||||
if let Some(action) = parse_git_action(&directive) {
|
||||
actions.push(action);
|
||||
}
|
||||
remaining = &source[directive.raw.len()..];
|
||||
} else if let Some((_, attributes)) = source.split_once('{')
|
||||
&& let Some((_, suffix)) = attributes.split_once('}')
|
||||
{
|
||||
remaining = suffix;
|
||||
} else {
|
||||
visible.push_str(source);
|
||||
return (visible, actions);
|
||||
};
|
||||
let Some(close_brace) = directive[open_brace + 1..].find('}') else {
|
||||
visible.push_str(&remaining[start..]);
|
||||
return (visible, actions);
|
||||
};
|
||||
let close_brace = open_brace + 1 + close_brace;
|
||||
let name = &directive[..open_brace];
|
||||
let attributes = &directive[open_brace + 1..close_brace];
|
||||
if let Some(action) = parse_git_action(name, attributes) {
|
||||
actions.push(action);
|
||||
}
|
||||
remaining = &directive[close_brace + 1..];
|
||||
}
|
||||
visible.push_str(remaining);
|
||||
(visible, actions)
|
||||
}
|
||||
|
||||
fn directive_integer(attributes: &HashMap<String, String>, name: &str) -> Option<i64> {
|
||||
attributes
|
||||
fn directive_integer(directive: &AssistantDirective<'_>, name: &str) -> Option<i64> {
|
||||
directive
|
||||
.attributes
|
||||
.get(name)?
|
||||
.trim()
|
||||
.trim_start_matches(['P', 'p'])
|
||||
@@ -177,95 +174,32 @@ fn title_has_priority(title: &str) -> bool {
|
||||
&& bytes[3] == b']'
|
||||
}
|
||||
|
||||
fn parse_code_comment_attributes(input: &str) -> Option<HashMap<String, String>> {
|
||||
let mut attributes = HashMap::new();
|
||||
let mut rest = input.trim();
|
||||
while !rest.is_empty() {
|
||||
let equals = rest.find('=')?;
|
||||
let name = rest[..equals].trim();
|
||||
if name.is_empty() {
|
||||
return None;
|
||||
}
|
||||
rest = rest[equals + 1..].trim_start();
|
||||
let (value, next) = if let Some(quoted) = rest.strip_prefix('"') {
|
||||
parse_quoted_value(quoted)?
|
||||
} else {
|
||||
let end = rest.find(char::is_whitespace).unwrap_or(rest.len());
|
||||
(rest[..end].to_string(), &rest[end..])
|
||||
};
|
||||
attributes.insert(name.to_string(), value);
|
||||
rest = next.trim_start();
|
||||
}
|
||||
Some(attributes)
|
||||
}
|
||||
|
||||
fn parse_git_action(name: &str, attributes: &str) -> Option<GitActionDirective> {
|
||||
let attrs = parse_attributes(attributes)?;
|
||||
let cwd = attrs.get("cwd")?.clone();
|
||||
match name {
|
||||
fn parse_git_action(directive: &AssistantDirective<'_>) -> Option<GitActionDirective> {
|
||||
let cwd = directive.attributes.get("cwd")?.to_string();
|
||||
match directive.name {
|
||||
"git-stage" => Some(GitActionDirective::Stage { cwd }),
|
||||
"git-commit" => Some(GitActionDirective::Commit { cwd }),
|
||||
"git-create-branch" => Some(GitActionDirective::CreateBranch {
|
||||
cwd,
|
||||
branch: attrs.get("branch")?.clone(),
|
||||
branch: directive.attributes.get("branch")?.to_string(),
|
||||
}),
|
||||
"git-push" => Some(GitActionDirective::Push {
|
||||
cwd,
|
||||
branch: attrs.get("branch")?.clone(),
|
||||
branch: directive.attributes.get("branch")?.to_string(),
|
||||
}),
|
||||
"git-create-pr" => Some(GitActionDirective::CreatePr {
|
||||
cwd,
|
||||
branch: attrs.get("branch")?.clone(),
|
||||
url: attrs.get("url").cloned(),
|
||||
is_draft: attrs.get("isDraft").is_some_and(|value| value == "true"),
|
||||
branch: directive.attributes.get("branch")?.to_string(),
|
||||
url: directive.attributes.get("url").map(ToString::to_string),
|
||||
is_draft: directive
|
||||
.attributes
|
||||
.get("isDraft")
|
||||
.is_some_and(|value| value == "true"),
|
||||
}),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_attributes(input: &str) -> Option<std::collections::HashMap<String, String>> {
|
||||
let mut attrs = std::collections::HashMap::new();
|
||||
let mut rest = input.trim();
|
||||
while !rest.is_empty() {
|
||||
let eq = rest.find('=')?;
|
||||
let key = rest[..eq].trim();
|
||||
if key.is_empty() {
|
||||
return None;
|
||||
}
|
||||
rest = rest[eq + 1..].trim_start();
|
||||
let (value, next) = if let Some(quoted) = rest.strip_prefix('"') {
|
||||
let end = quoted.find('"')?;
|
||||
(quoted[..end].to_string(), "ed[end + 1..])
|
||||
} else {
|
||||
let end = rest.find(char::is_whitespace).unwrap_or(rest.len());
|
||||
(rest[..end].to_string(), &rest[end..])
|
||||
};
|
||||
attrs.insert(key.to_string(), value);
|
||||
rest = next.trim_start();
|
||||
}
|
||||
Some(attrs)
|
||||
}
|
||||
|
||||
fn parse_quoted_value(input: &str) -> Option<(String, &str)> {
|
||||
let mut value = String::new();
|
||||
let mut characters = input.char_indices().peekable();
|
||||
|
||||
while let Some((index, character)) = characters.next() {
|
||||
if character == '"' {
|
||||
return Some((value, &input[index + 1..]));
|
||||
}
|
||||
match character {
|
||||
'\\' if characters.peek().is_some_and(|(_, next)| *next == '"') => {
|
||||
value.push('"');
|
||||
characters.next();
|
||||
}
|
||||
_ => value.push(character),
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -274,7 +208,7 @@ mod tests {
|
||||
#[test]
|
||||
fn strips_and_parses_git_action_directives() {
|
||||
let parsed = parse_assistant_markdown(
|
||||
"Done\n\n::git-stage{cwd=\"/repo\"} ::git-push{cwd=\"/repo\" branch=\"feat/x\"} ::git-stage{cwd=\"C:\\repo\\\"}",
|
||||
"Done\n\n::git-stage{cwd=\"/repo\"} ::git-push{cwd=\"/repo\" branch=\"feat/x\"} ::git-stage{cwd=\"C:\\repo\\\"} ::git-create-branch{cwd=\"/tmp/repo\\\" branch=\"feat\"}",
|
||||
Path::new("/repo"),
|
||||
);
|
||||
|
||||
@@ -292,6 +226,10 @@ mod tests {
|
||||
GitActionDirective::Stage {
|
||||
cwd: "C:\\repo\\".to_string(),
|
||||
},
|
||||
GitActionDirective::CreateBranch {
|
||||
cwd: "/tmp/repo\\".to_string(),
|
||||
branch: "feat".to_string(),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
@@ -304,25 +242,49 @@ mod tests {
|
||||
assert!(parsed.git_actions.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_unquoted_git_flags_and_quoted_closing_braces() {
|
||||
let cwd = Path::new("/repo");
|
||||
let parsed = parse_assistant_markdown(
|
||||
r#"Done ::git-create-pr{cwd="C:\repo" branch="feature/{rollout}" isDraft=true}"#,
|
||||
cwd,
|
||||
);
|
||||
insta::assert_snapshot!(parsed.visible_markdown, @"Done");
|
||||
assert_eq!(
|
||||
parsed.git_actions,
|
||||
vec![GitActionDirective::CreatePr {
|
||||
cwd: r"C:\repo".to_string(),
|
||||
branch: "feature/{rollout}".to_string(),
|
||||
url: None,
|
||||
is_draft: true,
|
||||
}],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_code_comment_directives_as_markdown() {
|
||||
let parsed = parse_assistant_markdown(
|
||||
concat!(
|
||||
"Found two issues.\n\n",
|
||||
r#"::code-comment{title="Fix body= parsing" body="Keep role=\"tab\", ::git-stage{cwd=/tmp}, file=, and \n literal." file="/repo/src/app.ts" start=10 end=12 priority="P2"}"#,
|
||||
r#"::code-comment{title="Fix body= parsing" body="C:\temp says \"foo\" here; keep role=\"tab\", ::git-stage{cwd=/tmp}, file=, and \n literal." file="/repo/src/app.ts" start=10 end=12 priority="P2"} ::git-stage{cwd="/repo"}"#,
|
||||
"\n\n",
|
||||
r#":::code-comment{title="[P1] Clamp the range" body="The line range should match the App." file="codex/src/range.ts" start=8 end=2 priority=3}"#,
|
||||
r#":::code-comment{title='[P1] Clamp the range' body='The line range should match the App.' file='codex/src/range.ts' start=8 end=2 priority=3}"#,
|
||||
),
|
||||
Path::new("/repo"),
|
||||
);
|
||||
|
||||
insta::assert_snapshot!("code_comment_directive_fallback", parsed.visible_markdown);
|
||||
assert!(parsed.git_actions.is_empty());
|
||||
assert_eq!(
|
||||
parsed.git_actions,
|
||||
vec![GitActionDirective::Stage {
|
||||
cwd: "/repo".to_string()
|
||||
}],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_non_directive_and_malformed_code_comment_text() {
|
||||
let markdown = "Mention `::code-comment{title=\"Example\"}` inline.\n::code-comment{title=\"Missing body\" file=\"/repo/src/app.ts\"}";
|
||||
let markdown = "Mention `::code-comment{title=\"Example\"}` and `::git-push}` inline.\n::code-comment{title=\"Missing body\" file=\"/repo/src/app.ts\"}";
|
||||
let parsed = parse_assistant_markdown(markdown, Path::new("/repo"));
|
||||
|
||||
assert_eq!(parsed.visible_markdown, markdown);
|
||||
|
||||
@@ -123,6 +123,7 @@ mod experimental_features;
|
||||
mod permission_discovery;
|
||||
mod pets;
|
||||
pub use custom_terminal::Terminal;
|
||||
mod assistant_directives;
|
||||
mod auto_review_denials;
|
||||
mod cwd_prompt;
|
||||
mod debug_config;
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
//! canonical target to avoid duplicate file references.
|
||||
//!
|
||||
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_string::normalize_markdown_hash_location_suffix;
|
||||
use regex_lite::Regex;
|
||||
use std::path::Path;
|
||||
@@ -85,21 +86,7 @@ fn comparable_local_link_path(text: &str) -> Option<String> {
|
||||
} else {
|
||||
std::borrow::Cow::Borrowed(text)
|
||||
};
|
||||
let (mut path, _) = parse_local_link_target(&text)?;
|
||||
if text.starts_with("file://") {
|
||||
let url = Url::parse(&text).ok()?;
|
||||
// The display parser's fallback preserves URL escapes on platforms that cannot convert
|
||||
// this URL to a native path. Decode that fallback exactly once for comparison as well.
|
||||
if url.to_file_path().is_err() {
|
||||
path = urlencoding::decode(&path)
|
||||
.unwrap_or(std::borrow::Cow::Borrowed(&path))
|
||||
.into_owned();
|
||||
}
|
||||
// Unix URL conversion retains the slash before a Windows drive; ignore it here only.
|
||||
if matches!(path.as_bytes(), [b'/', drive, b':', b'/', ..] if drive.is_ascii_alphabetic()) {
|
||||
path.remove(0);
|
||||
}
|
||||
}
|
||||
let (path, _) = parse_local_link_target(&text)?;
|
||||
Some(normalize_local_link_path_text(&path).to_lowercase())
|
||||
}
|
||||
|
||||
@@ -127,7 +114,9 @@ pub(super) fn render_local_link_target(dest_url: &str, cwd: Option<&Path>) -> Op
|
||||
fn parse_local_link_target(dest_url: &str) -> Option<(String, Option<String>)> {
|
||||
if dest_url.starts_with("file://") {
|
||||
let url = Url::parse(dest_url).ok()?;
|
||||
let path_text = file_url_to_local_path_text(&url)?;
|
||||
// Infer the path's OS from the URI, not the host running the TUI.
|
||||
let path = PathUri::parse(&url[..url::Position::AfterPath]).ok()?;
|
||||
let path_text = normalize_local_link_path_text(&path.inferred_native_path_string());
|
||||
let location_suffix = url
|
||||
.fragment()
|
||||
.and_then(normalize_hash_location_suffix_fragment);
|
||||
@@ -175,41 +164,13 @@ fn normalize_hash_location_suffix_fragment(fragment: &str) -> Option<String> {
|
||||
///
|
||||
/// The suffix must occur at the end of the input; embedded colons elsewhere in the path are left
|
||||
/// alone. This is what keeps Windows drive letters like `C:/...` from being misread as locations.
|
||||
fn extract_colon_location_suffix(path_text: &str) -> Option<String> {
|
||||
pub(super) fn extract_colon_location_suffix(path_text: &str) -> Option<String> {
|
||||
COLON_LOCATION_SUFFIX_RE
|
||||
.find(path_text)
|
||||
.filter(|matched| matched.end() == path_text.len())
|
||||
.map(|matched| matched.as_str().to_string())
|
||||
}
|
||||
|
||||
/// Convert a `file://` URL into the normalized local-path text used for transcript rendering.
|
||||
///
|
||||
/// This prefers `Url::to_file_path()` for standard file URLs. When that rejects Windows-oriented
|
||||
/// encodings, we reconstruct a display path from the host/path parts so UNC paths and drive-letter
|
||||
/// URLs still render sensibly.
|
||||
fn file_url_to_local_path_text(url: &Url) -> Option<String> {
|
||||
if let Ok(path) = url.to_file_path() {
|
||||
return Some(normalize_local_link_path_text(&path.to_string_lossy()));
|
||||
}
|
||||
|
||||
// Fall back to string reconstruction for cases `to_file_path()` rejects, especially UNC-style
|
||||
// hosts and Windows drive paths encoded in URL form.
|
||||
let mut path_text = url.path().to_string();
|
||||
if let Some(host) = url.host_str()
|
||||
&& !host.is_empty()
|
||||
&& host != "localhost"
|
||||
{
|
||||
path_text = format!("//{host}{path_text}");
|
||||
} else if matches!(
|
||||
path_text.as_bytes(),
|
||||
[b'/', drive, b':', b'/', ..] if drive.is_ascii_alphabetic()
|
||||
) {
|
||||
path_text.remove(0);
|
||||
}
|
||||
|
||||
Some(normalize_local_link_path_text(&path_text))
|
||||
}
|
||||
|
||||
/// Normalize local-path text into the transcript display form.
|
||||
///
|
||||
/// Display normalization is intentionally lexical: it does not touch the filesystem, resolve
|
||||
|
||||
@@ -983,6 +983,27 @@ fn file_link_keeps_absolute_paths_outside_cwd() {
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_links_preserve_foreign_windows_paths_and_anchors_snapshot() {
|
||||
let rendered = [
|
||||
("file:///C:/repo/Quarterly%20Report.xlsx", "C:/repo"),
|
||||
(
|
||||
"file://server/share/Quarterly%20Report.xlsx",
|
||||
"//server/share",
|
||||
),
|
||||
]
|
||||
.map(|(destination, cwd)| {
|
||||
render_markdown_text_for_cwd(&format!("[report]({destination}#L4C2)"), Path::new(cwd))
|
||||
.to_string()
|
||||
})
|
||||
.join("\n");
|
||||
|
||||
insta::assert_snapshot!(rendered, @r"
|
||||
report (Quarterly Report.xlsx:4:2)
|
||||
report (Quarterly Report.xlsx:4:2)
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_link_appends_hash_anchor_when_label_lacks_it() {
|
||||
let text = render_markdown_text_for_cwd(
|
||||
|
||||
+1
-1
@@ -5,7 +5,7 @@ expression: parsed.visible_markdown
|
||||
Found two issues.
|
||||
|
||||
- [P2] Fix body= parsing — src/app.ts:10-12
|
||||
Keep role="tab", ::git-stage{cwd=/tmp}, file=, and \n literal.
|
||||
C:\temp says "foo" here; keep role="tab", ::git-stage{cwd=/tmp}, file=, and \n literal.
|
||||
|
||||
- [P1] Clamp the range — codex/src/range.ts:8
|
||||
The line range should match the App.
|
||||
|
||||
Reference in New Issue
Block a user