diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index 3ef2c14..da73054 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -469,6 +469,10 @@ impl ApplyService { validate_email_secrets_present(bundle, ¤t.secret_names)?; let plan = compute_diff(¤t, bundle); let mut report = ApplyReport::default(); + // §4.7 warning: an enabled binding pointing at a disabled script is + // deployed-but-unreachable. Not an error (it's valid desired state), + // but surfaced so the operator isn't surprised by a silent 404. + report.warnings.extend(disabled_target_warnings(bundle)); let bundle_scripts: HashMap = bundle .scripts @@ -1422,6 +1426,42 @@ fn reject_reserved_path(path: &str) -> Result<(), ApplyError> { Ok(()) } +/// §4.7 reachability warnings: an *enabled* route or trigger bound to a +/// script the manifest marks *disabled* is deployed but unreachable (the +/// route 404s, the trigger won't fire). Valid desired state, so a warning — +/// not an error — keyed on the bundle alone. +fn disabled_target_warnings(bundle: &Bundle) -> Vec { + let disabled: HashSet<&str> = bundle + .scripts + .iter() + .filter(|s| !s.enabled) + .map(|s| s.name.as_str()) + .collect(); + if disabled.is_empty() { + return Vec::new(); + } + let mut out = Vec::new(); + for r in &bundle.routes { + if r.enabled && disabled.contains(r.script.as_str()) { + out.push(format!( + "route `{}` is enabled but its script `{}` is disabled — it will 404", + route_key(r.method.as_deref(), r.host_kind, &r.host, r.path_kind, &r.path), + r.script + )); + } + } + for t in &bundle.triggers { + if disabled.contains(t.script()) { + out.push(format!( + "{} trigger targets disabled script `{}` — it will not fire", + t.kind_str(), + t.script() + )); + } + } + out +} + /// Per-kind structural validation for one desired trigger — kept at parity /// with the interactive trigger API so `apply` can't write a trigger the /// dashboard would have rejected. Pure (no live state), so it's unit-tested @@ -2105,6 +2145,33 @@ mod tests { ); } + #[test] + fn warns_on_enabled_binding_to_disabled_script() { + let mut b = empty_bundle(); + let mut s = bundle_script("h", "x"); + s.enabled = false; + b.scripts = vec![s]; + b.routes = vec![BundleRoute { + script: "h".into(), + method: None, + host_kind: HostKind::Any, + host: String::new(), + host_param_name: None, + path_kind: PathKind::Exact, + path: "/h".into(), + dispatch_mode: DispatchMode::Sync, + enabled: true, + }]; + let w = disabled_target_warnings(&b); + assert!( + w.iter().any(|m| m.contains("will 404")), + "expected a 404 reachability warning: {w:?}" + ); + // No warning when the script is active. + b.scripts[0].enabled = true; + assert!(disabled_target_warnings(&b).is_empty()); + } + #[test] fn trigger_diff_create_noop_delete() { let s = script("h", "x"); diff --git a/crates/picloud-cli/tests/cli.rs b/crates/picloud-cli/tests/cli.rs index 33cd52d..d7e1216 100644 --- a/crates/picloud-cli/tests/cli.rs +++ b/crates/picloud-cli/tests/cli.rs @@ -20,6 +20,7 @@ mod apps; mod auth; mod dead_letters; mod email_queue; +mod enabled; mod init; mod invoke; mod logs; diff --git a/crates/picloud-cli/tests/enabled.rs b/crates/picloud-cli/tests/enabled.rs new file mode 100644 index 0000000..ef37477 --- /dev/null +++ b/crates/picloud-cli/tests/enabled.rs @@ -0,0 +1,100 @@ +//! `enabled` three-state lifecycle, end to end: disabling a script via the +//! manifest makes it non-invocable (404 on the execute-by-id bypass), and +//! re-enabling restores it — proving the data path + runtime honoring. + +use std::fs; +use std::path::Path; + +use serde_json::Value; +use tempfile::TempDir; + +use crate::common; +use crate::common::cleanup::AppGuard; + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn disabling_a_script_makes_it_uninvocable_then_reenable() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let slug = common::unique_slug("enabled"); + common::pic_as(&env) + .args(["apps", "create", &slug]) + .assert() + .success(); + let _guard = AppGuard::new(&env.url, &env.token, &slug); + + let dir = TempDir::new().unwrap(); + fs::create_dir_all(dir.path().join("scripts")).unwrap(); + fs::write(dir.path().join("scripts/hello.rhai"), "\"hi\"").unwrap(); + let manifest_path = dir.path().join("picloud.toml"); + let manifest = |enabled_line: &str| { + format!( + "[app]\nslug = \"{slug}\"\nname = \"Enabled\"\n\n\ + [[scripts]]\nname = \"hello\"\nfile = \"scripts/hello.rhai\"\n{enabled_line}" + ) + }; + + // Active → invocable. + fs::write(&manifest_path, manifest("")).unwrap(); + apply(&env, &manifest_path); + let id = script_id(&env, &slug, "hello"); + assert_eq!(invoke_status(&env, &id), 200, "active script must invoke"); + + // Disabled → 404 (not invocable), but still deployed (re-pull would show it). + fs::write(&manifest_path, manifest("enabled = false\n")).unwrap(); + apply(&env, &manifest_path); + assert_eq!( + invoke_status(&env, &id), + 404, + "disabled script must 404 on execute-by-id" + ); + + // Re-enabled → invocable again. + fs::write(&manifest_path, manifest("enabled = true\n")).unwrap(); + apply(&env, &manifest_path); + assert_eq!( + invoke_status(&env, &id), + 200, + "re-enabled script must invoke" + ); +} + +fn apply(env: &common::TestEnv, manifest_path: &Path) { + common::pic_as(env) + .args(["apply", "--file"]) + .arg(manifest_path) + .assert() + .success(); +} + +/// Resolve a script's id via the admin API (the manifest carries no ids). +fn script_id(env: &common::TestEnv, slug: &str, name: &str) -> String { + let client = reqwest::blocking::Client::new(); + let scripts: Vec = client + .get(format!("{}/api/v1/admin/scripts?app={slug}", env.url)) + .bearer_auth(&env.token) + .send() + .unwrap() + .json() + .unwrap(); + scripts + .into_iter() + .find(|s| s["name"] == name) + .and_then(|s| s["id"].as_str().map(String::from)) + .expect("script id") +} + +/// POST the execute-by-id bypass and return the HTTP status code. +fn invoke_status(env: &common::TestEnv, id: &str) -> u16 { + let client = reqwest::blocking::Client::new(); + client + .post(format!("{}/api/v1/execute/{id}", env.url)) + .bearer_auth(&env.token) + .body("{}") + .send() + .unwrap() + .status() + .as_u16() +}