From 1c132dccca2ca8a86118ebaeefb3b04ecef79fca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Villase=C3=B1or=20Montfort?= <195970+montfort@users.noreply.github.com> Date: Sat, 2 May 2026 19:25:03 -0600 Subject: [PATCH] =?UTF-8?q?feat(framework+cli):=20Phase=202=20PR=207=20?= =?UTF-8?q?=E2=80=94=20pre-PR=20hook=20+=20devtrail=20init=20--hooks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the opt-in friction layer that runs `devtrail charter drift` before `git push`. Per principle #6 (cognitive discipline > raw productivity), the hook is virtuous when consented to — and never installed by default. Framework: - dist/.devtrail/hooks/pre-pr.sh: bash hook that scans docs/charters/*.md for status: in-progress, runs `devtrail charter drift --range $UPSTREAM..HEAD` for each, exits non-zero if any reports unaccounted drift. AILOG-suppression in the CLI (PR 3) silences alerts on paths already documented as risks. Default upstream is origin/main; override via DEVTRAIL_UPSTREAM env var. Manual install: cp .devtrail/hooks/pre-pr.sh .git/hooks/pre-push. CLI: - New --hooks flag on `devtrail init` (default: false). After init succeeds, copies .devtrail/hooks/pre-pr.sh to .git/hooks/pre-push and makes it executable. Refuses to overwrite an existing pre-push hook (warns and continues without). Skips silently if the project isn't a git repository. Behavior tradeoffs: - The hook short-circuits when devtrail is not in PATH, so adopters who haven't installed the CLI yet can still push without friction. - The hook respects --no-verify (standard git escape hatch); the documented use case is "drift was intentional, captured in AILOG, push anyway". Tests: - 4 unit tests in init::hook_tests: * install_pre_pr_hook_copies_and_makes_executable (happy path) * install_pre_pr_hook_skips_when_not_a_git_repo * install_pre_pr_hook_refuses_to_overwrite_existing * install_pre_pr_hook_errors_when_source_missing 382/382 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- cli/src/commands/init.rs | 149 ++++++++++++++++++++++++++++++++- cli/src/main.rs | 8 +- dist/.devtrail/hooks/pre-pr.sh | 71 ++++++++++++++++ 3 files changed, 226 insertions(+), 2 deletions(-) create mode 100755 dist/.devtrail/hooks/pre-pr.sh diff --git a/cli/src/commands/init.rs b/cli/src/commands/init.rs index 015a446..0a96759 100644 --- a/cli/src/commands/init.rs +++ b/cli/src/commands/init.rs @@ -9,7 +9,7 @@ use crate::inject; use crate::manifest::DistManifest; use crate::utils; -pub fn run(path: &str) -> Result<()> { +pub fn run(path: &str, install_hooks: bool) -> Result<()> { let target = PathBuf::from(path) .canonicalize() .unwrap_or_else(|_| PathBuf::from(path)); @@ -61,6 +61,27 @@ pub fn run(path: &str) -> Result<()> { // Save checksums save_initial_checksums(&target, &release.tag_name)?; + // Install pre-PR hook (opt-in via --hooks). + if install_hooks { + match install_pre_pr_hook(&target) { + Ok(installed) => { + if installed { + println!( + " {} pre-PR hook installed at {}", + "✓".green().bold(), + ".git/hooks/pre-push".dimmed() + ); + } + } + Err(e) => { + utils::warn(&format!( + "Failed to install pre-PR hook: {}. Continuing without it.", + e + )); + } + } + } + // Print summary println!(); utils::success("DevTrail initialized successfully!"); @@ -298,6 +319,132 @@ fn save_initial_checksums(target: &Path, version: &str) -> Result<()> { Ok(()) } +/// Install `.devtrail/hooks/pre-pr.sh` as `.git/hooks/pre-push`. Returns +/// `Ok(true)` on success, `Ok(false)` if the project is not a git repo +/// (so the caller can skip silently). Errors only on actual filesystem +/// failures (the hook source is missing, the destination can't be written, +/// etc.). +fn install_pre_pr_hook(target: &Path) -> Result { + let git_dir = target.join(".git"); + if !git_dir.exists() { + utils::warn( + "Skipping --hooks: not a git repository (no .git/ directory). Run 'git init' first.", + ); + return Ok(false); + } + + let source = target.join(".devtrail/hooks/pre-pr.sh"); + if !source.exists() { + bail!( + "pre-PR hook source not found at {}. The framework distribution may be incomplete.", + source.display() + ); + } + + let hooks_dir = git_dir.join("hooks"); + std::fs::create_dir_all(&hooks_dir) + .with_context(|| format!("Failed to create {}", hooks_dir.display()))?; + + let dest = hooks_dir.join("pre-push"); + if dest.exists() { + utils::warn(&format!( + "Refusing to overwrite existing hook at {}. Move or remove it, then re-run with --hooks.", + dest.display() + )); + return Ok(false); + } + + std::fs::copy(&source, &dest) + .with_context(|| format!("Failed to copy hook to {}", dest.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(&dest)?.permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&dest, perms)?; + } + Ok(true) +} + +#[cfg(test)] +mod hook_tests { + use super::*; + use tempfile::TempDir; + + fn setup_tempdir_with_hook_source(tmp: &Path) { + std::fs::create_dir_all(tmp.join(".git/objects")).unwrap(); + std::fs::create_dir_all(tmp.join(".devtrail/hooks")).unwrap(); + std::fs::write( + tmp.join(".devtrail/hooks/pre-pr.sh"), + "#!/usr/bin/env bash\necho hook\n", + ) + .unwrap(); + } + + #[test] + fn install_pre_pr_hook_copies_and_makes_executable() { + let tmp = TempDir::new().unwrap(); + setup_tempdir_with_hook_source(tmp.path()); + + let installed = install_pre_pr_hook(tmp.path()).unwrap(); + assert!(installed); + + let dest = tmp.path().join(".git/hooks/pre-push"); + assert!(dest.exists()); + let body = std::fs::read_to_string(&dest).unwrap(); + assert!(body.contains("hook")); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&dest).unwrap().permissions().mode(); + assert_eq!(mode & 0o111, 0o111, "hook must be executable"); + } + } + + #[test] + fn install_pre_pr_hook_skips_when_not_a_git_repo() { + let tmp = TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join(".devtrail/hooks")).unwrap(); + std::fs::write( + tmp.path().join(".devtrail/hooks/pre-pr.sh"), + "#!/usr/bin/env bash\nexit 0\n", + ) + .unwrap(); + + let installed = install_pre_pr_hook(tmp.path()).unwrap(); + assert!(!installed); + } + + #[test] + fn install_pre_pr_hook_refuses_to_overwrite_existing() { + let tmp = TempDir::new().unwrap(); + setup_tempdir_with_hook_source(tmp.path()); + let dest = tmp.path().join(".git/hooks/pre-push"); + std::fs::create_dir_all(dest.parent().unwrap()).unwrap(); + std::fs::write(&dest, "#!/bin/sh\necho existing\n").unwrap(); + + let installed = install_pre_pr_hook(tmp.path()).unwrap(); + assert!(!installed); + + // Original content is preserved. + let body = std::fs::read_to_string(&dest).unwrap(); + assert!(body.contains("existing")); + } + + #[test] + fn install_pre_pr_hook_errors_when_source_missing() { + let tmp = TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join(".git/objects")).unwrap(); + // Note: no .devtrail/hooks/pre-pr.sh + + let result = install_pre_pr_hook(tmp.path()); + assert!(result.is_err()); + let msg = format!("{:?}", result.unwrap_err()); + assert!(msg.contains("pre-PR hook source not found")); + } +} + /// Simple recursive directory walker fn walkdir(dir: PathBuf) -> Result> { let mut files = Vec::new(); diff --git a/cli/src/main.rs b/cli/src/main.rs index 8bfcfd3..dd9140d 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -39,6 +39,12 @@ enum Commands { /// Target directory (default: current directory) #[arg(default_value = ".")] path: String, + /// After init, install the framework's pre-PR hook (runs + /// `devtrail charter drift` automatically before `git push`). + /// Opt-in per principle #6 — friction with consent. Requires the + /// project to be a git repository. + #[arg(long)] + hooks: bool, }, /// Update both framework and CLI to the latest version Update { @@ -308,7 +314,7 @@ fn main() { let cli = Cli::parse(); let result = match cli.command { - Commands::Init { path } => commands::init::run(&path), + Commands::Init { path, hooks } => commands::init::run(&path, hooks), Commands::Update { method } => commands::update::run(&method), Commands::UpdateFramework => commands::update_framework::run(), Commands::UpdateCli { method } => commands::update_cli::run(&method), diff --git a/dist/.devtrail/hooks/pre-pr.sh b/dist/.devtrail/hooks/pre-pr.sh new file mode 100755 index 0000000..11a462a --- /dev/null +++ b/dist/.devtrail/hooks/pre-pr.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# pre-pr.sh — Run `devtrail charter drift` on any Charter currently +# `status: in-progress` before opening a PR. Designed to be installed via +# `devtrail init --hooks` as a git pre-push hook, or invoked manually +# (e.g., from a Makefile target) when the operator prefers explicit invocation. +# +# Behavior: +# - For each Charter in docs/charters/*.md whose frontmatter has +# `status: in-progress`, run `devtrail charter drift --range +# ..HEAD`. AILOG-suppression in the CLI silences alerts on +# paths already documented as risks in the Charter's originating AILOGs. +# - Exits 0 when there's nothing in-progress, or when all in-progress +# Charters report clean (or AILOG-suppressed) drift. +# - Exits 1 when at least one Charter reports unaccounted drift, with a +# human-readable summary pointing to remediation paths. +# +# Configuration (environment): +# DEVTRAIL_UPSTREAM git ref to compare HEAD against (default: origin/main) +# +# Why this is opt-in: per devtrail-design-principles.md §6, friction is +# virtuous when the operator consents to it. We never install this hook +# automatically; adopters opt in via `devtrail init --hooks` or by copying +# this file into .git/hooks/pre-push themselves. + +set -euo pipefail + +if ! command -v devtrail >/dev/null 2>&1; then + # devtrail not installed (or not in PATH); the hook should be transparent + # rather than blocking pushes for adopters who haven't yet opted in. + exit 0 +fi + +UPSTREAM="${DEVTRAIL_UPSTREAM:-origin/main}" + +if [ ! -d docs/charters ]; then + exit 0 # repo doesn't use Charters +fi + +charters=$(grep -l '^status: in-progress' docs/charters/*.md 2>/dev/null || true) +if [ -z "$charters" ]; then + exit 0 # nothing in-progress; nothing to check +fi + +echo "[devtrail pre-pr] Checking drift on in-progress Charters (range: $UPSTREAM..HEAD)..." +echo "" + +exit_code=0 +for charter_file in $charters; do + charter_id=$(awk '/^charter_id:/ {print $2; exit}' "$charter_file" | tr -d '"') + if [ -z "$charter_id" ]; then + echo " [skip] $charter_file has no charter_id in frontmatter" + continue + fi + echo " -- $charter_id ($charter_file) --" + if ! devtrail charter drift "$charter_id" --range "$UPSTREAM..HEAD"; then + exit_code=1 + fi + echo "" +done + +if [ $exit_code -ne 0 ]; then + echo "" + echo "[devtrail pre-pr] Drift detected on at least one Charter." + echo " - Address it before opening the PR (see hints above), or" + echo " - Document the drift in an AILOG under '## Risk' as 'R" + echo " (new, not in Charter)' so the next run suppresses it, or" + echo " - Bypass with 'git push --no-verify' if the drift is intentional" + echo " and acknowledged." +fi + +exit $exit_code