lint(clippy): disallow blocking std::io and std::net on Tokio paths

Extends the workspace clippy.toml — which already bans std:🧵:sleep,
std:🧵:spawn, and std::process::Command::new on Tokio paths — with:

- disallowed-types: std::io::{Read, Write, BufRead, BufReader, BufWriter}
  and std::net::{TcpStream, TcpListener, UdpSocket}
- disallowed-methods: std::io::{stdin, stdout, stderr}

Non-blocking std::io items (Error, ErrorKind, Result, IsTerminal, Cursor)
remain allowed. std::fs is intentionally deferred.

Annotates ~24 pre-existing sync call sites with #[expect(..., reason = "...")]
matching the established pattern. All annotations describe why blocking I/O
is intentional in that context (sync CLI command, test helper, pre-fork
flush, etc.), so a future conversion to async will surface as an unfulfilled
lint expectation instead of silently drifting.

Fixes one real Tokio-path issue surfaced by the new lint:
fabro-cli's server-start daemon-health poller (try_connect) was a sync fn
called from async execute_daemon; std::net::TcpStream::connect_timeout
blocked a Tokio worker for up to 100ms per poll iteration. Converted to
tokio::net::{TcpStream, UnixStream} with tokio::time::timeout.

One follow-up flagged in-code: fabro-agent/src/cli.rs's JSON event writer
uses std::io::stdout() inside tokio::spawn. Annotated with a FOLLOW-UP
reason pointing at tokio::io::stdout; left unchanged since volume is low
and scope exceeded this pass.

Verified: clippy clean, cargo +nightly fmt --check clean, full nextest
workspace run (4131 passed, 182 skipped).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-19 16:06:02 -04:00
parent b849738a5b
commit 95b101a26f
No known key found for this signature in database
34 changed files with 262 additions and 8 deletions

View file

@ -5,9 +5,22 @@ disallowed-methods = [
{ path = "std::thread::spawn", reason = "Prefer Tokio task APIs on async paths; document intentional dedicated OS threads with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::thread::Builder::spawn", reason = "Prefer Tokio task APIs on async paths; document intentional dedicated OS threads with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::process::Command::new", reason = "Prefer tokio::process::Command on Tokio paths; document intentional synchronous subprocesses with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::io::stdin", reason = "Returns a blocking handle; prefer tokio::io::stdin on Tokio paths. Document intentional sync stdin with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::io::stdout", reason = "Returns a blocking handle; prefer tokio::io::stdout on Tokio paths. Document intentional sync stdout with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::io::stderr", reason = "Returns a blocking handle; prefer tokio::io::stderr on Tokio paths. Document intentional sync stderr with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "reqwest::Client::new", reason = "Use fabro_http::http_client() or fabro_http::test_http_client()", allow-invalid = true },
{ path = "reqwest::Client::builder", reason = "Use fabro_http::HttpClientBuilder::new()", allow-invalid = true },
{ path = "reqwest::blocking::Client::new", reason = "Use fabro_http::blocking_http_client() or fabro_http::blocking_test_http_client()", allow-invalid = true },
{ path = "reqwest::blocking::Client::builder", reason = "Use fabro_http::BlockingHttpClientBuilder::new()", allow-invalid = true },
{ path = "reqwest::get", reason = "Build a fabro_http client and send the request explicitly", allow-invalid = true },
]
disallowed-types = [
{ path = "std::io::Read", reason = "Blocking trait; prefer tokio::io::AsyncReadExt on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::io::Write", reason = "Blocking trait; prefer tokio::io::AsyncWriteExt on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::io::BufRead", reason = "Blocking trait; prefer tokio::io::AsyncBufReadExt on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::io::BufReader", reason = "Blocking buffered reader; prefer tokio::io::BufReader on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::io::BufWriter", reason = "Blocking buffered writer; prefer tokio::io::BufWriter on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::net::TcpStream", reason = "Blocking socket; prefer tokio::net::TcpStream on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::net::TcpListener", reason = "Blocking accept; prefer tokio::net::TcpListener on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
{ path = "std::net::UdpSocket", reason = "Blocking recv/send; prefer tokio::net::UdpSocket on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" },
]

View file

@ -1,3 +1,8 @@
#[expect(
clippy::disallowed_types,
reason = "CLI entry point writes to stdout/stderr; blocking std::io::Write is intentional and \
scoped to the CLI binary, not to any library code used by Tokio services"
)]
use std::io::{IsTerminal, Write};
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
@ -137,6 +142,12 @@ fn is_auto_approved(level: PermissionLevel, category: &str) -> bool {
}
#[allow(clippy::print_stderr)]
#[expect(
clippy::disallowed_methods,
reason = "Interactive tool-approval prompt: blocking on stderr flush and stdin read_line is \
the entire point. Invoked from an async tool hook, but the callback is a \
user-gated pause point; concurrent tasks on the same worker are acceptable."
)]
fn build_tool_approval(
permissions: PermissionLevel,
is_interactive: bool,
@ -442,6 +453,10 @@ pub async fn run_with_args_and_client(
// Build tool approval callback
let permissions = args.permissions.unwrap_or(PermissionLevel::ReadWrite);
#[expect(
clippy::disallowed_methods,
reason = "is_terminal() on stdin is a non-blocking fstat; no actual I/O performed"
)]
let is_interactive = std::io::stdin().is_terminal() && !args.auto_approve;
let tool_approval = build_tool_approval(permissions, is_interactive, styles);
let tool_hooks: Arc<dyn ToolHookCallback> = Arc::new(ToolApprovalAdapter(tool_approval));
@ -539,6 +554,13 @@ pub async fn run_with_args_and_client(
OutputFormat::Json => {
while let Ok(event) = rx.recv().await {
if let Ok(json) = serde_json::to_string(&event) {
#[expect(
clippy::disallowed_methods,
reason = "FOLLOW-UP: blocking stdout inside tokio::spawn. Acceptable \
today (low-volume event stream, CLI output), but should \
migrate to tokio::io::stdout to avoid worker stalls under \
pipe backpressure."
)]
let mut stdout = std::io::stdout().lock();
let _ = writeln!(stdout, "{json}");
let _ = stdout.flush();

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `config` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `config` command: blocking std::io::stdout is the intended output mechanism"
)]
use std::io::Write;
use std::path::Path;

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `graph` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `graph` command: blocking std::io::stdout is the intended output mechanism"
)]
use std::io::Write;
use anyhow::{Context, bail};

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `parse` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `parse` command: blocking std::io::stdout is the intended output mechanism"
)]
use std::io::Write;
use fabro_config::project::resolve_workflow;

