From 02c0b8b2538511a1152d49a147407ca45c239fdc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:56:07 +0000 Subject: [PATCH] Update shared instruction blocks to 0.1.4 Applied from aicers/agent-instructions at 0.1.4, which this repository's drift check is now pinned to. Do not edit the marked blocks here; change them upstream and cut a release. --- .agents/instructions.toml | 2 +- AGENTS.md | 76 +++++++++++++++++++++++++++++---------- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/.agents/instructions.toml b/.agents/instructions.toml index ed64752..c5836db 100644 --- a/.agents/instructions.toml +++ b/.agents/instructions.toml @@ -2,5 +2,5 @@ # it is on, and the shared blocks it has. Its drift check reads this file, # and its apply job rewrites it. Which blocks a repository should carry is # decided upstream in repos.json, not here. -ref = "0.1.1" +ref = "0.1.4" blocks = ["workflow", "rust", "changelog"] diff --git a/AGENTS.md b/AGENTS.md index 3788173..a1cfac2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,9 +79,11 @@ re-run its sync. The drift check in CI fails if this copy diverges. ### Errors and panics -- **Error types**: Use `anyhow::Result` for application code and - `thiserror` for library code — that is, use `thiserror` when a caller - needs to match on the error kind, `anyhow` otherwise. +- **Error types**: Use `thiserror` where a caller needs to match on the + error kind, `anyhow` otherwise. Application and library are the usual + shorthand for that split, not the rule itself: a binary whose layers + pick a recovery strategy from the error still needs typed errors, and + a library reaching an application boundary may use `anyhow`. - **Context**: Attach context to every fallible call that crosses a meaningful boundary: `.with_context(|| format!("reading config from {path}"))`. State what was being attempted and on which concrete target @@ -169,6 +171,16 @@ Where the crate has async code: - **No orphan tasks**: Do not discard the `JoinHandle` returned by `tokio::spawn`. Hold it, or use a `JoinSet`, and cancel outstanding tasks on shutdown. A dropped handle turns a task failure into silence. + - Dropping a `JoinSet` aborts its async tasks at whichever `.await` + point each has reached. Locals are dropped, so `Drop` runs and an + RAII guard still releases; what is lost is the rest of the body, + which is everything that had to be awaited — a flush, a commit, a + goodbye frame — along with the task's result. None of it reaches a + `spawn_blocking` task: abort may prevent one that has not started, + and cannot stop one that has, so a blocking task needs its own way + of being told to stop. Where shutdown must be graceful, signal the + tasks — a cancellation token, a closed channel — and `join_next` + until the set drains. Do not let the set's `Drop` be the shutdown. - **No locks across `.await`**: Never hold a `std::sync::Mutex`/`RwLock` guard across an `.await` point (`clippy::await_holding_lock`). Use `tokio::sync` primitives, or scope the guard so it is dropped first. @@ -194,6 +206,22 @@ Where the crate has async code: - **Atomic writes**: Write state, config, and other files that another process may read atomically — write to a temporary file in the same directory, then `fs::rename`. Never truncate-and-write in place. + - The temporary file is created with the permissions the finished + file needs, by the rule below. `rename` puts the temporary file's + inode in place, so the destination ends up with whatever mode the + temporary had and not the mode of the file it replaced. Which + mode that is depends on how the temporary was made — `OpenOptions` + leaves it to the umask, `tempfile` defaults to `0o600` — and + neither is a decision anyone made about this file. The write meant + to preserve the file is what changes it. + - Atomic replacement is not durability. The rename either happens or + does not, but neither it nor the bytes before it are on disk until + they are flushed. Where the file has to survive a crash or a power + loss — anything the program reads back to resume from — `sync_all` + the temporary file before the rename, then open the containing + directory and `sync_all` that too. This costs a disk round trip + each time, so it is a decision per file rather than a default: + where a write takes it, say in a comment what is being protected. - **Restrictive permissions at creation**: A file holding a secret gets its final permissions as it is created, never afterwards — `set_permissions` once the bytes are on disk leaves a window in which @@ -245,15 +273,18 @@ Where the crate verifies certificates: Where the crate handles key material or secrets: - Compare secrets, tokens, MACs, and certificate fingerprints in - constant time — `ring::constant_time::verify_slices_are_equal` in a - crate that already depends on `ring`. Never `==`: the derived - `PartialEq` on a secret-bearing type is a timing oracle. + constant time — `constant_time::verify_slices_are_equal`, from + whichever crypto stack the crate already depends on; `ring` and + `aws-lc-rs` both spell it that way. Never `==`: the derived + `PartialEq` on a secret-bearing type is a timing oracle. A crate + with no such stack does not grow a hand-written loop instead — + nothing stops the compiler turning one back into an early return. - Draw key material, and any value whose security rests on being unguessable (session identifiers, API keys, opaque bearer tokens), - from a cryptographically secure source (`ring::rand::SystemRandom`) - — never from a general-purpose PRNG, a timestamp, or a process ID. - A signed token such as a JWT is not drawn this way at all: its - strength comes from the signing key, which is key material. + from a cryptographically secure source — the same stack's + `rand::SystemRandom`. Never a general-purpose PRNG, a timestamp, or + a process ID. A signed token such as a JWT is not drawn this way at + all: its strength comes from the signing key, which is key material. - A nonce must meet whatever its construction documents, which is usually uniqueness under a given key rather than randomness. Counter and deterministically derived nonces are correct where the algorithm @@ -308,14 +339,23 @@ Where the crate handles key material or secrets: - Use `tempfile::tempdir()` for tests that need temporary files or directories. Never write to fixed paths. -- Do not mutate the process environment in tests. Pass the value in as a - parameter, or set it for a child process with `Command::env`. In - edition 2024 `env::set_var` is `unsafe` because another thread reading - the environment concurrently is undefined behaviour, and a `Mutex` - shared between tests does not establish otherwise: it serialises the - tests and says nothing about a runtime thread or a dependency reading - in the background. Where tests already do this, removing it is the - fix, not adding another lock. +- Do not mutate the process environment in tests with `env::set_var` or + `env::remove_var`. In Rust 2024 these are `unsafe`: outside Windows, + another thread reading the environment concurrently can cause + undefined behaviour. A `Mutex` shared between tests does not make the + call sound — it serialises only the code that takes the lock, not a + runtime thread or a dependency reading in the background. Where a + test already mutates the environment, remove the mutation rather than + adding another lock. +- Test environment-dependent behaviour by passing the value in — as a + parameter, a configuration map, or a resolver the test substitutes. + For a child process, set its environment with `Command::env`, + `Command::env_remove`, or `Command::env_clear`, rather than changing + this process's. +- Keep `env::var` at a thin composition boundary: read the values once + and pass them to the logic underneath. Test that boundary through a + child process when an exact environment must be asserted; logic that + receives its values needs no environment at all. - Do not synchronise with `sleep`. Await the condition, or use `tokio::time` pause/advance with the `test-util` feature. - Do not hard-code port numbers; bind port 0 and read back the assigned