refactor(g3-cli): eliminate code-path aliasing in config and project content loading
Consolidate duplicated logic into canonical shared functions: - Extract load_config_with_cli_overrides() to utils.rs - Was duplicated in lib.rs and accumulative.rs with subtle differences - lib.rs version had Chrome diagnostics + provider validation - accumulative.rs version was missing both - Now all callers use the complete canonical implementation - Extract combine_project_content() to project_files.rs - Was duplicated inline in lib.rs and agent_mode.rs - Simplified implementation using iterator flatten - Added unit tests for all cases This eliminates drift risk where the duplicated implementations could diverge over time (accumulative.rs was already missing Chrome diagnostics and provider validation). Agent: fowler
This commit is contained in:
@@ -7,7 +7,6 @@ use rustyline::DefaultEditor;
|
||||
use std::path::PathBuf;
|
||||
use tracing::error;
|
||||
|
||||
use g3_config::Config;
|
||||
use g3_core::project::Project;
|
||||
use g3_core::Agent;
|
||||
|
||||
@@ -16,6 +15,7 @@ use crate::cli_args::Cli;
|
||||
use crate::interactive::run_interactive;
|
||||
use crate::simple_output::SimpleOutput;
|
||||
use crate::ui_writer_impl::ConsoleUiWriter;
|
||||
use crate::utils::load_config_with_cli_overrides;
|
||||
|
||||
/// Run accumulative autonomous mode - accumulates requirements from user input
|
||||
/// and runs autonomous mode after each input.
|
||||
@@ -309,35 +309,3 @@ async fn handle_command(
|
||||
_ => Ok(CommandResult::Unknown),
|
||||
}
|
||||
}
|
||||
|
||||
fn load_config_with_cli_overrides(cli: &Cli) -> Result<Config> {
|
||||
let mut config = Config::load_with_overrides(
|
||||
cli.config.as_deref(),
|
||||
cli.provider.clone(),
|
||||
cli.model.clone(),
|
||||
)?;
|
||||
|
||||
// Apply webdriver flag override
|
||||
if cli.webdriver {
|
||||
config.webdriver.enabled = true;
|
||||
}
|
||||
|
||||
// Apply chrome-headless flag override
|
||||
if cli.chrome_headless {
|
||||
config.webdriver.enabled = true;
|
||||
config.webdriver.browser = g3_config::WebDriverBrowser::ChromeHeadless;
|
||||
}
|
||||
|
||||
// Apply safari flag override
|
||||
if cli.safari {
|
||||
config.webdriver.enabled = true;
|
||||
config.webdriver.browser = g3_config::WebDriverBrowser::Safari;
|
||||
}
|
||||
|
||||
// Apply no-auto-compact flag override
|
||||
if cli.manual_compact {
|
||||
config.agent.auto_compact = false;
|
||||
}
|
||||
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
@@ -7,7 +7,7 @@ use tracing::debug;
|
||||
use g3_core::ui_writer::UiWriter;
|
||||
use g3_core::Agent;
|
||||
|
||||
use crate::project_files::{read_agents_config, read_project_memory, read_project_readme};
|
||||
use crate::project_files::{combine_project_content, read_agents_config, read_project_memory, read_project_readme};
|
||||
use crate::simple_output::SimpleOutput;
|
||||
use crate::ui_writer_impl::ConsoleUiWriter;
|
||||
|
||||
@@ -177,23 +177,11 @@ pub async fn run_agent_mode(
|
||||
));
|
||||
|
||||
// Combine all content for the agent's context
|
||||
let combined_content = {
|
||||
let mut parts = Vec::new();
|
||||
if let Some(agents) = agents_content_opt {
|
||||
parts.push(agents);
|
||||
}
|
||||
if let Some(readme) = readme_content_opt {
|
||||
parts.push(readme);
|
||||
}
|
||||
if let Some(memory) = memory_content_opt {
|
||||
parts.push(memory);
|
||||
}
|
||||
if parts.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(parts.join("\n\n"))
|
||||
}
|
||||
};
|
||||
let combined_content = combine_project_content(
|
||||
agents_content_opt,
|
||||
readme_content_opt,
|
||||
memory_content_opt,
|
||||
);
|
||||
|
||||
// Create agent with custom system prompt
|
||||
let ui_writer = ConsoleUiWriter::new();
|
||||
|
||||
@@ -31,10 +31,10 @@ use accumulative::run_accumulative_mode;
|
||||
use agent_mode::run_agent_mode;
|
||||
use autonomous::run_autonomous;
|
||||
use interactive::run_interactive;
|
||||
use project_files::{read_agents_config, read_project_memory, read_project_readme};
|
||||
use project_files::{combine_project_content, read_agents_config, read_project_memory, read_project_readme};
|
||||
use simple_output::SimpleOutput;
|
||||
use ui_writer_impl::ConsoleUiWriter;
|
||||
use utils::setup_workspace_directory;
|
||||
use utils::{load_config_with_cli_overrides, setup_workspace_directory};
|
||||
|
||||
pub async fn run() -> Result<()> {
|
||||
let cli = Cli::parse();
|
||||
@@ -162,77 +162,6 @@ fn create_project(cli: &Cli, workspace_dir: &PathBuf) -> Result<Project> {
|
||||
}
|
||||
}
|
||||
|
||||
fn load_config_with_cli_overrides(cli: &Cli) -> Result<Config> {
|
||||
let mut config = Config::load_with_overrides(
|
||||
cli.config.as_deref(),
|
||||
cli.provider.clone(),
|
||||
cli.model.clone(),
|
||||
)?;
|
||||
|
||||
// Apply webdriver flag override
|
||||
if cli.webdriver {
|
||||
config.webdriver.enabled = true;
|
||||
}
|
||||
|
||||
// Apply chrome-headless flag override
|
||||
if cli.chrome_headless {
|
||||
config.webdriver.enabled = true;
|
||||
config.webdriver.browser = g3_config::WebDriverBrowser::ChromeHeadless;
|
||||
|
||||
// Run Chrome diagnostics on first use
|
||||
let report =
|
||||
g3_computer_control::run_chrome_diagnostics(config.webdriver.chrome_binary.as_deref());
|
||||
println!("{}", report.format_report());
|
||||
}
|
||||
|
||||
// Apply safari flag override
|
||||
if cli.safari {
|
||||
config.webdriver.enabled = true;
|
||||
config.webdriver.browser = g3_config::WebDriverBrowser::Safari;
|
||||
}
|
||||
|
||||
// Apply no-auto-compact flag override
|
||||
if cli.manual_compact {
|
||||
config.agent.auto_compact = false;
|
||||
}
|
||||
|
||||
// Validate provider if specified
|
||||
if let Some(ref provider) = cli.provider {
|
||||
let valid_providers = ["anthropic", "databricks", "embedded", "openai"];
|
||||
if !valid_providers.contains(&provider.as_str()) {
|
||||
return Err(anyhow::anyhow!(
|
||||
"Invalid provider '{}'. Valid options: {:?}",
|
||||
provider,
|
||||
valid_providers
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
fn combine_project_content(
|
||||
agents_content: Option<String>,
|
||||
readme_content: Option<String>,
|
||||
memory_content: Option<String>,
|
||||
) -> Option<String> {
|
||||
let mut parts = Vec::new();
|
||||
if let Some(agents) = agents_content {
|
||||
parts.push(agents);
|
||||
}
|
||||
if let Some(readme) = readme_content {
|
||||
parts.push(readme);
|
||||
}
|
||||
if let Some(memory) = memory_content {
|
||||
parts.push(memory);
|
||||
}
|
||||
if parts.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(parts.join("\n\n"))
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_console_mode(
|
||||
cli: Cli,
|
||||
config: Config,
|
||||
|
||||
@@ -82,6 +82,26 @@ pub fn read_project_memory(workspace_dir: &Path) -> Option<String> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Combine AGENTS.md, README, and memory content into a single string.
|
||||
///
|
||||
/// Returns None if all inputs are None, otherwise joins non-None parts with double newlines.
|
||||
pub fn combine_project_content(
|
||||
agents_content: Option<String>,
|
||||
readme_content: Option<String>,
|
||||
memory_content: Option<String>,
|
||||
) -> Option<String> {
|
||||
let parts: Vec<String> = [agents_content, readme_content, memory_content]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.collect();
|
||||
|
||||
if parts.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(parts.join("\n\n"))
|
||||
}
|
||||
}
|
||||
|
||||
/// Format a byte size for display.
|
||||
fn format_size(len: usize) -> String {
|
||||
if len < 1000 {
|
||||
@@ -178,4 +198,26 @@ mod tests {
|
||||
assert!(truncated.ends_with("..."));
|
||||
assert_eq!(truncated.len(), 100);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_combine_project_content_all_some() {
|
||||
let result = combine_project_content(
|
||||
Some("agents".to_string()),
|
||||
Some("readme".to_string()),
|
||||
Some("memory".to_string()),
|
||||
);
|
||||
assert_eq!(result, Some("agents\n\nreadme\n\nmemory".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_combine_project_content_partial() {
|
||||
let result = combine_project_content(None, Some("readme".to_string()), None);
|
||||
assert_eq!(result, Some("readme".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_combine_project_content_all_none() {
|
||||
let result = combine_project_content(None, None, None);
|
||||
assert_eq!(result, None);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,10 +2,12 @@
|
||||
|
||||
use anyhow::Result;
|
||||
use crossterm::style::{Color, ResetColor, SetForegroundColor};
|
||||
use g3_config::Config;
|
||||
use g3_core::ui_writer::UiWriter;
|
||||
use g3_core::Agent;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::cli_args::Cli;
|
||||
use crate::simple_output::SimpleOutput;
|
||||
|
||||
/// Display context window progress bar.
|
||||
@@ -89,3 +91,56 @@ pub fn setup_workspace_directory() -> Result<PathBuf> {
|
||||
|
||||
Ok(workspace_dir)
|
||||
}
|
||||
|
||||
/// Load configuration with CLI argument overrides applied.
|
||||
///
|
||||
/// This is the canonical function for loading config with CLI overrides.
|
||||
/// All CLI entry points should use this to ensure consistent behavior.
|
||||
pub fn load_config_with_cli_overrides(cli: &Cli) -> Result<Config> {
|
||||
let mut config = Config::load_with_overrides(
|
||||
cli.config.as_deref(),
|
||||
cli.provider.clone(),
|
||||
cli.model.clone(),
|
||||
)?;
|
||||
|
||||
// Apply webdriver flag override
|
||||
if cli.webdriver {
|
||||
config.webdriver.enabled = true;
|
||||
}
|
||||
|
||||
// Apply chrome-headless flag override
|
||||
if cli.chrome_headless {
|
||||
config.webdriver.enabled = true;
|
||||
config.webdriver.browser = g3_config::WebDriverBrowser::ChromeHeadless;
|
||||
|
||||
// Run Chrome diagnostics on first use
|
||||
let report =
|
||||
g3_computer_control::run_chrome_diagnostics(config.webdriver.chrome_binary.as_deref());
|
||||
println!("{}", report.format_report());
|
||||
}
|
||||
|
||||
// Apply safari flag override
|
||||
if cli.safari {
|
||||
config.webdriver.enabled = true;
|
||||
config.webdriver.browser = g3_config::WebDriverBrowser::Safari;
|
||||
}
|
||||
|
||||
// Apply no-auto-compact flag override
|
||||
if cli.manual_compact {
|
||||
config.agent.auto_compact = false;
|
||||
}
|
||||
|
||||
// Validate provider if specified
|
||||
if let Some(ref provider) = cli.provider {
|
||||
let valid_providers = ["anthropic", "databricks", "embedded", "openai"];
|
||||
if !valid_providers.contains(&provider.as_str()) {
|
||||
return Err(anyhow::anyhow!(
|
||||
"Invalid provider '{}'. Valid options: {:?}",
|
||||
provider,
|
||||
valid_providers
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user