fix: sync the settings the environment declares, not the base ones

CHANGELOG.md

@@ -14,6 +14,7 @@ air-gapped signing
 * fix: canister settings from the manifest are no longer silently discarded when a canister is created through the legacy management-canister fallback (a CloudEngine subnet with no registered engine operator). That path went through `ic-utils`, which encodes `create_canister`'s argument as a bare `canister_settings` record rather than the `record { settings : opt canister_settings; ... }` the interface spec defines, so the replica read no settings at all and created the canister with defaults. `icp deploy` masked this by syncing settings afterwards; `icp canister create` does not, and left the canister unconfigured.
 * fix: `icp canister logs` output formats are corrected. `--json` now emits machine-readable JSON and the default emits the human-readable lines (the two were swapped), and `--follow --json` emits newline-delimited JSON, one record per line, streamed as each record arrives. This is breaking for scripts: parsing the default output as JSON now requires `--json`, and consumers of `--follow --json` must read one JSON object per line.
 * fix: `icp canister status` again falls back on the publicly readable state-tree information when the caller may not read the status. Replicas now reject those calls with `IC0542`, which the fallback did not recognise, so the command failed with `Error looking up canister <id>` instead of printing the controllers and module hash. `IC0541`, returned on subnets with administrators, is now recognised too, and the fallback no longer depends on whether the rejection arrives certified or uncertified.
+* fix: `icp canister settings sync` now applies the settings the selected environment declares, not only the canister's base settings. It resolved the canister from the project's pre-override record, so a `settings:` block under `environments:` — including the one a vendored member declares for its own environments — was ignored: a setting declared only there was never written, and the command reported success having changed nothing. `icp deploy` already read the environment's record, so the two disagreed about the same canister in the same environment.
 
 ## Experimental
 

crates/icp-cli/tests/canister_settings_tests.rs

@@ -1661,3 +1661,114 @@ async fn canister_settings_show_not_a_controller() {
         .success()
         .stdout(contains(principal_alice.as_str()));
 }
+
+/// `settings sync` applies the settings the *environment* declares, not the
+/// canister's base settings. Consolidation layers an environment's `settings:`
+/// override onto a clone of the base record, so reading the base record syncs
+/// pre-override settings — and for a variable declared only in the override,
+/// syncs nothing at all.
+#[tokio::test]
+async fn canister_settings_sync_applies_environment_override() {
+    let ctx = TestContext::new();
+
+    // Setup project
+    let project_dir = ctx.create_project_dir("icp");
+
+    // Use vendored WASM
+    let wasm = ctx.make_asset("example_icp_mo.wasm");
+
+    // `API_KEY` is declared only by the environment override — the canister's
+    // own settings block never mentions it.
+    let pm = formatdoc! {r#"
+        canisters:
+          - name: my-canister
+            build:
+              steps:
+                - type: script
+                  command: cp '{wasm}' "$ICP_WASM_OUTPUT_PATH"
+
+        {NETWORK_RANDOM_PORT}
+
+        environments:
+          - name: random-environment
+            network: random-network
+            settings:
+              my-canister:
+                environment_variables:
+                  API_KEY: from-the-environment
+    "#};
+
+    write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest");
+
+    // Start network
+    let _g = ctx.start_network_in(&project_dir, "random-network").await;
+    ctx.ping_until_healthy(&project_dir, "random-network");
+
+    // Deploy project
+    clients::icp(&ctx, &project_dir, Some("random-environment".to_string()))
+        .mint_cycles(200 * TRILLION);
+
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args(["deploy", "--environment", "random-environment"])
+        .assert()
+        .success();
+
+    // Drop the variable on-canister, so only `settings sync` can put it back.
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args([
+            "canister",
+            "settings",
+            "update",
+            "my-canister",
+            "--environment",
+            "random-environment",
+            "--remove-environment-variable",
+            "API_KEY",
+        ])
+        .assert()
+        .success();
+
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args([
+            "canister",
+            "settings",
+            "show",
+            "my-canister",
+            "--environment",
+            "random-environment",
+        ])
+        .assert()
+        .success()
+        .stdout(contains("API_KEY").not());
+
+    // Sync must restore it from the environment override.
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args([
+            "canister",
+            "settings",
+            "sync",
+            "my-canister",
+            "--environment",
+            "random-environment",
+        ])
+        .assert()
+        .success();
+
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args([
+            "canister",
+            "settings",
+            "show",
+            "my-canister",
+            "--environment",
+            "random-environment",
+        ])
+        .assert()
+        .success()
+        .stdout(contains("API_KEY: from-the-environment"));
+}

crates/icp/src/context/mod.rs

@@ -249,27 +249,35 @@ impl Context {
         }
     }
 
