Skip to content

Commit e81cee1

Browse files
echobtBounty Bot
andauthored
fix: batch fixes for issues #1958, #1959, #1960, #1961, #1962, #1965, #1966, #1968, #1969, #1970 [skip ci] (#372)
Fixes: - #1958: Add early validation for --agent flag in acp command - #1959: Add --cookie flag to scrape command - #1960: Extend --method flag to support POST in scrape command - #1961: Add --no-follow-redirects flag to scrape command - #1962: Improve --attach help documentation in run command - #1965: Show version comparison in upgrade --check output - #1966: Fix pipe crash in upgrade --changelog by handling SIGPIPE gracefully - #1968: Handle 'upgrade VERSION' when VERSION matches current version - #1969: Improve error messages for login --with-api-key in headless environments - #1970: Clarify error message when git repo has no origin remote Co-authored-by: Bounty Bot <bounty-bot@factory.ai>
1 parent fc0d76a commit e81cee1

6 files changed

Lines changed: 170 additions & 62 deletions

File tree

cortex-cli/src/acp_cmd.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! The ACP protocol enables IDE integration (like Zed) with Cortex.
44
//! Supports both stdio and HTTP transports for flexible integration.
55
6-
use anyhow::Result;
6+
use anyhow::{Result, bail};
77
use clap::Parser;
88
use cortex_common::resolve_model_alias;
99
use std::net::SocketAddr;
@@ -56,6 +56,19 @@ pub struct AcpCli {
5656
impl AcpCli {
5757
/// Run the ACP server command.
5858
pub async fn run(self) -> Result<()> {
59+
// Validate agent exists early if specified (Issue #1958)
60+
if let Some(ref agent_name) = self.agent {
61+
let registry = cortex_engine::AgentRegistry::new();
62+
// Scan for agents in standard locations
63+
let _ = registry.scan().await;
64+
if !registry.exists(agent_name).await {
65+
bail!(
66+
"Agent not found: '{}'. Use 'cortex agent list' to see available agents.",
67+
agent_name
68+
);
69+
}
70+
}
71+
5972
// Build configuration
6073
let mut config = cortex_engine::Config::default();
6174

cortex-cli/src/login.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,17 @@ pub async fn run_login_with_api_key(config_overrides: CliConfigOverrides, api_ke
4646
check_duplicate_config_overrides(&config_overrides);
4747
let cortex_home = get_cortex_home();
4848

49+
// Ensure cortex home directory exists for encrypted file fallback
50+
if let Err(e) = std::fs::create_dir_all(&cortex_home) {
51+
print_error(&format!(
52+
"Failed to create cortex home directory at {}: {e}\n\n\
53+
For headless/CI environments, set CORTEX_API_KEY environment variable instead:\n \
54+
export CORTEX_API_KEY=your-api-key",
55+
cortex_home.display()
56+
));
57+
std::process::exit(1);
58+
}
59+
4960
// Create secure auth data
5061
let data = SecureAuthData::with_api_key(api_key);
5162

@@ -67,7 +78,18 @@ pub async fn run_login_with_api_key(config_overrides: CliConfigOverrides, api_ke
6778
std::process::exit(0);
6879
}
6980
Err(e) => {
70-
print_error(&format!("Login failed: {e}"));
81+
// Provide helpful error message for headless environments (Issue #1969)
82+
let error_str = e.to_string();
83+
if error_str.contains("keyring") || error_str.contains("secret service") {
84+
print_error(&format!(
85+
"Login failed in headless environment: {e}\n\n\
86+
For headless/CI environments, use the CORTEX_API_KEY environment variable:\n \
87+
export CORTEX_API_KEY=your-api-key\n\n\
88+
Or on Linux, install a secret service like gnome-keyring or kwallet."
89+
));
90+
} else {
91+
print_error(&format!("Login failed: {e}"));
92+
}
7193
std::process::exit(1);
7294
}
7395
}

cortex-cli/src/pr_cmd.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,15 @@ fn get_git_remote_url() -> Result<String> {
431431
.context("Failed to get git remote URL")?;
432432

433433
if !output.status.success() {
434-
bail!("No 'origin' remote found. Is this a git repository?");
434+
// Provide clearer error message (Issue #1970)
435+
bail!(
436+
"No 'origin' remote found.\n\n\
437+
This is a git repository, but it doesn't have an 'origin' remote configured.\n\
438+
Add one with:\n \
439+
git remote add origin <repository-url>\n\n\
440+
Example:\n \
441+
git remote add origin https://github.com/username/repo.git"
442+
);
435443
}
436444

437445
let url = String::from_utf8_lossy(&output.stdout).trim().to_string();

cortex-cli/src/run_cmd.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,11 @@ pub struct RunCli {
115115
#[arg(long = "title")]
116116
pub title: Option<String>,
117117

118-
/// Attach to a running Cortex server (e.g., http://localhost:3000).
119-
#[arg(long = "attach")]
118+
/// Attach to a running Cortex server instead of starting locally.
119+
/// Value should be the server URL (e.g., "http://localhost:3000").
120+
/// Use this to connect to a remote Cortex instance or an existing
121+
/// local server started with 'cortex serve'.
122+
#[arg(long = "attach", value_name = "URL")]
120123
pub attach: Option<String>,
121124

122125
/// Port for the local server (defaults to random port).

cortex-cli/src/scrape_cmd.rs

Lines changed: 59 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub struct ScrapeCommand {
5858

5959
/// HTTP method to use for the request.
6060
/// Use HEAD to check headers without downloading content.
61-
/// Supported values: GET (default), HEAD.
61+
/// Supported values: GET (default), HEAD, POST.
6262
#[arg(long, default_value = "GET", value_name = "METHOD")]
6363
pub method: String,
6464

@@ -80,6 +80,17 @@ pub struct ScrapeCommand {
8080
#[arg(long = "header", short = 'H', value_name = "HEADER")]
8181
pub headers: Vec<String>,
8282

83+
/// Cookie to send with the request (format: "name=value").
84+
/// Can be specified multiple times for multiple cookies.
85+
/// Example: --cookie "session=abc123" --cookie "user=john"
86+
#[arg(long = "cookie", value_name = "COOKIE")]
87+
pub cookies: Vec<String>,
88+
89+
/// Disable following HTTP redirects.
90+
/// By default, up to 10 redirects are followed.
91+
#[arg(long)]
92+
pub no_follow_redirects: bool,
93+
8394
/// Strip images from output.
8495
#[arg(long)]
8596
pub no_images: bool,
@@ -227,8 +238,13 @@ impl ScrapeCommand {
227238
// Build HTTP client with redirect policy and cookie store
228239
// Cookie store is enabled to persist cookies across redirects, which is
229240
// required for auth-gated pages that set cookies then redirect.
241+
let redirect_policy = if self.no_follow_redirects {
242+
reqwest::redirect::Policy::none()
243+
} else {
244+
reqwest::redirect::Policy::limited(10)
245+
};
230246
let mut client_builder = create_client_builder()
231-
.redirect(reqwest::redirect::Policy::limited(10))
247+
.redirect(redirect_policy)
232248
.cookie_store(true);
233249

234250
// Override timeout if specified (0 means no timeout)
@@ -249,33 +265,46 @@ impl ScrapeCommand {
249265

250266
// Parse the HTTP method
251267
let method_upper = self.method.to_uppercase();
252-
let use_head = match method_upper.as_str() {
253-
"GET" => false,
254-
"HEAD" => true,
255-
_ => bail!("Unsupported HTTP method: {}. Use GET or HEAD.", self.method),
256-
};
257268

258269
// Add custom headers
259270
let parsed_headers = parse_headers(&self.headers)?;
260271

272+
// Build cookie header from --cookie flags
273+
let cookie_header = if !self.cookies.is_empty() {
274+
Some(self.cookies.join("; "))
275+
} else {
276+
None
277+
};
278+
261279
if self.verbose {
262-
eprintln!(
263-
"Fetching: {} (method: {})",
264-
self.url,
265-
if use_head { "HEAD" } else { "GET" }
266-
);
280+
eprintln!("Fetching: {} (method: {})", self.url, method_upper);
267281
}
268282

269-
// Handle HEAD and GET requests separately for proper type inference
270-
if use_head {
271-
// HEAD request - just return headers
272-
let mut request = client.head(&self.url);
273-
for (name, value) in &parsed_headers {
274-
request = request.header(name.as_str(), value.as_str());
275-
}
283+
// Build request based on HTTP method
284+
let mut request = match method_upper.as_str() {
285+
"GET" => client.get(&self.url),
286+
"HEAD" => client.head(&self.url),
287+
"POST" => client.post(&self.url),
288+
_ => bail!(
289+
"Unsupported HTTP method: {}. Use GET, HEAD, or POST.",
290+
self.method
291+
),
292+
};
276293

277-
let response = request.send().await.context("Failed to fetch URL")?;
294+
// Add custom headers
295+
for (name, value) in &parsed_headers {
296+
request = request.header(name.as_str(), value.as_str());
297+
}
298+
299+
// Add cookies if specified
300+
if let Some(ref cookies) = cookie_header {
301+
request = request.header("Cookie", cookies.as_str());
302+
}
303+
304+
let response = request.send().await.context("Failed to fetch URL")?;
278305

306+
// For HEAD requests, just show headers and return
307+
if method_upper == "HEAD" {
279308
if !response.status().is_success() {
280309
bail!(
281310
"HTTP error: {} {}",
@@ -307,27 +336,7 @@ impl ScrapeCommand {
307336
return Ok(());
308337
}
309338

310-
// GET request - fetch content
311-
let mut request = client.get(&self.url);
312-
for (name, value) in &parsed_headers {
313-
request = request.header(name.as_str(), value.as_str());
314-
}
315-
316-
// Send request with improved timeout error message (Issue #1985)
317-
let timeout_secs = self.timeout;
318-
let response = request.send().await.map_err(|e| {
319-
if e.is_timeout() {
320-
anyhow::anyhow!(
321-
"Request timed out after {} second{}. Use --timeout to increase the timeout.",
322-
timeout_secs,
323-
if timeout_secs == 1 { "" } else { "s" }
324-
)
325-
} else if e.is_connect() {
326-
anyhow::anyhow!("Failed to connect to URL: {}", e)
327-
} else {
328-
anyhow::anyhow!("Failed to fetch URL: {}", e)
329-
}
330-
})?;
339+
// For GET/POST, check response status
331340

332341
if !response.status().is_success() {
333342
bail!(
@@ -1420,9 +1429,12 @@ mod tests {
14201429
url: String::new(),
14211430
output: None,
14221431
format: "markdown".to_string(),
1432+
method: "GET".to_string(),
14231433
timeout: 30,
14241434
user_agent: None,
14251435
headers: vec![],
1436+
cookies: vec![],
1437+
no_follow_redirects: false,
14261438
no_images: false,
14271439
no_links: false,
14281440
selector: None,
@@ -1445,9 +1457,12 @@ mod tests {
14451457
url: " ".to_string(),
14461458
output: None,
14471459
format: "markdown".to_string(),
1460+
method: "GET".to_string(),
14481461
timeout: 30,
14491462
user_agent: None,
14501463
headers: vec![],
1464+
cookies: vec![],
1465+
no_follow_redirects: false,
14511466
no_images: false,
14521467
no_links: false,
14531468
selector: None,
@@ -1472,9 +1487,12 @@ mod tests {
14721487
url: "https://example.com".to_string(),
14731488
output: None,
14741489
format: "markdown".to_string(),
1490+
method: "GET".to_string(),
14751491
timeout: 0,
14761492
user_agent: None,
14771493
headers: vec![],
1494+
cookies: vec![],
1495+
no_follow_redirects: false,
14781496
no_images: false,
14791497
no_links: false,
14801498
selector: None,

0 commit comments

Comments
 (0)