Guard edge-app create against EDGE_APP_ID, warn on stale id after delete

docs/EdgeApps.md

@@ -294,7 +294,11 @@ The `syntax` field specifies the version of the manifest file. The current versi
 
 The `id` field is a unique identifier for the Edge App. This ID is generated by the system and is used to identify the Edge App across the platform.
 
-> **Deprecated:** Reading the `id` from `screenly.yml` is deprecated. Set the `EDGE_APP_ID` environment variable instead — for example, via a repo secret in CI or a local `.env` file loaded into your shell. When `EDGE_APP_ID` is set, it takes priority over the manifest's `id` field. The CLI still falls back to reading `id` from `screenly.yml` for now, but prints a deprecation warning each time it does.
+> **Deprecated:** Reading the `id` from `screenly.yml` is deprecated. Set the `EDGE_APP_ID` environment variable instead, for example via a repo secret in CI or a local `.env` file loaded into your shell. When `EDGE_APP_ID` is set, it takes priority over the manifest's `id` field. The CLI still falls back to reading `id` from `screenly.yml` for now, but prints a deprecation warning each time it does.
+>
+> `screenly edge-app create` and `screenly edge-app create --in-place` refuse to run while `EDGE_APP_ID` is set, since both commands always create a brand new Edge App and would otherwise leave the configured id pointing at an app you didn't mean to create. Unset `EDGE_APP_ID` first if you want to create a new app, or use `screenly edge-app deploy` if you meant to deploy to the existing one.
+>
+> `screenly edge-app delete` clears the `id` field from `screenly.yml`, but it can't clear `EDGE_APP_ID` from your environment. If it's still set, subsequent commands keep resolving to the deleted app's id until you unset or update it yourself; the CLI prints a warning after deletion as a reminder.
 
 #### Entrypoint
 

src/cli.rs

@@ -11,6 +11,7 @@ use thiserror::Error;
 
 use crate::authentication::{verify_and_store_token, Authentication, AuthenticationError, Config};
 use crate::commands;
+use crate::commands::edge_app::app::{edge_app_id_from_env, EDGE_APP_ID_ENV};
 use crate::commands::edge_app::instance_manifest::InstanceManifest;
 use crate::commands::edge_app::manifest::EdgeAppManifest;
 use crate::commands::edge_app::server::MOCK_DATA_FILENAME;
@@ -996,6 +997,11 @@ pub fn handle_cli_edge_app_command(command: &EdgeAppCommands, output: OutputForm
                             std::process::exit(1);
                         }
                     }
