Skip to content
Draft
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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,9 @@ jobs:
env:
CI_RUST_SHARD: ${{ matrix.shard }}
run: python3 scripts/ci_rust_test.py --report target/ci-test-reports/${{ matrix.shard }}.json -- cargo test --locked ${{ matrix.packages }}
- name: Run real Clippy smoke regression
if: matrix.shard == 'runner'
run: cargo test --locked -p webcodex-runner --features runner-real-process-tests --bin webcodex-runner runner_real_process_rust_clippy_warnings_pass_and_correctness_fails -- --ignored
- name: Run experimental Code Mode and receipt tests
if: matrix.shard == 'server'
run: |
Expand Down
16 changes: 13 additions & 3 deletions crates/webcodex-core/src/project_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,18 @@ pub use crate::project_operation::{
pub enum ProjectValidationAction {
FormatCheck,
Check,
/// Run only a Runner-selected Node `lint` script; never fall back to check/typecheck.
/// Run Runner-owned Rust Clippy or a Node `lint` script.
Lint,
Test,
}
impl ProjectValidationAction {
pub fn kind_for_backend(self, backend: &str) -> &'static str {
if self == Self::Lint && backend == "rust" {
"lint"
} else {
self.kind()
}
}
pub fn kind(self) -> &'static str {
match self {
Self::FormatCheck => "format",
Expand Down Expand Up @@ -128,6 +135,9 @@ impl ProjectValidationRequest {
})?;
}
if self.dependency_resolution_policy.is_some() {
if self.action == ProjectValidationAction::Lint {
return Err("dependency_resolution_policy_unsupported: lint offline policy requires maintainer approval".into());
}
if !matches!(
self.action,
ProjectValidationAction::Check | ProjectValidationAction::Test
Expand Down Expand Up @@ -192,8 +202,8 @@ impl ProjectValidationProvenance {
self.request.validate().is_ok()
&& validate_relative(&self.recipe_root).is_ok()
&& matches!(self.backend.as_str(), "rust" | "go" | "python" | "node")
// Explicit lint is Node-only, including retained provenance.
&& (self.request.action != ProjectValidationAction::Lint || self.backend == "node")
// Lint has a closed backend set, including retained provenance.
&& (self.request.action != ProjectValidationAction::Lint || matches!(self.backend.as_str(), "node" | "rust"))
&& (self.request.dependency_resolution_policy.is_none() || self.backend == "rust")
&& [
&self.root_digest,
Expand Down
32 changes: 30 additions & 2 deletions crates/webcodex-core/src/project_validation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ fn project_node_check_step_identity_capability_and_metadata_are_closed() {
}

#[test]
fn explicit_lint_provenance_is_node_only() {
fn explicit_lint_provenance_has_closed_backend_set() {
let mut provenance = ProjectValidationProvenance {
request: ProjectValidationRequest {
project_id: "demo".into(),
Expand All @@ -116,7 +116,13 @@ fn explicit_lint_provenance_is_node_only() {
invocation_digest: "c".repeat(64),
};
assert!(provenance.is_valid());
for backend in ["rust", "go", "python"] {
provenance.backend = "rust".into();
assert!(provenance.is_valid());
provenance.request.dependency_resolution_policy =
Some(ProjectDependencyResolutionPolicy::Offline);
assert!(!provenance.is_valid());
provenance.request.dependency_resolution_policy = None;
for backend in ["go", "python"] {
provenance.backend = backend.into();
assert!(!provenance.is_valid(), "unsupported lint backend {backend}");
}
Expand Down Expand Up @@ -349,6 +355,28 @@ fn project_validation_package_scope_roundtrips_and_is_bounded() {
}
}

#[test]
fn project_validation_locked_policy_accepts_rust_lint() {
for adapter in [
ProjectValidationAdapter::Rust,
ProjectValidationAdapter::Auto,
] {
let request = ProjectValidationRequest {
project_id: "demo".into(),
cwd: None,
action: ProjectValidationAction::Lint,
adapter,
scope: None,
dependency_policy: Some(ProjectDependencyPolicy {
mode: ProjectDependencyMode::Locked,
}),
dependency_resolution_policy: None,
test: None,
};
assert!(request.validate().is_ok());
}
}

#[test]
fn project_validation_locked_policy_rejects_format_check_without_silent_ignore() {
let request = ProjectValidationRequest {
Expand Down
2 changes: 1 addition & 1 deletion crates/webcodex-core/src/runner_job_receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl RetainedJobReceipt {
|| context
.validation_steps
.iter()
.any(|step| !matches!(step.as_str(), "format" | "check" | "test"))
.any(|step| !matches!(step.as_str(), "format" | "check" | "lint" | "test"))
|| snapshot
.validation_progress
.as_ref()
Expand Down
7 changes: 7 additions & 0 deletions crates/webcodex-core/src/runner_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,12 @@ runner_capabilities! {
#[serde(default)]
pub project_validation_v1: bool = false;
}
/// Runner-owned Cargo Clippy lint, including scope and locked policy.
ProjectValidationRustClippy => RUNNER_CAPABILITY_PROJECT_VALIDATION_RUST_CLIPPY("project_validation_rust_clippy_v1"),
v2_baseline = false {
#[serde(default, skip_serializing_if = "is_false")]
pub project_validation_rust_clippy_v1: bool = false;
}
/// Runner-owned project build planning plus typed StartBuild admission.
/// Missing on older Runners is false and must fail closed.
ProjectBuild => RUNNER_CAPABILITY_PROJECT_BUILD("project_build_v1"),
Expand Down Expand Up @@ -2917,6 +2923,7 @@ mod envelope_tests {
project_validation_python_ruff_v1: false,
project_validation_node_script_check_v1: false,
project_validation_node_tap_v1: false,
project_validation_rust_clippy_v1: false,
structured_go_test_tool: true,
structured_go_test_packages: true,
structured_process_argv: true,
Expand Down
42 changes: 38 additions & 4 deletions crates/webcodex-core/src/runner_protocol/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,25 @@ impl ShellJobValidationStep {
let args = self.args.iter().map(String::as_str).collect::<Vec<_>>();
match (self.name.as_str(), self.program.as_str()) {
("format", "cargo") => args == ["fmt", "--", "--check"],
("lint", "cargo") => {
args.starts_with(&["clippy"])
&& args.ends_with(&["--", "-D", "clippy::correctness"])
&& args.len() >= 5
&& args.iter().filter(|arg| **arg == "-p").count() <= 8
&& args[1..args.len() - 3].contains(&"--all-targets")
&& args[1..args.len() - 3].iter().all(|a| {
!matches!(
*a,
"--offline" | "--all-features" | "--no-default-features" | "--features"
)
})
&& is_canonical_cargo_flags(
&args[1..args.len() - 3],
false,
true,
allow_project_workspace,
)
}
("check", "cargo") => is_canonical_cargo_check_args(&args, allow_project_workspace),
("test", "cargo") => is_canonical_cargo_test_args(&args, allow_project_workspace),
("check", "go") => is_canonical_go_vet_args(&args),
Expand Down Expand Up @@ -406,7 +425,7 @@ impl ShellJobValidationStep {

pub fn is_project_workspace_cargo(&self) -> bool {
self.program == "cargo"
&& matches!(self.name.as_str(), "check" | "test")
&& matches!(self.name.as_str(), "check" | "lint" | "test")
&& self.args.iter().any(|arg| arg == "--workspace")
&& self.is_canonical_project_step()
}
Expand Down Expand Up @@ -1003,14 +1022,20 @@ impl ShellJobValidationMetadata {
return false;
};
if !provenance.is_valid()
|| provenance.request.action.kind() != self.kind
|| provenance
.request
.action
.kind_for_backend(&provenance.backend)
!= self.kind
|| (provenance.backend == "rust"
&& project_all_packages
&& !self.steps[0].is_project_workspace_cargo())
|| !matches!(
(provenance.backend.as_str(), self.adapter.as_str()),
("rust", "cargo_fmt" | "cargo_check" | "cargo_test")
| ("go", "go_vet" | "go_test")
(
"rust",
"cargo_fmt" | "cargo_check" | "cargo_clippy" | "cargo_test"
) | ("go", "go_vet" | "go_test")
| (
"python",
"python:pytest:test" | "python:ruff:check" | "python:ruff:format"
Expand Down Expand Up @@ -1055,6 +1080,15 @@ impl ShellJobValidationMetadata {
"cargo_fmt" => {
self.kind == "format" && step.name == "format" && step.program == "cargo"
}
"cargo_clippy" => {
self.tool == "project_validate"
&& self.kind == "lint"
&& step.name == "lint"
&& step.program == "cargo"
&& self.project_validation.as_ref().is_some_and(|p| {
p.request.action == crate::project_validation::ProjectValidationAction::Lint
})
}
"cargo_check" => {
self.kind == "check" && step.name == "check" && step.program == "cargo"
}
Expand Down
17 changes: 17 additions & 0 deletions crates/webcodex-core/src/validation_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub enum ToolValidationIdentityKind {
None,
CargoFmt,
CargoCheck,
CargoClippy,
CargoTest,
GoTest,
GoVet,
Expand All @@ -38,6 +39,7 @@ impl ToolValidationIdentityKind {
Self::None | Self::Project => None,
Self::CargoFmt => Some("cargo_fmt"),
Self::CargoCheck => Some("cargo_check"),
Self::CargoClippy => Some("cargo_clippy"),
Self::CargoTest => Some("cargo_test"),
Self::GoTest => Some("go_test"),
Self::GoVet => Some("go_vet"),
Expand Down Expand Up @@ -150,6 +152,21 @@ pub fn structured_validation_target_identity(
}
semantic
}
ToolValidationIdentityKind::CargoClippy => {
let packages =
normalized_cargo_target_packages(obj.get("package"), obj.get("packages"))?;
let mut semantic = serde_json::json!({
"tool": tool_name, "kind": "lint", "cwd": cwd,
"all_targets": true, "deny": "clippy::correctness",
});
if packages.as_ref().is_some_and(|packages| packages.len() > 1) {
semantic["packages"] = serde_json::json!(packages);
} else {
semantic["package"] =
serde_json::json!(packages.and_then(|mut values| values.pop()));
}
semantic
}
ToolValidationIdentityKind::CargoTest => {
if obj.get("filter_present").and_then(Value::as_bool) == Some(true)
&& obj.get("filter").is_none()
Expand Down
1 change: 1 addition & 0 deletions crates/webcodex-runner-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ pub fn generated_runner_config_toml(opts: &RunnerInitOptions) -> Result<String,
project_validation_python_ruff_v1: false,
project_validation_node_script_check_v1: false,
project_validation_node_tap_v1: false,
project_validation_rust_clippy_v1: false,
// Like JSON parsing, first-class durable go_test support is
// advertised by the running binary, never by generated static config.
structured_go_test_tool: false,
Expand Down
4 changes: 4 additions & 0 deletions crates/webcodex-runner-registry/src/job_updates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,10 @@ impl RunnerRegistry {
.iter()
.any(|step| step.node_json_framework().is_some())
.then_some(RunnerFeature::ProjectValidationNodeJson),
validation_steps
.iter()
.any(|step| step.program == "cargo" && step.name == "lint")
.then_some(RunnerFeature::ProjectValidationRustClippy),
node_test.then_some(RunnerFeature::ProjectValidationNodeTap),
(project_test_options || go_test_filter)
.then_some(RunnerFeature::ProjectValidationTestOptions),
Expand Down
2 changes: 1 addition & 1 deletion crates/webcodex-runner-registry/src/reconciliation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ fn validate_context(
|| context
.validation_steps
.iter()
.any(|step| !matches!(step.as_str(), "format" | "check" | "test"))
.any(|step| !matches!(step.as_str(), "format" | "check" | "lint" | "test"))
{
return Err("job inventory validation_steps are invalid".to_string());
}
Expand Down
6 changes: 6 additions & 0 deletions crates/webcodex-runner-registry/src/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,11 @@ impl RunnerRegistry {
RunnerFeature::ProjectValidationNodeScriptCheck
},
);
let clippy_capability = (payload.action
== webcodex_core::project_validation::ProjectValidationAction::Lint
&& payload.adapter
== webcodex_core::project_validation::ProjectValidationAdapter::Rust)
.then_some(RunnerFeature::ProjectValidationRustClippy);
let requires_package_scope = payload
.scope
.as_ref()
Expand Down Expand Up @@ -2838,6 +2843,7 @@ impl RunnerRegistry {
&runner.runner_features,
[
Some(RunnerFeature::ProjectValidation),
clippy_capability,
python_capability,
node_capability,
requires_all_packages.then_some(RunnerFeature::ProjectAllPackages),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,3 +707,69 @@ async fn project_validation_python_preplan_selects_capability_by_action() {
}
}
}

#[tokio::test]
async fn rust_clippy_old_runner_is_fenced_at_planning_and_job_admission() {
for hint in [
ProjectValidationAdapter::Rust,
ProjectValidationAdapter::Auto,
] {
let registry = RunnerRegistry::default();
let mut runner = registration(true);
runner.capabilities.project_validation_rust_clippy_v1 = false;
registry.register(runner).await.unwrap();
let mut request = locked_validation_request();
request.action = ProjectValidationAction::Lint;
request.adapter = hint;
let access = auth_context(None, true);
if hint == ProjectValidationAdapter::Rust {
let error = registry
.enqueue_project_validation_plan(
"policy-runner".into(),
request.clone(),
Some(&access),
)
.await
.unwrap_err();
assert!(
error.contains("project_validation_rust_clippy_v1"),
"{error}"
);
} // Auto planning stays available for existing Node lint; resolved Clippy is gated below.
let mut metadata = locked_validation_metadata();
let validation = metadata.validation.as_mut().unwrap();
validation.project_validation.as_mut().unwrap().request = request;
validation.adapter = "cargo_clippy".into();
validation.kind = "lint".into();
validation.steps[0].name = "lint".into();
validation.steps[0].args = [
"clippy",
"--locked",
"--all-targets",
"--",
"-D",
"clippy::correctness",
]
.map(str::to_string)
.to_vec();
metadata.validation_steps = validation.steps.clone();
assert!(validation.is_valid());
let error = registry
.start_job_with_metadata(start_request("cargo clippy"), "test".into(), metadata)
.await
.unwrap_err();
assert!(
error.contains("project_validation_rust_clippy_v1"),
"{error}"
);
assert!(registry.list_jobs(Some(10)).await.is_empty());
assert!(registry
.poll(RunnerPollRequest {
client_id: "policy-runner".into(),
runner_instance_id: "inst".into()
})
.await
.unwrap()
.is_none());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub(super) fn validate_runner_job_context_operation(
|| context
.validation_steps
.iter()
.any(|step| !matches!(step.as_str(), "format" | "check" | "test"))
.any(|step| !matches!(step.as_str(), "format" | "check" | "lint" | "test"))
{
return Err("job recovery context validation_steps are invalid".to_string());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub(super) fn validate_detached_recovery_context(
|| context
.validation_steps
.iter()
.any(|step| !matches!(step.as_str(), "format" | "check" | "test"))
.any(|step| !matches!(step.as_str(), "format" | "check" | "lint" | "test"))
{
return Err("detached Job recovery validation_steps are invalid".to_string());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub(super) fn process_running_activity() -> ShellJobActivity {
pub(super) fn validation_step_activity(step: &ShellJobValidationStep) -> ShellJobActivity {
let phase = match step.name.as_str() {
"format" => ShellJobActivityPhase::ValidationFormat,
"check" => ShellJobActivityPhase::ValidationCheck,
"check" | "lint" => ShellJobActivityPhase::ValidationCheck,
"test" => ShellJobActivityPhase::ValidationTest,
_ => unreachable!("canonical validation step name"),
};
Expand Down
Loading
Loading