diff --git a/Cargo.lock b/Cargo.lock index f6e55ec..234ea27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,15 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "aho-corasick" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c982642fa9e8606056828ee9a8505737230110bb1099153c79efe865c59d12ba" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "1.0.0" @@ -335,12 +344,13 @@ dependencies = [ [[package]] name = "gitkit" -version = "0.5.0" +version = "0.6.0" dependencies = [ "anyhow", "clap", "flate2", "inquire", + "regex", "serde", "serde_json", "serial_test", @@ -663,6 +673,35 @@ dependencies = [ "bitflags 2.13.0", ] +[[package]] +name = "regex" +version = "1.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f020237b6c8eed93db2e2cb53c00c60a8e1bc73da7d073199a1180401450218d" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad8553b9b26413251cbf30e620595c7a41b3887f03da04579c0e6b0d6a06b4b2" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" + [[package]] name = "ring" version = "0.17.14" diff --git a/Cargo.toml b/Cargo.toml index 4deb0cf..405d2bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "gitkit" -version = "0.5.0" +version = "0.6.0" edition = "2021" description = "Standalone CLI for configuring git repos — hooks, .gitignore, and .gitattributes" license = "MIT" @@ -21,6 +21,7 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" tar = "0.4" toml = "0.8" +regex = "1" ureq = "2" [dev-dependencies] diff --git a/README.md b/README.md index a224115..7b1922c 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Set up a git repo the way you actually work — one guided flow for hooks, `.git ### Demo -![Demo](assets/demo.gif) +![Demo](demo/dist/demo.gif) --- diff --git a/assets/demo.gif b/assets/demo.gif deleted file mode 100644 index 004babd..0000000 Binary files a/assets/demo.gif and /dev/null differ diff --git a/docs/cli-reference.md b/docs/cli-reference.md index d38962d..6751660 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -53,6 +53,20 @@ Running `gitkit` with no command starts the interactive wizard. `git commit --no-verify` and `git push --no-verify` bypass the lock — see [Lock](lock.md) for why that is accepted rather than defended against. +## Uninstall + +| Command | Description | +|---|---| +| `gitkit uninstall` | Remove gitkit hooks from every repository it has touched | +| `gitkit uninstall --data` | Also remove local state under `~/.gitkit` (builds, registry) | +| `gitkit uninstall --yes` | Skip the confirmation prompt | +| `gitkit uninstall --dry-run` | Print what would be done without changing anything | + +By default, `gitkit uninstall` lists every repository in the registry, shows what hooks are +installed, and asks for confirmation before removing anything. It restores any hand-written hook +that gitkit had absorbed when it first installed its dispatcher. The gitkit binary itself is never +removed — see [Installation](installation.md#uninstall) for how to remove it. + ## Ignore | Command | Description | diff --git a/docs/hooks.md b/docs/hooks.md index 41b174e..414edae 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -1,6 +1,6 @@ --- title: Hooks -description: Built-in hooks (conventional commits, no-body messages, AI trailer rejection, secret detection, branch naming, invisible Unicode detection) and custom shell commands. +description: Built-in hooks (conventional commits, no-body messages, AI trailer rejection, secret detection, branch naming, invisible Unicode detection, user-defined message rules) and custom shell commands. order: 4 --- @@ -15,6 +15,7 @@ Built-ins are embedded in the binary — no network required. | `conventional-commits` | `commit-msg` | Validates Conventional Commits format | | `no-body` | `commit-msg` | Rejects a commit message that has a body | | `no-trailers` | `commit-msg` | Rejects commit messages carrying AI attribution trailers | +| `message-rules` | `commit-msg` | Validates commit messages against user-defined regex rules in `.gitmessage-rules.json` | | `no-secrets` | `pre-commit` | Detects common secret patterns in staged changes | | `branch-naming` | `pre-commit` | Validates branch name matches convention | | `no-invisibles` | `pre-commit` | Rejects added lines carrying invisible Unicode characters | @@ -65,6 +66,98 @@ with" line. Genuine human `Co-Authored-By:` trailers are left untouched — a rule in a prompt is advisory, this hook is not. The commit is refused with the offending line and its line number; it never rewrites your message. +### `message-rules` + +Validates the commit message against **rules you define** in a committed +file at `.gitmessage-rules.json` in the repository root. Each rule is +a regex pattern with a direction (`must_match` or `must_not_match`), a +scope (`subject` or `whole_message`), and a message shown when the rule +fires. Because the rules file is tracked by git, it travels with the +repository and applies to everyone who clones it. + +#### Configuring rules + +Create `.gitmessage-rules.json` in the repository root with a JSON +array of rules: + +```json +[ + { + "name": "jira-prefix", + "pattern": "^[A-Z]+-\\d+", + "direction": "must_match", + "scope": "subject", + "message": "Subject must start with a JIRA ticket prefix (e.g. PROJ-123)" + } +] +``` + +A negative rule — forbidding something — uses `must_not_match`: + +```json +[ + { + "name": "no-trailer-in-subject", + "pattern": "Co-Authored-By:", + "direction": "must_not_match", + "scope": "subject", + "message": "Subject must not contain trailer lines; move Co-Authored-By to the body" + } +] +``` + +Commit this file to the repository so every contributor shares the same +rules. + +#### Directions + +- **`must_match`** — the pattern must match the scoped text. A JIRA prefix + rule is a positive match: the subject must contain `^[A-Z]+-\d+`. +- **`must_not_match`** — the pattern must not match. A forbidden-trailer + rule is a negative match: the subject must not contain `Co-Authored-By:`. + This catches what `no-trailers` misses — a trailer smuggled into the + subject line after a semicolon. + +#### Scopes + +- **`subject`** — only the first line of the commit message is checked. +- **`whole_message`** — the entire commit message (subject, body, trailers) + is checked. + +#### Installing + +```bash +gitkit hooks add message-rules +``` + +If no rules are configured, the command refuses with a clear message rather +than installing a hook that always passes. Patterns are validated at install +time — a regex that does not compile is rejected immediately, naming the +rule and the compile error, not deferred to commit time. + +#### Regex flavour + +Patterns use **Rust's `regex` crate** syntax, which is ERE-like: character +classes, alternation, grouping, anchors, and quantifiers all work. **No +lookahead, lookbehind, or backreferences** — if a pattern uses these PCRE +features, it will be rejected at configuration time with a compile error. +When in doubt, test the pattern with `rg ''` (ripgrep uses the +same engine). + +The installed hook delegates to `gitkit` itself for regex evaluation, so +the same engine that validated the pattern at install time evaluates it at +commit time — no lossy conversion to POSIX ERE. + +#### Multiple rules + +All rules run on every commit. The hook reports **every** failing rule, not +just the first, then exits non-zero. Rules compose — a JIRA prefix rule and +a subject-length rule are separate rules that fire independently. + +Revert (`Revert "..."`), merge (`Merge branch '...'`), `fixup!` and +`squash!` commit messages are auto-generated and are always accepted +regardless of rules. + ### `no-invisibles` Rejects a commit that **adds** a line containing an invisible Unicode diff --git a/docs/installation.md b/docs/installation.md index b591027..fedef89 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -65,11 +65,23 @@ the update to maintain consistency. ## Uninstall +First, remove gitkit's hooks from every repository it has touched: + +```bash +gitkit uninstall +``` + +This lists every repository in the registry, shows what hooks are installed, and asks for +confirmation before removing anything. It restores any hand-written hook that gitkit had absorbed +when it first installed its dispatcher. Add `--data` to also remove local state under `~/.gitkit` +(builds, registry). + +Then remove the binary itself: + **Linux / macOS:** ```bash rm -f ~/.local/bin/gitkit -rm -rf ~/.gitkit/ # saved builds (optional) ``` **Windows (PowerShell):** @@ -77,3 +89,9 @@ rm -rf ~/.gitkit/ # saved builds (optional) ```powershell Remove-Item "$env:LOCALAPPDATA\gitkit\gitkit.exe" -Force ``` + +If gitkit was installed via `cargo install gitkit`, remove it with: + +```bash +cargo uninstall gitkit +``` diff --git a/docs/lock.md b/docs/lock.md index 3efbdea..3791205 100644 --- a/docs/lock.md +++ b/docs/lock.md @@ -13,6 +13,7 @@ committing to a repository, locally, for the duration of a session. gitkit lock # block commits until `gitkit unlock` gitkit lock --push # block pushes instead of commits gitkit lock --all # block both commits and pushes +gitkit lock --refs # block reference updates (strongest lock) gitkit lock --reason "Agent session" # custom message shown on a blocked operation gitkit lock --timeout 30m # auto-expires after 30 minutes gitkit lock status # show whether a lock is active @@ -27,10 +28,10 @@ or modify which operations are locked without removing the existing lock. ## How it works `gitkit lock` writes a small JSON state file at `.git/gitkit.lock` and -installs `pre-commit` and/or `pre-push` hooks that read it. The hooks are -pure POSIX `sh` — no dependency on the `gitkit` binary — so they stay fast -on every commit and push. A missing, empty, or malformed lock file is always -treated as unlocked: a corrupt lock never blocks an operation. +installs hooks that read it. The hooks are pure POSIX `sh` — no dependency +on the `gitkit` binary — so they stay fast on every commit and push. A +missing, empty, or malformed lock file is always treated as unlocked: a +corrupt lock never blocks an operation. By default, `gitkit lock` blocks commits only. Use `--push` to add push blocking, or `--all` to block both. You can call lock multiple times to @@ -44,10 +45,60 @@ removes the backups. The lock is per-repository, local only, and never committed or pushed — it lives entirely under `.git/`. +## Axes: what the lock covers + +The lock has four independent axes, each controlled separately: + +| Axis | Flag | Hook installed | Bypass with `--no-verify`? | +|----------|-----------|-----------------------------|----------------------------| +| Commit | (default) | `pre-commit` | Yes | +| Push | `--push` | `pre-push` | Yes | +| Rebase | (always) | `pre-rebase` | Yes | +| Refs | `--refs` | `reference-transaction` | **No** | + +`--all` enables commit and push, but **not** refs. Reference protection +is a separate, stronger axis that must be opted into explicitly with +`--refs`. This is a deliberate product decision: silently freezing refs +when a user typed `--all` would surprise existing users. + +### Rebase blocking + +`gitkit lock` always installs a `pre-rebase` hook alongside whatever other +operations are locked. There is no `--rebase` flag — rebase blocking is +part of every lock, because a lock whose purpose is to hold a repository +still while an agent works in it should also hold rebases still. + +Like commit and push, rebase blocking can be bypassed with +`git rebase --no-verify`. + +### Reference protection (`--refs`) + +`gitkit lock --refs` installs a `reference-transaction` hook that rejects +updates to `HEAD` and `refs/heads/*` while allowing `refs/remotes/*` so +`git fetch` keeps working. This is the **only** lock axis that cannot be +bypassed with `--no-verify`, because git does not apply `--no-verify` to +the `reference-transaction` hook. + +**Trade-off**: this is the strongest lock gitkit offers, but it is also +the most intrusive. A `reference-transaction` hook that rejects too +broadly can make a repository feel broken in ways that are hard to +attribute. gitkit rejects narrowly (only `HEAD` and `refs/heads/*`), +but you should still be aware that enabling `--refs` changes the +behaviour of ordinary git commands like `git commit` (which updates +`HEAD`) and `git switch -c` (which creates a new `refs/heads/*` ref). + +**Branch creation**: with `--refs` active, `git switch -c new-branch` +and `git checkout -b new-branch` are blocked, because they write to +`refs/heads/`. Create branches before taking the lock, or use +`git update-ref` on `refs/remotes/*` if you need to record a position +without touching local refs. + +Every rejection message names `gitkit unlock` as the supported way out. + ## Status output Both `gitkit lock status` (human-readable) and `gitkit lock status --json` -(machine-readable) show per-operation status. This lets you see at a glance +(machine-readable) show per-axis status. This lets you see at a glance which operations are currently locked: ``` @@ -56,9 +107,12 @@ Locked at: 2026-01-01T10:00:00Z Expires at: 2026-01-01T10:30:00Z Commit: locked Push: not locked +Rebase: not locked +Refs: not locked ``` -This shows that commits are blocked, but pushes are allowed. +This shows that commits are blocked, but pushes, rebases, and ref +updates are allowed. ## Machine-readable status: `lock status --json` @@ -79,7 +133,7 @@ rely on a key being renamed or removed without a version bump): | Key | Type | Meaning | |---------------|-------------------|--------------------------------------------------------------------------| | `active` | `bool` | Whether the lock currently blocks the operations it lists — `false` if there is no lock, the lock file is malformed, `operations` is empty, or the lock has expired. | -| `operations` | `string[]` | The operations the lock covers. Can be `"commit"`, `"push"`, or both. Empty when there is no lock. | +| `operations` | `string[]` | The operations the lock covers. Can be `"commit"`, `"push"`, `"rebase"`, and/or `"refs"`. Empty when there is no lock. | | `locked_at` | `string \| null` | RFC 3339 timestamp the lock was set, or `null` when there is no lock. | | `expires_at` | `string \| null` | RFC 3339 timestamp the lock expires, or `null` for a lock with no timeout (or no lock at all). | | `reason` | `string \| null` | The `--reason` text, or `null` when there is no lock. | @@ -113,7 +167,7 @@ below is the supported contract for either path. | `locked_at` | `string` | RFC 3339 timestamp the lock was set. | | `expires_at` | `string \| null` | RFC 3339 timestamp the lock expires, or `null` for no timeout. | | `reason` | `string` | The `--reason` text, or empty string if none was given. | -| `operations` | `string[]` | The operations the lock covers. | +| `operations` | `string[]` | The operations the lock covers. Can include `"commit"`, `"push"`, `"rebase"`, and/or `"refs"`. | Notes for a direct reader: @@ -129,9 +183,16 @@ Notes for a direct reader: ## Limitations: `--no-verify` bypass -Both `git commit --no-verify` and `git push --no-verify` bypass their -respective hooks, including the lock checks. **This is expected and not -treated as a bug.** The lock's threat model is an AI agent following its -instructions, not a human deliberately working around a local safeguard — -so no attempt is made to defend against `--no-verify`. If you need a -guarantee that survives a determined bypass, this is not that guarantee. +`git commit --no-verify`, `git push --no-verify`, and +`git rebase --no-verify` bypass their respective hooks, including the +lock checks. **This is expected and not treated as a bug.** The lock's +threat model is an AI agent following its instructions, not a human +deliberately working around a local safeguard — so no attempt is made +to defend against `--no-verify` for commit, push, or rebase. + +The one exception is `--refs`: the `reference-transaction` hook is not +affected by `--no-verify`, so reference protection survives it. This is +the entire reason `--refs` exists as a separate axis. + +If you need a guarantee that survives a determined bypass for all +operations, this is not that guarantee. diff --git a/src/attributes/mod.rs b/src/attributes/mod.rs index e68dba6..e783ff1 100644 --- a/src/attributes/mod.rs +++ b/src/attributes/mod.rs @@ -105,6 +105,7 @@ pub(crate) fn apply_presets(labels: &[&str]) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use serial_test::serial; use tempfile::TempDir; fn make_git_repo() -> TempDir { @@ -135,6 +136,7 @@ mod tests { assert!(PRESET_BINARY.contains("*.png binary")); } + #[serial] #[test] fn apply_presets_line_endings_writes_content() { let dir = make_git_repo(); @@ -146,6 +148,7 @@ mod tests { assert!(content.contains("eol=lf")); } + #[serial] #[test] fn apply_presets_binary_files_writes_content() { let dir = make_git_repo(); @@ -157,6 +160,7 @@ mod tests { assert!(content.contains("*.png binary")); } + #[serial] #[test] fn apply_presets_both_presets() { let dir = make_git_repo(); @@ -169,6 +173,7 @@ mod tests { assert!(content.contains("*.png binary")); } + #[serial] #[test] fn apply_presets_skips_unknown_labels() { let dir = make_git_repo(); @@ -180,6 +185,7 @@ mod tests { assert!(content.is_empty()); } + #[serial] #[test] fn apply_presets_does_not_duplicate() { let dir = make_git_repo(); @@ -191,6 +197,7 @@ mod tests { assert_eq!(content.matches("eol=lf").count(), 1); } + #[serial] #[test] fn apply_presets_appends_to_existing_content() { let dir = make_git_repo(); diff --git a/src/config/mod.rs b/src/config/mod.rs index 393f15f..ba99051 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -709,6 +709,11 @@ mod tests { #[serial] #[test] fn apply_configs_non_dry_run_in_temp_repo() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::process::Command::new("git") .args(["init"]) @@ -719,16 +724,26 @@ mod tests { let _ = std::env::set_current_dir(dir.path()); let single: &[(&str, &str)] = &[("push.autoSetupRemote", "true")]; let result = apply_configs(single, false, ConfigScope::Local); - // May fail if CWD race — just verify no panic let _ = result; if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] #[test] fn apply_configs_non_dry_run_already_set() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::process::Command::new("git") .args(["init"]) @@ -745,11 +760,22 @@ mod tests { if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] #[test] fn apply_configs_non_dry_run_multiple_configs() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::process::Command::new("git") .args(["init"]) @@ -769,6 +795,12 @@ mod tests { if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } // ── git_config_set ──────────────────────────────────────────────────── @@ -920,6 +952,11 @@ mod tests { #[serial] #[test] fn apply_config_keys_multiple_valid_non_dry_run() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::process::Command::new("git") .args(["init"]) @@ -938,6 +975,12 @@ mod tests { if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } // ── show_config ─────────────────────────────────────────────────────── diff --git a/src/hooks/builtins.rs b/src/hooks/builtins.rs index ace4936..ad7e41a 100644 --- a/src/hooks/builtins.rs +++ b/src/hooks/builtins.rs @@ -42,12 +42,35 @@ pub(crate) const ALL: &[Builtin] = &[ description: "Rejects a commit message that has a body", script: NO_BODY, }, + Builtin { + name: "message-rules", + hook: "commit-msg", + description: + "Validates commit messages against user-defined regex rules in .gitmessage-rules.json", + script: MESSAGE_RULES_PLACEHOLDER, + }, ]; pub(crate) fn get(name: &str) -> Option<&'static Builtin> { ALL.iter().find(|b| b.name == name) } +/// The four message prefixes git writes itself, never a human: `git revert`, +/// a `git merge` conflict message, `git commit --fixup` and `git commit +/// --squash`. A convention meant for hand-authored subjects must not reject +/// them. This is the single source of truth — the shell macro below and the +/// Rust evaluation in `message_rules` both derive from this list, so the set +/// can only drift if someone edits this constant without updating its users. +pub(crate) const AUTO_GENERATED_MESSAGE_PREFIXES: &[&str] = + &["Revert \"", "Merge ", "fixup! ", "squash! "]; + +/// Returns true if `subject` starts with one of the auto-generated prefixes. +pub(crate) fn is_auto_generated_message(subject: &str) -> bool { + AUTO_GENERATED_MESSAGE_PREFIXES + .iter() + .any(|prefix| subject.starts_with(prefix)) +} + /// The four message shapes git writes itself, never a human: `git revert`, /// a `git merge` conflict message, `git commit --fixup` and `git commit /// --squash`. A convention meant for hand-authored subjects must not reject @@ -69,6 +92,7 @@ esac const CONVENTIONAL_COMMITS: &str = concat!( r#"#!/bin/sh +# gitkit-builtin: conventional-commits # Validates only the first line (the subject). grep matches line-by-line, so # without this the whole message would pass if ANY line looked conventional, # not just the first. @@ -100,6 +124,7 @@ fi pub(crate) const AI_VENDOR_NOREPLY_ADDRESSES: &[&str] = &["noreply@anthropic.com"]; const NO_TRAILERS: &str = r#"#!/bin/sh +# gitkit-builtin: no-trailers # Rejects commit messages carrying AI attribution trailers: a Co-Authored-By, # Assisted-By or AI-Assisted-By line naming a known AI vendor no-reply # address, a Claude-Session line, or a "Generated with" line. Genuine human @@ -120,6 +145,7 @@ fi "#; const NO_SECRETS: &str = r#"#!/bin/sh +# gitkit-builtin: no-secrets # Detects common secret patterns. Not exhaustive — use dedicated tools for production. patterns='(AKIA[0-9A-Z]{16}|AIza[0-9A-Za-z_-]{35}|ghp_[0-9A-Za-z]{36}|sk-[0-9A-Za-z]{48}|password\s*=\s*["'"'"'][^"'"'"']{8,})' if git diff --cached --diff-filter=ACM | grep -qE "$patterns"; then @@ -130,6 +156,7 @@ fi "#; const BRANCH_NAMING: &str = r#"#!/bin/sh +# gitkit-builtin: branch-naming branch=$(git symbolic-ref --short HEAD) pattern='^(main|master|develop|release/.+|hotfix/.+|feat/.+|feature/.+|fix/.+|chore/.+)$' if ! echo "$branch" | grep -qE "$pattern"; then @@ -147,6 +174,7 @@ fi // actual scan lives in `no_invisibles.rs`, is pure Rust std library, and is // unit-tested directly there. const NO_INVISIBLES: &str = r#"#!/bin/sh +# gitkit-builtin: no-invisibles # Rejects added lines carrying invisible Unicode: zero-width characters, # bidi controls (also the "Trojan Source" vector) and Unicode tag # characters. Only lines this commit adds are scanned, not the whole file — @@ -156,6 +184,7 @@ exec gitkit hooks scan-invisibles const NO_BODY: &str = concat!( r#"#!/bin/sh +# gitkit-builtin: no-body # Rejects a commit message with a body. A conforming message is one line, # with any number of trailing newlines. The only body this hook allows is a # blank line followed by a BREAKING CHANGE:/BREAKING-CHANGE: footer (case @@ -212,6 +241,18 @@ done "# ); +/// Placeholder script for the `message-rules` builtin. The actual script is +/// generated dynamically at install time (see `message_rules::generate_script`). +/// This placeholder is only used for registration in `ALL` and for health-check +/// matching — the installed script always starts with the same marker comment, +/// so `detect_builtin` identifies it by marker, not by exact content match. +const MESSAGE_RULES_PLACEHOLDER: &str = r#"#!/bin/sh +# gitkit-builtin: message-rules +# This is a placeholder. The real script is generated by gitkit from rules +# configured in .gitmessage-rules.json. +exit 0 +"#; + #[cfg(test)] mod tests { use super::*; @@ -361,6 +402,31 @@ mod tests { ); } + #[test] + fn shell_exemption_macro_covers_every_shared_prefix() { + let exemption = auto_generated_message_exemption!(); + for prefix in AUTO_GENERATED_MESSAGE_PREFIXES { + assert!( + exemption.contains(prefix), + "the shell exemption macro must contain every prefix from \ + AUTO_GENERATED_MESSAGE_PREFIXES — missing '{prefix}'" + ); + } + } + + #[test] + fn is_auto_generated_message_matches_the_shared_prefixes() { + for prefix in AUTO_GENERATED_MESSAGE_PREFIXES { + let subject = format!("{prefix}something"); + assert!( + is_auto_generated_message(&subject), + "is_auto_generated_message must return true for prefix '{prefix}'" + ); + } + assert!(!is_auto_generated_message("feat: hand-authored")); + assert!(!is_auto_generated_message("fix: also hand-authored")); + } + #[test] fn no_body_is_registered_as_commit_msg_builtin() { let builtin = get("no-body").unwrap(); diff --git a/src/hooks/message_rules.rs b/src/hooks/message_rules.rs new file mode 100644 index 0000000..cb0c72c --- /dev/null +++ b/src/hooks/message_rules.rs @@ -0,0 +1,571 @@ +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; +use std::path::{Path, PathBuf}; + +use super::builtins; +use crate::utils::find_repo_root; + +const RULES_FILE: &str = ".gitmessage-rules.json"; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub(crate) struct MessageRule { + pub name: String, + pub pattern: String, + pub direction: Direction, + pub scope: Scope, + pub message: String, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub(crate) enum Direction { + MustMatch, + MustNotMatch, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub(crate) enum Scope { + Subject, + WholeMessage, +} + +fn rules_path() -> Result { + let root = find_repo_root().context("not inside a git repository")?; + Ok(root.join(RULES_FILE)) +} + +pub(crate) fn load_rules() -> Result> { + let path = match rules_path() { + Ok(p) => p, + Err(_) => return Ok(Vec::new()), + }; + + if !path.exists() { + return Ok(Vec::new()); + } + + let content = std::fs::read_to_string(&path) + .with_context(|| format!("failed to read rules file: {}", path.display()))?; + + if content.trim().is_empty() { + return Ok(Vec::new()); + } + + let rules: Vec = serde_json::from_str(&content) + .with_context(|| format!("failed to parse rules file: {}", path.display()))?; + Ok(rules) +} + +pub(crate) fn validate_rules() -> Result> { + let rules = load_rules()?; + anyhow::ensure!( + !rules.is_empty(), + "no message-rules configured — create '{}' with at least one rule \ + before running 'gitkit hooks add message-rules'", + RULES_FILE + ); + for rule in &rules { + regex::Regex::new(&rule.pattern).with_context(|| { + format!( + "rule '{}': pattern '{}' is not a valid regex", + rule.name, rule.pattern + ) + })?; + } + Ok(rules) +} + +pub(crate) fn generate_script() -> String { + r#"#!/bin/sh +# gitkit-builtin: message-rules +# Validates the commit message against user-defined rules from .gitmessage-rules.json. +# Delegates to gitkit itself for the actual regex evaluation, so patterns use +# the same Rust regex engine that was validated at install time. +exec gitkit hooks scan-message-rules "$1" +"# + .to_string() +} + +/// Pure rule-checking logic: iterate rules, regex match by scope, collect failures. +/// Shared by both `evaluate_message` (production) and tests. +pub(crate) fn check_rules(rules: &[MessageRule], full_message: &str) -> Vec { + let subject = full_message.lines().next().unwrap_or(""); + + if builtins::is_auto_generated_message(subject) { + return Vec::new(); + } + + let mut failures: Vec = Vec::new(); + + for rule in rules { + let re = match regex::Regex::new(&rule.pattern) { + Ok(r) => r, + Err(_) => continue, + }; + let text = match rule.scope { + Scope::Subject => subject, + Scope::WholeMessage => full_message, + }; + + let matched = re.is_match(text); + + match rule.direction { + Direction::MustMatch => { + if !matched { + failures.push(format!("ERROR: [{}] {}", rule.name, rule.message)); + } + } + Direction::MustNotMatch => { + if matched { + failures.push(format!("ERROR: [{}] {}", rule.name, rule.message)); + } + } + } + } + + failures +} + +pub(crate) fn evaluate_message(msg_file: &Path) -> Result<()> { + let rules = load_rules()?; + let full_message = std::fs::read_to_string(msg_file) + .with_context(|| format!("failed to read commit message file: {}", msg_file.display()))?; + + let failures = check_rules(&rules, &full_message); + + if failures.is_empty() { + Ok(()) + } else { + for f in &failures { + eprintln!("{f}"); + } + std::process::exit(1); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn make_rule( + name: &str, + pattern: &str, + direction: Direction, + scope: Scope, + message: &str, + ) -> MessageRule { + MessageRule { + name: name.to_string(), + pattern: pattern.to_string(), + direction, + scope, + message: message.to_string(), + } + } + + fn evaluate_rules(rules: &[MessageRule], message: &str) -> (bool, Vec) { + let failures = check_rules(rules, message); + if failures.is_empty() { + (true, failures) + } else { + (false, failures) + } + } + + #[test] + fn positive_must_match_rule_accepts_matching_subject() { + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, failures) = evaluate_rules(&rules, "PROJ-123 fix the thing\n"); + assert!(accepted, "expected acceptance: {failures:?}"); + } + + #[test] + fn positive_must_match_rule_rejects_non_matching_subject() { + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, failures) = evaluate_rules(&rules, "fix the thing\n"); + assert!(!accepted, "expected rejection"); + assert!( + failures[0].contains("jira-prefix"), + "failures: {failures:?}" + ); + assert!( + failures[0].contains("JIRA ticket prefix"), + "failures: {failures:?}" + ); + } + + #[test] + fn negative_must_not_match_rule_accepts_clean_subject() { + let rules = vec![make_rule( + "no-trailer-in-subject", + "Co-Authored-By:", + Direction::MustNotMatch, + Scope::Subject, + "Subject must not contain trailer lines", + )]; + let (accepted, failures) = evaluate_rules(&rules, "fix: the thing\n"); + assert!(accepted, "expected acceptance: {failures:?}"); + } + + #[test] + fn negative_must_not_match_rule_rejects_matching_subject() { + let rules = vec![make_rule( + "no-trailer-in-subject", + "Co-Authored-By:", + Direction::MustNotMatch, + Scope::Subject, + "Subject must not contain trailer lines", + )]; + let (accepted, failures) = evaluate_rules(&rules, "fix: the thing; Co-Authored-By: bot\n"); + assert!(!accepted, "expected rejection"); + assert!( + failures[0].contains("no-trailer-in-subject"), + "failures: {failures:?}" + ); + } + + #[test] + fn subject_scope_does_not_fire_on_body_only_match() { + let rules = vec![make_rule( + "no-trailer-in-subject", + "Co-Authored-By:", + Direction::MustNotMatch, + Scope::Subject, + "Subject must not contain trailer lines", + )]; + let (accepted, failures) = evaluate_rules( + &rules, + "fix: the thing\n\nCo-Authored-By: bot \n", + ); + assert!( + accepted, + "a subject-scoped rule must not fire on a body-only match: {failures:?}" + ); + } + + #[test] + fn whole_message_scope_fires_on_body_match() { + let rules = vec![make_rule( + "no-ai-trailer", + "noreply@anthropic\\.com", + Direction::MustNotMatch, + Scope::WholeMessage, + "Message must not contain AI vendor attribution", + )]; + let (accepted, failures) = evaluate_rules( + &rules, + "fix: the thing\n\nCo-Authored-By: Claude \n", + ); + assert!( + !accepted, + "a whole-message rule must fire on a body match: {failures:?}" + ); + } + + #[test] + fn whole_message_scope_includes_subject_line() { + let rules = vec![make_rule( + "body-keyword", + "BREAKING", + Direction::MustMatch, + Scope::WholeMessage, + "Message body must mention BREAKING", + )]; + let (accepted, _failures) = evaluate_rules(&rules, "BREAKING: fix the thing\n"); + assert!(accepted, "whole-message scope includes the subject line"); + } + + #[test] + fn multiple_failing_rules_all_reported() { + let rules = vec![ + make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + ), + make_rule( + "min-length", + r".{15,}", + Direction::MustMatch, + Scope::Subject, + "Subject must be at least 15 characters", + ), + ]; + let (accepted, failures) = evaluate_rules(&rules, "fix: short\n"); + assert!(!accepted); + assert!( + failures.iter().any(|f| f.contains("jira-prefix")), + "failures: {failures:?}" + ); + assert!( + failures.iter().any(|f| f.contains("min-length")), + "failures: {failures:?}" + ); + } + + #[test] + #[allow(clippy::invalid_regex)] + fn uncompilable_pattern_rejected_at_validation() { + let result = regex::Regex::new("[invalid"); + assert!(result.is_err(), "an unclosed bracket must not compile"); + } + + #[test] + fn generated_script_starts_with_shebang_and_marker() { + let script = generate_script(); + assert!(script.starts_with("#!/bin/sh")); + assert!(script.contains("# gitkit-builtin: message-rules")); + } + + #[test] + fn generated_script_execs_into_gitkit() { + let script = generate_script(); + assert!( + script.contains("exec gitkit hooks scan-message-rules"), + "the installed hook must exec back into gitkit for real regex evaluation: {script}" + ); + } + + #[test] + fn exempt_revert_messages() { + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, _) = evaluate_rules( + &rules, + "Revert \"feat(x): add thing\"\n\nThis reverts commit abc123.\n", + ); + assert!(accepted, "revert messages must be exempt"); + } + + #[test] + fn exempt_merge_messages() { + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, _) = evaluate_rules(&rules, "Merge branch 'develop' into main\n"); + assert!(accepted, "merge messages must be exempt"); + } + + #[test] + fn exempt_fixup_messages() { + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, _) = evaluate_rules(&rules, "fixup! feat(x): add thing\n"); + assert!(accepted, "fixup messages must be exempt"); + } + + #[test] + fn exempt_squash_messages() { + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, _) = evaluate_rules(&rules, "squash! feat(x): add thing\n"); + assert!(accepted, "squash messages must be exempt"); + } + + #[test] + fn whole_message_anchor_semantics_match_rust_regex() { + let rules = vec![make_rule( + "must-start-with-breaking", + r"^BREAKING", + Direction::MustMatch, + Scope::WholeMessage, + "Message must start with BREAKING", + )]; + let (accepted, _) = evaluate_rules(&rules, "not breaking\n\nBREAKING: smuggled in later\n"); + assert!( + !accepted, + "^ under Rust regex must anchor to start of whole message, not match any line" + ); + } + + #[test] + fn message_rules_exemption_covers_all_shared_prefixes() { + for prefix in builtins::AUTO_GENERATED_MESSAGE_PREFIXES { + let msg = format!("{prefix}something\n"); + let rules = vec![make_rule( + "jira-prefix", + r"^[A-Z]+-\d+", + Direction::MustMatch, + Scope::Subject, + "Subject must start with a JIRA ticket prefix", + )]; + let (accepted, _) = evaluate_rules(&rules, &msg); + assert!( + accepted, + "prefix '{prefix}' must be exempt — AUTO_GENERATED_MESSAGE_PREFIXES and \ + is_auto_generated_message must stay in sync" + ); + } + } + + #[test] + #[serial_test::serial] + fn validate_rules_rejects_when_no_rules_configured() { + let dir = tempfile::TempDir::new().unwrap(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(dir.path()) + .output() + .unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + let result = validate_rules(); + assert!( + result.is_err(), + "validate_rules must refuse when no rules are configured" + ); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("no message-rules configured"), + "error must name the missing config: {err_msg}" + ); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[test] + #[serial_test::serial] + fn validate_rules_rejects_uncompilable_pattern() { + let dir = tempfile::TempDir::new().unwrap(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(dir.path()) + .output() + .unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + let bad_rule = MessageRule { + name: "bad".to_string(), + pattern: "[invalid".to_string(), + direction: Direction::MustMatch, + scope: Scope::Subject, + message: "bad pattern".to_string(), + }; + let json = serde_json::to_string_pretty(&[bad_rule]).unwrap(); + std::fs::write(dir.path().join(".gitmessage-rules.json"), json).unwrap(); + + let result = validate_rules(); + assert!(result.is_err(), "uncompilable pattern must be rejected"); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("bad"), + "error must name the rule: {err_msg}" + ); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[test] + #[serial_test::serial] + fn load_rules_returns_empty_when_file_not_present() { + let dir = tempfile::TempDir::new().unwrap(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(dir.path()) + .output() + .unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + let rules = load_rules().unwrap(); + assert!( + rules.is_empty(), + "no rules should be returned when file is not present" + ); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[test] + #[serial_test::serial] + fn load_rules_parses_json_from_tracked_file() { + let dir = tempfile::TempDir::new().unwrap(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(dir.path()) + .output() + .unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + let rule = MessageRule { + name: "jira-prefix".to_string(), + pattern: r"^[A-Z]+-\d+".to_string(), + direction: Direction::MustMatch, + scope: Scope::Subject, + message: "JIRA prefix required".to_string(), + }; + let json = serde_json::to_string_pretty(&[rule]).unwrap(); + std::fs::write(dir.path().join(".gitmessage-rules.json"), json).unwrap(); + + let rules = load_rules().unwrap(); + assert_eq!(rules.len(), 1); + assert_eq!(rules[0].name, "jira-prefix"); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[test] + fn rules_file_path_is_tracked_constant() { + assert_eq!(RULES_FILE, ".gitmessage-rules.json"); + } + + #[test] + fn rule_serialization_roundtrip() { + let rule = MessageRule { + name: "test".to_string(), + pattern: ".*".to_string(), + direction: Direction::MustNotMatch, + scope: Scope::WholeMessage, + message: "test message".to_string(), + }; + let json = serde_json::to_string(std::slice::from_ref(&rule)).unwrap(); + let parsed: Vec = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.len(), 1); + assert_eq!(parsed[0], rule); + } +} diff --git a/src/hooks/mod.rs b/src/hooks/mod.rs index 1c6fa63..9c66027 100644 --- a/src/hooks/mod.rs +++ b/src/hooks/mod.rs @@ -5,6 +5,7 @@ use std::{fs, path::Path}; use crate::utils::{confirm, find_repo_root}; pub(crate) mod builtins; +mod message_rules; mod no_invisibles; #[derive(Subcommand)] @@ -51,6 +52,13 @@ pub enum HooksCommand { /// `no-invisibles` pre-commit hook; not meant to be run directly. #[command(hide = true)] ScanInvisibles, + /// Internal: evaluate commit message against user-defined rules. Execed + /// by the `message-rules` commit-msg hook; not meant to be run directly. + #[command(hide = true)] + ScanMessageRules { + /// Path to the commit message file (passed by git) + msg_file: String, + }, } pub fn run(cmd: HooksCommand) -> Result<()> { @@ -66,6 +74,9 @@ pub fn run(cmd: HooksCommand) -> Result<()> { HooksCommand::Remove { hook, yes, dry_run } => remove(&hook, yes, dry_run), HooksCommand::Show { hook } => show(&hook), HooksCommand::ScanInvisibles => no_invisibles::run(), + HooksCommand::ScanMessageRules { msg_file } => { + message_rules::evaluate_message(std::path::Path::new(&msg_file)) + } } } @@ -89,9 +100,33 @@ pub(crate) fn valid_hook_names() -> &'static [&'static str] { VALID_HOOKS } -/// Identifies which built-in (if any) an installed hook file corresponds to, -/// by exact script comparison. Built-ins are written verbatim on install. +/// Extracts the builtin identity marker from script content, if present. +/// The marker is a shell comment line of the form `# gitkit-builtin: `. +fn extract_marker(content: &str) -> Option<&str> { + for line in content.lines() { + let trimmed = line.trim(); + if let Some(name) = trimmed.strip_prefix("# gitkit-builtin: ") { + let name = name.trim(); + if !name.is_empty() { + return Some(name); + } + } + } + None +} + +/// Identifies which built-in (if any) an installed hook file corresponds to. +/// First tries the machine-readable marker (`# gitkit-builtin: `), which +/// survives script changes across versions. Falls back to exact-content match +/// so hooks installed by pre-marker versions of gitkit keep being recognised. pub(crate) fn detect_builtin(hook_file: &str, content: &str) -> Option<&'static builtins::Builtin> { + if let Some(name) = extract_marker(content) { + if let Some(b) = builtins::get(name) { + if b.hook == hook_file { + return Some(b); + } + } + } builtins::ALL .iter() .find(|b| b.hook == hook_file && content.trim() == b.script.trim()) @@ -176,8 +211,9 @@ pub(crate) fn list_parts(hooks_dir: &Path, hook_name: &str) -> Vec { /// Ensures `.git/hooks/` is a gitkit dispatcher, migrating any /// pre-existing content into `gitkit.d//` as a part first: -/// - if it's a script matching a known builtin verbatim (the pre-composition -/// damaged shape), it's absorbed under that builtin's name. +/// - if it's a known builtin (by marker or exact content match), it's absorbed +/// under that builtin's name with the **current** script — so an outdated +/// builtin is replaced, not frozen as an untouchable hand-written hook. /// - otherwise (a hand-written hook) it's absorbed as [`PRESERVED_PART_NAME`]. /// /// A no-op if the dispatcher is already in place. @@ -189,15 +225,32 @@ fn ensure_dispatcher(dir: &Path, hook_name: &str) -> Result<()> { let content = fs::read_to_string(&hook_path).unwrap_or_default(); if !is_dispatcher(&content, hook_name) { fs::create_dir_all(&parts).context("Failed to create gitkit.d parts directory")?; - let migrated_name = detect_builtin(hook_name, &content) - .map(|b| b.name.to_string()) - .unwrap_or_else(|| PRESERVED_PART_NAME.to_string()); - let migrated_path = parts.join(&migrated_name); - if !migrated_path.exists() { - fs::write(&migrated_path, &content).with_context(|| { - format!("Failed to migrate existing '{hook_name}' hook into gitkit.d") - })?; - set_executable(&migrated_path)?; + + if let Some(builtin) = detect_builtin(hook_name, &content) { + let migrated_path = parts.join(builtin.name); + let current_script = if builtin.name == "message-rules" { + message_rules::generate_script() + } else { + builtin.script.to_owned() + }; + let is_outdated = content.trim() != current_script.trim(); + if is_outdated { + println!("Updating outdated builtin: {}", builtin.name); + } + if !migrated_path.exists() || is_outdated { + fs::write(&migrated_path, ¤t_script).with_context(|| { + format!("Failed to migrate builtin '{}' into gitkit.d", builtin.name) + })?; + set_executable(&migrated_path)?; + } + } else { + let migrated_path = parts.join(PRESERVED_PART_NAME); + if !migrated_path.exists() { + fs::write(&migrated_path, &content).with_context(|| { + format!("Failed to migrate existing '{hook_name}' hook into gitkit.d") + })?; + set_executable(&migrated_path)?; + } } } } else { @@ -271,8 +324,11 @@ fn remove_part(dir: &Path, hook_name: &str, part_name: &str) -> Result<()> { /// Health of one installed part within `gitkit.d//`, mirroring /// [`classify_hook`]'s categories but also accounting for the dispatcher's -/// own executable bit — a part stays `Dormant` if the dispatcher that would -/// run it is itself not executable, since git never even reaches it then. +/// own executable bit — a part stays `Dormant`/`ModifiedDormant` if the +/// dispatcher that would run it is itself not executable, since git never +/// even reaches it then. The execute bit is orthogonal to content: a +/// non-executable part is dormant regardless of whether its content is +/// recognized. pub(crate) fn classify_part(dir: &Path, hook_name: &str, part_name: &str) -> Result { let part_path = parts_dir(dir, hook_name).join(part_name); if !part_path.exists() { @@ -287,19 +343,20 @@ pub(crate) fn classify_part(dir: &Path, hook_name: &str, part_name: &str) -> Res let recognized = match std::str::from_utf8(&bytes) { Ok(content) => { part_name == PRESERVED_PART_NAME + || (part_name == "message-rules" + && content.contains("# gitkit-builtin: message-rules")) || builtins::get(part_name).is_some_and(|b| content.trim() == b.script.trim()) } Err(_) => false, }; - if !recognized { - return Ok(HookHealth::Modified); - } + let executable = dispatcher_ok && is_executable(&part_path)?; - if dispatcher_ok && is_executable(&part_path)? { - Ok(HookHealth::Active) - } else { - Ok(HookHealth::Dormant) + match (recognized, executable) { + (true, true) => Ok(HookHealth::Active), + (true, false) => Ok(HookHealth::Dormant), + (false, true) => Ok(HookHealth::Modified), + (false, false) => Ok(HookHealth::ModifiedDormant), } } @@ -377,9 +434,16 @@ fn add_builtin_part( let dir = hooks_dir()?; let part_path = parts_dir(&dir, builtin.hook).join(builtin.name); + let script = if builtin.name == "message-rules" { + let _rules = message_rules::validate_rules()?; + message_rules::generate_script() + } else { + builtin.script.to_owned() + }; + if part_path.exists() && !force { let existing = fs::read_to_string(&part_path).unwrap_or_default(); - if existing.trim() != builtin.script.trim() + if existing.trim() != script.trim() && !confirm( &format!("Hook '{}' already exists. Overwrite?", builtin.name), yes, @@ -391,14 +455,11 @@ fn add_builtin_part( } if dry_run { - println!( - "[dry-run] Would write hook '{}':\n{}", - builtin.hook, builtin.script - ); + println!("[dry-run] Would write hook '{}':\n{}", builtin.hook, script); return Ok(()); } - install_part(builtin.hook, builtin.name, builtin.script)?; + install_part(builtin.hook, builtin.name, &script)?; record_applied(builtin.name); println!("Installed hook '{}'.", builtin.hook); Ok(()) @@ -412,7 +473,13 @@ fn add_quiet(hook_or_builtin: &str, command: Option<&str>, force: bool) -> Resul "'{hook_or_builtin}' is a built-in hook — no command needed" ); let _ = force; - install_part(builtin.hook, builtin.name, builtin.script)?; + let script = if builtin.name == "message-rules" { + let _rules = message_rules::validate_rules()?; + message_rules::generate_script() + } else { + builtin.script.to_owned() + }; + install_part(builtin.hook, builtin.name, &script)?; record_applied(builtin.name); return Ok(()); } @@ -446,7 +513,13 @@ fn resolve_hook<'a>(hook_or_builtin: &'a str, command: Option<&str>) -> Result<( command.is_none(), "'{hook_or_builtin}' is a built-in hook — no command needed" ); - return Ok((builtin.hook, builtin.script.to_owned())); + let script = if builtin.name == "message-rules" { + let _rules = message_rules::validate_rules()?; + message_rules::generate_script() + } else { + builtin.script.to_owned() + }; + return Ok((builtin.hook, script)); } let cmd = command.ok_or_else(|| { @@ -576,16 +649,24 @@ pub(crate) enum HookHealth { /// Present but its content matches no builtin. Not an error: the user /// may have edited it deliberately, or it's a custom (non-builtin) hook. Modified, + /// Content matches no builtin AND the file is not executable — git silently + /// ignores it. Both facts are reported: the user must learn it does not run. + ModifiedDormant, /// No file at this hook's path. Absent, } /// Classifies an installed hook file's health for `gitkit status`, by -/// comparing its content against the builtin catalogue via [`detect_builtin`]. -/// Reads the file itself rather than trusting a caller-supplied string, so -/// non-UTF-8 content is classified `Modified` instead of erroring — git runs -/// a hook regardless of its encoding, so an unreadable-as-text file is not a -/// failure, just content gitkit can't match against a builtin. +/// comparing its content against the builtin catalogue via exact-content +/// match and checking the execute bit. The execute bit is orthogonal to +/// content: a non-executable hook is dormant regardless of whether its +/// content matches a builtin, so a hand-edited hook that lost its execute +/// bit is `ModifiedDormant` — both facts reported, not one. +/// Reads the file itself rather than trusting a caller-supplied string, +/// so non-UTF-8 content is classified `Modified`/`ModifiedDormant` instead +/// of erroring — git runs a hook regardless of its encoding, so an +/// unreadable-as-text file is not a failure, just content gitkit can't +/// match against a builtin. pub(crate) fn classify_hook(hook_name: &str, path: &Path) -> Result { if !path.exists() { return Ok(HookHealth::Absent); @@ -593,18 +674,19 @@ pub(crate) fn classify_hook(hook_name: &str, path: &Path) -> Result let bytes = fs::read(path).with_context(|| format!("Failed to read hook '{hook_name}'"))?; let is_builtin = match std::str::from_utf8(&bytes) { - Ok(content) => detect_builtin(hook_name, content).is_some(), + Ok(content) => builtins::ALL + .iter() + .any(|b| b.hook == hook_name && content.trim() == b.script.trim()), Err(_) => false, }; - if !is_builtin { - return Ok(HookHealth::Modified); - } + let executable = is_executable(path)?; - if is_executable(path)? { - Ok(HookHealth::Active) - } else { - Ok(HookHealth::Dormant) + match (is_builtin, executable) { + (true, true) => Ok(HookHealth::Active), + (true, false) => Ok(HookHealth::Dormant), + (false, true) => Ok(HookHealth::Modified), + (false, false) => Ok(HookHealth::ModifiedDormant), } } @@ -613,6 +695,25 @@ mod tests { use super::*; use serial_test::serial; + /// Sets `GITKIT_HOME` to a fresh temp dir for the duration of `f`, + /// restoring the original value afterward. This isolates the registry + /// from the test suite: any `record_best_effort` call during the test + /// writes to the temp dir, not the user's real `~/.gitkit`. + fn with_isolated_registry(f: F) { + let dir = tempfile::TempDir::new().unwrap(); + let original = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", dir.path()); + } + f(); + unsafe { + match &original { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } + } + #[test] fn resolve_hook_returns_builtin_script() { let (hook, script) = resolve_hook("conventional-commits", None).unwrap(); @@ -722,6 +823,9 @@ mod tests { #[test] fn resolve_hook_all_builtins_resolvable() { for b in available_builtins() { + if b.name == "message-rules" { + continue; + } let result = resolve_hook(b.name, None); assert!(result.is_ok(), "Failed to resolve builtin: {}", b.name); let (hook, script) = result.unwrap(); @@ -1093,45 +1197,49 @@ mod tests { #[serial] #[test] fn install_builtin_writes_hook_file() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); - let result = install_builtin("no-secrets", true); - assert!(result.is_ok()); - let hook_path = dir.path().join(".git").join("hooks").join("pre-commit"); - assert!(hook_path.exists()); - let part_path = dir - .path() - .join(".git") - .join("hooks") - .join("gitkit.d") - .join("pre-commit") - .join("no-secrets"); - assert!(part_path.exists()); - let content = std::fs::read_to_string(&part_path).unwrap(); - assert!(content.contains("secret")); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + let result = install_builtin("no-secrets", true); + assert!(result.is_ok()); + let hook_path = dir.path().join(".git").join("hooks").join("pre-commit"); + assert!(hook_path.exists()); + let part_path = dir + .path() + .join(".git") + .join("hooks") + .join("gitkit.d") + .join("pre-commit") + .join("no-secrets"); + assert!(part_path.exists()); + let content = std::fs::read_to_string(&part_path).unwrap(); + assert!(content.contains("secret")); + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn install_custom_writes_hook_file() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); - let result = install_custom("pre-commit", "cargo fmt --check", true); - assert!(result.is_ok()); - let hook_path = dir.path().join(".git").join("hooks").join("pre-commit"); - assert!(hook_path.exists()); - let content = std::fs::read_to_string(&hook_path).unwrap(); - assert!(content.contains("cargo fmt --check")); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + let result = install_custom("pre-commit", "cargo fmt --check", true); + assert!(result.is_ok()); + let hook_path = dir.path().join(".git").join("hooks").join("pre-commit"); + assert!(hook_path.exists()); + let content = std::fs::read_to_string(&hook_path).unwrap(); + assert!(content.contains("cargo fmt --check")); + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } // ── list() paths ────────────────────────────────────────────────────── @@ -1585,218 +1693,312 @@ mod tests { #[serial] #[test] fn installing_two_builtins_for_one_hook_leaves_both_as_parts_and_a_dispatcher() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); - - install_builtin("conventional-commits", true).unwrap(); - install_builtin("no-body", true).unwrap(); - - let hooks_dir = dir.path().join(".git").join("hooks"); - let parts = hooks_dir.join("gitkit.d").join("commit-msg"); - let cc_part = parts.join("conventional-commits"); - let nb_part = parts.join("no-body"); - assert!(cc_part.exists()); - assert!(nb_part.exists()); - assert!(is_executable(&cc_part).unwrap()); - assert!(is_executable(&nb_part).unwrap()); - - let dispatcher_path = hooks_dir.join("commit-msg"); - assert!(dispatcher_path.exists()); - assert!(is_executable(&dispatcher_path).unwrap()); - let dispatcher_content = std::fs::read_to_string(&dispatcher_path).unwrap(); - assert!(is_dispatcher(&dispatcher_content, "commit-msg")); - - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + install_builtin("conventional-commits", true).unwrap(); + install_builtin("no-body", true).unwrap(); + + let hooks_dir = dir.path().join(".git").join("hooks"); + let parts = hooks_dir.join("gitkit.d").join("commit-msg"); + let cc_part = parts.join("conventional-commits"); + let nb_part = parts.join("no-body"); + assert!(cc_part.exists()); + assert!(nb_part.exists()); + assert!(is_executable(&cc_part).unwrap()); + assert!(is_executable(&nb_part).unwrap()); + + let dispatcher_path = hooks_dir.join("commit-msg"); + assert!(dispatcher_path.exists()); + assert!(is_executable(&dispatcher_path).unwrap()); + let dispatcher_content = std::fs::read_to_string(&dispatcher_path).unwrap(); + assert!(is_dispatcher(&dispatcher_content, "commit-msg")); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn dispatcher_rejects_message_that_violates_only_conventional_commits() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); - - install_builtin("conventional-commits", true).unwrap(); - install_builtin("no-body", true).unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); - - // Non-conventional single-line subject: fails conventional-commits, - // would pass no-body on its own. - let (accepted, output) = run_dispatcher(&hooks_dir, "commit-msg", "just a message\n"); - assert!(!accepted, "expected rejection: {output}"); - assert!( - output.contains("Conventional Commits"), - "must name the conventional-commits failure: {output}" - ); + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + install_builtin("conventional-commits", true).unwrap(); + install_builtin("no-body", true).unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + + let (accepted, output) = run_dispatcher(&hooks_dir, "commit-msg", "just a message\n"); + assert!(!accepted, "expected rejection: {output}"); + assert!( + output.contains("Conventional Commits"), + "must name the conventional-commits failure: {output}" + ); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn dispatcher_rejects_message_that_violates_only_no_body() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); - - install_builtin("conventional-commits", true).unwrap(); - install_builtin("no-body", true).unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); - - // Conventional subject followed by a bulleted body: passes - // conventional-commits, fails no-body. - let (accepted, output) = run_dispatcher( - &hooks_dir, - "commit-msg", - "feat(x): add thing\n\n- bullet one\n- bullet two\n", - ); - assert!(!accepted, "expected rejection: {output}"); + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + install_builtin("conventional-commits", true).unwrap(); + install_builtin("no-body", true).unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + + let (accepted, output) = run_dispatcher( + &hooks_dir, + "commit-msg", + "feat(x): add thing\n\n- bullet one\n- bullet two\n", + ); + assert!(!accepted, "expected rejection: {output}"); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn dispatcher_accepts_message_that_passes_both_builtins() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); - - install_builtin("conventional-commits", true).unwrap(); - install_builtin("no-body", true).unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); - - let (accepted, output) = run_dispatcher(&hooks_dir, "commit-msg", "feat(x): add thing\n"); - assert!(accepted, "expected acceptance: {output}"); - - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + install_builtin("conventional-commits", true).unwrap(); + install_builtin("no-body", true).unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + + let (accepted, output) = + run_dispatcher(&hooks_dir, "commit-msg", "feat(x): add thing\n"); + assert!(accepted, "expected acceptance: {output}"); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn installing_same_builtin_twice_leaves_exactly_one_part() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); - install_builtin("conventional-commits", true).unwrap(); - install_builtin("conventional-commits", true).unwrap(); + install_builtin("conventional-commits", true).unwrap(); + install_builtin("conventional-commits", true).unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); - let parts = list_parts(&hooks_dir, "commit-msg"); - assert_eq!(parts, vec!["conventional-commits".to_string()]); + let hooks_dir = dir.path().join(".git").join("hooks"); + let parts = list_parts(&hooks_dir, "commit-msg"); + assert_eq!(parts, vec!["conventional-commits".to_string()]); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn hand_written_hook_is_preserved_runs_and_is_restored_when_last_builtin_removed() { - let dir = tempfile::TempDir::new().unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); - std::fs::create_dir_all(&hooks_dir).unwrap(); - std::fs::write( - hooks_dir.join("commit-msg"), - "#!/bin/sh\necho hand-written ran >&2\n", - ) - .unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + std::fs::create_dir_all(&hooks_dir).unwrap(); + std::fs::write( + hooks_dir.join("commit-msg"), + "#!/bin/sh\necho hand-written ran >&2\n", + ) + .unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); - install_builtin("conventional-commits", true).unwrap(); + install_builtin("conventional-commits", true).unwrap(); - let preserved_part = hooks_dir - .join("gitkit.d") - .join("commit-msg") - .join(PRESERVED_PART_NAME); - assert!(preserved_part.exists()); + let preserved_part = hooks_dir + .join("gitkit.d") + .join("commit-msg") + .join(PRESERVED_PART_NAME); + assert!(preserved_part.exists()); - let (_, output) = run_dispatcher(&hooks_dir, "commit-msg", "feat(x): add thing\n"); - assert!( - output.contains("hand-written ran"), - "hand-written hook must still run: {output}" - ); + let (_, output) = run_dispatcher(&hooks_dir, "commit-msg", "feat(x): add thing\n"); + assert!( + output.contains("hand-written ran"), + "hand-written hook must still run: {output}" + ); - remove_hook("conventional-commits", true).unwrap(); + remove_hook("conventional-commits", true).unwrap(); - assert!(!preserved_part.exists()); - assert!(!hooks_dir.join("gitkit.d").join("commit-msg").exists()); - let restored = std::fs::read_to_string(hooks_dir.join("commit-msg")).unwrap(); - assert!(restored.contains("hand-written ran")); + assert!(!preserved_part.exists()); + assert!(!hooks_dir.join("gitkit.d").join("commit-msg").exists()); + let restored = std::fs::read_to_string(hooks_dir.join("commit-msg")).unwrap(); + assert!(restored.contains("hand-written ran")); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn removing_one_of_two_builtins_leaves_the_other_running() { - let dir = tempfile::TempDir::new().unwrap(); - std::fs::create_dir(dir.path().join(".git")).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); - install_builtin("conventional-commits", true).unwrap(); - install_builtin("no-body", true).unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); + install_builtin("conventional-commits", true).unwrap(); + install_builtin("no-body", true).unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); - remove_hook("no-body", true).unwrap(); + remove_hook("no-body", true).unwrap(); - let parts = list_parts(&hooks_dir, "commit-msg"); - assert_eq!(parts, vec!["conventional-commits".to_string()]); - assert!(hooks_dir.join("commit-msg").exists()); + let parts = list_parts(&hooks_dir, "commit-msg"); + assert_eq!(parts, vec!["conventional-commits".to_string()]); + assert!(hooks_dir.join("commit-msg").exists()); - // conventional-commits still rejects a non-conventional subject. - let (accepted, _) = run_dispatcher(&hooks_dir, "commit-msg", "not conventional\n"); - assert!(!accepted, "surviving builtin must still run"); + let (accepted, _) = run_dispatcher(&hooks_dir, "commit-msg", "not conventional\n"); + assert!(!accepted, "surviving builtin must still run"); - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); - } + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); } #[serial] #[test] fn migration_absorbs_a_bare_builtin_script_when_a_new_builtin_is_added() { - let dir = tempfile::TempDir::new().unwrap(); - let hooks_dir = dir.path().join(".git").join("hooks"); - std::fs::create_dir_all(&hooks_dir).unwrap(); - // The pre-composition damaged shape: a bare builtin script sitting - // directly at .git/hooks/commit-msg. - let no_body = builtins::get("no-body").unwrap(); - std::fs::write(hooks_dir.join("commit-msg"), no_body.script).unwrap(); - let original = std::env::current_dir().ok(); - let _ = std::env::set_current_dir(dir.path()); + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + std::fs::create_dir_all(&hooks_dir).unwrap(); + let no_body = builtins::get("no-body").unwrap(); + std::fs::write(hooks_dir.join("commit-msg"), no_body.script).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + install_builtin("conventional-commits", true).unwrap(); + + let parts = list_parts(&hooks_dir, "commit-msg"); + assert_eq!( + parts, + vec!["conventional-commits".to_string(), "no-body".to_string()] + ); + let dispatcher_content = std::fs::read_to_string(hooks_dir.join("commit-msg")).unwrap(); + assert!(is_dispatcher(&dispatcher_content, "commit-msg")); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); + } - install_builtin("conventional-commits", true).unwrap(); + // ── GK-E: outdated builtin recognition ───────────────────────────────── - let parts = list_parts(&hooks_dir, "commit-msg"); - assert_eq!( - parts, - vec!["conventional-commits".to_string(), "no-body".to_string()] + #[test] + fn detect_builtin_by_marker_with_stale_content() { + let stale = "#!/bin/sh\n# gitkit-builtin: no-trailers\necho old-version\n"; + let detected = detect_builtin("commit-msg", stale); + assert!( + detected.is_some(), + "marker must identify the builtin even with stale content" ); - let dispatcher_content = std::fs::read_to_string(hooks_dir.join("commit-msg")).unwrap(); - assert!(is_dispatcher(&dispatcher_content, "commit-msg")); + assert_eq!(detected.unwrap().name, "no-trailers"); + } - if let Some(orig) = original { - let _ = std::env::set_current_dir(orig); + #[serial] + #[test] + fn installing_over_outdated_builtin_replaces_it_without_preserving() { + with_isolated_registry(|| { + let dir = tempfile::TempDir::new().unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + std::fs::create_dir_all(&hooks_dir).unwrap(); + std::fs::write( + hooks_dir.join("commit-msg"), + "#!/bin/sh\n# gitkit-builtin: no-trailers\necho old-version\n", + ) + .unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + install_builtin("conventional-commits", true).unwrap(); + + let parts = list_parts(&hooks_dir, "commit-msg"); + assert!( + !parts.contains(&PRESERVED_PART_NAME.to_string()), + "outdated builtin must not be preserved as 00-preexisting: {parts:?}" + ); + assert!(parts.contains(&"no-trailers".to_string())); + assert!(parts.contains(&"conventional-commits".to_string())); + + let nt_content = std::fs::read_to_string( + hooks_dir + .join("gitkit.d") + .join("commit-msg") + .join("no-trailers"), + ) + .unwrap(); + let current_nt = builtins::get("no-trailers").unwrap(); + assert_eq!(nt_content, current_nt.script); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + }); + } + + #[test] + fn detect_builtin_exact_content_fallback_for_markerless_script() { + let no_secrets = builtins::get("no-secrets").unwrap(); + let detected = detect_builtin("pre-commit", no_secrets.script); + assert!( + detected.is_some(), + "byte-identical script must still be detected" + ); + assert_eq!(detected.unwrap().name, "no-secrets"); + } + + #[test] + fn every_builtin_carries_a_marker_matching_its_name() { + for b in builtins::ALL { + let marker = extract_marker(b.script); + assert!( + marker.is_some(), + "builtin '{}' is missing its marker comment", + b.name + ); + assert_eq!( + marker.unwrap(), + b.name, + "builtin '{}' has marker for '{}' instead", + b.name, + marker.unwrap() + ); } } } diff --git a/src/ignore/mod.rs b/src/ignore/mod.rs index b9d84d2..008af98 100644 --- a/src/ignore/mod.rs +++ b/src/ignore/mod.rs @@ -536,6 +536,11 @@ skills-lock.json\n"; #[serial] #[test] fn add_templates_force_writes_gitignore() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::fs::create_dir(dir.path().join(".git")).unwrap(); let original = std::env::current_dir().ok(); @@ -547,11 +552,22 @@ skills-lock.json\n"; if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] #[test] fn add_templates_merge_with_existing_gitignore() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::fs::create_dir(dir.path().join(".git")).unwrap(); std::fs::write(dir.path().join(".gitignore"), "target/\n").unwrap(); @@ -565,11 +581,22 @@ skills-lock.json\n"; if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] #[test] fn add_templates_no_existing_gitignore() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } let dir = tempfile::TempDir::new().unwrap(); std::fs::create_dir(dir.path().join(".git")).unwrap(); let original = std::env::current_dir().ok(); @@ -581,6 +608,12 @@ skills-lock.json\n"; if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } // ── resolve_templates with builtins only ────────────────────────────── diff --git a/src/lock/mod.rs b/src/lock/mod.rs index b7c172d..a02aad1 100644 --- a/src/lock/mod.rs +++ b/src/lock/mod.rs @@ -50,6 +50,98 @@ fi exit 0 "#; +/// The pre-rebase hook gitkit installs. Same shape as the commit hook: +/// checks for `"rebase"` in `operations`, reads the lock file directly, +/// fails open on any missing/malformed/expired lock, and chains to a +/// backed-up user hook if any. +const REBASE_HOOK_SCRIPT: &str = r#"#!/bin/sh +# Installed by `gitkit lock`. Blocks rebases while a lock is active. +# See `gitkit lock status` / `gitkit unlock`. Bypass with `git rebase --no-verify`. + +git_dir=$(git rev-parse --git-dir 2>/dev/null) || exit 0 +lock_file="$git_dir/gitkit.lock" +orig_hook="$git_dir/hooks/pre-rebase.gitkit-orig" + +if [ -f "$lock_file" ]; then + ops=$(sed -n 's/.*"operations":\[\([^]]*\)\].*/\1/p' "$lock_file" 2>/dev/null) + if printf '%s' "$ops" | grep -qF '"rebase"'; then + expires=$(sed -n 's/.*"expires_at":"\([^"]*\)".*/\1/p' "$lock_file" 2>/dev/null) + blocked=1 + if [ -n "$expires" ]; then + now=$(date -u +%Y-%m-%dT%H:%M:%SZ) + if [ "$now" \> "$expires" ]; then + blocked=0 + fi + fi + if [ "$blocked" -eq 1 ]; then + reason=$(sed -n 's/.*"reason":"\([^"]*\)".*/\1/p' "$lock_file" 2>/dev/null) + echo "gitkit: rebase blocked - ${reason:-Agent session active}" >&2 + echo "gitkit: run 'gitkit unlock' to remove the lock, or 'git rebase --no-verify' to bypass it" >&2 + exit 1 + fi + fi +fi + +if [ -x "$orig_hook" ]; then + exec "$orig_hook" "$@" +fi + +exit 0 +"#; + +/// The reference-transaction hook gitkit installs with `--refs`. Pure POSIX +/// `sh`, no dependency on the `gitkit` binary — git calls this on its hot +/// ref-update path, several times per operation, so spawning a process here +/// would make ordinary git commands slow. Reads `gitkit.lock` directly. +/// +/// Rejects updates to `HEAD` and `refs/heads/*` while allowing `refs/remotes/*` +/// so `git fetch` keeps working. This is the only lock that `--no-verify` +/// cannot bypass. +/// +/// Git feeds ref updates on stdin as ` ` lines. We read them +/// all, check each one, and reject the whole transaction if any ref is +/// protected. +const REFERENCE_TRANSACTION_HOOK_SCRIPT: &str = r#"#!/bin/sh +# Installed by `gitkit lock --refs`. Blocks ref updates to HEAD and +# refs/heads/* while a lock is active. Allows refs/remotes/* so git fetch +# keeps working. This is the only lock that --no-verify cannot bypass. +# See `gitkit lock status` / `gitkit unlock`. + +git_dir=$(git rev-parse --git-dir 2>/dev/null) || exit 0 +lock_file="$git_dir/gitkit.lock" + +if [ ! -f "$lock_file" ]; then + exit 0 +fi + +ops=$(sed -n 's/.*"operations":\[\([^]]*\)\].*/\1/p' "$lock_file" 2>/dev/null) +if ! printf '%s' "$ops" | grep -qF '"refs"'; then + exit 0 +fi + +expires=$(sed -n 's/.*"expires_at":"\([^"]*\)".*/\1/p' "$lock_file" 2>/dev/null) +if [ -n "$expires" ]; then + now=$(date -u +%Y-%m-%dT%H:%M:%SZ) + if [ "$now" \> "$expires" ]; then + exit 0 + fi +fi + +reason=$(sed -n 's/.*"reason":"\([^"]*\)".*/\1/p' "$lock_file" 2>/dev/null) + +while read -r old new ref; do + case "$ref" in + HEAD|refs/heads/*) + echo "gitkit: ref update blocked - ${reason:-Agent session active}" >&2 + echo "gitkit: run 'gitkit unlock' to remove the lock" >&2 + exit 1 + ;; + esac +done + +exit 0 +"#; + /// The pre-push hook gitkit installs. Same shape as `LOCK_HOOK_SCRIPT`, but /// checks for `"push"` in `operations` instead of `"commit"`. Git feeds ref /// update lines on stdin; this hook never inspects them (decides purely from @@ -113,6 +205,16 @@ const PUSH_HOOK: HookSpec = HookSpec { script: PUSH_HOOK_SCRIPT, }; +const REBASE_HOOK: HookSpec = HookSpec { + name: "pre-rebase", + script: REBASE_HOOK_SCRIPT, +}; + +const REFERENCE_TRANSACTION_HOOK: HookSpec = HookSpec { + name: "reference-transaction", + script: REFERENCE_TRANSACTION_HOOK_SCRIPT, +}; + #[derive(Args)] pub struct LockArgs { #[command(subcommand)] @@ -129,6 +231,11 @@ pub struct LockArgs { /// Block both commits and pushes #[arg(long)] all: bool, + /// Block reference updates to HEAD and refs/heads/* (opt-in, never + /// included in --all or the default). This is the only lock that + /// --no-verify cannot bypass. + #[arg(long)] + refs: bool, } #[derive(Subcommand)] @@ -146,20 +253,25 @@ pub fn run(args: LockArgs) -> Result<()> { match args.action { Some(LockAction::Status { json }) => status(json), None => { - let ops = target_operations(args.push, args.all); + let ops = target_operations(args.push, args.all, args.refs); lock(args.timeout.as_deref(), args.reason.as_deref(), &ops) } } } -fn target_operations(push: bool, all: bool) -> Vec<&'static str> { - if all { +fn target_operations(push: bool, all: bool, refs: bool) -> Vec<&'static str> { + let mut ops = if all { vec!["commit", "push"] } else if push { vec!["push"] } else { vec!["commit"] + }; + ops.push("rebase"); + if refs { + ops.push("refs"); } + ops } pub fn unlock() -> Result<()> { @@ -170,6 +282,8 @@ pub fn unlock() -> Result<()> { } uninstall_hook(&COMMIT_HOOK)?; uninstall_hook(&PUSH_HOOK)?; + uninstall_hook(&REBASE_HOOK)?; + uninstall_hook(&REFERENCE_TRANSACTION_HOOK)?; println!("Unlocked. Commits and pushes are no longer blocked by gitkit."); Ok(()) } @@ -226,6 +340,13 @@ fn lock(timeout: Option<&str>, reason: Option<&str>, ops: &[&str]) -> Result<()> if lf.operations.iter().any(|op| op == "push") { install_hook(&PUSH_HOOK).context("Failed to install pre-push hook")?; } + if lf.operations.iter().any(|op| op == "rebase") { + install_hook(&REBASE_HOOK).context("Failed to install pre-rebase hook")?; + } + if lf.operations.iter().any(|op| op == "refs") { + install_hook(&REFERENCE_TRANSACTION_HOOK) + .context("Failed to install reference-transaction hook")?; + } println!("Locked: {}", lf.reason); println!("Locked operations: {}", lf.operations.join(", ")); @@ -343,6 +464,8 @@ fn status_report(lf: &LockFile, now: &str) -> Vec { let commit_locked = !expired && lf.operations.iter().any(|op| op == "commit"); let push_locked = !expired && lf.operations.iter().any(|op| op == "push"); + let rebase_locked = !expired && lf.operations.iter().any(|op| op == "rebase"); + let refs_locked = !expired && lf.operations.iter().any(|op| op == "refs"); lines.push(format!( "Commit: {}", if commit_locked { @@ -355,6 +478,18 @@ fn status_report(lf: &LockFile, now: &str) -> Vec { "Push: {}", if push_locked { "locked" } else { "not locked" } )); + lines.push(format!( + "Rebase: {}", + if rebase_locked { + "locked" + } else { + "not locked" + } + )); + lines.push(format!( + "Refs: {}", + if refs_locked { "locked" } else { "not locked" } + )); lines } @@ -1305,6 +1440,7 @@ mod tests { reason: None, push: false, all: false, + refs: false, }); assert!(result.is_ok()); @@ -1327,6 +1463,7 @@ mod tests { reason: Some("ci".to_string()), push: false, all: false, + refs: false, }); assert!(result.is_ok()); assert!(dir.path().join(".git").join(LOCK_FILE_NAME).exists()); @@ -1350,11 +1487,12 @@ mod tests { reason: None, push: true, all: false, + refs: false, }); assert!(result.is_ok()); let content = std::fs::read_to_string(dir.path().join(".git").join(LOCK_FILE_NAME)).unwrap(); - assert!(content.contains("\"operations\":[\"push\"]")); + assert!(content.contains("\"operations\":[\"push\",\"rebase\"]")); if let Some(orig) = original { let _ = std::env::set_current_dir(orig); @@ -1375,11 +1513,12 @@ mod tests { reason: None, push: false, all: true, + refs: false, }); assert!(result.is_ok()); let content = std::fs::read_to_string(dir.path().join(".git").join(LOCK_FILE_NAME)).unwrap(); - assert!(content.contains("\"operations\":[\"commit\",\"push\"]")); + assert!(content.contains("\"operations\":[\"commit\",\"push\",\"rebase\"]")); if let Some(orig) = original { let _ = std::env::set_current_dir(orig); @@ -1389,18 +1528,43 @@ mod tests { // ── target_operations ──────────────────────────────────────────────────── #[test] - fn target_operations_defaults_to_commit() { - assert_eq!(target_operations(false, false), vec!["commit"]); + fn target_operations_defaults_to_commit_and_rebase() { + assert_eq!( + target_operations(false, false, false), + vec!["commit", "rebase"] + ); + } + + #[test] + fn target_operations_push_flag_locks_push_and_rebase() { + assert_eq!( + target_operations(true, false, false), + vec!["push", "rebase"] + ); } #[test] - fn target_operations_push_flag_locks_push_only() { - assert_eq!(target_operations(true, false), vec!["push"]); + fn target_operations_all_flag_locks_commit_push_and_rebase() { + assert_eq!( + target_operations(false, true, false), + vec!["commit", "push", "rebase"] + ); } #[test] - fn target_operations_all_flag_locks_both() { - assert_eq!(target_operations(false, true), vec!["commit", "push"]); + fn target_operations_refs_flag_adds_refs() { + assert_eq!( + target_operations(false, false, true), + vec!["commit", "rebase", "refs"] + ); + } + + #[test] + fn target_operations_all_with_refs_does_not_duplicate() { + assert_eq!( + target_operations(false, true, true), + vec!["commit", "push", "rebase", "refs"] + ); } // ── status_report() per-operation reporting ────────────────────────────── @@ -1684,4 +1848,221 @@ mod tests { }; std::fs::write(path, lf.to_json()).unwrap(); } + + // ── rebase hook script sanity ──────────────────────────────────────────── + + #[test] + fn rebase_hook_script_is_valid_shell_shebang() { + assert!(REBASE_HOOK_SCRIPT.starts_with("#!/bin/sh")); + } + + #[test] + fn rebase_hook_script_references_lock_file_and_backup() { + assert!(REBASE_HOOK_SCRIPT.contains("gitkit.lock")); + assert!(REBASE_HOOK_SCRIPT.contains("pre-rebase.gitkit-orig")); + assert!(REBASE_HOOK_SCRIPT.contains("--no-verify")); + assert!(REBASE_HOOK_SCRIPT.contains("\"rebase\"")); + } + + // ── reference-transaction hook script sanity ───────────────────────────── + + #[test] + fn reference_transaction_hook_script_is_valid_shell_shebang() { + assert!(REFERENCE_TRANSACTION_HOOK_SCRIPT.starts_with("#!/bin/sh")); + } + + #[test] + fn reference_transaction_hook_script_does_not_invoke_gitkit_binary() { + assert!( + !REFERENCE_TRANSACTION_HOOK_SCRIPT.contains("$(gitkit"), + "reference-transaction hook must not spawn the gitkit binary" + ); + assert!( + !REFERENCE_TRANSACTION_HOOK_SCRIPT.contains("exec gitkit"), + "reference-transaction hook must not exec the gitkit binary" + ); + } + + #[test] + fn reference_transaction_hook_script_rejects_head_and_refs_heads() { + assert!(REFERENCE_TRANSACTION_HOOK_SCRIPT.contains("HEAD|refs/heads/*")); + } + + #[test] + fn reference_transaction_hook_script_mentions_gitkit_unlock() { + assert!(REFERENCE_TRANSACTION_HOOK_SCRIPT.contains("gitkit unlock")); + } + + // ── status_report with rebase and refs axes ────────────────────────────── + + #[test] + fn status_report_shows_rebase_and_refs_axes() { + let lf = LockFile { + locked_at: "2026-01-01T00:00:00Z".to_string(), + expires_at: None, + reason: "agent session".to_string(), + operations: vec![ + "commit".to_string(), + "push".to_string(), + "rebase".to_string(), + "refs".to_string(), + ], + }; + let lines = status_report(&lf, "2026-01-01T00:05:00Z").join("\n"); + assert!(lines.contains("Commit: locked"), "status was: {lines}"); + assert!(lines.contains("Push: locked"), "status was: {lines}"); + assert!(lines.contains("Rebase: locked"), "status was: {lines}"); + assert!(lines.contains("Refs: locked"), "status was: {lines}"); + } + + #[test] + fn status_report_shows_rebase_and_refs_not_locked_when_absent() { + let lf = LockFile { + locked_at: "2026-01-01T00:00:00Z".to_string(), + expires_at: None, + reason: "agent session".to_string(), + operations: vec!["commit".to_string()], + }; + let lines = status_report(&lf, "2026-01-01T00:05:00Z").join("\n"); + assert!(lines.contains("Rebase: not locked"), "status was: {lines}"); + assert!(lines.contains("Refs: not locked"), "status was: {lines}"); + } + + // ── lock with refs installs reference-transaction hook ─────────────────── + + #[serial] + #[test] + fn lock_with_refs_installs_reference_transaction_hook() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + lock(None, None, &["refs"]).unwrap(); + + let content = + std::fs::read_to_string(dir.path().join(".git").join(LOCK_FILE_NAME)).unwrap(); + assert!(content.contains("\"operations\":[\"refs\"]")); + assert!(dir + .path() + .join(".git") + .join("hooks") + .join("reference-transaction") + .exists()); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[serial] + #[test] + fn lock_with_rebase_installs_pre_rebase_hook() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + lock(None, None, &["rebase"]).unwrap(); + + let content = + std::fs::read_to_string(dir.path().join(".git").join(LOCK_FILE_NAME)).unwrap(); + assert!(content.contains("\"operations\":[\"rebase\"]")); + assert!(dir + .path() + .join(".git") + .join("hooks") + .join("pre-rebase") + .exists()); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[serial] + #[test] + fn unlock_removes_reference_transaction_hook() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + lock(None, None, &["refs"]).unwrap(); + assert!(dir + .path() + .join(".git") + .join("hooks") + .join("reference-transaction") + .exists()); + + unlock().unwrap(); + assert!(!dir + .path() + .join(".git") + .join("hooks") + .join("reference-transaction") + .exists()); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[serial] + #[test] + fn unlock_removes_pre_rebase_hook() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + lock(None, None, &["rebase"]).unwrap(); + assert!(dir + .path() + .join(".git") + .join("hooks") + .join("pre-rebase") + .exists()); + + unlock().unwrap(); + assert!(!dir + .path() + .join(".git") + .join("hooks") + .join("pre-rebase") + .exists()); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[serial] + #[test] + fn lock_with_commit_only_does_not_install_rebase_or_refs_hooks() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join(".git")).unwrap(); + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + + lock(None, None, &["commit"]).unwrap(); + + assert!(!dir + .path() + .join(".git") + .join("hooks") + .join("pre-rebase") + .exists()); + assert!(!dir + .path() + .join(".git") + .join("hooks") + .join("reference-transaction") + .exists()); + + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } } diff --git a/src/main.rs b/src/main.rs index 6c7536f..ec13afb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,6 +13,7 @@ mod init; mod lock; mod registry; mod status; +mod uninstall; mod utils; #[derive(Parser)] @@ -63,6 +64,8 @@ enum Command { Lock(lock::LockArgs), /// Remove an active commit/push lock Unlock, + /// Remove gitkit hooks from every repository it has touched + Uninstall(uninstall::UninstallArgs), } fn main() -> Result<()> { @@ -79,5 +82,6 @@ fn main() -> Result<()> { Some(Command::Build { action }) => builds::run(action), Some(Command::Lock(args)) => lock::run(args), Some(Command::Unlock) => lock::unlock(), + Some(Command::Uninstall(args)) => uninstall::run(args), } } diff --git a/src/registry/mod.rs b/src/registry/mod.rs index a04b5a3..dad33fe 100644 --- a/src/registry/mod.rs +++ b/src/registry/mod.rs @@ -24,11 +24,34 @@ pub(crate) struct RegistryEntry { pub applied: Vec, } +#[cfg(test)] +static TEST_REGISTRY_DIR: std::sync::OnceLock = std::sync::OnceLock::new(); + +#[cfg(test)] +fn ensure_test_registry_dir() -> PathBuf { + TEST_REGISTRY_DIR + .get_or_init(|| { + let dir = tempfile::TempDir::new().unwrap(); + dir.keep() + }) + .clone() +} + pub(crate) fn registry_path() -> Result { - let home = std::env::var("HOME") - .or_else(|_| std::env::var("USERPROFILE")) - .context("Neither HOME nor USERPROFILE environment variable is set")?; - Ok(PathBuf::from(home).join(".gitkit").join("registry.toml")) + if let Ok(gitkit_home) = std::env::var("GITKIT_HOME") { + return Ok(PathBuf::from(gitkit_home).join("registry.toml")); + } + #[cfg(test)] + { + Ok(ensure_test_registry_dir().join("registry.toml")) + } + #[cfg(not(test))] + { + let home = std::env::var("HOME") + .or_else(|_| std::env::var("USERPROFILE")) + .context("Neither HOME nor USERPROFILE environment variable is set")?; + Ok(PathBuf::from(home).join(".gitkit").join("registry.toml")) + } } /// Loads the ledger. A missing, empty, or unparseable file yields an empty @@ -47,7 +70,7 @@ pub(crate) fn load() -> Registry { pub(crate) fn save(registry: &Registry) -> Result<()> { let path = registry_path()?; if let Some(parent) = path.parent() { - fs::create_dir_all(parent).context("Failed to create ~/.gitkit directory")?; + fs::create_dir_all(parent).context("Failed to create gitkit registry directory")?; } let content = toml::to_string_pretty(registry).context("Failed to serialize registry")?; fs::write(&path, content).context("Failed to write registry")?; @@ -211,37 +234,69 @@ mod tests { use super::*; use serial_test::serial; - /// Points HOME at a fresh temp dir for the duration of `f`, restoring - /// the original value afterward. Never touches the real ~/.gitkit. - fn with_temp_home(f: F) { + /// Provides a clean, isolated registry for tests that need an empty starting state. + /// Sets GITKIT_HOME to a unique temp dir, ensuring no interference from other tests. + fn with_clean_registry(f: F) { let dir = tempfile::TempDir::new().unwrap(); - let original = std::env::var("HOME").ok(); + let orig = std::env::var("GITKIT_HOME").ok(); unsafe { - std::env::set_var("HOME", dir.path()); + std::env::set_var("GITKIT_HOME", dir.path()); } - f(dir.path()); + f(); unsafe { - match &original { - Some(h) => std::env::set_var("HOME", h), - None => std::env::remove_var("HOME"), + match &orig { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), } } } + #[test] + fn registry_path_returns_registry_toml_filename() { + let path = registry_path().unwrap(); + assert_eq!(path.file_name().unwrap(), "registry.toml"); + } + + #[test] + fn registry_path_resolves_under_test_dir_not_real_home() { + let path = registry_path().unwrap(); + let real_home = std::env::var("HOME").unwrap_or_default(); + let real_gitkit = PathBuf::from(&real_home) + .join(".gitkit") + .join("registry.toml"); + assert_ne!( + path, real_gitkit, + "registry_path() in tests must NOT resolve to the real ~/.gitkit/registry.toml" + ); + } + #[serial] #[test] - fn registry_path_lives_under_gitkit() { - with_temp_home(|_| { - let path = registry_path().unwrap(); - assert!(path.to_string_lossy().contains(".gitkit")); - assert_eq!(path.file_name().unwrap(), "registry.toml"); - }); + fn registry_path_honors_gitkit_home_over_test_default() { + let gitkit_home = tempfile::TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", gitkit_home.path()); + } + let path = registry_path().unwrap(); + assert!( + path.starts_with(gitkit_home.path()), + "GITKIT_HOME must take precedence: got {}", + path.display() + ); + assert_eq!(path.file_name().unwrap(), "registry.toml"); + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] #[test] fn load_missing_registry_returns_default() { - with_temp_home(|_| { + with_clean_registry(|| { let reg = load(); assert!(reg.repos.is_empty()); }); @@ -250,9 +305,8 @@ mod tests { #[serial] #[test] fn load_corrupt_registry_returns_default_not_panic() { - with_temp_home(|home| { - let dir = home.join(".gitkit"); - fs::create_dir_all(&dir).unwrap(); + with_clean_registry(|| { + let dir = ensure_test_registry_dir(); fs::write(dir.join("registry.toml"), "not valid toml {{{").unwrap(); let reg = load(); assert!(reg.repos.is_empty()); @@ -262,9 +316,8 @@ mod tests { #[serial] #[test] fn load_empty_registry_returns_default() { - with_temp_home(|home| { - let dir = home.join(".gitkit"); - fs::create_dir_all(&dir).unwrap(); + with_clean_registry(|| { + let dir = ensure_test_registry_dir(); fs::write(dir.join("registry.toml"), "").unwrap(); let reg = load(); assert!(reg.repos.is_empty()); @@ -274,7 +327,7 @@ mod tests { #[serial] #[test] fn record_writes_entry_with_absolute_path() { - with_temp_home(|_| { + with_clean_registry(|| { let repo = tempfile::TempDir::new().unwrap(); record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); let reg = load(); @@ -289,7 +342,7 @@ mod tests { #[serial] #[test] fn record_twice_updates_single_entry_not_duplicate() { - with_temp_home(|_| { + with_clean_registry(|| { let repo = tempfile::TempDir::new().unwrap(); record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); record(repo.path(), &["hook:conventional-commits".to_string()]).unwrap(); @@ -307,7 +360,7 @@ mod tests { #[serial] #[test] fn record_same_item_twice_does_not_duplicate_in_list() { - with_temp_home(|_| { + with_clean_registry(|| { let repo = tempfile::TempDir::new().unwrap(); record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); @@ -328,7 +381,7 @@ mod tests { #[serial] #[test] fn record_empty_items_is_noop() { - with_temp_home(|_| { + with_clean_registry(|| { let repo = tempfile::TempDir::new().unwrap(); record(repo.path(), &[]).unwrap(); let reg = load(); @@ -339,22 +392,36 @@ mod tests { #[serial] #[test] fn record_best_effort_swallows_write_failure_without_panic() { - with_temp_home(|home| { - // Put a plain file where the ~/.gitkit directory would go, so - // `fs::create_dir_all` inside `save` fails. `record_best_effort` - // must warn and return, never panic or propagate — the actual - // caller (a hook install) must not be failed by this. - fs::write(home.join(".gitkit"), "not a directory").unwrap(); - let repo = tempfile::TempDir::new().unwrap(); - record_best_effort(repo.path(), &["hook:no-secrets".to_string()]); - assert!(record(repo.path(), &["hook:no-secrets".to_string()]).is_err()); - }); + let blocker = tempfile::TempDir::new().unwrap(); + let blocker_file = blocker.path().join("blocker"); + fs::write(&blocker_file, "not a directory").unwrap(); + + let orig = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", &blocker_file); + } + + let repo = tempfile::TempDir::new().unwrap(); + let result = record(repo.path(), &["hook:no-secrets".to_string()]); + assert!( + result.is_err(), + "record must fail when GITKIT_HOME is a file" + ); + + record_best_effort(repo.path(), &["hook:no-secrets".to_string()]); + + unsafe { + match &orig { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] #[test] fn save_and_load_roundtrip() { - with_temp_home(|_| { + with_clean_registry(|| { let mut reg = Registry::default(); reg.repos.insert( "/tmp/example".to_string(), @@ -370,6 +437,43 @@ mod tests { }); } + #[serial] + #[test] + fn test_suite_does_not_write_to_real_registry() { + let real_home = match std::env::var("HOME") { + Ok(h) => h, + Err(_) => return, + }; + let real_registry = PathBuf::from(&real_home) + .join(".gitkit") + .join("registry.toml"); + let real_existed = real_registry.exists(); + let real_mtime_before = fs::metadata(&real_registry) + .ok() + .and_then(|m| m.modified().ok()); + + let repo = tempfile::TempDir::new().unwrap(); + crate::registry::record_best_effort( + repo.path(), + &["hook:test-isolation-check".to_string()], + ); + + if real_existed { + let real_mtime_after = fs::metadata(&real_registry) + .ok() + .and_then(|m| m.modified().ok()); + assert_eq!( + real_mtime_before, real_mtime_after, + "the real ~/.gitkit/registry.toml must not be modified by tests" + ); + } else { + assert!( + !real_registry.exists(), + "the real ~/.gitkit/registry.toml must not be created by tests" + ); + } + } + // ── civil_from_days / now_timestamp ───────────────────────────────────── #[test] diff --git a/src/status/mod.rs b/src/status/mod.rs index a34e342..698401b 100644 --- a/src/status/mod.rs +++ b/src/status/mod.rs @@ -103,14 +103,8 @@ fn run_global(prune: bool) -> Result<()> { for (path_str, entry) in ®istry.repos { let path = Path::new(path_str); - if !path.exists() { + if let Ok(false) = path.try_exists() { gone.push(path_str.clone()); - println!("{path_str}"); - println!( - " ✗ gone — repository no longer exists (last applied {})", - entry.applied_at - ); - println!(); continue; } @@ -138,7 +132,7 @@ fn run_global(prune: bool) -> Result<()> { }; match health { - HookHealth::Dormant => { + HookHealth::Dormant | HookHealth::ModifiedDormant => { any_dormant = true; problems.push(format!( " ✗ {name} ({hook_file}) — dormant: not executable, git ignores it" @@ -175,7 +169,7 @@ fn run_global(prune: bool) -> Result<()> { println!("Pruned {} repositories that no longer exist.", gone.len()); } else if !gone.is_empty() { println!( - "{} repositories are gone. Re-run with `gitkit status --global --prune` to remove them from the registry.", + "{} repositories are gone (path no longer exists). Re-run with `gitkit status --global --prune` to remove them from the registry.", gone.len() ); } @@ -269,7 +263,7 @@ fn print_hooks(repair: bool) -> Result { let health = crate::hooks::classify_hook(&hook_name, &path)?; - if repair && health == HookHealth::Dormant { + if repair && matches!(health, HookHealth::Dormant | HookHealth::ModifiedDormant) { crate::hooks::set_executable(&path)?; let label = builtin_label(&hook_name, &path); println!(" ✓ {label} ({hook_name}) — repaired: set executable, git will now run it"); @@ -288,6 +282,18 @@ fn print_hooks(repair: bool) -> Result { " ✗ {label} ({hook_name}) — dormant: not executable, so git ignores it and never runs it (fix with `gitkit status --repair`)" ); } + HookHealth::ModifiedDormant => { + dormant_found = true; + let content = fs::read_to_string(&path).unwrap_or_default(); + let first_cmd = content + .lines() + .find(|l| !l.starts_with('#') && !l.starts_with("set ") && !l.trim().is_empty()) + .unwrap_or("(custom)") + .trim(); + println!( + " ✗ {hook_name} — dormant: not executable, so git ignores it and never runs it; also modified: {first_cmd:?} (fix with `gitkit status --repair`)" + ); + } HookHealth::Modified => { let content = fs::read_to_string(&path).unwrap_or_default(); let first_cmd = content @@ -324,7 +330,7 @@ fn print_dispatcher_parts(hooks_dir: &Path, hook_name: &str, repair: bool) -> Re let part_path = crate::hooks::parts_dir(hooks_dir, hook_name).join(&part_name); let health = crate::hooks::classify_part(hooks_dir, hook_name, &part_name)?; - if repair && health == HookHealth::Dormant { + if repair && matches!(health, HookHealth::Dormant | HookHealth::ModifiedDormant) { crate::hooks::set_executable(&part_path)?; println!( " ✓ {part_name} ({hook_name}) — repaired: set executable, git will now run it" @@ -340,6 +346,12 @@ fn print_dispatcher_parts(hooks_dir: &Path, hook_name: &str, repair: bool) -> Re " ✗ {part_name} ({hook_name}) — dormant: not executable, so git ignores it and never runs it (fix with `gitkit status --repair`)" ); } + HookHealth::ModifiedDormant => { + dormant_found = true; + println!( + " ✗ {part_name} ({hook_name}) — dormant: not executable, so git ignores it and never runs it; also modified since install (fix with `gitkit status --repair`)" + ); + } HookHealth::Modified => { println!(" ~ {part_name} ({hook_name}) — modified since install"); } @@ -696,9 +708,12 @@ mod tests { #[serial] #[test] fn repair_sets_executable_bit_on_dormant_part_and_on_dispatcher() { - // GK-A: two builtins composed on one git hook via the gitkit.d - // dispatcher; both the dispatcher and one of its parts lose their - // executable bit, and `--repair` must fix both. + let home = TempDir::new().unwrap(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("GITKIT_HOME", home.path().join(".gitkit")); + } + let dir = TempDir::new().unwrap(); std::fs::create_dir(dir.path().join(".git")).unwrap(); let original = std::env::current_dir().ok(); @@ -737,6 +752,12 @@ mod tests { if let Some(orig) = original { let _ = std::env::set_current_dir(orig); } + unsafe { + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } } #[serial] @@ -928,9 +949,11 @@ mod tests { fs::create_dir_all(repo.path().join(".git").join("hooks")).unwrap(); let orig_home = std::env::var("HOME").ok(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); let orig_cwd = std::env::current_dir().ok(); unsafe { std::env::set_var("HOME", home.path()); + std::env::set_var("GITKIT_HOME", home.path().join(".gitkit")); } let _ = std::env::set_current_dir(repo.path()); @@ -944,6 +967,10 @@ mod tests { Some(h) => std::env::set_var("HOME", h), None => std::env::remove_var("HOME"), } + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } } } @@ -1147,9 +1174,6 @@ mod tests { #[test] fn ledger_write_failure_does_not_fail_hook_installation() { with_temp_home_and_repo(|home, repo| { - // Block ~/.gitkit from being created, so the registry write - // inside install_builtin fails. The hook install itself must - // still succeed. fs::write(home.join(".gitkit"), "not a directory").unwrap(); let result = crate::hooks::install_builtin("no-secrets", true); assert!(result.is_ok()); @@ -1157,4 +1181,170 @@ mod tests { assert!(hook_path.exists()); }); } + + #[serial] + #[test] + fn prune_removes_only_gone_entries_and_reports_count() { + with_temp_home_and_repo(|_home, repo| { + crate::hooks::install_builtin("no-secrets", true).unwrap(); + let live_key = repo.to_string_lossy().to_string(); + + let mut reg = crate::registry::load(); + let gone_paths: Vec = (0..3).map(|i| format!("/tmp/gone-repo-{i}")).collect(); + for fake_path in &gone_paths { + reg.repos.insert( + fake_path.clone(), + crate::registry::RegistryEntry { + path: fake_path.clone(), + applied_at: "2026-01-01T00:00:00Z".to_string(), + applied: vec!["hook:no-secrets".to_string()], + }, + ); + } + crate::registry::save(®).unwrap(); + + let _ = std::env::set_current_dir(std::env::temp_dir()); + fs::remove_dir_all(repo).unwrap(); + + run_global(true).unwrap(); + let reg_after = crate::registry::load(); + assert!( + !reg_after.repos.contains_key(&live_key), + "the gone live-key repo must be pruned" + ); + for g in &gone_paths { + assert!( + !reg_after.repos.contains_key(g), + "gone entry {g} must be pruned" + ); + } + }); + } + + #[serial] + #[test] + fn prune_does_not_remove_existing_path_that_is_not_a_repo() { + with_temp_home_and_repo(|_home, repo| { + crate::hooks::install_builtin("no-secrets", true).unwrap(); + + let existing_non_repo = tempfile::TempDir::new().unwrap(); + let non_repo_key = existing_non_repo.path().to_string_lossy().to_string(); + let mut reg = crate::registry::load(); + reg.repos.insert( + non_repo_key.clone(), + crate::registry::RegistryEntry { + path: non_repo_key.clone(), + applied_at: "2026-01-01T00:00:00Z".to_string(), + applied: vec!["hook:no-secrets".to_string()], + }, + ); + crate::registry::save(®).unwrap(); + + let repo_buf = repo.to_path_buf(); + let _ = std::env::set_current_dir(std::env::temp_dir()); + fs::remove_dir_all(&repo_buf).unwrap(); + + run_global(true).unwrap(); + let reg_after = crate::registry::load(); + assert!( + reg_after.repos.contains_key(&non_repo_key), + "a path that still exists on disk must survive prune even if it is not a repo" + ); + }); + } + + // ── GK-J: dormant check for non-executable hooks ──────────────────────── + + #[serial] + #[test] + fn non_executable_modified_hook_is_reported_as_dormant() { + let dir = TempDir::new().unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + std::fs::create_dir_all(&hooks_dir).unwrap(); + let path = hooks_dir.join("pre-push"); + std::fs::write(&path, "#!/bin/sh\ncargo test\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o644); + std::fs::set_permissions(&path, perms).unwrap(); + } + + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + let dormant_found = print_hooks(false).unwrap(); + #[cfg(unix)] + assert!( + dormant_found, + "a non-executable modified hook must be reported dormant" + ); + #[cfg(not(unix))] + assert!( + !dormant_found, + "the executable bit does not apply on non-Unix targets" + ); + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[serial] + #[test] + fn repair_sets_executable_bit_on_non_executable_modified_hook() { + let dir = TempDir::new().unwrap(); + let hooks_dir = dir.path().join(".git").join("hooks"); + std::fs::create_dir_all(&hooks_dir).unwrap(); + let path = hooks_dir.join("pre-push"); + std::fs::write(&path, "#!/bin/sh\ncargo test\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o644); + std::fs::set_permissions(&path, perms).unwrap(); + } + + let original = std::env::current_dir().ok(); + let _ = std::env::set_current_dir(dir.path()); + let dormant_found = print_hooks(true).unwrap(); + assert!(!dormant_found, "repair should leave nothing dormant"); + #[cfg(unix)] + assert!( + crate::hooks::is_executable(&path).unwrap(), + "repair must set the executable bit on a modified hook" + ); + if let Some(orig) = original { + let _ = std::env::set_current_dir(orig); + } + } + + #[test] + fn classify_hook_returns_modified_dormant_for_non_executable_non_builtin() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("pre-push"); + std::fs::write(&path, "#!/bin/sh\ncargo test\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o644); + std::fs::set_permissions(&path, perms).unwrap(); + } + let health = crate::hooks::classify_hook("pre-push", &path).unwrap(); + #[cfg(unix)] + assert_eq!(health, HookHealth::ModifiedDormant); + #[cfg(not(unix))] + assert_eq!(health, HookHealth::Modified); + } + + #[test] + fn classify_hook_returns_modified_for_executable_non_builtin() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("pre-push"); + std::fs::write(&path, "#!/bin/sh\ncargo test\n").unwrap(); + crate::hooks::set_executable(&path).unwrap(); + let health = crate::hooks::classify_hook("pre-push", &path).unwrap(); + assert_eq!(health, HookHealth::Modified); + } } diff --git a/src/uninstall/mod.rs b/src/uninstall/mod.rs new file mode 100644 index 0000000..1aee4c4 --- /dev/null +++ b/src/uninstall/mod.rs @@ -0,0 +1,517 @@ +use anyhow::{Context, Result}; +use clap::Args; +use std::fs; +use std::path::{Path, PathBuf}; + +use crate::hooks; +use crate::registry; + +#[derive(Args)] +pub struct UninstallArgs { + /// Also remove local state under ~/.gitkit (builds, registry) + #[arg(long)] + pub data: bool, + + /// Skip the confirmation prompt + #[arg(short, long)] + pub yes: bool, + + /// Print what would be done without changing anything + #[arg(long)] + pub dry_run: bool, +} + +pub fn run(args: UninstallArgs) -> Result<()> { + let plan = build_plan(args.data)?; + + print_plan(&plan); + + if plan.is_empty() { + println!("\nNothing to uninstall."); + print_binary_note(); + return Ok(()); + } + + if args.dry_run { + println!("\n[dry-run] No changes made."); + return Ok(()); + } + + if !args.yes && !crate::utils::confirm("\nProceed with uninstall?", false) { + println!("Aborted."); + return Ok(()); + } + + execute_plan(&plan)?; + + if plan.remove_local_data { + remove_local_data()?; + } + + println!("\nUninstall complete."); + print_binary_note(); + Ok(()) +} + +fn print_binary_note() { + println!("\nThe gitkit binary was not removed."); + if let Some(method) = detect_install_method() { + println!("It was installed via {method} — remove it with that tool's uninstall command."); + } else { + println!("Remove it manually from wherever it was installed."); + } +} + +fn detect_install_method() -> Option<&'static str> { + let exe = std::env::current_exe().ok()?; + let exe_str = exe.to_string_lossy(); + if exe_str.contains(".cargo/bin") { + Some("cargo") + } else if exe_str.contains(".local/bin") || exe_str.contains("/usr/local/bin") { + Some("the install script") + } else if exe_str.contains("Homebrew") || exe_str.contains("homebrew") { + Some("Homebrew") + } else { + None + } +} + +// ── Plan ───────────────────────────────────────────────────────────────────── + +struct UninstallPlan { + repos: Vec, + remove_local_data: bool, +} + +impl UninstallPlan { + fn is_empty(&self) -> bool { + self.repos.iter().all(|r| !r.exists || r.hooks.is_empty()) && !self.remove_local_data + } +} + +struct RepoPlan { + path: String, + exists: bool, + hooks: Vec, +} + +struct HookPlan { + hook_name: String, + has_dispatcher: bool, + parts: Vec, + has_preexisting: bool, +} + +fn build_plan(include_data: bool) -> Result { + let reg = registry::load(); + let mut repos = Vec::new(); + + for (key, entry) in ®.repos { + let repo_path = PathBuf::from(&entry.path); + let exists = repo_path.exists(); + + let mut hook_plans = Vec::new(); + + if exists { + let git_dir = repo_path.join(".git"); + if git_dir.exists() { + let hooks_dir = git_dir.join("hooks"); + if hooks_dir.exists() { + for hook_name in hooks::valid_hook_names() { + let dispatcher_path = hooks_dir.join(hook_name); + if !dispatcher_path.exists() { + continue; + } + let content = fs::read_to_string(&dispatcher_path).unwrap_or_default(); + if !hooks::is_dispatcher(&content, hook_name) { + continue; + } + + let parts = hooks::list_parts(&hooks_dir, hook_name); + let has_preexisting = parts.iter().any(|p| p == hooks::PRESERVED_PART_NAME); + + hook_plans.push(HookPlan { + hook_name: hook_name.to_string(), + has_dispatcher: true, + parts, + has_preexisting, + }); + } + } + } + } + + repos.push(RepoPlan { + path: key.clone(), + exists, + hooks: hook_plans, + }); + } + + Ok(UninstallPlan { + repos, + remove_local_data: include_data, + }) +} + +fn print_plan(plan: &UninstallPlan) { + println!("gitkit uninstall will:"); + + let mut any_repo = false; + for repo in &plan.repos { + if !repo.exists { + println!("\n {} — repository no longer exists, skipping", repo.path); + continue; + } + if repo.hooks.is_empty() { + println!("\n {} — no gitkit hooks found, skipping", repo.path); + continue; + } + + any_repo = true; + println!("\n {}", repo.path); + for hook in &repo.hooks { + if hook.has_preexisting { + println!( + " restore .git/hooks/{} from absorbed hand-written hook", + hook.hook_name + ); + } + if hook.has_dispatcher { + println!( + " remove .git/hooks/{} (gitkit dispatcher)", + hook.hook_name + ); + } + if !hook.parts.is_empty() { + println!( + " remove .git/hooks/gitkit.d/{}/ ({} part{})", + hook.hook_name, + hook.parts.len(), + if hook.parts.len() == 1 { "" } else { "s" } + ); + } + } + } + + if !any_repo && plan.repos.is_empty() { + println!(" (no repositories in registry)"); + } + + if plan.remove_local_data { + println!("\n remove ~/.gitkit/ (local state: builds, registry)"); + } else { + println!("\n keep ~/.gitkit/ (use --data to also remove local state)"); + } +} + +// ── Execute ────────────────────────────────────────────────────────────────── + +fn execute_plan(plan: &UninstallPlan) -> Result<()> { + for repo in &plan.repos { + if !repo.exists { + continue; + } + if repo.hooks.is_empty() { + continue; + } + + let repo_path = PathBuf::from(&repo.path); + if let Err(e) = execute_repo_plan(&repo_path, &repo.hooks) { + eprintln!("Warning: failed to clean {}: {e}", repo.path); + } + } + + remove_registry_entries(plan)?; + + Ok(()) +} + +fn execute_repo_plan(repo_path: &Path, hooks: &[HookPlan]) -> Result<()> { + let hooks_dir = repo_path.join(".git").join("hooks"); + + for hook in hooks { + if let Err(e) = execute_hook_plan(&hooks_dir, hook) { + eprintln!( + "Warning: failed to clean hook {} in {}: {e}", + hook.hook_name, + repo_path.display() + ); + } + } + + Ok(()) +} + +fn execute_hook_plan(hooks_dir: &Path, hook: &HookPlan) -> Result<()> { + let dispatcher_path = hooks_dir.join(&hook.hook_name); + let parts_dir = hooks::parts_dir(hooks_dir, &hook.hook_name); + + if hook.has_preexisting { + let preexisting_path = parts_dir.join(hooks::PRESERVED_PART_NAME); + if preexisting_path.exists() { + let content = fs::read(&preexisting_path) + .with_context(|| format!("Failed to read preserved hook for {}", hook.hook_name))?; + fs::write(&dispatcher_path, &content) + .with_context(|| format!("Failed to restore original {} hook", hook.hook_name))?; + hooks::set_executable(&dispatcher_path)?; + } + } + + if parts_dir.exists() { + let _ = fs::remove_dir_all(&parts_dir); + } + + if hook.has_dispatcher && dispatcher_path.exists() { + let content = fs::read_to_string(&dispatcher_path).unwrap_or_default(); + if hooks::is_dispatcher(&content, &hook.hook_name) { + fs::remove_file(&dispatcher_path) + .with_context(|| format!("Failed to remove dispatcher for {}", hook.hook_name))?; + } + } + + Ok(()) +} + +fn remove_registry_entries(plan: &UninstallPlan) -> Result<()> { + let mut reg = registry::load(); + for repo in &plan.repos { + reg.repos.remove(&repo.path); + } + registry::save(®) +} + +fn remove_local_data() -> Result<()> { + let home = std::env::var("HOME") + .or_else(|_| std::env::var("USERPROFILE")) + .context("Neither HOME nor USERPROFILE environment variable is set")?; + let gitkit_dir = PathBuf::from(home).join(".gitkit"); + if gitkit_dir.exists() { + fs::remove_dir_all(&gitkit_dir).context("Failed to remove ~/.gitkit")?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serial_test::serial; + + fn with_temp_home(f: F) { + let dir = tempfile::TempDir::new().unwrap(); + let original = std::env::var("HOME").ok(); + let orig_gitkit_home = std::env::var("GITKIT_HOME").ok(); + unsafe { + std::env::set_var("HOME", dir.path()); + std::env::set_var("GITKIT_HOME", dir.path().join(".gitkit")); + } + f(dir.path()); + unsafe { + match &original { + Some(h) => std::env::set_var("HOME", h), + None => std::env::remove_var("HOME"), + } + match &orig_gitkit_home { + Some(h) => std::env::set_var("GITKIT_HOME", h), + None => std::env::remove_var("GITKIT_HOME"), + } + } + } + + fn init_repo(path: &Path) { + fs::create_dir_all(path.join(".git").join("hooks")).unwrap(); + } + + fn install_dispatcher(hooks_dir: &Path, hook_name: &str) { + let script = hooks::dispatcher_script(hook_name); + let hook_path = hooks_dir.join(hook_name); + fs::write(&hook_path, &script).unwrap(); + hooks::set_executable(&hook_path).unwrap(); + } + + fn install_part(hooks_dir: &Path, hook_name: &str, part_name: &str, content: &str) { + let parts_dir = hooks::parts_dir(hooks_dir, hook_name); + fs::create_dir_all(&parts_dir).unwrap(); + let part_path = parts_dir.join(part_name); + fs::write(&part_path, content).unwrap(); + hooks::set_executable(&part_path).unwrap(); + } + + #[serial] + #[test] + fn uninstall_removes_dispatcher_and_parts() { + with_temp_home(|_home| { + let repo = tempfile::TempDir::new().unwrap(); + init_repo(repo.path()); + let hooks_dir = repo.path().join(".git").join("hooks"); + install_dispatcher(&hooks_dir, "pre-commit"); + let builtin = hooks::builtins::get("no-secrets").unwrap(); + install_part(&hooks_dir, "pre-commit", "no-secrets", builtin.script); + + registry::record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); + + let plan = build_plan(false).unwrap(); + assert_eq!(plan.repos.len(), 1); + assert_eq!(plan.repos[0].hooks.len(), 1); + assert!(plan.repos[0].hooks[0].has_dispatcher); + + execute_plan(&plan).unwrap(); + + assert!(!hooks_dir.join("pre-commit").exists()); + assert!(!hooks::parts_dir(&hooks_dir, "pre-commit").exists()); + }); + } + + #[serial] + #[test] + fn uninstall_restores_preexisting_hook() { + with_temp_home(|_home| { + let repo = tempfile::TempDir::new().unwrap(); + init_repo(repo.path()); + let hooks_dir = repo.path().join(".git").join("hooks"); + + let original_hook = "#!/bin/sh\necho my custom hook\n"; + install_dispatcher(&hooks_dir, "pre-commit"); + install_part( + &hooks_dir, + "pre-commit", + hooks::PRESERVED_PART_NAME, + original_hook, + ); + let builtin = hooks::builtins::get("no-secrets").unwrap(); + install_part(&hooks_dir, "pre-commit", "no-secrets", builtin.script); + + registry::record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); + + let plan = build_plan(false).unwrap(); + assert!(plan.repos[0].hooks[0].has_preexisting); + + execute_plan(&plan).unwrap(); + + let restored = hooks_dir.join("pre-commit"); + assert!(restored.exists()); + let content = fs::read_to_string(&restored).unwrap(); + assert_eq!(content, original_hook); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = fs::metadata(&restored).unwrap().permissions(); + assert!( + perms.mode() & 0o111 != 0, + "restored hook should be executable" + ); + } + + assert!(!hooks::parts_dir(&hooks_dir, "pre-commit").exists()); + }); + } + + #[serial] + #[test] + fn uninstall_skips_missing_repo() { + with_temp_home(|_home| { + let fake_path = "/tmp/nonexistent-repo-gitkit-test-12345"; + let mut reg = registry::load(); + reg.repos.insert( + fake_path.to_string(), + registry::RegistryEntry { + path: fake_path.to_string(), + applied_at: "2026-01-01T00:00:00Z".to_string(), + applied: vec!["hook:no-secrets".to_string()], + }, + ); + registry::save(®).unwrap(); + + let plan = build_plan(false).unwrap(); + assert_eq!(plan.repos.len(), 1); + assert!(!plan.repos[0].exists); + + execute_plan(&plan).unwrap(); + + let reg_after = registry::load(); + assert!(!reg_after.repos.contains_key(fake_path)); + }); + } + + #[serial] + #[test] + fn dry_run_does_not_change_disk() { + with_temp_home(|_home| { + let repo = tempfile::TempDir::new().unwrap(); + init_repo(repo.path()); + let hooks_dir = repo.path().join(".git").join("hooks"); + install_dispatcher(&hooks_dir, "pre-commit"); + let builtin = hooks::builtins::get("no-secrets").unwrap(); + install_part(&hooks_dir, "pre-commit", "no-secrets", builtin.script); + + registry::record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); + + let args = UninstallArgs { + data: false, + yes: false, + dry_run: true, + }; + run(args).unwrap(); + + assert!(hooks_dir.join("pre-commit").exists()); + assert!(hooks::parts_dir(&hooks_dir, "pre-commit").exists()); + }); + } + + #[serial] + #[test] + fn without_data_flag_gitkit_dir_survives() { + with_temp_home(|home| { + let gitkit_dir = home.join(".gitkit"); + fs::create_dir_all(&gitkit_dir).unwrap(); + fs::write(gitkit_dir.join("registry.toml"), "").unwrap(); + + let repo = tempfile::TempDir::new().unwrap(); + init_repo(repo.path()); + let hooks_dir = repo.path().join(".git").join("hooks"); + install_dispatcher(&hooks_dir, "pre-commit"); + let builtin = hooks::builtins::get("no-secrets").unwrap(); + install_part(&hooks_dir, "pre-commit", "no-secrets", builtin.script); + + registry::record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); + + let args = UninstallArgs { + data: false, + yes: true, + dry_run: false, + }; + run(args).unwrap(); + + assert!(gitkit_dir.exists()); + }); + } + + #[serial] + #[test] + fn with_data_flag_removes_gitkit_dir() { + with_temp_home(|home| { + let gitkit_dir = home.join(".gitkit"); + fs::create_dir_all(&gitkit_dir).unwrap(); + fs::write(gitkit_dir.join("registry.toml"), "").unwrap(); + + let repo = tempfile::TempDir::new().unwrap(); + init_repo(repo.path()); + let hooks_dir = repo.path().join(".git").join("hooks"); + install_dispatcher(&hooks_dir, "pre-commit"); + let builtin = hooks::builtins::get("no-secrets").unwrap(); + install_part(&hooks_dir, "pre-commit", "no-secrets", builtin.script); + + registry::record(repo.path(), &["hook:no-secrets".to_string()]).unwrap(); + + let args = UninstallArgs { + data: true, + yes: true, + dry_run: false, + }; + run(args).unwrap(); + + assert!(!gitkit_dir.exists()); + }); + } +} diff --git a/tests/integration.rs b/tests/integration.rs index 935ba50..d8dbd3c 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -996,10 +996,6 @@ fn lock_status_json_exit_code_zero_when_unlocked() { assert!(stdout.contains("\"active\":false"), "stdout was: {stdout}"); assert!(stdout.contains("\"expired\":false"), "stdout was: {stdout}"); assert!(stdout.contains("\"operations\":[]"), "stdout was: {stdout}"); - assert!( - stdout.contains("\"locked_at\":null"), - "stdout was: {stdout}" - ); } #[test] @@ -1038,7 +1034,7 @@ fn lock_status_json_exit_code_nonzero_when_locked() { "stdout was: {stdout}" ); assert!( - stdout.contains("\"operations\":[\"commit\"]"), + stdout.contains("\"operations\":[\"commit\",\"rebase\"]"), "stdout was: {stdout}" ); } @@ -1396,3 +1392,269 @@ fn status_lists_both_builtins_installed_for_one_git_hook() { ); assert!(stdout.contains("no-body"), "status output was:\n{stdout}"); } + +// ═══════════════════════════════════════════════════════════════════════════ +// Lock / pre-rebase integration tests +// ═══════════════════════════════════════════════════════════════════════════ + +fn git_rebase(dir: &std::path::Path, onto: &str) -> (bool, String) { + let output = Command::new("git") + .args(["rebase", onto]) + .current_dir(dir) + .output() + .expect("Failed to run git rebase"); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + (output.status.success(), format!("{stdout}{stderr}")) +} + +#[test] +fn lock_fixture_pre_rebase_blocks_and_unlock_releases() { + let dir = TempDir::new().unwrap(); + init_git_repo(dir.path()); + let binary = gitkit_binary(); + + let (ok, _) = git_commit_allow_empty(dir.path(), "initial commit"); + assert!(ok); + + let status = Command::new("git") + .args(["checkout", "-b", "feature"]) + .current_dir(dir.path()) + .status() + .unwrap(); + assert!(status.success()); + + let (ok, _) = git_commit_allow_empty(dir.path(), "feature commit"); + assert!(ok); + + let status = Command::new("git") + .args(["checkout", "master"]) + .current_dir(dir.path()) + .status() + .unwrap(); + assert!(status.success()); + + let (ok, _) = git_commit_allow_empty(dir.path(), "master commit"); + assert!(ok); + + let status = Command::new("git") + .args(["checkout", "feature"]) + .current_dir(dir.path()) + .status() + .unwrap(); + assert!(status.success()); + + // `gitkit lock` installs pre-rebase unconditionally alongside commit. + let lock_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["lock", "--reason", "Agent session active"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit lock"); + assert!(lock_out.status.success()); + + let hooks_dir = dir.path().join(".git").join("hooks"); + assert!( + hooks_dir.join("pre-rebase").exists(), + "pre-rebase hook should be installed by `gitkit lock`" + ); + + let (ok, msg) = git_rebase(dir.path(), "master"); + assert!(!ok, "rebase should fail while locked"); + assert!(msg.contains("rebase blocked"), "message was: {msg}"); + assert!(msg.contains("gitkit unlock"), "message was: {msg}"); + + // Unlock: rebase succeeds. + let unlock_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["unlock"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit unlock"); + assert!(unlock_out.status.success()); + + let (ok, _) = git_rebase(dir.path(), "master"); + assert!(ok, "rebase should succeed after unlock"); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Lock / reference-transaction integration tests +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn lock_fixture_reference_transaction_rejects_head_update() { + let dir = TempDir::new().unwrap(); + init_git_repo(dir.path()); + let binary = gitkit_binary(); + + let (ok, _) = git_commit_allow_empty(dir.path(), "initial commit"); + assert!(ok); + let (ok, _) = git_commit_allow_empty(dir.path(), "second commit"); + assert!(ok); + + // Install the reference-transaction hook via --refs. + let lock_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["lock", "--refs", "--reason", "Agent session active"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit lock --refs"); + assert!(lock_out.status.success()); + + let hooks_dir = dir.path().join(".git").join("hooks"); + assert!( + hooks_dir.join("reference-transaction").exists(), + "reference-transaction hook should be installed" + ); + + // Attempt to update HEAD to the previous commit (simulates a ref update). + let output = Command::new("git") + .args(["update-ref", "HEAD", "HEAD~1"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run git update-ref"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "HEAD update should be rejected by reference-transaction hook" + ); + assert!(stderr.contains("gitkit unlock"), "message was: {stderr}"); +} + +#[test] +fn lock_fixture_reference_transaction_permits_refs_remotes() { + let dir = TempDir::new().unwrap(); + init_git_repo(dir.path()); + let binary = gitkit_binary(); + + let (ok, _) = git_commit_allow_empty(dir.path(), "initial commit"); + assert!(ok); + + // Install the reference-transaction hook via --refs. + let lock_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["lock", "--refs", "--reason", "Agent session active"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit lock --refs"); + assert!(lock_out.status.success()); + + // Updating a refs/remotes/* ref should succeed. + let output = Command::new("git") + .args(["update-ref", "refs/remotes/origin/main", "HEAD"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run git update-ref"); + assert!( + output.status.success(), + "refs/remotes update should be permitted: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn lock_fixture_refs_not_in_default_lock() { + let dir = TempDir::new().unwrap(); + init_git_repo(dir.path()); + let binary = gitkit_binary(); + + let lock_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["lock"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit lock"); + assert!(lock_out.status.success()); + + let hooks_dir = dir.path().join(".git").join("hooks"); + assert!( + !hooks_dir.join("reference-transaction").exists(), + "reference-transaction hook should NOT be installed by default" + ); + + let lock_path = dir.path().join(".git").join("gitkit.lock"); + let content = std::fs::read_to_string(&lock_path).unwrap(); + assert!( + !content.contains("\"refs\""), + "refs should not be in default lock operations: {content}" + ); +} + +#[test] +fn lock_fixture_status_reports_axes() { + let dir = TempDir::new().unwrap(); + init_git_repo(dir.path()); + let binary = gitkit_binary(); + + let lock_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["lock", "--refs", "--reason", "Agent session active"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit lock --refs"); + assert!(lock_out.status.success()); + + let status_out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", dir.path()) + .args(["lock", "status"]) + .current_dir(dir.path()) + .output() + .expect("Failed to run gitkit lock status"); + assert!(status_out.status.success()); + + let stdout = String::from_utf8_lossy(&status_out.stdout); + assert!(stdout.contains("Commit:"), "status was: {stdout}"); + assert!(stdout.contains("Push:"), "status was: {stdout}"); + assert!(stdout.contains("Rebase:"), "status was: {stdout}"); + assert!(stdout.contains("Refs:"), "status was: {stdout}"); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// GK-I: `status --global` summarises gone entries instead of listing each +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn status_global_summarizes_gone_entries_in_one_line() { + let home = TempDir::new().unwrap(); + let gitkit_dir = home.path().join(".gitkit"); + std::fs::create_dir_all(&gitkit_dir).unwrap(); + + let mut registry_content = String::from("[repos]\n"); + for i in 0..5 { + let fake_path = format!("/tmp/gone-repo-{i}"); + registry_content.push_str(&format!( + "[repos.\"{fake_path}\"]\npath = \"{fake_path}\"\napplied_at = \"2026-01-01T00:00:00Z\"\napplied = [\"hook:no-secrets\"]\n" + )); + } + std::fs::write(gitkit_dir.join("registry.toml"), ®istry_content).unwrap(); + + let binary = gitkit_binary(); + let out = Command::new(&binary) + .env("GITKIT_NO_UPDATE_CHECK", "1") + .env("HOME", home.path()) + .args(["status", "--global"]) + .current_dir(home.path()) + .output() + .expect("Failed to run gitkit status --global"); + assert!(out.status.success()); + + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("5 repositories are gone"), + "expected a single summary line for 5 gone entries, stdout was:\n{stdout}" + ); + for i in 0..5 { + let fake_path = format!("/tmp/gone-repo-{i}"); + assert!( + !stdout.contains(&fake_path), + "stdout must NOT list individual gone entry {fake_path}, stdout was:\n{stdout}" + ); + } +}