pkgwatch/docs/ARCHITECTURE.md

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 (release_source, fetcher, verifier, builder, sanity, publisher, state), not generic buckets. (release_source is a directory module: the ReleaseSource trait and one file per host, re-exported from its mod.rs so the rest of the crate never names a host's file.) 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/ForgejoEndpoints (the ReleaseSource implementations, whose API root is what fetcher/verifier take), 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: release_source/github.rs's doc comment on GithubEndpoints::latest_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
• Package by feature vs. package by layer
• Coupling and cohesion as the foundations of a maintainable codebase
• Rust module and crate organization best practices
• Reducing technical debt in 2026 — IBM