Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src-tauri/src/acp/session_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1891,7 +1891,14 @@ pub(crate) fn background_keepalive_max_age() -> chrono::Duration {
std::env::var("CODEG_ACP_BACKGROUND_KEEPALIVE_MAX_SECS")
.ok()
.and_then(|v| v.trim().parse::<i64>().ok())
.filter(|v| *v >= 0)
// `chrono::Duration::seconds` below is an `expect` over
// `try_seconds`, so it PANICS past `i64::MAX / 1000`. This value is
// read on the 60-second idle sweep and on every background-watch
// tick, so an out-of-range env value would abort the process from a
// timer with nobody at the keyboard, and a panic there leaves no
// trace of which setting caused it. Out of range is invalid input
// like any other, so it takes the documented default.
.filter(|v| *v >= 0 && chrono::Duration::try_seconds(*v).is_some())
.unwrap_or(3600)
});
chrono::Duration::seconds(secs)
Expand Down
103 changes: 93 additions & 10 deletions src-tauri/src/chat_channel/backends/telegram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,22 +632,48 @@ fn redact_token(msg: String, token: &str) -> String {
}
}

/// Byte offset of the first case-insensitive `at_bot` in `text`, or `None`.
///
/// Searches `text` itself, so the offset it returns is a real byte offset into
/// `text`. Searching a `to_lowercase()` copy instead is what this used to do,
/// and it is unsound: lowercasing is not length preserving, so one character
/// before the mention silently shifts every later offset. `İ` (U+0130, two
/// bytes) lowercases to two code points (three bytes), the Kelvin sign `K`
/// (three bytes) lowercases to `k` (one byte), and `ẞ` (three bytes) lowercases
/// to `ß` (two). Slicing `text` at an offset found that way then lands in the
/// middle of a character, or past the end, and panics.
///
/// `eq_ignore_ascii_case` is the whole comparison because a Telegram bot
/// username is ASCII by definition (letters, digits and underscores, ending in
/// `bot`), so there is no non-ASCII folding to do on the needle.
///
/// `str::get` rather than indexing: it answers `None` for an end offset that is
/// out of range or not on a character boundary, which is exactly the case that
/// used to panic.
fn find_bot_mention(text: &str, at_bot: &str) -> Option<usize> {
text.char_indices().map(|(i, _)| i).find(|&i| {
text.get(i..i + at_bot.len())
.is_some_and(|candidate| candidate.eq_ignore_ascii_case(at_bot))
})
}

/// Strip `@bot_username` from text (case-insensitive).
/// Handles Telegram convention: `/command@botname args` → `/command args`
fn strip_bot_mention(text: &str, bot_username: &str) -> String {
if bot_username.is_empty() {
return text.to_string();
}
let at_bot = format!("@{}", bot_username);
let text_lower = text.to_lowercase();
let at_bot_lower = at_bot.to_lowercase();
if let Some(pos) = text_lower.find(&at_bot_lower) {
let mut result = String::with_capacity(text.len());
result.push_str(&text[..pos]);
result.push_str(&text[pos + at_bot.len()..]);
result.trim().to_string()
} else {
text.to_string()
match find_bot_mention(text, &at_bot) {
// `find_bot_mention` only reports an offset whose end is a character
// boundary, so both slices below are in range and aligned.
Some(pos) => {
let mut result = String::with_capacity(text.len());
result.push_str(&text[..pos]);
result.push_str(&text[pos + at_bot.len()..]);
result.trim().to_string()
}
None => text.to_string(),
}
}

Expand Down Expand Up @@ -728,7 +754,10 @@ fn telegram_should_process_text_message(
}

let at_bot = format!("@{}", bot_username);
text.to_lowercase().contains(&at_bot.to_lowercase())
// Same matcher `strip_bot_mention` uses, so a mention this accepts is one
// that will actually be stripped, and neither allocates a lowercased copy
// of every group message on the long-poll path.
find_bot_mention(text, &at_bot).is_some()
}

fn json_scalar_to_string(value: &serde_json::Value) -> Option<String> {
Expand Down Expand Up @@ -966,6 +995,60 @@ mod tests {
));
}

#[test]
fn strip_bot_mention_removes_the_mention_in_either_case() {
assert_eq!(
strip_bot_mention("/task@codeg_bot build", "codeg_bot"),
"/task build"
);
assert_eq!(
strip_bot_mention("/task@CodeG_Bot build", "codeg_bot"),
"/task build"
);
assert_eq!(strip_bot_mention("/task build", "codeg_bot"), "/task build");
assert_eq!(strip_bot_mention("/task@codeg_bot", ""), "/task@codeg_bot");
}

