From c41c67c9491fc74c018cf42732af839accd12392 Mon Sep 17 00:00:00 2001 From: iris Date: Tue, 8 Sep 2026 12:24:03 +0200 Subject: [PATCH] docs: fix genuine passive-voice hits in docs/process Fourth batch of hyperhive#4042's Passive pass (see #4098/#4099/#4100 for the first three and the read-every-hit discipline this pass uses). 44 hits across pr-review-gate.md (4), conventions.md (18), and gotchas.md (22) -- highest genuine-catch rate so far, 23/44 (~52%), because this architecture/mechanism documentation has a lot of "X does Y via Z" sentences where the actor is already named parenthetically or in a nearby clause -- the single most productive rewrite shape across every batch so far. Recurring rewrite shapes this batch: - Actor already named in the same sentence, just not as the grammatical subject: "X is configured per repo in its branch-protection settings" -> "Each repo's branch-protection settings configure X" (pr-review-gate.md); "the broker" (reserved names), "rsvg-convert" (PNG rendering), "the website repo" (HTML/CSS rendering), "systemd.globalEnvironment" (D-Bus address export), and several more -- all the same shape. - Subject already established one clause or one sentence earlier, just needs continuing rather than restarting with a new passive subject: "the harness reads HIVE_TOOL_GROUPS (...). Unrecognised tokens are logged and skipped." -> "...logging and skipping unrecognised tokens" (continues "the harness"), same pattern twice more (job_queue::templates::rebuild, HIVE_CAPABILITIES resolution). - Sibling-inconsistency: a bolded lead-in bullet was the one passive sentence in an otherwise-active paragraph/table (the read_host_journal capability row sat between two "may X" rows; the HTML+CSS bullet's own tail clauses were already active voice around the one passive lead phrase). - One caught-and-fixed authoring mistake worth noting for future passes: the first attempt at the "Nix treats X as a package" rewrite landed in the wrong sentence (a similarly-worded but unrelated passage two paragraphs up) -- caught by re-reading the diff before running vale, not by vale itself (which would have shown 0 remaining hits either way, since the intended sentence's hit just wouldn't have been touched -- a silently-wrong edit vale's own count can't catch). Re-reading the actual diff, not just trusting the before/ after hit count, is what caught it. 21 of 44 left alone -- same recurring legitimate shapes as prior batches (predicate-adjective copulas, quoted/literal text, generic- actor statements, negative-capability invariants, "is tracked/rooted/ scoped at X" property-description idioms, and two more thesis- statement headings matching the "Ownership is declared, not repaired" precedent from #4100). Verified: vale docs/process before/after -- 44 -> 21 write-good.Passive hits, exactly the 23 rewritten, re-read every changed line's full surrounding context after editing (not just the vale count) to catch exactly the kind of misplaced-edit mistake described above. --- docs/process/conventions.md | 34 +++++++++++++++++----------------- docs/process/gotchas.md | 33 ++++++++++++++++----------------- docs/process/pr-review-gate.md | 8 ++++---- 3 files changed, 37 insertions(+), 38 deletions(-) diff --git a/docs/process/conventions.md b/docs/process/conventions.md index e0d625b1..5de97651 100644 --- a/docs/process/conventions.md +++ b/docs/process/conventions.md @@ -77,7 +77,7 @@ namespace, not the wire surface. ## Recipient sentinels -A few recipient names are reserved by the broker and have special +The broker reserves a few recipient names, which have special meaning that ordinary agent labels can never collide with — agent name validation rejects any character outside `[a-z0-9_-]`, so the angle-bracket and asterisk shapes below are structurally safe. @@ -94,7 +94,7 @@ angle-bracket and asterisk shapes below are structurally safe. propagates with zero agent-side restart. - `` — fan-out to every direct descendant of the sender per `topology.json`. Resolved in `socket_server::handle_send` via - `topology::children_of(sender)`: one message is delivered to each + `topology::children_of(sender)`: it delivers one message to each child, bypassing the allow-list check (structural fan-out targets are never user-listed peers). No-op for leaf agents (returns `Ok` when the child set is empty). Lets a sub-manager nudge its subtree without @@ -211,8 +211,8 @@ Per-variant fields: and not counted. - `UnreadMatrix { rooms, summary }` — unread matrix notifications. Informational + not cancellable (clear with `mark_read`). Unlike - the others this is injected by the in-container harness, not - hive-c0re, because the matrix daemon lives inside the agent. + the others, the in-container harness injects this, not hive-c0re, + because the matrix daemon lives inside the agent. `age_seconds` saturates at zero on any clock anomaly (back-step, unsynchronised wall clock, etc.) so the bulleted list never @@ -299,8 +299,8 @@ JSON-line protocol with their own result types; RFC 9457 is an HTTP format. ## Tool groups -The MCP tool surface an agent receives is derived from a set of named -`ToolGroup` values (`hive_sh4re::permissions::ToolGroup`), not from a hardcoded +A set of named `ToolGroup` values (`hive_sh4re::permissions::ToolGroup`) +determines the MCP tool surface an agent receives, not a hardcoded binary flavor. | Group | Tools | @@ -316,9 +316,9 @@ binary flavor. | `forge` | `create_repo` — create git repos through hive-c0re (operator-gated merge) | | `web_tools` | none (gates the Claude built-ins `WebFetch`/`WebSearch`, not an MCP tool) | -**Always-on tools** — `set_status`, `compact`, and `mark_todos_done` are -exposed to every agent regardless of which groups it holds -(`ToolGroup::ALWAYS_ON_TOOLS`). The operator dashboard depends on every agent +**Always-on tools** — `ToolGroup::ALWAYS_ON_TOOLS` exposes `set_status`, +`compact`, and `mark_todos_done` to every agent regardless of which +groups it holds. The operator dashboard depends on every agent being able to report its status chip, and the server-side `SetStatus` handler has no tool-group check (only length validation), so gating it would only desync the `--allowedTools` list from what the host actually accepts. @@ -343,11 +343,11 @@ the new `HIVE_TOOL_GROUPS` env var. Agents with no entry get no var. **Runtime resolution** — at session start the harness reads `HIVE_TOOL_GROUPS` (a comma-separated list of snake_case group names injected by the meta renderer -from `tool-groups.json`). Unrecognised tokens are logged and skipped. Falls back +from `tool-groups.json`), logging and skipping unrecognised tokens. Falls back to `ToolGroup::AGENT_DEFAULT` (`messaging`, `meta`, `inbox`, `execution`) when the var is absent or empty. -**Updating the surface** — when a new `#[tool]` fn is added to `AgentServer` +**Updating the surface** — when you add a new `#[tool]` fn to `AgentServer` in `hive-agent-mcp/src/mcp/mod.rs`, add its name to the matching `ToolGroup::tools()` slice in `hive-sh4re/src/permissions.rs`. That's the single source of truth; `mcp_config::allowed_mcp_tools` (in `hive-agent/src/mcp_config.rs`) reads it at @@ -363,7 +363,7 @@ that allows the underlying resource access. | Capability | Effect | |---|---| | `manage_root_agent` | may lifecycle-manage the root/manager agent via `kill`/`start`/`restart` | -| `read_host_journal` | `get_host_journal` MCP tool is registered + `GET /journal-host` requests are served | +| `read_host_journal` | registers the `get_host_journal` MCP tool + serves `GET /journal-host` requests | | `query_agent_state` | may call `get_loose_ends` / `CountPendingReminders` targeting non-child agents | **Config storage** — per-agent capabilities live in @@ -381,8 +381,8 @@ After a change `meta::sync_agents` commits the updated file; the next agent rebuild picks up the new `HIVE_CAPABILITIES` env var. **Runtime resolution** — at session start the harness reads `HIVE_CAPABILITIES` -and resolves each token to a `Capability` variant. Unrecognised tokens are -logged and skipped. An absent or empty var means no extra capabilities. +and resolves each token to a `Capability` variant, logging and skipping +unrecognised ones. An absent or empty var means no extra capabilities. **Capability NOT configurable from `agent.nix`** — same reasoning as tool groups: an agent that could grant its own capabilities via a config commit would @@ -416,11 +416,11 @@ via `snapshotOpenDetails` / `restoreOpenDetails`. ## `rebuild` is the reconcile verb `job_queue::templates::rebuild` builds the DAG that reconciles a -container to its wanted state: `write_dropins` (the nspawn-conf +container to its wanted state: it folds `write_dropins` (the nspawn-conf rewrite — `PRIVATE_NETWORK=1`, `HOST_ADDRESS` = the bridge gateway IP, sets `EXTRA_NSPAWN_FLAGS` — plus the systemd resource-limits drop-in) -is folded into the `Swap` node, then `nixos-container update` + stop + -start runs across the `StopForUpdate → Swap → RebuildBookkeeping` +into the `Swap` node, then runs `nixos-container update` + stop + +start across the `StopForUpdate → Swap → RebuildBookkeeping` brace and the tail `Reconcile` node. `flake.nix` itself is no longer regenerated host-side on rebuild — it's tracked in the agent's proposed/applied repos and rides along on every fetch (see diff --git a/docs/process/gotchas.md b/docs/process/gotchas.md index 82bea946..b719c19c 100644 --- a/docs/process/gotchas.md +++ b/docs/process/gotchas.md @@ -91,7 +91,7 @@ Two things to get right when a daemon needs outbound TCP: works until DNS hands back the other family; - list `AF_NETLINK` too. glibc's `getaddrinfo` opens a netlink socket to enumerate local addresses before it returns any, so name resolution - fails without it even when `AF_INET` is allowed. + fails without it even when the unit allows `AF_INET`. **The directive is a claim about what the program does, and nothing re-checks it when the program changes.** A unit that only served a unix @@ -143,15 +143,15 @@ to options the operator writes: **any** option of type `package` fed a store-path _string_ coerces through `lib.toDerivation`, i.e. `builtins.storePath`. `environment.systemPackages` and `systemd.services..path` both do it (the latter takes plain -strings like `/run/wrappers` happily, but anything under -`builtins.storeDir` is treated as a package). A path handed to the +strings like `/run/wrappers` happily, but Nix treats anything under +`builtins.storeDir` as a package). A path handed to the container as text has to be wrapped in a real derivation — a symlink farm built from the interpolated string — before it can go anywhere a package is expected. The catch is that a path written into a generated flake is text, not a reference — the container's closure doesn't keep the binary alive. -The **host** does: the package is interpolated into +The **host** does: it interpolates the package into `/etc/hyperhive/serve.json`, so it lands in the host's system closure and is gc-rooted by the running generation. `builtins.toJSON` preserves string context, which is the load-bearing detail; discard the context @@ -204,14 +204,14 @@ the `TIME_WAIT` case from a clean previous exit; retry covers the genuine "previous process is still alive during a systemd restart overlap" case. REUSEADDR does **not** allow two simultaneous `LISTEN` sockets on the same port (that would be `SO_REUSEPORT`, -which we don't use) — exclusivity is preserved. +which we don't use) — it preserves exclusivity. ## Approvals ### Orphan approvals -If state dirs are wiped out from under a pending approval (test -scripts, manual `rm -rf`), the dashboard's next render marks them +If test scripts or a manual `rm -rf` wipe state dirs out from under a +pending approval, the dashboard's next render marks them `failed` with note `"agent state dir missing"` so they fall out of `pending`. They stay in sqlite for audit. @@ -323,7 +323,7 @@ rust build's `src` was the whole repo tree, any tweak to invalidated the cargo cache and forced a full rebuild. crane (and naersk before it) couldn't see "these inputs are unused by rust" on its own — the split breaks the coupling at the derivation boundary. -The agent-configs PNG is rendered from the SVG via `rsvg-convert` at +`rsvg-convert` renders the agent-configs PNG from the SVG at build time; librsvg dependency lives here, not in the rust derivation's `nativeBuildInputs`. @@ -400,10 +400,9 @@ connects to the compositor at `127.0.0.1:`. = invisible window even though CDP works). The fix is **not** to wrap each client in its own `dbus-run-session` (a private throwaway bus per process — that's a _separate_ session, defeating the one-session - model); it's this one shared bus, whose address is exported as - `DBUS_SESSION_BUS_ADDRESS=unix:path=/run/gui/bus` via - `systemd.globalEnvironment` so weston, the harness and every GUI client - inherit it. + model); it's this one shared bus; `systemd.globalEnvironment` exports + its address as `DBUS_SESSION_BUS_ADDRESS=unix:path=/run/gui/bus` so + weston, the harness and every GUI client inherit it. - **Fixed Wayland socket name (`--socket=wayland-0`)**: weston is launched with `--socket=wayland-0` so the socket path is deterministic. `nix/agent-modules/weston-vnc.nix` exports `WAYLAND_DISPLAY=wayland-0` @@ -471,12 +470,12 @@ Pipeline: - CommonMark from `nixosOptionsDoc.optionsCommonMark` is the only output — the source of truth, emitted as `.md`. -- **HTML + CSS is rendered downstream by the website repo** - (its own `options.nix`), which consumes this bundle's `host.md` / - `agent.md`, renders them with `cmark-gfm`, and shares one +- **The website repo renders HTML + CSS downstream** + (its own `options.nix`), consuming this bundle's `host.md` / + `agent.md`, rendering them with `cmark-gfm`, and sharing one stylesheet (`docs.css`) across `/options/` and the prose `/docs/` tree. Keeping rendering in the website means the theme has a single - home and the colours are shared. + home, sharing colours across both trees. - `transformOptions` strips the nix-store prefix from option declaration paths and rewrites them as forge URLs, so the rendered docs link back to the source. @@ -512,7 +511,7 @@ The fix (`nix/docs/default.nix`): never forces a package — no stubs needed, and the Rust/frontend build closure stays out of the eval. -3. Both `hostEval` and `agentEval` are evaluated from `nixSrc` paths +3. Nix evaluates both `hostEval` and `agentEval` from `nixSrc` paths (not `self`), so the docs drv dependency chain ends at `nixSrc`. Why `builtins.unsafeDiscardStringContext`? The path string diff --git a/docs/process/pr-review-gate.md b/docs/process/pr-review-gate.md index c3ead59d..47ad763f 100644 --- a/docs/process/pr-review-gate.md +++ b/docs/process/pr-review-gate.md @@ -6,9 +6,9 @@ human review that already happened. ## The gate has (up to) three parts, and they're per-repo settings -Whether a PR can merge, and what counts toward "can," is configured -per repo in its branch-protection settings — not a fact true of every -hive or every repo. The pieces a repo _can_ require: +Each repo's branch-protection settings configure whether a PR can +merge, and what counts toward "can" — not a fact true of every hive or +every repo. The pieces a repo _can_ require: - **CI is green** — the repo's required status checks pass on the PR's current head commit, if the repo requires any. @@ -21,7 +21,7 @@ hive or every repo. The pieces a repo _can_ require: (merges automatically the moment the other required conditions are met). -Where these are required, they're independent of each other. A +Where a repo requires these, they're independent of each other. A reviewer only ever owns the review-approval piece — CI resolves (or doesn't) on its own regardless of what a review says, and merge-arming is someone else's call.