Skip to content

Commit b265e76

Browse files
author
Bounty Bot
committed
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
1 parent 1358370 commit b265e76

13 files changed

Lines changed: 330 additions & 15 deletions

File tree

cortex-agents/src/custom/loader.rs

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ impl CustomAgentLoader {
168168
}
169169

170170
/// Parse YAML frontmatter from content.
171+
///
172+
/// Supports YAML anchors (`&name`), aliases (`*name`), and merge keys (`<<: *name`)
173+
/// which are resolved before returning the parsed value (#2199).
171174
fn parse_frontmatter(content: &str) -> Result<(serde_yaml::Value, String), CustomAgentError> {
172175
let content = content.trim();
173176

@@ -185,9 +188,62 @@ fn parse_frontmatter(content: &str) -> Result<(serde_yaml::Value, String), Custo
185188
let yaml_str = &content[3..end + 3];
186189
let body = content[end + 7..].trim();
187190

191+
// Parse YAML with anchor/alias support
188192
let frontmatter: serde_yaml::Value = serde_yaml::from_str(yaml_str)?;
189193

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

193249
/// Synchronous version of the loader.

cortex-cli/src/import_cmd.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ fn message_to_event(message: &ExportMessage, turn_id: &mut u64, cwd: &PathBuf) -
248248
id: None,
249249
parent_id: None,
250250
message: message.content.clone(),
251+
finish_reason: None,
251252
}),
252253
"tool" => {
253254
// Reconstruct tool result as ExecCommandEnd
@@ -277,6 +278,7 @@ fn message_to_event(message: &ExportMessage, turn_id: &mut u64, cwd: &PathBuf) -
277278
id: None,
278279
parent_id: None,
279280
message: format!("[System] {}", message.content),
281+
finish_reason: None,
280282
})
281283
}
282284
other => {
@@ -285,6 +287,7 @@ fn message_to_event(message: &ExportMessage, turn_id: &mut u64, cwd: &PathBuf) -
285287
id: None,
286288
parent_id: None,
287289
message: format!("[{other}] {}", message.content),
290+
finish_reason: None,
288291
})
289292
}
290293
};

