Skip to content

Commit 1bed2a5

Browse files
yogthosYogthos
andauthored
fix(F19+F20): strip UTF-8 BOM in read; bound interject channel (#91)
Two F-MEDIUM fixes from ROADMAP.md. ## F19 — UTF-8 BOM strip Old Windows-saved files start with U+FEFF (0xEF 0xBB 0xBF). Before this, the BOM survived `read_line` and ended up as an invisible prefix on the first line shown to the LLM. opencode's `Bom.readFile()` does the same strip on its side. `read.rs` line loop now tracks `first_line` and strips a leading `'\u{FEFF}'` from line 1 only. Mid-file BOMs (rare but possible when concatenating files) stay as regular chars. ## F20 — Bounded interject channel `AgentRunner::interject_tx: mpsc::UnboundedSender<()>` could grow without bound if the user hammered the interject keybind while the runner was in a long LLM call. Each press allocated an entry even though all but the first were redundant (the runner drains via `try_recv()` after the first wakeup). - Switch to `mpsc::Sender<()>` with capacity 64. - UI uses `try_send(())` instead of `send(())`: if the channel is full, no-op (we already have a wakeup queued). - Updated all 9 ownership transfer sites + the runner-side receiver type. ## Tests F19: two new tests in `agent::tools::read::tests`: - `read_strips_utf8_bom_from_first_line`: writes a file starting with `\u{FEFF}`, asserts the output has no BOM byte anywhere. - `read_only_strips_bom_at_start_of_file`: BOM in line 2 stays intact — guards against the strip applying to wrong lines. F20: no new tests (capacity-bound is a runtime contract; the existing interjection-queue tests in ui already exercise the try_send path). 681 pass (was 679). All build profiles clean. Co-authored-by: Yogthos <yogthos@gmail.com>
1 parent 4edacb2 commit 1bed2a5

3 files changed

Lines changed: 90 additions & 5 deletions

File tree

src/agent/runner.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,14 @@ pub struct AgentRunner {
119119
/// emits `AgentEvent::Interjected` with whatever assistant text had
120120
/// streamed so far, and the UI is responsible for queueing the next
121121
/// user turn. Unbounded because the signal payload is just `()`.
122-
pub interject_tx: mpsc::UnboundedSender<()>,
122+
/// F20: bounded so a user who hammers the interject keybind
123+
/// can't fill an unbounded queue while the runner is in a long
124+
/// LLM call. Only the FIRST signal needs to be received — all
125+
/// subsequent ones are noise (the runner drains via
126+
/// `try_recv()` after the first wakeup). 64 is generous; if
127+
/// the channel is full, `try_send` silently no-ops (we already
128+
/// have one queued).
129+
pub interject_tx: mpsc::Sender<()>,
123130
}
124131

125132
pub fn convert_history(session: &Session) -> Vec<Message> {
@@ -220,7 +227,7 @@ async fn run_stream<M, P>(
220227
prompt: &str,
221228
history: Vec<Message>,
222229
event_tx: &mpsc::Sender<AgentEvent>,
223-
interject_rx: &mut mpsc::UnboundedReceiver<()>,
230+
interject_rx: &mut mpsc::Receiver<()>,
224231
) -> StreamOutcome
225232
where
226233
M: CompletionModel + 'static,
@@ -361,7 +368,8 @@ where
361368
{
362369
cache.clear();
363370
let (event_tx, event_rx) = mpsc::channel::<AgentEvent>(256);
364-
let (interject_tx, mut interject_rx) = mpsc::unbounded_channel::<()>();
371+
// F20: bounded channel. See `AgentRunner::interject_tx` doc.
372+
let (interject_tx, mut interject_rx) = mpsc::channel::<()>(64);
365373

366374
let task = tokio::spawn(async move {
367375
let policy = RecoveryPolicy::default();

src/agent/tools/read.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,21 @@ impl Tool for ReadTool {
121121
let mut total_lines = 0usize;
122122
let mut excerpt_lines: Vec<(usize, String)> = Vec::with_capacity(limit);
123123
let want_end = offset.saturating_add(limit);
124+
let mut first_line = true;
124125
while let Some(line) = lines.next_line().await.transpose() {
125126
let mut line = line?;
127+
// F19: strip UTF-8 BOM from the FIRST line only. Old
128+
// Windows-saved files start with U+FEFF (0xEF 0xBB 0xBF);
129+
// when present, the BOM ended up as a leading 3-byte
130+
// invisible-character prefix in the LLM context.
131+
// opencode `read.ts` uses `Bom.readFile()` for the same
132+
// reason.
133+
if first_line {
134+
if let Some(stripped) = line.strip_prefix('\u{FEFF}') {
135+
line = stripped.to_string();
136+
}
137+
first_line = false;
138+
}
126139
if line.len() > MAX_LINE_BYTES {
127140
// Truncate by byte index — careful to land on a UTF-8
128141
// boundary. Drop bytes until we find one.
@@ -276,4 +289,60 @@ mod tests {
276289
let body_lines: Vec<&str> = out.lines().skip(2).collect();
277290
assert_eq!(body_lines.len(), 5);
278291
}
292+
293+
/// F19: UTF-8 BOM (U+FEFF, bytes 0xEF 0xBB 0xBF) at the start
294+
/// of a file is stripped before the line reaches the LLM. The
295+
/// raw 3-byte prefix would otherwise render as an
296+
/// invisible-character at the start of line 1.
297+
#[tokio::test]
298+
async fn read_strips_utf8_bom_from_first_line() {
299+
let path = temp_path("bom");
300+
let bom = "\u{FEFF}";
301+
std::fs::write(&path, format!("{bom}first\nsecond")).unwrap();
302+
303+
let tool = ReadTool::new(None, None);
304+
let out = tool
305+
.call(ReadArgs {
306+
path: path.to_string_lossy().into_owned(),
307+
offset: None,
308+
limit: None,
309+
})
310+
.await
311+
.unwrap();
312+
let _ = std::fs::remove_file(&path);
313+
314+
// Body lines (after the "File: …" header + blank line).
315+
let body: Vec<&str> = out.lines().skip(2).collect();
316+
assert_eq!(body, vec!["1: first", "2: second"]);
317+
// No BOM byte anywhere in the output.
318+
assert!(
319+
!out.contains('\u{FEFF}'),
320+
"BOM should be stripped: {:?}",
321+
out,
322+
);
323+
}
324+
325+
/// F19: only the FIRST line gets BOM-stripped. A mid-file BOM
326+
/// (extremely rare but possible) is preserved as a regular
327+
/// character.
328+
#[tokio::test]
329+
async fn read_only_strips_bom_at_start_of_file() {
330+
let path = temp_path("bom-mid");
331+
let bom = "\u{FEFF}";
332+
std::fs::write(&path, format!("first\n{bom}second")).unwrap();
333+
334+
let tool = ReadTool::new(None, None);
335+
let out = tool
336+
.call(ReadArgs {
337+
path: path.to_string_lossy().into_owned(),
338+
offset: None,
339+
limit: None,
340+
})
341+
.await
342+
.unwrap();
343+
let _ = std::fs::remove_file(&path);
344+
345+
// The mid-file BOM stays.
346+
assert!(out.contains('\u{FEFF}'));
347+
}
279348
}

src/ui/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,10 @@ pub async fn run_interactive(
457457
// Sender into the running agent's interjection channel. The UI signals
458458
// (unit-only payload) when a user-typed interjection is queued; the
459459
// runner honors it at the next tool-result boundary.
460-
let mut agent_interject: Option<mpsc::UnboundedSender<()>> = None;
460+
// F20: bounded mpsc::Sender. Multiple interject signals while
461+
// the runner is mid-call get coalesced — only the first wakeup
462+
// matters since the runner drains via try_recv() after waking.
463+
let mut agent_interject: Option<mpsc::Sender<()>> = None;
461464
let mut agent_line_started = false;
462465
let mut response_buf = String::new();
463466
// Count of `AgentEvent::ToolCall` events observed during the
@@ -1385,7 +1388,12 @@ pub async fn run_interactive(
13851388
// (race with Done) — harmless, queue still
13861389
// drains on the Done handler.
13871390
if let Some(tx) = agent_interject.as_ref() {
1388-
let _ = tx.send(());
1391+
// F20: try_send so a full channel
1392+
// (already-queued wakeup) is a
1393+
// no-op rather than blocking the
1394+
// UI thread. We only need ONE
1395+
// wakeup queued at a time.
1396+
let _ = tx.try_send(());
13891397
}
13901398
for line in text.lines() {
13911399
let safe_line = sanitize_output(line);

0 commit comments

Comments
 (0)