diff --git a/crates/openjd-model/src/template/validate_v2023_09/mod.rs b/crates/openjd-model/src/template/validate_v2023_09/mod.rs index 99a9daf6..1b0ec61e 100644 --- a/crates/openjd-model/src/template/validate_v2023_09/mod.rs +++ b/crates/openjd-model/src/template/validate_v2023_09/mod.rs @@ -99,11 +99,12 @@ pub struct EffectiveRules { pub allowed_job_param_types: std::collections::HashSet, pub allowed_task_param_types: std::collections::HashSet, pub allow_fmtstring_in_numeric_fields: bool, - /// When the `WRAP_ACTIONS` extension is enabled, an environment script - /// is valid if it defines any of `onEnter`, `onExit`, or one of the - /// RFC 0008 wrap hooks (`onWrapEnvEnter`, `onWrapTaskRun`, `onWrapEnvExit`). - /// Without the extension, the base 2023-09 rule applies: - /// `onEnter` is required whenever `script` is present. + /// An environment script must define at least one action whether or not + /// this flag is set, and either `onEnter` or `onExit` alone satisfies + /// that. The flag decides two things: whether the RFC 0008 wrap hooks + /// (`onWrapEnvEnter`, `onWrapTaskRun`, `onWrapEnvExit`) also count as an + /// acceptable choice, and which wording the error uses when no action is + /// defined. It does not make `onEnter` required. pub wrap_actions_enabled: bool, } diff --git a/crates/openjd-model/src/template/validate_v2023_09/structure.rs b/crates/openjd-model/src/template/validate_v2023_09/structure.rs index 219a7330..6067b3b0 100644 --- a/crates/openjd-model/src/template/validate_v2023_09/structure.rs +++ b/crates/openjd-model/src/template/validate_v2023_09/structure.rs @@ -389,29 +389,22 @@ pub fn validate_single_environment( if let Some(script) = &env.script { let script_path = path_field(path, "script"); let actions_path = path_field(&script_path, "actions"); - // Base 2023-09 requires `onEnter` whenever a `script` is present. - // RFC 0008 relaxes this: when `WRAP_ACTIONS` is enabled, an env may - // define only wrap hooks (or any single action) without a standalone - // `onEnter`. Concretely, require at least one of the five known - // actions to be present so we don't accept an empty `actions: {}`. - if rules.wrap_actions_enabled { - // RFC 0008: an env may define only wrap hooks (or any single - // action) without a standalone `onEnter`. Require at least one of - // the five known actions so an empty `actions: {}` is still - // rejected. The all-or-nothing wrap-hook rule (defining any wrap - // hook requires all three) is enforced separately in - // `wrap_actions.rs`, not here. - if !script.actions.has_any_action() { - errors.add( - &actions_path, - "must define at least one of onEnter or onExit, or the complete set of wrap hooks (onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit together).", - ); - } - } else if script.actions.on_enter.is_none() { - // Preserve the original wording when the extension is not enabled - // so pre-RFC error messages don't change. `on_enter.is_none()` - // also covers the empty-`actions` case. - errors.add(&actions_path, "onEnter is required."); + // An environment script must define at least one action. Either + // `onEnter` or `onExit` alone satisfies this, so an environment whose + // only work is cleanup is valid. With `WRAP_ACTIONS` a complete set of + // wrap hooks also satisfies it. Requiring at least one of the five + // known actions is what rejects an empty `actions: {}`. The + // all-or-nothing wrap-hook rule (defining any wrap hook requires all + // three) is enforced separately in `wrap_actions.rs`, not here. + if !script.actions.has_any_action() { + errors.add( + &actions_path, + if rules.wrap_actions_enabled { + "must define at least one of onEnter or onExit, or the complete set of wrap hooks (onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit together)." + } else { + "must define at least one of onEnter or onExit." + }, + ); } for (name, action) in script.actions.iter_named() { validate_action( diff --git a/crates/openjd-model/tests/integration/test_actions_and_steps.rs b/crates/openjd-model/tests/integration/test_actions_and_steps.rs index 117b7bc2..28810b43 100644 --- a/crates/openjd-model/tests/integration/test_actions_and_steps.rs +++ b/crates/openjd-model/tests/integration/test_actions_and_steps.rs @@ -296,7 +296,7 @@ fn test_env_actions_empty() { }"#; check_err( s, - &["jobEnvironments[0] -> script -> actions:\n\tonEnter is required."], + &["jobEnvironments[0] -> script -> actions:\n\tmust define at least one of onEnter or onExit."], ); } @@ -599,10 +599,9 @@ fn test_env_action_on_exit() { "steps": [{"name": "S", "script": {"actions": {"onRun": {"command": "foo"}}}}], "jobEnvironments": [{"name": "E", "script": {"actions": {"onExit": {"command": "foo"}}}}] }"#; - check_err( - s, - &["jobEnvironments[0] -> script -> actions:\n\tonEnter is required."], - ); + // Either ordinary action alone is sufficient, so a cleanup-only + // environment is valid. + decode_ok(s); } #[test] diff --git a/crates/openjd-model/tests/integration/test_environment_template.rs b/crates/openjd-model/tests/integration/test_environment_template.rs index a5efd000..192b9476 100644 --- a/crates/openjd-model/tests/integration/test_environment_template.rs +++ b/crates/openjd-model/tests/integration/test_environment_template.rs @@ -744,18 +744,41 @@ fn test_env_name_513_chars_with_extension_fails() { } // ══════════════════════════════════════════════════════════════ -// Bug fix: onEnter is required per spec §4.3 +// Environment actions: at least one of onEnter or onExit (spec §4.3) // ══════════════════════════════════════════════════════════════ #[test] -fn test_env_actions_on_enter_required() { - // onExit alone should fail — onEnter is required - check_env_err( +fn test_env_actions_on_exit_only_is_valid() { + // onExit alone is sufficient: a queue environment whose only work is + // cleanup does not need an onEnter. + decode_ok( r#"{ "specificationVersion": "environment-2023-09", "environment": {"name": "Foo", "script": {"actions": {"onExit": {"command": "cleanup"}}}} }"#, - &["environment -> script -> actions:\n\tonEnter is required."], + ); +} + +#[test] +fn test_env_actions_on_enter_only_is_valid() { + decode_ok( + r#"{ + "specificationVersion": "environment-2023-09", + "environment": {"name": "Foo", "script": {"actions": {"onEnter": {"command": "setup"}}}} + }"#, + ); +} + +#[test] +fn test_env_actions_empty_is_rejected() { + // Removing the onEnter requirement must not make an empty actions object + // acceptable. + check_env_err( + r#"{ + "specificationVersion": "environment-2023-09", + "environment": {"name": "Foo", "script": {"actions": {}}} + }"#, + &["environment -> script -> actions:\n\tmust define at least one of onEnter or onExit."], ); } diff --git a/crates/openjd-model/tests/integration/test_wrap_actions.rs b/crates/openjd-model/tests/integration/test_wrap_actions.rs index a40f8897..3a708a57 100644 --- a/crates/openjd-model/tests/integration/test_wrap_actions.rs +++ b/crates/openjd-model/tests/integration/test_wrap_actions.rs @@ -713,6 +713,31 @@ fn defining_zero_wrap_hooks_accepted() { ); } +// ════════════════════════════════════════════════════════════════════ +// At-least-one-action rule, WRAP_ACTIONS wording +// +// An environment script must define at least one action. With +// WRAP_ACTIONS enabled the error names the wrap hooks as a third +// acceptable choice alongside onEnter and onExit. The wording without +// the extension is covered in test_environment_template.rs. +// ════════════════════════════════════════════════════════════════════ + +#[test] +fn empty_env_actions_rejected_with_wrap_hooks_named() { + expect_env_err( + r#"{ + "specificationVersion": "environment-2023-09", + "extensions": ["WRAP_ACTIONS", "EXPR"], + "environment": { + "name": "Empty", + "script": {"actions": {}} + } + }"#, + WRAP_EXTS, + &["environment -> script -> actions:\n\tmust define at least one of onEnter or onExit, or the complete set of wrap hooks (onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit together)."], + ); +} + // ════════════════════════════════════════════════════════════════════ // EXPR prerequisite (RFC 0008) //