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>
This commit is contained in:
@@ -0,0 +1,228 @@
|
||||
# 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:
|
||||
|
||||
```rust
|
||||
/// 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:
|
||||
|
||||
```rust
|
||||
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
|
||||
|
||||
```rust
|
||||
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:
|
||||
|
||||
```rust
|
||||
/// `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.
|
||||
|
||||
```rust
|
||||
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
|
||||
|
||||
```rust
|
||||
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
|
||||
|
||||
```rust
|
||||
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
|
||||
|
||||
```rust
|
||||
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
|
||||
|
||||
```rust
|
||||
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
|
||||
|
||||
```rust
|
||||
write_frame(&mut stream, &env)
|
||||
.map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
|
||||
```
|
||||
|
||||
become
|
||||
|
||||
```rust
|
||||
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.
|
||||
Reference in New Issue
Block a user