View file

@ -1,3 +1,13 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `render-graph` command: reads DOT from stdin and writes rendered output \
to stdout via blocking std::io"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `render-graph` command: uses blocking std::io::stdin/stdout"
)]
use std::io::{Read, Write};
const RENDER_ERROR_PREFIX: &str = "RENDER_ERROR:";

View file

@ -1,3 +1,8 @@
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `repo init` command: interactive prompts read from std::io::stdin"
)]
use std::path::PathBuf;
use anyhow::{Context, Result, bail};

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `run attach` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `run attach` command: writes to std::io::stdout/stderr directly"
)]
use std::io::{IsTerminal, Write};
#[cfg(test)]
use std::path::Path;

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `run diff` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `run diff` command: writes diff output to std::io::stdout directly"
)]
use std::io::{self, IsTerminal, Write};
use anyhow::{Context, Result, bail};

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `run logs` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `run logs` command: streams log lines to std::io::stdout directly"
)]
use std::fmt::Write as _;
use std::io::{self, IsTerminal, Write};
use std::time::Duration;

View file

@ -1,3 +1,8 @@
#![expect(
clippy::disallowed_methods,
reason = "sync CLI run-progress renderer: writes to std::io::stderr directly"
)]
use fabro_types::RunEvent;
mod event;
@ -43,6 +48,10 @@ impl ProgressUI {
}
#[cfg(test)]
#[expect(
clippy::disallowed_types,
reason = "test helper accepts a sync blocking writer to capture rendered output"
)]
fn new_plain_test(out: Box<dyn std::io::Write + Send>, verbose: bool, colors: bool) -> Self {
Self::with_renderer(ProgressRenderer::new_plain(out, colors), verbose)
}
@ -412,6 +421,10 @@ impl ProgressUI {
#[cfg(test)]
mod tests {
#![allow(clippy::absolute_paths, clippy::needless_pass_by_value)]
#![expect(
clippy::disallowed_types,
reason = "tests capture rendered output into a Vec<u8> via the std::io::Write trait"
)]
use std::io::{self, Write};
use std::sync::{Arc, Mutex};

View file

@ -1,3 +1,9 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI run-progress renderer: generic over blocking std::io::Write to render \
terminal progress lines"
)]
use std::io::Write;
use std::sync::Mutex;