cortex-cli/src/main.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,12 @@ struct ResumeCommand {
379379
#[arg(long = "all", default_value_t = false)]
380380
all: bool,
381381

382+
/// Do not persist session changes (incompatible with resume, will error).
383+
/// This flag is invalid for `resume` command since resuming inherently
384+
/// requires session persistence (#2197).
385+
#[arg(long = "no-session", default_value_t = false)]
386+
no_session: bool,
387+
382388
#[clap(flatten)]
383389
config_overrides: CliConfigOverrides,
384390
}
@@ -745,6 +751,15 @@ fn generate_completions(shell: Shell) {
745751
async fn run_resume(resume_cli: ResumeCommand) -> Result<()> {
746752
use cortex_protocol::ConversationId;
747753

754+
// Validate that --no-session is not used with resume (#2197)
755+
if resume_cli.no_session {
756+
anyhow::bail!(
757+
"The --no-session flag is incompatible with the 'resume' command. \
758+
Resuming a session inherently requires session persistence. \
759+
If you want to start a new session without persistence, use 'cortex run --no-session' instead."
760+
);
761+
}
762+
748763
let config = cortex_engine::Config::default();
749764

750765
let id_str = match (resume_cli.session_id, resume_cli.last) {

cortex-cli/src/run_cmd.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,20 +245,32 @@ struct FileAttachment {
245245
impl RunCli {
246246
/// Run the command.
247247
pub async fn run(self) -> Result<()> {
248-
// Validate temperature if provided
248+
// Validate temperature if provided using epsilon-based comparison
249+
// to handle floating-point boundary issues (#2179)
250+
const EPSILON: f32 = 1e-6;
249251
if let Some(temp) = self.temperature {
250-
if !(0.0..=2.0).contains(&temp) {
252+
if temp < -EPSILON || temp > 2.0 + EPSILON {
251253
bail!("Temperature must be between 0.0 and 2.0, got {temp}");
252254
}
253255
}
254256

255-
// Validate top_p if provided
257+
// Validate top_p if provided using epsilon-based comparison
256258
if let Some(top_p) = self.top_p {
257-
if !(0.0..=1.0).contains(&top_p) {
259+
if top_p < -EPSILON || top_p > 1.0 + EPSILON {
258260
bail!("top-p must be between 0.0 and 1.0, got {top_p}");
259261
}
260262
}
261263

264+
// Warn if both temperature and top-p are specified (#2175)
265+
if self.temperature.is_some() && self.top_p.is_some() {
266+
eprintln!(
267+
"{}Warning:{} Using both --temperature and --top-p together may produce unpredictable results. \
268+
Most LLM providers recommend using only one sampling parameter at a time.",
269+
TermColor::Yellow.ansi_code(),
270+
TermColor::Default.ansi_code()
271+
);
272+
}
273+
262274
// Validate command is not empty if provided
263275
if let Some(ref cmd) = self.command {
264276
if cmd.trim().is_empty() {
@@ -612,6 +624,7 @@ impl RunCli {
612624
let mut streaming_started = false;
613625
let mut error_occurred = false;
614626
let mut task_completed = false;
627+
let mut response_truncated = false; // Track if response was truncated (#2174)
615628

616629
// Set up timeout if specified
617630
let timeout_duration = if self.timeout > 0 {
@@ -651,6 +664,19 @@ impl RunCli {
651664
match &event.msg {
652665
EventMsg::AgentMessage(msg) => {
653666
final_message = msg.message.clone();
667+
// Check if response was truncated due to token limit (#2174)
668+
if let Some(ref reason) = msg.finish_reason {
669+
if reason == "length" {
670+
response_truncated = true;
671+
if !is_json {
672+
eprintln!(
673+
"{}Warning:{} Response was truncated due to max token limit.",
674+
TermColor::Yellow.ansi_code(),
675+
TermColor::Default.ansi_code()
676+
);
677+
}
678+
}
679+
}
654680
}
655681
EventMsg::AgentMessageDelta(delta) => {
656682
// Handle streaming output
@@ -770,6 +796,8 @@ impl RunCli {
770796
"message": final_message,
771797
"events": event_count,
772798
"success": !error_occurred,
799+
"truncated": response_truncated,
800+
"finish_reason": if response_truncated { "length" } else if error_occurred { "error" } else { "stop" },
773801
});
774802
println!("{}", serde_json::to_string_pretty(&result)?);
775803
}
@@ -812,9 +840,14 @@ impl RunCli {
812840
drop(handle);
813841
let _ = session_task.await;
814842

843+
// Exit with appropriate code (#2174)
815844
if error_occurred {
816845
std::process::exit(1);
817846
}
847+
if response_truncated {
848+
// Exit with code 2 to indicate truncation (distinct from error code 1)
849+
std::process::exit(2);
850+
}
818851

819852
Ok(())
820853
}

cortex-common/src/http_client.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
//! - `create_client_with_timeout(duration)` - Custom timeout
77
//!
88
//! All clients include: User-Agent, tcp_nodelay, and proper error handling.
9+
//!
10+
//! DNS caching is configured with reasonable TTL to allow failover and load
11+
//! balancer updates (#2177).
912
1013
use reqwest::Client;
1114
use std::time::Duration;
@@ -22,6 +25,11 @@ pub const STREAMING_TIMEOUT: Duration = Duration::from_secs(300);
2225
/// Short timeout for health checks (5 seconds)
2326
pub const HEALTH_CHECK_TIMEOUT: Duration = Duration::from_secs(5);
2427

28+
/// Connection pool idle timeout to ensure DNS is re-resolved periodically.
29+
/// This helps with failover scenarios where DNS records change (#2177).
30+
/// Set to 60 seconds to balance between performance and DNS freshness.
31+
pub const POOL_IDLE_TIMEOUT: Duration = Duration::from_secs(60);
32+
2533
/// Creates an HTTP client with default configuration (30s timeout).
2634
///
2735
/// Includes: User-Agent, tcp_nodelay, 30s timeout.
@@ -58,12 +66,17 @@ pub fn create_health_check_client() -> Result<Client, String> {
5866
/// All clients include:
5967
/// - User-Agent: `cortex-cli/{version}`
6068
/// - tcp_nodelay: true (for lower latency)
69+
/// - pool_idle_timeout: 60s (for DNS TTL respect, #2177)
6170
/// - Specified timeout
6271
pub fn create_client_with_timeout(timeout: Duration) -> Result<Client, String> {
6372
Client::builder()
6473
.user_agent(USER_AGENT)
6574
.timeout(timeout)
6675
.tcp_nodelay(true)
76+
// Ensure connections are closed periodically to allow DNS re-resolution
77+
// This prevents stale IP addresses from being used after DNS changes (#2177)
78+
.pool_idle_timeout(POOL_IDLE_TIMEOUT)
79+
.pool_max_idle_per_host(4)
6780
.build()
6881
.map_err(|e| format!("Failed to build HTTP client: {e}"))
6982
}
@@ -84,6 +97,9 @@ pub fn create_client_builder() -> reqwest::ClientBuilder {
8497
.user_agent(USER_AGENT)
8598
.timeout(DEFAULT_TIMEOUT)
8699
.tcp_nodelay(true)
100+
// Ensure connections are closed periodically to allow DNS re-resolution (#2177)
101+
.pool_idle_timeout(POOL_IDLE_TIMEOUT)
102+
.pool_max_idle_per_host(4)
87103
}
88104

89105
/// Creates a blocking HTTP client with default configuration.

cortex-core/src/widgets/input.rs

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -237,33 +237,47 @@ impl<'a> CortexInput<'a> {
237237
///
238238
/// If not currently browsing history, starts from the most recent item.
239239
/// If already at the oldest item, stays there.
240+
///
241+
/// Note: When running in terminal multiplexers (tmux/screen) with alternate
242+
/// screen mode, arrow keys may sometimes be misinterpreted. Use Ctrl+P as
243+
/// an alternative, or configure your multiplexer's escape-time setting (#2201).
240244
pub fn history_prev(&mut self) {
241245
if self.history.is_empty() {
242246
return;
243247
}
244248

249+
// Calculate new index with bounds checking to prevent wrapping issues (#2201)
245250
let new_index = match self.history_index {
246251
None => {
247252
// Start browsing from most recent
248-
self.history.len() - 1
253+
self.history.len().saturating_sub(1)
249254
}
250255
Some(0) => {
251-
// Already at oldest, stay there
256+
// Already at oldest, stay there (don't wrap)
252257
0
253258
}
254259
Some(idx) => {
255-
// Move to older item
256-
idx - 1
260+
// Move to older item, ensuring we don't underflow
261+
idx.saturating_sub(1)
257262
}
258263
};
259264

260-
self.history_index = Some(new_index);
261-
self.set_text(&self.history[new_index].clone());
265+
// Only update if the index actually changed or we're starting navigation
266+
if self.history_index != Some(new_index) {
267+
self.history_index = Some(new_index);
268+
if let Some(text) = self.history.get(new_index) {
269+
self.set_text(&text.clone());
270+
}
271+
}
262272
}
263273

264274
/// Navigate to the next history item (Down key).
265275
///
266276
/// If at the most recent item, clears the input and exits history browsing.
277+
///
278+
/// Note: When running in terminal multiplexers (tmux/screen) with alternate
279+
/// screen mode, arrow keys may sometimes be misinterpreted. Use Ctrl+N as
280+
/// an alternative, or configure your multiplexer's escape-time setting (#2201).
267281
pub fn history_next(&mut self) {
268282
if self.history.is_empty() {
269283
return;
@@ -273,15 +287,18 @@ impl<'a> CortexInput<'a> {
273287
None => {
274288
// Not browsing history, do nothing
275289
}
276-
Some(idx) if idx >= self.history.len() - 1 => {
290+
Some(idx) if idx >= self.history.len().saturating_sub(1) => {
277291
// At most recent, exit history browsing and clear
278292
self.history_index = None;
279293
self.clear();
280294
}
281295
Some(idx) => {
282-
// Move to more recent item
283-
self.history_index = Some(idx + 1);
284-
self.set_text(&self.history[idx + 1].clone());
296+
// Move to more recent item with bounds checking (#2201)
297+
let new_idx = (idx + 1).min(self.history.len().saturating_sub(1));
298+
self.history_index = Some(new_idx);
299+
if let Some(text) = self.history.get(new_idx) {
300+
self.set_text(&text.clone());
301+
}
285302
}
286303
}
287304
}

cortex-engine/src/agent/core.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ impl CortexAgent {
288288
id: None,
289289
parent_id: None,
290290
message: response.clone(),
291+
finish_reason: None,
291292
}))
292293
.await;
293294

cortex-engine/src/session.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,7 @@ impl Session {
817817
id: None,
818818
parent_id: None,
819819
message: full_content.clone(),
820+
finish_reason: None,
820821
}))
821822
.await;
822823
}
@@ -1204,6 +1205,7 @@ impl Session {
12041205
id: None,
12051206
parent_id: None,
12061207
message: text.to_string(),
1208+
finish_reason: None,
12071209
}))
12081210
.await;
12091211
}
@@ -1568,6 +1570,7 @@ impl Session {
15681570
id: None,
15691571
parent_id: None,
15701572
message: text.to_string(),
1573+
finish_reason: None,
15711574
}),
15721575
})?;
15731576
}

0 commit comments

Comments
 (0)