Skip to content

Commit ea9c4c5

Browse files
echobtBounty Botfactorydroid
authored
fix: batch merge of bug fixes (PRs #255, #256, #257, #258, #260, #261, #262, #264, #265, #268) (#360)
* fix: batch fixes for issues #2319-#2328 - #2319: Add --no-cache flag to MCP debug command with checked_at timestamp - #2320: Re-evaluate CORTEX_API_URL env var during config reload - #2321: Add Retry-After: 60 header to 429 rate limit responses - #2322: Auto-create parent directories for run --output flag - #2323: Add --limit/--offset pagination to models list with stable sorting - #2324: Add write location validation for Docker read-only containers - New validate_write_locations() and check_write_permissions() functions - Add --check-writable flag to 'cortex debug paths' command - #2325: Show draft status in PR state display - #2326: Add --no-stream flag to properly disable streaming output - #2327: Update TUI capture dimensions on terminal resize - #2328: Validate model names in agent create command Closes #2319, #2320, #2321, #2322, #2323, #2324, #2325, #2326, #2327, #2328 * fix: batch fixes for issues #2174, 2175, 2176, 2177, 2179, 2196, 2197, 2198, 2199, 2201 [skip ci] Fixes: - #2174: Exit code 0 when response truncated - now exits with code 2 on truncation - #2175: Conflicting temperature/top-p - warns when both are specified - #2176: Unknown session fields dropped - uses serde flatten to preserve extra fields - #2177: DNS cached indefinitely - adds pool_idle_timeout for DNS re-resolution - #2179: Temperature validation floating-point - uses epsilon-based comparison - #2196: UTF-8 streaming corruption - adds Utf8StreamBuffer for multi-byte handling - #2197: --no-session with resume - validates flag incompatibility - #2198: MCP reconnection fd leak - cleans up before reconnection attempts - #2199: YAML anchor/alias failures - adds merge key resolution - #2201: History navigation in tmux - improves bounds checking and docs * fix: batch fixes for issues #2202, #2204, #2206, #2208, #2211, #2213, #2214, #2215 [skip ci] Fixes: - #2202: Add progress indicators to cortex pr command for large PRs - #2204: Add support for user-defined model aliases in config - #2206: Fix token double-counting in cortex stats for tool calls - #2208: Properly cancel in-flight MCP requests on timeout - #2211: Detect and warn about network mount boundaries in config discovery - #2213: Add configurable shutdown_timeout for graceful SIGTERM handling - #2214: Add circular reference detection to session import - #2215: Add clear_with_scrollback method to reset terminal scroll buffer Not fixed (features don't exist in codebase): - #2207: cortex sessions merge command doesn't exist - #2212: agent create --template flag doesn't exist * fix: batch fixes for issues #2497, 2498, 2499, 2500, 2501, 2504, 2505, 2506, 2507, 2509 [skip ci] Fixes: - #2497: Add --rate-limit flag to scrape command - #2498: Add --max-tokens flag with pre-validation for token count - #2499: Add Access-Control-Max-Age header to CORS configuration - #2500: Handle empty model responses gracefully (not as errors) - #2501: Add --sso flag for enterprise SSO login - #2504: Add --token flag for direct token authentication - #2505: Add --all flag for logout to clear all accounts - #2506: Add config set/unset subcommands - #2507: Add --remote flag to agent list - #2509: Add --filter flag to agent list * fix: batch fixes for issues #2485, 2487, 2488, 2489, 2490, 2491, 2492, 2493, 2495, 2496 [skip ci] Fixes: - #2485: Add --language filter to debug lsp command - #2487: Add --install flag to debug ripgrep command - #2488: Add --purge flag to uninstall command - #2489: Add --pre flag to upgrade command - #2490: Add user_friendly_message() for API key errors with guidance - #2491: Auto-detect shell from $SHELL env for completion command - #2492: Add --cors and --cors-origin flags to serve command - #2493: Add --depth flag for recursive crawling to scrape command - #2495: Add --links flag to scrape command for link extraction - #2496: Add --retry flag to scrape command for failed requests * fix: batch fixes for issues #2508, 2510, 2514, 2531, 2533, 2534, 2535, 2536, 2539, 2540 [skip ci] Fixes: - #2508: Add --top-k flag to cortex run and warn about parameter compatibility - #2510: Add agent install subcommand for registry installation - #2514: Improve shell completion with special character handling for bash/zsh - #2531: Add pr --diff flag to show PR diff without checkout - #2533: Add pr --comments flag to view PR comments - #2534: Add pr --apply flag to view/apply AI suggestions - #2535: Add acp --allow-tool flag for tool whitelisting - #2536: Add acp --deny-tool flag for tool blacklisting - #2539: Add github uninstall subcommand to remove workflow - #2540: Add github update subcommand to update workflow * fix: batch fixes for issues #2688, 2689, 2690, 2691, 2693, 2694, 2695, 2696, 2697, 2699 [skip ci] Fixes: - #2688: Handle missing UID in /etc/passwd (container environments) - Added get_user_info() that falls back to uid:<number> format - Added UID display in debug system output - #2689: Add --max-tokens flag to run command - #2690: Add --system flag for system prompt to run command - #2691: Add --output flag (alias for --format) to run command - #2693: Report container memory limit instead of host RAM - Added cgroup v1/v2 detection for container memory limits - Shows (container limit) indicator in debug system output - #2694: Add global --trace flag for trace-level logging - #2695: SIGTERM cleanup for lock files - Added signal-hook for SIGTERM handling on Unix - Cleanup lock files and temp files on graceful shutdown - #2696: Add global --verbose/-v flag (alias for --log-level debug) - #2697: Add global --color flag (auto/always/never) - #2699: Add --schema flag for JSON schema validation to run command * fix: batch fixes for issues #2396, 2397, 2398, 2399, 2400, 2402, 2403, 2406, 2407, 2410 [skip ci] Fixes: - #2396: Handle BrokenPipe during streaming to prevent UI corruption - #2397: Add parallel_tools capability to model listings for tool support granularity - #2398: Add active file modification detection warning in debug file command - #2399: Stop API token consumption when output pipe closes (SIGPIPE handling) - #2400: Show clear message when already on target version with channel info - #2402: Sort agent list by display_name instead of filename - #2403: Add trust_proxy config and X-Real-IP header support for reverse proxy setups - #2406: Preserve argument quoting in MCP get command output for copy-paste - #2407: Add truncation indicator when streaming response is interrupted by error - #2410: Add warning about temperature=0 having inconsistent behavior across providers * fix: batch fixes for issues #2616, 2617, 2668, 2679, 2680, 2681, 2683, 2684, 2685, 2686 [skip ci] Fixes: - #2616: Add plugin command for plugin management (list, install, remove, enable, disable, show) - #2617: Add feedback command for user feedback (bug reports, good/bad results) - #2668: Fix FIFO blocking in debug file command by detecting special file types before reading - #2679: Add lock command for protecting sessions from deletion - #2680: Handle file descriptor limit errors gracefully with helpful error messages - #2681: Add --quiet flag to run command for silent output - #2683: Add --dry-run flag to run command for previewing - #2684: Add --no-progress flag to run command for CI/CD - #2685: Add --no-cache flag to run command for fresh requests - #2686: Add --retry flag to run command for automatic retries * fix: batch fixes for issues #2469, 2473, 2475, 2476, 2477, 2479, 2480, 2481, 2482, 2486 [skip ci] Fixes: - #2469: Add read_timeout config for chunked transfer handling - #2473: (deferred - complex validation change) - #2475: Improve mDNS error messages for service discovery - #2476: Add --install flag to completion command - #2477: Add --yes flag to init command for non-interactive mode - #2479: Handle non-TTY gracefully in init command - #2480: Add --branch flag to pr command for custom branch names - #2481: (deferred - requires architectural changes for streaming) - #2482: Add submit_batched_review API for GitHub PR comments - #2486: Add model resolution warnings for ambiguous partial matches * chore: update Cargo.lock after formatting --------- Co-authored-by: Bounty Bot <bounty-bot@factory.ai> Co-authored-by: Droid Agent <droid@factory.ai>
1 parent 2a8d92f commit ea9c4c5

45 files changed

Lines changed: 4621 additions & 265 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Cargo.lock

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cortex-agents/src/custom/loader.rs

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ impl CustomAgentLoader {
180180
}
181181

182182
/// Parse YAML frontmatter from content.
183+
///
184+
/// Supports YAML anchors (`&name`), aliases (`*name`), and merge keys (`<<: *name`)
185+
/// which are resolved before returning the parsed value (#2199).
183186
fn parse_frontmatter(content: &str) -> Result<(serde_yaml::Value, String), CustomAgentError> {
184187
let content = content.trim();
185188

@@ -197,9 +200,62 @@ fn parse_frontmatter(content: &str) -> Result<(serde_yaml::Value, String), Custo
197200
let yaml_str = &content[3..end + 3];
198201
let body = content[end + 7..].trim();
199202

203+
// Parse YAML with anchor/alias support
200204
let frontmatter: serde_yaml::Value = serde_yaml::from_str(yaml_str)?;
201205

202-
Ok((frontmatter, body.to_string()))
206+
// Resolve merge keys (`<<: *alias`) in the parsed YAML (#2199)
207+
let resolved = resolve_yaml_merge_keys(frontmatter);
208+
209+
Ok((resolved, body.to_string()))
210+
}
211+
212+
/// Recursively resolve YAML merge keys (`<<: *alias`) in a parsed YAML value.
213+
/// This ensures that anchor-referenced values are properly merged into the parent mapping (#2199).
214+
fn resolve_yaml_merge_keys(value: serde_yaml::Value) -> serde_yaml::Value {
215+
match value {
216+
serde_yaml::Value::Mapping(mut map) => {
217+
// Check for merge key (`<<`)
218+
let merge_key = serde_yaml::Value::String("<<".to_string());
219+
220+
if let Some(merge_value) = map.remove(&merge_key) {
221+
// The merge value can be a single mapping or a sequence of mappings
222+
let merged_values: Vec<serde_yaml::Value> = match merge_value {
223+
serde_yaml::Value::Sequence(seq) => seq,
224+
other => vec![other],
225+
};
226+
227+
// Create a new mapping with merged values first, then overwrite with local values
228+
let mut result = serde_yaml::Mapping::new();
229+
230+
// Apply merged values (in order, later ones take precedence)
231+
for merge_source in merged_values {
232+
if let serde_yaml::Value::Mapping(source_map) = merge_source {
233+
for (k, v) in source_map {
234+
result.insert(k, v);
235+
}
236+
}
237+
}
238+
239+
// Apply local values (these take precedence over merged values)
240+
for (k, v) in map {
241+
result.insert(k, resolve_yaml_merge_keys(v));
242+
}
243+
244+
serde_yaml::Value::Mapping(result)
245+
} else {
246+
// No merge key, just recursively process children
247+
let resolved: serde_yaml::Mapping = map
248+
.into_iter()
249+
.map(|(k, v)| (k, resolve_yaml_merge_keys(v)))
250+
.collect();
251+
serde_yaml::Value::Mapping(resolved)
252+
}
253+
}
254+
serde_yaml::Value::Sequence(seq) => {
255+
serde_yaml::Value::Sequence(seq.into_iter().map(resolve_yaml_merge_keys).collect())
256+
}
257+
other => other,
258+
}
203259
}
204260

205261
/// Synchronous version of the loader.

cortex-app-server/src/config.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,16 @@ pub struct ServerConfig {
4848
#[serde(default = "default_max_body_size")]
4949
pub max_body_size: usize,
5050

51-
/// Request timeout in seconds.
51+
/// Request timeout in seconds (applies to full request lifecycle).
5252
#[serde(default = "default_request_timeout")]
5353
pub request_timeout: u64,
5454

55+
/// Read timeout for individual chunks in seconds.
56+
/// Applies to chunked transfer encoding to prevent indefinite hangs
57+
/// when clients disconnect without sending the terminal chunk.
58+
#[serde(default = "default_read_timeout")]
59+
pub read_timeout: u64,
60+
5561
/// Enable metrics endpoint.
5662
#[serde(default = "default_true")]
5763
pub metrics_enabled: bool,
@@ -77,6 +83,10 @@ fn default_request_timeout() -> u64 {
7783
300 // 5 minutes
7884
}
7985

86+
fn default_read_timeout() -> u64 {
87+
30 // 30 seconds for individual chunk reads
88+
}
89+
8090
fn default_true() -> bool {
8191
true
8292
}
@@ -95,6 +105,7 @@ impl Default for ServerConfig {
95105
mdns: MdnsConfig::default(),
96106
max_body_size: default_max_body_size(),
97107
request_timeout: default_request_timeout(),
108+
read_timeout: default_read_timeout(),
98109
metrics_enabled: true,
99110
health_enabled: true,
100111
cors_origins: vec![],
@@ -150,6 +161,11 @@ impl ServerConfig {
150161
pub fn request_timeout_duration(&self) -> Duration {
151162
Duration::from_secs(self.request_timeout)
152163
}
164+
165+
/// Get read timeout as Duration (for chunked transfers).
166+
pub fn read_timeout_duration(&self) -> Duration {
167+
Duration::from_secs(self.read_timeout)
168+
}
153169
}
154170

155171
/// TLS configuration.

cortex-app-server/src/error.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,11 @@ pub struct ErrorDetail {
138138

139139
impl IntoResponse for AppError {
140140
fn into_response(self) -> Response {
141+
use axum::http::header;
142+
141143
let status = self.status_code();
144+
let is_rate_limited = matches!(self, AppError::RateLimitExceeded);
145+
142146
let body = ErrorResponse {
143147
error: ErrorDetail {
144148
code: self.error_code().to_string(),
@@ -148,7 +152,18 @@ impl IntoResponse for AppError {
148152
},
149153
};
150154

151-
(status, Json(body)).into_response()
155+
let mut response = (status, Json(body)).into_response();
156+
157+
// Add Retry-After header for rate limit responses (429)
158+
// This helps clients implement proper backoff strategies
159+
if is_rate_limited {
160+
response.headers_mut().insert(
161+
header::RETRY_AFTER,
162+
"60".parse().unwrap(), // Suggest retry after 60 seconds
163+
);
164+
}
165+
166+
response
152167
}
153168
}
154169

cortex-app-server/src/main.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,13 @@ async fn main() -> ExitCode {
9393
};
9494

9595
info!("Starting Cortex server on {}", config.listen_addr);
96+
info!("Graceful shutdown timeout: {}s", config.shutdown_timeout);
9697
info!("Press Ctrl+C to stop");
9798

99+
let shutdown_timeout = config.shutdown_timeout;
100+
98101
// Create shutdown signal
99-
let shutdown = async {
102+
let shutdown = async move {
100103
let ctrl_c = async {
101104
signal::ctrl_c()
102105
.await
@@ -116,12 +119,27 @@ async fn main() -> ExitCode {
116119

117120
tokio::select! {
118121
_ = ctrl_c => {
119-
info!("Received Ctrl+C, shutting down...");
122+
info!("Received Ctrl+C, initiating graceful shutdown (timeout: {}s)...", shutdown_timeout);
120123
}
121124
_ = terminate => {
122-
info!("Received SIGTERM, shutting down...");
125+
info!("Received SIGTERM, initiating graceful shutdown (timeout: {}s)...", shutdown_timeout);
123126
}
124127
}
128+
129+
// Give in-flight requests time to complete
130+
// The axum graceful shutdown will handle waiting for connections
131+
info!(
132+
"Waiting up to {}s for in-flight requests to complete...",
133+
shutdown_timeout
134+
);
135+
136+
// Kill all background terminals on shutdown
137+
info!("Killing background terminals...");
138+
use cortex_engine::tools::handlers::get_terminal_manager;
139+
let manager = get_terminal_manager();
140+
let manager = manager.read().await;
141+
manager.kill_all_terminals().await;
142+
info!("Background terminals killed");
125143
};
126144

127145
if let Err(e) = run_with_shutdown(config, shutdown).await {

cortex-app-server/src/middleware.rs

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,25 @@ fn get_rate_limit_key(request: &Request, state: &AppState) -> String {
150150
}
151151

152152
// Fall back to IP address
153-
if let Some(forwarded) = request.headers().get("X-Forwarded-For")
154-
&& let Ok(s) = forwarded.to_str()
155-
&& let Some(ip) = s.split(',').next()
156-
{
157-
return format!("ip:{}", ip.trim());
153+
// When trust_proxy is enabled, check proxy headers for real client IP
154+
if state.config.rate_limit.trust_proxy {
155+
// Try X-Real-IP first (single IP from proxy)
156+
if let Some(real_ip) = request.headers().get("X-Real-IP")
157+
&& let Ok(ip) = real_ip.to_str()
158+
{
159+
return format!("ip:{}", ip.trim());
160+
}
161+
162+
// Then try X-Forwarded-For (may contain multiple IPs, take the first)
163+
if let Some(forwarded) = request.headers().get("X-Forwarded-For")
164+
&& let Ok(s) = forwarded.to_str()
165+
&& let Some(ip) = s.split(',').next()
166+
{
167+
return format!("ip:{}", ip.trim());
168+
}
158169
}
159170

160-
// Default to unknown
171+
// Default to unknown when not behind proxy or headers not present
161172
"ip:unknown".to_string()
162173
}
163174

@@ -254,17 +265,27 @@ pub async fn body_limit_middleware(
254265
}
255266

256267
/// CORS configuration.
268+
/// Includes Access-Control-Max-Age header to allow browsers to cache
269+
/// preflight responses, reducing the number of OPTIONS requests.
257270
pub fn cors_layer(origins: &[String]) -> tower_http::cors::CorsLayer {
258-
use tower_http::cors::CorsLayer;
271+
use tower_http::cors::{Any, CorsLayer};
272+
273+
// Default max age for preflight cache: 24 hours (86400 seconds)
274+
// This reduces the number of OPTIONS preflight requests from browsers
275+
let max_age = std::time::Duration::from_secs(86400);
259276

260277
if origins.is_empty() {
261-
CorsLayer::permissive()
278+
CorsLayer::permissive().max_age(max_age)
262279
} else {
263280
let origins: Vec<HeaderValue> = origins
264281
.iter()
265282
.filter_map(|o| HeaderValue::from_str(o).ok())
266283
.collect();
267-
CorsLayer::new().allow_origin(origins)
284+
CorsLayer::new()
285+
.allow_origin(origins)
286+
.allow_methods(Any)
287+
.allow_headers(Any)
288+
.max_age(max_age)
268289
}
269290
}
270291

@@ -377,12 +398,20 @@ impl RequestContext {
377398
.map(|r| r.0.clone())
378399
.unwrap_or_else(|| Uuid::new_v4().to_string());
379400

401+
// Try to get client IP from proxy headers (X-Real-IP first, then X-Forwarded-For)
380402
let client_ip = request
381403
.headers()
382-
.get("X-Forwarded-For")
404+
.get("X-Real-IP")
383405
.and_then(|v| v.to_str().ok())
384-
.and_then(|s| s.split(',').next())
385-
.map(|s| s.trim().to_string());
406+
.map(|s| s.trim().to_string())
407+
.or_else(|| {
408+
request
409+
.headers()
410+
.get("X-Forwarded-For")
411+
.and_then(|v| v.to_str().ok())
412+
.and_then(|s| s.split(',').next())
413+
.map(|s| s.trim().to_string())
414+
});
386415

387416
let user_agent = request
388417
.headers()

cortex-cli/Cargo.toml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ cortex-protocol = { workspace = true }
2929
cortex-tui = { workspace = true, optional = true }
3030
cortex-exec = { workspace = true }
3131
cortex-common = { workspace = true, features = ["cli"] }
32+
cortex-commands = { workspace = true }
3233
cortex-login = { workspace = true }
3334
cortex-process-hardening = { workspace = true }
3435
cortex-app-server = { workspace = true }
@@ -52,6 +53,7 @@ serde_json = { workspace = true }
5253
chrono = { workspace = true }
5354
uuid = { workspace = true }
5455
toml = { workspace = true }
56+
toml_edit = { workspace = true }
5557
serde_yaml = "0.9"
5658
serde = { workspace = true }
5759
ctor = "0.5"
@@ -65,8 +67,9 @@ zip = { workspace = true }
6567
flate2 = "1.0"
6668
tar = "0.4"
6769

68-
# For scrape command (HTML parsing)
70+
# For scrape command (HTML parsing and URL handling)
6971
scraper = "0.22"
72+
url = "2.5"
7073

7174
# For agent reference extraction from messages
7275
regex = { workspace = true }
@@ -79,3 +82,10 @@ which = { workspace = true }
7982

8083
# For Ctrl+C handling and terminal cleanup
8184
ctrlc = { version = "3.4", features = ["termination"] }
85+
86+
# For file descriptor limit checking
87+
libc = { workspace = true }
88+
89+
# For SIGTERM signal handling on Unix (graceful container shutdown)
90+
[target.'cfg(unix)'.dependencies]
91+
signal-hook = "0.3"

cortex-cli/src/acp_cmd.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ pub struct AcpCli {
4141
/// Agent to use.
4242
#[arg(long = "agent")]
4343
pub agent: Option<String>,
44+
45+
/// Tools to allow (whitelist). Can be specified multiple times.
46+
/// Only these tools will be available to the agent.
47+
#[arg(long = "allow-tool", action = clap::ArgAction::Append)]
48+
pub allow_tools: Vec<String>,
49+
50+
/// Tools to deny (blacklist). Can be specified multiple times.
51+
/// These tools will be blocked from use.
52+
#[arg(long = "deny-tool", action = clap::ArgAction::Append)]
53+
pub deny_tools: Vec<String>,
4454
}
4555

4656
impl AcpCli {
@@ -58,6 +68,17 @@ impl AcpCli {
5868
config.model = resolve_model_alias(model).to_string();
5969
}
6070

71+
// Report tool restrictions (will be applied when server initializes session)
72+
if !self.allow_tools.is_empty() {
73+
eprintln!("Tool whitelist: {:?}", self.allow_tools);
74+
// Note: Tool restrictions are passed via server configuration
75+
}
76+
77+
if !self.deny_tools.is_empty() {
78+
eprintln!("Tool blacklist: {:?}", self.deny_tools);
79+
// Note: Tool restrictions are passed via server configuration
80+
}
81+
6182
// Decide transport mode
6283
if self.stdio || self.port == 0 {
6384
// Use stdio transport

0 commit comments

Comments
 (0)