Files
boxmaker/docs/plans/M5a/18-repeated-code.md
T
kyleandClaude Opus 5.5 64190c3701 M5a plan: task 16, the gate passes on macOS; the review's tasks become 17 to 20
The work moved to titan (macOS), where the gate failed for reasons older
than M5a: sockets under macOS's long TMPDIR pass its 104-byte limit,
gatewayd reads /proc/self for its uid, and /tmp is /private/tmp. Task 16
sets TMPDIR=/tmp for every process Cargo runs (.cargo/config.toml [env]),
has gatewayd take its uid from the owner of a pipe it makes (no dependency,
no unsafe), and hands over a toolkit shell.rs that compares resolved paths.
On a scratch tree the gate passes on titan and on talos with 936 tests, and
a wrong uid fails 7 gatewayd tests. Tasks 16 to 19 are renumbered 17 to 20.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-25 16:41:44 -07:00

7.7 KiB

M5a task 18: the same few lines written once, not four times

Branch: m5a (run git switch m5a; git status --short must be empty, otherwise stop) Commit subject: memoryd, proto, brokerd, bxctl: repeated error lines written once

Goal

No behaviour change. In four places the same lines are written out again and again. Each one becomes a small helper, so a later change to a sentence or a check is made once. Found in the M5a review (2026-09-24).

File Repeated Becomes
crates/memoryd/src/edit.rs the stale and "named twice" check, 4 times in consolidate a function claim
crates/proto/src/memory.rs if let Some(problem) = entry_problem(..) { return Err(problem); }, 9 times in MemoryArgs::check a function entry
crates/brokerd/src/builtin.rs the GrantProblem, twice in grants a closure problem
crates/bxctl/src/memory.rs cannot reach memoryd at …, 4 times in run a closure unreachable

Files

  • Modify: the four files in the table, docs/implementer-log.md
  • No test changes: the existing tests cover every line changed.

The edits, exactly

Nothing else in these files changes. Leave the comments that are there as they are.

1. crates/memoryd/src/edit.rs