View file

@ -1,3 +1,9 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `run` subprocess wrapper: reads server subprocess stdout line-by-line via \
std::io::BufReader; not on a Tokio path"
)]
use std::io::{BufRead as StdBufRead, BufReader as StdBufReader};
use std::path::{Path, PathBuf};
use std::sync::Arc;

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `run wait` command: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `run wait` command: prints status to std::io::stdout directly"
)]
use std::io::Write;
use anyhow::{Result, bail};

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `secret set` command: reads secret from stdin via blocking std::io::Read"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `secret set` command: reads secret from std::io::stdin"
)]
use std::io::{IsTerminal, Read as _};
use anyhow::{Context as _, Result, bail};

View file

@ -12,6 +12,7 @@ use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs};
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
use fabro_util::{Home, dev_token, session_secret};
use tokio::net::{TcpStream, UnixStream};
use tokio::process::Command as TokioCommand;
use tokio::time;
@ -438,7 +439,7 @@ async fn execute_daemon(
while elapsed < timeout {
if let Some(record) = record::read_server_record(&record_path) {
if try_connect(&record.bind) {
if try_connect(&record.bind).await {
if announce {
let pid = child.id().unwrap_or_default();
maybe_warn_host_port_fallback(bind, &record.bind, printer);
@ -534,12 +535,15 @@ async fn acquire_lock(storage_dir: &Path) -> Result<std::fs::File> {
Ok(lock_file)
}
fn try_connect(bind: &Bind) -> bool {
async fn try_connect(bind: &Bind) -> bool {
let connect_timeout = Duration::from_millis(100);
match bind {
Bind::Tcp(addr) => {
std::net::TcpStream::connect_timeout(addr, Duration::from_millis(100)).is_ok()
}
Bind::Unix(path) => std::os::unix::net::UnixStream::connect(path).is_ok(),
Bind::Tcp(addr) => time::timeout(connect_timeout, TcpStream::connect(addr))
.await
.is_ok_and(|r| r.is_ok()),
Bind::Unix(path) => time::timeout(connect_timeout, UnixStream::connect(path))
.await
.is_ok_and(|r| r.is_ok()),
}
}

View file

@ -1,3 +1,8 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `uninstall` command: blocking std::io::Write is the intended output mechanism"
)]
use std::fs;
use std::io::Write;
use std::path::{Path, PathBuf};

View file

@ -1,3 +1,13 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI `upgrade` command: blocking std::io::Write and std::io::BufReader for \
release download + prompt output"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `upgrade` command: interactive confirm prompt reads from std::io::stdin"
)]
use std::fs;
use std::io::{IsTerminal, Write};
use std::path::{Path, PathBuf};

View file

@ -1,3 +1,8 @@
#![expect(
clippy::disallowed_methods,
reason = "sync CLI `version` command: writes version info to std::io::stderr"
)]
use std::io::IsTerminal;
use anyhow::Result;

View file

@ -367,8 +367,15 @@ async fn main_inner() -> (String, Result<()>) {
}));
match result {
Ok(buf) => {
use std::io::Write;
std::io::stdout().write_all(&buf)?;
#[expect(
clippy::disallowed_types,
clippy::disallowed_methods,
reason = "sync CLI: stream shell completions to stdout"
)]
{
use std::io::Write;
std::io::stdout().write_all(&buf)?;
}
}
Err(_) => {
anyhow::bail!(

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI: provider auth reads an API key from stdin via std::io::Read"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI: provider auth reads an API key from std::io::stdin"
)]
use std::io::Read;
use std::sync::Arc;

View file

@ -1,3 +1,12 @@
#![expect(
clippy::disallowed_types,
reason = "sync CLI utilities: blocking std::io::Write is the intended output mechanism"
)]
#![expect(
clippy::disallowed_methods,
reason = "sync CLI utilities: blocking std::io::stdout is the intended output mechanism"
)]
use std::io::Write;
use std::path::{Path, PathBuf};
use std::time::Duration;

View file

@ -1,3 +1,8 @@
#![expect(
clippy::disallowed_types,
reason = "integration tests: read child-process stdout line-by-line via std::io::BufReader"
)]
use std::io::{BufRead, BufReader, Read};
use std::process::{Output, Stdio};
use std::sync::mpsc;

View file

