From 600c6bf6cbdc91afa8dbac91b88c91321fcb0988 Mon Sep 17 00:00:00 2001 From: SaikrishnaGundeti Date: Thu, 3 Sep 2026 13:32:23 +0200 Subject: [PATCH 1/2] feat(change): label PR-touched apps vs leftover drift Emit pr_touched_apps / also_still_drifted_apps CI outputs and step-summary sections for Path A2 plan clarity. Full reconcile execute semantics are unchanged; labels use changed paths + env YAML mapping + dry-run app names. Co-authored-by: Cursor --- docs/ci-outputs.md | 14 +- docs/environments.md | 42 +++- src/commands/change/plan.rs | 57 ++++- src/commands/change/verify.rs | 4 +- src/lib.rs | 1 + src/output.rs | 83 ++++++- src/pr_preview.rs | 395 ++++++++++++++++++++++++++++++++++ 7 files changed, 572 insertions(+), 24 deletions(-) create mode 100644 src/pr_preview.rs diff --git a/docs/ci-outputs.md b/docs/ci-outputs.md index d5b173a..48d8375 100644 --- a/docs/ci-outputs.md +++ b/docs/ci-outputs.md @@ -31,6 +31,18 @@ If the platform's file path variable is missing (e.g. `GITHUB_OUTPUT` unset), th | `diff_has_destructive` | `true` when the diff includes deletions | | `plan_ids` | Comma-separated external plan ids when `change plan` fans out across environments | | `plan_count` | Number of plans created by environment fan-out (`1` when `--environment` is set) | +| `pr_touched_apps` | Comma-separated app names in the plan that overlap PR/changed paths (labeling only) | +| `also_still_drifted_apps` | Comma-separated plan apps that are still drifted but not touched by this PR | +| `pr_preview_summary` | Human line: `this PR touches: …; also still drifted: …` | + +`pr_*` keys are emitted only when changed paths are available (`--changed-paths`, +`--changed-paths-file`, `DESLICER_CHANGED_PATHS`, or a GitHub `pull_request` git +range) **and** a dry-run diff with change items exists. They never filter which +apps the plan packs or executes — full desired-vs-observed reconcile remains the +execute semantics. + +GitHub Actions also receives a **job step summary** (markdown table, plus a PR +preview section when labels are present) when `GITHUB_STEP_SUMMARY` is set. ### `change status` @@ -44,8 +56,6 @@ If the platform's file path variable is missing (e.g. `GITHUB_OUTPUT` unset), th | `fully_completed_items` | Items applied on every target host | | `diff_*` | Same keys as verify when a persisted dry-run diff exists | -GitHub Actions also receives a **job step summary** (markdown table) when `GITHUB_STEP_SUMMARY` is set. - ### `change deploy` (queued, with `--no-wait`) | Key | Description | diff --git a/docs/environments.md b/docs/environments.md index 961e41a..0d80266 100644 --- a/docs/environments.md +++ b/docs/environments.md @@ -127,14 +127,50 @@ Monorepos may define many files: └── dr-failover.yml ``` -Select per job: +Keep workflows thin: one matrix row (or job) per stem. Resolve groups by **name** from +each YAML (`inventory_group`) — do not hard-code a single repo-level `TARGET_GROUP_ID` +when stems map different host groups. ```yaml -- run: deslicer change deploy --environment staging -- run: deslicer change deploy --environment production +strategy: + fail-fast: false + matrix: + target: [staging, production] +steps: + - run: deslicer inventory validate --environment ${{ matrix.target }} + - run: deslicer change plan --environment ${{ matrix.target }} + # omit --target-group when the stem YAML has exactly one destination with apps; + # otherwise pass --target-group +``` + +Select deploy per job: + +```yaml +- run: deslicer change deploy --environment staging --plan-id ... +- run: deslicer change deploy --environment production --plan-id ... if: github.ref == 'refs/heads/main' ``` +**Blocker (multi-group, same SHA):** Observer’s idempotent index is currently +`(tenant_id, repository_url, commit_sha)`. Two plans for different +`target_group_id` values on the same commit collide until that index includes +`target_group_id`. Prefer one destination-with-apps per stem, or different +commits/environments, until that Observer change lands. + +## Lifecycle gates (Path A2) + +Suggested thin CI shape (logic stays in the CLI — no reusable workflow core): + +| Event | Suggested steps | Notes | +|-------|-----------------|-------| +| `pull_request` | `inventory validate` → `change plan` (draft / pending_approval) | Preview only; approve in portal when ready | +| `push` to default branch / manual full run | `change plan` (full reconcile) → approve → deploy | Same reconcile semantics as PR — packs all drifted mapped apps | +| Nothing mapped | Skip plan | Exit early when the stem has no `source_path` apps | + +PR jobs may pass changed paths (or rely on GitHub PR detection) so CI outputs +label **this PR touches** vs **also still drifted** without filtering execute. +See [ci-outputs.md](ci-outputs.md). + ## Air-gapped override When resolve-backend cannot reach deslicer-ai, operators may set `OBSERVER_API_URL` to talk to Observer directly. Environment files still name the logical target for workflows; the override is a break-glass path documented in [installation.md](installation.md). diff --git a/src/commands/change/plan.rs b/src/commands/change/plan.rs index dd09a01..bf4fe42 100644 --- a/src/commands/change/plan.rs +++ b/src/commands/change/plan.rs @@ -8,6 +8,7 @@ use crate::commands::pipeline::{ use crate::errors::CliError; use crate::observer_client::{ChangePlan, Client, OrchestratedPlan}; use crate::output::{emit_change_plan, emit_change_plan_with_diff, emit_change_plans}; +use crate::pr_preview::{labels_for_plan_context, resolve_changed_paths, PrPreviewLabels}; use crate::token_source::TokenSource; use crate::Ctx; @@ -42,6 +43,17 @@ pub struct Args { /// Optional plan name for the bundle-sourced plan. #[arg(long)] pub name: Option, + + /// Optional file of changed repo paths (one per line) for PR preview labeling. + /// Clarity only — does not filter which apps the plan packs. When omitted, + /// the CLI uses `DESLICER_CHANGED_PATHS` or a GitHub pull_request git range. + #[arg(long, value_name = "FILE")] + pub changed_paths_file: Option, + + /// Comma/newline-separated changed paths (same labeling semantics as + /// `--changed-paths-file`). + #[arg(long, value_name = "PATHS")] + pub changed_paths: Option, } /// Compile polling: the ephemeral compile-runner takes seconds to a few @@ -219,7 +231,7 @@ pub async fn run(ctx: Ctx, args: Args) -> i32 { // or environment discovery. Handle it before the proxy-mode gate. if session.is_observer_api_token() { return match run_direct_git_plan(&session, &client, &args, environment.as_deref()).await { - Ok(plan) => emit_change_plan(&plan), + Ok(plan) => emit_ready_plan(&client, &args, environment.as_deref(), plan).await, Err(err) => map_cli_error(ctx.log_format, err), }; } @@ -364,13 +376,8 @@ async fn run_git_plans( } let emit_code = if plans.len() == 1 { - if let Some(ready) = last_ready.as_ref() { - let diff = client - .get_dry_run_diff(&ready.id) - .await - .ok() - .and_then(|body| crate::diff_summary::diff_counts_from_observer_value(&body)); - emit_change_plan_with_diff(ready, diff.as_ref()) + if let Some(ready) = last_ready { + emit_ready_plan(client, args, environment, ready).await } else { emit_change_plan(&plans[0]) } @@ -385,6 +392,40 @@ async fn run_git_plans( } } +fn preview_labels_for( + args: &Args, + environment: Option<&str>, + diff_body: Option<&serde_json::Value>, +) -> Option { + let changed = resolve_changed_paths( + args.changed_paths_file.as_deref(), + args.changed_paths.as_deref(), + )?; + let env_yaml = environment.and_then(|stem| { + crate::target_group::read_environment_yaml(stem) + .ok() + .flatten() + }); + labels_for_plan_context(diff_body, env_yaml.as_deref(), &changed) +} + +async fn emit_ready_plan( + client: &Client, + args: &Args, + environment: Option<&str>, + plan: ChangePlan, +) -> i32 { + let diff_body = client.get_dry_run_diff(&plan.id).await.ok(); + let counts = diff_body + .as_ref() + .and_then(crate::diff_summary::diff_counts_from_observer_value); + let preview = preview_labels_for(args, environment, diff_body.as_ref()); + if let Some(ref labels) = preview { + eprintln!("{}", labels.human_summary()); + } + emit_change_plan_with_diff(&plan, counts.as_ref(), preview.as_ref()) +} + #[cfg(test)] mod tests { use super::environments_for_plan; diff --git a/src/commands/change/verify.rs b/src/commands/change/verify.rs index 27e6ae0..a98c645 100644 --- a/src/commands/change/verify.rs +++ b/src/commands/change/verify.rs @@ -98,11 +98,11 @@ pub async fn run(ctx: Ctx, args: Args) -> i32 { if let Some(ref counts) = counts { emit_message(&crate::output::diff_count_pairs(counts)); } - emit_change_plan_with_diff(&plan, counts.as_ref()) + emit_change_plan_with_diff(&plan, counts.as_ref(), None) } Err(err) => { eprintln!("dry-run accepted, but the diff could not be fetched: {err}"); - emit_change_plan_with_diff(&plan, None) + emit_change_plan_with_diff(&plan, None, None) } } } diff --git a/src/lib.rs b/src/lib.rs index e598e64..138f0d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ pub mod observer_client; pub mod observer_token; pub mod oidc_exchange; pub mod output; +pub mod pr_preview; pub mod reporting; pub mod resolver; pub mod session_portal; diff --git a/src/output.rs b/src/output.rs index 8122f72..84b4b05 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1,6 +1,7 @@ use crate::ci::{detect_platform, CiPlatform}; use crate::diff_summary::DiffCounts; use crate::observer_client::{ChangePlan, ExecutionQueued, ExecutionSummary, PlanProgress}; +use crate::pr_preview::PrPreviewLabels; use std::collections::BTreeMap; use std::fs::OpenOptions; use std::io::{self, Write}; @@ -113,7 +114,12 @@ pub fn emit_diff_counts(counts: &DiffCounts) -> i32 { emit_to_sink(&diff_count_pairs(counts)) } -fn plan_summary_markdown(title: &str, plan: &ChangePlan, diff: Option<&DiffCounts>) -> String { +fn plan_summary_markdown( + title: &str, + plan: &ChangePlan, + diff: Option<&DiffCounts>, + preview: Option<&PrPreviewLabels>, +) -> String { let mut lines = vec![ format!("## {title}"), String::new(), @@ -136,16 +142,47 @@ fn plan_summary_markdown(title: &str, plan: &ChangePlan, diff: Option<&DiffCount lines.push("| Destructive | yes |".to_string()); } } - lines.join("\n") + let mut body = lines.join("\n"); + if let Some(labels) = preview { + body.push_str("\n\n"); + body.push_str(&labels.markdown_section()); + } + body +} + +pub fn pr_preview_pairs(labels: &PrPreviewLabels) -> Vec<(&'static str, String)> { + vec![ + ("pr_touched_apps", labels.pr_touched_apps.join(",")), + ( + "also_still_drifted_apps", + labels.also_still_drifted_apps.join(","), + ), + ("pr_preview_summary", labels.human_summary()), + ] } pub fn emit_change_plan(plan: &ChangePlan) -> i32 { - emit_change_plan_with_diff(plan, None) + emit_change_plan_with_diff(plan, None, None) } -pub fn emit_change_plan_with_diff(plan: &ChangePlan, diff: Option<&DiffCounts>) -> i32 { +pub fn emit_change_plan_with_diff( + plan: &ChangePlan, + diff: Option<&DiffCounts>, + preview: Option<&PrPreviewLabels>, +) -> i32 { println!("{}", serde_json::to_string(plan).unwrap_or_default()); - let summary = if let Some(counts) = diff { + let summary = if let Some(labels) = preview.filter(|value| !value.is_empty()) { + let base = if let Some(counts) = diff { + counts.human_summary() + } else { + plan.display_summary() + }; + if base.is_empty() { + labels.human_summary() + } else { + format!("{base}; {}", labels.human_summary()) + } + } else if let Some(counts) = diff { counts.human_summary() } else { plan.display_summary() @@ -159,7 +196,11 @@ pub fn emit_change_plan_with_diff(plan: &ChangePlan, diff: Option<&DiffCounts>) if let Some(counts) = diff { pairs.extend(diff_count_pairs(counts)); } - let _ = append_github_step_summary(&plan_summary_markdown("Deslicer plan", plan, diff)); + if let Some(labels) = preview { + pairs.extend(pr_preview_pairs(labels)); + } + let _ = + append_github_step_summary(&plan_summary_markdown("Deslicer plan", plan, diff, preview)); emit_to_sink(&pairs) } @@ -236,8 +277,12 @@ pub fn emit_plan_status( pairs.extend(diff_count_pairs(counts)); } if let Some(plan) = plan { - let _ = - append_github_step_summary(&plan_summary_markdown("Deslicer plan status", plan, diff)); + let _ = append_github_step_summary(&plan_summary_markdown( + "Deslicer plan status", + plan, + diff, + None, + )); } emit_to_sink(&pairs) } @@ -338,13 +383,33 @@ mod tests { name: None, summary: None, }; - let _ = append_github_step_summary(&plan_summary_markdown("Test", &plan, None)); + let _ = append_github_step_summary(&plan_summary_markdown("Test", &plan, None, None)); std::env::remove_var("GITHUB_STEP_SUMMARY"); let content = std::fs::read_to_string(&path).unwrap(); assert!(content.contains("pending_approval")); assert!(content.contains("ext")); } + #[test] + fn step_summary_includes_pr_preview_labels() { + let labels = PrPreviewLabels { + pr_touched_apps: vec!["demo_ci_app".into()], + also_still_drifted_apps: vec!["TA-linux".into()], + }; + let plan = ChangePlan { + id: "row".into(), + plan_id: Some("ext".into()), + status: "pending_approval".into(), + name: None, + summary: None, + }; + let markdown = plan_summary_markdown("Test", &plan, None, Some(&labels)); + assert!(markdown.contains("This PR touches")); + assert!(markdown.contains("demo_ci_app")); + assert!(markdown.contains("Also still drifted")); + assert!(markdown.contains("TA-linux")); + } + #[test] fn plans_summary_lists_each_plan() { let plans = [ diff --git a/src/pr_preview.rs b/src/pr_preview.rs new file mode 100644 index 0000000..9491b83 --- /dev/null +++ b/src/pr_preview.rs @@ -0,0 +1,395 @@ +//! PR preview labeling: which plan apps this PR touches vs leftover drift. +//! +//! Clarity only — does **not** change which apps the plan packs or executes. +//! Full desired-vs-observed reconcile still includes every drifted mapped app. + +use std::collections::BTreeSet; +use std::path::Path; +use std::process::Command; + +use serde_json::Value; + +use crate::environment_yaml::extract_apps_blocks; + +/// Apps in a compiled plan, split by whether the PR/changed paths touched them. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct PrPreviewLabels { + /// Drifted apps whose `source_path` overlaps a changed path. + pub pr_touched_apps: Vec, + /// Drifted apps still in the plan that this PR did not touch. + pub also_still_drifted_apps: Vec, +} + +impl PrPreviewLabels { + pub fn is_empty(&self) -> bool { + self.pr_touched_apps.is_empty() && self.also_still_drifted_apps.is_empty() + } + + pub fn human_summary(&self) -> String { + let touched = if self.pr_touched_apps.is_empty() { + "(none)".to_string() + } else { + self.pr_touched_apps.join(", ") + }; + let drifted = if self.also_still_drifted_apps.is_empty() { + "(none)".to_string() + } else { + self.also_still_drifted_apps.join(", ") + }; + format!("this PR touches: {touched}; also still drifted: {drifted}") + } + + pub fn markdown_section(&self) -> String { + let mut lines = vec![ + "### PR preview (labeling only)".to_string(), + String::new(), + "Full reconcile still packs every drifted mapped app. Labels do not filter execute." + .to_string(), + String::new(), + format!( + "- **This PR touches:** {}", + format_app_list(&self.pr_touched_apps) + ), + format!( + "- **Also still drifted:** {}", + format_app_list(&self.also_still_drifted_apps) + ), + ]; + lines.push(String::new()); + lines.join("\n") + } +} + +fn format_app_list(apps: &[String]) -> String { + if apps.is_empty() { + "_(none)_".to_string() + } else { + apps.iter() + .map(|app| format!("`{app}`")) + .collect::>() + .join(", ") + } +} + +/// Unique `app_name` values from a plan dry-run / PlanDiffResponse body. +pub fn app_names_from_diff(root: &Value) -> Vec { + let mut names = BTreeSet::new(); + for item in change_items(root) { + if let Some(name) = item.get("app_name").and_then(Value::as_str) { + let trimmed = name.trim(); + if !trimmed.is_empty() { + names.insert(trimmed.to_string()); + } + } + } + names.into_iter().collect() +} + +fn change_items(root: &Value) -> &[Value] { + root.get("diff") + .and_then(|diff| diff.get("change_items")) + .or_else(|| root.get("change_items")) + .and_then(Value::as_array) + .map(Vec::as_slice) + .unwrap_or(&[]) +} + +/// Mapped apps from environment YAML (`source_path` → display label = basename). +pub fn mapped_apps_from_yaml(env_yaml: &str) -> Vec<(String, String)> { + let mut out = Vec::new(); + let mut seen = BTreeSet::new(); + for block in extract_apps_blocks(env_yaml) { + for source_path in block.source_paths() { + let normalized = normalize_repo_path(&source_path); + if normalized.is_empty() || !seen.insert(normalized.clone()) { + continue; + } + let label = app_label_from_source_path(&normalized); + out.push((normalized, label)); + } + } + out +} + +fn app_label_from_source_path(source_path: &str) -> String { + Path::new(source_path) + .file_name() + .and_then(|name| name.to_str()) + .filter(|name| !name.is_empty()) + .unwrap_or(source_path) + .to_string() +} + +fn normalize_repo_path(path: &str) -> String { + path.trim() + .trim_start_matches("./") + .replace('\\', "/") + .trim_matches('/') + .to_string() +} + +/// True when `changed` is the mapped app root or a file under it. +pub fn path_touches_source(changed: &str, source_path: &str) -> bool { + let changed = normalize_repo_path(changed); + let source = normalize_repo_path(source_path); + if changed.is_empty() || source.is_empty() { + return false; + } + changed == source || changed.starts_with(&(source + "/")) +} + +/// Labels for apps that appear in the plan diff, given PR/changed paths. +/// +/// When `changed_paths` is empty, returns `None` (no labeling context). +/// Mapping YAML changes under `.deslicer/environments/` mark every mapped app +/// as touched for labeling (still does not filter execute). +pub fn label_plan_apps( + drifted_app_names: &[String], + mapped_apps: &[(String, String)], + changed_paths: &[String], +) -> Option { + if changed_paths.is_empty() || drifted_app_names.is_empty() { + return None; + } + + let env_mapping_changed = changed_paths.iter().any(|path| { + let normalized = normalize_repo_path(path); + normalized.starts_with(".deslicer/environments/") || normalized == ".deslicer/environments" + }); + + let mut touched_labels = BTreeSet::new(); + for (source_path, label) in mapped_apps { + let hit = env_mapping_changed + || changed_paths + .iter() + .any(|path| path_touches_source(path, source_path)); + if hit { + touched_labels.insert(label.clone()); + } + } + + // Also match drifted app_name directly against path basenames / segments when + // YAML was unavailable (direct UUID plan without local env file). + if mapped_apps.is_empty() { + for app in drifted_app_names { + let needle = format!("/{app}/"); + let suffix = format!("/{app}"); + if changed_paths.iter().any(|path| { + let normalized = normalize_repo_path(path); + normalized == *app || normalized.ends_with(&suffix) || normalized.contains(&needle) + }) { + touched_labels.insert(app.clone()); + } + } + } + + let mut pr_touched = Vec::new(); + let mut also_drifted = Vec::new(); + for app in drifted_app_names { + if touched_labels.contains(app) { + pr_touched.push(app.clone()); + } else { + also_drifted.push(app.clone()); + } + } + + Some(PrPreviewLabels { + pr_touched_apps: pr_touched, + also_still_drifted_apps: also_drifted, + }) +} + +/// Resolve changed paths from CLI flags / env / GitHub PR git range. +pub fn resolve_changed_paths( + changed_paths_file: Option<&Path>, + changed_paths_csv: Option<&str>, +) -> Option> { + if let Some(file) = changed_paths_file { + return read_paths_file(file); + } + if let Some(csv) = changed_paths_csv + .map(str::trim) + .filter(|value| !value.is_empty()) + { + return Some(split_path_list(csv)); + } + if let Ok(raw) = std::env::var("DESLICER_CHANGED_PATHS") { + let trimmed = raw.trim(); + if !trimmed.is_empty() { + return Some(split_path_list(trimmed)); + } + } + discover_github_pr_changed_paths() +} + +fn split_path_list(raw: &str) -> Vec { + let mut paths = Vec::new(); + let mut seen = BTreeSet::new(); + for part in raw.split(['\n', '\r', ',', ';']) { + let path = normalize_repo_path(part); + if !path.is_empty() && seen.insert(path.clone()) { + paths.push(path); + } + } + paths +} + +fn read_paths_file(path: &Path) -> Option> { + let content = std::fs::read_to_string(path).ok()?; + let paths = split_path_list(&content); + if paths.is_empty() { + None + } else { + Some(paths) + } +} + +fn discover_github_pr_changed_paths() -> Option> { + let event_name = std::env::var("GITHUB_EVENT_NAME").ok()?; + if event_name != "pull_request" && event_name != "pull_request_target" { + return None; + } + let event_path = std::env::var("GITHUB_EVENT_PATH").ok()?; + let body: Value = serde_json::from_str(&std::fs::read_to_string(event_path).ok()?).ok()?; + let base = body + .pointer("/pull_request/base/sha") + .and_then(Value::as_str)? + .to_string(); + let head = body + .pointer("/pull_request/head/sha") + .and_then(Value::as_str) + .map(str::to_string) + .or_else(|| std::env::var("GITHUB_SHA").ok())?; + git_diff_name_only(&base, &head) +} + +fn git_diff_name_only(base: &str, head: &str) -> Option> { + let output = Command::new("git") + .args(["diff", "--name-only", &format!("{base}...{head}")]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let text = String::from_utf8_lossy(&output.stdout); + let paths = split_path_list(&text); + if paths.is_empty() { + None + } else { + Some(paths) + } +} + +/// Build labels from a dry-run body + optional env YAML + changed paths. +pub fn labels_for_plan_context( + diff_body: Option<&Value>, + env_yaml: Option<&str>, + changed_paths: &[String], +) -> Option { + let diff = diff_body?; + let drifted = app_names_from_diff(diff); + if drifted.is_empty() { + return None; + } + let mapped = env_yaml.map(mapped_apps_from_yaml).unwrap_or_default(); + label_plan_apps(&drifted, &mapped, changed_paths) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn extracts_unique_app_names_from_plan_diff() { + let body = json!({ + "diff": { + "change_items": [ + { "app_name": "demo_ci_app", "config_path": "local/inputs.conf" }, + { "app_name": "TA-linux", "config_path": "default/props.conf" }, + { "app_name": "demo_ci_app", "config_path": "local/props.conf" } + ] + } + }); + assert_eq!( + app_names_from_diff(&body), + vec!["TA-linux".to_string(), "demo_ci_app".to_string()] + ); + } + + #[test] + fn path_prefix_match_is_rooted() { + assert!(path_touches_source( + "apps/demo_ci_app/local/inputs.conf", + "apps/demo_ci_app" + )); + assert!(path_touches_source("apps/demo_ci_app", "apps/demo_ci_app")); + assert!(!path_touches_source( + "apps/demo_ci_app_extra/local/x.conf", + "apps/demo_ci_app" + )); + } + + #[test] + fn labels_split_touched_vs_leftover_drift() { + let drifted = vec!["demo_ci_app".into(), "TA-linux".into()]; + let mapped = vec![ + ("apps/demo_ci_app".into(), "demo_ci_app".into()), + ("apps/TA-linux".into(), "TA-linux".into()), + ]; + let changed = vec!["apps/demo_ci_app/local/inputs.conf".into()]; + let labels = label_plan_apps(&drifted, &mapped, &changed).expect("labels"); + assert_eq!(labels.pr_touched_apps, vec!["demo_ci_app".to_string()]); + assert_eq!(labels.also_still_drifted_apps, vec!["TA-linux".to_string()]); + assert!(labels + .human_summary() + .contains("this PR touches: demo_ci_app")); + assert!(labels + .human_summary() + .contains("also still drifted: TA-linux")); + } + + #[test] + fn env_yaml_change_marks_all_mapped_as_touched() { + let drifted = vec!["demo_ci_app".into(), "TA-linux".into()]; + let mapped = vec![ + ("apps/demo_ci_app".into(), "demo_ci_app".into()), + ("apps/TA-linux".into(), "TA-linux".into()), + ]; + let changed = vec![".deslicer/environments/acme-prod.yml".into()]; + let labels = label_plan_apps(&drifted, &mapped, &changed).expect("labels"); + assert_eq!(labels.pr_touched_apps.len(), 2); + assert!(labels.also_still_drifted_apps.is_empty()); + } + + #[test] + fn no_changed_paths_skips_labeling() { + let drifted = vec!["demo_ci_app".into()]; + assert!(label_plan_apps(&drifted, &[], &[]).is_none()); + } + + #[test] + fn mapped_apps_from_yaml_use_basename_labels() { + let yaml = "\ +destinations: + - inventory_group: indexers + apps: + - source_path: apps/demo_ci_app + - source_path: apps/TA-linux +"; + let mapped = mapped_apps_from_yaml(yaml); + assert_eq!( + mapped, + vec![ + ("apps/demo_ci_app".into(), "demo_ci_app".into()), + ("apps/TA-linux".into(), "TA-linux".into()), + ] + ); + } + + #[test] + fn split_path_list_dedupes() { + let paths = split_path_list("apps/a\napps/b,apps/a"); + assert_eq!(paths, vec!["apps/a".to_string(), "apps/b".to_string()]); + } +} From 6c18e92a04e6570fb78db22e46056fb70c32053a Mon Sep 17 00:00:00 2001 From: SaikrishnaGundeti Date: Thu, 3 Sep 2026 13:33:05 +0200 Subject: [PATCH 2/2] docs: note Observer migration 206 for multi-group same SHA Co-authored-by: Cursor --- docs/environments.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/environments.md b/docs/environments.md index 0d80266..c54a4b4 100644 --- a/docs/environments.md +++ b/docs/environments.md @@ -151,11 +151,11 @@ Select deploy per job: if: github.ref == 'refs/heads/main' ``` -**Blocker (multi-group, same SHA):** Observer’s idempotent index is currently -`(tenant_id, repository_url, commit_sha)`. Two plans for different -`target_group_id` values on the same commit collide until that index includes -`target_group_id`. Prefer one destination-with-apps per stem, or different -commits/environments, until that Observer change lands. +**Multi-group, same SHA:** Observer migration `206` scopes the idempotent +index by `target_group_id` (plus an unscoped index when the group is null), so +matrix/fan-out across inventory groups on one commit is supported on Observer +builds that include that migration. Older Observer builds still collide on +`(tenant, repo, commit)` alone. ## Lifecycle gates (Path A2)