+    /// The canister as it is configured *in this environment*.
+    ///
+    /// The record comes from the environment's own canister map, not from
+    /// [`crate::Project::canisters`]: consolidation layers the environment's `settings:`
+    /// overrides — the root's, and each vendored member's for its own
+    /// environments — onto a clone of the base record, so the base one still
+    /// carries the pre-override settings. Reading it here would silently apply
+    /// the wrong settings for every canister an environment overrides.
     pub async fn get_canister_and_path_for_env(
         &self,
         canister_name: &str,
         environment: &EnvironmentSelection,
     ) -> Result<(PathBuf, Canister), GetEnvCanisterError> {
         let p = self.project.load().await?;
-        let Some((path, canister)) = p.get_canister(canister_name) else {
+        if p.get_canister(canister_name).is_none() {
             return CanisterNotFoundInProjectSnafu {
                 canister_name: canister_name.to_owned(),
             }
             .fail();
-        };
+        }
 
         let env = self.get_environment(environment).await?;
-        if !env.contains_canister(canister_name) {
+        let Some((path, canister)) = env.canisters.get(canister_name) else {
             return CanisterNotInEnvSnafu {
                 canister_name: canister_name.to_owned(),
                 environment_name: environment.name().to_owned(),
             }
             .fail();
-        }
+        };
         Ok((path.clone(), canister.clone()))
     }
 

crates/icp/src/context/tests.rs

@@ -953,3 +953,86 @@ async fn test_get_agent_explicit_environment_inside_project() {
     // Should use the network from the "test" environment (which is "staging")
     assert_eq!(agent.read_root_key(), staging_root_key);
 }
+
+/// The canister must come back as the *environment* configures it. Consolidation
+/// layers an environment's `settings:` overrides — the root's, and each vendored
+/// member's for its own environments — onto a clone of the base record, so the
+/// base one still holds the pre-override settings. `icp canister settings sync`
+/// writes whatever this hands back.
+#[tokio::test]
+async fn get_canister_for_env_returns_the_environments_settings_not_the_base() {
+    let mut project = MockProjectLoader::minimal().project;
+
+    // The base record holds the pre-override settings...
+    project
+        .canisters
+        .get_mut("backend")
+        .unwrap()
+        .1
+        .settings
+        .environment_variables = None;
+
+    // ...while the environment holds the override declared for it.
+    let overridden = HashMap::from([("API_KEY".to_string(), "from-the-environment".to_string())]);
+    project
+        .environments
+        .get_mut("default")
+        .unwrap()
+        .canisters
+        .get_mut("backend")
+        .unwrap()
+        .1
+        .settings
+        .environment_variables = Some(overridden.clone());
+
+    let ctx = Context {
+        project: Arc::new(MockProjectLoader::new(project)),
+        ..Context::mocked()
+    };
+
+    let (_, canister) = ctx
+        .get_canister_and_path_for_env(
+            "backend",
+            &EnvironmentSelection::Named("default".to_string()),
+        )
+        .await
+        .expect("backend is declared in the default environment");
+
+    assert_eq!(canister.settings.environment_variables, Some(overridden));
+}
+
+/// A canister the project declares but the selected environment does not is
+/// still reported as missing from that environment, not silently served from
+/// the project's base map.
+#[tokio::test]
+async fn get_canister_for_env_rejects_a_canister_the_environment_omits() {
+    let mut project = MockProjectLoader::minimal().project;
+    project
+        .environments
+        .get_mut("default")
+        .unwrap()
+        .canisters
+        .shift_remove("backend");
+
+    let ctx = Context {
+        project: Arc::new(MockProjectLoader::new(project)),
+        ..Context::mocked()
+    };
+
+    let error = ctx
+        .get_canister_and_path_for_env(
+            "backend",
+            &EnvironmentSelection::Named("default".to_string()),
+        )
+        .await
+        .expect_err("backend is not in the default environment");
+
+    assert!(
+        matches!(
+            &error,
+            GetEnvCanisterError::CanisterNotInEnv { canister_name, environment_name }
+                if canister_name == "backend" && environment_name == "default"
+        ),
+        "unexpected error: {error}"
+    );
+}