Plan 002: Stop the relay from panicking on non-ASCII request IDs

[duyetbot]

Plan 002: Stop the relay from panicking on non-ASCII request IDs

Executor instructions: Follow this plan step by step. Run every
verification command and confirm the expected result before moving to the
next step. If anything in the "STOP conditions" section occurs, stop and
report — do not improvise. When done, update the status row for this plan
in plans/README.md — unless a reviewer dispatched you and told you they
maintain the index.

Drift check (run first): git diff --stat 61ee3c7..HEAD -- src/relay.rs
If src/relay.rs changed since this plan was written, compare the
"Current state" excerpts against the live code before proceeding; on a
mismatch, treat it as a STOP condition.

Status

• Priority: P1
• Effort: S
• Risk: LOW
• Depends on: none
• Category: bug
• Planned at: commit 61ee3c7, 2026-08-26

Why this matters

The relay names its worker threads by slicing the first bytes of the
cloud-provided request id. The cloud controls that string. If a frame ever
carries a multibyte UTF-8 character within the first 8 bytes (a hostile or
buggy peer can send anything), the byte-slice panics with
byte index N is not a char boundary. Because the release profile builds with
panic = "abort", one malformed frame kills the whole relay process — the
user's local-model bridge goes down until they notice and restart it.

Current state

• src/relay.rs — the relay module. In spawn_request (lines 831–843):

fn spawn_request(state: &ConnState, frame: RequestFrame, target: &'static str) {
    let cancel = Arc::new(AtomicBool::new(false));
    state
        .in_flight
        .lock()
        .unwrap()
        .insert(frame.id.clone(), Arc::clone(&cancel));
    let tx = state.tx.clone();
    std::thread::Builder::new()
        .name(format!("relay-req-{}", &frame.id[..frame.id.len().min(8)]))
        .spawn(move || handle_request(&tx, &frame, target, &cancel))
        .expect("spawn relay worker");
}

frame.id comes straight from the wire: parse_server_frame (lines 168–190)
accepts any JSON string for id.

Repo conventions: pure helpers get inline unit tests next to the code (see the
#[cfg(test)] mod tests block at the bottom of src/relay.rs, e.g.
utf8_flush_len_never_splits_a_codepoint). Match that style.

Commands you will need

Purpose	Command	Expected on success
Unit tests	cargo test --locked --lib relay	all pass incl. new test
Clippy on lib	cargo clippy --locked --all-targets	no NEW warnings vs baseline (13 pre-existing)

Scope

In scope:

• src/relay.rs

Out of scope:

• Any other module. Do not refactor parse_server_frame, the reconnect loop,
  or thread naming conventions elsewhere.
• No changes to the wire protocol.

Git workflow

• Branch: advisor/002-relay-id-char-boundary
• Commit style: conventional commits, e.g. fix(relay): never panic on multibyte request ids when naming worker threads
• Do NOT push or open a PR.

Steps

Step 1: Add a small helper that truncates on a char boundary

Add near spawn_request (or near the other small helpers like
utf8_flush_len):

/// First up-to-`max_chars` characters of `s`, safe on any input (the id is
/// cloud-controlled and only used for diagnostics).
fn short_id(s: &str, max_chars: usize) -> String {
    s.chars().take(max_chars).collect()
}

Step 2: Use it in spawn_request

Replace the .name(...) line:

.name(format!("relay-req-{}", short_id(&frame.id, 8)))

Leave everything else about spawn_request untouched (the in-flight map still
keys on the full id).

Verify: cargo build --locked → exit 0.

Step 3: Add unit tests

In src/relay.rs's existing mod tests, add:

#[test]
fn short_id_is_char_boundary_safe_and_bounded() {
    assert_eq!(short_id("abcdefghijklmnop", 8), "abcdefgh");
    // Multibyte inside the first 8 BYTES must not panic.
    assert_eq!(short_id("héllo-world", 4), "héll");
    assert_eq!(short_id("🚀🚀🚀", 2), "🚀🚀");
    assert_eq!(short_id("", 8), "");
}

Verify: cargo test --locked --lib relay → all pass including
short_id_is_char_boundary_safe_and_bounded.

Test plan

• New tests live in the existing mod tests in src/relay.rs (pattern:
  utf8_flush_len_never_splits_a_codepoint at src/relay.rs:1145).
• Coverage: ASCII truncation, multibyte-within-8-bytes, emoji (4-byte), empty.

Done criteria

ALL must hold:

• grep -n 'frame.id\[' src/relay.rs returns no matches (byte-slicing gone).
• grep -n 'chars().take' src/relay.rs finds the new helper usage path (helper + call site).
• cargo test --locked --lib relay exits 0 with the new test present.
• git status --porcelain shows only src/relay.rs (+ plans/README.md if you were told to update it).

STOP conditions

Stop and report if:

• The excerpt of spawn_request doesn't match the live file.
• You find OTHER untrusted-string byte-slices in relay.rs while editing — report them rather than fixing beyond scope (note them in your final message).

Maintenance notes

• If request ids later gain a documented format (e.g. always ASCII), this
  helper can shrink back to slicing — but keep it total; the peer is remote.
• Reviewer: check no other &str[..n] indexing on wire-derived strings was
  introduced.