.forgejo/workflows/ci.yml

@@ -110,9 +110,10 @@ jobs:
           command -v cargo-llvm-cov >/dev/null 2>&1 || cargo install cargo-llvm-cov --locked
 
       # Reports coverage only — no --fail-under-lines yet. main.rs is
-      # excluded: thin orchestration glue exercised by the real end-to-end
-      # `cargo run` against live GitHub, not unit tests, so it's not a
-      # meaningful signal here. See Makefile.toml > coverage-report.
+      # excluded: thin argv dispatch exercised by the real end-to-end
+      # `cargo run`, not unit tests, so it's not a meaningful signal here.
+      # See Makefile.toml > coverage-report for why pipeline.rs, despite
+      # being mostly untestable I/O orchestration too, stays included.
       - name: Coverage
         run: cargo llvm-cov --ignore-filename-regex 'main\.rs' --summary-only
 

ARCHITECTURE.md

@@ -0,0 +1,131 @@
+# pkgwatch — code organization
+
+Status: written 2026-09-17, once the build/publish pipeline PR gave this
+project enough real code to have actual conventions worth writing down,
+instead of guessing at them in advance.
+
+This is distinct from `SPEC.md`, which is the product design (what
+pkgwatch does and why). This file is about how the *code* implementing
+that design is organized, so it stays readable as it grows past PoC size
+instead of quietly accumulating debt. Researched against current industry
+practice rather than asserted from habit — see Further reading.
+
+## Principles
+
+1. **One module, one job — and say what it is, up front.**
+   Ousterhout's "deep modules": the best modules expose a lot of
+   functionality through a simple interface, hiding the complexity behind
+   it. The two failure modes he names — *change amplification* (one
+   conceptual change forces edits in many places) and *obscurity* (a
+   reader can't tell where responsibility lives) — are both symptoms of
+   modules that don't have one clear job.
+   **Rule**: every `src/*.rs` file opens with a `//!` doc comment stating
+   its one responsibility in a sentence. If it can't be one sentence, the
+   module is doing too much.
+   **Example already here**: `builder.rs`'s job is "turn an
+   already-downloaded, already-verified artifact into a built package." It
+   hides PKGBUILD templating, upstream-string validation, and the
+   `makepkg` invocation behind one `build()` call — none of that leaks to
+   callers.
+
+2. **Organize by pipeline stage (feature), not by technical layer.**
+   The package-by-feature vs. package-by-layer research is consistent:
+   feature-based grouping gives high cohesion within a module and low
+   coupling between modules; layer-based grouping (`models/`, `utils/`,
+   `helpers/`) tends toward the opposite, and a single feature change ends
+   up touching files scattered across every layer.
+   **Rule**: modules are named after what they do in the pipeline
+   (`checker`, `fetcher`, `verifier`, `builder`, `sanity`, `publisher`,
+   `state`), not generic buckets. A new pipeline stage gets a new module
+   named after the stage, not a method bolted onto an existing one.
+   **Anti-example to keep watching for**: a `utils.rs` grab-bag. `hash.rs`
+   could look like one but isn't — it exists for exactly one piece of
+   shared logic (`sha256_hex`) that two real stages (`verifier`,
+   `builder`) both need, not as a place to dump unrelated helpers.
+
+3. **Separate pure decision logic from I/O ("functional core, imperative
+   shell").**
+   A function that decides *and* does in the same body can't be tested
+   without standing up everything the "does" half touches — often a
+   network call, a subprocess, or the filesystem. Pulling the decision out
+   into its own pure function makes it trivially unit-testable and makes
+   the I/O half thin enough that it obviously matches the decision.
+   **Applied this PR**: `pipeline::process_package`'s tier dispatch
+   (publish now / still pending / newly pending / verification failed) was
+   originally inline in a function that also made the real network and
+   build calls. Pulled out into `decide_tier_action`, a pure function with
+   its own unit tests covering all four outcomes, no I/O involved.
+
+4. **Every network, subprocess, filesystem-root, or environment boundary
+   is injectable.**
+   Same testability goal as #3, applied to the specific ways this program
+   reaches outside itself. A consistent shape beats ad hoc mocking invented
+   per call site.
+   **Already in force**: `GithubEndpoints` (checker/fetcher/verifier),
+   `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR`
+   (main, for manual dry runs against a scratch repo instead of the real
+   one). A new external call follows the same shape: production code calls
+   a thin wrapper with the real default; tests call the parameterized
+   version with a fake.
+
+5. **`main.rs` is a dispatcher, not the program.**
+   Found by looking at this project's own `main.rs`: it grew to 278 lines
+   and zero tests over the course of one PR, because "it's just the entry
+   point" is an easy excuse to skip separating logic from wiring — even
+   though Rust doesn't actually stop you from unit-testing a binary
+   crate's `main.rs`. The Rust community convention of splitting
+   entry-point parsing from application logic exists precisely so the
+   logic ends up somewhere it's normal to test.
+   **Rule**: `main.rs` may parse `argv`, build shared clients, and print
+   output. It must not contain a pipeline decision, a network/subprocess
+   call, or anything with a test worth writing — that belongs in
+   `pipeline.rs`.
+   **Applied this PR**: moved `process_package`, `fetch_and_verify`,
+   `build_and_publish`, `run_review`, and `approve` out of `main.rs` into a
+   new `pipeline.rs`, leaving `main.rs` as argument dispatch only.
+
+6. **Validate at the boundary, once — don't scatter checks.**
+   Already stated project-wide (see the user's global instructions: don't
+   validate scenarios that can't happen, validate at system boundaries).
+   **Example already here**: `builder.rs`'s `validate_pkgname`/
+   `validate_pkgver`/`validate_shell_safe` run once, at PKGBUILD-generation
+   time, against every upstream-controlled string — not sprinkled through
+   whatever code happens to produce those strings.
+
+7. **Don't build generality the currently-tracked packages don't need.**
+   Already the load-bearing design principle in `SPEC.md` ("a small fixed
+   set of PKGBUILD shapes," "extend when a third real shape shows up").
+   Restated here because it's also a tech-debt principle in its own right:
+   speculative abstraction is debt too — every future reader has to
+   understand it whether or not it's ever exercised.
+
+8. **Every non-obvious structural decision gets one sentence of "why,"
+   inline.**
+   Standard tech-debt-prevention advice is to keep Architecture Decision
+   Records; a single-crate personal tool doesn't need a `docs/adr/`
+   directory, but the same information — why this way and not the obvious
+   alternative — needs to live somewhere a future reader will actually see
+   it: the doc comment on the thing itself.
+   **Example already here**: `checker.rs`'s doc comment on
+   `latest_github_release` explains why the newest Atom-feed entry isn't
+   trusted outright (scaleway-cli's `-dbg1` tag has no real Release behind
+   it) — the reasoning lives right next to the code it justifies, not in a
+   commit message or a separate design doc no one will find later.
+
+## What's machine-enforced vs. what isn't
+
+`cargo make ci` (format, clippy, cognitive-complexity threshold, coverage,
+audit) mechanically enforces what's checkable: style, a handful of lint
+categories, a complexity ceiling, and that coverage doesn't quietly
+regress. It does **not** enforce module cohesion, naming, or "is this
+logic in the right module" — those stay code-review questions. Worth
+being honest about that boundary rather than implying CI catches
+everything above.
+
+## Further reading
+
+- [A Philosophy of Software Design — deep modules & information hiding, summary](https://medium.com/swlh/a-philosophy-of-software-design-by-john-ousterhout-4a00d0ff9f1c)
+- [Package by feature vs. package by layer](https://medium.com/@felixnjunge78/package-by-feature-vs-package-by-layer-which-one-wins-11ee03921fed)
+- [Coupling and cohesion as the foundations of a maintainable codebase](https://medium.com/@iamprovidence/coupling-and-cohesion-foundations-that-affect-your-entire-codebase-77d06d44af0d)
+- [Rust module and crate organization best practices](https://softwarepatternslexicon.com/rust/idiomatic-rust-patterns/module-and-crate-organization-best-practices/)
+- [Reducing technical debt in 2026 — IBM](https://www.ibm.com/think/insights/reduce-technical-debt)

Makefile.toml

@@ -30,9 +30,15 @@ args = ["test"]
 # same-named custom one.
 #
 # Reports coverage only; not gated on a threshold yet — main.rs is thin
-# orchestration glue exercised by the real end-to-end `cargo run`, not unit
+# argv dispatch exercised by the real end-to-end `cargo run`, not unit
 # tests, so it's excluded here rather than dragging the number down for
-# reasons unrelated to test quality.
+# reasons unrelated to test quality. pipeline.rs is deliberately NOT
+# excluded even though it's mostly network/subprocess/filesystem
+# orchestration too (hence its own low number) — its one pure decision
+# function (decide_tier_action) is unit tested and should stay visible in
+# this report; excluding the whole file would hide that signal along with
+# the untested parts. See ARCHITECTURE.md > "separate pure decision logic
+# from I/O."
 [tasks.coverage-report]
 command = "cargo"
 args = ["llvm-cov", "--ignore-filename-regex", "main\\.rs", "--summary-only"]

SPEC.md

@@ -2,6 +2,10 @@
 
 Status: design draft, pre-PoC. Captures the design discussion as of 2026-09-11.
 
+This is the *product* design — what pkgwatch does and why. For how the
+code implementing it is organized (module boundaries, testability
+conventions, what CI does and doesn't enforce), see `ARCHITECTURE.md`.
+
 ## Problem
 
 Software not packaged by the distro (Arch/Manjaro here) usually gets installed

src/builder.rs

@@ -1,3 +1,7 @@
+//! Turns an already-downloaded, already-verified artifact into a built
+//! pacman package: generates a PKGBUILD, then runs `makepkg`. Hides all
+//! PKGBUILD templating and upstream-string validation behind `build()`.
+
 use crate::config::Package;
 use crate::hash::sha256_hex;
 use anyhow::{Context, Result, bail};

src/config.rs

@@ -1,3 +1,7 @@
+//! Parses `packages.d/*.toml` into typed, in-memory `Package` records.
+//! The only module that knows the TOML shape — everything downstream
+//! works with `Package`/`Verification`/`SanityCheck`, never raw TOML.
+
 use anyhow::{Context, Result};
 use serde::Deserialize;
 use std::collections::HashMap;

src/fetcher.rs

@@ -1,3 +1,7 @@
+//! Downloads a named GitHub release asset to a local path. The only
+//! module that talks to the releases API for asset bytes — `checker` only
+//! resolves version tags, never downloads.
+
 use crate::github::GithubEndpoints;
 use anyhow::{Context, Result};
 use serde::Deserialize;

src/hash.rs

@@ -1,3 +1,7 @@
+//! One function, shared by two real callers (`verifier`, `builder`) —
+//! not a general-purpose utils dump. See ARCHITECTURE.md > "organize by
+//! pipeline stage, not by layer" for why that distinction matters.
+
 use sha2::{Digest, Sha256};
 
 /// Shared by `verifier` (same-origin-sha256 checks) and `builder` (every

src/main.rs

@@ -1,278 +1,31 @@
+//! Entry point: parses `argv` and dispatches to `pipeline`. Nothing here
+//! makes a network/subprocess call or contains a decision worth a test —
+//! see ARCHITECTURE.md > "main is a dispatcher, not the program."
+
 mod builder;
 mod checker;
 mod config;
 mod fetcher;
 mod github;
 mod hash;
+mod pipeline;
 mod publisher;
 mod sanity;
 mod state;
 mod verifier;
 
-use anyhow::{Context, Result, bail};
-use config::Package;
-use fetcher::DownloadedAsset;
-use github::GithubEndpoints;
-use std::path::{Path, PathBuf};
-use verifier::VerificationResult;
-
-const PACKAGES_DIR: &str = "packages.d";
-const STATE_DIR: &str = "state";
-const WORK_DIR: &str = "work";
-/// Not a repo pkgwatch invents: this is the existing, already-registered
-/// local pacman repo on this box (see `[custom]` in /etc/pacman.conf and
-/// its `Server = file://...` line). pkgwatch adds packages to it; it does
-/// not create the repo or touch pacman.conf.
-const CUSTOM_REPO_NAME: &str = "custom";
-const CUSTOM_REPO_SUBPATH: &str = ".local/share/pacman/custom";
+use anyhow::{Result, bail};
 
 /// check -> fetch -> verify -> build -> sanity-check -> publish, for
 /// whatever is in packages.d/. Tier 1-3 passes auto-publish; tier 4-6
-/// passes queue for `pkgwatch review`. See SPEC.md > Architecture.
+/// passes queue for `pkgwatch review`. See SPEC.md > Architecture for what
+/// each stage does, and ARCHITECTURE.md for how the code implementing it
+/// is organized.
 fn main() -> Result<()> {
     let args: Vec<String> = std::env::args().skip(1).collect();
     match args.first().map(String::as_str) {
-        None => run_check(),
-        Some("review") => run_review(&args[1..]),
+        None => pipeline::run_check(),
+        Some("review") => pipeline::run_review(&args[1..]),
         Some(other) => bail!("unknown subcommand '{other}' (expected: review)"),
     }
 }
-
-fn build_client() -> Result<reqwest::blocking::Client> {
-    Ok(reqwest::blocking::Client::builder()
-        .user_agent("pkgwatch/0.1 (PoC; https://code.austinschaefer.com)")
-        .build()?)
-}
-
-fn custom_repo_dir() -> Result<PathBuf> {
-    // Override for testing against a scratch repo instead of the real one
-    // at $HOME/.local/share/pacman/custom.
-    if let Ok(dir) = std::env::var("PKGWATCH_REPO_DIR") {
-        return Ok(PathBuf::from(dir));
-    }
-    let home = std::env::var("HOME").context("HOME is not set")?;
-    Ok(Path::new(&home).join(CUSTOM_REPO_SUBPATH))
-}
-
-fn run_check() -> Result<()> {
-    let client = build_client()?;
-    let endpoints = GithubEndpoints::default();
-    let packages_dir = Path::new(PACKAGES_DIR);
-    let state_dir = Path::new(STATE_DIR);
-    let work_dir = Path::new(WORK_DIR);
-
-    let packages = config::load_packages_dir(packages_dir)?;
-    if packages.is_empty() {
-        println!("no packages configured under {}/", packages_dir.display());
-        return Ok(());
-    }
-
-    let mut any_failed = false;
-    for (name, pkg) in &packages {
-        println!("== {name} ({}) ==", pkg.repo);
-        if let Err(err) = process_package(&client, &endpoints, state_dir, work_dir, name, pkg) {
-            eprintln!("  error: {err:#}");
-            any_failed = true;
-        }
-    }
-
-    if any_failed {
-        bail!("one or more packages failed — see errors above");
-    }
-    Ok(())
-}
-
-fn process_package(
-    client: &reqwest::blocking::Client,
-    endpoints: &GithubEndpoints,
-    state_dir: &Path,
-    work_dir: &Path,
-    name: &str,
-    pkg: &Package,
-) -> Result<()> {
-    let latest = checker::latest_github_release(client, endpoints, &pkg.repo)?;
-    let last_seen = state::load_last_version(state_dir, name);
-    if last_seen.as_deref() == Some(latest.as_str()) {
-        println!("  up to date at {latest}");
-        return Ok(());
-    }
-    println!("  new version detected: {latest} (previously: {last_seen:?})");
-
-    let fetched = fetch_and_verify(client, endpoints, work_dir, name, pkg, &latest)?;
-    println!("  fetched {}", fetched.asset.path.display());
-    println!(
-        "  verification (tier {}): {} — {}",
-        fetched.verification.tier,
-        if fetched.verification.passed {
-            "PASS"
-        } else {
-            "FAIL"
-        },
-        fetched.verification.justification
-    );
-
-    if !fetched.verification.passed {
-        println!("  verification failed — not publishing, not updating state");
-        return Ok(());
-    }
-
-    if fetched.verification.tier <= 3 {
-        println!("  tier 1-3 pass: building + publishing");
-        build_and_publish(name, pkg, &latest, &fetched)?;
-        state::save_last_version(state_dir, name, &latest)?;
-        state::clear_pending_version(state_dir, name)?;
-        println!("  published {name} {latest}");
-    } else if state::load_pending_version(state_dir, name).as_deref() == Some(latest.as_str()) {
-        println!("  tier 4-6 pass: still pending review (`pkgwatch review` to see it)");
-    } else {
-        state::save_pending_version(state_dir, name, &latest)?;
-        println!("  tier 4-6 pass: flagged for human review (`pkgwatch review` to approve)");
-    }
-    Ok(())
-}
-
-struct FetchVerifyResult {
-    version: String,
-    asset_name: String,
-    dest_dir: PathBuf,
-    asset: DownloadedAsset,
-    verification: VerificationResult,
-}
-
-/// Shared by the normal check loop (tier 1-3 auto-path) and `pkgwatch
-/// review --approve` (which re-verifies before publishing rather than
-/// trusting a possibly-stale flag from an earlier run).
-fn fetch_and_verify(
-    client: &reqwest::blocking::Client,
-    endpoints: &GithubEndpoints,
-    work_dir: &Path,
-    name: &str,
-    pkg: &Package,
-    tag: &str,
-) -> Result<FetchVerifyResult> {
-    let version = checker::version_from_tag(tag).to_string();
-    let asset_name = pkg.asset_pattern.replace("{version}", &version);
-    let dest_dir = work_dir.join(name).join(tag);
-
-    let asset = fetcher::download_asset(client, endpoints, &pkg.repo, tag, &asset_name, &dest_dir)?;
-    let verification = verifier::verify(
-        client,
-        endpoints,
-        &pkg.verification,
-        &pkg.repo,
-        tag,
-        &asset.path,
-        &dest_dir,
-    )?;
-
-    Ok(FetchVerifyResult {
-        version,
-        asset_name,
-        dest_dir,
-        asset,
-        verification,
-    })
-}
-
-fn build_and_publish(
-    name: &str,
-    pkg: &Package,
-    tag: &str,
-    fetched: &FetchVerifyResult,
-) -> Result<()> {
-    // Fail fast if the repo isn't registered in pacman.conf, before
-    // spending several seconds on a makepkg build that would otherwise
-    // succeed and then publish somewhere pacman never syncs from.
-    let repo_dir = custom_repo_dir()?;
-    publisher::ensure_registered(CUSTOM_REPO_NAME, &repo_dir)?;
-
-    let build_dir = fetched.dest_dir.join("build");
-    let req = builder::BuildRequest {
-        pkg_name: name,
-        pkg,
-        version: &fetched.version,
-        repo: &pkg.repo,
-        asset_name: &fetched.asset_name,
-        download_url: &fetched.asset.download_url,
-        artifact_path: &fetched.asset.path,
-    };
-    let built = builder::build(&req, &build_dir)?;
-    println!("  built {}", built.package_path.display());
-
-    if let Some(check) = &pkg.sanity_check {
-        let bin_dir = built.pkgdir.join("usr/bin");
-        sanity::run(check, &bin_dir, &fetched.version)
-            .with_context(|| format!("sanity check for {name} {tag}"))?;
-        println!("  sanity check passed");
-    }
-
-    let published = publisher::publish(&built.package_path, &repo_dir, CUSTOM_REPO_NAME)?;
-    println!(
-        "  added to {} repo: {}",
-        CUSTOM_REPO_NAME,
-        published.display()
-    );
-    println!(
-        "  not installed automatically — run `sudo pacman -Syu` (or `sudo pacman -S {name}`) to pick it up"
-    );
-    Ok(())
-}
-
-fn run_review(args: &[String]) -> Result<()> {
-    let packages_dir = Path::new(PACKAGES_DIR);
-    let state_dir = Path::new(STATE_DIR);
-    let work_dir = Path::new(WORK_DIR);
-    let packages = config::load_packages_dir(packages_dir)?;
-
-    match args {
-        [] => {
-            let mut any = false;
-            for (name, _) in &packages {
-                if let Some(pending) = state::load_pending_version(state_dir, name) {
-                    println!(
-                        "{name}: {pending} pending review (run `pkgwatch review {name} --approve`)"
-                    );
-                    any = true;
-                }
-            }
-            if !any {
-                println!("no packages pending review");
-            }
-            Ok(())
-        }
-        [name, flag] if flag == "--approve" => {
-            let (_, pkg) = packages.iter().find(|(n, _)| n == name).with_context(|| {
-                format!("no package named '{name}' in {}/", packages_dir.display())
-            })?;
-            let tag = state::load_pending_version(state_dir, name)
-                .with_context(|| format!("'{name}' has no pending review"))?;
-            approve(state_dir, work_dir, name, pkg, &tag)
-        }
-        _ => bail!("usage: pkgwatch review [<name> --approve]"),
-    }
-}
-
-fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &str) -> Result<()> {
-    let client = build_client()?;
-    let endpoints = GithubEndpoints::default();
-
-    // Re-verify rather than trusting the earlier flag: the artifact at
-    // this tag could in principle have changed since it was queued.
-    let fetched = fetch_and_verify(&client, &endpoints, work_dir, name, pkg, tag)?;
-    if !fetched.verification.passed {
-        bail!(
-            "re-verification failed on approve: {}",
-            fetched.verification.justification
-        );
-    }
-    println!(
-        "re-verified (tier {}): {}",
-        fetched.verification.tier, fetched.verification.justification
-    );
-
-    build_and_publish(name, pkg, tag, &fetched)?;
-    state::save_last_version(state_dir, name, tag)?;
-    state::clear_pending_version(state_dir, name)?;
-    println!("approved and published {name} {tag}");
-    Ok(())
-}

src/pipeline.rs

@@ -0,0 +1,341 @@
+//! Orchestrates one run of check -> fetch -> verify -> build ->
+//! sanity-check -> publish across every configured package, plus the
+//! `review` subcommand for tier 4-6 approvals. The only module that calls
+//! more than one other pipeline-stage module — see ARCHITECTURE.md > "main
+//! is a dispatcher, not the program" for why this lives here and not in
+//! `main.rs`.
+
+use crate::builder;
+use crate::checker;
+use crate::config::{self, Package};
+use crate::fetcher::{self, DownloadedAsset};
+use crate::github::GithubEndpoints;
+use crate::publisher;
+use crate::sanity;
+use crate::state;
+use crate::verifier::{self, VerificationResult};
+use anyhow::{Context, Result, bail};
+use std::path::{Path, PathBuf};
+
+const PACKAGES_DIR: &str = "packages.d";
+const STATE_DIR: &str = "state";
+const WORK_DIR: &str = "work";
+/// Not a repo pkgwatch invents: this is the existing, already-registered
+/// local pacman repo on this box (see `[custom]` in /etc/pacman.conf and
+/// its `Server = file://...` line). pkgwatch adds packages to it; it does
+/// not create the repo or touch pacman.conf.
+const CUSTOM_REPO_NAME: &str = "custom";
+const CUSTOM_REPO_SUBPATH: &str = ".local/share/pacman/custom";
+
+fn build_client() -> Result<reqwest::blocking::Client> {
+    Ok(reqwest::blocking::Client::builder()
+        .user_agent("pkgwatch/0.1 (PoC; https://code.austinschaefer.com)")
+        .build()?)
+}
+
+fn custom_repo_dir() -> Result<PathBuf> {
+    // Override for testing against a scratch repo instead of the real one
+    // at $HOME/.local/share/pacman/custom.
+    if let Ok(dir) = std::env::var("PKGWATCH_REPO_DIR") {
+        return Ok(PathBuf::from(dir));
+    }
+    let home = std::env::var("HOME").context("HOME is not set")?;
+    Ok(Path::new(&home).join(CUSTOM_REPO_SUBPATH))
+}
+
+pub fn run_check() -> Result<()> {
+    let client = build_client()?;
+    let endpoints = GithubEndpoints::default();
+    let packages_dir = Path::new(PACKAGES_DIR);
+    let state_dir = Path::new(STATE_DIR);
+    let work_dir = Path::new(WORK_DIR);
+
+    let packages = config::load_packages_dir(packages_dir)?;
+    if packages.is_empty() {
+        println!("no packages configured under {}/", packages_dir.display());
+        return Ok(());
+    }
+
+    let mut any_failed = false;
+    for (name, pkg) in &packages {
+        println!("== {name} ({}) ==", pkg.repo);
+        if let Err(err) = process_package(&client, &endpoints, state_dir, work_dir, name, pkg) {
+            eprintln!("  error: {err:#}");
+            any_failed = true;
+        }
+    }
+
+    if any_failed {
+        bail!("one or more packages failed — see errors above");
+    }
+    Ok(())
+}
+
+/// What to do about a package after verification, derived purely from the
+/// verification outcome and whether this exact version is already queued
+/// for review — no I/O. See ARCHITECTURE.md > "separate pure decision
+/// logic from I/O."
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum TierAction {
+    /// Verification failed outright — don't build, don't touch state.
+    VerificationFailed,
+    /// Tier 1-3: safe to auto-build and publish immediately.
+    Publish,
+    /// Tier 4-6, and this exact version was already flagged on an earlier
+    /// run — nothing new to report.
+    StillPending,
+    /// Tier 4-6, and this version hasn't been flagged yet.
+    NewlyPending,
+}
+
+fn decide_tier_action(tier: u8, passed: bool, already_pending_this_version: bool) -> TierAction {
+    if !passed {
+        return TierAction::VerificationFailed;
+    }
+    if tier <= 3 {
+        return TierAction::Publish;
+    }
+    if already_pending_this_version {
+        TierAction::StillPending
+    } else {
+        TierAction::NewlyPending
+    }
+}
+
+fn process_package(
+    client: &reqwest::blocking::Client,
+    endpoints: &GithubEndpoints,
+    state_dir: &Path,
+    work_dir: &Path,
+    name: &str,
+    pkg: &Package,
+) -> Result<()> {
+    let latest = checker::latest_github_release(client, endpoints, &pkg.repo)?;
+    let last_seen = state::load_last_version(state_dir, name);
+    if last_seen.as_deref() == Some(latest.as_str()) {
+        println!("  up to date at {latest}");
+        return Ok(());
+    }
+    println!("  new version detected: {latest} (previously: {last_seen:?})");
+
+    let fetched = fetch_and_verify(client, endpoints, work_dir, name, pkg, &latest)?;
+    println!("  fetched {}", fetched.asset.path.display());
+    println!(
+        "  verification (tier {}): {} — {}",
+        fetched.verification.tier,
+        if fetched.verification.passed {
+            "PASS"
+        } else {
+            "FAIL"
+        },
+        fetched.verification.justification
+    );
+
+    let already_pending =
+        state::load_pending_version(state_dir, name).as_deref() == Some(latest.as_str());
+    match decide_tier_action(
+        fetched.verification.tier,
+        fetched.verification.passed,
+        already_pending,
+    ) {
+        TierAction::VerificationFailed => {
+            println!("  verification failed — not publishing, not updating state");
+        }
+        TierAction::Publish => {
+            println!("  tier 1-3 pass: building + publishing");
+            build_and_publish(name, pkg, &latest, &fetched)?;
+            state::save_last_version(state_dir, name, &latest)?;
+            state::clear_pending_version(state_dir, name)?;
+            println!("  published {name} {latest}");
+        }
+        TierAction::StillPending => {
+            println!("  tier 4-6 pass: still pending review (`pkgwatch review` to see it)");
+        }
+        TierAction::NewlyPending => {
+            state::save_pending_version(state_dir, name, &latest)?;
+            println!("  tier 4-6 pass: flagged for human review (`pkgwatch review` to approve)");
+        }
+    }
+    Ok(())
+}
+
+struct FetchVerifyResult {
+    version: String,
+    asset_name: String,
+    dest_dir: PathBuf,
+    asset: DownloadedAsset,
+    verification: VerificationResult,
+}
+
+/// Shared by the normal check loop (tier 1-3 auto-path) and `pkgwatch
+/// review --approve` (which re-verifies before publishing rather than
+/// trusting a possibly-stale flag from an earlier run).
+fn fetch_and_verify(
+    client: &reqwest::blocking::Client,
+    endpoints: &GithubEndpoints,
+    work_dir: &Path,
+    name: &str,
+    pkg: &Package,
+    tag: &str,
+) -> Result<FetchVerifyResult> {
+    let version = checker::version_from_tag(tag).to_string();
+    let asset_name = pkg.asset_pattern.replace("{version}", &version);
+    let dest_dir = work_dir.join(name).join(tag);
+
+    let asset = fetcher::download_asset(client, endpoints, &pkg.repo, tag, &asset_name, &dest_dir)?;
+    let verification = verifier::verify(
+        client,
+        endpoints,
+        &pkg.verification,
+        &pkg.repo,
+        tag,
+        &asset.path,
+        &dest_dir,
+    )?;
+
+    Ok(FetchVerifyResult {
+        version,
+        asset_name,
+        dest_dir,
+        asset,
+        verification,
+    })
+}
+
+fn build_and_publish(
+    name: &str,
+    pkg: &Package,
+    tag: &str,
+    fetched: &FetchVerifyResult,
+) -> Result<()> {
+    // Fail fast if the repo isn't registered in pacman.conf, before
+    // spending several seconds on a makepkg build that would otherwise
+    // succeed and then publish somewhere pacman never syncs from.
+    let repo_dir = custom_repo_dir()?;
+    publisher::ensure_registered(CUSTOM_REPO_NAME, &repo_dir)?;
+
+    let build_dir = fetched.dest_dir.join("build");
+    let req = builder::BuildRequest {
+        pkg_name: name,
+        pkg,
+        version: &fetched.version,
+        repo: &pkg.repo,
+        asset_name: &fetched.asset_name,
+        download_url: &fetched.asset.download_url,
+        artifact_path: &fetched.asset.path,
+    };
+    let built = builder::build(&req, &build_dir)?;
+    println!("  built {}", built.package_path.display());
+
+    if let Some(check) = &pkg.sanity_check {
+        let bin_dir = built.pkgdir.join("usr/bin");
+        sanity::run(check, &bin_dir, &fetched.version)
+            .with_context(|| format!("sanity check for {name} {tag}"))?;
+        println!("  sanity check passed");
+    }
+
+    let published = publisher::publish(&built.package_path, &repo_dir, CUSTOM_REPO_NAME)?;
+    println!(
+        "  added to {} repo: {}",
+        CUSTOM_REPO_NAME,
+        published.display()
+    );
+    println!(
+        "  not installed automatically — run `sudo pacman -Syu` (or `sudo pacman -S {name}`) to pick it up"
+    );
+    Ok(())
+}
+
+pub fn run_review(args: &[String]) -> Result<()> {
+    let packages_dir = Path::new(PACKAGES_DIR);
+    let state_dir = Path::new(STATE_DIR);
+    let work_dir = Path::new(WORK_DIR);
+    let packages = config::load_packages_dir(packages_dir)?;
+
+    match args {
+        [] => {
+            let mut any = false;
+            for (name, _) in &packages {
+                if let Some(pending) = state::load_pending_version(state_dir, name) {
+                    println!(
+                        "{name}: {pending} pending review (run `pkgwatch review {name} --approve`)"
+                    );
+                    any = true;
+                }
+            }
+            if !any {
+                println!("no packages pending review");
+            }
+            Ok(())
+        }
+        [name, flag] if flag == "--approve" => {
+            let (_, pkg) = packages.iter().find(|(n, _)| n == name).with_context(|| {
+                format!("no package named '{name}' in {}/", packages_dir.display())
+            })?;
+            let tag = state::load_pending_version(state_dir, name)
+                .with_context(|| format!("'{name}' has no pending review"))?;
+            approve(state_dir, work_dir, name, pkg, &tag)
+        }
+        _ => bail!("usage: pkgwatch review [<name> --approve]"),
+    }
+}
+
+fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &str) -> Result<()> {
+    let client = build_client()?;
+    let endpoints = GithubEndpoints::default();
+
+    // Re-verify rather than trusting the earlier flag: the artifact at
+    // this tag could in principle have changed since it was queued.
+    let fetched = fetch_and_verify(&client, &endpoints, work_dir, name, pkg, tag)?;
+    if !fetched.verification.passed {
+        bail!(
+            "re-verification failed on approve: {}",
+            fetched.verification.justification
+        );
+    }
+    println!(
+        "re-verified (tier {}): {}",
+        fetched.verification.tier, fetched.verification.justification
+    );
+
+    build_and_publish(name, pkg, tag, &fetched)?;
+    state::save_last_version(state_dir, name, tag)?;
+    state::clear_pending_version(state_dir, name)?;
+    println!("approved and published {name} {tag}");
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn decide_tier_action_failed_verification_overrides_everything() {
+        assert_eq!(
+            decide_tier_action(2, false, false),
+            TierAction::VerificationFailed
+        );
+        assert_eq!(
+            decide_tier_action(4, false, true),
+            TierAction::VerificationFailed
+        );
+    }
+
+    #[test]
+    fn decide_tier_action_tier_1_to_3_publishes() {
+        assert_eq!(decide_tier_action(1, true, false), TierAction::Publish);
+        assert_eq!(decide_tier_action(2, true, false), TierAction::Publish);
+        assert_eq!(decide_tier_action(3, true, true), TierAction::Publish);
+    }
+
+    #[test]
+    fn decide_tier_action_tier_4_to_6_newly_pending_when_not_seen_before() {
+        assert_eq!(decide_tier_action(4, true, false), TierAction::NewlyPending);
+        assert_eq!(decide_tier_action(6, true, false), TierAction::NewlyPending);
+    }
+
+    #[test]
+    fn decide_tier_action_tier_4_to_6_still_pending_when_already_flagged() {
+        assert_eq!(decide_tier_action(4, true, true), TierAction::StillPending);
+    }
+}

src/publisher.rs

@@ -1,3 +1,8 @@
+//! Gets a built package into the local pacman repo: copies it in, runs
+//! `repo-add`, and checks the repo is actually registered in
+//! `/etc/pacman.conf` first. The only module that touches the repo
+//! directory or pacman.conf.
+
 use anyhow::{Context, Result, bail};
 use std::path::{Path, PathBuf};
 use std::process::Command;

src/sanity.rs

@@ -1,3 +1,7 @@
+//! Post-build correctness check: runs the freshly built binary and
+//! confirms it reports the version pkgwatch believes it just built. Not a
+//! security control — see SPEC.md > Verification trust tiers.
+
 use crate::config::SanityCheck;
 use anyhow::{Context, Result, bail};
 use regex::Regex;

src/state.rs

@@ -1,3 +1,7 @@
+//! Persists two independent per-package facts as plain files: the last
+//! published version, and any version currently pending human review.
+//! The only module that touches `state/` on disk.
+
 use anyhow::Result;
 use std::path::Path;
 

src/verifier.rs

@@ -1,3 +1,8 @@
+//! Runs the trust-tier-specific check declared for a package against a
+//! downloaded artifact, and reports a pass/fail plus the tier it implies.
+//! The only module that knows what each `Verification::method` actually
+//! proves — see SPEC.md > Verification trust tiers.
+
 use crate::checker::version_from_tag;
 use crate::config::Verification;
 use crate::fetcher;