Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
//! 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
|
2026-09-17 11:22:40 +00:00
|
|
|
//! more than one other pipeline-stage module — see docs/ARCHITECTURE.md > "main
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
//! 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
|
2026-09-17 11:22:40 +00:00
|
|
|
/// for review — no I/O. See docs/ARCHITECTURE.md > "separate pure decision
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
/// 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
|
|
|
|
|
);
|
|
|
|
|
|
Fix issues from code review: shell-escaping gap, exit code, and more
A single-agent code review of this branch's diff (builder/pipeline/
publisher/sanity/hash + main/config/fetcher/state/verifier changes)
found six real issues, all fixed here:
- builder.rs: validate_shell_safe only rejected a literal single quote
and newline, written for the single-quoted PKGBUILD fields. But
asset_name (via install_source) and binary_name land in the install()
line, which is necessarily double-quoted so ${srcdir}/${pkgdir} can
expand — where $, backtick, and backslash are still live. Not
currently exploitable (the one variable component, version, is already
independently constrained by validate_pkgver's strict charset), but a
latent gap relying on that coincidence rather than the validator
actually covering its real use context. Widened the reject-list to
cover both quoting styles, added regression tests including one at the
generate_pkgbuild level. Corrected SPEC.md's "single-quoted" claim to
match.
- pipeline.rs: a verification failure returned Ok(()) from
process_package, so run_check never counted it as a failure and the
process exited 0 even on a failed cryptographic/attestation check —
exactly the event a monitoring setup (systemd OnFailure=, cron
mail-on-error) most needs a non-zero exit to catch. Now bails, which
run_check already treats as a package failure. Added an integration
test against a mocked GitHub server exercising this exact path.
- builder.rs: find_built_package hardcoded the .pkg.tar.zst suffix, so a
box with a different PKGEXT in makepkg.conf would report a false
"makepkg failed" for a build that actually succeeded. Widened to match
any .pkg.tar.* compression. Added direct unit tests (it had none).
- pipeline.rs: a newer tier 4-6 version silently overwrote a still-
unreviewed older pending version with no indication anything was
superseded. Now says so explicitly.
- hash.rs: builder/verifier each read a whole downloaded artifact into
memory via std::fs::read just to hash it, doubling peak memory for no
reason since the file's already on disk. Added sha256_hex_file,
streamed in fixed-size chunks; both callers switched to it.
- Deduplicated two near-identical test-only "write an executable shell
script" helpers (publisher.rs, sanity.rs) into a shared
src/test_support.rs.
75 tests (was 63), cargo make ci clean. Re-verified end to end against
the real astral-sh/uv release after all six fixes — build, sanity check,
and publish into a scratch repo all still succeed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 10:06:40 +00:00
|
|
|
let previously_pending = state::load_pending_version(state_dir, name);
|
|
|
|
|
let already_pending = previously_pending.as_deref() == Some(latest.as_str());
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
match decide_tier_action(
|
|
|
|
|
fetched.verification.tier,
|
|
|
|
|
fetched.verification.passed,
|
|
|
|
|
already_pending,
|
|
|
|
|
) {
|
Fix issues from code review: shell-escaping gap, exit code, and more
A single-agent code review of this branch's diff (builder/pipeline/
publisher/sanity/hash + main/config/fetcher/state/verifier changes)
found six real issues, all fixed here:
- builder.rs: validate_shell_safe only rejected a literal single quote
and newline, written for the single-quoted PKGBUILD fields. But
asset_name (via install_source) and binary_name land in the install()
line, which is necessarily double-quoted so ${srcdir}/${pkgdir} can
expand — where $, backtick, and backslash are still live. Not
currently exploitable (the one variable component, version, is already
independently constrained by validate_pkgver's strict charset), but a
latent gap relying on that coincidence rather than the validator
actually covering its real use context. Widened the reject-list to
cover both quoting styles, added regression tests including one at the
generate_pkgbuild level. Corrected SPEC.md's "single-quoted" claim to
match.
- pipeline.rs: a verification failure returned Ok(()) from
process_package, so run_check never counted it as a failure and the
process exited 0 even on a failed cryptographic/attestation check —
exactly the event a monitoring setup (systemd OnFailure=, cron
mail-on-error) most needs a non-zero exit to catch. Now bails, which
run_check already treats as a package failure. Added an integration
test against a mocked GitHub server exercising this exact path.
- builder.rs: find_built_package hardcoded the .pkg.tar.zst suffix, so a
box with a different PKGEXT in makepkg.conf would report a false
"makepkg failed" for a build that actually succeeded. Widened to match
any .pkg.tar.* compression. Added direct unit tests (it had none).
- pipeline.rs: a newer tier 4-6 version silently overwrote a still-
unreviewed older pending version with no indication anything was
superseded. Now says so explicitly.
- hash.rs: builder/verifier each read a whole downloaded artifact into
memory via std::fs::read just to hash it, doubling peak memory for no
reason since the file's already on disk. Added sha256_hex_file,
streamed in fixed-size chunks; both callers switched to it.
- Deduplicated two near-identical test-only "write an executable shell
script" helpers (publisher.rs, sanity.rs) into a shared
src/test_support.rs.
75 tests (was 63), cargo make ci clean. Re-verified end to end against
the real astral-sh/uv release after all six fixes — build, sanity check,
and publish into a scratch repo all still succeed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 10:06:40 +00:00
|
|
|
// Bail rather than just print-and-return: a verification failure
|
|
|
|
|
// is exactly the kind of event a monitoring setup (systemd
|
|
|
|
|
// OnFailure=, cron mail-on-error) needs a non-zero exit to catch —
|
|
|
|
|
// see run_check, which treats an Err here as a failed package.
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
TierAction::VerificationFailed => {
|
Fix issues from code review: shell-escaping gap, exit code, and more
A single-agent code review of this branch's diff (builder/pipeline/
publisher/sanity/hash + main/config/fetcher/state/verifier changes)
found six real issues, all fixed here:
- builder.rs: validate_shell_safe only rejected a literal single quote
and newline, written for the single-quoted PKGBUILD fields. But
asset_name (via install_source) and binary_name land in the install()
line, which is necessarily double-quoted so ${srcdir}/${pkgdir} can
expand — where $, backtick, and backslash are still live. Not
currently exploitable (the one variable component, version, is already
independently constrained by validate_pkgver's strict charset), but a
latent gap relying on that coincidence rather than the validator
actually covering its real use context. Widened the reject-list to
cover both quoting styles, added regression tests including one at the
generate_pkgbuild level. Corrected SPEC.md's "single-quoted" claim to
match.
- pipeline.rs: a verification failure returned Ok(()) from
process_package, so run_check never counted it as a failure and the
process exited 0 even on a failed cryptographic/attestation check —
exactly the event a monitoring setup (systemd OnFailure=, cron
mail-on-error) most needs a non-zero exit to catch. Now bails, which
run_check already treats as a package failure. Added an integration
test against a mocked GitHub server exercising this exact path.
- builder.rs: find_built_package hardcoded the .pkg.tar.zst suffix, so a
box with a different PKGEXT in makepkg.conf would report a false
"makepkg failed" for a build that actually succeeded. Widened to match
any .pkg.tar.* compression. Added direct unit tests (it had none).
- pipeline.rs: a newer tier 4-6 version silently overwrote a still-
unreviewed older pending version with no indication anything was
superseded. Now says so explicitly.
- hash.rs: builder/verifier each read a whole downloaded artifact into
memory via std::fs::read just to hash it, doubling peak memory for no
reason since the file's already on disk. Added sha256_hex_file,
streamed in fixed-size chunks; both callers switched to it.
- Deduplicated two near-identical test-only "write an executable shell
script" helpers (publisher.rs, sanity.rs) into a shared
src/test_support.rs.
75 tests (was 63), cargo make ci clean. Re-verified end to end against
the real astral-sh/uv release after all six fixes — build, sanity check,
and publish into a scratch repo all still succeed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 10:06:40 +00:00
|
|
|
bail!("verification failed — not publishing, not updating state");
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
}
|
|
|
|
|
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)?;
|
Fix issues from code review: shell-escaping gap, exit code, and more
A single-agent code review of this branch's diff (builder/pipeline/
publisher/sanity/hash + main/config/fetcher/state/verifier changes)
found six real issues, all fixed here:
- builder.rs: validate_shell_safe only rejected a literal single quote
and newline, written for the single-quoted PKGBUILD fields. But
asset_name (via install_source) and binary_name land in the install()
line, which is necessarily double-quoted so ${srcdir}/${pkgdir} can
expand — where $, backtick, and backslash are still live. Not
currently exploitable (the one variable component, version, is already
independently constrained by validate_pkgver's strict charset), but a
latent gap relying on that coincidence rather than the validator
actually covering its real use context. Widened the reject-list to
cover both quoting styles, added regression tests including one at the
generate_pkgbuild level. Corrected SPEC.md's "single-quoted" claim to
match.
- pipeline.rs: a verification failure returned Ok(()) from
process_package, so run_check never counted it as a failure and the
process exited 0 even on a failed cryptographic/attestation check —
exactly the event a monitoring setup (systemd OnFailure=, cron
mail-on-error) most needs a non-zero exit to catch. Now bails, which
run_check already treats as a package failure. Added an integration
test against a mocked GitHub server exercising this exact path.
- builder.rs: find_built_package hardcoded the .pkg.tar.zst suffix, so a
box with a different PKGEXT in makepkg.conf would report a false
"makepkg failed" for a build that actually succeeded. Widened to match
any .pkg.tar.* compression. Added direct unit tests (it had none).
- pipeline.rs: a newer tier 4-6 version silently overwrote a still-
unreviewed older pending version with no indication anything was
superseded. Now says so explicitly.
- hash.rs: builder/verifier each read a whole downloaded artifact into
memory via std::fs::read just to hash it, doubling peak memory for no
reason since the file's already on disk. Added sha256_hex_file,
streamed in fixed-size chunks; both callers switched to it.
- Deduplicated two near-identical test-only "write an executable shell
script" helpers (publisher.rs, sanity.rs) into a shared
src/test_support.rs.
75 tests (was 63), cargo make ci clean. Re-verified end to end against
the real astral-sh/uv release after all six fixes — build, sanity check,
and publish into a scratch repo all still succeed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 10:06:40 +00:00
|
|
|
match previously_pending {
|
|
|
|
|
Some(superseded) => println!(
|
|
|
|
|
" tier 4-6 pass: flagged for human review, superseding still-unreviewed {superseded} (`pkgwatch review` to approve {latest})"
|
|
|
|
|
),
|
|
|
|
|
None => println!(
|
|
|
|
|
" tier 4-6 pass: flagged for human review (`pkgwatch review` to approve)"
|
|
|
|
|
),
|
|
|
|
|
}
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
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);
|
|
|
|
|
}
|
Fix issues from code review: shell-escaping gap, exit code, and more
A single-agent code review of this branch's diff (builder/pipeline/
publisher/sanity/hash + main/config/fetcher/state/verifier changes)
found six real issues, all fixed here:
- builder.rs: validate_shell_safe only rejected a literal single quote
and newline, written for the single-quoted PKGBUILD fields. But
asset_name (via install_source) and binary_name land in the install()
line, which is necessarily double-quoted so ${srcdir}/${pkgdir} can
expand — where $, backtick, and backslash are still live. Not
currently exploitable (the one variable component, version, is already
independently constrained by validate_pkgver's strict charset), but a
latent gap relying on that coincidence rather than the validator
actually covering its real use context. Widened the reject-list to
cover both quoting styles, added regression tests including one at the
generate_pkgbuild level. Corrected SPEC.md's "single-quoted" claim to
match.
- pipeline.rs: a verification failure returned Ok(()) from
process_package, so run_check never counted it as a failure and the
process exited 0 even on a failed cryptographic/attestation check —
exactly the event a monitoring setup (systemd OnFailure=, cron
mail-on-error) most needs a non-zero exit to catch. Now bails, which
run_check already treats as a package failure. Added an integration
test against a mocked GitHub server exercising this exact path.
- builder.rs: find_built_package hardcoded the .pkg.tar.zst suffix, so a
box with a different PKGEXT in makepkg.conf would report a false
"makepkg failed" for a build that actually succeeded. Widened to match
any .pkg.tar.* compression. Added direct unit tests (it had none).
- pipeline.rs: a newer tier 4-6 version silently overwrote a still-
unreviewed older pending version with no indication anything was
superseded. Now says so explicitly.
- hash.rs: builder/verifier each read a whole downloaded artifact into
memory via std::fs::read just to hash it, doubling peak memory for no
reason since the file's already on disk. Added sha256_hex_file,
streamed in fixed-size chunks; both callers switched to it.
- Deduplicated two near-identical test-only "write an executable shell
script" helpers (publisher.rs, sanity.rs) into a shared
src/test_support.rs.
75 tests (was 63), cargo make ci clean. Re-verified end to end against
the real astral-sh/uv release after all six fixes — build, sanity check,
and publish into a scratch repo all still succeed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 10:06:40 +00:00
|
|
|
|
|
|
|
|
/// Regression test for the exit-code gap this PR fixes: a verification
|
|
|
|
|
/// failure previously returned `Ok(())` from `process_package`, so
|
|
|
|
|
/// `run_check` never counted it as a failure and the process exited 0
|
|
|
|
|
/// even though the single most security-relevant check had failed.
|
|
|
|
|
/// Exercises the full check -> fetch -> verify path against a mocked
|
|
|
|
|
/// GitHub (no real network), stopping before any build/publish step
|
|
|
|
|
/// since verification failure returns before reaching those.
|
|
|
|
|
#[test]
|
|
|
|
|
fn process_package_returns_err_on_verification_failure() {
|
|
|
|
|
let mut server = mockito::Server::new();
|
|
|
|
|
let endpoints = GithubEndpoints {
|
|
|
|
|
web: server.url(),
|
|
|
|
|
api: server.url(),
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
let feed = format!(
|
|
|
|
|
r#"<feed><link rel="alternate" href="{}/o/r/releases/tag/v1.0.0"/></feed>"#,
|
|
|
|
|
server.url()
|
|
|
|
|
);
|
|
|
|
|
let _atom = server
|
|
|
|
|
.mock("GET", "/o/r/releases.atom")
|
|
|
|
|
.with_status(200)
|
|
|
|
|
.with_body(feed)
|
|
|
|
|
.create();
|
|
|
|
|
|
|
|
|
|
let asset_url = format!("{}/download/thing.tar.gz", server.url());
|
|
|
|
|
let sums_url = format!("{}/download/SHA256SUMS", server.url());
|
|
|
|
|
let release_body = format!(
|
|
|
|
|
r#"{{"assets": [
|
|
|
|
|
{{"name": "thing.tar.gz", "browser_download_url": "{asset_url}"}},
|
|
|
|
|
{{"name": "SHA256SUMS", "browser_download_url": "{sums_url}"}}
|
|
|
|
|
]}}"#
|
|
|
|
|
);
|
|
|
|
|
let _release = server
|
|
|
|
|
.mock("GET", "/repos/o/r/releases/tags/v1.0.0")
|
|
|
|
|
.with_status(200)
|
|
|
|
|
.with_body(release_body)
|
|
|
|
|
.create();
|
|
|
|
|
let _asset = server
|
|
|
|
|
.mock("GET", "/download/thing.tar.gz")
|
|
|
|
|
.with_status(200)
|
|
|
|
|
.with_body(b"artifact-bytes".as_slice())
|
|
|
|
|
.create();
|
|
|
|
|
// Wrong hash for "artifact-bytes" — forces a verification failure.
|
|
|
|
|
let _sums = server
|
|
|
|
|
.mock("GET", "/download/SHA256SUMS")
|
|
|
|
|
.with_status(200)
|
|
|
|
|
.with_body(
|
|
|
|
|
"0000000000000000000000000000000000000000000000000000000000000000 thing.tar.gz\n",
|
|
|
|
|
)
|
|
|
|
|
.create();
|
|
|
|
|
|
|
|
|
|
let pkg: Package = toml::from_str(
|
|
|
|
|
r#"
|
|
|
|
|
repo = "o/r"
|
|
|
|
|
asset_pattern = "thing.tar.gz"
|
|
|
|
|
[verification]
|
|
|
|
|
method = "same-origin-sha256"
|
|
|
|
|
checksum_asset_pattern = "SHA256SUMS"
|
|
|
|
|
"#,
|
|
|
|
|
)
|
|
|
|
|
.unwrap();
|
|
|
|
|
|
|
|
|
|
let client = reqwest::blocking::Client::new();
|
|
|
|
|
let state_dir = tempfile::tempdir().unwrap();
|
|
|
|
|
let work_dir = tempfile::tempdir().unwrap();
|
|
|
|
|
|
|
|
|
|
let err = process_package(
|
|
|
|
|
&client,
|
|
|
|
|
&endpoints,
|
|
|
|
|
state_dir.path(),
|
|
|
|
|
work_dir.path(),
|
|
|
|
|
"thing",
|
|
|
|
|
&pkg,
|
|
|
|
|
)
|
|
|
|
|
.unwrap_err();
|
|
|
|
|
|
|
|
|
|
assert!(err.to_string().contains("verification failed"));
|
|
|
|
|
// Neither published nor queued for review — a failed verification
|
|
|
|
|
// shouldn't leave any trace in state.
|
|
|
|
|
assert_eq!(state::load_last_version(state_dir.path(), "thing"), None);
|
|
|
|
|
assert_eq!(state::load_pending_version(state_dir.path(), "thing"), None);
|
|
|
|
|
}
|
Add ARCHITECTURE.md and apply it to this PR's code
Researched current industry practice on code organization/maintainability
(Ousterhout's deep modules and information hiding, package-by-feature vs.
package-by-layer, functional-core/imperative-shell testability, tech-debt
prevention via ADR-equivalent inline rationale) and wrote it into
ARCHITECTURE.md as a set of concrete, project-specific rules rather than
a generic essay — each principle cites a real example already in this
codebase or fixed by this commit. Cross-linked from SPEC.md, which stays
about product design, not code organization.
Applied it to this PR's own code:
- Pulled process_package/fetch_and_verify/build_and_publish/run_review/
approve out of main.rs into a new pipeline.rs. main.rs's own main() had
grown to 278 lines and zero tests by treating "it's just the entry
point" as an excuse to skip separating logic from wiring; now main.rs
is argv dispatch only.
- Extracted decide_tier_action as a pure function (verification outcome +
pending-state -> what to do), replacing dispatch logic that was
previously inlined into a function that also made the real network/
build calls. Four unit tests, no I/O, covering all four outcomes.
- Added a `//!` module doc comment to every file touched in this branch,
each stating that module's one job in a sentence, per the "deep
modules" principle the spec argues for.
Coverage's reported total drops (94% -> 78%) because pipeline.rs is
deliberately NOT excluded from it the way main.rs is, even though it's
mostly the same kind of untestable I/O orchestration — excluding it would
hide decide_tier_action's real unit-test coverage along with the untested
parts. Noted inline in Makefile.toml/ci.yml so the number doesn't look
like a quality regression at a glance.
Also added a project reference memory pointing at ARCHITECTURE.md rather
than duplicating its content there, per this session's own memory-hygiene
rules (architecture/conventions are derivable from the repo and shouldn't
be duplicated somewhere that can go stale).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-17 09:42:18 +00:00
|
|
|
}
|