@ -2,6 +2,10 @@
clippy::disallowed_methods,
reason = "These CLI integration tests intentionally spawn the real fabro binary and stream DOT over stdio to verify the internal render subprocess contract."
)]
#![expect(
clippy::disallowed_types,
reason = "integration tests write DOT to the spawned child's stdin via std::io::Write"
)]
use std::io::Write;
use std::process::{Command, Stdio};

View file

@ -2,6 +2,10 @@
clippy::disallowed_methods,
reason = "These CLI integration tests spawn real fabro worker subprocesses and observe their lifecycle."
)]
#![expect(
clippy::disallowed_types,
reason = "integration tests read the spawned child's stdout via std::io::Read"
)]
use std::io::Read;
use std::process::{Child, ExitStatus, Output, Stdio};

View file

@ -1,3 +1,9 @@
#![expect(
clippy::disallowed_types,
reason = "integration test: occupies a fixed TCP port via sync std::net::TcpListener to \
verify the server-start fallback path when the default port is unavailable"
)]
use std::process::Stdio;
use std::sync::{Arc, Barrier};
use std::time::{Duration, Instant};

View file

@ -211,6 +211,10 @@ pub(crate) fn parse_compose_multi(
}
#[cfg(test)]
#[expect(
clippy::disallowed_types,
reason = "test helpers write compose fixtures to temp files via sync std::io"
)]
mod tests {
use std::io::Write;

View file

@ -218,6 +218,12 @@ impl Interviewer for ConsoleInterviewer {
async fn ask(&self, question: Question) -> Answer {
// If stdin is a TTY, use dialoguer for interactive arrow-key navigation.
// Otherwise, fall back to the line-based reader for piped input.
#[expect(
clippy::disallowed_methods,
reason = "is_terminal() on the std stdin handle is a non-blocking fstat check; no \
actual I/O performed. The real blocking read runs inside spawn_blocking \
below."
)]
if std::io::stdin().is_terminal() {
if let Some(ref context_text) = question.context_display {
let rendered = self.styles.render_markdown(context_text);

View file

@ -725,6 +725,11 @@ fn server_bind_title(bind: &Bind) -> String {
}
#[cfg(test)]
#[expect(
clippy::disallowed_types,
reason = "tests reserve/probe ports via sync std::net::TcpListener; the async server under \
test uses tokio::net::TcpListener separately"
)]
mod tests {
use std::path::PathBuf;
use std::time::Duration;

View file

@ -26,6 +26,12 @@ pub fn spawn_detached(args: &[&str], env: &[(&str, &str)], env_remove: &[&str])
#[cfg(unix)]
#[allow(clippy::exit)]
#[expect(
clippy::disallowed_types,
clippy::disallowed_methods,
reason = "pre-fork cleanup: flush the blocking stdout/stderr buffers before double-forking \
so the child does not inherit buffered data; sync std::io is required here"
)]
fn spawn_detached_unix(args: &[&str], env: &[(&str, &str)], env_remove: &[&str]) {
// Flush stdout/stderr before forking so the child process doesn't inherit
// buffered data that would be flushed again on child exit, causing

View file

@ -1,4 +1,8 @@
use std::fs;
#[expect(
clippy::disallowed_types,
reason = "sync atomic write of the local dev token file; not on an async path"
)]
use std::io::Write as _;
use std::path::Path;

View file

@ -1,3 +1,9 @@
#![expect(
clippy::disallowed_types,
reason = "file-backed tracing sink: sync BufWriter<File> is intentional; writes happen on a \
dedicated per-event guard and are not in an async hot path"
)]
use std::io::{self, BufWriter, Write};
use std::path::Path;
use std::sync::atomic::{AtomicBool, Ordering};

View file

@ -1,4 +1,8 @@
use std::collections::HashMap;
#[expect(
clippy::disallowed_types,
reason = "in-memory Vec<u8>::write_all for jsonl serialization; no filesystem or network I/O"
)]
use std::io::Write;
use std::path::{Component, Path, PathBuf};

View file

@ -250,6 +250,10 @@ pub struct AppState {
/// Panics if openssl is not available or the key is invalid. This is acceptable
/// because the fake server is test infrastructure and openssl is already
/// required by the test helpers that generate key pairs.
#[expect(
clippy::disallowed_types,
reason = "test harness: piping private key PEM into openssl via sync std::io is intentional"
)]
pub fn derive_public_key_pem(private_key_pem: &str) -> String {
use std::io::Write;
use std::process::{Command, Stdio};