Skip to content

A cut fs_read says so first, a program reads the whole file, peek takes a start, and a judgement example - #796

Merged
ljchang merged 7 commits into
mainfrom
fix/read-cut-and-judgement
Nov 1, 2026
Merged

ljchang merged 7 commits into
mainfrom
fix/read-cut-and-judgement

Conversation

@ljchang

@ljchang ljchang commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

The 2026-11-01 battery (seven fictional tasks, none told about code)
answered three right. The failures had four causes:

  • A 1.5 MB log answered "0 failures" (7). fs_read stopped at 200 KB and
    said so after the text; the dispatch's 24 KB cap kept the head, so the
    note sat in the spill file's tail and the marker named the 200 KB read as
    the whole output. The model read on through spill files and took one's
    end for the log's. Now the notice opens the result, with the file's size
    and the line to read on from.
  • A program's fs_read hit the same 200 KB cap. The gate now marks a host
    call (ToolCtx::program_room), and fs_read there reads up to the host-
    byte limit (64 MB): the result is a handle and never enters the context
    window the turn cap protects.
  • peek(h, n) after find read a note's head, not the text around the
    hit: Marco 18 of 25. peek takes start (a character offset, negative
    from the end).
  • Asked which meetings ended without agreement, the model searched for a
    keyword and made no sub-call: 4 of 10. The description now calls a
    meaning question judgement and shows llm_map over each item's relevant
    part, in a domain unlike any fixture.

With program reads up to 64 MB, a loop catching LimitError: could hold
one whole result per read in the record, so each read now applies the
conversation's own ceiling (record::RETAINED_BYTES) while the program
runs.

Docs: ARCHITECTURE.md (a cut read says so first), CODE-MODE-DESIGN.md §3
(peek), §5 (the record bound), §13 (the battery).

Tests, each failing with its fix reverted:

  • a_cut_read_says_so_first_and_a_program_reads_the_whole_file (builtin): the notice opens the result with the file's real size, survives cap_result at 24 KB, names an offset that continues the read, and a program_room read returns the whole file.
  • a_programs_read_holds_the_whole_file (monty, through the real gate): a 300 KB file is one handle of 300,000 characters with no cut.
  • peek_reads_from_a_start_offset: offsets, negative start, an out-of-range start, and the count being what was returned.
  • the_judgement_example_runs_as_written: the description's text runs, one pass per ticket, each pass seeing only the resolution.
  • a_program_that_reads_past_its_byte_limit_holds_a_bounded_record: six reads, five refused, all recorded, two held at a 2,500-byte ceiling.

cargo test -p mecha-core --lib 3842 passed; clippy -D warnings clean with all features and with --no-default-features.

…es a start, and code mode has a judgement example

The 2026-11-01 battery (seven fictional tasks, none told about `code`)
answered three right. The failures had four causes:

- A 1.5 MB log answered "0 failures" (7). `fs_read` stopped at 200 KB and
  said so after the text; the dispatch's 24 KB cap kept the head, so the
  note sat in the spill file's tail and the marker named the 200 KB read as
  the whole output. The model read on through spill files and took one's
  end for the log's. Now the notice opens the result, with the file's size
  and the line to read on from.
- A program's `fs_read` hit the same 200 KB cap. The gate now marks a host
  call (`ToolCtx::for_program`), and `fs_read` there reads up to the host-
  byte limit (64 MB): the result is a handle and never enters the context
  window the turn cap protects.
- `peek(h, n)` after `find` read a note's head, not the text around the
  hit: Marco 18 of 25. `peek` takes `start` (a character offset, negative
  from the end).
- Asked which meetings ended without agreement, the model searched for a
  keyword and made no sub-call: 4 of 10. The description now calls a
  meaning question judgement and shows `llm_map` over each item's relevant
  part, in a domain unlike any fixture.

With program reads up to 64 MB, a loop catching `LimitError:` could hold
one whole result per read in the record, so each read now applies the
conversation's own ceiling (`record::RETAINED_BYTES`) while the program
runs.

