From 700850096924ed908e0e55eb8e4e373b46498cf6 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 21:12:33 +0000 Subject: [PATCH 1/4] fix(native): explain preflight refusals by service and field --- docs/guides/native-candidate.md | 28 ++- packages/runtime-core/src/error.rs | 43 ++++ .../src/project/compose/dependency_cache.rs | 130 +++++++++--- .../src/project/compose/service.rs | 45 ++++- .../runtime-core/src/provider/graph/config.rs | 188 ++++++++++++++---- .../runtime-core/src/provider/graph/routes.rs | 167 ++++++++++++---- .../runtime-core/tests/project_contract.rs | 55 +++++ src/backends/native-configuration-refusals.ts | 68 +++++++ src/backends/native-preflight.ts | 136 +++++++++++-- src/backends/native-runtime-client.ts | 42 +++- src/backends/native-start-refusals.ts | 10 + src/commands/doctor-native-preflight.ts | 4 +- src/lib/native-compose-backend-features.ts | 36 +++- tests/native-configuration-refusals.test.ts | 83 ++++++++ tests/native-preflight-cli.test.ts | 182 ++++++++++++++++- tests/native-runtime-client.test.ts | 35 ++++ 16 files changed, 1117 insertions(+), 135 deletions(-) create mode 100644 src/backends/native-configuration-refusals.ts create mode 100644 tests/native-configuration-refusals.test.ts diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index 028d26d10..e6b61d97e 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -201,7 +201,7 @@ hack doctor --native-preflight --profile dev,ops hack doctor --native-preflight --json ``` -It lists the blockers and notes for the project's `.hack/docker-compose.yml`, +It lists blockers, consent needed and notes for the project's `.hack/docker-compose.yml`, each with a plain sentence and a fix. It judges what `hack up` judges: the Compose document after the selected native adaptation and the project-wide extra hosts are applied, reviewed with the profiles you pass. @@ -212,6 +212,18 @@ foreground `up`; native publishes TCP only. Graph budgets, readiness, probes and restart restrictions use the same admission checks. Inactive profile services do not contribute publication blockers. +Graph subset and route refusals identify each affected service and field, with +a separate reason and fix. Incompatible plans show their underlying field +diagnostics instead of a second generic refusal. Invalid dependency-cache +declarations identify the offending labels across services; foreign-platform +findings name each service. Only names and field paths are shown, never authored +values. Structural parse errors can still stop review early; fix them and rerun. + +Missing shared-source consent appears under **Consent needed**, separately from +configuration blockers. Review still runs without consent, including in a +non-interactive terminal, and `--json` retains the full discovered blocker list. +Reporting consent does not grant it or start the VM. + The Compose input must be a nonempty regular file of at most 256 KiB, not a symbolic link. Unreadable, changing or oversized input is reported as `compose_file_unreadable`; the check never waits for a FIFO writer. @@ -247,13 +259,18 @@ service receiving only those adapter credentials can still be refused at serve. With `--json` the report is the `data` of the standard result envelope: +Report version 2 moves `shared_source_grant_required` from `blockers` into +`consent_needed`. Consumers should use `ready` for the overall verdict and +inspect both lists for what remains to do. + ```json { - "report_version": 1, + "report_version": 2, "backend": "native", "compose_file": ".hack/docker-compose.yml", "ready": false, "runtime_review": { "status": "completed" }, + "consent_needed": [], "blockers": [ { "severity": "blocker", @@ -269,12 +286,13 @@ With `--json` the report is the `data` of the standard result envelope: } ``` -- `ready` is true only when there is no blocker and the runtime review +- `ready` is true only when there is no blocker or missing consent and the runtime review completed. -- The exit status follows the blockers: `0` with none, `1` otherwise. A skipped +- The exit status is `1` if blockers or missing consent remain, `0` otherwise. A skipped or unavailable review is not a blocker, so that run exits `0` with `ready: false`. With `--json` the envelope is `ok: true` in both cases; read - `ready` and `blockers` for the verdict. + `ready`, `blockers` and `consent_needed` for the verdict. `consent_needed` + contains findings with severity `consent`, including `shared_source_grant_required`. - `gate` is `cli` or `runtime`: which of the two checks reported the finding. - `runtime_review.status` is one of: - `completed`: the review ran. diff --git a/packages/runtime-core/src/error.rs b/packages/runtime-core/src/error.rs index 1272750b5..62d5e228d 100644 --- a/packages/runtime-core/src/error.rs +++ b/packages/runtime-core/src/error.rs @@ -144,6 +144,10 @@ pub struct RouteProbeDiagnostic { /// `process_refusals`, `plan_change` and `route_probe`. #[derive(Debug, Default, Serialize)] pub struct RefusalDiagnostics { + /// File-only explanations. Every sentence and field is a compile-time literal; + /// only validated service names come from the project. Never contains values. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub configuration_refusals: Vec, /// Every service whose selected process a private wrapper refused, found in /// one pass before any runtime state is reserved. #[serde(skip_serializing_if = "Option::is_none")] @@ -155,6 +159,34 @@ pub struct RefusalDiagnostics { pub route_probe: Option, } +#[derive(Clone, Debug, Serialize)] +pub struct ConfigurationRefusal { + pub text_fixed: bool, + pub service: Option>, + pub field: &'static str, + pub message: &'static str, + pub fix: &'static str, +} + +impl ConfigurationRefusal { + pub fn new(field: &'static str, message: &'static str, fix: &'static str) -> Self { + Self { + text_fixed: true, + service: None, + field, + message, + fix, + } + } + + pub fn with_service(mut self, service: &str) -> Self { + self.service = CandidateError::new("configuration", "Configuration refusal.") + .with_service(service) + .service; + self + } +} + /// Closed diagnostic vocabulary for the read-only log follower. No stream bytes. #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "kebab-case")] @@ -201,6 +233,17 @@ pub struct CandidateError { } impl CandidateError { + pub fn with_configuration_refusals(mut self, mut findings: Vec) -> Self { + // Malformed lists can repeat the same field thousands of times. Keep + // every distinct explanation, not one copy per authored value. This + // also bounds the serialized review error below the CLI pipe budget. + let mut seen = std::collections::BTreeSet::new(); + findings.retain(|item| seen.insert((item.service.clone(), item.field, item.message))); + self.refusals + .get_or_insert_with(Default::default) + .configuration_refusals = findings; + self + } pub fn new(code: &'static str, message: impl Into) -> Self { let message = message.into(); Self { diff --git a/packages/runtime-core/src/project/compose/dependency_cache.rs b/packages/runtime-core/src/project/compose/dependency_cache.rs index 5d96eed9b..7bea52a13 100644 --- a/packages/runtime-core/src/project/compose/dependency_cache.rs +++ b/packages/runtime-core/src/project/compose/dependency_cache.rs @@ -28,17 +28,32 @@ const DEFAULT_RUNTIME: &[&str] = &[ pub(super) fn recognized(key: &str) -> bool { matches!(key, VOLUME | LOCKFILES | RUNTIME | BOOTSTRAP) } -fn refused() -> CandidateError { +fn detail( + key: &str, + message: &'static str, + fix: &'static str, +) -> crate::error::ConfigurationRefusal { + let field = match key { + VOLUME => "labels.hack.dependencies.cache-volume", + LOCKFILES => "labels.hack.dependencies.lockfiles", + RUNTIME => "labels.hack.dependencies.runtime-files", + BOOTSTRAP => "labels.hack.dependencies.bootstrap", + _ => "labels", + }; + crate::error::ConfigurationRefusal::new(field, message, fix) +} +fn refused(findings: Vec) -> CandidateError { problem( "invalid_dependency_cache", - "Dependency cache requires a literal named volume, bounded relative file paths and a boolean bootstrap declaration; values omitted.", + "Dependency cache declarations are invalid; values omitted.", ) + .with_configuration_refusals(findings) } -fn paths(value: Option<&String>, defaults: &[&str]) -> Result<(Vec, bool), CandidateError> { +fn paths(value: Option<&String>, defaults: &[&str]) -> Option<(Vec, bool)> { let mut paths = std::collections::BTreeSet::new(); if let Some(value) = value { if value.len() > 8192 { - return Err(refused()); + return None; } for path in value.split(',').map(str::trim).filter(|p| !p.is_empty()) { if path.len() > 512 @@ -48,11 +63,11 @@ fn paths(value: Option<&String>, defaults: &[&str]) -> Result<(Vec, bool .split('/') .any(|p| p.is_empty() || p == "." || p == "..") { - return Err(refused()); + return None; } paths.insert(path.to_owned()); if paths.len() > 32 { - return Err(refused()); + return None; } } } @@ -60,51 +75,105 @@ fn paths(value: Option<&String>, defaults: &[&str]) -> Result<(Vec, bool if !explicit { paths.extend(defaults.iter().map(|p| (*p).to_owned())); } - Ok((paths.into_iter().collect(), explicit)) + Some((paths.into_iter().collect(), explicit)) } +/// Collect independent label errors without ever storing a label value in a diagnostic. +/// The accepted cache and the standalone-bootstrap exception are unchanged. pub(super) fn parse(value: Option<&Value>) -> Result, CandidateError> { let mut labels = BTreeMap::new(); - let mut insert = |key: &str, value: &Value| -> Result<(), CandidateError> { + let mut findings = Vec::new(); + let mut insert = |key: &str, value: &Value| { if !recognized(key) { - return Ok(()); + return; } let value = match value { Value::String(v) => v.clone(), Value::Bool(v) if key == BOOTSTRAP => v.to_string(), - _ => return Err(refused()), + _ => { + findings.push(detail(key, "A dependency-cache label has the wrong type.", "Use a string for cache-volume and file lists; bootstrap accepts a boolean or its literal string form.")); + return; + } }; if labels.insert(key.to_owned(), value).is_some() { - return Err(refused()); + findings.push(detail( + key, + "A dependency-cache label is declared more than once.", + "Keep exactly one declaration of this label.", + )); } - Ok(()) }; match value { None | Some(Value::Null) => return Ok(None), Some(Value::Object(values)) => { for (key, value) in values { - insert(key, value)?; + insert(key, value); } } Some(Value::Array(values)) => { for value in values { - let value = value.as_str().ok_or_else(refused)?; - let (key, value) = value.split_once('=').unwrap_or((value, "")); - insert(key, &Value::String(value.into()))?; + if let Some(value) = value.as_str() { + let (key, value) = value.split_once('=').unwrap_or((value, "")); + insert(key, &Value::String(value.into())); + } else { + findings.push(detail( + "", + "The label list contains a non-string entry.", + "Use a mapping or a list of key=value strings for labels.", + )); + return Err(refused(findings)); + } } } - _ => return Err(refused()), + _ => { + return Err(refused(vec![detail( + "", + "Labels are neither a mapping nor a list.", + "Use a mapping or a list of key=value strings for labels.", + )])); + } } if labels.is_empty() || (labels.len() == 1 && labels.contains_key(BOOTSTRAP)) { - return Ok(None); + return if findings.is_empty() { + Ok(None) + } else { + Err(refused(findings)) + }; + } + let volume = labels + .get(VOLUME) + .map(|v| v.trim().to_owned()) + .unwrap_or_default(); + if super::fields::identifier(&volume).is_err() { + findings.push(detail(VOLUME, "The dependency cache has no literal named-volume identifier.", "Set hack.dependencies.cache-volume to one declared named volume, without interpolation.")); + } + let lockfiles = paths(labels.get(LOCKFILES), DEFAULT_LOCKFILES); + let runtime = paths(labels.get(RUNTIME), DEFAULT_RUNTIME); + for (key, valid) in [ + (LOCKFILES, lockfiles.is_some()), + (RUNTIME, runtime.is_some()), + ] { + if !valid { + findings.push(detail(key, "The dependency-cache file list is not a bounded list of relative paths.", "Use at most 32 comma-separated relative paths (512 bytes each, 8192 bytes total), without variables, backslashes, control characters, empty, dot or parent components.")); + } } - let volume = labels.get(VOLUME).ok_or_else(refused)?.trim().to_owned(); - super::fields::identifier(&volume).map_err(|_| refused())?; - let (lockfiles, lockfiles_explicit) = paths(labels.get(LOCKFILES), DEFAULT_LOCKFILES)?; - let (runtime_files, _) = paths(labels.get(RUNTIME), DEFAULT_RUNTIME)?; let bootstrap = match labels.get(BOOTSTRAP).map(String::as_str) { None | Some("false") => false, Some("true") => true, - _ => return Err(refused()), + _ => { + findings.push(detail( + BOOTSTRAP, + "The dependency-cache bootstrap declaration is not boolean.", + "Use true or false for hack.dependencies.bootstrap.", + )); + false + } + }; + if !findings.is_empty() { + return Err(refused(findings)); + } + let (Some((lockfiles, lockfiles_explicit)), Some((runtime_files, _))) = (lockfiles, runtime) + else { + return Err(refused(vec![])); }; Ok(Some(DependencyCachePlan { volume, @@ -119,6 +188,21 @@ pub(super) fn parse(value: Option<&Value>) -> Result mod tests { use super::*; use serde_json::json; + #[test] + fn duplicate_label_flood_keeps_a_small_error_with_prior_findings() { + let mut values = vec![json!("hack.dependencies.lockfiles=private-sentinel"); 4096]; + values.push(json!(42)); + let error = parse(Some(&Value::Array(values))).unwrap_err(); + assert_eq!(error.code, "invalid_dependency_cache"); + let wire = serde_json::to_value(error).unwrap(); + let details = wire["configuration_refusals"].as_array().unwrap(); + assert_eq!(details.len(), 2); + assert_eq!(details[0]["field"], "labels.hack.dependencies.lockfiles"); + assert_eq!(details[1]["field"], "labels"); + assert!(wire.to_string().len() < 8192); + assert!(!wire.to_string().contains("private-sentinel")); + } + #[test] fn actual_declaration_and_list_form_are_equivalent() { let map = json!({"hack.dependencies.cache-volume":"node_modules", "hack.dependencies.lockfiles":"bun.lock", "hack.dependencies.runtime-files":"package.json,bunfig.toml,mise.toml,.tool-versions,packages/db/prisma/schema.prisma", "hack.dependencies.bootstrap":"true"}); diff --git a/packages/runtime-core/src/project/compose/service.rs b/packages/runtime-core/src/project/compose/service.rs index 4aee19da9..9a98c02f1 100644 --- a/packages/runtime-core/src/project/compose/service.rs +++ b/packages/runtime-core/src/project/compose/service.rs @@ -729,6 +729,47 @@ pub(super) fn compile( "Compose requires 1 to 64 services in WU03.", )); } + let mut result = BTreeMap::new(); + let mut first_error = None; + let mut findings = Vec::new(); + for (name, value) in values { + let one = Map::from_iter([(name.clone(), value.clone())]); + match compile_services(project, base, &one, profiles, volumes, diagnostics) { + Ok(services) => result.extend(services), + Err(mut error) if error.code == "invalid_dependency_cache" => { + if let Some(details) = &mut error.refusals { + findings.extend( + std::mem::take(&mut details.configuration_refusals) + .into_iter() + .map(|item| item.with_service(name)), + ); + } + if first_error.is_none() { + first_error = Some(error.with_service(name)); + } + } + Err(error) => { + first_error.get_or_insert(error); + } + } + } + match first_error { + Some(error) if error.code == "invalid_dependency_cache" => { + Err(error.with_configuration_refusals(findings)) + } + Some(error) => Err(error), + None => Ok(result), + } +} + +fn compile_services( + project: &Path, + base: &Path, + values: &Map, + profiles: &[String], + volumes: &BTreeMap, + diagnostics: &mut Vec, +) -> Result, CandidateError> { let mut result = BTreeMap::new(); for (name, value) in values { identifier(name)?; @@ -883,7 +924,7 @@ pub(super) fn compile( return Err(problem( "invalid_dependency_cache", "Dependency cache volume must be declared; values omitted.", - )); + ).with_configuration_refusals(vec![crate::error::ConfigurationRefusal::new("labels.hack.dependencies.cache-volume", "The dependency-cache volume is not declared.", "Declare the named volume at top level and mount it in the initializer service.")])); } // A shared dependency cache outlives and is consumed beyond one graph. if !volumes[&cache.volume].disposable_caches.is_empty() { @@ -1105,7 +1146,7 @@ pub(super) fn compile( return Err(problem( "invalid_dependency_cache", "Dependency cache volume must be mounted by its declaring service; values omitted.", - )); + ).with_configuration_refusals(vec![crate::error::ConfigurationRefusal::new("volumes", "The dependency-cache volume is not mounted by its initializer.", "Mount the declared cache volume in the service that declares hack.dependencies.cache-volume.")])); } let mut healthcheck = health( m.get("healthcheck"), diff --git a/packages/runtime-core/src/provider/graph/config.rs b/packages/runtime-core/src/provider/graph/config.rs index 6d2c37097..233e2e25b 100644 --- a/packages/runtime-core/src/provider/graph/config.rs +++ b/packages/runtime-core/src/provider/graph/config.rs @@ -487,46 +487,74 @@ fn declared_configuration( let mut volumes = BTreeSet::new(); let mut total_memory = 0u64; let mut total_cpus = 0f64; - for service in plan.services.values().filter(|s| s.active) { - if (!service.ports.is_empty() && !loopback_ports) - || service.labels.keys().any(|key| { - !(routing_enrolled && service.routing.is_some() && routes::recognized(key)) - && !(service.one_shot && key == "hack.service.one-shot") - // Inert hostname declarations carry no traffic; they never - // reach the engine (graph labels are ownership-only). - && !(service.inert_route_labels - && service.routing.is_none() - && matches!(key.as_str(), "caddy" | "caddy.tls")) - // Plain metadata is reviewed with a warning and dropped here. - && !crate::project::inert_metadata_label(key) - && !(service.dependency_cache.is_some() - && matches!( - key.as_str(), - "hack.dependencies.cache-volume" - | "hack.dependencies.lockfiles" - | "hack.dependencies.runtime-files" - | "hack.dependencies.bootstrap" - )) - }) - // A local driver is reviewed with a warning and not applied; an - // external sink was already an error at review. - || service - .logging - .as_ref() - .is_some_and(|logging| logging.driver == "[unsupported]") - || service.platform.as_deref().is_some_and(|p| { - !(["linux/arm64", "linux/arm64/v8"].contains(&p) - || (p == "linux/amd64" && service.amd64_emulation.is_some())) - }) + let mut findings = Vec::new(); + let mut first_error = None; + for (name, service) in plan.services.iter().filter(|(_, s)| s.active) { + let mut refuse = |field, message, fix| { + findings.push( + crate::error::ConfigurationRefusal::new(field, message, fix).with_service(name), + ) + }; + if !service.ports.is_empty() && !loopback_ports { + refuse( + "ports", + "This graph driver has no enrolled port publication.", + "Start through hack up so it owns the loopback publishers, or remove ports.", + ); + } + if service.labels.keys().any(|key| { + !(routing_enrolled && service.routing.is_some() && routes::recognized(key)) + && !(service.one_shot && key == "hack.service.one-shot") + && !(service.inert_route_labels + && service.routing.is_none() + && matches!(key.as_str(), "caddy" | "caddy.tls")) + && !crate::project::inert_metadata_label(key) + && !(service.dependency_cache.is_some() + && matches!( + key.as_str(), + "hack.dependencies.cache-volume" + | "hack.dependencies.lockfiles" + | "hack.dependencies.runtime-files" + | "hack.dependencies.bootstrap" + )) + }) { + refuse( + "labels", + "The service has labels the graph driver cannot apply.", + "Remove unsupported router or ownership labels; keep supported Caddy, one-shot and dependency-cache declarations. Use Compose if another router needs these labels.", + ); + } + if service + .logging + .as_ref() + .is_some_and(|logging| logging.driver == "[unsupported]") { - return Err(unsupported()); + refuse( + "logging.driver", + "The service selects an external logging driver.", + "Remove logging or use json-file, local or none. Native retains bounded container logs; use hack logs.", + ); + } + if service.platform.as_deref().is_some_and(|p| { + !(["linux/arm64", "linux/arm64/v8"].contains(&p) + || (p == "linux/amd64" && service.amd64_emulation.is_some())) + }) { + refuse( + "platform", + "The service platform is not admitted by this graph driver.", + "Remove platform or use an arm64 image; linux/amd64 requires explicit service opt-in under amd64Emulation in the native adaptation.", + ); } if service.network_mode == "project-networks" { if service.networks.is_empty() || service.networks.len() > MAX_NETWORKS || service.networks.iter().collect::>().len() != service.networks.len() { - return Err(unsupported()); + refuse( + "networks", + "The service needs between one and 32 distinct project networks.", + "Attach the service to at least one declared network and remove duplicate or excess attachments.", + ); } for network in &service.networks { if !plan @@ -534,24 +562,40 @@ fn declared_configuration( .get(network) .is_some_and(|n| n.driver == "bridge") { - return Err(unsupported()); + refuse( + "networks", + "A service network is undeclared or does not use the bridge driver.", + "Declare each attached network with the bridge driver, or select an isolated-network adaptation for a supported external network.", + ); } networks.insert(network.clone()); } } else if service.network_mode != "none" || !service.networks.is_empty() { - return Err(unsupported()); + refuse( + "network_mode", + "The service network mode cannot be combined with its network attachments.", + "Remove network_mode and use project networks, or use none with no network attachments. Namespace sharing requires Compose.", + ); } for mount in &service.mounts { if mount.subpath.as_deref().is_some_and(|value| { mount.kind != "volume" || !project::valid_volume_subpath(value) }) { - return Err(unsupported()); + refuse( + "volumes.volume.subpath", + "A mount subpath is not a safe named-volume-relative directory.", + "Use subpath only on a named volume, with a nonempty relative path and no dot, parent or empty components.", + ); } if mount.kind == "bind" { continue; } if mount.kind != "volume" || !plan.volumes.contains_key(&mount.source) { - return Err(unsupported()); + refuse( + "volumes", + "A mount is neither a bind nor a declared named volume.", + "Declare the named volume at top level, or use a supported bind mount. Other mount types require Compose.", + ); } volumes.insert(mount.source.clone()); } @@ -559,6 +603,9 @@ fn declared_configuration( // count toward graph reservations. Docker represents omitted caps as zero. let memory = service.limits.memory_bytes.unwrap_or(0); let cpus = service.limits.cpus.unwrap_or(0.0); + if !findings.is_empty() && first_error.is_none() { + first_error = Some(unsupported()); + } if service .limits .memory_bytes @@ -576,16 +623,25 @@ fn declared_configuration( .stop_grace_period_nanos .is_some_and(|n| n > 30_000_000_000) { - return Err(error( - "graph_budget", - "A service exceeds the bounded graph profile.", - )); + first_error.get_or_insert_with(|| { + error( + "graph_budget", + "A service exceeds the bounded graph profile.", + ) + }); } // ShmSize is a tmpfs ceiling, not preallocated RSS. Its actual usage is // already constrained by an explicit container memory cap or the VM pool. - total_memory += memory; + total_memory = total_memory.saturating_add(memory); total_cpus += cpus; } + if let Some(error) = first_error { + return Err(if error.code == "graph_subset" { + error.with_configuration_refusals(findings) + } else { + error + }); + } if networks.len() > MAX_NETWORKS || volumes.len() > MAX_VOLUMES || total_memory > MAX_TOTAL_MEMORY_BYTES @@ -1187,6 +1243,54 @@ mod compose_field_tests { json!({"services":{"web":service},"networks":{"private":{"internal":true}}}) } + #[test] + fn subset_reports_all_services_and_keeps_the_first_error_precedence() { + let fixture = super::super::tests::Fixture::new(); + let candidate_root = super::super::tests::Fixture::new(); + let candidate = Candidate::discover(&candidate_root.0).unwrap(); + state::write(&fixture.0.join("compose.yaml"), &document(json!({}))).unwrap(); + let mut plan = project::plan( + &candidate, + PlanOptions { + branch: None, + project: &fixture.0, + compose_file: Path::new("compose.yaml"), + profiles: &[], + }, + ) + .unwrap() + .plan; + let mut bad = plan.services["web"].clone(); + bad.platform = Some("private-sentinel".into()); + bad.network_mode = "private-sentinel".into(); + bad.limits.memory_bytes = Some(u64::MAX); + plan.services.insert("api".into(), bad.clone()); + plan.services.insert("web".into(), bad); + let error = declared_configuration(&plan, true, true).unwrap_err(); + assert_eq!(error.code, "graph_subset"); + let wire = serde_json::to_value(error).unwrap(); + for service in ["api", "web"] { + for field in ["platform", "network_mode"] { + assert!( + wire["configuration_refusals"] + .as_array() + .unwrap() + .iter() + .any(|item| item["service"] == service && item["field"] == field) + ); + } + } + assert!(!wire.to_string().contains("private-sentinel")); + // An earlier service budget still wins over a later subset error. + let first = plan.services.get_mut("api").unwrap(); + first.platform = None; + first.network_mode = "project-networks".into(); + assert_eq!( + declared_configuration(&plan, true, true).unwrap_err().code, + "graph_budget" + ); + } + #[test] fn stdin_open_keeps_unattached_stdin_and_never_allocates_a_terminal() { for (fields, open) in [ diff --git a/packages/runtime-core/src/provider/graph/routes.rs b/packages/runtime-core/src/provider/graph/routes.rs index 503339d7c..a640590a9 100644 --- a/packages/runtime-core/src/provider/graph/routes.rs +++ b/packages/runtime-core/src/provider/graph/routes.rs @@ -37,61 +37,120 @@ fn validate_services( graph: &Graph, enrolled: bool, ) -> Result<(), CandidateError> { - let mut hosts = BTreeSet::new(); + let mut hosts = BTreeMap::new(); + let mut findings = Vec::new(); let mut count = 0; for (name, service) in services.iter().filter(|(_, service)| service.active) { let Some(route) = &service.routing else { continue; }; + let mut refuse = |field, message, fix| { + findings.push( + crate::error::ConfigurationRefusal::new(field, message, fix).with_service(name), + ) + }; count += 1; if count > 32 || !enrolled { - return Err(refused()); + refuse( + "labels.caddy", + "The route has no available enrolled publication.", + "Keep at most 32 routed services and start through hack up after configuring native HTTPS.", + ); } let health = service .healthcheck .as_ref() .filter(|health| !health.disabled); let probe = health.and_then(|health| health.native_http.as_ref()); - if graph.services.get(name).map(|service| service.ready) != Some(Condition::Healthy) - || service.network_mode != "project-networks" - || service.networks.is_empty() - || !probe.is_some_and(|probe| probe.port == route.port && probe.validate().is_ok()) - || probe.is_some_and(|probe| { - probe.policy.is_some() - && *probe != super::super::http_probe::HttpProbe::inferred_route(route.port) - }) - || !valid_intent(route) - || !["caddy", "caddy.reverse_proxy", "caddy.tls"] - .iter() - .all(|key| { - service.labels.get(*key).is_some_and(|value| { - value.environment_references.is_empty() && value.literal_redacted - }) - }) - || service.labels.keys().any(|key| { - !recognized(key) - // Plain metadata beside the route is reviewed with a warning - // and never reaches the engine or the router. - && !crate::project::inert_metadata_label(key) - && !(service.dependency_cache.is_some() - && matches!( - key.as_str(), - "hack.dependencies.cache-volume" - | "hack.dependencies.lockfiles" - | "hack.dependencies.runtime-files" - | "hack.dependencies.bootstrap" - )) - }) - { - return Err(refused()); + if graph.services.get(name).map(|service| service.ready) != Some(Condition::Healthy) { + refuse( + "healthcheck", + "The routed service does not have healthy readiness.", + "Use a long-running service with an enabled native HTTP healthcheck for route readiness.", + ); + } + if service.network_mode != "project-networks" || service.networks.is_empty() { + refuse( + "networks", + "The routed service has no project-network endpoint.", + "Remove network_mode and attach the routed service to a declared project network.", + ); + } + if !probe.is_some_and(|probe| probe.port == route.port && probe.validate().is_ok()) { + refuse( + "healthcheck.x-hack-http", + "The route needs a valid native HTTP probe on its upstream port.", + "Set healthcheck.x-hack-http to the route port and readiness path, or omit the authored healthcheck to use the inferred route probe. A command-only healthcheck cannot establish route readiness.", + ); + } + if probe.is_some_and(|probe| { + probe.policy.is_some() + && *probe != super::super::http_probe::HttpProbe::inferred_route(route.port) + }) { + refuse( + "healthcheck.x-hack-http", + "The inferred route probe policy was changed.", + "Omit the authored healthcheck to use the default inferred probe, or declare an explicit native HTTP probe without an inferred policy.", + ); + } + if !valid_intent(route) { + refuse( + "labels.caddy", + "The route does not have canonical distinct hostnames and a nonzero port.", + "Declare one to eight distinct canonical hostnames and a literal upstream port in the supported Caddy labels.", + ); + } + for key in ["caddy", "caddy.reverse_proxy", "caddy.tls"] { + if !service.labels.get(key).is_some_and(|value| { + value.environment_references.is_empty() && value.literal_redacted + }) { + let field = match key { + "caddy" => "labels.caddy", + "caddy.tls" => "labels.caddy.tls", + _ => "labels.caddy.reverse_proxy", + }; + refuse( + field, + "A required route label is missing or uses an environment reference.", + "Declare literal caddy hostnames, caddy.reverse_proxy with {{upstreams PORT}}, and caddy.tls: internal.", + ); + } + } + if service.labels.keys().any(|key| { + !recognized(key) + && !crate::project::inert_metadata_label(key) + && !(service.dependency_cache.is_some() + && matches!( + key.as_str(), + "hack.dependencies.cache-volume" + | "hack.dependencies.lockfiles" + | "hack.dependencies.runtime-files" + | "hack.dependencies.bootstrap" + )) + }) { + refuse( + "labels", + "The routed service has unsupported semantic labels.", + "Remove other router or ownership labels; keep the supported Caddy and dependency-cache labels. Use Compose if another router is required.", + ); } for host in &route.hostnames { - if !hosts.insert(host) { - return Err(refused()); + if let Some(previous) = hosts.insert(host, name) { + let detail = crate::error::ConfigurationRefusal::new( + "labels.caddy", + "Multiple routes claim the same hostname.", + "Assign distinct hostnames to the routed services; one hostname can belong to only one route.", + ); + findings.push(detail.clone().with_service(previous)); + findings.push(detail.with_service(name)); } } } - Ok(()) + if findings.is_empty() { + Ok(()) + } else { + Err(refused().with_configuration_refusals(findings)) + } } #[cfg(test)] @@ -116,6 +175,40 @@ mod tests { (plan, graph) } + #[test] + fn reports_each_service_and_independent_route_field_without_values() { + let (mut plan, mut graph) = fixture(); + let service = plan.get_mut("search").unwrap(); + service.healthcheck = None; + service.networks.clear(); + service.labels.remove("caddy.tls"); + service.routing.as_mut().unwrap().hostnames = vec!["private-sentinel.example".into()]; + plan.insert("other".into(), plan["search"].clone()); + graph + .services + .insert("other".into(), graph.services["search"].clone()); + let error = validate_services(&plan, &graph, true).unwrap_err(); + assert_eq!(error.code, "graph_route_binding"); + let wire = serde_json::to_value(&error).unwrap(); + let findings = wire["configuration_refusals"].as_array().unwrap(); + for service in ["search", "other"] { + for field in [ + "networks", + "healthcheck.x-hack-http", + "labels.caddy.tls", + "labels.caddy", + ] { + assert!( + findings + .iter() + .any(|item| item["service"] == service && item["field"] == field), + "{wire}" + ); + } + } + assert!(!wire.to_string().contains("private-sentinel")); + } + #[test] fn actual_search_requires_enrollment_and_matching_health() { let (mut plan, mut graph) = fixture(); diff --git a/packages/runtime-core/tests/project_contract.rs b/packages/runtime-core/tests/project_contract.rs index 63ebfbd98..3ea951ec2 100644 --- a/packages/runtime-core/tests/project_contract.rs +++ b/packages/runtime-core/tests/project_contract.rs @@ -66,6 +66,61 @@ impl Drop for Fixture { } const BASIC: &str = "services:\n web:\n image: alpine:3.21\n command: [echo, hello]\n"; +#[test] +fn cache_refusal_lists_independent_fields_across_services_without_values() { + let service = serde_json::json!({"image":"alpine:3.21", "labels":{ + "hack.dependencies.cache-volume":"${private-sentinel}", + "hack.dependencies.lockfiles":"../private-sentinel", + "hack.dependencies.runtime-files":"${private-sentinel}", + "hack.dependencies.bootstrap":"private-sentinel" + }}); + let fixture = + Fixture::new(&serde_json::json!({"services":{"api":service,"worker":service}}).to_string()); + let error = project::plan(&fixture.candidate, fixture.options()).unwrap_err(); + assert_eq!(error.code, "invalid_dependency_cache"); + let wire = serde_json::to_value(error).unwrap(); + let findings = wire["configuration_refusals"].as_array().unwrap(); + assert_eq!(findings.len(), 8); + for name in ["api", "worker"] { + for field in ["cache-volume", "lockfiles", "runtime-files", "bootstrap"] { + assert!( + findings.iter().any(|item| item["service"] == name + && item["field"] == format!("labels.hack.dependencies.{field}")), + "{wire}" + ); + } + } + assert!(!wire.to_string().contains("private-sentinel")); + assert!(!fixture.candidate.state_root.exists()); +} + +#[test] +fn missing_cache_declaration_and_initializer_mount_have_service_locations() { + let fixture = Fixture::new( + r#"services: + api: + image: alpine:3.21 + labels: {hack.dependencies.cache-volume: undeclared} + worker: + image: alpine:3.21 + labels: {hack.dependencies.cache-volume: deps} +volumes: + deps: {} +"#, + ); + let error = project::plan(&fixture.candidate, fixture.options()).unwrap_err(); + let wire = serde_json::to_value(error).unwrap(); + let findings = wire["configuration_refusals"].as_array().unwrap(); + assert_eq!(findings.len(), 2); + assert_eq!(findings[0]["service"], "api"); + assert_eq!( + findings[0]["field"], + "labels.hack.dependencies.cache-volume" + ); + assert_eq!(findings[1]["service"], "worker"); + assert_eq!(findings[1]["field"], "volumes"); +} + #[test] fn standard_route_without_healthcheck_infers_a_reviewed_probe_without_editing_source() { use hack_runtime_core::provider::http_probe::HttpProbe; diff --git a/src/backends/native-configuration-refusals.ts b/src/backends/native-configuration-refusals.ts new file mode 100644 index 000000000..829e71f65 --- /dev/null +++ b/src/backends/native-configuration-refusals.ts @@ -0,0 +1,68 @@ +import { isRecord } from "../lib/guards.ts"; + +const SERVICE = /^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$/; +const TEXT = /^[\x20-\x7e]{1,400}$/; +const FIELDS = new Set([ + "ports", + "labels", + "logging.driver", + "platform", + "networks", + "network_mode", + "volumes", + "volumes.volume.subpath", + "healthcheck", + "healthcheck.x-hack-http", + "labels.caddy", + "labels.caddy.tls", + "labels.caddy.reverse_proxy", + "labels.hack.dependencies.cache-volume", + "labels.hack.dependencies.lockfiles", + "labels.hack.dependencies.runtime-files", + "labels.hack.dependencies.bootstrap", +]); + +export type NativeConfigurationRefusal = { + readonly service?: string; + readonly field: string; + readonly message: string; + readonly fix: string; +}; + +/** Runtime literals only, with closed field paths and bounded service identifiers. + * Like message_fixed, text_fixed is set by the Rust literal-only constructor. + * The whole batch is rejected on malformed input; raw runtime text never escapes. + */ +export function readNativeConfigurationRefusals( + value: unknown +): readonly NativeConfigurationRefusal[] | undefined { + if (!Array.isArray(value) || value.length === 0 || value.length > 4096) { + return; + } + const result: NativeConfigurationRefusal[] = []; + for (const item of value) { + if ( + !( + isRecord(item) && + item.text_fixed === true && + typeof item.field === "string" && + FIELDS.has(item.field) && + typeof item.message === "string" && + TEXT.test(item.message) && + typeof item.fix === "string" && + TEXT.test(item.fix) && + (item.service == null || + (typeof item.service === "string" && SERVICE.test(item.service))) + ) + ) { + return; + } + result.push({ + field: item.field, + message: item.message, + fix: item.fix, + ...(typeof item.service === "string" ? { service: item.service } : {}), + }); + } + return result; +} diff --git a/src/backends/native-preflight.ts b/src/backends/native-preflight.ts index 04c7b5ed2..4f7462ddf 100644 --- a/src/backends/native-preflight.ts +++ b/src/backends/native-preflight.ts @@ -77,7 +77,7 @@ import { readNativeStartRefusals } from "./native-start-refusals.ts"; export const NATIVE_PREFLIGHT_COMMAND = "hack doctor --native-preflight"; export type NativePreflightFinding = { - readonly severity: "blocker" | "warning"; + readonly severity: "blocker" | "warning" | "consent"; /** Which check produced it: the CLI's admission or the runtime review. */ readonly gate: "cli" | "runtime"; /** Stable, closed name. Runtime findings use the review's own codes. */ @@ -103,11 +103,11 @@ export type NativePreflightReviewStatus = | "unavailable"; export type NativePreflightReport = { - readonly report_version: 1; + readonly report_version: 2; readonly backend: "native"; /** The Compose file, relative to the project root. */ readonly compose_file: string; - /** No blocker was found and the runtime review completed. */ + /** No blocker or missing consent, and the runtime review completed. */ readonly ready: boolean; readonly runtime_review: { readonly status: NativePreflightReviewStatus; @@ -119,6 +119,7 @@ export type NativePreflightReport = { */ readonly code?: string; }; + readonly consent_needed: readonly NativePreflightFinding[]; readonly blockers: readonly NativePreflightFinding[]; readonly warnings: readonly NativePreflightFinding[]; }; @@ -126,7 +127,7 @@ export type NativePreflightReport = { type Explanation = { readonly message: string; readonly fix: string }; const USE_COMPOSE = - "Run this project on the Compose backend (HACK_RUNTIME_BACKEND=compose), or remove it."; + "If the project requires this unsupported configuration, use the Compose backend (HACK_RUNTIME_BACKEND=compose)."; const FEATURE_FINDINGS: Readonly> = { duplicate_hack_network: { @@ -201,6 +202,14 @@ const PREPARATION_FINDINGS: Readonly< * listed is still reported, by its code and field, with a pointer to the guide. */ const RUNTIME_FINDINGS: Readonly> = { + missing_service_source: { + message: "The service has no image or local build context.", + fix: "Declare an image, or a supported local build with native builds enabled.", + }, + conflicting_network_owners: { + message: "The service declares both networks and network_mode.", + fix: "Keep project network attachments and remove network_mode, or use network_mode: none with no attachments.", + }, privileged_service: { message: "The service asks for privileged mode.", fix: "Remove privileged. Use cap_add for the specific capability if native allows it.", @@ -1035,6 +1044,25 @@ async function collectNativeCliFindings(opts: { } ); for (const feature of reading.features) { + if ( + feature === "foreign_platform" && + reading.foreignPlatformServices?.length + ) { + const services = reading.foreignPlatformServices.filter((name) => + SERVICE.test(name) + ); + for (const service of services) { + found.push( + finding( + { ...blocker, code: feature, service, field: "platform" }, + FEATURE_FINDINGS[feature] + ) + ); + } + if (services.length > 0) { + continue; + } + } found.push( finding({ ...blocker, code: feature }, FEATURE_FINDINGS[feature]) ); @@ -1101,7 +1129,11 @@ async function collectNativeCliFindings(opts: { if (!opts.sharedSource) { found.push( finding( - { ...blocker, code: "shared_source_grant_required" }, + { + ...blocker, + severity: "consent", + code: "shared_source_grant_required", + }, { message: "Native shares the whole project directory with its VM, including ignored files, and needs your consent first.", @@ -1128,11 +1160,26 @@ function runtimeFinding( ) { return null; } - const field = + let field = typeof diagnostic.field === "string" && FIELD.test(diagnostic.field) ? diagnostic.field : undefined; const service = field ? serviceOfField(field, services) : undefined; + // Older reviewed plans locate these at the service root. Refine the report + // without changing plan identity or any admission predicate. + const serviceFields: Readonly> = { + privileged_service: "privileged", + missing_service_source: "image", + unsupported_network_mode: "network_mode", + conflicting_network_owners: "networks", + }; + if ( + service && + field === `services.${service}` && + serviceFields[diagnostic.code] + ) { + field = `${field}.${serviceFields[diagnostic.code]}`; + } return finding( { severity: diagnostic.severity === "error" ? "blocker" : "warning", @@ -1159,6 +1206,23 @@ function runtimeStartFindings( } const findings: NativePreflightFinding[] = []; for (const refusal of refusals) { + if (refusal.configuration?.length) { + for (const detail of refusal.configuration) { + findings.push( + finding( + { + severity: "blocker", + gate: "runtime", + code: refusal.code, + service: detail.service, + field: detail.field, + }, + detail + ) + ); + } + continue; + } const service = refusal.service; const field = NATIVE_START_REFUSAL_ENTRIES[refusal.code]; findings.push( @@ -1189,7 +1253,10 @@ type RuntimeReview = { }; /** How a failed review request is reported: a project verdict, or no review. */ -function reviewFailure(error: unknown): RuntimeReview { +function reviewFailure( + error: unknown, + services: readonly string[] +): RuntimeReview { const code = error instanceof NativeRuntimeRequestError && typeof error.nativeCode === "string" && @@ -1202,6 +1269,30 @@ function reviewFailure(error: unknown): RuntimeReview { findings: [], }; } + if ( + error instanceof NativeRuntimeRequestError && + error.nativeConfigurationRefusals?.length + ) { + const findings = error.nativeConfigurationRefusals + .filter( + (item) => item.service === undefined || services.includes(item.service) + ) + .map((item) => + finding( + { + severity: "blocker", + gate: "runtime", + code, + service: item.service, + field: item.field, + }, + item + ) + ); + if (findings.length) { + return { review: { status: "stopped", code }, findings }; + } + } const text = RUNTIME_FINDINGS[code] ?? { message: `The native review stopped with ${code}.`, fix: "See the native backend guide for this code.", @@ -1288,7 +1379,10 @@ async function collectNativeRuntimeFindings(opts: { if (isNativeRuntimeStopError(error)) { throw error; } - return reviewFailure(error); + return reviewFailure( + error, + Object.keys(opts.adapted.services).filter((name) => SERVICE.test(name)) + ); } finally { if (directory !== undefined) { await rm(directory, { recursive: true, force: true }); @@ -1317,7 +1411,15 @@ async function collectNativeRuntimeFindings(opts: { findings, }; } - findings.push(...startFindings); + // Plan diagnostics already name the actual incompatible fields. Do not add + // an opaque duplicate which tells the user only that diagnostics exist. + findings.push( + ...startFindings.filter( + (item) => + item.code !== "graph_incompatible" || + !findings.some((detail) => detail.severity === "blocker") + ) + ); // Native start refuses on this flag alone; never report ready against it. if ( reply.plan.enrollment_compatible !== true && @@ -1527,7 +1629,7 @@ export async function collectNativePreflight( return true; }); for (const found of runtime.findings) { - const key = `${found.severity}\0${found.code}\0${found.service ?? ""}\0${found.field ?? ""}`; + const key = `${found.severity}\0${found.code}\0${found.service ?? ""}\0${found.field ?? ""}\0${found.message}`; if ( reported.has(`${found.code}\0${found.service ?? ""}`) || exact.has(key) @@ -1539,10 +1641,14 @@ export async function collectNativePreflight( } const blockers = all.filter((found) => found.severity === "blocker"); return { - report_version: 1, + report_version: 2, backend: "native", compose_file: relative(opts.projectRoot, opts.composeFile), - ready: blockers.length === 0 && runtime.review.status === "completed", + ready: + blockers.length === 0 && + !all.some((found) => found.severity === "consent") && + runtime.review.status === "completed", + consent_needed: all.filter((found) => found.severity === "consent"), runtime_review: runtime.review, blockers, warnings: all.filter((found) => found.severity === "warning"), @@ -1606,6 +1712,12 @@ export function renderNativePreflightReport( lines.push(...findingLines(found)); } } + if (report.consent_needed.length > 0) { + lines.push("", "Consent needed before native start:"); + for (const found of report.consent_needed) { + lines.push(...findingLines(found)); + } + } if (report.warnings.length > 0) { lines.push( "", diff --git a/src/backends/native-runtime-client.ts b/src/backends/native-runtime-client.ts index fd46de9aa..426089560 100644 --- a/src/backends/native-runtime-client.ts +++ b/src/backends/native-runtime-client.ts @@ -8,6 +8,10 @@ import { } from "../lib/native-process-wrappers.ts"; import { nativeAuthoredExecOptions } from "./native-authored-exec-options.ts"; import { nativeBetaCandidatePointer } from "./native-beta.ts"; +import { + type NativeConfigurationRefusal, + readNativeConfigurationRefusals, +} from "./native-configuration-refusals.ts"; import { type NativePlanChange, nativePlanChangeGuidance, @@ -30,6 +34,7 @@ const NATIVE_TAIL = /^[1-9][0-9]{0,3}$/; const NATIVE_RUN = /^[a-f0-9]{32}$/; const NATIVE_HASH = /^[a-f0-9]{64}$/; interface NativeFailure { + readonly configurationRefusals?: readonly NativeConfigurationRefusal[]; readonly code: string; readonly logPhase?: NativeLogFailurePhase; readonly causeCode?: string; @@ -179,6 +184,9 @@ export class NativeRuntimeRequestError extends ClosedCliError { readonly nativeMessage: string | undefined; readonly nativeService: string | undefined; readonly nativeStopFailures: readonly NativeStopFailure[] | undefined; + readonly nativeConfigurationRefusals: + | readonly NativeConfigurationRefusal[] + | undefined; constructor(opts: { readonly stopSignal?: "SIGINT" | "SIGTERM"; @@ -189,6 +197,7 @@ export class NativeRuntimeRequestError extends ClosedCliError { readonly nativeMessage?: string; readonly nativeService?: string; readonly nativeStopFailures?: readonly NativeStopFailure[]; + readonly nativeConfigurationRefusals?: readonly NativeConfigurationRefusal[]; readonly nativeStartupPhase?: "static-preflight"; }) { super(opts.message); @@ -198,6 +207,7 @@ export class NativeRuntimeRequestError extends ClosedCliError { this.nativeMessage = opts.nativeMessage; this.nativeService = opts.nativeService; this.nativeStopFailures = opts.nativeStopFailures; + this.nativeConfigurationRefusals = opts.nativeConfigurationRefusals; if (opts.nativeCode && ERROR_CODE.test(opts.nativeCode)) { retainedFailures.set( this, @@ -286,6 +296,18 @@ export interface NativeRuntimeSelection { const MAX_OUTPUT_BYTES = 16 * 1024 * 1024; const MAX_PRIVATE_INPUT_BYTES = 256 * 1024; +// A read-only Compose review can report cache-label errors for all 64 services. +// Keep ordinary control errors at 8 KiB and never admit startup output here. +const MAX_PLAN_FAILURE_BYTES = 1024 * 1024; + +function failureReplyBudget( + args: readonly string[], + privateInput?: Uint8Array +): number { + return args[0] === "project" && args[1] === "plan" && !privateInput + ? MAX_PLAN_FAILURE_BYTES + : 8192; +} /** * Exit-relative bound on draining an owned native child's stdout and stderr. @@ -432,7 +454,11 @@ export async function invokeNativeRuntime(opts: { drainTimer = setTimeout(() => drain.abort(), exitDrainBoundMs); }); } - const failureDetails = readNativeFailure(child.stderr, drain?.signal); + const failureDetails = readNativeFailure( + child.stderr, + drain?.signal, + failureReplyBudget(opts.args, opts.privateInput) + ); let timedOut = false; let canceled = false; const cancel = () => { @@ -714,6 +740,9 @@ function completionResponse( nativeMessage: timedOut ? undefined : failure?.message, nativeService: timedOut ? undefined : failure?.service, nativeStopFailures: timedOut ? undefined : failure?.stopFailures, + nativeConfigurationRefusals: timedOut + ? undefined + : failure?.configurationRefusals, }); } try { @@ -847,6 +876,9 @@ function nativeFailureFromReply( ...(logPhase ? { logPhase } : {}), ...(causeCode ? { causeCode } : {}), ...nativeFailureDetail(value), + configurationRefusals: readNativeConfigurationRefusals( + value.configuration_refusals + ), ...(planChange ? { planChange } : {}), ...(["engine_protocol", "engine_rejected", "engine_not_found"].includes( value.code === "graph_owner_recovery" && @@ -874,7 +906,8 @@ function processRefusalsOf(value: unknown): { async function readNativeFailure( stream: ReadableStream, signal?: AbortSignal, - limit = 8192 + limit = 8192, + startupLogs = false ): Promise { const reader = stream.getReader(); const cancel = () => { @@ -903,7 +936,7 @@ async function readNativeFailure( } return nativeFailureFromReply( JSON.parse(text + decoder.decode()), - limit > 8192 + startupLogs ); } catch { return; @@ -971,7 +1004,8 @@ export async function readNativeStartupFailure( const failure = await readNativeFailure( stream, signal, - NATIVE_STARTUP_FAILURE_REPLY_BYTES + NATIVE_STARTUP_FAILURE_REPLY_BYTES, + true ); return failure === undefined ? undefined diff --git a/src/backends/native-start-refusals.ts b/src/backends/native-start-refusals.ts index b1a06e056..f8bd6d1d5 100644 --- a/src/backends/native-start-refusals.ts +++ b/src/backends/native-start-refusals.ts @@ -1,10 +1,15 @@ import { isRecord } from "../lib/guards.ts"; +import { + type NativeConfigurationRefusal, + readNativeConfigurationRefusals, +} from "./native-configuration-refusals.ts"; import { nativeFailureDetail } from "./native-runtime-client.ts"; const CODE = /^[a-z][a-z0-9_]{0,63}$/; export type NativeStartRefusal = { readonly code: string; + readonly configuration?: readonly NativeConfigurationRefusal[]; readonly message?: string; readonly service?: string; }; @@ -29,6 +34,11 @@ export function readNativeStartRefusals( const detail = nativeFailureDetail(refusal); result.push({ code: refusal.code, + configuration: readNativeConfigurationRefusals( + refusal.configuration_refusals + )?.filter( + (item) => item.service === undefined || services.includes(item.service) + ), ...(detail.message ? { message: detail.message } : {}), ...(detail.service && services.includes(detail.service) ? { service: detail.service } diff --git a/src/commands/doctor-native-preflight.ts b/src/commands/doctor-native-preflight.ts index 81859d344..26122b178 100644 --- a/src/commands/doctor-native-preflight.ts +++ b/src/commands/doctor-native-preflight.ts @@ -212,5 +212,7 @@ export async function runNativePreflightCommand(opts: { } else { process.stdout.write(renderNativePreflightReport(report)); } - return report.blockers.length === 0 ? 0 : 1; + return report.blockers.length === 0 && report.consent_needed.length === 0 + ? 0 + : 1; } diff --git a/src/lib/native-compose-backend-features.ts b/src/lib/native-compose-backend-features.ts index 1ca0b04ac..9f765c099 100644 --- a/src/lib/native-compose-backend-features.ts +++ b/src/lib/native-compose-backend-features.ts @@ -109,6 +109,22 @@ export type ComposeBackendFeatureOptions = { readonly buildsEnabled?: boolean; }; +/** Service locations from the same platform predicate used for admission. */ +function foreignPlatformServices( + compose: unknown, + emulated: ReadonlySet = new Set() +): string[] { + if (!(isRecord(compose) && isRecord(compose.services))) { + return []; + } + return Object.entries(compose.services) + .filter( + ([name, service]) => + isRecord(service) && isForeignPlatform(service, emulated.has(name)) + ) + .map(([name]) => name); +} + /** * Compose treats `external: true` and the legacy `external: {name: ...}` form * as a network the project does not own. The runtime's reviewed @@ -372,6 +388,7 @@ export function hasBasicComposeBuild(compose: unknown): boolean { /** What doctor reports for one Compose file: closed names and flags only. */ export type ComposeBackendReading = { + readonly foreignPlatformServices?: readonly string[]; readonly features: ComposeBackendFeature[]; /** A basic build that native image-project `up` refuses; see {@link hasBasicComposeBuild}. */ readonly basicBuild: boolean; @@ -400,12 +417,16 @@ export async function classifyComposeBackendReading( opts?: Parameters[1] ): Promise { const buildsEnabled = opts?.buildsEnabled === true; + const foreign: string[] = []; + const features = await classifyAdapted( + compose, + adaptationCandidates(opts), + buildsEnabled, + foreign + ); return { - features: await classifyAdapted( - compose, - adaptationCandidates(opts), - buildsEnabled - ), + features, + ...(foreign.length ? { foreignPlatformServices: foreign } : {}), // With native builds enabled, native `up` builds a basic build itself. basicBuild: !buildsEnabled && hasBasicComposeBuild(compose), }; @@ -440,7 +461,8 @@ function intersect( async function classifyAdapted( compose: unknown, candidates: readonly (string | undefined)[], - buildsEnabled: boolean + buildsEnabled: boolean, + foreign?: string[] ): Promise { const features = findComposeBackendFeatures(compose, { buildsEnabled }); const network = features.includes("external_network"); @@ -449,6 +471,7 @@ async function classifyAdapted( candidates.every((candidate) => candidate === undefined) || !(network || platform) ) { + foreign?.push(...foreignPlatformServices(compose)); return features; } const services = @@ -475,6 +498,7 @@ async function classifyAdapted( ); } } + foreign?.push(...foreignPlatformServices(compose, emulated)); return findComposeBackendFeatures(compose, { isolatedNetworks: isolated, emulatedServices: emulated, diff --git a/tests/native-configuration-refusals.test.ts b/tests/native-configuration-refusals.test.ts new file mode 100644 index 000000000..12ec94d4f --- /dev/null +++ b/tests/native-configuration-refusals.test.ts @@ -0,0 +1,83 @@ +import { expect, test } from "bun:test"; +import { readFile } from "node:fs/promises"; +import { readNativeConfigurationRefusals } from "../src/backends/native-configuration-refusals.ts"; + +const detail = { + text_fixed: true, + service: "api.worker", + field: "healthcheck.x-hack-http", + message: "The route needs a native HTTP probe.", + fix: "Declare the probe on the route port.", +}; + +test("the runtime configuration field and literal text vocabulary fits the decoder", async () => { + const paths = [ + "provider/graph/config.rs", + "provider/graph/routes.rs", + "project/compose/dependency_cache.rs", + "project/compose/service.rs", + ]; + let count = 0; + for (const path of paths) { + const source = + (await readFile(`packages/runtime-core/src/${path}`, "utf8")).split( + "#[cfg(test)]" + )[0] ?? ""; + // All producers use these literal-only constructors, including closures + // and detail()'s closed key-to-field match. Ignore test fixture literals. + const calls = + /(?:ConfigurationRefusal::new|refuse|detail)\(\s*("(?:[^"\\]|\\.)*"|[A-Z_]+|key|field)\s*,\s*("(?:[^"\\]|\\.)*")\s*,\s*("(?:[^"\\]|\\.)*")/g; + for (const match of source.matchAll(calls)) { + const token = match[1] ?? ""; + const field = token.startsWith('"') + ? (JSON.parse(token) as string) + : "labels"; + const value = { + ...detail, + field: field || "labels", + message: JSON.parse(match[2] ?? "null") as unknown, + fix: JSON.parse(match[3] ?? "null") as unknown, + }; + expect( + readNativeConfigurationRefusals([value]), + `${path}: ${field}` + ).toBeDefined(); + count++; + } + for (const match of source.matchAll(/=>\s*"(labels(?:\.[a-z.-]+)?)"/g)) { + expect( + readNativeConfigurationRefusals([{ ...detail, field: match[1] }]), + path + ).toBeDefined(); + } + } + expect(count).toBeGreaterThanOrEqual(20); +}); + +test("configuration details accept only literal-marked, bounded field and service locations", () => { + expect( + readNativeConfigurationRefusals([{ ...detail, raw: "private-sentinel" }]) + ).toEqual([ + { + service: detail.service, + field: detail.field, + message: detail.message, + fix: detail.fix, + }, + ]); + for (const bad of [ + null, + {}, + { ...detail, text_fixed: false }, + { ...detail, service: "../private-sentinel" }, + { ...detail, field: "labels.private-sentinel" }, + { ...detail, message: "private-sentinel\n" }, + { ...detail, fix: "x".repeat(401) }, + { ...detail, service: "s".repeat(129) }, + ]) { + expect(readNativeConfigurationRefusals([detail, bad])).toBeUndefined(); + } + expect( + readNativeConfigurationRefusals(Array.from({ length: 4097 }, () => detail)) + ).toBeUndefined(); +}); diff --git a/tests/native-preflight-cli.test.ts b/tests/native-preflight-cli.test.ts index 44dd53fff..12e8c8e88 100644 --- a/tests/native-preflight-cli.test.ts +++ b/tests/native-preflight-cli.test.ts @@ -215,6 +215,180 @@ function codes(report: NativePreflightReport, gate: "cli" | "runtime") { const GRANT = { HACK_NATIVE_SHARED_SOURCE: "1" }; +test( + "an unreportable service name retains the foreign-platform refusal", + async () => { + const f = await fixture({ + services: { + ["s".repeat(129)]: { image: IMAGE, platform: "linux/amd64" }, + }, + }); + await f.review([]); + const { report, code } = await preflight(f, { ...f.native, ...GRANT }); + expect(code).toBe(1); + expect(report.ready).toBe(false); + expect( + report.blockers.some( + (item) => item.code === "foreign_platform" && item.service === undefined + ) + ).toBe(true); + }, + SPAWN_TIMEOUT_MS +); + +test( + "consent never hides per-service configuration findings in text or JSON", + async () => { + const f = await fixture({ + services: { + api: { image: IMAGE, environment: { PRIVATE } }, + worker: { image: IMAGE }, + }, + }); + const detail = (service: string, field: string) => ({ + text_fixed: true, + service, + field, + message: "This service configuration is not supported.", + fix: "Remove this field or use a supported declaration.", + value: PRIVATE, + }); + await f.review( + [ + { + severity: "error", + code: "privileged_service", + field: "services.api", + message: PRIVATE, + }, + ], + {}, + "both", + [ + { + code: "graph_incompatible", + message_fixed: true, + message: "The reviewed graph contains blocking diagnostics.", + }, + { + code: "graph_subset", + configuration_refusals: [ + detail("api", "labels"), + detail("worker", "platform"), + ], + }, + { + code: "graph_route_binding", + configuration_refusals: [detail("api", "healthcheck.x-hack-http")], + }, + ] + ); + const ungranted = await preflight(f, f.native); + const granted = await preflight(f, { ...f.native, ...GRANT }); + expect(ungranted.code).toBe(1); + expect(ungranted.report.blockers).toEqual(granted.report.blockers); + expect(ungranted.report.consent_needed.map((item) => item.code)).toEqual([ + "shared_source_grant_required", + ]); + expect(granted.report.consent_needed).toEqual([]); + expect( + ungranted.report.blockers.map((item) => [ + item.code, + item.service, + item.field, + ]) + ).toEqual([ + ["privileged_service", "api", "services.api.privileged"], + ["graph_subset", "api", "labels"], + ["graph_subset", "worker", "platform"], + ["graph_route_binding", "api", "healthcheck.x-hack-http"], + ]); + const text = await cli(f, ["doctor", "--native-preflight"], f.native); + expect(text.output).toContain("Consent needed before native start:"); + expect(text.output).toContain("4 blockers:"); + expect(text.output).toContain("healthcheck.x-hack-http"); + expect(text.output + ungranted.output + granted.output).not.toContain( + PRIVATE + ); + expect( + (await f.calls()).every((line) => line.includes("project plan")) + ).toBe(true); + }, + SPAWN_TIMEOUT_MS +); + +test( + "cache review errors carry every service and field across the error pipe", + async () => { + const services = Object.fromEntries( + Array.from({ length: 64 }, (_, index) => [`s${index}`, { image: IMAGE }]) + ); + const f = await fixture({ services }); + const configuration_refusals = Object.keys(services).flatMap((service) => + [ + "labels.hack.dependencies.lockfiles", + "labels.hack.dependencies.runtime-files", + ].map((field) => ({ + text_fixed: true, + service, + field, + message: + "The dependency-cache file list is not a bounded list of relative paths.", + fix: "Use relative paths without interpolation or parent components.", + })) + ); + await f.reply("project-plan.err", { + code: "invalid_dependency_cache", + message: PRIVATE, + configuration_refusals, + }); + const { report, output } = await preflight(f, f.native); + expect(report.runtime_review).toEqual({ + status: "stopped", + code: "invalid_dependency_cache", + }); + expect(report.blockers).toHaveLength(128); + expect(new Set(report.blockers.map((item) => item.service)).size).toBe(64); + expect(report.consent_needed).toHaveLength(1); + expect(output).not.toContain(PRIVATE); + await f.reply("project-plan.err", { + code: "invalid_dependency_cache", + configuration_refusals: [ + { ...configuration_refusals[0], text_fixed: false, message: PRIVATE }, + ], + }); + const malformed = await preflight(f, f.native); + expect(malformed.report.blockers).toHaveLength(1); + expect(malformed.output).not.toContain(PRIVATE); + }, + SPAWN_TIMEOUT_MS +); + +test( + "foreign platform findings name all offending services without platform values", + async () => { + const f = await fixture({ + services: { + api: { image: IMAGE, platform: `linux/${PRIVATE}` }, + worker: { image: IMAGE, platform: "linux/amd64" }, + native: { image: IMAGE, platform: "linux/arm64" }, + }, + }); + await f.review([]); + const { report, output } = await preflight(f, { ...f.native, ...GRANT }); + expect( + report.blockers + .filter((item) => item.code === "foreign_platform") + .map((item) => [item.service, item.field]) + ).toEqual([ + ["api", "platform"], + ["worker", "platform"], + ]); + expect(output).not.toContain(PRIVATE); + }, + SPAWN_TIMEOUT_MS +); + /** Replies that carry native `up` past its source checks to service admission. */ async function scriptSourceShare(f: Fixture): Promise { await f.reply("runtime-check-project-share.out", { @@ -278,7 +452,7 @@ test( ]); const { report, code, output } = await preflight(f, f.native); expect(code).toBe(1); - expect(report.report_version).toBe(1); + expect(report.report_version).toBe(2); expect(report.backend).toBe("native"); expect(report.compose_file).toBe(".hack/docker-compose.yml"); expect(report.ready).toBe(false); @@ -292,7 +466,6 @@ test( "extra_hosts_without_dependencies:web", "invalid_restart:jobs", "missing_image:jobs", - "shared_source_grant_required", "unsupported_label:api", "unsupported_logging:db", ]); @@ -527,7 +700,10 @@ test( // Without the grant, both name the grant. const ungranted = await fixture({ services: { web: { image: IMAGE } } }); const report = (await preflight(ungranted, ungranted.native)).report; - expect(codes(report, "cli")).toEqual(["shared_source_grant_required"]); + expect(codes(report, "cli")).toEqual([]); + expect(report.consent_needed.map((item) => item.code)).toEqual([ + "shared_source_grant_required", + ]); const refused = await cli(ungranted, ["up"], ungranted.native); expect(refused.output).toContain("E_PERMISSION"); // A clean project has no CLI blocker and native up goes on to the runtime. diff --git a/tests/native-runtime-client.test.ts b/tests/native-runtime-client.test.ts index 61ed069ce..5307b38a6 100644 --- a/tests/native-runtime-client.test.ts +++ b/tests/native-runtime-client.test.ts @@ -148,6 +148,7 @@ async function fixture() { binary, `#!${process.execPath} if (process.argv.includes("hang")) await Bun.sleep(60_000); +if (process.argv.includes("configuration-batch")) { console.error(JSON.stringify({code:"invalid_dependency_cache",message:"synthetic-secret-diagnostic",configuration_refusals:Array.from({length:128},(_,i)=>({text_fixed:true,service:"s"+i,field:"labels.hack.dependencies.lockfiles",message:"The file list is invalid.",fix:"Use relative paths without variables."})),startup_logs:[{service:"web",lines:["synthetic-secret-diagnostic"]}]})); process.exit(2); } if (process.argv.includes("old-provider")) { console.error(JSON.stringify({code:"provider_pin_changed",message:"synthetic-secret-diagnostic"})); process.exit(2); } if (process.argv.includes("fixed-message")) { console.error(JSON.stringify({code:"graph_image",message:"Pinned image architecture or implicit volumes are incompatible.",message_fixed:true,service:"web"})); process.exit(2); } if (process.argv.includes("bridge-socket-failure")) { console.error(JSON.stringify({code:"bridge_socket_timeout",message:"Native bridge sockets did not become private and owned before the startup deadline. Run the same selected executor with the identical --candidate-root argument used for the failed start, followed by runtime recover --json before retrying.",message_fixed:true})); process.exit(2); } @@ -775,6 +776,40 @@ test("managed input uses stdin with EOF and is absent from argv and inherited en } }); +test("large configuration batches are limited to read-only plan errors", async () => { + const runtime = await fixture(); + const plan = await invokeNativeRuntime({ + runtime, + cwd: runtime.home, + args: [ + "project", + "plan", + "--project", + runtime.home, + "--file", + "configuration-batch", + "--json", + ], + }).catch((error: unknown) => error); + expect(plan).toBeInstanceOf(NativeRuntimeRequestError); + expect(plan).toMatchObject({ nativeCode: "invalid_dependency_cache" }); + if (!(plan instanceof NativeRuntimeRequestError)) { + throw new Error("Expected plan refusal"); + } + expect(plan.nativeConfigurationRefusals).toHaveLength(128); + expect(JSON.stringify(plan)).not.toContain("synthetic-secret-diagnostic"); + const control = await invokeNativeRuntime({ + runtime, + cwd: runtime.home, + args: ["graph", "configuration-batch"], + }).catch((error: unknown) => error); + expect(control).toBeInstanceOf(NativeRuntimeRequestError); + expect(control).toMatchObject({ + nativeCode: undefined, + nativeConfigurationRefusals: undefined, + }); +}); + test("native failure preserves only its validated structured code", async () => { const runtime = await fixture(); try { From 056b3334d30e8554e69550b281d4b85b809873b3 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 21:32:30 +0000 Subject: [PATCH 2/4] test(native): handle early staging refusals and bound fixture setup --- .../runtime-core/src/provider/environment.rs | 31 ++++++++++++++++--- ...e-compose-adoption-file-generation.test.ts | 4 ++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/runtime-core/src/provider/environment.rs b/packages/runtime-core/src/provider/environment.rs index 9ca05fde2..0d1d9f479 100644 --- a/packages/runtime-core/src/provider/environment.rs +++ b/packages/runtime-core/src/provider/environment.rs @@ -942,7 +942,11 @@ mod tests { } /// Runs the guest member-staging function on the host shell against a scratch root. /// `stat -c` is avoided there so the same body runs on macOS and the Linux guest. - fn stage_members(layout: &str, input: &[u8]) -> (bool, std::path::PathBuf) { + fn stage_members( + layout: &str, + input: &[u8], + wait_for_refusal: bool, + ) -> (bool, std::path::PathBuf) { use std::{ io::Write, process::{Command, Stdio}, @@ -977,12 +981,31 @@ mod tests { .stderr(Stdio::piped()) .spawn() .unwrap(); - child.stdin.take().unwrap().write_all(input).unwrap(); + let mut stdin = child.stdin.take().unwrap(); + if wait_for_refusal { + // Exercise the scheduler order where malformed layout is refused + // before the parent gets to write, without a timing-dependent sleep. + assert!(!child.wait().unwrap().success()); + } + let written = stdin.write_all(input); + drop(stdin); let output = child.wait_with_output().unwrap(); + if let Err(error) = written { + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); + assert!(!output.status.success()); + assert!(output.stdout.is_empty()); + } assert!(output.stderr.is_empty()); (output.stdout == b"staged\n", root) } #[test] + fn staging_refusal_before_input_is_reaped_and_never_reports_success() { + let (ok, root) = stage_members("x a:1:1", b"synthetic-only", true); + assert!(!ok); + assert!(!root.join("values.json").exists()); + std::fs::remove_dir_all(root).unwrap(); + } + #[test] fn member_staging_reads_exact_lengths_and_refuses_malformed_input() { use std::os::unix::fs::PermissionsExt; let deadline = Instant::now() + Duration::from_secs(60); @@ -999,7 +1022,7 @@ mod tests { let staged = PendingEnvironment::until_with_files("web", &values, &files, deadline).unwrap(); let (layout, input) = staged.staging_input(); - let (ok, root) = stage_members(&layout, input.as_bytes()); + let (ok, root) = stage_members(&layout, input.as_bytes(), false); assert!(ok); assert_eq!( std::fs::read(root.join("values.json")).unwrap(), @@ -1055,7 +1078,7 @@ mod tests { bytes.to_vec(), ), ] { - let (ok, root) = stage_members(&layout, &input); + let (ok, root) = stage_members(&layout, &input, false); assert!(!ok, "{layout}"); for name in ["files/a.json", "files/b.bin", "files/a", "files/b"] { let _ = std::fs::set_permissions( diff --git a/tests/native-compose-adoption-file-generation.test.ts b/tests/native-compose-adoption-file-generation.test.ts index 6b48c1756..fe9e7f5d3 100644 --- a/tests/native-compose-adoption-file-generation.test.ts +++ b/tests/native-compose-adoption-file-generation.test.ts @@ -814,6 +814,8 @@ test.each([ }, 30_000); test("HACK-1359: file8 rejects missing and non-finite mutation deadlines without effects", async () => { + // Preparation still performs the full retained-file proof before the invalid + // deadline assertions. Give it the same setup budget as the adjacent cases. const { store, generation } = await prepared(); try { await assertInvalidRetainedMutationDeadlines({ @@ -828,4 +830,4 @@ test("HACK-1359: file8 rejects missing and non-finite mutation deadlines without } finally { await store.close(); } -}); +}, 30_000); From 294019d88fb138f8a6edadf011dbf159437d50ad Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 22:25:24 +0000 Subject: [PATCH 3/4] fix(native): preserve configuration refusals during authored startup --- docs/guides/native-candidate.md | 6 ++ packages/runtime-core/src/error.rs | 3 +- .../runtime-core/src/provider/graph/config.rs | 8 +-- src/backends/native-authored-project-start.ts | 28 +++++++- src/backends/native-runtime-client.ts | 26 ++----- .../helpers/native-authored-start-fixture.ts | 3 +- tests/native-authored-project-start.test.ts | 72 +++++++++++++++++++ tests/native-runtime-client.test.ts | 48 ++++++++++--- 8 files changed, 157 insertions(+), 37 deletions(-) diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index e6b61d97e..8d49cf8f7 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -263,6 +263,12 @@ Report version 2 moves `shared_source_grant_required` from `blockers` into `consent_needed`. Consumers should use `ready` for the overall verdict and inspect both lists for what remains to do. +Configuration finding `field` paths are relative to the named service (for +example, `labels.hack.dependencies.lockfiles`); plan diagnostics use absolute +paths such as `services.api.privileged`. Large configuration refusal batches +retain their code and service locations during authored startup as well as +preflight. Raw runtime stderr and startup output are not configuration findings. + ```json { "report_version": 2, diff --git a/packages/runtime-core/src/error.rs b/packages/runtime-core/src/error.rs index 62d5e228d..34f848413 100644 --- a/packages/runtime-core/src/error.rs +++ b/packages/runtime-core/src/error.rs @@ -236,7 +236,8 @@ impl CandidateError { pub fn with_configuration_refusals(mut self, mut findings: Vec) -> Self { // Malformed lists can repeat the same field thousands of times. Keep // every distinct explanation, not one copy per authored value. This - // also bounds the serialized review error below the CLI pipe budget. + // also bounds current producers below the CLI's shared 1 MiB error + // envelope, including planning and startup consumers. let mut seen = std::collections::BTreeSet::new(); findings.retain(|item| seen.insert((item.service.clone(), item.field, item.message))); self.refusals diff --git a/packages/runtime-core/src/provider/graph/config.rs b/packages/runtime-core/src/provider/graph/config.rs index 233e2e25b..4ffd15707 100644 --- a/packages/runtime-core/src/provider/graph/config.rs +++ b/packages/runtime-core/src/provider/graph/config.rs @@ -628,6 +628,7 @@ fn declared_configuration( "graph_budget", "A service exceeds the bounded graph profile.", ) + .with_service(name) }); } // ShmSize is a tmpfs ceiling, not preallocated RSS. Its actual usage is @@ -1285,10 +1286,9 @@ mod compose_field_tests { let first = plan.services.get_mut("api").unwrap(); first.platform = None; first.network_mode = "project-networks".into(); - assert_eq!( - declared_configuration(&plan, true, true).unwrap_err().code, - "graph_budget" - ); + let budget = declared_configuration(&plan, true, true).unwrap_err(); + assert_eq!(budget.code, "graph_budget"); + assert_eq!(budget.service.as_deref(), Some("api")); } #[test] diff --git a/src/backends/native-authored-project-start.ts b/src/backends/native-authored-project-start.ts index eb57a40e9..db671b177 100644 --- a/src/backends/native-authored-project-start.ts +++ b/src/backends/native-authored-project-start.ts @@ -61,6 +61,10 @@ import { nativeAuthoredStorageToolRequired, resolveNativeAuthoredStorageTool, } from "./native-authored-storage-tool.ts"; +import { + type NativeConfigurationRefusal, + readNativeConfigurationRefusals, +} from "./native-configuration-refusals.ts"; import { type NativeExitDiagnostic, serveNativeAuthoredProjectGraph, @@ -133,6 +137,7 @@ export class NativeAuthoredProjectStartError extends Error { readonly nativeCauseCode?: string; readonly stage: Stage; readonly compilerCode?: string; + readonly nativeConfigurationRefusals?: readonly NativeConfigurationRefusal[]; constructor(opts: { readonly outcome: Outcome; @@ -141,6 +146,7 @@ export class NativeAuthoredProjectStartError extends Error { readonly nativeCauseCode?: string; readonly stage?: Stage; readonly compilerCode?: string; + readonly configurationRefusals?: unknown; }) { const nativeCode = opts.nativeCode && CODE.test(opts.nativeCode) @@ -161,8 +167,17 @@ export class NativeAuthoredProjectStartError extends Error { retained: "startup evidence is retained; inspect owned state before retrying", }[opts.outcome]; + const configuration = readNativeConfigurationRefusals( + opts.configurationRefusals + ); + const findings = configuration + ?.map( + (item) => + `${item.service ? `Service ${item.service}, ` : ""}${item.field}: ${item.message} ${item.fix}` + ) + .join("\n"); super( - `Native authored startup failed${nativeCode ? ` (${nativeCode})` : ""}; stage ${stage}${compilerCode ? ` (${compilerCode})` : ""}; ${detail}. Values omitted.` + `Native authored startup failed${nativeCode ? ` (${nativeCode})` : ""}; stage ${stage}${compilerCode ? ` (${compilerCode})` : ""}; ${detail}. Values omitted.${findings ? `\n${findings}` : ""}` ); this.outcome = opts.outcome; this.canceled = opts.canceled; @@ -173,6 +188,7 @@ export class NativeAuthoredProjectStartError extends Error { : undefined; this.stage = stage; this.compilerCode = compilerCode; + this.nativeConfigurationRefusals = configuration; } } @@ -856,6 +872,16 @@ async function failedAdmission(opts: { nativeCauseCode, stage: opts.stage, compilerCode: compilerDiagnostic(error), + // These entries have already passed the runtime decoder. Restore only its + // literal-text marker for the startup error's independently checked boundary. + configurationRefusals: + error instanceof NativeRuntimeRequestError && + error.nativeCode === nativeCode + ? error.nativeConfigurationRefusals?.map((item) => ({ + ...item, + text_fixed: true, + })) + : undefined, }); } diff --git a/src/backends/native-runtime-client.ts b/src/backends/native-runtime-client.ts index 426089560..c7f545d8e 100644 --- a/src/backends/native-runtime-client.ts +++ b/src/backends/native-runtime-client.ts @@ -296,18 +296,10 @@ export interface NativeRuntimeSelection { const MAX_OUTPUT_BYTES = 16 * 1024 * 1024; const MAX_PRIVATE_INPUT_BYTES = 256 * 1024; -// A read-only Compose review can report cache-label errors for all 64 services. -// Keep ordinary control errors at 8 KiB and never admit startup output here. -const MAX_PLAN_FAILURE_BYTES = 1024 * 1024; - -function failureReplyBudget( - args: readonly string[], - privateInput?: Uint8Array -): number { - return args[0] === "project" && args[1] === "plan" && !privateInput - ? MAX_PLAN_FAILURE_BYTES - : 8192; -} +// Configuration refusals can originate during planning or startup, including +// authored graph native plan/frontend-plan. Keep the same bounded envelope for +// every structured error; startup output still requires its explicit decoder. +const MAX_NATIVE_FAILURE_BYTES = 1024 * 1024; /** * Exit-relative bound on draining an owned native child's stdout and stderr. @@ -454,11 +446,7 @@ export async function invokeNativeRuntime(opts: { drainTimer = setTimeout(() => drain.abort(), exitDrainBoundMs); }); } - const failureDetails = readNativeFailure( - child.stderr, - drain?.signal, - failureReplyBudget(opts.args, opts.privateInput) - ); + const failureDetails = readNativeFailure(child.stderr, drain?.signal); let timedOut = false; let canceled = false; const cancel = () => { @@ -906,7 +894,7 @@ function processRefusalsOf(value: unknown): { async function readNativeFailure( stream: ReadableStream, signal?: AbortSignal, - limit = 8192, + limit = MAX_NATIVE_FAILURE_BYTES, startupLogs = false ): Promise { const reader = stream.getReader(); @@ -1004,7 +992,7 @@ export async function readNativeStartupFailure( const failure = await readNativeFailure( stream, signal, - NATIVE_STARTUP_FAILURE_REPLY_BYTES, + Math.max(NATIVE_STARTUP_FAILURE_REPLY_BYTES, MAX_NATIVE_FAILURE_BYTES), true ); return failure === undefined diff --git a/tests/helpers/native-authored-start-fixture.ts b/tests/helpers/native-authored-start-fixture.ts index 636d8ce7d..60d967c25 100644 --- a/tests/helpers/native-authored-start-fixture.ts +++ b/tests/helpers/native-authored-start-fixture.ts @@ -270,6 +270,7 @@ export function createNativeAuthoredStartHarness() { readonly planFailure?: boolean; /** Closed native code for `planFailure` (default `native_graph_compile`). */ readonly planFailureCode?: string; + readonly planConfigurationRefusals?: readonly unknown[]; readonly failBeforeReceipt?: boolean; readonly removedBeforeReady?: boolean; readonly foreignRemovalReview?: boolean; @@ -576,7 +577,7 @@ if(['plan','serve','frontend-plan','frontend-serve'].includes(action)){ review.review_id=createHash('sha256').update('hack.native-graph-review/v1\\0').update(JSON.stringify(provenance)).digest('hex'); } if(planning){ - if(${options.planFailure === true}){console.error(JSON.stringify({code:${JSON.stringify(options.planFailureCode ?? "native_graph_compile")},message:'private-rejected-plan-detail'}));process.exit(2)} + if(${options.planFailure === true}){console.error(JSON.stringify({code:${JSON.stringify(options.planFailureCode ?? "native_graph_compile")},message:'private-rejected-plan-detail',configuration_refusals:${JSON.stringify(options.planConfigurationRefusals)}}));process.exit(2)} if(${options.changeDuringPlan === true})await appendFile(${JSON.stringify(envFile)},'\\n'); console.log(JSON.stringify(review));process.exit(0); } diff --git a/tests/native-authored-project-start.test.ts b/tests/native-authored-project-start.test.ts index e256562a7..db7670ecb 100644 --- a/tests/native-authored-project-start.test.ts +++ b/tests/native-authored-project-start.test.ts @@ -43,6 +43,78 @@ const { groupEnded, } = createNativeAuthoredStartHarness(); +const configurationRefusals = Array.from({ length: 12 }, (_, index) => + ["lockfiles", "runtime-files", "cache-volume", "bootstrap"].map((field) => ({ + text_fixed: true, + service: `service${index}`, + field: `labels.hack.dependencies.${field}`, + message: "The dependency cache field is invalid.", + fix: "Correct this field in the service configuration before starting the project. Use a declared, mounted named volume and relative file paths without variables or parent traversal.", + })) +).flat(); + +test("authored startup keeps validated configuration findings and drops malformed batches", () => { + expect(JSON.stringify(configurationRefusals).length).toBeGreaterThan(8192); + const error = new NativeAuthoredProjectStartError({ + outcome: "not-started", + canceled: false, + stage: "native-plan", + nativeCode: "invalid_dependency_cache", + configurationRefusals, + }); + expect(error.nativeCode).toBe("invalid_dependency_cache"); + expect(error.nativeConfigurationRefusals).toHaveLength(48); + for (let index = 0; index < 12; index++) { + expect(error.message).toContain( + `Service service${index}, labels.hack.dependencies.` + ); + } + const malformed = new NativeAuthoredProjectStartError({ + outcome: "not-started", + canceled: false, + configurationRefusals: [...configurationRefusals, { message: CANARY }], + }); + expect(malformed.nativeConfigurationRefusals).toBeUndefined(); + expect(JSON.stringify(malformed)).not.toContain(CANARY); + expect(malformed.message).not.toContain(CANARY); +}); + +macTest( + "authored up preserves a configuration refusal larger than 8 KiB before reservation", + async () => { + const selected = await fixture({ + planFailure: true, + planFailureCode: "invalid_dependency_cache", + planConfigurationRefusals: configurationRefusals, + }); + const error = await failure( + serveNativeAuthoredProject({ + ...selected, + run, + startupTimeoutMs: STARTUP_TIMEOUT_MS, + }), + "not-started" + ); + expect(error).toHaveProperty("stage", "native-plan"); + expect(error).toHaveProperty("nativeCode", "invalid_dependency_cache"); + if (!(error instanceof NativeAuthoredProjectStartError)) { + throw new Error("Expected authored startup refusal"); + } + expect(error.nativeConfigurationRefusals).toHaveLength(48); + for (let index = 0; index < 12; index++) { + expect(error.message).toContain( + `Service service${index}, labels.hack.dependencies.` + ); + } + expect(await artifacts(selected.scope)).toEqual([]); + expect(await Bun.file(selected.delivery).exists()).toBe(false); + const calls = await Bun.file(selected.calls).text(); + expect(calls).toContain('"plan","--source-file"'); + expect(calls).not.toContain('"serve"'); + expect(JSON.stringify(error)).not.toContain("private-rejected-plan-detail"); + } +); + macTest( "HACK-1272: a consumer refused before journal reservation retires its exact start intent", async () => { diff --git a/tests/native-runtime-client.test.ts b/tests/native-runtime-client.test.ts index 5307b38a6..9d83d9a66 100644 --- a/tests/native-runtime-client.test.ts +++ b/tests/native-runtime-client.test.ts @@ -776,7 +776,7 @@ test("managed input uses stdin with EOF and is absent from argv and inherited en } }); -test("large configuration batches are limited to read-only plan errors", async () => { +test("large configuration batches keep codes and services on every request path", async () => { const runtime = await fixture(); const plan = await invokeNativeRuntime({ runtime, @@ -798,16 +798,42 @@ test("large configuration batches are limited to read-only plan errors", async ( } expect(plan.nativeConfigurationRefusals).toHaveLength(128); expect(JSON.stringify(plan)).not.toContain("synthetic-secret-diagnostic"); - const control = await invokeNativeRuntime({ - runtime, - cwd: runtime.home, - args: ["graph", "configuration-batch"], - }).catch((error: unknown) => error); - expect(control).toBeInstanceOf(NativeRuntimeRequestError); - expect(control).toMatchObject({ - nativeCode: undefined, - nativeConfigurationRefusals: undefined, - }); + for (const args of [ + ["graph", "configuration-batch"], + [ + "graph", + "native", + "plan", + "--source-file", + "configuration-batch", + "--json", + ], + [ + "graph", + "native", + "frontend-plan", + "--source-file", + "configuration-batch", + "--json", + ], + ]) { + const control = await invokeNativeRuntime({ + runtime, + cwd: runtime.home, + args, + }).catch((error: unknown) => error); + expect(control).toBeInstanceOf(NativeRuntimeRequestError); + expect(control).toMatchObject({ nativeCode: "invalid_dependency_cache" }); + if (!(control instanceof NativeRuntimeRequestError)) { + throw new Error("Expected configuration refusal"); + } + expect( + control.nativeConfigurationRefusals?.map((item) => item.service) + ).toEqual(Array.from({ length: 128 }, (_, i) => `s${i}`)); + expect(JSON.stringify(control)).not.toContain( + "synthetic-secret-diagnostic" + ); + } }); test("native failure preserves only its validated structured code", async () => { From 6cc53d691d9db1f97422ce557d75a0674de367c0 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Sat, 14 Nov 2026 14:41:45 +0000 Subject: [PATCH 4/4] fix(native): preserve consent in upgrade preflight summaries --- docs/guides/native-candidate.md | 5 ++++- src/lib/upgrade-check-probes.ts | 15 ++++++++++++++- src/lib/upgrade-check-project.ts | 2 +- tests/native-preflight-cli.test.ts | 24 ++++++++++++++++++++++++ tests/upgrade-check.test.ts | 22 ++++++++++++++++++++++ 5 files changed, 65 insertions(+), 3 deletions(-) diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index b9422ef03..3eb0a95e4 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -199,7 +199,10 @@ values. Structural parse errors can still stop review early; fix them and rerun. Missing shared-source consent appears under **Consent needed**, separately from configuration blockers. Review still runs without consent, including in a non-interactive terminal, and `--json` retains the full discovered blocker list. -Reporting consent does not grant it or start the VM. +Reporting consent does not grant it or start the VM. The file-only native check in +`hack upgrade check` also reports outstanding consent separately and does not call +the project native-ready until it is granted; native readiness remains informational +for a Compose upgrade. The Compose input must be a nonempty regular file of at most 256 KiB, not a symbolic link. Unreadable, changing or oversized input is reported as diff --git a/src/lib/upgrade-check-probes.ts b/src/lib/upgrade-check-probes.ts index 8a7b382d2..a59757cef 100644 --- a/src/lib/upgrade-check-probes.ts +++ b/src/lib/upgrade-check-probes.ts @@ -1,4 +1,5 @@ import { resolve } from "node:path"; +import type { NativePreflightReport } from "../backends/native-preflight.ts"; import { resolveSystemdSummary } from "../commands/daemon-systemd.ts"; import { collectProjectNativePreflight } from "../commands/doctor-native-preflight.ts"; import { inspectLinuxGlobalDns } from "../commands/global-linux-dns.ts"; @@ -83,6 +84,7 @@ export type GlobalInfraProbe = { export type NativePreflightProbe = { readonly ready: boolean; + readonly consentNeeded: number; readonly blockers: number; readonly warnings: number; }; @@ -257,9 +259,20 @@ async function probeNativePreflight(opts: { profiles: [], runtimeReview: false, }); + return nativePreflightProbeFromReport(report); +} + +/** File-only upgrade check: consent stays distinct, and skipped runtime review is not a blocker. */ +export function nativePreflightProbeFromReport( + report: Pick< + NativePreflightReport, + "blockers" | "warnings" | "consent_needed" + > +): NativePreflightProbe { return { - ready: report.blockers.length === 0, + ready: report.blockers.length === 0 && report.consent_needed.length === 0, blockers: report.blockers.length, warnings: report.warnings.length, + consentNeeded: report.consent_needed.length, }; } diff --git a/src/lib/upgrade-check-project.ts b/src/lib/upgrade-check-project.ts index 667ecf1d5..5dba7f42a 100644 --- a/src/lib/upgrade-check-project.ts +++ b/src/lib/upgrade-check-project.ts @@ -549,7 +549,7 @@ function nativeFinding( // Native is optional: a Compose project upgrades without it. id: "native-preflight", status: "info", - summary: `Not ready for the native backend (${probe.blockers} blocker(s)); Compose is unaffected.`, + summary: `Not ready for the native backend (${probe.blockers} blocker(s), ${probe.consentNeeded} consent item(s)); Compose is unaffected.`, args: null, action: null, }; diff --git a/tests/native-preflight-cli.test.ts b/tests/native-preflight-cli.test.ts index 8a8bde41a..cbb269416 100644 --- a/tests/native-preflight-cli.test.ts +++ b/tests/native-preflight-cli.test.ts @@ -13,6 +13,7 @@ import { import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import type { NativePreflightReport } from "../src/backends/native-preflight.ts"; +import { nativePreflightProbeFromReport } from "../src/lib/upgrade-check-probes.ts"; import { nativeSourceSizeReceipt } from "./helpers/native-source-size.ts"; import { privateCliEnv } from "./helpers/private-cli-env.ts"; @@ -1762,3 +1763,26 @@ test( }, SPAWN_TIMEOUT_MS ); + +test( + "file-only upgrade readiness retains outstanding native consent", + async () => { + const f = await fixture({ services: { web: { image: IMAGE } } }); + await f.review([]); + const ungranted = await preflight(f, f.native); + expect(ungranted.code).toBe(1); + expect(nativePreflightProbeFromReport(ungranted.report)).toMatchObject({ + ready: false, + blockers: 0, + consentNeeded: 1, + }); + const granted = await preflight(f, { ...f.native, ...GRANT }); + expect(granted.code).toBe(0); + expect(nativePreflightProbeFromReport(granted.report)).toMatchObject({ + ready: true, + blockers: 0, + consentNeeded: 0, + }); + }, + SPAWN_TIMEOUT_MS +); diff --git a/tests/upgrade-check.test.ts b/tests/upgrade-check.test.ts index d7ca20b1a..49f58a890 100644 --- a/tests/upgrade-check.test.ts +++ b/tests/upgrade-check.test.ts @@ -1694,3 +1694,25 @@ test("hack upgrade snapshot, then rollback restores exact bytes; a changed file expect(listed.stdout).toContain("Choose a snapshot to restore"); expect(listed.stdout).toContain(snapshot.manifest_path); }, 60_000); + +test("native consent is informational for upgrade and remains distinct from blockers", async () => { + const project = await liveProject("native-consent"); + const findings = await inspectUpgradeProject({ + project: { ...project, project: project.name }, + homeDir: home, + deadline: Date.now() + 30_000, + nativePreflight: async () => ({ + ready: false, + blockers: 0, + warnings: 0, + consentNeeded: 1, + }), + }); + expect( + findings.find((finding) => finding.id === "native-preflight") + ).toMatchObject({ + status: "info", + summary: + "Not ready for the native backend (0 blocker(s), 1 consent item(s)); Compose is unaffected.", + }); +});