Skip to main content

max / makenotwork

Give DiscardSink and shell quoting one definition each Four byte-identical `DiscardSink` sinks stood in sando's deploy and gates and bento's runner and artifact_record. All four drop what they are handed for the same reason: the caller reads the captured RunOutput, not a live stream. One `pub struct DiscardSink` now sits beside `LogSink` in ops-exec, which both crates already depend on, and the four copies are gone. sando's `shell_escape` was character-for-character `ops_exec::sh_quote`, which the file already imported for its own tests. Deleted in favour of it. The server keeps its copy: it does not depend on ops-exec, and taking the deploy plane's ssh/rsync crate for three lines is the worse trade, so the copy now names the canonical one and says why. Both lib.rs headers promised a TUI consumer that no longer exists; sando-tui was retired on 2026-07-22 and bento never had one. Also fixes the sixteen pre-existing rustdoc failures that made `RUSTDOCFLAGS="-D warnings" cargo doc --document-private-items` unusable as a gate on either crate: links to private items rewritten as plain code spans, ASCII diagrams fenced as `text` so their `<placeholder>` names stop parsing as HTML tags, and two links repointed at the items they meant.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session
https://claude.ai/code/session_01EEmeiSJnmyL98QzA5Dwsvz
Author: Max Johnson <me@maxj.phd> · 2026-09-05 02:06 UTC
Signed with PGP, not checked
Commit: d8db8e8f2bbfd7c90dd0a706f6a9f6588b2809da
Parent: d676bbe
18 files changed, +68 insertions, -84 deletions
@@ -1239,7 +1239,14 @@
1239 1239 Ok(())
1240 1240 }
1241 1241
1242 - /// Escape a string for safe use in a shell command.
1242 + /// Escape a string for safe use in a shell command: complete POSIX
1243 + /// single-quoting, so no metacharacter inside can act.
1244 + ///
1245 + /// The canonical copy is `ops_exec::sh_quote` in
1246 + /// `MNW/shared/ops-exec/src/remote.rs`, which is character-for-character this
1247 + /// function. It is duplicated rather than imported because the server does not
1248 + /// depend on `ops-exec` and taking the deploy plane's ssh/rsync crate as a
1249 + /// dependency for three lines is the worse trade. Fix both if either changes.
1243 1250 fn shell_escape(s: &str) -> String {
1244 1251 format!("'{}'", s.replace('\'', "'\\''"))
1245 1252 }
@@ -23,7 +23,7 @@
23 23 use crate::state::AppState;
24 24 use chrono::{DateTime, Utc};
25 25 use ops_artifact::{ArtifactRecord, GateRecord, Manifest, Provenance, Scope, Verdict};
26 - use ops_exec::{Action, LogSink, Step as OpStep};
26 + use ops_exec::{Action, DiscardSink, Step as OpStep};
27 27 use std::path::PathBuf;
28 28
29 29 /// Where a target's record lands: beside the artifacts it describes, in that
@@ -230,14 +230,6 @@
230 230 Some((end - start).num_seconds().max(0))
231 231 }
232 232
233 - /// Sinks the toolchain probe's output. It is read from `RunOutput`, not streamed.
234 - struct DiscardSink;
235 -
236 - #[async_trait::async_trait]
237 - impl LogSink for DiscardSink {
238 - async fn write_chunk(&mut self, _bytes: &[u8]) {}
239 - }
240 -
241 233 #[cfg(test)]
242 234 mod tests {
243 235 use super::*;
@@ -171,7 +171,7 @@
171 171 /// Recipes read it via `feature_flags()`.
172 172 pub features: Vec<String>,
173 173 /// App or library. Decides which capability a `verify` step is gated on;
174 - /// see [`action_for`].
174 + /// see `action_for`.
175 175 pub kind: Kind,
176 176 pub target_run_id: i64,
177 177 /// Capability-scoped executor per build host. Recipe commands dispatch
@@ -527,7 +527,7 @@
527 527
528 528 /// Finalize the open step (if any): close its log, stamp the DB row, emit
529 529 /// `StepDone`. Idempotent when no step is open. A step flagged via
530 - /// [`fail_current_step`] is recorded `Failed` regardless of the requested
530 + /// `fail_current_step` is recorded `Failed` regardless of the requested
531 531 /// status, and added to the ledger `publish` consults.
532 532 pub fn finish_step(self: &Arc<Self>, status: Status) -> Result<()> {
533 533 let st = self.current.lock().unwrap().take();
@@ -595,9 +595,9 @@
595 595 .ok_or_else(|| anyhow::anyhow!("unknown build host `{name}` (not in topology)"))
596 596 }
597 597
598 - /// The ssh string for `name` (for `collect`'s remote scp source).
599 - /// The transport that moves artifacts off `name`. Distinct from
600 - /// [`RecipeCtx::exec_for`]'s executor: an agent host signs over `AgentRpc`
598 + /// The transport that moves artifacts off `name`, for `collect`'s remote
599 + /// scp source. Distinct from `RecipeCtx::exec`'s executor: an agent host
600 + /// signs over `AgentRpc`
601 601 /// but is collected from over ssh (`state::build_sync`).
602 602 fn host_sync(&self, name: &str) -> Result<Arc<dyn Executor>> {
603 603 self.syncs
@@ -1,11 +1,11 @@
1 1 //! bento-daemon as a library.
2 2 //!
3 - //! Exposes every module so the `bentod` binary (`src/main.rs`) and the `bento`
4 - //! TUI (`../tui`) share wire-facing types — events, domain newtypes — by import
5 - //! rather than duplication. External consumers mainly need `domain` and
6 - //! `events`.
3 + //! Exposes every module so the `bentod` binary (`src/main.rs`) can share
4 + //! wire-facing types (events, domain newtypes) by import rather than
5 + //! duplication. There is no TUI member: `bento/` holds `daemon`, `deploy` and
6 + //! `driver`. External consumers mainly need `domain` and `events`.
7 7 //!
8 - //! Design: `bento-overview` in the shared code wiki (`~/Code/_private/wiki/`).
8 + //! Design: `bento-overview` in the shared code wiki (`~/Wiki/`).
9 9 //! <!-- wiki: bento-overview -->
10 10
11 11 pub mod archive;
@@ -11,20 +11,10 @@
11 11 use crate::events::{self, Event};
12 12 use crate::state::AppState;
13 13 use anyhow::{Context, Result};
14 - use ops_exec::{Action, LogSink, Step as OpStep};
14 + use ops_exec::{Action, DiscardSink, Step as OpStep};
15 15 use std::path::PathBuf;
16 16 use std::sync::Arc;
17 17
18 - /// A [`LogSink`] that drops what it's handed — the release preflight runs git on
19 - /// each host for its exit code and (via a separate `rev-parse`) the sha in
20 - /// `RunOutput`, not for a streamed log, so there is no step to stream into.
21 - struct DiscardSink;
22 -
23 - #[async_trait::async_trait]
24 - impl LogSink for DiscardSink {
25 - async fn write_chunk(&mut self, _bytes: &[u8]) {}
26 - }
27 -
28 18 /// Preflight barrier: put every host that will build a target for this release
29 19 /// into a worktree detached at the tag `v<version>`, and refuse the build unless
30 20 /// they all report the SAME commit. Without it each host builds whatever its own
@@ -51,7 +41,7 @@
51 41 ///
52 42 /// A worktree costs the working files only, since it shares the repository's
53 43 /// object store, and is re-pinned with `--force`, which is safe precisely because
54 - /// nothing but Bento writes there. Where it lives is [`Host::worktree_root`],
44 + /// nothing but Bento writes there. Where it lives is [`crate::topology::Host::worktree_root`],
55 45 /// and that doc carries why the path has to sit under `~/Code`.
56 46 ///
57 47 /// Returns where each host will build, for the recipes to be pointed at, and the
@@ -3,7 +3,7 @@
3 3 //! Spec + rationale: maintainer wiki.
4 4 //! <!-- wiki: release-status-payload -->
5 5 //!
6 - //! `GET /status.json` serves this. [`payload`] is a pure function of
6 + //! `GET /status.json` serves this. `payload` is a pure function of
7 7 //! `(apps, now)`, so a fixture renders identically forever and the mapping is
8 8 //! testable without a database.
9 9 //!
@@ -295,7 +295,7 @@
295 295 /// `<worktree_root>/<repo>/<app>`, `repo` being the checkout's directory
296 296 /// name. `None` for a host with no worktree root, which cannot build.
297 297 ///
298 - /// Joined with `/` rather than [`PathBuf::join`] because the result is a
298 + /// Joined with `/` rather than [`std::path::Path::join`] because the result is a
299 299 /// path on the REMOTE host, rendered into git commands there. The daemon is
300 300 /// Linux and one build host is Windows, where `PathBuf` would render `\` and
301 301 /// the remote shell would not thank us; git takes forward slashes on every
@@ -446,8 +446,8 @@
446 446 ///
447 447 /// This is the whole point of the seam. `intake::accept` publishes the bundle
448 448 /// content-addressed once it has proved the bytes are the ones the record
449 - /// vouches for, which lands it at exactly the state [`publish`] leaves a
450 - /// Sando-built bundle in — so the two paths join at [`record_and_gate`] and
449 + /// vouches for, which lands it at exactly the state `publish` leaves a
450 + /// Sando-built bundle in, so the two paths join at `record_and_gate` and
451 451 /// nothing downstream knows or cares which one it came from.
452 452 ///
453 453 /// `staged` must already sit under `release_root/staging/` (publishing is an
@@ -72,8 +72,12 @@
72 72
73 73 /// Pull the first *root-cause* panic message out of libtest's captured
74 74 /// output. libtest (Rust 2021+) prints each captured panic as:
75 - /// thread '<test>' panicked at <file>:<line>:<col>:
76 - /// <message>
75 + ///
76 + /// ```text
77 + /// thread '<test>' panicked at <file>:<line>:<col>:
78 + /// <message>
79 + /// ```
80 + ///
77 81 /// We return the first panic's message, skipping "...poisoned" messages in
78 82 /// favour of the first non-poison one, because a single real panic in shared
79 83 /// setup (a `std::sync::Once`) poisons it and makes every *other* test report