fix: address Codex review — 6 fixes for v2 installer

Based on full review of rc11..rc19 changes:

1. ENVIRONMENT NAME PERSISTENCE: Read from terraform state first,
   then .loki-env file, then generate new suffix. Prevents destroying
   old resources on reinstall from same checkout.

2. PACK/PROFILE SELECTION: PacksLoaded and ProfilesLoaded no longer
   auto-chain to the next action. They set the cursor on the default
   and render the screen, so user can see and change the selection
   before pressing Enter.

3. UTF-8 SAFE TRUNCATION: Deploy screen log truncation now uses
   chars().take() instead of byte slicing to avoid panics on
   multibyte characters.

4. ARTIFACT LOG GARBLING: ArtifactRecorded values now have newlines
   collapsed to spaces before display. Strip \r from terraform output.

5. BEDROCK FORM: Reverted enable_satellite_services to true (security
   services should run on first install). Instead, gated bedrock form
   invoke on request_quota_increases == true (defaults to false).

6. STACK NAME IN POST-INSTALL: Record environment_name as stack_name
   artifact so post-install screen shows the actual deployed name.

Author: Loki

REVIEW.md

@@ -0,0 +1,139 @@
+# Review: `v0.2.0-rc11..v0.2.0-rc19`
+
+## CRITICAL
+
+No critical findings in this range.
+
+## HIGH
+
+### 1. `enable_satellite_services` is now effectively hard-disabled for all Terraform installs
+
+Files:
+- `deploy/terraform/variables.tf:206`
+- `tools/loki-installer/src/adapters/terraform.rs:444`
+- `methods/terraform.yaml:12`
+- `packs/openclaw/manifest.yaml:19`
+
+What changed:
+- `enable_satellite_services` now defaults to `false` in Terraform.
+- The adapter can only pass `-var=enable_satellite_services=...` if `adapter_options` contains that key.
+- Nothing in the Terraform method manifest or pack manifests exposes `enable_satellite_services` as a user-selectable or defaulted option.
+
+Impact:
+- Fresh Terraform installs will now skip the entire satellite-services path by default, including the Bedrock form Lambda, security enablement Lambda, and builder admin-user setup.
+- The description in `variables.tf` says this should be disabled only for reinstalls or existing-VPC cases, but the current wiring makes `false` the only reachable value.
+
+Why this is a bug:
+- This is not just a safer default. It is a behavioral regression that silently removes resources from every Terraform deployment.
+
+Recommended fix:
+- Keep the Terraform variable default aligned with the intended default behavior for first installs.
+- If you want reinstall-specific behavior, derive it in the installer and pass it explicitly, instead of changing the Terraform default to an unreachable value.
+
+### 2. The environment-name persistence fix is repo-global and does not actually solve reinstall/state continuity safely
+
+Files:
+- `tools/loki-installer/src/adapters/terraform.rs:196`
+
+What changed:
+- When `resolved_stack_name` is absent, Terraform now reads `deploy/terraform/.loki-env` and reuses `ENVIRONMENT_NAME`.
+- If the file does not exist, it generates a random suffix and immediately writes it back.
+
+Impact:
+- All future Terraform installs from the same checkout share one repo-local environment name, regardless of pack, profile, region, AWS account, or workspace.
+- A second install from the same repo can unintentionally target the first install’s naming namespace.
+- A reinstall from a fresh checkout still generates a new name, so the original “destroy old resources on reinstall” problem is not actually solved across machines or clones.
+
+Why this is a bug:
+- The fix stores identity in an unscoped local sidecar file rather than in deployment state.
+- That avoids churn only in the narrow “same checkout, same repo dir” case, and creates cross-install collisions in the broader case.
+
+Recommended fix:
+- Prefer existing Terraform state first, not a random local file.
+- Practical order:
+  1. If the user supplied `stack_name`, use it.
+  2. If Terraform state already exists for the selected workspace, read the prior `environment_name` from state before planning.
+  3. Only if no state exists, generate a name once and persist it.
+- If you keep a local file fallback, key it by at least workspace + pack + profile + region + AWS account, not a single repo-wide `.loki-env`.
+
+### 3. Deploy log truncation is not safe and likely does not fix the reported garbling reliably
+
+Files:
+- `tools/loki-installer/src/tui/screens/deploy.rs:15`
+- `tools/loki-installer/src/tui/runtime.rs:334`
+
+What changed:
+- The deploy screen now truncates log lines using `content_with_width`.
+- Truncation is done with `line.len()` and `&line[..max_line]`.
+
+Impact:
+- `len()` is byte length, not terminal display width.
+- `&line[..max_line]` can panic if `max_line` lands inside a multibyte UTF-8 character.
+- This code uses the outer widget width (`body[1].width`) rather than the inner content width after borders, so even ASCII truncation is off by the block chrome.
+
+Why this is a bug:
+- The current implementation is not width-aware and is not UTF-8 safe.
+- That means it can still wrap badly, and with the wrong input it can crash instead of merely truncating.
+
+Recommended fix:
+- Measure display width with `unicode-width`.
+- Truncate by `chars()` or grapheme boundaries, not raw byte slicing.
+- Use the inner paragraph width, not the block width, when computing available columns.
+
+## MEDIUM
+
+### 4. Simple mode still does not let the user see or change the preselected pack/profile before the plan is built
+
+Files:
+- `tools/loki-installer/src/tui/update.rs:280`
+- `tools/loki-installer/src/tui/update.rs:307`
+- `tools/loki-installer/src/tui/runtime.rs:93`
+
+What happens:
+- In simple mode, `DoctorPreflight -> LoadPacks`.
+- `PacksLoaded` auto-selects `openclaw` and returns `LoadProfiles`.
+- `ProfilesLoaded` auto-selects `builder` and returns `LoadMethods`.
+- In `ProfileSelection`, simple mode immediately returns `BuildPlan`.
+- `run_actions()` drains this queue synchronously in one pass.
+
+Impact:
+- The user does not get an interactive stop on pack or profile selection.
+- They jump straight from doctor to review.
+- The only way to change pack/profile is later, from review, by pressing `A` to switch modes and restart selection.
+
+Answer to the question:
+- No, not in the current simple-mode runtime flow.
+
+Recommended fix:
+- Decide which behavior you want:
+  - If simple mode should still be editable, stop the queue on the pack/profile screens and wait for input.
+  - If simple mode should be non-editable, make that explicit in the UX and present the chosen defaults only on review.
+
+### 5. The new post-install summary will often show `Stack: <environment_name>` for Terraform installs
+
+Files:
+- `tools/loki-installer/src/tui/screens/post_install.rs:25`
+- `methods/terraform.yaml:5`
+- `tools/loki-installer/src/adapters/terraform.rs:380`
+
+What changed:
+- The post-install screen now tries `resolved_stack_name`, then `artifacts["stack_name"]`, then `request.stack_name`.
+- For Terraform, `requires_stack_name` is `false`, so `resolved_stack_name` is normally `None`.
+- The Terraform adapter records outputs like `instance_id`/`public_ip`, but it does not record the chosen `environment_name` as an artifact.
+
+Impact:
+- Successful Terraform installs can land on the new completion screen with a placeholder instead of the actual deployed environment name.
+
+Recommended fix:
+- Record the final `environment_name` into session artifacts during Terraform apply/output capture, and have post-install read that artifact explicitly.
+
+## LOW
+
+### 6. The `admin_setup_basic` count mismatch was fixed correctly
+
+Files:
+- `deploy/terraform/main.tf:638`
+
+Assessment:
+- The new `count = var.enable_satellite_services && var.profile_name == "builder" ? 1 : 0` now matches the producer resource `aws_iam_role.admin_setup_lambda[0]`.
+- I did not find a remaining count/index mismatch in this specific block after the change.