/// The long-poll loop runs unattended, and any member of a bound group
/// chooses the text it parses. Finding the mention in a `to_lowercase()`
/// copy and then slicing the original panicked on every case below, because
/// lowercasing is not length preserving: `İ` grows by a byte, `K` shrinks
/// by two, `ẞ` shrinks by one. The offsets then land mid character or past
/// the end of the string.
#[test]
fn strip_bot_mention_survives_text_whose_lowercase_is_a_different_length() {
// capital I with dot above, 2 bytes -> 3
assert_eq!(
strip_bot_mention("\u{130}@codeg_bot hi", "codeg_bot"),
"\u{130} hi"
);
// Kelvin sign, 3 bytes -> 1
assert_eq!(
strip_bot_mention("\u{212A}@codeg_bot hi", "codeg_bot"),
"\u{212A} hi"
);
// capital sharp s, 3 bytes -> 2
assert_eq!(
strip_bot_mention("\u{1E9E}@codeg_bot hi", "codeg_bot"),
"\u{1E9E} hi"
);
// Enough drift to run the old offset off the end of the string.
assert_eq!(
strip_bot_mention("\u{130}\u{130}@codeg_bot", "codeg_bot"),
"\u{130}\u{130}"
);
assert_eq!(
strip_bot_mention("\u{212A}\u{212A}@codeg_bot x", "codeg_bot"),
"\u{212A}\u{212A} x"
);
// A mention whose tail is a wide character is not a mention, and
// deciding that must not slice through the character.
assert_eq!(
strip_bot_mention("@codeg_bo\u{65E5}", "codeg_bot"),
"@codeg_bo\u{65E5}"
);
}

