diff --git a/docs/plans/M5a/16-macos-gate.md b/docs/plans/M5a/16-macos-gate.md new file mode 100644 index 0000000..3e5da9c --- /dev/null +++ b/docs/plans/M5a/16-macos-gate.md @@ -0,0 +1,112 @@ +# M5a task 16: the gate passes on macOS as well as on Linux + +**Branch:** `m5a` (run `git switch m5a`; `git status --short` must be empty, otherwise stop) +**Commit subject:** `gate: pass on macOS: tests use /tmp, gatewayd finds its uid without /proc` + +## Goal + +Work moved to titan, a Mac, on 2026-09-24. There `make gate` fails although the code is right, +because three places assume Linux. The harness itself still runs on straylight (Linux); this +task only makes the gate pass on both. No behaviour change on Linux, and no new test. + +1. **Unix socket paths are too long.** Tests put sockets under `std::env::temp_dir()`. On macOS + that is `/var/folders/…/T/`, about 50 bytes, and a socket's whole path may be at most 104 + bytes there, so some tests' daemons cannot bind (for example brokerd's + `serve_runner.rs` and gatewayd's `serve_restart.rs`). A new `.cargo/config.toml` sets + `TMPDIR=/tmp` for every process Cargo runs, tests included. +2. **`gatewayd` reads `/proc/self`** for the user id it runs as, when it checks a secret file's + owner. macOS has no `/proc`. `std` has no call for the user id, and `unsafe` (for `libc`) is + not allowed, so a new function `user_id` takes the owner of a pipe the process has just made, + which is the user the process runs as on Linux and on macOS (checked on both: 1000 on talos, + 501 on titan, the same as `id -u`). +3. **`toolkit`'s `shell.rs` test** expects `pwd` to print `/tmp`; on macOS `/tmp` is a link to + `/private/tmp`, and `pwd` prints the resolved path. A new version of that given test compares + with the resolved path. + +## Files + +- Create: `.cargo/config.toml` (at the repository root, next to `Cargo.toml`) +- Copy: a **new version** of the given `crates/toolkit/tests/shell.rs`, from + `docs/plans/M5a/files/crates/toolkit/tests/shell.rs` +- Modify: `crates/gatewayd/src/secrets.rs`, `docs/implementer-log.md` + +## The changes, exactly + +**1. Create `.cargo/config.toml`** with exactly this content: + +```toml +# Tests put Unix sockets under `std::env::temp_dir()`. macOS's is long (/var/folders/…/T/), and a +# socket's path there can pass the 104 bytes macOS allows, so every process Cargo runs, tests +# included, uses /tmp. +[env] +TMPDIR = { value = "/tmp", force = true } +``` + +**2. `crates/gatewayd/src/secrets.rs`**, in `read_checked`. Nothing else in the file changes. + +**a.** In the comment, the line + +```rust + // on `named`): the uid of "/proc/self"; " is not owned by the user gatewayd runs as"; +``` + +becomes + +```rust + // on `named`): the uid of `user_id()`; " is not owned by the user gatewayd runs as"; +``` + +**b.** The three lines + +```rust + let owner = std::fs::metadata("/proc/self") + .map_err(|e| format!("cannot read /proc/self: {}", e))? + .uid(); +``` + +become the one line + +```rust + let owner = user_id()?; +``` + +**c.** Insert this function immediately above the line +`/// The text without one trailing newline; not empty; UTF-8.` (the doc comment of `value`), with +one blank line between them: + +```rust +/// The user id this process runs as: the owner of a pipe it has just made. `std` has no call for +/// it, and `/proc/self` is Linux only. +fn user_id() -> Result { + let (reader, _writer) = std::io::pipe().map_err(|e| format!("cannot make a pipe: {e}"))?; + let pipe = std::fs::File::from(std::os::fd::OwnedFd::from(reader)); + let meta = pipe + .metadata() + .map_err(|e| format!("cannot read a pipe: {e}"))?; + Ok(meta.uid()) +} +``` + +No `use` line changes: `MetadataExt` (for `.uid()`) is already imported, and the rest is named in +full. + +## Steps + +- [ ] **1. Create `.cargo/config.toml`**, then copy the test: + `cp docs/plans/M5a/files/crates/toolkit/tests/shell.rs crates/toolkit/tests/` +- [ ] **2. Make the three edits to `secrets.rs`.** Run `cargo fmt --all`. +- [ ] **3. Run the tests.** `cargo test -p gatewayd -p toolkit -p brokerd`: every suite passes. +- [ ] **4. Run the gate.** `make gate`. Expected last line: `gate: ok`, with 936 tests, the same + as before this task. On macOS it failed before this task, with 8 tests failing; now it passes. +- [ ] **5. Log and commit.** + `git add .cargo/config.toml crates/gatewayd crates/toolkit docs/implementer-log.md && git commit` + +## Done when + +- `make gate` prints `gate: ok` with 936 tests. + +## Stop and report if + +- Any line quoted above is not in the file exactly as shown. +- `cargo-deny` is missing (`error: no such command: deny`): report it, do not install anything. +- Any test fails after the changes. Do not change a test. diff --git a/docs/plans/M5a/16-memoryd-no-slicing.md b/docs/plans/M5a/17-memoryd-no-slicing.md similarity index 97% rename from docs/plans/M5a/16-memoryd-no-slicing.md rename to docs/plans/M5a/17-memoryd-no-slicing.md index 6a0be82..8a0774f 100644 --- a/docs/plans/M5a/16-memoryd-no-slicing.md +++ b/docs/plans/M5a/17-memoryd-no-slicing.md @@ -1,4 +1,4 @@ -# M5a task 16: `memoryd` takes the day and the month with `get`, not by slicing +# M5a task 17: `memoryd` takes the day and the month with `get`, not by slicing **Branch:** `m5a` (run `git switch m5a`; `git status --short` must be empty, otherwise stop) **Commit subject:** `memoryd: the day and the month of a time with get, never a slice` diff --git a/docs/plans/M5a/17-repeated-code.md b/docs/plans/M5a/18-repeated-code.md similarity index 99% rename from docs/plans/M5a/17-repeated-code.md rename to docs/plans/M5a/18-repeated-code.md index a61304f..da75c83 100644 --- a/docs/plans/M5a/17-repeated-code.md +++ b/docs/plans/M5a/18-repeated-code.md @@ -1,4 +1,4 @@ -# M5a task 17: the same few lines written once, not four times +# 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` diff --git a/docs/plans/M5a/18-no-forged-headers.md b/docs/plans/M5a/19-no-forged-headers.md similarity index 98% rename from docs/plans/M5a/18-no-forged-headers.md rename to docs/plans/M5a/19-no-forged-headers.md index 401ab2c..1a2b270 100644 --- a/docs/plans/M5a/18-no-forged-headers.md +++ b/docs/plans/M5a/19-no-forged-headers.md @@ -1,4 +1,4 @@ -# M5a task 18: a note cannot forge a header in `notes.md` +# M5a task 19: a note cannot forge a header in `notes.md` **Branch:** `m5a` (run `git switch m5a`; `git status --short` must be empty, otherwise stop) **Commit subject:** `proto, memoryd: no line of an entry may start with §, and headers are exact` diff --git a/docs/plans/M5a/19-review-ids.md b/docs/plans/M5a/20-review-ids.md similarity index 98% rename from docs/plans/M5a/19-review-ids.md rename to docs/plans/M5a/20-review-ids.md index 34f2cef..57547b3 100644 --- a/docs/plans/M5a/19-review-ids.md +++ b/docs/plans/M5a/20-review-ids.md @@ -1,4 +1,4 @@ -# M5a task 19: a staged write never reuses an id +# M5a task 20: a staged write never reuses an id **Branch:** `m5a` (run `git switch m5a`; `git status --short` must be empty, otherwise stop) **Commit subject:** `memoryd: a staged write's id counts every write staged that day` diff --git a/docs/plans/M5a/README.md b/docs/plans/M5a/README.md index 6f0d63e..948871e 100644 --- a/docs/plans/M5a/README.md +++ b/docs/plans/M5a/README.md @@ -1,7 +1,7 @@ # M5a implementation plan: local memory > **For the implementing model:** do not work from this file. The owner gives you one task file at -> a time (`01-…` to `19-…`). This file is the index for the owner and the reviewer. +> a time (`01-…` to `20-…`). This file is the index for the owner and the reviewer. **Goal:** a new role, `memoryd`, owns the agent's memory: identity (`soul.md`, `memory.md`, `user.md`, in every new session's prompt) and experience (`notes.md` and the session logs, found @@ -64,13 +64,14 @@ the skeletons were cut (tip T25). | 13 | `13-loopd-memory.md` | the block, the core tools, `session_updated` | `memory.rs`; `baseline.rs`, `pointers.rs`, `channel.rs` updated | reference; 6 mutations caught | | 14 | `14-bxctl-memory.md` | `bxctl reindex`, `bxctl memory` | `cli_memory.rs` | reference; 5 mutations caught | | 15 | `15-soul-and-device.md` | the starter soul; device checks with `memoryd` | `device.rs` updated, `support/memoryd.rs` | reference; `make verify-device` on straylight | -| 16 | `16-memoryd-no-slicing.md` | the day and the month with `get`, not a slice (review nit) | the existing ones | reference | -| 17 | `17-repeated-code.md` | repeated error lines written once (review nit) | the existing ones | reference; 4 mutations caught | -| 18 | `18-no-forged-headers.md` | a note cannot forge a `notes.md` header (review finding) | `forgery.rs`; `memory_args.rs`, `notes.rs` updated | reference; 4 mutations caught | -| 19 | `19-review-ids.md` | a staged write never reuses an id (review finding) | `review_ids.rs` | reference; 2 mutations caught | +| 16 | `16-macos-gate.md` | the gate passes on macOS: tests use /tmp, `gatewayd` finds its uid without `/proc` | `shell.rs` updated | reference; gate on titan and talos; 1 mutation caught (7 tests) | +| 17 | `17-memoryd-no-slicing.md` | the day and the month with `get`, not a slice (review nit) | the existing ones | reference | +| 18 | `18-repeated-code.md` | repeated error lines written once (review nit) | the existing ones | reference; 4 mutations caught | +| 19 | `19-no-forged-headers.md` | a note cannot forge a `notes.md` header (review finding) | `forgery.rs`; `memory_args.rs`, `notes.rs` updated | reference; 4 mutations caught | +| 20 | `20-review-ids.md` | a staged write never reuses an id (review finding) | `review_ids.rs` | reference; 2 mutations caught | -At the end of task 15: 936 tests (854 before task 01); tasks 16 and 17 change no behaviour and -keep 936; at the end of task 18, 939; at the end of task 19, 941. Each task's count was checked by running +At the end of task 15: 936 tests (854 before task 01); tasks 16 to 18 add no test and +keep 936; at the end of task 19, 939; at the end of task 20, 941. Each task's count was checked by running the gate at its end state on its own. ## Changes during the run @@ -92,14 +93,18 @@ the gate at its end state on its own. are rustfmt-clean. The attempt is saved in `.state/runs/M5a/04-attempt-2/`. Resume from task 04. The measurements behind this are in `docs/research/2026-09-24-chat-template-tokens.md`, added after the run. -- 2026-09-24, after the review: tasks 16 and 17 clean up the review's nits. Task 18 closes a hole +- 2026-09-24, after the review: tasks 17 and 18 clean up the review's nits. Task 19 closes a hole the review found in the spec and the reference alike: a note's text could hold a line shaped like a `notes.md` header, so an untrusted session could plant a `moved from memory.md` entry that - `recall` showed as identity. The spec was changed (`4ae827a`, section 3). Since task 18 hands + `recall` showed as identity. The spec was changed (`4ae827a`, section 3). Since task 19 hands over new versions of two given tests, task 02's `memory_args.rs` is now `memory_args.rs-task02` - and task 06's `notes.rs` is now `notes.rs-task06`. Task 19 fixes review ids: counting only the + and task 06's `notes.rs` is now `notes.rs-task06`. Task 20 fixes review ids: counting only the waiting writes gave a write staged after an accepted one the accepted one's id, and the write - was lost. Resume from task 16. + was lost. +- 2026-09-24, later: the work moved to titan (macOS) while talos's disk is replaced, and there + the gate failed for reasons older than M5a: socket paths under macOS's long `$TMPDIR`, + `gatewayd` reading `/proc/self`, and `/tmp` being `/private/tmp`. Task 16 makes it pass on + both; the review's tasks moved to 17 to 20. Resume from task 16. ## Running it diff --git a/docs/plans/M5a/files/crates/toolkit/tests/shell.rs b/docs/plans/M5a/files/crates/toolkit/tests/shell.rs new file mode 100644 index 0000000..4d6dd72 --- /dev/null +++ b/docs/plans/M5a/files/crates/toolkit/tests/shell.rs @@ -0,0 +1,91 @@ +//! `toolkit shell`, run as `brokerd` runs it, with the host's `/bin/sh`. Do not edit. + +mod support; + +use proto::tools::ShellArgs; +use support::{TempDir, json, toolkit}; +use toolkit::shell::MAX_OUTPUT; + +fn sh(command: &str, cwd: Option<&str>) -> support::Ran { + let args = ShellArgs { + command: command.to_string(), + cwd: cwd.map(str::to_string), + }; + toolkit(&["shell"], &json(&args)) +} + +#[test] +fn output_and_errors_come_back_in_order_then_the_exit_status() { + let ran = sh("echo one; echo two >&2; echo three", None); + assert_eq!(ran.code, 0); + assert_eq!(ran.stdout, "one\ntwo\nthree\n\n[exit 0]"); +} + +#[test] +fn a_failing_command_is_still_a_result_with_its_status() { + let ran = sh("echo nope; exit 3", None); + assert_eq!((ran.code, ran.stdout.as_str()), (0, "nope\n\n[exit 3]")); + let ran = sh("true", None); + assert_eq!(ran.stdout, "\n[exit 0]"); +} + +#[test] +fn a_command_killed_by_a_signal_says_so() { + let ran = sh("kill -9 $$", None); + assert_eq!( + (ran.code, ran.stdout.as_str()), + (0, "\n[killed by signal 9]") + ); +} + +#[test] +fn the_command_runs_in_cwd_or_in_tmp() { + let dir = TempDir::new("cwd"); + let here = dir.path().to_str().unwrap(); + // `pwd` prints the path with symbolic links resolved: on macOS, /tmp is /private/tmp. + let real = |path: &str| std::fs::canonicalize(path).unwrap().display().to_string(); + let ran = sh("pwd", Some(here)); + assert_eq!(ran.stdout, format!("{}\n\n[exit 0]", real(here))); + let ran = sh("pwd", None); + assert_eq!(ran.stdout, format!("{}\n\n[exit 0]", real("/tmp"))); +} + +#[test] +fn a_missing_cwd_is_exit_1() { + let ran = sh("pwd", Some("/no/such/dir")); + assert_eq!( + (ran.code, ran.stdout.as_str()), + (1, "shell: /no/such/dir: no such directory") + ); +} + +#[test] +fn standard_input_is_empty() { + let ran = sh("cat; echo done", None); + assert_eq!(ran.stdout, "done\n\n[exit 0]"); +} + +#[test] +fn output_past_the_limit_is_dropped_and_the_command_still_finishes() { + let ran = sh( + &format!( + "head -c {} /dev/zero | tr '\\0' a; echo; echo end >&2; exit 4", + MAX_OUTPUT + 5000 + ), + None, + ); + assert_eq!(ran.code, 0); + let expected_tail = format!("\n[output after {MAX_OUTPUT} bytes dropped]\n[exit 4]"); + assert!( + ran.stdout.ends_with(&expected_tail), + "{}", + &ran.stdout[ran.stdout.len() - 80..] + ); + assert_eq!(ran.stdout.len(), MAX_OUTPUT + expected_tail.len()); +} + +#[test] +fn output_that_is_not_utf8_is_replaced_not_refused() { + let ran = sh("printf 'a\\377b'", None); + assert_eq!(ran.stdout, "a\u{fffd}b\n[exit 0]"); +}