deploy/terraform/main.tf

@@ -347,7 +347,7 @@ resource "aws_lambda_function" "bedrock_form" {
 }
 
 resource "null_resource" "bedrock_form_invoke" {
-  count      = var.enable_satellite_services ? 1 : 0
+  count      = var.enable_satellite_services && var.request_quota_increases == "true" ? 1 : 0
   depends_on = [aws_lambda_function.bedrock_form]
 
   provisioner "local-exec" {

deploy/terraform/variables.tf

@@ -205,7 +205,7 @@ variable "request_quota_increases" {
 
 variable "enable_satellite_services" {
   type        = bool
-  default     = false
+  default     = true
   description = "Enable satellite services (security monitoring, admin user, bedrock form lambda). Set false for re-installs or existing VPC deployments."
 }
 

tools/loki-installer/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "loki-installer"
-version = "0.2.0-rc19"
+version = "0.2.0-rc20"
 edition = "2024"
 description = "Rust implementation of the Loki Installer V2 CLI, planner, TUI, and session runtime"
 license = "Apache-2.0"

tools/loki-installer/Cargo.toml

@@ -166,7 +166,10 @@ pub(crate) async fn run_command_streaming(
 fn strip_ansi(input: &str) -> String {
     let re = Regex::new(r"\x1b\[[0-9;]*[a-zA-Z]").unwrap();
     let cleaned = re.replace_all(input, "");
-    cleaned.replace(['│', '╵', '╷'], "").trim().to_string()
+    cleaned
+        .replace(['│', '╵', '╷'], "").replace('\r', "")
+        .trim()
+        .to_string()
 }
 
 pub(crate) fn spawn_child(spec: &CommandSpec) -> Result<tokio::process::Child, AdapterError> {

tools/loki-installer/src/adapters/support.rs

@@ -197,9 +197,29 @@ impl TerraformContext {
             .resolved_stack_name
             .clone()
             .unwrap_or_else(|| {
+                // 1. Try reading from existing terraform state
+                if let Some(name) = read_env_name_from_tf_state(&working_dir) {
+                    return name;
+                }
+                // 2. Try reading from .loki-env (persisted from previous install)
+                let env_file = Path::new(&working_dir).join(".loki-env");
+                if let Ok(contents) = fs::read_to_string(&env_file) {
+                    for line in contents.lines() {
+                        if let Some(name) = line.strip_prefix("ENVIRONMENT_NAME=") {
+                            let name = name.trim();
+                            if !name.is_empty() {
+                                return name.to_string();
+                            }
+                        }
+                    }
+                }
+                // 3. Generate a new unique name
                 let suffix = &uuid::Uuid::new_v4().to_string()[..8];
                 format!("loki-{}-{suffix}", plan.resolved_pack.id)
             });
+        // Persist environment name for future re-installs
+        let env_file = Path::new(&working_dir).join(".loki-env");
+        let _ = fs::write(&env_file, format!("ENVIRONMENT_NAME={environment_name}\n"));
 
         Ok(Self {
             terraform_bin,
@@ -240,6 +260,7 @@ async fn apply_terraform(
     }
 
     let mut artifacts = BTreeMap::new();
+    artifacts.insert("stack_name".into(), context.environment_name.clone());
 
     for step in &plan.deploy_steps {
         if phase_is_past(session.phase, step.phase) {
@@ -608,6 +629,37 @@ fn terraform_install_path() -> Result<PathBuf, AdapterError> {
     Ok(dir.join("terraform"))
 }
 
+fn read_env_name_from_tf_state(working_dir: &str) -> Option<String> {
+    let state_path = Path::new(working_dir).join("terraform.tfstate");
+    let contents = fs::read_to_string(&state_path).ok()?;
+    // Look for environment_name in the state's root module outputs or resource attributes
+    // The IAM role name pattern is "${environment_name}-role"
+    for line in contents.lines() {
+        let line = line.trim();
+        if line.contains("\"name\"") && line.contains("-role\"") {
+            // Extract name like "loki-openclaw-abc12345-role"
+            if let (Some(start), Some(end)) = (line.find('"'), line.rfind("-role\"")) {
+                let candidate = &line[start + 1..end];
+                if candidate.starts_with("loki-") {
+                    return Some(candidate.to_string());
+                }
+            }
+        }
+    }
+    // Fallback: try terraform output
+    let output = Command::new("terraform")
+        .args(["-chdir", working_dir, "output", "-raw", "environment_name"])
+        .output()
+        .ok()?;
+    if output.status.success() {
+        let name = String::from_utf8_lossy(&output.stdout).trim().to_string();
+        if !name.is_empty() {
+            return Some(name);
+        }
+    }
+    None
+}
+
 fn can_write_to(dir: &Path) -> bool {
     if fs::create_dir_all(dir).is_err() {
         return false;

tools/loki-installer/src/adapters/terraform.rs

@@ -243,6 +243,7 @@ impl InstallEventSink for TuiEventSink {
                     .await
             }
             InstallEvent::ArtifactRecorded { key, value } => {
+                let value = value.lines().collect::<Vec<_>>().join(" ");
                 let display_value = if value.len() > 80 {
                     format!("{}…", &value[..80])
                 } else {

tools/loki-installer/src/tui/runtime.rs

@@ -75,8 +75,10 @@ pub fn content_with_width(state: &AppState, max_width: usize) -> Text<'static> {
     for line in &state.deployment.logs[start..end] {
         // Truncate long lines to avoid horizontal overflow
         let max_line = max_width.saturating_sub(6);
-        let display = if line.len() > max_line {
-            format!(" - {}…", &line[..max_line])
+        let char_len: usize = line.chars().count();
+        let display = if char_len > max_line {
+            let truncated: String = line.chars().take(max_line).collect();
+            format!(" - {truncated}…")
         } else {
             format!(" - {line}")
         };

tools/loki-installer/src/tui/screens/deploy.rs

@@ -119,10 +119,6 @@ pub fn update(state: &mut AppState, event: InstallerEvent) -> Vec<AppAction> {
                         state.request_draft.pack_id = Some(pack.id.clone());
                         state.request_draft.profile_id = pack.default_profile.clone();
                         state.request_draft.method_id = pack.default_method;
-                        state.auto_selected_pack = true;
-                        return vec![AppAction::LoadProfiles {
-                            pack_id: pack.id.clone(),
-                        }];
                     }
                 }
             }
@@ -142,12 +138,6 @@ pub fn update(state: &mut AppState, event: InstallerEvent) -> Vec<AppAction> {
                     state.request_draft.profile_cursor = profile_idx;
                     if let Some(profile) = state.profiles.get(profile_idx) {
                         state.request_draft.profile_id = Some(profile.id.clone());
-                        state.auto_selected_profile = true;
-                        if let Some(pack_id) = &state.request_draft.pack_id {
-                            return vec![AppAction::LoadMethods {
-                                pack_id: pack_id.clone(),
-                            }];
-                        }
                     }
                 }
             }
@@ -630,11 +620,8 @@ mod tests {
 
         assert_eq!(state.packs.len(), 1);
         assert_eq!(state.request_draft.pack_id.as_deref(), Some("openclaw"));
-        assert!(state.auto_selected_pack);
-        assert!(matches!(
-            actions.as_slice(),
-            [AppAction::LoadProfiles { pack_id }] if pack_id == "openclaw"
-        ));
+        assert_eq!(state.screen, ScreenId::PackSelection);
+        assert!(matches!(actions.as_slice(), [AppAction::Render]));
     }
 
     #[test]
@@ -671,11 +658,8 @@ mod tests {
         );
 
         assert_eq!(state.request_draft.profile_id.as_deref(), Some("builder"));
-        assert!(state.auto_selected_profile);
-        assert!(matches!(
-            actions.as_slice(),
-            [AppAction::LoadMethods { pack_id }] if pack_id == "openclaw"
-        ));
+        assert_eq!(state.screen, ScreenId::ProfileSelection);
+        assert!(matches!(actions.as_slice(), [AppAction::Render]));
     }
 
     #[test]