#[test]
fn text_filter_allows_unmentioned_text_in_topic_mode() {
assert!(telegram_should_process_text_message(
Expand Down
21 changes: 17 additions & 4 deletions src-tauri/src/chat_channel/command_dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,17 @@ const CONFIG_CACHE_TTL_SECS: u64 = 30;
struct CommandConfigCache {
prefix: String,
lang: Lang,
last_refresh: Instant,
/// `None` until the first refresh, which is what forces one.
///
/// This used to be a plain `Instant` seeded with
/// `Instant::now() - Duration::from_secs(TTL + 1)`, which is a panic, not a
/// saturating subtraction: `Sub<Duration> for Instant` is a `checked_sub`
/// plus `expect`. On Windows an `Instant` is the QPC reading, i.e. time
/// since boot, so starting codeg inside the first 31 seconds of a boot
/// aborted the process here. codeg ships an autostart plugin, so launching
/// at login is an ordinary case, and this runs inside a long-lived spawned
/// task with nobody at the keyboard.
last_refresh: Option<Instant>,
}

impl CommandConfigCache {
Expand All @@ -34,12 +44,15 @@ impl CommandConfigCache {
prefix: DEFAULT_COMMAND_PREFIX.to_string(),
lang: Lang::default(),
// Force refresh on first use
last_refresh: Instant::now() - Duration::from_secs(CONFIG_CACHE_TTL_SECS + 1),
last_refresh: None,
}
}

async fn refresh_if_needed(&mut self, db: &DatabaseConnection) {
if self.last_refresh.elapsed() < Duration::from_secs(CONFIG_CACHE_TTL_SECS) {
if self
.last_refresh
.is_some_and(|at| at.elapsed() < Duration::from_secs(CONFIG_CACHE_TTL_SECS))
{
return;
}

Expand All @@ -50,7 +63,7 @@ impl CommandConfigCache {
self.lang = Lang::from_str_lossy(&val);
}

self.last_refresh = Instant::now();
self.last_refresh = Some(Instant::now());
}
}

Expand Down
24 changes: 18 additions & 6 deletions src-tauri/src/chat_channel/event_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,17 @@ struct EventConfigCache {
/// feed as IM channels, but are not debounced and ignore the per-channel
/// filter (an automation consumer wants the complete stream).
webhooks: Vec<String>,
last_refresh: Instant,
/// `None` until the first clean refresh, which is what forces one.
///
/// This used to be a plain `Instant` seeded with
/// `Instant::now() - Duration::from_secs(TTL + 1)`, which is a panic, not a
/// saturating subtraction: `Sub<Duration> for Instant` is a `checked_sub`
/// plus `expect`. On Windows an `Instant` is the QPC reading, i.e. time
/// since boot, so starting codeg inside the first 31 seconds of a boot
/// aborted the process here. codeg ships an autostart plugin, so launching
/// at login is an ordinary case, and this runs inside a long-lived spawned
/// task with nobody at the keyboard.
last_refresh: Option<Instant>,
/// Value of `EVENT_CONFIG_EPOCH` at the last refresh; a mismatch forces an
/// immediate refresh even within the TTL window.
last_epoch: u64,
Expand All @@ -92,7 +102,7 @@ impl EventConfigCache {
enabled_channels: Vec::new(),
webhooks: Vec::new(),
// Force refresh on first use
last_refresh: Instant::now() - Duration::from_secs(CONFIG_CACHE_TTL_SECS + 1),
last_refresh: None,
last_epoch: 0,
}
}
Expand All @@ -113,7 +123,9 @@ impl EventConfigCache {
// pending and the cached global_filter may already be out of date.
let config_changed = epoch != self.last_epoch;
if !config_changed
&& self.last_refresh.elapsed() < Duration::from_secs(CONFIG_CACHE_TTL_SECS)
&& self
.last_refresh
.is_some_and(|at| at.elapsed() < Duration::from_secs(CONFIG_CACHE_TTL_SECS))
{
return;
}
Expand Down Expand Up @@ -191,7 +203,7 @@ impl EventConfigCache {
// TTL elapses. The other reads above already keep-prior / fail-closed on
// their own errors, so re-reading them while retrying is harmless.
if filter_ok {
self.last_refresh = Instant::now();
self.last_refresh = Some(Instant::now());
self.last_epoch = epoch;
}
}
Expand Down Expand Up @@ -526,7 +538,7 @@ mod permission_push_tests {
event_filter_json: None,
}],
webhooks: Vec::new(),
last_refresh: Instant::now(),
last_refresh: Some(Instant::now()),
last_epoch: 0,
}
}
Expand Down Expand Up @@ -1409,7 +1421,7 @@ mod permission_push_tests {
app_metadata_service::upsert_value(&db.conn, EVENT_FILTER_KEY, "{corrupt")
.await
.unwrap();
config.last_refresh = Instant::now() - Duration::from_secs(CONFIG_CACHE_TTL_SECS + 1);
config.last_refresh = None;
config.refresh_with_epoch(&db.conn, EPOCH).await;

assert!(
Expand Down
38 changes: 31 additions & 7 deletions src-tauri/src/logging/budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,20 +85,29 @@ fn unix_day() -> i32 {
unix_day_from_secs(secs)
}

/// The name `tracing_appender` gives the `Rotation::DAILY` file covering `now`:
/// `{prefix}.{%Y-%m-%d}.{suffix}`.
///
/// Shared, because two things other than the appender need to name that exact
/// file and neither may drift from it: [`resume_point`] measures it, and
/// [`crate::logging::panic_hook`] appends a crash record to it synchronously.
pub fn daily_file_name(prefix: &str, suffix: &str, now: chrono::DateTime<chrono::Utc>) -> String {
format!("{prefix}.{}.{suffix}", now.format("%Y-%m-%d"))
}

/// What a fresh [`BudgetedWriter`] must resume from: the current UTC day, and
/// how many bytes that day's file already holds.
///
/// The filename is reconstructed the way `tracing_appender` builds it for
/// `Rotation::DAILY` — `{prefix}.{%Y-%m-%d}.{suffix}` — from the *same* instant
/// as the day number, so the two can't disagree across a midnight boundary. A
/// missing or unreadable file reads as `0`: a restart that can't measure the file
/// gets a full ceiling, which is the pre-existing behavior and strictly safer
/// than refusing to log.
/// `Rotation::DAILY` (see [`daily_file_name`]) from the *same* instant as the
/// day number, so the two can't disagree across a midnight boundary. A missing
/// or unreadable file reads as `0`: a restart that can't measure the file gets
/// a full ceiling, which is the pre-existing behavior and strictly safer than
/// refusing to log.
pub fn resume_point(dir: &Path, prefix: &str, suffix: &str) -> (i32, u64) {
let now = chrono::Utc::now();
let day = unix_day_from_secs(now.timestamp());
let name = format!("{prefix}.{}.{suffix}", now.format("%Y-%m-%d"));
let existing = std::fs::metadata(dir.join(name))
let existing = std::fs::metadata(dir.join(daily_file_name(prefix, suffix, now)))
.map(|m| m.len())
.unwrap_or(0);
(day, existing)
Expand Down Expand Up @@ -730,6 +739,21 @@ mod tests {
);
}

/// Two readers now reconstruct this name (the budget's resume measurement
/// and the panic hook's synchronous append), so the scheme is pinned rather
/// than left implicit in each of them.
#[test]
fn daily_file_name_matches_the_appenders_scheme() {
let day = chrono::DateTime::parse_from_rfc3339("2026-09-09T19:26:25Z")
.unwrap()
.with_timezone(&chrono::Utc);
assert_eq!(daily_file_name("codeg", "log", day), "codeg.2026-09-09.log");
assert_eq!(
daily_file_name("codeg-server", "log", day),
"codeg-server.2026-09-09.log"
);
}

#[test]
fn resume_point_measures_todays_file_and_ignores_other_days() {
let dir = tempfile::tempdir().unwrap();
Expand Down
Loading
Loading