+
+                    if let Some(id) = edge_app_id_from_env() {
+                        eprintln!("Warning: the {EDGE_APP_ID_ENV} environment variable is still set to \"{id}\". Commands will keep using that id until you unset or update it.");
+                    }
+
                     std::process::exit(0);
                 }
                 Err(e) => {

src/commands/edge_app/app.rs

@@ -29,10 +29,21 @@ use crate::commands::edge_app::utils::{
 use crate::commands::edge_app::EdgeAppCommand;
 use crate::commands::{CommandError, EdgeApps};
 
-const EDGE_APP_ID_ENV: &str = "EDGE_APP_ID";
+pub(crate) const EDGE_APP_ID_ENV: &str = "EDGE_APP_ID";
 
 pub const INJECT_JS_FILE_NAME: &str = "screenly_inject.js";
 
+pub(crate) fn edge_app_id_from_env() -> Option<String> {
+    std::env::var(EDGE_APP_ID_ENV).ok().and_then(|id| {
+        let trimmed = id.trim();
+        if trimmed.is_empty() {
+            None
+        } else {
+            Some(trimmed.to_string())
+        }
+    })
+}
+
 // Edge apps commands
 impl EdgeAppCommand {
     pub fn create(
@@ -46,6 +57,12 @@ impl EdgeAppCommand {
         ))?;
         let index_html_path = parent_dir_path.join("index.html");
 
+        if let Some(id) = edge_app_id_from_env() {
+            return Err(CommandError::InitializationError(format!(
+                "An Edge App id is already configured via the {EDGE_APP_ID_ENV} environment variable ({id}). 'screenly edge-app create' always creates a new Edge App; unset {EDGE_APP_ID_ENV} first if you want to create a new one, or use 'screenly edge-app deploy' if you meant to deploy to the existing app"
+            )));
+        }
+
         if Path::new(&path).exists() || Path::new(&index_html_path).exists() {
             return Err(CommandError::FileSystemError(format!(
                 "The directory {} already contains a screenly.yml or index.html file. Use --in-place if you want to create an Edge App in this directory",
@@ -184,6 +201,12 @@ impl EdgeAppCommand {
             )));
         }
 
+        if let Some(id) = edge_app_id_from_env() {
+            return Err(CommandError::InitializationError(format!(
+                "An Edge App id is already configured via the {EDGE_APP_ID_ENV} environment variable ({id}). The operation can only proceed when no Edge App id is already configured; unset {EDGE_APP_ID_ENV} first if you want to create a new Edge App, or use 'screenly edge-app deploy' if you meant to deploy to the existing app"
+            )));
+        }
+
         let data = fs::read_to_string(path)?;
         let mut manifest: EdgeAppManifest = serde_yaml::from_str(&data)?;
 
@@ -701,11 +724,8 @@ impl EdgeAppCommand {
     }
 
     pub fn get_app_id(&self, path: Option<String>) -> Result<String, CommandError> {
-        if let Ok(id) = std::env::var(EDGE_APP_ID_ENV) {
-            let id = id.trim();
-            if !id.is_empty() {
-                return Ok(id.to_string());
-            }
+        if let Some(id) = edge_app_id_from_env() {
+            return Ok(id);
         }
 
         let edge_app_manifest = EdgeAppManifest::new(&transform_edge_app_path_to_manifest(&path)?)?;
@@ -1748,6 +1768,93 @@ mod tests {
         assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap());
     }
 
+    #[test]
+    fn test_edge_app_create_when_env_var_is_set_should_return_error() {
+        let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) =
+            prepare_edge_apps_test(false, false);
+
+        let _lock = lock_test();
+        let _env = set_env(
+            OsString::from(EDGE_APP_ID_ENV),
+            "01ENVOVERRIDEXXXXXXXXXXXXX",
+        );
+        let result = command.create(
+            "Best app ever",
+            tmp_dir.path().join("screenly.yml").as_path(),
+            None,
+        );
+
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("EDGE_APP_ID environment variable (01ENVOVERRIDEXXXXXXXXXXXXX)"));
+        assert!(!tmp_dir.path().join("screenly.yml").exists());
+    }
+
+    #[test]
+    fn test_create_in_place_edge_app_when_env_var_is_set_should_return_error() {
+        let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) =
+            prepare_edge_apps_test(false, false);
+
+        File::create(tmp_dir.path().join("index.html")).unwrap();
+
+        let manifest = EdgeAppManifest {
+            id: None,
+            syntax: MANIFEST_VERSION.to_owned(),
+            ..Default::default()
+        };
+
+        EdgeAppManifest::save_to_file(&manifest, tmp_dir.path().join("screenly.yml").as_path())
+            .unwrap();
+
+        let _lock = lock_test();
+        let _env = set_env(
+            OsString::from(EDGE_APP_ID_ENV),
+            "01ENVOVERRIDEXXXXXXXXXXXXX",
+        );
+        let result = command.create_in_place(
+            "Best app ever",
+            tmp_dir.path().join("screenly.yml").as_path(),
+        );
+
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("EDGE_APP_ID environment variable (01ENVOVERRIDEXXXXXXXXXXXXX)"));
+    }
+
+    #[test]
+    fn test_create_in_place_edge_app_when_env_var_is_set_and_manifest_is_malformed_should_still_return_env_var_error(
+    ) {
+        let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) =
+            prepare_edge_apps_test(false, false);
+
+        File::create(tmp_dir.path().join("index.html")).unwrap();
+        fs::write(
+            tmp_dir.path().join("screenly.yml"),
+            "not: [valid, yaml: manifest",
+        )
+        .unwrap();
+
+        let _lock = lock_test();
+        let _env = set_env(
+            OsString::from(EDGE_APP_ID_ENV),
+            "01ENVOVERRIDEXXXXXXXXXXXXX",
+        );
+        let result = command.create_in_place(
+            "Best app ever",
+            tmp_dir.path().join("screenly.yml").as_path(),
+        );
+
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("EDGE_APP_ID environment variable (01ENVOVERRIDEXXXXXXXXXXXXX)"));
+    }
+
     #[test]
     fn test_clear_app_id_should_remove_app_id_from_manifest() {
         let (temp_dir, command, _mock_server, _manifest, _instance_manifest) =