Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/webcodex-core/src/runner_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4669,8 +4669,8 @@ mod filter_canonical_tests {

#[test]
fn cargo_value_contract_normalizes_exactly_once_and_fails_closed() {
// The shared normalization contract used by both the synchronous
// command builders and the structured Job argv builder.
// The shared normalization contract used by adapter-owned validation
// plans and Runner-side canonical-step validation.
assert_eq!(
normalize_cargo_value("serde").unwrap(),
Some("serde".to_string())
Expand Down
12 changes: 6 additions & 6 deletions crates/webcodex-core/src/runner_protocol/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,9 @@ fn is_canonical_cargo_test_args(args: &[&str]) -> bool {
}

/// Normalize and validate one value-taking Cargo argument (`--features`,
/// `-p`). This is the single shared contract used by the synchronous command
/// builders and the structured long-Job argv builder, so a given request runs
/// identical arguments no matter how long it takes.
/// `-p`). This is the single shared normalization contract used by validation
/// planning and Runner-side canonical-step validation, so command-text and
/// structured-argv projections preserve identical argument semantics.
///
/// Applies exactly one leading/trailing whitespace trim, then rejects values
/// that are NUL/control-containing, longer than [`CARGO_VALUE_MAX_BYTES`],
Expand Down Expand Up @@ -523,9 +523,9 @@ fn node_script_allowed(kind: &str, script: &str) -> bool {

/// Normalize and validate the single argv value that may follow `cargo test`:
/// a libtest name substring, never a Cargo option. This is the shared contract
/// used by the planner (`safe_rust_filter`), the synchronous command builder,
/// and the structured long-Job argv builder, so a given filter runs identically
/// regardless of runtime path.
/// used by the recipe planner (`safe_rust_filter`), adapter-owned validation
/// planning, and Runner-side canonical-step validation, so a given filter keeps
/// identical semantics regardless of runtime path.
///
/// Applies exactly one leading/trailing trim and rejects control bytes,
/// over-long values, and anything that begins with `-` after trimming, so a
Expand Down
31 changes: 21 additions & 10 deletions crates/webcodex-validation/src/adapters/go.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use super::{ValidationAdapter, ValidationCommandOptions, ValidationFailureEvidence};
use super::{
read_only_validation_plan, ReadOnlyValidationPlan, ValidationAdapter, ValidationCommandOptions,
ValidationFailureEvidence, ValidationPlanArg,
};
use webcodex_core::runner_protocol::normalize_go_test_packages;
use webcodex_core::shell_quote::shell_escape_simple;
use webcodex_core::validation_evidence::{parse_go_test_diagnostics, ValidationDiagnostics};

struct GoTestValidationAdapter;
Expand All @@ -20,7 +22,10 @@ impl ValidationAdapter for GoTestValidationAdapter {
"go_test"
}

fn build_command(&self, options: ValidationCommandOptions) -> Result<String, String> {
fn build_readonly_plan(
&self,
options: ValidationCommandOptions,
) -> Result<ReadOnlyValidationPlan, String> {
if options.check
|| options.filter.is_some()
|| options.lib.is_some()
Expand All @@ -33,14 +38,20 @@ impl ValidationAdapter for GoTestValidationAdapter {
{
return Err("go_test does not accept Cargo validation command options".to_string());
}
let Some(packages) = options.go_packages.as_deref() else {
return Ok("go test -json ./...".to_string());
};
let packages = normalize_go_test_packages(Some(packages))
let explicit_packages = options.go_packages.is_some();
let packages = normalize_go_test_packages(options.go_packages.as_deref())
.map_err(|reason| format!("packages {reason}"))?;
let mut command = vec!["go".to_string(), "test".to_string(), "-json".to_string()];
command.extend(packages.iter().map(|package| shell_escape_simple(package)));
Ok(command.join(" "))
let mut args = vec![
ValidationPlanArg::Literal("test"),
ValidationPlanArg::Literal("-json"),
];
if explicit_packages {
args.extend(packages.into_iter().map(ValidationPlanArg::Value));
} else {
debug_assert_eq!(packages.as_slice(), ["./..."]);
args.push(ValidationPlanArg::Literal("./..."));
}
read_only_validation_plan("test", "go", args)
}

fn parse(
Expand Down
70 changes: 69 additions & 1 deletion crates/webcodex-validation/src/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
mod go;
mod rust;

use webcodex_core::runner_protocol::ShellJobValidationStep;
use webcodex_core::shell_quote::shell_escape_simple;
use webcodex_core::validation_evidence::ValidationDiagnostics;
use webcodex_core::workflow_session_contract::ExecutionPurpose;

Expand All @@ -24,6 +26,64 @@ pub struct ValidationCommandOptions {
pub go_packages: Option<Vec<String>>,
}

/// Adapter-owned, read-only validation plan.
///
/// `structured_step` is the canonical execution representation. The command
/// string is only the compatibility projection required by the existing
/// synchronous capture path; it is derived from the same typed arguments and
/// must never be parsed back into execution authority.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReadOnlyValidationPlan {
pub compatibility_command: String,
pub structured_step: ShellJobValidationStep,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub(super) enum ValidationPlanArg {
Literal(&'static str),
Value(String),
}

impl ValidationPlanArg {
fn raw(&self) -> String {
match self {
Self::Literal(value) => (*value).to_string(),
Self::Value(value) => value.clone(),
}
}

fn rendered(&self) -> String {
match self {
Self::Literal(value) => (*value).to_string(),
Self::Value(value) => shell_escape_simple(value),
}
}
}

pub(super) fn read_only_validation_plan(
name: &'static str,
program: &'static str,
args: Vec<ValidationPlanArg>,
) -> Result<ReadOnlyValidationPlan, String> {
let structured_step = ShellJobValidationStep {
name: name.to_string(),
program: program.to_string(),
args: args.iter().map(ValidationPlanArg::raw).collect(),
env: Vec::new(),
};
if !structured_step.is_canonical() {
return Err("structured validation step is not canonical".to_string());
}
let compatibility_command = std::iter::once(program.to_string())
.chain(args.iter().map(ValidationPlanArg::rendered))
.collect::<Vec<_>>()
.join(" ");
Ok(ReadOnlyValidationPlan {
compatibility_command,
structured_step,
})
}

pub struct ValidationFailureEvidence<'a> {
pub success: bool,
pub reported_failure_kind: Option<&'a str>,
Expand All @@ -38,7 +98,15 @@ pub trait ValidationAdapter: Sync {

fn tool_identity(&self) -> &'static str;

fn build_command(&self, options: ValidationCommandOptions) -> Result<String, String>;
fn build_readonly_plan(
&self,
options: ValidationCommandOptions,
) -> Result<ReadOnlyValidationPlan, String>;

fn build_command(&self, options: ValidationCommandOptions) -> Result<String, String> {
self.build_readonly_plan(options)
.map(|plan| plan.compatibility_command)
}

fn parse(
&self,
Expand Down
103 changes: 62 additions & 41 deletions crates/webcodex-validation/src/adapters/rust.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use super::{ValidationAdapter, ValidationCommandOptions, ValidationFailureEvidence};
use super::{
read_only_validation_plan, ReadOnlyValidationPlan, ValidationAdapter, ValidationCommandOptions,
ValidationFailureEvidence, ValidationPlanArg,
};
use webcodex_core::runner_protocol::{
normalize_cargo_packages, normalize_cargo_value, normalize_rust_test_filter,
};
use webcodex_core::shell_quote::shell_escape_simple;
use webcodex_core::validation_evidence::{
parse_cargo_check_diagnostics, parse_cargo_test_diagnostics, ValidationDiagnostics,
};
Expand Down Expand Up @@ -54,17 +56,44 @@ impl ValidationAdapter for RustValidationAdapter {
self.tool_identity
}

fn build_command(&self, options: ValidationCommandOptions) -> Result<String, String> {
fn build_readonly_plan(
&self,
options: ValidationCommandOptions,
) -> Result<ReadOnlyValidationPlan, String> {
if options.go_packages.is_some() {
return Err("Cargo validation does not accept go_test packages".to_string());
}
match self.kind {
RustAdapterKind::Format => Ok(cargo_fmt_command(options.check)),
RustAdapterKind::Check => cargo_check_command(options),
RustAdapterKind::Test => cargo_test_command(options),
RustAdapterKind::Format => {
if !options.check {
return Err("cargo_fmt mutation is not read-only validation".to_string());
}
read_only_validation_plan(
"format",
"cargo",
vec![
ValidationPlanArg::Literal("fmt"),
ValidationPlanArg::Literal("--"),
ValidationPlanArg::Literal("--check"),
],
)
}
RustAdapterKind::Check => cargo_check_plan(options),
RustAdapterKind::Test => cargo_test_plan(options),
}
}

fn build_command(&self, options: ValidationCommandOptions) -> Result<String, String> {
if self.kind == RustAdapterKind::Format && !options.check {
if options.go_packages.is_some() {
return Err("Cargo validation does not accept go_test packages".to_string());
}
return Ok("cargo fmt".to_string());
}
self.build_readonly_plan(options)
.map(|plan| plan.compatibility_command)
}

fn parse(
&self,
stdout_excerpt: &str,
Expand Down Expand Up @@ -148,83 +177,75 @@ impl ValidationAdapter for RustValidationAdapter {
}
}

fn cargo_fmt_command(check: bool) -> String {
if check {
"cargo fmt -- --check".to_string()
} else {
"cargo fmt".to_string()
}
}

fn cargo_check_command(options: ValidationCommandOptions) -> Result<String, String> {
fn cargo_check_plan(options: ValidationCommandOptions) -> Result<ReadOnlyValidationPlan, String> {
let features = validate_arg("features", options.features)?;
let packages = normalize_cargo_packages(
options.package.as_deref(),
options.cargo_packages.as_deref(),
)
.map_err(|reason| format!("packages {reason}"))?;
let mut args = vec!["cargo".to_string(), "check".to_string()];
let mut args = vec![ValidationPlanArg::Literal("check")];
if options.all_targets.unwrap_or(true) {
args.push("--all-targets".to_string());
args.push(ValidationPlanArg::Literal("--all-targets"));
}
if options.all_features.unwrap_or(false) {
args.push("--all-features".to_string());
args.push(ValidationPlanArg::Literal("--all-features"));
}
if options.no_default_features.unwrap_or(false) {
args.push("--no-default-features".to_string());
args.push(ValidationPlanArg::Literal("--no-default-features"));
}
if let Some(features) = features {
args.push("--features".to_string());
args.push(shell_escape_simple(&features));
args.push(ValidationPlanArg::Literal("--features"));
args.push(ValidationPlanArg::Value(features));
}
for package in packages.into_iter().flatten() {
args.push("-p".to_string());
args.push(shell_escape_simple(&package));
args.push(ValidationPlanArg::Literal("-p"));
args.push(ValidationPlanArg::Value(package));
}
Ok(args.join(" "))
read_only_validation_plan("check", "cargo", args)
}

fn cargo_test_command(options: ValidationCommandOptions) -> Result<String, String> {
fn cargo_test_plan(options: ValidationCommandOptions) -> Result<ReadOnlyValidationPlan, String> {
if options.cargo_packages.is_some() {
return Err("cargo_test does not accept cargo_check packages".to_string());
}
let filter = validate_filter(options.filter)?;
let features = validate_arg("features", options.features)?;
let package = validate_arg("package", options.package)?;
let mut args = vec!["cargo".to_string(), "test".to_string()];
let mut args = vec![ValidationPlanArg::Literal("test")];
if let Some(filter) = filter {
args.push(shell_escape_simple(&filter));
args.push(ValidationPlanArg::Value(filter));
}
if options.lib.unwrap_or(false) {
args.push("--lib".to_string());
args.push(ValidationPlanArg::Literal("--lib"));
}
if options.all_targets.unwrap_or(false) {
args.push("--all-targets".to_string());
args.push(ValidationPlanArg::Literal("--all-targets"));
}
if options.all_features.unwrap_or(false) {
args.push("--all-features".to_string());
args.push(ValidationPlanArg::Literal("--all-features"));
}
if options.no_default_features.unwrap_or(false) {
args.push("--no-default-features".to_string());
args.push(ValidationPlanArg::Literal("--no-default-features"));
}
if let Some(features) = features {
args.push("--features".to_string());
args.push(shell_escape_simple(&features));
args.push(ValidationPlanArg::Literal("--features"));
args.push(ValidationPlanArg::Value(features));
}
if let Some(package) = package {
args.push("-p".to_string());
args.push(shell_escape_simple(&package));
args.push(ValidationPlanArg::Literal("-p"));
args.push(ValidationPlanArg::Value(package));
}
if options.no_run.unwrap_or(false) {
args.push("--no-run".to_string());
args.push(ValidationPlanArg::Literal("--no-run"));
}
Ok(args.join(" "))
read_only_validation_plan("test", "cargo", args)
}

/// Normalize one value-taking Cargo option using the same shared contract the
/// structured Job argv builder uses, so a request produces identical
/// arguments whether it runs synchronously or as a long Job. The error label
/// is mapped to the tool-facing message.
/// Normalize one value-taking Cargo option before the adapter emits its single
/// read-only validation plan. The command-text and structured-argv projections
/// are then derived from that same normalized value. The error label is mapped
/// to the tool-facing message.
fn validate_arg(label: &str, value: Option<String>) -> Result<Option<String>, String> {
match value {
Some(raw) => match normalize_cargo_value(&raw) {
Expand Down
4 changes: 2 additions & 2 deletions crates/webcodex-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ mod profile_tests;
mod recipe_tests;

pub use adapters::{
execution_purpose_for_validation_kind, validation_adapter_for_tool, ValidationAdapter,
ValidationCommandOptions, ValidationFailureEvidence,
execution_purpose_for_validation_kind, validation_adapter_for_tool, ReadOnlyValidationPlan,
ValidationAdapter, ValidationCommandOptions, ValidationFailureEvidence,
};
pub use evidence::{
current_validation_evidence_for_session, event_is_job_acceptance_only,
Expand Down
26 changes: 26 additions & 0 deletions crates/webcodex-validation/src/profile_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,32 @@ fn rust_profile_selects_cargo_fmt_adapter_and_preserves_command() {
);
}

#[test]
fn cargo_fmt_read_only_plan_excludes_mutating_format() {
let adapter = validation_adapter_for_tool("cargo_fmt").expect("cargo_fmt adapter");
let readonly = adapter
.build_readonly_plan(ValidationCommandOptions {
check: true,
..ValidationCommandOptions::default()
})
.unwrap();
assert_eq!(readonly.compatibility_command, "cargo fmt -- --check");
assert_eq!(readonly.structured_step.name, "format");
assert_eq!(readonly.structured_step.program, "cargo");
assert_eq!(readonly.structured_step.args, ["fmt", "--", "--check"]);
assert!(readonly.structured_step.is_canonical());

assert!(adapter
.build_readonly_plan(ValidationCommandOptions::default())
.is_err());
assert_eq!(
adapter
.build_command(ValidationCommandOptions::default())
.unwrap(),
"cargo fmt"
);
}

#[test]
fn rust_profile_selects_cargo_check_adapter_and_preserves_command() {
let adapter = validation_adapter_for_tool("cargo_check").expect("cargo_check adapter");
Expand Down
Loading
Loading