From 681cdf5af20adc6e76edadf15d8e116a232308b1 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 17:35:59 -0700 Subject: [PATCH 01/10] fix: sync CLAUDE.md test counts and dependency description with codebase Update test count to 231 (178 unit + 4 doc + 17 integration + 10 proptest + 22 memory). Mention rusqlite as memory crate dependency. Fix trailing period on H-4 purity bullet. --- CLAUDE.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9760beb..c16a5e7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,7 +92,7 @@ After pushing, poll PR checks and review comments in a single loop for up to 10 ## Key Design Decisions - **Three-layer sandbox**: Session resolution -> context-aware path checking -> git safety regex -- **H-4 purity**: PreToolUse hook (`permissions`) NEVER writes files. Uses `resolve_readonly()` +- **H-4 purity**: PreToolUse hook (`permissions`) NEVER writes files. Uses `resolve_readonly()`. - **Lazy worktrees**: `WORKTREE_MISSING:` denials trigger `ensure-worktree` on-demand - **Config persistence**: `.agents/`, `.claude/` redirect to main checkout when gitignored; if tracked by git (dir exists in worktree), allowed in-place - **Committed repo files**: `CLAUDE.md`, `AGENTS.md` are version-controlled — allowed in worktrees @@ -177,7 +177,7 @@ All shell scripts follow the [Google Shell Style Guide](https://google.github.io ## Testing -219 tests (166 unit + 5 doc + 13 integration + 10 proptest + 25 memory) plus 4 fuzz targets. +231 tests (178 unit + 4 doc + 17 integration + 10 proptest + 22 memory) plus 4 fuzz targets. Run with `make test` or `cargo test`. Test patterns: @@ -230,5 +230,5 @@ Every workflow change must pass `actionlint` + `zizmor --pedantic` in CI. ## Dependencies -5 crates: `serde`, `serde_json`, `regex`, `flate2`, `libc`. No async runtime, -no network deps, no proc macros. +5 crates: `serde`, `serde_json`, `regex`, `flate2`, `libc`. Memory crate adds +`rusqlite`. No async runtime, no network deps, no proc macros. From 0ce43606558b86acda22ee411f8673fdb7fa87a9 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 17:38:21 -0700 Subject: [PATCH 02/10] feat: add WHAT/FIX/REF remediation format to all denial messages (directive 22) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restructure all hook denial messages to follow the harness engineering WHAT/FIX/REF format so the agent can self-repair without human intervention: - lib.rs: WORKTREE_MISSING now includes WHAT/FIX/REF - sandbox.rs: REDIRECT and fallback BLOCKED messages use WHAT/FIX/REF - gitcheck.rs: all 8 git safety blocks + worktree enforcement use WHAT/FIX/REF with pointers to CLAUDE.md and docs/architecture.md Reference: harness.md Pillar 2c — custom linters with remediation messages. --- hooks/src/gitcheck.rs | 65 +++++++++++++++++++++++++++++-------------- hooks/src/lib.rs | 8 ++++-- hooks/src/sandbox.rs | 13 +++++++-- 3 files changed, 60 insertions(+), 26 deletions(-) diff --git a/hooks/src/gitcheck.rs b/hooks/src/gitcheck.rs index 4de1cde..25cb2df 100644 --- a/hooks/src/gitcheck.rs +++ b/hooks/src/gitcheck.rs @@ -72,6 +72,9 @@ static RE_GIT_C_PATH: LazyLock = LazyLock::new(|| Regex::new(r#"\bgit\s+-C\s+("[^"]+"|'[^']+'|(\S+))"#).unwrap()); /// Run all 8 git safety checks against a Bash command. +/// +/// Denial messages use the WHAT/FIX/REF remediation format so the agent +/// can self-repair without human intervention. pub fn check_git_safety(cmd: &str) -> GitResult { // FR-GS-1: Force push without --force-with-lease if RE_GIT_PUSH.is_match(cmd) @@ -79,67 +82,86 @@ pub fn check_git_safety(cmd: &str) -> GitResult { && !RE_FORCE_WITH_LEASE.is_match(cmd) { return GitResult::Block( - "BLOCKED: Force push without --force-with-lease. Use: git push --force-with-lease origin ".into(), + "WHAT: Force push without --force-with-lease. \ + FIX: Use `git push --force-with-lease origin ` instead. \ + REF: CLAUDE.md#supply-chain-policy".into(), ); } // FR-GS-2: Push to main/master if RE_PUSH_TO_MAIN.is_match(cmd) { return GitResult::Block( - "BLOCKED: Direct push to main/master. Create a feature branch and open a PR instead." - .into(), + "WHAT: Direct push to main/master. \ + FIX: Create a feature branch and open a PR instead. \ + REF: CLAUDE.md#commit-convention".into(), ); } // FR-GS-3: Refspec push to main/master if RE_REFSPEC_MAIN.is_match(cmd) { return GitResult::Block( - "BLOCKED: Push to main/master via refspec. Create a feature branch and open a PR instead." - .into(), + "WHAT: Push to main/master via refspec. \ + FIX: Create a feature branch and open a PR instead. \ + REF: CLAUDE.md#commit-convention".into(), ); } // FR-GS-4: Delete main/master if RE_DELETE_MAIN.is_match(cmd) { - return GitResult::Block("BLOCKED: Deleting main/master branch is not allowed.".into()); + return GitResult::Block( + "WHAT: Deleting main/master branch is not allowed. \ + FIX: Do not delete protected branches. \ + REF: CLAUDE.md#supply-chain-policy".into(), + ); } if RE_DELETE_REFSPEC.is_match(cmd) { return GitResult::Block( - "BLOCKED: Deleting main/master branch via empty refspec is not allowed.".into(), + "WHAT: Deleting main/master branch via empty refspec is not allowed. \ + FIX: Do not delete protected branches. \ + REF: CLAUDE.md#supply-chain-policy".into(), ); } // FR-GS-5: --no-verify if RE_NO_VERIFY.is_match(cmd) { return GitResult::Block( - "BLOCKED: git push --no-verify bypasses pre-push hooks. Fix the hook failures instead." - .into(), + "WHAT: git push --no-verify bypasses pre-push hooks. \ + FIX: Fix the hook failures instead of skipping them. \ + REF: CLAUDE.md#lint-suppression-policy".into(), ); } // FR-GS-6: --follow-tags if RE_FOLLOW_TAGS.is_match(cmd) { return GitResult::Block( - "BLOCKED: git push --follow-tags pushes ALL matching local tags. Push tags explicitly: git push origin ".into(), + "WHAT: git push --follow-tags pushes ALL matching local tags. \ + FIX: Push tags explicitly: `git push origin `. \ + REF: CLAUDE.md#releases".into(), ); } // FR-GS-7: Delete semver tags (local and remote) if RE_DELETE_SEMVER_TAG.is_match(cmd) { return GitResult::Block( - "BLOCKED: Deleting semantic version tags is not allowed. Release a new patch version instead.".into(), + "WHAT: Deleting semantic version tags is not allowed. \ + FIX: Release a new patch version instead. \ + REF: CLAUDE.md#releases".into(), ); } if RE_DELETE_REMOTE_TAG.is_match(cmd) { return GitResult::Block( - "BLOCKED: Deleting remote semantic version tags is not allowed. Release a new patch version instead.".into(), + "WHAT: Deleting remote semantic version tags is not allowed. \ + FIX: Release a new patch version instead. \ + REF: CLAUDE.md#releases".into(), ); } // FR-GS-8: Hard reset to origin/main|master if RE_HARD_RESET.is_match(cmd) { return GitResult::Block( - "BLOCKED: git reset --hard origin/main|master discards all local work. Use: git stash or git reset --soft".into(), + "WHAT: git reset --hard origin/main discards all local work. \ + FIX: Use `git stash` or `git reset --soft` instead. \ + REF: CLAUDE.md#key-design-decisions".into(), ); } @@ -204,9 +226,9 @@ pub fn check_worktree_enforcement( return Some(crate::worktree_missing_msg(&repo)); } return Some(format!( - "BLOCKED: Git op on main checkout ({repo}). \ - Use worktree: {ws_str}/{repo}/.worktrees/{short_id}. \ - Tip: run git -C fetch origin before creating new branches" + "WHAT: Git operation targets main checkout ({repo}), not the session worktree. \ + FIX: Use `git -C {ws_str}/{repo}/.worktrees/{short_id}/` instead. \ + REF: docs/architecture.md#key-invariants" )); } } @@ -228,9 +250,9 @@ pub fn check_worktree_enforcement( return Some(crate::worktree_missing_msg(&repo)); } return Some(format!( - "BLOCKED: Git op on main checkout ({repo}). \ - Use worktree: {ws_str}/{repo}/.worktrees/{short_id}. \ - Tip: run git -C fetch origin before creating new branches" + "WHAT: Git operation targets main checkout ({repo}), not the session worktree. \ + FIX: Use `git -C {ws_str}/{repo}/.worktrees/{short_id}/` instead. \ + REF: docs/architecture.md#key-invariants" )); } } @@ -241,8 +263,9 @@ pub fn check_worktree_enforcement( if !RE_GIT_C.is_match(cmd) && !RE_CD_PATH.is_match(cmd) && RE_GIT_CHECKOUT_SWITCH.is_match(cmd) { return Some(format!( - "BLOCKED: Bare git checkout/switch — worktrees are active. Use: git -C /.worktrees/{}/ checkout ...", - short_id + "WHAT: Bare git checkout/switch — worktrees are active. \ + FIX: Use `git -C /.worktrees/{short_id}/ checkout ...` instead. \ + REF: docs/architecture.md#key-invariants" )); } diff --git a/hooks/src/lib.rs b/hooks/src/lib.rs index 8bc6a04..ca5d4bb 100644 --- a/hooks/src/lib.rs +++ b/hooks/src/lib.rs @@ -21,11 +21,15 @@ pub mod session; pub mod worktree; /// Format a WORKTREE_MISSING denial message for lazy worktree creation. +/// +/// Uses the WHAT/FIX/REF remediation format so the agent can self-repair. pub fn worktree_missing_msg(repo: &str) -> String { let bin = config::bin_dir().join("ensure-worktree"); format!( - "WORKTREE_MISSING:{repo} \ - — Run: {} {repo}", + "WORKTREE_MISSING:{repo} — \ + WHAT: No worktree exists for repo '{repo}' in this session. \ + FIX: Run `{} {repo}` to create one, then retry the write. \ + REF: docs/architecture.md#key-invariants", bin.display() ) } diff --git a/hooks/src/sandbox.rs b/hooks/src/sandbox.rs index bcac28a..777040e 100644 --- a/hooks/src/sandbox.rs +++ b/hooks/src/sandbox.rs @@ -163,9 +163,14 @@ pub fn check_path_with_context( return PathDecision::Deny(crate::worktree_missing_msg(&repo)); } let rel = extract_rel_path(&resolved, &repo, ws_str); - return PathDecision::Deny(format!( - "REDIRECT: Use worktree path instead: {}/{}/.worktrees/{}/{}", + let wt_path = format!( + "{}/{}/.worktrees/{}/{}", ws_str, repo, sess.short_id, rel + ); + return PathDecision::Deny(format!( + "WHAT: Write targets main checkout, not the session worktree. \ + FIX: Use the worktree path instead: {wt_path}. \ + REF: docs/architecture.md#key-invariants" )); } } else { @@ -190,7 +195,9 @@ pub fn check_path_with_context( return PathDecision::Deny(crate::worktree_missing_msg(&repo)); } return PathDecision::Deny( - "BLOCKED: No worktree for this session. All repo writes are blocked to prevent editing the main checkout.".into(), + "WHAT: No worktree exists and repo name could not be extracted. \ + FIX: Ensure the write path is inside a known repo directory. \ + REF: docs/architecture.md#key-invariants".into(), ); } From 2d36ec9a07d2ba195236a38aac147fb64c5582aa Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 18:26:59 -0700 Subject: [PATCH 03/10] fix: reconcile CLAUDE.md test count with cargo test output (246) Greptile correctly identified the count (231) didn't match the actual cargo test output (246). Missing: claude_md (5), memory integration (4), separate proptest binary (10). Correct: 188 hooks unit + 5 claude_md + 17 integration + 10 proptest + 22 memory unit + 4 memory integration = 246. --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index c16a5e7..9b806bb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -177,7 +177,7 @@ All shell scripts follow the [Google Shell Style Guide](https://google.github.io ## Testing -231 tests (178 unit + 4 doc + 17 integration + 10 proptest + 22 memory) plus 4 fuzz targets. +246 tests (188 hooks unit + 5 claude_md + 17 integration + 10 proptest + 22 memory unit + 4 memory integration) plus 4 fuzz targets. Run with `make test` or `cargo test`. Test patterns: From 94f3052549522f0090e5f2a49f69c75f6d858a23 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:10:47 -0700 Subject: [PATCH 04/10] fix: apply rustfmt to gitcheck remediation messages --- hooks/src/gitcheck.rs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/hooks/src/gitcheck.rs b/hooks/src/gitcheck.rs index 25cb2df..cb1b5cb 100644 --- a/hooks/src/gitcheck.rs +++ b/hooks/src/gitcheck.rs @@ -84,7 +84,8 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: Force push without --force-with-lease. \ FIX: Use `git push --force-with-lease origin ` instead. \ - REF: CLAUDE.md#supply-chain-policy".into(), + REF: CLAUDE.md#supply-chain-policy" + .into(), ); } @@ -93,7 +94,8 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: Direct push to main/master. \ FIX: Create a feature branch and open a PR instead. \ - REF: CLAUDE.md#commit-convention".into(), + REF: CLAUDE.md#commit-convention" + .into(), ); } @@ -102,7 +104,8 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: Push to main/master via refspec. \ FIX: Create a feature branch and open a PR instead. \ - REF: CLAUDE.md#commit-convention".into(), + REF: CLAUDE.md#commit-convention" + .into(), ); } @@ -111,14 +114,16 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: Deleting main/master branch is not allowed. \ FIX: Do not delete protected branches. \ - REF: CLAUDE.md#supply-chain-policy".into(), + REF: CLAUDE.md#supply-chain-policy" + .into(), ); } if RE_DELETE_REFSPEC.is_match(cmd) { return GitResult::Block( "WHAT: Deleting main/master branch via empty refspec is not allowed. \ FIX: Do not delete protected branches. \ - REF: CLAUDE.md#supply-chain-policy".into(), + REF: CLAUDE.md#supply-chain-policy" + .into(), ); } @@ -127,7 +132,8 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: git push --no-verify bypasses pre-push hooks. \ FIX: Fix the hook failures instead of skipping them. \ - REF: CLAUDE.md#lint-suppression-policy".into(), + REF: CLAUDE.md#lint-suppression-policy" + .into(), ); } @@ -136,7 +142,8 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: git push --follow-tags pushes ALL matching local tags. \ FIX: Push tags explicitly: `git push origin `. \ - REF: CLAUDE.md#releases".into(), + REF: CLAUDE.md#releases" + .into(), ); } @@ -145,14 +152,16 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: Deleting semantic version tags is not allowed. \ FIX: Release a new patch version instead. \ - REF: CLAUDE.md#releases".into(), + REF: CLAUDE.md#releases" + .into(), ); } if RE_DELETE_REMOTE_TAG.is_match(cmd) { return GitResult::Block( "WHAT: Deleting remote semantic version tags is not allowed. \ FIX: Release a new patch version instead. \ - REF: CLAUDE.md#releases".into(), + REF: CLAUDE.md#releases" + .into(), ); } @@ -161,7 +170,8 @@ pub fn check_git_safety(cmd: &str) -> GitResult { return GitResult::Block( "WHAT: git reset --hard origin/main discards all local work. \ FIX: Use `git stash` or `git reset --soft` instead. \ - REF: CLAUDE.md#key-design-decisions".into(), + REF: CLAUDE.md#key-design-decisions" + .into(), ); } From 675b90807d695390c4c52d8710d6361d6b711b87 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:42:01 -0700 Subject: [PATCH 05/10] fix: apply rustfmt to sandbox remediation messages --- hooks/src/sandbox.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hooks/src/sandbox.rs b/hooks/src/sandbox.rs index 777040e..bdabc01 100644 --- a/hooks/src/sandbox.rs +++ b/hooks/src/sandbox.rs @@ -163,10 +163,8 @@ pub fn check_path_with_context( return PathDecision::Deny(crate::worktree_missing_msg(&repo)); } let rel = extract_rel_path(&resolved, &repo, ws_str); - let wt_path = format!( - "{}/{}/.worktrees/{}/{}", - ws_str, repo, sess.short_id, rel - ); + let wt_path = + format!("{}/{}/.worktrees/{}/{}", ws_str, repo, sess.short_id, rel); return PathDecision::Deny(format!( "WHAT: Write targets main checkout, not the session worktree. \ FIX: Use the worktree path instead: {wt_path}. \ @@ -197,7 +195,8 @@ pub fn check_path_with_context( return PathDecision::Deny( "WHAT: No worktree exists and repo name could not be extracted. \ FIX: Ensure the write path is inside a known repo directory. \ - REF: docs/architecture.md#key-invariants".into(), + REF: docs/architecture.md#key-invariants" + .into(), ); } From f3201e41177204711419bfb8290bc1d6ffc08481 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:51:27 -0700 Subject: [PATCH 06/10] fix(gitcheck): include origin/master in FR-GS-8 remediation message --- hooks/src/gitcheck.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/src/gitcheck.rs b/hooks/src/gitcheck.rs index cb1b5cb..531215a 100644 --- a/hooks/src/gitcheck.rs +++ b/hooks/src/gitcheck.rs @@ -168,7 +168,7 @@ pub fn check_git_safety(cmd: &str) -> GitResult { // FR-GS-8: Hard reset to origin/main|master if RE_HARD_RESET.is_match(cmd) { return GitResult::Block( - "WHAT: git reset --hard origin/main discards all local work. \ + "WHAT: git reset --hard origin/main|master discards all local work. \ FIX: Use `git stash` or `git reset --soft` instead. \ REF: CLAUDE.md#key-design-decisions" .into(), From 90a258dc100e4cfa557a77ca5b3396dcac266f76 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 21:51:00 -0700 Subject: [PATCH 07/10] chore: add CodeRabbit config with 5 harness-derived custom checks --- .coderabbit.yaml | 119 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 .coderabbit.yaml diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000..fdb9635 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,119 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +early_access: true +language: en-US +reviews: + request_changes_workflow: true + high_level_summary: false + high_level_summary_in_walkthrough: true + fail_commit_status: true + auto_apply_labels: true + auto_assign_reviewers: true + poem: false + slop_detection: + label: slop + auto_review: + enabled: true + pre_merge_checks: + docstrings: + mode: error + title: + mode: error + requirements: >- + Must follow conventional commit. The type should match the intent, + the description and optional scope should be concise, lowercase and + in the imperative mood. Ideally total length should be within 72 + characters. Breaking changes should be marked with a ! (bang) after + the type + description: + mode: error + issue_assessment: + mode: error + custom_checks: + - name: CLAUDE.md stays in sync + mode: error + instructions: >- + When source files under hooks/ or memory/ change (Cargo.toml, + src/bin/*, src/lib.rs, src/*.rs), verify CLAUDE.md is updated if + any of these are now stale: + Binary count matches [[bin]] entries across all Cargo.toml files. + Architecture tree lists exactly the files that exist under hooks/src/. + Dependency count matches [dependencies] entries in hooks/Cargo.toml. + All dependency crate names appear as backtick-quoted words in the + Dependencies section. + Test count in the Testing section reflects actual test suite sizes. + Flag as "must fix" if any claim is provably wrong. Ignore if the + PR doesn't touch structural files. + + - name: Panic = deny in all binaries + mode: error + instructions: >- + Every file under hooks/src/bin/*.rs must wrap its main logic in + std::panic::catch_unwind and deny (exit non-zero or return a deny + JSON response) on panic. The sole exception is persona_inject.rs, + which may fail open (persona injection is not a security gate). If + a new binary is added or an existing one is modified, verify + catch_unwind is present and the panic path produces a deny outcome. + Flag if the panic path silently allows the operation. This is a + security invariant: hooks must never fail open unless explicitly + exempted. + + - name: "H-4 purity: permissions never writes" + mode: error + instructions: >- + hooks/src/bin/permissions.rs (the PreToolUse hook) must NEVER + perform file I/O writes. It must use resolve_readonly() from + session.rs, never resolve_readwrite(). Check that no + std::fs::write, std::fs::File::create, std::fs::OpenOptions with + write/append, or any other write syscall is introduced. If the PR + adds write operations to permissions.rs, flag as "must fix". This + is a structural security boundary. + + - name: Architecture boundaries enforced + mode: error + instructions: >- + Read docs/architecture.md from the repository root. Verify these + constraints against the PR diff: + Layer direction: infrastructure modules (config.rs, output.rs, + changelog.rs, log.rs, mcp.rs) must NOT import from core modules + (sandbox, gitcheck, session, worktree). Check use/mod statements. + Crate independence: memory/ must not depend on hooks/ and vice + versa. Check Cargo.toml changes. + Forbidden deps: no async runtimes (tokio, async-std, smol), no + network clients (reqwest, hyper, ureq), no proc-macro crates + beyond serde_derive. Check Cargo.toml changes. + Denial message format: all Block() and Deny() messages in + gitcheck.rs, sandbox.rs, and lib.rs must contain WHAT:, FIX:, and + REF: tokens. Exception: the REDIRECT message for config paths in + sandbox.rs (containing "config path must persist") is exempt. + Module placement: new source files must be placed in the correct + layer (infra vs core vs binary) per the architecture doc. + Only flag issues provably violated by the PR diff. + + - name: Harness engineering principles + mode: error + instructions: >- + Read .agents/research/harness.md and GOALS.md from the repository + root for context. Evaluate the PR against these principles: + Error messages are instructions: any new error, log, or denial + message visible to the agent should be actionable with WHAT went + wrong and HOW to fix it. Cryptic messages like "operation failed" + without remediation are a flag. (harness Pillar 4a) + No silent pass-through: if a check, gate, or hook encounters an + error, it must fail closed (deny/block), not silently allow. This + applies to catch_unwind paths, file I/O errors, and regex + compilation. (harness Pillar 3d) + Change risk: flag if the PR modifies security-critical files + (sandbox.rs, gitcheck.rs, session.rs, worktree/) without + proportional test coverage. These are high-risk paths. (harness + Pillar 3a, GOALS directive 23) + Test quality: the PR should not remove tests without replacing + them. Test code must use fictional repo names (acme-api, web-app), + never real company names. Property-based tests should cover + sandbox and gitcheck changes. (GOALS directives 4, 10) + Only flag concrete violations visible in the diff. + +issue_enrichment: + auto_enrich: + enabled: true + labeling: + auto_apply_labels: true From 3038a7c2eace10f6e99cdb2f8368686c595e94b1 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 22:00:47 -0700 Subject: [PATCH 08/10] chore: add Greptile review instructions referencing GOALS and architecture --- .greptile.yaml | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 .greptile.yaml diff --git a/.greptile.yaml b/.greptile.yaml new file mode 100644 index 0000000..0673094 --- /dev/null +++ b/.greptile.yaml @@ -0,0 +1,62 @@ +review_instructions: | + You are reviewing a Rust workspace that provides session isolation hooks + and persistent memory for AI coding agents. The project enforces its own + quality bar through fitness goals and harness engineering principles. + + ## Source of truth + + Read these files before reviewing: + - `GOALS.md` — fitness goals, directives (steered increase/complete), and + gates with check commands and weights + - `docs/architecture.md` — layer diagram, module map, forbidden + dependencies, key invariants + - `CLAUDE.md` — build/test/lint commands, commit convention, design + decisions, testing patterns + + ## What to check + + ### Gates (from GOALS.md ## Gates table) + + Every PR must not regress any gate. The gates table in GOALS.md lists + check commands and weights. Higher-weight gates are more critical: + - weight 8: cargo-build, cargo-test + - weight 5: integration, ci-green, branch-protect + - weight 3: cargo-clippy, cargo-fmt, rustdoc, claude-md-valid, + shellcheck, actionlint, zizmor + + Flag if a PR would provably break a gate. + + ### Directives (from GOALS.md ## Directives) + + Each directive has a steer value (increase, complete, or unmarked). + For directives steered "increase", the PR should not regress them. + Key directives to watch: + - Directive 2: sandbox hardening — no weakened path checks + - Directive 10: test coverage floor — no net test removal + - Directive 11: conventional commits — PR title must match + - Directive 13: CLAUDE.md sync — structural changes need doc updates + - Directive 17: SHA-pinned actions — no rolling tags in workflows + - Directive 22: WHAT/FIX/REF denial messages — new Block()/Deny() + strings must include all three tokens + + ### Architecture (from docs/architecture.md) + + - Layer direction: infra must not import core, core must not import bins + - Crate independence: hooks and memory are independent + - Forbidden deps: no async, no network, no new proc macros + - Key invariants: panic=deny, H-4 purity, lazy worktrees + + ### Harness principles + + - Error messages must be actionable (WHAT/FIX/REF or equivalent) + - Hooks must fail closed, never fail open (except persona_inject) + - High-risk files (sandbox.rs, gitcheck.rs, session.rs, worktree/) + need proportional test coverage + - No lint suppressions without justification in the PR description + + ## What NOT to flag + + - Style preferences already handled by rustfmt and clippy + - Missing comments on private internals (only public API needs docs) + - Hypothetical issues not visible in the diff + - Low-risk changes (docs-only, comments, formatting) missing tests From ed9e3a491fb59a65d963c37fd9b3562978ff576b Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 22:01:48 -0700 Subject: [PATCH 09/10] chore: add Greptile review config with GOALS-driven rules --- .greptile.yaml | 62 ---------------------------- greptile.json | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 62 deletions(-) delete mode 100644 .greptile.yaml create mode 100644 greptile.json diff --git a/.greptile.yaml b/.greptile.yaml deleted file mode 100644 index 0673094..0000000 --- a/.greptile.yaml +++ /dev/null @@ -1,62 +0,0 @@ -review_instructions: | - You are reviewing a Rust workspace that provides session isolation hooks - and persistent memory for AI coding agents. The project enforces its own - quality bar through fitness goals and harness engineering principles. - - ## Source of truth - - Read these files before reviewing: - - `GOALS.md` — fitness goals, directives (steered increase/complete), and - gates with check commands and weights - - `docs/architecture.md` — layer diagram, module map, forbidden - dependencies, key invariants - - `CLAUDE.md` — build/test/lint commands, commit convention, design - decisions, testing patterns - - ## What to check - - ### Gates (from GOALS.md ## Gates table) - - Every PR must not regress any gate. The gates table in GOALS.md lists - check commands and weights. Higher-weight gates are more critical: - - weight 8: cargo-build, cargo-test - - weight 5: integration, ci-green, branch-protect - - weight 3: cargo-clippy, cargo-fmt, rustdoc, claude-md-valid, - shellcheck, actionlint, zizmor - - Flag if a PR would provably break a gate. - - ### Directives (from GOALS.md ## Directives) - - Each directive has a steer value (increase, complete, or unmarked). - For directives steered "increase", the PR should not regress them. - Key directives to watch: - - Directive 2: sandbox hardening — no weakened path checks - - Directive 10: test coverage floor — no net test removal - - Directive 11: conventional commits — PR title must match - - Directive 13: CLAUDE.md sync — structural changes need doc updates - - Directive 17: SHA-pinned actions — no rolling tags in workflows - - Directive 22: WHAT/FIX/REF denial messages — new Block()/Deny() - strings must include all three tokens - - ### Architecture (from docs/architecture.md) - - - Layer direction: infra must not import core, core must not import bins - - Crate independence: hooks and memory are independent - - Forbidden deps: no async, no network, no new proc macros - - Key invariants: panic=deny, H-4 purity, lazy worktrees - - ### Harness principles - - - Error messages must be actionable (WHAT/FIX/REF or equivalent) - - Hooks must fail closed, never fail open (except persona_inject) - - High-risk files (sandbox.rs, gitcheck.rs, session.rs, worktree/) - need proportional test coverage - - No lint suppressions without justification in the PR description - - ## What NOT to flag - - - Style preferences already handled by rustfmt and clippy - - Missing comments on private internals (only public API needs docs) - - Hypothetical issues not visible in the diff - - Low-risk changes (docs-only, comments, formatting) missing tests diff --git a/greptile.json b/greptile.json new file mode 100644 index 0000000..70c1e5b --- /dev/null +++ b/greptile.json @@ -0,0 +1,108 @@ +{ + "labels": [], + "comment": "", + "enabled": true, + "fixWithAI": true, + "hideFooter": false, + "strictness": 2, + "statusCheck": true, + "commentTypes": [ + "logic", + "syntax", + "style" + ], + "instructions": "", + "disabledLabels": [], + "excludeAuthors": [ + "dependabot[bot]", + "renovate[bot]" + ], + "ignoreKeywords": "", + "ignorePatterns": "greptile.json\n", + "includeAuthors": [], + "summarySection": { + "included": true, + "collapsible": true, + "defaultOpen": false + }, + "excludeBranches": [], + "fileChangeLimit": 100, + "includeBranches": [], + "includeKeywords": "", + "triggerOnDrafts": false, + "triggerOnUpdates": false, + "updateSummaryOnly": false, + "issuesTableSection": { + "included": true, + "collapsible": true, + "defaultOpen": false + }, + "statusCommentsEnabled": true, + "confidenceScoreSection": { + "included": true, + "collapsible": false, + "defaultOpen": false + }, + "sequenceDiagramSection": { + "included": true, + "collapsible": true, + "defaultOpen": false + }, + "shouldUpdateDescription": false, + "commentsOutsideDiffSection": { + "included": true, + "collapsible": true, + "defaultOpen": true + }, + "customContext": { + "other": [ + { + "scope": [], + "content": "This is a Rust workspace providing session isolation hooks and persistent memory for AI coding agents. Read GOALS.md for fitness directives and gates. Read docs/architecture.md for layer diagram, forbidden dependencies, and key invariants. Read CLAUDE.md for build commands, commit convention, and design decisions." + } + ], + "rules": [ + { + "scope": [], + "rule": "Every hooks/src/bin/*.rs must use catch_unwind and deny on panic (fail-closed). Exception: persona_inject.rs may fail open. Flag any binary missing catch_unwind or silently passing on panic." + }, + { + "scope": [], + "rule": "hooks/src/bin/permissions.rs (PreToolUse hook) must NEVER write files. It must use resolve_readonly(), never resolve_readwrite(). No std::fs::write, File::create, or OpenOptions with write. Flag any write I/O added to permissions.rs." + }, + { + "scope": [], + "rule": "All Block() and Deny() denial messages in gitcheck.rs, sandbox.rs, and lib.rs must contain WHAT:, FIX:, and REF: tokens. Exception: the REDIRECT message for config paths in sandbox.rs (containing 'config path must persist') is exempt. Flag new denial messages missing any of the three fields." + }, + { + "scope": [], + "rule": "Infrastructure modules (config.rs, output.rs, changelog.rs, log.rs, mcp.rs) must NOT import from core modules (sandbox, gitcheck, session, worktree). memory/ must not depend on hooks/. No async runtimes, network clients, or proc-macro crates beyond serde_derive in Cargo.toml." + }, + { + "scope": [], + "rule": "When hooks/src/ structure, Cargo.toml binaries, or dependencies change, CLAUDE.md must be updated to match: binary count, architecture tree, dependency count and names, test count. PR title must follow conventional commits: type(scope): description." + }, + { + "scope": [], + "rule": "Changes to security-critical files (sandbox.rs, gitcheck.rs, session.rs, worktree/) are high-risk and need proportional test coverage. No net test removal. Test code must use fictional repo names (acme-api, web-app), never real company names. No lint suppressions (#[allow(...)], // nolint) without justification in the PR description." + } + ], + "files": [ + { + "scope": [], + "path": "GOALS.md", + "description": "Fitness goals, directives (with steer values), and gates (with check commands and weights). Higher-weight gates are more critical. Directives steered 'increase' must not regress." + }, + { + "scope": [], + "path": "docs/architecture.md", + "description": "Layer diagram (binaries → core → infra), module map, forbidden dependency directions, cross-cutting concerns, and key invariants (panic=deny, H-4 purity, lazy worktrees)." + }, + { + "scope": [], + "path": "CLAUDE.md", + "description": "Build/test/lint commands, commit convention, design decisions, testing patterns, lint suppression policy, shell style guide. Must stay in sync with codebase (enforced by tests/claude_md.rs)." + } + ] + } +} From 4af11a47446abd83b581a6cd99317eea3a22f304 Mon Sep 17 00:00:00 2001 From: Frits <4488681+frits-v@users.noreply.github.com> Date: Wed, 25 Mar 2026 22:14:36 -0700 Subject: [PATCH 10/10] fix(sandbox): migrate FR-PS-1 system path denials to WHAT/FIX/REF format --- hooks/src/sandbox.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hooks/src/sandbox.rs b/hooks/src/sandbox.rs index bdabc01..0a85361 100644 --- a/hooks/src/sandbox.rs +++ b/hooks/src/sandbox.rs @@ -63,8 +63,9 @@ pub fn check_path_with_context( // FR-PS-1: System paths — check BEFORE resolving symlinks if is_system_path(&path_str) { return PathDecision::Deny(format!( - "BLOCKED: Cannot write to system path: {}", - raw_path + "WHAT: Write to system path {raw_path} is not allowed. \ + FIX: Write to a project or temp directory instead. \ + REF: docs/architecture.md#forbidden-dependencies" )); } @@ -74,8 +75,9 @@ pub fn check_path_with_context( // System paths check on resolved path (catches /private/etc, /private/var on macOS) if is_system_path(&resolved) || is_private_system_path(&resolved) { return PathDecision::Deny(format!( - "BLOCKED: Cannot write to system path: {}", - raw_path + "WHAT: Write to system path {raw_path} is not allowed (resolves to {resolved}). \ + FIX: Write to a project or temp directory instead. \ + REF: docs/architecture.md#forbidden-dependencies" )); }