Docs: ARCHITECTURE.md (a cut read says so first), CODE-MODE-DESIGN.md §3
(peek), §5 (the record bound), §13 (the battery).
Comment thread mecha-core/src/agent.rs Outdated
Comment on lines +5951 to +5956
let mut tool_ctx = Self::call_ctx(cx, id, turn_taint, recorded_taint, context, work, None);
// `Refuse` is a program's host call and nothing else (`Asking`): its
// result is a handle, so the tool need not cut it for a context window.
if asking == Asking::Refuse {
Arc::make_mut(&mut tool_ctx).for_program = true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for_program's own doc says "Set by Agent::gate for a host call alone; never the model's", but nothing enforces that — unlike host_calls, which call_ctx resets to None explicitly because a child run would otherwise inherit it.

call_ctx builds its ctx with ..(*cx.tools).clone(), so for_program is inherited from the run's base ToolCtx. The one way a call ctx becomes a run's base ctx is subagent.rs:350 (tools: Arc::new(ctx.clone())). If a subagent were ever admitted into a program, every turn call in the child — including fs_read — would see for_program = true and return up to 64 MB straight into the child model's context window, which is exactly what the flag's comment says cannot happen.

That route is closed today, and tested (a_subagent_is_never_a_program_function): Subagent declares no effect(), so it resolves Undeclared and Admission::admits rejects it. So this is latent, not live. But it is one effect() away, and the structural fix is a one-liner — take it as a parameter next to host_calls rather than letting it ride the .. so the claim in the comment is what the code does:

fn call_ctx(..., host_calls: Option<..>, for_program: bool) -> Arc<ToolCtx> {
    Arc::new(ToolCtx { ..., host_calls, for_program, ..(*cx.tools).clone() })
}

That also drops the Arc::make_mut here.

@claude

claude Bot commented Nov 1, 2026

Copy link
Copy Markdown

Reviewed against CLAUDE.md's invariants. The change is sound — I checked the security-critical surface first and found nothing wrong there.

What I verified rather than assumed:

  • for_program cannot widen a turn's read. Asking::Refuse has exactly one construction site outside tests (serve_host_calls, agent.rs:6032), so the mark really is a host call and nothing else. FsRead::call still resolves through ctx.resolve_for(..., Access::Read) before the cap is chosen, so the path jail is untouched; taint, the interlock, and the interlock → hook → approver order are not touched at all.
  • The cut offset is arithmetically right and the test is not vacuous. cut = offset + text.matches('\n').count() counts the complete lines kept and resumes on the line the cap bisected, so reading on overlaps rather than skips. I traced the test's fixture by hand (40,000 lines of 22 bytes, cap 200,000 → 9,090 complete lines, cut at 9,091 with 20 of its bytes kept), so next.content.starts_with(last_kept) is a real assertion and not one that passes because the cut happened to land on a line boundary. The notice surviving cap_result at 24 KB is the actual mechanism of the 2026-11-01 incident, and it is asserted directly.
  • peek's start. Bounds-checked both ends; start + chars cannot underflow for any i64 (chars >= 0); negative indexing matches slice's norm; byte offsets go through bytes_at with ascending inputs; and peeked counts what was returned, which the 4 + 2 = 6 bytes assertion pins.
  • bound_at fails in the safe direction. An evicted call keeps its record and loses its bytes, so grounding reads no referent rather than an empty one — the "unknown is never clean" shape. The new test holds at 2,500 with 1,000-byte reads, where the old behaviour would hold 6,000.
  • Ran read_window_tests (5 passed) and tool::code (86 passed). Note that a_programs_read_holds_the_whole_file and the_judgement_example_runs_as_written both skip without monty — this file's existing convention, but it does mean those two report green here without having run.

Two things to weigh at merge; neither changes the shape of the change.

1. for_program is not actually "set by the gate alone". Detail inline on agent.rs:5951. call_ctx builds its ctx with ..(*cx.tools).clone(), so the flag is inherited, and subagent.rs:350 (tools: Arc::new(ctx.clone())) is the one place a call ctx becomes a run's base ctx. The field's own doc comment makes a claim the code does not enforce — unlike host_calls, which call_ctx resets to None for exactly this reason. Closed today by Subagent declaring no effect(), and tested (a_subagent_is_never_a_program_function), so this is latent, not live. Passing it as a call_ctx parameter beside host_calls closes it and drops the Arc::make_mut.

2. The cap is the maximum host-byte limit, not what the program has left. CODE-MODE-DESIGN.md §5 bounds the record for a loop that catches LimitError: and reads on — but not the work. A program with 1 KB of host_bytes remaining that reads a 64 MB file still pays a full 64 MB read, a 64 MB Vec, a to_owned() and an Arc::from copy before shared refuses the handle, and TOOL_READS is 2,048, all inside the 600 s wall clock. Before this PR each such read cost 200 KB. Nothing is unbounded and it is self-inflicted against the owner's own files, so I would call this an observation rather than a finding — but if you want it closed, handing fs_read min(HOST_BYTES, remaining) makes the refusal cheap as well as correct.

Minor, take or leave: the second example's gate at code.rs:798 spells the predicate admission.listings().any(|n| n == "fs_list") where listings from line 749 is still in scope and line 768 spells the same test listings.contains(&"fs_list"). Two spellings of one condition is how a later change to one misses the other.

… stamped by call_ctx so no run inherits it, and the battery's rerun is recorded
…under the allow gate already has, and a test's doc comment is back on its test
Comment thread mecha-core/src/tool/builtin.rs Outdated
let mut text = if selected {
// The line the cut fell on: every complete line kept, plus the one it
// bisected, so reading on from it overlaps a little, the safe direction.
let cut = truncated.then(|| offset + text.matches('\n').count() as u64);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cut == offset whenever the kept text has no newline, so the notice names the offset that was just read.

cut = offset + text.matches('\n').count(). When the first kept line is itself longer than cap, the kept text contains no newline, the count is 0, and cut == offset — the read that just happened. FsRead then renders This is not the whole file: read on with offset {line}, and fs_read has no byte offset, so following that instruction returns byte-for-byte the same result and the same notice. The model makes no progress at all.

The new test pins exactly this shape without noticing the advice is circular:

let data = "é".repeat(MAX_OUTPUT_BYTES);   // no newline anywhere
...
assert_eq!(cut, Some(1), "the cut fell on the only line");

Read at offset = 1, so the notice says "read on with offset 1". The realistic fixtures are a minified JSON log, a one-line CSV, or a 1.5 MB log written without line breaks — the same family as the 2026-11-01 incident this change exists to fix.

The old message (use offset and limit to read more) was unhelpful but not false; this one is an instruction that cannot advance, and the repo's own convention is that an error says what to do instead. cut == offset is distinguishable here and deserves its own sentence — e.g. "line {line} alone is longer than this {cap}-byte limit, so offset cannot read past it" — rather than an offset the model will retry.

@claude

claude Bot commented Nov 1, 2026

Copy link
Copy Markdown

Second pass on this branch, after the two findings from the first review were addressed (program_room is now a call_ctx parameter rather than inherited through ..(*cx.tools).clone(), and the cap is the program's remaining room rather than the maximum). Both fixes are real and the new a_call_context_never_inherits_a_programs_mark fails if either is reverted.

One medium finding, inline on builtin.rs:255: the cut offset makes no progress when the bisected line is itself longer than the cap. cut = offset + newlines, so a read whose kept text contains no newline gets cut == offset, and the notice tells the model "read on with offset {offset}" — the read it just made. fs_read has no byte offset, so that instruction returns identical bytes forever. The new test a_huge_single_line_is_capped_before_reading_the_tail pins cut == Some(1) for a read at offset 1 ("é".repeat(MAX_OUTPUT_BYTES), no newline) without noticing the advice is circular. The shape is a minified JSON log or a one-line CSV — the same family as the 1.5 MB log this change exists to fix, and the old message, while unhelpful, was at least not false.

Everything else I checked held up:

  • Security surface untouched. FsRead::call still goes through ctx.resolve_for(.., Access::Read) before the cap is chosen, so the jail is unaffected; program_room is read by exactly one tool and changes only how many bytes it returns. Taint, .from_outside(), and the interlock → hook → approver order are not touched. Asking::Refuse and Some(room) are set together at the one serve_host_calls site (agent.rs:6030); every other gate/call_ctx site passes None.
  • The program read really does bypass only the context-window cap. serve_host_calls hands Gated::Ran's output straight to HostAnswer::Ran with no cap_result, so a handle never enters the turn's budget — the premise the 64 MB cap rests on. room = host_bytes - bytes is exactly what charge will accept, so a full-room read is chargeable and the refusal that follows a cut read is deliberate (the test comment says as much).
  • The notice survives the dispatch's cut, asserted directly against cap_result(.., 24_000, ..) rather than assumed — that was the actual mechanism of the incident.
  • peek(h, n, start) is bounds-checked on both ends, start + chars cannot underflow for any i64 since chars >= 0, bytes_at gets ascending offsets, negative indexing matches slice's, and peeked counts what was returned (the 4 + 2 = 6 assertion pins that, not the requested n).
  • bound_at fails in the safe direction — held only grows, so once the ceiling is crossed every older Held is unloaded; an evicted call keeps its record and loses its bytes, which grounding reads as no referent rather than an empty one.
  • No stale references to the old [truncated: byte limit reached…] string anywhere in the tree or docs; no new Config field, so no ConfigLayer gap.
  • cargo test -p mecha-core --lib tool::code 86 passed, tool::builtin::read_window_tests 5 passed, cargo clippy -p mecha-core --all-targets clean.

Minor, for the merge summary rather than a change:

  • a_programs_read_holds_the_whole_file and the_judgement_example_runs_as_written both skip without monty (confirmed here: SKIPPED: the pinned monty worker or the platform's sandbox is unavailable), so they report green without running. That matches the_descriptions_example_runs_as_written's existing convention, but note that assert!(describe(&admitted).contains(JUDGE_EXAMPLE)) sits after the skip return and needs no interpreter — hoisting it above would mean a machine without monty still catches the describe() gate dropping the example.
  • The PR body names ToolCtx::for_program; the field that shipped is ToolCtx::program_room (ARCHITECTURE.md already says the latter).

Merging is yours to call.

…ffset gets past it rather than repeat the read, and the judgement example's description check runs without an interpreter
Comment thread mecha-core/src/tool/builtin.rs Outdated
let mut text = if selected {
// The line the cut fell on: every complete line kept, plus the one it
// bisected, so reading on from it overlaps a little, the safe direction.
let cut = truncated.then(|| offset + text.matches('\n').count() as u64);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cut line is recounted from the kept text, and misreports a cut that lands exactly on a line boundary — which is the one case where the notice gives the model a dead end.

Trace, cap = 20, file = "a"*20 + "\n" + "second line\nthird\n":

  • iter 1: newline = Some(20), take = 21, take.min(cap + 1 - 0) = 21, so bytes is 21 bytes and line becomes 2.
  • loop exits (21 <= 20 false), truncated = true, bytes.truncate(20) drops the newline.
  • text now holds zero newlines, so cut = offset + 0 = Some(1).

call then takes the line == offset branch (builtin.rs:133) and tells the model "line 1 alone is longer than that, so no offset gets past it with fs_read" — but line 1 fit exactly, and offset: 2 would have read the rest. The same trace on "a"*20 + "\n" (file is cap + 1 bytes) reports This is not the whole file when the whole content was returned and only the terminating newline was lost.

This is not as narrow as it looks: it fires whenever a line boundary falls on byte cap exactly, i.e. for any fixed-width log whose record length divides 200,000 (20, 25, 32, 40, 50, 64, 80, 100 bytes…). The new fixture here is 22 bytes/line, so it misses the case. For a program's read cap is the remaining room, an arbitrary number.

The loop already tracks the right answer: at exit line is the line the cut bisected, or — in the boundary case — the first unread line. It agrees with the recount in every bisected case (checked against "one\ntwo\nthree is a long line here\nfour\n" at cap = 12: both give 3) and is correct in the boundary case where the recount is not.

Suggested change
let cut = truncated.then(|| offset + text.matches('\n').count() as u64);
let cut = truncated.then_some(line);

Worth a case in a_cut_read_says_so_first_and_a_program_reads_the_whole_file with a line length that divides the cap, asserting the notice names an offset that reads on rather than the dead end.

@claude

claude Bot commented Nov 1, 2026

Copy link
Copy Markdown

Reviewed against the invariants in CLAUDE.md. The security-relevant part of this change is sound, and the one bug I found is in the new notice's arithmetic, not in the gate.

The program_room mark holds where it has to. ToolCtx::program_room is set only in Agent::call_ctx, listed ahead of ..(*cx.tools).clone(), so the base context's value is overwritten rather than inherited — the same shape as host_calls. I checked that call_ctx is the only construction site for either (grep over mecha-core/src finds program_room in mod.rs for the field and Default, and in agent.rs only at call_ctx and its callers), so a subagent run whose RunContext is built from a host call's ctx cannot carry the mark into its own turn reads. a_call_context_never_inherits_a_programs_mark measures exactly that, and would fail if the field were left to the struct-update syntax.

The relaxation itself is justified for the reason given: the result becomes a handle, text leaves a handle only through peek/str[n] under peek_bytes, fs_list listings are names, and a failed read's raise carries the stub rather than the text — so nothing of the extra bytes reaches the context window the 200 KB cap protects. fs_read stays private(), local, not external; no taint or egress axis moves. Dispatch order, tool_result pairing, compaction and Config/ConfigLayer are untouched.

Tests measure the guarantee rather than asserting it. a_programs_read_holds_the_whole_file goes through the real gate (so it would catch cap_result being applied to a host answer), and the cap_result(…, 24_000, …) assertion in a_cut_read_says_so_first_… reproduces the actual 2026-11-01 mechanism rather than the fix's shape. peek_reads_from_a_start_offset covers the negative start, the out-of-range raise, and that the charge is what was returned. peek's start matches slice's semantics (negative from the end, out of range raises, start == len is empty), and kwargs reach it through Args::get's fallback, so peek(h, start=…) works as the description promises.

One finding

read_text_window's new cut recounts newlines in the kept text instead of using the line the loop already tracks, so a cut that lands exactly on a line boundary reports the previous line — and in the first-line case produces the "no offset gets past it with fs_read" dead end for a file that offset: 2 would read on from. Inline, with the trace and a one-line suggestion. Medium: it fires for any fixed-width log whose record length divides 200,000, and the new fixture's 22-byte lines miss it.

Observations, not requests

  • The same tail-marker shape is still live in two neighbours. shell appends [output exceeded {MAX_OUTPUT_BYTES} bytes; …] after the body (builtin.rs:815) and truncate appends [truncated: {what} was N bytes …] (builtin.rs:38, used by http_fetch). cap_result keeps the head and spills the rest, so for a 1.5 MB command output the model sees "the first 24000 of 200068 bytes", reads the spill file, and never learns that 1.3 MB was discarded as it streamed — the fs_read incident with a different tool in front of it. Not this PR's scope, but it is the same incident and the fix is the same move.
  • A 64 MB read peaks at several times that. read_text_window grows bytes from Vec::new() (amortized doubling, so up to ~2× cap reserved), then text.to_owned() copies it, then Arc::from(out.content) copies again — ~3–4× the file for one read, transient, on a box that is also holding a model resident. Vec::with_capacity(cap.min(file_len)) would remove the doubling slack cheaply.
  • A program's cut read is unreachable through the gate, because the notice is prepended: content is room bytes plus ~150, which room_for/the host-byte charge then refuses. The program_room: Some(1000) case in the test therefore measures FsRead directly rather than a handle a program could hold — which the test's own comment says ("the refusal that follows costs a small read"), so this is a note, not a complaint.
  • Record blobs now reach up to RETAINED_BYTES of program bytes per run in the session's blob store, where a program's reads were 200 KB each before. The bound is the point of the retain change and it is a strict improvement over the unbounded term, but the disk figure per session is new.

… a cut just before a newline reads on from the next line rather than declare a dead end
Comment on lines +108 to +114
// A program's read becomes a handle, which never enters the context
// window the turn cap protects, so it may hold the file up to the
// program's own host-byte limit (`CODE-MODE-DESIGN.md` §5).
let cap = match ctx.program_room {
Some(room) => room.min(crate::tool::code::defaults::HOST_BYTES),
None => MAX_OUTPUT_BYTES,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cut program read now yields no handle at all, where it used to yield the first 200 KB.

cap is set to exactly room, and then — when the cut happens — the notice is prepended to the text (lines 141-146), so the result is room + ~175 bytes. Back in Run::read, shared() → charge(text.len()) refuses anything with n > host_bytes - bytes, i.e. n > room (code.rs:1215). So a cut program read is refused by the charge by construction, every time:

  • fresh program, 100 MB log: cap = 64 MB, content 64 MB + 175 → LimitError: host bytes: a result of 67109039 bytes would take this program past its 67108864 (0 held), no handle.
  • before this PR: cap was MAX_OUTPUT_BYTES, content 200 KB + 60, the charge passed, and the program got a handle over the head of the file to find/count/peek.

The code comment and ARCHITECTURE.md both say the read is "capped at what the program can still hold", which is off by the notice: room bytes is exactly what it cannot hold once the notice is on the front. code_tests.rs's "the refusal that follows costs a small read" reads as if the refusal were inevitable, but it isn't — it's caused by the notice overhead, and the error the model sees names a byte count with no hint that offset/limit would get it a readable slice.

Two ways out: size the cap below the notice (room.saturating_sub(NOTICE_MAX) with the notice length bounded), or charge only the file bytes and keep the notice out of the handle's length. Either way it would be worth a test at the gate level — the monty test only exercises room = 64 MB on a 300 KB file, so no test covers what a program actually gets back when the cut fires.

@claude

claude Bot commented Nov 1, 2026

Copy link
Copy Markdown

Reviewed against CLAUDE.md's invariants. One finding, medium, left inline on builtin.rs:108-114: cap = room plus a prepended notice makes the result room + ~175 bytes, so charge() (code.rs:1215) refuses every cut program read by construction — a program reading a file larger than its remaining host-byte room now gets a LimitError and no handle, where before the PR it got the first 200 KB. The comment and ARCHITECTURE.md both describe the cap as "what the program can still hold", which is off by the notice's length.

Everything else checks out, and the parts that could have gone wrong didn't:

  • program_room is stamped, never inherited. call_ctx sets it explicitly alongside host_calls before ..(*cx.tools).clone() (agent.rs:6178), which matters because subagent.rs:334 builds a child's RunContext from the parent call's ToolCtx — so a host call's mark would otherwise have reached the child's turn reads. a_call_context_never_inherits_a_programs_mark measures both directions, and it fails on the old shape.
  • No security surface moved. ctx.resolve_for still gates the path (builtin.rs:101); taint, external, the interlock and the gate order are untouched; the host-call path still takes Agent::gate in Asking::Refuse with program_room as data, not as an escape. Nothing here fails open.
  • peek(h, n, start) adds no reach. start is a character offset like find's, resolved against the handle's own text() (so a sliced handle stays correct), range-checked before bytes_at, and still charged text.len() against the one peek_bytes cap — the same bytes were already reachable via slice + peek. peek_reads_from_a_start_offset covers the negative start, the out-of-range start, and that the count is what was returned.
  • The record bound is real where it needs to be. On a refused read the handle is never minted, so bound_at actually drops the only Arc — which is the Code mode at scale: fs_list names as str, tool reads at 2,048, sub-calls kept for judgement #793 loop. The newest-first policy matches bound_held, and dropped bytes degrade to "no referent", which grounding reads fail-closed. a_program_that_reads_past_its_byte_limit_holds_a_bounded_record pins six calls / two held at a 2,500-byte ceiling.
  • The cut-line arithmetic is right at the boundaries I could find. line is file-absolute and advances only when a newline was fully consumed, so the edge.log case (cut exactly on the newline) names line 2 and line == offset really does imply that line alone exceeds the cap — the "no offset gets past it" branch cannot loop, and a two-step case (offset=1, huge line 2) still terminates.
  • Cached prefix. describe is fixed at construction and both examples are gated on the same fs_list + fs_read pair, so JUDGE_EXAMPLE costs one prefix repayment, not one per turn. the_judgement_example_runs_as_written asserts describe still contains it before the monty skip, so a machine without the interpreter still catches the example being dropped — good instinct, and it's the kind of thing a silent skip usually hides.
  • No Config/ConfigLayer field, no provider matching in agent.rs, no change to the tool_use/tool_result pairing or the compaction cut.

Minor, not worth a push: the host-call path still has no cap_result, so the 64 MB ceiling is the only bound on what one read puts in the program record and its blob (bounded in turn by RETAINED_BYTES) — consistent with the design, just a bigger number than before. Merging is yours.

…ce, so it is a handle the program can hold rather than a refused charge
@claude

claude Bot commented Nov 1, 2026

Copy link
Copy Markdown

Review: sound. No major or medium finding.

I read CLAUDE.md, docs/ARCHITECTURE.md §A cut read says so first, and CODE-MODE-DESIGN.md §3/§5/§13 before the diff, then checked the four mechanisms against the tree.

Security invariants — intact.

  • FsRead::call still opens with ctx.resolve_for(arg_str(&input, "path")?, Access::Read)? (builtin.rs:105); nothing in the new cap path or the notice touches a raw path. The only new use of the resolved path is path.as_os_str().len() and path.display(), both after resolution.
  • program_room is the structural twin of host_calls: stamped per call in Agent::call_ctx (agent.rs:6181), None in ToolCtx::default (tool/mod.rs:1212), and Some(room) only from the HostRequest::Call arm of serve_host_calls (agent.rs:6040). grep -rn program_room confirms the only reader is builtin.rs:121 — no tool other than fs_read can widen itself off the mark, and a_call_context_never_inherits_a_programs_mark pins the non-inheritance that would otherwise have reached a subagent's turn reads. Dispatch order, .from_outside(), and Conversation taint are untouched; self.external |= out.external in Run::read is unchanged.
  • Nothing moved from fail-closed to fail-open. record::bound_at is called after the push and before the self.names(...) early return (code.rs:1364-1366), so no recorded call escapes the bound, and an unloaded one becomes Bytes::Unloaded — no referent, never an empty one.

The byte arithmetic holds. charge refuses on bytes + n > host_bytes, so room = host_bytes - bytes is exactly the largest result that still passes — the notice reserve is what makes a cut program read fit rather than be refused whole, which is the stated fix. read_text_window's loop guard (bytes.len() <= cap) keeps cap + 1 - bytes.len() >= 1, so the new take.min(...) can't underflow. I worked through the cut line counter by hand on the cases the comment names: a cut mid-line reports that line (overlapping read-on, safe direction); a cut whose last byte is a newline increments past a line whose content was fully kept, so line + 1 is right and the edge.log assertion isn't accidental.

peek's start. Positive, negative-from-end, start == chars (empty, not an error), start > chars (HandleError), and the count being bytes returned rather than bytes requested all check out; bytes_at takes ascending offsets and tolerates from == from + take. Args::get falls back to kwargs, so peek(h, start=5) and peek(h, 5) both resolve as documented.

Tests measure the new guarantees. Each of the five would fail with its fix reverted, and a_cut_read_says_so_first_and_a_program_reads_the_whole_file tests the thing that actually broke — it runs the output back through cap_result at 24 KB and asserts the notice survives the head cut, which is the 2026-11-01 incident itself rather than a restatement of the new code. the_judgement_example_runs_as_written asserting describe(&admitted).contains(JUDGE_EXAMPLE) before the monty() skip is the right shape: a machine without the worker still catches describe dropping the example.

Verified here: cargo test -p mecha-core --lib → 3849 passed, 0 failed, 22 ignored. cargo clippy -p mecha-core --all-targets → no warnings. The two monty-gated tests skipped on this runner (no installed worker), so their bodies are unmeasured in this environment — worth running once with MECHA_MONTY_WORKER set, since a_programs_read_holds_the_whole_file is the only thing that exercises the mark through the real gate.

Minor, for you to weigh at merge — no change requested

  1. NOTICE_BYTES reserves path.as_os_str().len() but the notice renders path.display() (builtin.rs:124 vs :154). For a non-UTF-8 path each invalid byte becomes a 3-byte U+FFFD, so display() can exceed the reserved length. The measured notice is ~265 bytes against a 512-byte reserve, so it would take >110 invalid bytes in the workspace path to overrun — not worth code, but the two spellings being different is the kind of thing that looks deliberate later.

  2. When room lands between the notice's real length and NOTICE_BYTES + path.len(), cap saturates to 0 and the program gets a handle that is the notice and no file content, with is_error: false — where before the raise it would have got a LimitError from the charge. count(h, …) then answers about the notice. The notice is first now, so anything that peeks sees it, and this is the extreme of a tradeoff the design already accepts for every cut program read (a 99 KB read of a 1 MB file answers about 99 KB and raises nothing). Flagging it only because the {cap}-byte-limit and "line 1 alone is longer than that" wording are both false at cap == 0.

  3. Allocation amplification on the selected path. With program_room raising cap 320×, a program read that passes offset runs text.lines().collect::<Vec<_>>().join("\n") over up to 64 MB (builtin.rs:292) — a Vec of one &str per line on top of the Vec<u8>, the String, the format! result and the Arc<str>. For a short-lined 64 MB log that is a few hundred MB transient for one call, outside the pool that bounds what handles hold. Inherent to the raise rather than introduced by it, and a String built in place from the retained range would close it if it ever shows up.

Merging is yours.

@ljchang
ljchang merged commit b949255 into main Nov 1, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant