core-agent-ide/codex-rs/execpolicy/src/error.rs

102 lines
2.8 KiB
Rust
Raw Normal View History

use starlark::Error as StarlarkError;
use thiserror::Error;
pub type Result<T> = std::result::Result<T, Error>;
Another round of improvements for config error messages (#9746) In a [recent PR](https://github.com/openai/codex/pull/9182), I made some improvements to config error messages so errors didn't leave app server clients in a dead state. This is a follow-on PR to make these error messages more readable and actionable for both TUI and GUI users. For example, see #9668 where the user was understandably confused about the source of the problem and how to fix it. The improved error message: 1. Clearly identifies the config file where the error was found (which is more important now that we support layered configs) 2. Provides a line and column number of the error 3. Displays the line where the error occurred and underlines it For example, if my `config.toml` includes the following: ```toml [features] collaboration_modes = "true" ``` Here's the current CLI error message: ``` Error loading config.toml: invalid type: string "true", expected a boolean in `features` ``` And here's the improved message: ``` Error loading config.toml: /Users/etraut/.codex/config.toml:43:23: invalid type: string "true", expected a boolean | 43 | collaboration_modes = "true" | ^^^^^^ ``` The bulk of the new logic is contained within a new module `config_loader/diagnostics.rs` that is responsible for calculating the text range for a given toml path (which is more involved than I would have expected). In addition, this PR adds the file name and text range to the `ConfigWarningNotification` app server struct. This allows GUI clients to present the user with a better error message and an optional link to open the errant config file. This was a suggestion from @.bolinfest when he reviewed my previous PR.
2026-01-23 20:11:09 -08:00
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct TextPosition {
pub line: usize,
pub column: usize,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct TextRange {
pub start: TextPosition,
pub end: TextPosition,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ErrorLocation {
pub path: String,
pub range: TextRange,
}
#[derive(Debug, Error)]
pub enum Error {
#[error("invalid decision: {0}")]
InvalidDecision(String),
#[error("invalid pattern element: {0}")]
InvalidPattern(String),
#[error("invalid example: {0}")]
InvalidExample(String),
#[error("invalid rule: {0}")]
InvalidRule(String),
#[error(
"expected every example to match at least one rule. rules: {rules:?}; unmatched examples: \
{examples:?}"
)]
ExampleDidNotMatch {
rules: Vec<String>,
examples: Vec<String>,
execpolicy: add host_executable() path mappings (#12964) ## Why `execpolicy` currently keys `prefix_rule()` matching off the literal first token. That works for rules like `["/usr/bin/git"]`, but it means shared basename rules such as `["git"]` do not help when a caller passes an absolute executable path like `/usr/bin/git`. This PR lays the groundwork for basename-aware matching without changing existing callers yet. It adds typed host-executable metadata and an opt-in resolution path in `codex-execpolicy`, so a follow-up PR can adopt the new behavior in `unix_escalation.rs` and other call sites without having to redesign the policy layer first. ## What Changed - added `host_executable(name = ..., paths = [...])` to the execpolicy parser and validated it with `AbsolutePathBuf` - stored host executable mappings separately from prefix rules inside `Policy` - added `MatchOptions` and opt-in `*_with_options()` APIs that preserve existing behavior by default - implemented exact-first matching with optional basename fallback, gated by `host_executable()` allowlists when present - normalized executable names for cross-platform matching so Windows paths like `git.exe` can satisfy `host_executable(name = "git", ...)` - updated `match` / `not_match` example validation to exercise the host-executable resolution path instead of only raw prefix-rule matching - preserved source locations for deferred example-validation errors so policy load failures still point at the right file and line - surfaced `resolvedProgram` on `RuleMatch` so callers can tell when a basename rule matched an absolute executable path - preserved host executable metadata when requirements policies overlay file-based policies in `core/src/exec_policy.rs` - documented the new rule shape and CLI behavior in `execpolicy/README.md` ## Verification - `cargo test -p codex-execpolicy` - added coverage in `execpolicy/tests/basic.rs` for parsing, precedence, empty allowlists, basename fallback, exact-match precedence, and host-executable-backed `match` / `not_match` examples - added a regression test in `core/src/exec_policy.rs` to verify requirements overlays preserve `host_executable()` metadata - verified `cargo test -p codex-core --lib`, including source-rendering coverage for deferred validation errors
2026-02-27 12:59:24 -08:00
location: Option<ErrorLocation>,
},
#[error("expected example to not match rule `{rule}`: {example}")]
execpolicy: add host_executable() path mappings (#12964) ## Why `execpolicy` currently keys `prefix_rule()` matching off the literal first token. That works for rules like `["/usr/bin/git"]`, but it means shared basename rules such as `["git"]` do not help when a caller passes an absolute executable path like `/usr/bin/git`. This PR lays the groundwork for basename-aware matching without changing existing callers yet. It adds typed host-executable metadata and an opt-in resolution path in `codex-execpolicy`, so a follow-up PR can adopt the new behavior in `unix_escalation.rs` and other call sites without having to redesign the policy layer first. ## What Changed - added `host_executable(name = ..., paths = [...])` to the execpolicy parser and validated it with `AbsolutePathBuf` - stored host executable mappings separately from prefix rules inside `Policy` - added `MatchOptions` and opt-in `*_with_options()` APIs that preserve existing behavior by default - implemented exact-first matching with optional basename fallback, gated by `host_executable()` allowlists when present - normalized executable names for cross-platform matching so Windows paths like `git.exe` can satisfy `host_executable(name = "git", ...)` - updated `match` / `not_match` example validation to exercise the host-executable resolution path instead of only raw prefix-rule matching - preserved source locations for deferred example-validation errors so policy load failures still point at the right file and line - surfaced `resolvedProgram` on `RuleMatch` so callers can tell when a basename rule matched an absolute executable path - preserved host executable metadata when requirements policies overlay file-based policies in `core/src/exec_policy.rs` - documented the new rule shape and CLI behavior in `execpolicy/README.md` ## Verification - `cargo test -p codex-execpolicy` - added coverage in `execpolicy/tests/basic.rs` for parsing, precedence, empty allowlists, basename fallback, exact-match precedence, and host-executable-backed `match` / `not_match` examples - added a regression test in `core/src/exec_policy.rs` to verify requirements overlays preserve `host_executable()` metadata - verified `cargo test -p codex-core --lib`, including source-rendering coverage for deferred validation errors
2026-02-27 12:59:24 -08:00
ExampleDidMatch {
rule: String,
example: String,
location: Option<ErrorLocation>,
},
#[error("starlark error: {0}")]
Starlark(StarlarkError),
}
Another round of improvements for config error messages (#9746) In a [recent PR](https://github.com/openai/codex/pull/9182), I made some improvements to config error messages so errors didn't leave app server clients in a dead state. This is a follow-on PR to make these error messages more readable and actionable for both TUI and GUI users. For example, see #9668 where the user was understandably confused about the source of the problem and how to fix it. The improved error message: 1. Clearly identifies the config file where the error was found (which is more important now that we support layered configs) 2. Provides a line and column number of the error 3. Displays the line where the error occurred and underlines it For example, if my `config.toml` includes the following: ```toml [features] collaboration_modes = "true" ``` Here's the current CLI error message: ``` Error loading config.toml: invalid type: string "true", expected a boolean in `features` ``` And here's the improved message: ``` Error loading config.toml: /Users/etraut/.codex/config.toml:43:23: invalid type: string "true", expected a boolean | 43 | collaboration_modes = "true" | ^^^^^^ ``` The bulk of the new logic is contained within a new module `config_loader/diagnostics.rs` that is responsible for calculating the text range for a given toml path (which is more involved than I would have expected). In addition, this PR adds the file name and text range to the `ConfigWarningNotification` app server struct. This allows GUI clients to present the user with a better error message and an optional link to open the errant config file. This was a suggestion from @.bolinfest when he reviewed my previous PR.
2026-01-23 20:11:09 -08:00
impl Error {
execpolicy: add host_executable() path mappings (#12964) ## Why `execpolicy` currently keys `prefix_rule()` matching off the literal first token. That works for rules like `["/usr/bin/git"]`, but it means shared basename rules such as `["git"]` do not help when a caller passes an absolute executable path like `/usr/bin/git`. This PR lays the groundwork for basename-aware matching without changing existing callers yet. It adds typed host-executable metadata and an opt-in resolution path in `codex-execpolicy`, so a follow-up PR can adopt the new behavior in `unix_escalation.rs` and other call sites without having to redesign the policy layer first. ## What Changed - added `host_executable(name = ..., paths = [...])` to the execpolicy parser and validated it with `AbsolutePathBuf` - stored host executable mappings separately from prefix rules inside `Policy` - added `MatchOptions` and opt-in `*_with_options()` APIs that preserve existing behavior by default - implemented exact-first matching with optional basename fallback, gated by `host_executable()` allowlists when present - normalized executable names for cross-platform matching so Windows paths like `git.exe` can satisfy `host_executable(name = "git", ...)` - updated `match` / `not_match` example validation to exercise the host-executable resolution path instead of only raw prefix-rule matching - preserved source locations for deferred example-validation errors so policy load failures still point at the right file and line - surfaced `resolvedProgram` on `RuleMatch` so callers can tell when a basename rule matched an absolute executable path - preserved host executable metadata when requirements policies overlay file-based policies in `core/src/exec_policy.rs` - documented the new rule shape and CLI behavior in `execpolicy/README.md` ## Verification - `cargo test -p codex-execpolicy` - added coverage in `execpolicy/tests/basic.rs` for parsing, precedence, empty allowlists, basename fallback, exact-match precedence, and host-executable-backed `match` / `not_match` examples - added a regression test in `core/src/exec_policy.rs` to verify requirements overlays preserve `host_executable()` metadata - verified `cargo test -p codex-core --lib`, including source-rendering coverage for deferred validation errors
2026-02-27 12:59:24 -08:00
pub fn with_location(self, location: ErrorLocation) -> Self {
match self {
Error::ExampleDidNotMatch {
rules,
examples,
location: None,
} => Error::ExampleDidNotMatch {
rules,
examples,
location: Some(location),
},
Error::ExampleDidMatch {
rule,
example,
location: None,
} => Error::ExampleDidMatch {
rule,
example,
location: Some(location),
},
other => other,
}
}
Another round of improvements for config error messages (#9746) In a [recent PR](https://github.com/openai/codex/pull/9182), I made some improvements to config error messages so errors didn't leave app server clients in a dead state. This is a follow-on PR to make these error messages more readable and actionable for both TUI and GUI users. For example, see #9668 where the user was understandably confused about the source of the problem and how to fix it. The improved error message: 1. Clearly identifies the config file where the error was found (which is more important now that we support layered configs) 2. Provides a line and column number of the error 3. Displays the line where the error occurred and underlines it For example, if my `config.toml` includes the following: ```toml [features] collaboration_modes = "true" ``` Here's the current CLI error message: ``` Error loading config.toml: invalid type: string "true", expected a boolean in `features` ``` And here's the improved message: ``` Error loading config.toml: /Users/etraut/.codex/config.toml:43:23: invalid type: string "true", expected a boolean | 43 | collaboration_modes = "true" | ^^^^^^ ``` The bulk of the new logic is contained within a new module `config_loader/diagnostics.rs` that is responsible for calculating the text range for a given toml path (which is more involved than I would have expected). In addition, this PR adds the file name and text range to the `ConfigWarningNotification` app server struct. This allows GUI clients to present the user with a better error message and an optional link to open the errant config file. This was a suggestion from @.bolinfest when he reviewed my previous PR.
2026-01-23 20:11:09 -08:00
pub fn location(&self) -> Option<ErrorLocation> {
match self {
execpolicy: add host_executable() path mappings (#12964) ## Why `execpolicy` currently keys `prefix_rule()` matching off the literal first token. That works for rules like `["/usr/bin/git"]`, but it means shared basename rules such as `["git"]` do not help when a caller passes an absolute executable path like `/usr/bin/git`. This PR lays the groundwork for basename-aware matching without changing existing callers yet. It adds typed host-executable metadata and an opt-in resolution path in `codex-execpolicy`, so a follow-up PR can adopt the new behavior in `unix_escalation.rs` and other call sites without having to redesign the policy layer first. ## What Changed - added `host_executable(name = ..., paths = [...])` to the execpolicy parser and validated it with `AbsolutePathBuf` - stored host executable mappings separately from prefix rules inside `Policy` - added `MatchOptions` and opt-in `*_with_options()` APIs that preserve existing behavior by default - implemented exact-first matching with optional basename fallback, gated by `host_executable()` allowlists when present - normalized executable names for cross-platform matching so Windows paths like `git.exe` can satisfy `host_executable(name = "git", ...)` - updated `match` / `not_match` example validation to exercise the host-executable resolution path instead of only raw prefix-rule matching - preserved source locations for deferred example-validation errors so policy load failures still point at the right file and line - surfaced `resolvedProgram` on `RuleMatch` so callers can tell when a basename rule matched an absolute executable path - preserved host executable metadata when requirements policies overlay file-based policies in `core/src/exec_policy.rs` - documented the new rule shape and CLI behavior in `execpolicy/README.md` ## Verification - `cargo test -p codex-execpolicy` - added coverage in `execpolicy/tests/basic.rs` for parsing, precedence, empty allowlists, basename fallback, exact-match precedence, and host-executable-backed `match` / `not_match` examples - added a regression test in `core/src/exec_policy.rs` to verify requirements overlays preserve `host_executable()` metadata - verified `cargo test -p codex-core --lib`, including source-rendering coverage for deferred validation errors
2026-02-27 12:59:24 -08:00
Error::ExampleDidNotMatch { location, .. }
| Error::ExampleDidMatch { location, .. } => location.clone(),
Another round of improvements for config error messages (#9746) In a [recent PR](https://github.com/openai/codex/pull/9182), I made some improvements to config error messages so errors didn't leave app server clients in a dead state. This is a follow-on PR to make these error messages more readable and actionable for both TUI and GUI users. For example, see #9668 where the user was understandably confused about the source of the problem and how to fix it. The improved error message: 1. Clearly identifies the config file where the error was found (which is more important now that we support layered configs) 2. Provides a line and column number of the error 3. Displays the line where the error occurred and underlines it For example, if my `config.toml` includes the following: ```toml [features] collaboration_modes = "true" ``` Here's the current CLI error message: ``` Error loading config.toml: invalid type: string "true", expected a boolean in `features` ``` And here's the improved message: ``` Error loading config.toml: /Users/etraut/.codex/config.toml:43:23: invalid type: string "true", expected a boolean | 43 | collaboration_modes = "true" | ^^^^^^ ``` The bulk of the new logic is contained within a new module `config_loader/diagnostics.rs` that is responsible for calculating the text range for a given toml path (which is more involved than I would have expected). In addition, this PR adds the file name and text range to the `ConfigWarningNotification` app server struct. This allows GUI clients to present the user with a better error message and an optional link to open the errant config file. This was a suggestion from @.bolinfest when he reviewed my previous PR.
2026-01-23 20:11:09 -08:00
Error::Starlark(err) => err.span().map(|span| {
let resolved = span.resolve_span();
ErrorLocation {
path: span.filename().to_string(),
range: TextRange {
start: TextPosition {
line: resolved.begin.line + 1,
column: resolved.begin.column + 1,
},
end: TextPosition {
line: resolved.end.line + 1,
column: resolved.end.column + 1,
},
},
}
}),
_ => None,
}
}
}