a. Insert this function immediately above the line fn consolidate(, with one blank line between them:

/// Mark the entry `old` names as named: stale if `before` has no such entry, an error if it was
/// named already.
fn claim(
    file: MemoryFile,
    before: &[String],
    named: &mut HashSet<usize>,
    old: &str,
) -> Result<(), String> {
    let pos = position(before, old).ok_or_else(|| stale(file))?;
    if named.insert(pos) {
        Ok(())
    } else {
        Err(format!(
            "consolidate names an entry of {} twice",
            file.name()
        ))
    }
}

b. In consolidate, this block appears four times, in the Keep, Rewrite, Move and Drop arms:

                let pos = position(before, &old).ok_or_else(|| stale(file))?;
                if !named.insert(pos) {
                    return Err(format!(
                        "consolidate names an entry of {} twice",
                        file.name()
                    ));
                }

Replace each of the four with the one line

                claim(file, before, &mut named, &old)?;

The line after it in each arm (after.push(..) or out.push(..)) stays. The New arm does not have the block and does not change.

2. crates/proto/src/memory.rs

a. Insert this function immediately above the line impl RecallArgs {, with one blank line between them:

/// `entry_problem` as a `Result`, so a check can use `?`.
fn entry(text: &str, max: usize) -> Result<(), String> {
    match entry_problem(text, max) {
        Some(problem) => Err(problem),
        None => Ok(()),
    }
}

b. In impl MemoryArgs, replace the whole function pub fn check(&self) -> Result<(), String> (from that line to its closing }, the Ok(()) at its end included) with the following. Keep the doc comment above it.

    pub fn check(&self) -> Result<(), String> {
        match self {
            MemoryArgs::Add { text, .. } => entry(text, MAX_ENTRY),
            MemoryArgs::Replace { old, text, .. } => {
                entry(old, MAX_ENTRY)?;
                entry(text, MAX_ENTRY)
            }
            MemoryArgs::Remove { old, .. } => entry(old, MAX_ENTRY),
            MemoryArgs::Note { text } => entry(text, MAX_NOTE),
            MemoryArgs::Consolidate { entries, .. } => {
                if entries.is_empty() {
                    return Err("consolidate needs every entry of the file".to_string());
                }
                for line in entries {
                    match line.action().map_err(str::to_string)? {
                        EntryAction::Keep(old)
                        | EntryAction::Move(old)
                        | EntryAction::Drop(old) => entry(&old, MAX_ENTRY)?,
                        EntryAction::Rewrite { old, text } => {
                            entry(&old, MAX_ENTRY)?;
                            entry(&text, MAX_ENTRY)?;
                        }
                        EntryAction::New(text) => entry(&text, MAX_ENTRY)?,
                    }
                }
                Ok(())
            }
        }
    }

RecallArgs::check does not change.

3. crates/brokerd/src/builtin.rs

In grants, inside the for loop, the lines

        let grant = toml::from_str::<Grant>(text).map_err(|error| GrantProblem {
            file: id.to_string(),
            line: None,
            problem: error.to_string(),
        })?;
        let sha256 = sha256(text.as_bytes()).map_err(|error| GrantProblem {
            file: id.to_string(),
            line: None,
            problem: error.to_string(),
        })?;

become

        let problem = |error: String| GrantProblem {
            file: id.to_string(),
            line: None,
            problem: error,
        };
        let grant = toml::from_str::<Grant>(text).map_err(|e| problem(e.to_string()))?;
        let sha256 = sha256(text.as_bytes()).map_err(|e| problem(e.to_string()))?;

4. crates/bxctl/src/memory.rs

a. In run, the first lines of the body

    let mut stream = UnixStream::connect(socket)
        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
    stream
        .set_read_timeout(Some(timeout))
        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
    stream
        .set_write_timeout(Some(timeout))
        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;

become

    let unreachable =
        |e: &dyn std::fmt::Display| format!("cannot reach memoryd at {}: {e}", socket.display());
    let mut stream = UnixStream::connect(socket).map_err(|e| unreachable(&e))?;
    stream
        .set_read_timeout(Some(timeout))
        .map_err(|e| unreachable(&e))?;
    stream
        .set_write_timeout(Some(timeout))
        .map_err(|e| unreachable(&e))?;

b. Further down in run, the lines

    write_frame(&mut stream, &env)
        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;

become

    write_frame(&mut stream, &env).map_err(|e| unreachable(&e))?;

The memoryd did not answer line after it does not change: it is a different sentence.

Steps

  • 1. Make the edits, one file at a time, running cargo check -p <crate> after each (memoryd, proto, brokerd, bxctl). Then run cargo fmt --all.
  • 2. Check the repeats are gone. Each of these prints the number shown:
    • grep -c 'named.insert' crates/memoryd/src/edit.rs prints 1
    • grep -c 'if let Some(problem) = entry_problem' crates/proto/src/memory.rs prints 0
    • grep -c 'GrantProblem {' crates/brokerd/src/builtin.rs prints 2 (the closure, and the comment above the loop)
    • grep -c 'cannot reach memoryd' crates/bxctl/src/memory.rs prints 1
  • 3. Run the tests. cargo test -p memoryd -p proto -p brokerd -p bxctl: every suite passes.
  • 4. Run the gate. make gate. Expected last line: gate: ok, with 936 tests, the same as before this task.
  • 5. Log and commit. git add crates/memoryd crates/proto crates/brokerd crates/bxctl docs/implementer-log.md && git commit

Done when

  • make gate prints gate: ok, and step 2's four counts are as shown.

Stop and report if

  • Any block quoted above is not in the file exactly as shown, or appears a different number of times than the task says.
  • Any test fails. This task changes no behaviour, so a failure means an edit is wrong; do not change a test.