From 5d93b95b072feaa538c4f3e78da9f924328868cb Mon Sep 17 00:00:00 2001 From: Sephyi Date: Sun, 19 Apr 2026 19:06:11 +0200 Subject: [PATCH 1/4] chore: add clippy.toml with disallowed-methods and disallowed-macros Add a minimal, pragmatic `clippy.toml` that catches two common anti-patterns at lint time: - `disallowed-methods` blocks `std::process::Command::new`, pushing async call-sites toward `tokio::process::Command` or `spawn_blocking`. Synchronous contexts (integration tests, CLI bootstrap before the runtime starts) opt out locally via `#[allow(clippy::disallowed_methods)]` with a comment. - `disallowed-macros` blocks `std::dbg` so leftover debug scaffolding can't ship; `tracing::{debug,trace}` is the durable replacement. Existing legitimate sync `Command` uses are annotated rather than rewritten: - `src/app.rs::hook_dir` runs in the synchronous handle_hook bootstrap path; F-002 will migrate it off sync Command, and the allow here documents the coordination so this PR doesn't block on F-002. - `tests/history.rs` and `tests/porcelain.rs` set a module-level `#![allow(clippy::disallowed_methods)]` because integration tests legitimately shell out to git / the commitbee binary synchronously. Closes audit entry F-018 from #3. --- clippy.toml | 23 +++++++++++++++++++++++ src/app.rs | 5 +++++ tests/history.rs | 5 +++++ tests/porcelain.rs | 5 +++++ 4 files changed, 38 insertions(+) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000..a260f2c --- /dev/null +++ b/clippy.toml @@ -0,0 +1,23 @@ +# SPDX-FileCopyrightText: 2026 Sephyi +# +# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial + +# Minimal, pragmatic clippy config to catch common anti-patterns at lint time. +# +# Keep this file small. Each rule should have a clear justification in its +# `reason` so reviewers understand why the flagged call was blocked. + +# Disallow sync `std::process::Command` — the async pipeline must not block the +# tokio runtime. Use `tokio::process::Command` or wrap in +# `tokio::task::spawn_blocking` at async call-sites. Synchronous contexts +# (integration tests, CLI bootstrap before the runtime starts) may allow this +# lint locally with `#[allow(clippy::disallowed_methods)]` and a comment. +disallowed-methods = [ + { path = "std::process::Command::new", reason = "prefer tokio::process::Command or spawn_blocking in async contexts" }, +] + +# Disallow the `dbg!` macro — leftover debug scaffolding should never ship. +# Use `tracing::{debug,trace}` for durable diagnostics. +disallowed-macros = [ + { path = "std::dbg", reason = "leftover debug scaffolding; remove before committing" }, +] diff --git a/src/app.rs b/src/app.rs index cae18dc..fb099e0 100644 --- a/src/app.rs +++ b/src/app.rs @@ -990,6 +990,11 @@ impl App { // Verify we're in a git repo first let _git = GitService::discover()?; + // `hook_dir` is called from a synchronous `handle_hook` path (CLI + // bootstrap, no tokio runtime live yet). F-002 will migrate the hook / + // clipboard paths off sync `Command`; until then, allow the lint here + // so the new clippy.toml rule doesn't block unrelated PRs. + #[allow(clippy::disallowed_methods)] let output = std::process::Command::new("git") .args(["rev-parse", "--git-dir"]) .output()?; diff --git a/tests/history.rs b/tests/history.rs index 53c7582..f24a002 100644 --- a/tests/history.rs +++ b/tests/history.rs @@ -2,6 +2,11 @@ // // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial +// Integration tests are synchronous and legitimately use `std::process::Command` +// to build git fixtures in tempdirs; the `disallowed_methods` rule in +// clippy.toml targets async-context misuse, which does not apply here. +#![allow(clippy::disallowed_methods)] + use commitbee::services::history::{HistoryContext, HistoryService}; // ─── Subject Analysis (Pure Functions) ─────────────────────────────────────── diff --git a/tests/porcelain.rs b/tests/porcelain.rs index 0588174..121b997 100644 --- a/tests/porcelain.rs +++ b/tests/porcelain.rs @@ -23,6 +23,11 @@ //! deferred to a follow-up — it requires the full pipeline setup and belongs //! in its own test file. +// Integration tests are synchronous and legitimately use `std::process::Command` +// to shell out to `git` / the `commitbee` binary; the `disallowed_methods` rule +// in clippy.toml targets async-context misuse, which does not apply here. +#![allow(clippy::disallowed_methods)] + use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; From d39753b23149f47c1d929f4320a520a81b1f3600 Mon Sep 17 00:00:00 2001 From: Sephyi Date: Thu, 30 Apr 2026 17:38:14 +0200 Subject: [PATCH 2/4] Update src/app.rs The justification comment is inaccurate: this code runs under the Tokio runtime (main is #[tokio::main] and Commands::Hook is handled inside app.run().await), so std::process::Command here can still block a runtime worker thread. Either update the comment to reflect the actual execution context and why blocking is acceptable temporarily, or (preferably) move the blocking call behind spawn_blocking / tokio::process::Command so the new lint remains meaningful in production code paths. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/app.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/app.rs b/src/app.rs index fb099e0..edf5cec 100644 --- a/src/app.rs +++ b/src/app.rs @@ -990,14 +990,17 @@ impl App { // Verify we're in a git repo first let _git = GitService::discover()?; - // `hook_dir` is called from a synchronous `handle_hook` path (CLI - // bootstrap, no tokio runtime live yet). F-002 will migrate the hook / - // clipboard paths off sync `Command`; until then, allow the lint here - // so the new clippy.toml rule doesn't block unrelated PRs. - #[allow(clippy::disallowed_methods)] - let output = std::process::Command::new("git") - .args(["rev-parse", "--git-dir"]) - .output()?; + let output = if tokio::runtime::Handle::try_current().is_ok() { + tokio::task::block_in_place(|| { + std::process::Command::new("git") + .args(["rev-parse", "--git-dir"]) + .output() + })? + } else { + std::process::Command::new("git") + .args(["rev-parse", "--git-dir"]) + .output()? + }; if !output.status.success() { return Err(Error::Git("Cannot find .git directory".into())); From 8780fd8031bde6cbb2dc128c81a80a2b8b68183d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:40:11 +0000 Subject: [PATCH 3/4] fix: correct inaccurate hook_dir comment (runs under tokio runtime) Agent-Logs-Url: https://github.com/Sephyi/commitbee/sessions/ac83fbfb-5a4f-427f-ac91-403ed6f107a8 Co-authored-by: Sephyi <89100305+Sephyi@users.noreply.github.com> --- src/app.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/app.rs b/src/app.rs index edf5cec..537fc0b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -990,17 +990,16 @@ impl App { // Verify we're in a git repo first let _git = GitService::discover()?; - let output = if tokio::runtime::Handle::try_current().is_ok() { - tokio::task::block_in_place(|| { - std::process::Command::new("git") - .args(["rev-parse", "--git-dir"]) - .output() - })? - } else { - std::process::Command::new("git") - .args(["rev-parse", "--git-dir"]) - .output()? - }; + // `hook_dir` runs under the Tokio runtime (`main` is `#[tokio::main]` + // and hook commands reach here via `app.run().await`), so this + // `std::process::Command` can block a worker thread. F-002 will + // migrate the hook / clipboard paths to `tokio::process::Command`; + // until then, allow the lint locally so the new clippy.toml rule + // does not block unrelated PRs. + #[allow(clippy::disallowed_methods)] + let output = std::process::Command::new("git") + .args(["rev-parse", "--git-dir"]) + .output()?; if !output.status.success() { return Err(Error::Git("Cannot find .git directory".into())); From c171c8854350b385cab30ef5c7fcb6829a85bd64 Mon Sep 17 00:00:00 2001 From: Sephyi Date: Thu, 30 Apr 2026 17:44:02 +0200 Subject: [PATCH 4/4] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/history.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/history.rs b/tests/history.rs index f24a002..a49f7f7 100644 --- a/tests/history.rs +++ b/tests/history.rs @@ -2,10 +2,10 @@ // // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial -// Integration tests are synchronous and legitimately use `std::process::Command` -// to build git fixtures in tempdirs; the `disallowed_methods` rule in -// clippy.toml targets async-context misuse, which does not apply here. -#![allow(clippy::disallowed_methods)] +// Keep `clippy::disallowed_methods` enabled for this file. If a synchronous +// helper needs `std::process::Command` to build git fixtures in a tempdir, +// allow it narrowly at that specific helper or call site with a short +// justification rather than silencing the lint for async tests as well. use commitbee::services::history::{HistoryContext, HistoryService};