toolkit: no thread panic in http_fetch, no casts, exact egress-proxy form

Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)

crates/toolkit/src/fetch.rs

@@ -80,7 +80,16 @@ pub fn fetch_with(curl: &Path, args: &HttpFetchArgs) -> Outcome {
         Some(r) => r,
         None => return Outcome::tool_error("http_fetch: curl has no standard error".to_string()),
     };
-    let stderr_handle = std::thread::spawn(move || read_capped(stderr_reader));
+    // `Builder`, not `spawn`, which panics when the system refuses a thread.
+    let stderr_handle = match std::thread::Builder::new().spawn(move || read_capped(stderr_reader))
+    {
+        Ok(handle) => handle,
+        Err(e) => {
+            let _ = child.kill();
+            let _ = child.wait();
+            return Outcome::tool_error(format!("http_fetch: cannot start a thread: {e}"));
+        }
+    };
 
     let mut body = Vec::new();
     let mut stdout = match child.stdout.take() {

crates/toolkit/src/files.rs

@@ -35,7 +35,10 @@ pub fn read_file(args: &ReadFileArgs) -> Outcome {
     };
 
     let mut buf = Vec::new();
-    let n = match file.take(MAX_READ as u64 + 1).read_to_end(&mut buf) {
+    let n = match file
+        .take(u64::try_from(MAX_READ).map_or(u64::MAX, |n| n.saturating_add(1)))
+        .read_to_end(&mut buf)
+    {
         Err(e) => return tool_err("read_file", path, &e.to_string()),
         Ok(n) => n,
     };

crates/toolkit/src/input.rs

@@ -33,7 +33,9 @@ impl From<std::io::Error> for InputError {
 /// (`Read::take`); more than MAX_INPUT is TooLarge.
 pub fn read_input(stdin: &mut dyn std::io::Read) -> Result<String, InputError> {
     let mut buf = Vec::new();
-    let n = stdin.take(MAX_INPUT as u64 + 1).read_to_end(&mut buf)?;
+    let n = stdin
+        .take(u64::try_from(MAX_INPUT).map_or(u64::MAX, |n| n.saturating_add(1)))
+        .read_to_end(&mut buf)?;
     if n > MAX_INPUT {
         return Err(InputError::TooLarge);
     }

crates/toolkit/src/main.rs

@@ -18,7 +18,8 @@ fn main() -> ExitCode {
 /// The `egress-proxy --socket <path> --allow <list>` form. Anything else, including a program name
 /// that is not `egress-proxy`, returns `None` so the tool form handles it.
 fn parse_egress_proxy(args: &[OsString]) -> Option<ExitCode> {
-    if args.first()?.as_bytes() != b"egress-proxy" {
+    // Exactly five words: a longer list is not this form, and goes to the tool form (exit 2).
+    if args.len() != 5 || args.first()?.as_bytes() != b"egress-proxy" {
         return None;
     }
     // The form is `egress-proxy --socket <path> --allow <list>`, in that order.

crates/toolkit/tests/egress_form.rs

@@ -0,0 +1,42 @@
+//! `toolkit egress-proxy` takes exactly `--socket <path> --allow <list>`: with anything after
+//! them it is not the proxy, and exits 2 at once rather than listening (M3b review). Do not edit.
+
+use std::process::{Command, Stdio};
+use std::time::{Duration, Instant};
+
+#[test]
+fn trailing_arguments_are_not_the_proxy_form() {
+    let dir = std::env::temp_dir().join(format!("tk-egress-form-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let socket = dir.join("egress.sock");
+    let mut child = Command::new(env!("CARGO_BIN_EXE_toolkit"))
+        .args([
+            "egress-proxy",
+            "--socket",
+            socket.to_str().unwrap(),
+            "--allow",
+            "example.com",
+            "extra",
+        ])
+        .stdin(Stdio::null())
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .spawn()
+        .unwrap();
+    let until = Instant::now() + Duration::from_secs(3);
+    let status = loop {
+        if let Some(status) = child.try_wait().unwrap() {
+            break status;
+        }
+        if Instant::now() > until {
+            let _ = child.kill();
+            let _ = child.wait();
+            panic!("it is listening: trailing arguments were accepted");
+        }
+        std::thread::sleep(Duration::from_millis(20));
+    };
+    assert_eq!(status.code(), Some(2));
+    assert!(!socket.exists(), "no socket was made");
+    let _ = std::fs::remove_dir_all(&dir);
+}

docs/implementer-log.md

@@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone.
 
 | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model |
 |---|---|---|---|---|---|---|---|
+| M3b/17-toolkit-nits | 2026-09-23 | done | 1 | pass | none | Three small fixes. `fetch.rs`: replaced `std::thread::spawn` with a `Builder::new().spawn` match that kills and waits on a spawn error and returns `Outcome::tool_error("http_fetch: cannot start a thread: {e}")`. `input.rs` and `files.rs`: replaced `MAX_INPUT as u64 + 1` / `MAX_READ as u64 + 1` with `u64::try_from(MAX_*).map_or(u64::MAX, |n| n.saturating_add(1))`. `main.rs` `parse_egress_proxy`: the first check is now `args.len() != 5 || args.first()?.as_bytes() != b"egress-proxy"` so a longer list goes to the tool form (exit 2). Copied `tests/egress_form.rs`; the one test failed before the fix and passed in 0.02s after. `grep "thread::spawn\| as u64" crates/toolkit/src/` prints nothing. `make gate` prints `gate: ok` first run. | ? |
 | M3b/16-brokerd-log-escaping | 2026-09-23 | done | 1 | pass | none | Escaped the container's standard error before logging it (M3b review finding 5) and prefixed/quoted three small texts (finding 7). `container.rs` `answer`: exit 2 logs `brokerd: {name}: the tool could not run: {err:?}` instead of the raw stderr; exit 125..=127 logs `brokerd: podman could not start {name}: {err:?}\n{RUNBOOK}` (was `{err}\n{RUNBOOK}`); the `_` arm logs `brokerd: container {name} exited {status}: {err:?}` (the trailing `\n{err}` moved inside the debug format). `start_egress`: the proxy's `podman run -d` failure now logs `{stderr:?}`. `main.rs`: the runtime notice prints `brokerd: {runtime_notice}`. `config.rs`: the image and memory `[runner]` errors use `{:?}` so the bad value is quoted. Copied `tests/container_log.rs` and `tests/notices.rs`; 4, 2, 2 and 7 passed; `make gate` prints `gate: ok` first run. | ? |
 | M3b/15-brokerd-pipes-grace | 2026-09-23 | done | 1 | pass | none | Fixed the two M3b review findings in `pipes.rs`. Added `pub const GRACE: Duration = Duration::from_secs(2)`; replaced the `JoinHandle`-holding `Io` with one holding `Option<Receiver<(Vec<u8>, bool)>>` for stdout/stderr and added the `Finished { out, truncated, err, open }` struct. `Io::start` now returns `std::io::Result<Io>`: the stdin writer is started with `Builder::new().spawn(...)` and its handle dropped (never joined, a spawn error returned with `?`); a new private `reader(pipe, cap) -> io::Result<Receiver<...>>` starts one detached reader per pipe and returns a spawn error, used for both stdout (`cap`) and stderr (`err_cap`). `Io::finish(grace)` sets one `until = Instant::now() + grace` and calls a private `collect(rx, until)` per receiver: `recv_timeout(until.saturating_duration_since(now))` → `Ok` keeps bytes, `Timeout` sets `open`, `Disconnected` (panicked reader) counts closed; `open` is `stdout_open || stderr_open`. `read_capped` unchanged in behavior, now `cap.saturating_sub(kept.len())` and `chunk.get(..take).unwrap_or_default()` (no indexing, no wrapping subtraction). `container.rs`: added `pub const OUTPUT_OPEN` after `TIMED_OUT`, `use crate::pipes::{GRACE, Io}`, and replaced the `run_container` tail with the task's `Io::start` match (on spawn error, `podman kill`/`rm -f`, `child.kill()`/`wait()`, log + `Unavailable(CANNOT_START)`), then `io.finish(GRACE)` and the `done.open && status.is_some()` check returning `Failed(OUTPUT_OPEN)` (a timed-out call, `status` None, still answers `TIMED_OUT`). All `std::thread::spawn` gone from `crates/brokerd/src/`. Copied `tests/container_grace.rs`; 4 passed in ~4.4s, container 11, container_egress 6, all five runs; `make gate` prints `gate: ok` first run. | ? |
 | M3b/14-brokerd-pipes-module | 2026-09-23 | done | 1 | pass | none | Pure move: cut `struct Io`, `impl Io { start, finish }`, and `fn read_capped` from `container.rs` and pasted them into `crates/brokerd/src/pipes.rs` with bodies unchanged and `pub(crate)` visibility; `Io::start` now calls `crate::container::STDERR_KEPT` (the constant stays in `container.rs` since the tests import it). Added the module doc comment and the `use std::io::{Read, Write}`, `use std::process::Child`, `use std::thread::JoinHandle` lines. `lib.rs` gained `pub mod pipes;` between `ledger` and `podman`. `container.rs` gained `use crate::pipes::Io;` and lost the `Read, Write` and `JoinHandle` imports the compiler reported unused; nothing else changed. `cargo fmt --all` reflowed the `STDERR_KEPT` call line in `start` to wrap. `cargo check`/clippy clean; container 11, container_egress 6, serve_runner 2 pass; `grep "struct Io\|fn read_capped" container.rs` prints nothing and `container.rs` is 350 lines. `make gate` prints `gate: ok` on the first run. | ? |