update in oss to make enterprise cli interactive#1612
update in oss to make enterprise cli interactive#1612nikhilsinhaparseable merged 2 commits intoparseablehq:mainfrom
Conversation
WalkthroughAdded enterprise environment-variable prompting with mode detection, a process-wide guard to parse Changes
Sequence DiagramsequenceDiagram
actor User
participant EnvFile as .parseable.env
participant CLI as CLI Args
participant Detect as Mode Detection
participant Base as Base Prompts
participant Mode as Mode-Specific Prompts
participant Collector as collect_prompts
User->>EnvFile: load_env_file()
EnvFile-->>User: parsed envs (only once via ENV_FILE_LOADED)
CLI->>Detect: read --mode / args
EnvFile->>Detect: read P_MODE
Detect-->>User: resolved mode (or unset)
User->>Base: prompt_enterprise_envs()
Base->>Collector: collect base prompts (license, P_CLUSTER_SECRET, maybe P_MODE)
Collector-->>User: prompt questions
User-->>Collector: responses
Collector-->>Base: base env pairs
Base->>Mode: get_enterprise_mode_prompts(resolved mode)
Mode->>Collector: collect mode-specific prompts
Collector-->>User: mode-specific questions
User-->>Collector: responses
Collector-->>Mode: mode-specific env pairs
Mode-->>User: combined enterprise env pairs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/interactive.rs`:
- Around line 530-537: detect_mode() can return None causing unwrap_or_default()
to produce an empty string and the header to read " mode"; change the fallback
to a descriptive string (e.g., "default" or "unknown") so the call becomes
detect_mode().unwrap_or("default".to_string()) (or equivalent) and then pass
that into collect_prompts(&mode_prompts, is_interactive, &format!("{mode}
mode"), &mut collected) so the printed header is meaningful when no mode is
detected.
- Around line 253-264: The CLI parsing in detect_mode() returns Some("all") for
"--mode all" whereas the env var branch normalizes "all" to None; update the CLI
arg handling in detect_mode() (the loop over std::env::args()) to mirror the env
var logic by mapping any value that equals "all" (case-insensitive) to None
instead of Some("all") so detect_mode() returns None for both env and CLI "all"
inputs, ensuring get_enterprise_base_prompts() sees is_none() consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8b877fc3-5336-4759-a255-4cb424a988b4
📒 Files selected for processing (2)
src/interactive.rssrc/lib.rs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/interactive.rs (1)
246-264:⚠️ Potential issue | 🟠 MajorCLI
--modeis ignored whenP_MODEis set.At Line 247,
detect_mode()returns fromP_MODEbefore evaluating CLI args (Lines 255-264). An explicit CLI mode can’t override env-derived mode, which can drive the wrong enterprise prompt set.🐛 Proposed fix
fn detect_mode() -> Option<String> { - if let Ok(mode) = std::env::var(MODE_ENV) { - let mode = mode.to_lowercase(); - if mode != "all" { - return Some(mode); - } - return None; - } - let args: Vec<String> = std::env::args().collect(); for (i, arg) in args.iter().enumerate() { let value = if arg == "--mode" { args.get(i + 1).map(|v| v.to_lowercase()) } else { arg.strip_prefix("--mode=").map(|v| v.to_lowercase()) }; if let Some(mode) = value { return (mode != "all").then_some(mode); } } - None + if let Ok(mode) = std::env::var(MODE_ENV) { + let mode = mode.to_lowercase(); + return (mode != "all").then_some(mode); + } + + None }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/interactive.rs` around lines 246 - 264, detect_mode currently returns immediately when MODE_ENV is set, preventing CLI "--mode" from overriding it; change the logic in detect_mode so it parses CLI args first (inspect std::env::args for "--mode" and "--mode=...") and if a mode is found return that (return (mode != "all").then_some(mode)), otherwise fall back to reading std::env::var(MODE_ENV) and return that (again treating "all" as None). Refer to the detect_mode function, the MODE_ENV check using std::env::var, and the CLI parsing branch that looks for "--mode" / "--mode=" to implement the reorder/fallback behavior.
🧹 Nitpick comments (1)
src/interactive.rs (1)
120-123: Usestd::sync::Onceinstead ofAtomicBool::swap(true, Relaxed)for one-time env file loading.The current implementation calls
swap(true)before file parsing completes (line 121 before lines 124+), creating a race condition: concurrent callers could return early before env vars are populated.Onceprovides the correct "initialize exactly once and wait until done" semantics. Whileload_env_file()is currently only called during single-threaded startup, usingOncewould make the code robust against future concurrent usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/interactive.rs` around lines 120 - 123, Replace the current atomic-flag pattern in load_env_file (which uses ENV_FILE_LOADED.swap(true, Ordering::Relaxed)) with a std::sync::Once to guarantee waiting until initialization completes; specifically, introduce a Once (e.g., static ENV_FILE_ONCE: Once = Once::new()) and call ENV_FILE_ONCE.call_once(|| { /* existing env-file parsing and setting logic moved here */ }), removing the swap and early return so concurrent callers block until the call_once closure finishes, preserving the existing parsing behavior but with correct one-time initialization semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/interactive.rs`:
- Around line 246-264: detect_mode currently returns immediately when MODE_ENV
is set, preventing CLI "--mode" from overriding it; change the logic in
detect_mode so it parses CLI args first (inspect std::env::args for "--mode" and
"--mode=...") and if a mode is found return that (return (mode !=
"all").then_some(mode)), otherwise fall back to reading std::env::var(MODE_ENV)
and return that (again treating "all" as None). Refer to the detect_mode
function, the MODE_ENV check using std::env::var, and the CLI parsing branch
that looks for "--mode" / "--mode=" to implement the reorder/fallback behavior.
---
Nitpick comments:
In `@src/interactive.rs`:
- Around line 120-123: Replace the current atomic-flag pattern in load_env_file
(which uses ENV_FILE_LOADED.swap(true, Ordering::Relaxed)) with a
std::sync::Once to guarantee waiting until initialization completes;
specifically, introduce a Once (e.g., static ENV_FILE_ONCE: Once = Once::new())
and call ENV_FILE_ONCE.call_once(|| { /* existing env-file parsing and setting
logic moved here */ }), removing the swap and early return so concurrent callers
block until the call_once closure finishes, preserving the existing parsing
behavior but with correct one-time initialization semantics.
Summary by CodeRabbit
New Features
Refactor