ldraney/claude-custom

Fork 0

feat: cross-repo isolation hook and dev agent profile update #215

Merged

forgejo_admin merged 1 commit from 205-cross-repo-isolation into main

2026-03-28 18:48:25 +00:00

forgejo_admin commented

2026-03-28 18:43:48 +00:00

Contributor

Summary

Prevents parallel Dev agents from clobbering shared checkouts when working on repos other than the spawning repo. Adds a PreToolUse hook that detects cd ~/repo && git checkout patterns on non-spawning repos and blocks with guidance to clone to /tmp/{repo}-{branch} instead. Incident 2026-03-26: three parallel agents all ran cd ~/westside-playground && git checkout on the same shared directory, destroying each other's work.

Changes

hooks/cross-repo-isolation.sh (NEW) -- PreToolUse Bash hook that pattern-matches cd ~/repo && git checkout/switch commands, identifies the spawning repo from CWD (including worktree paths), and blocks cross-repo checkouts with a deny + safe alternative message. Lightweight string matching only, no filesystem checks.
agents/dev.md -- Added "Cross-Repo Isolation" section with safe/unsafe patterns and rules. Added constraint line: "Never cd ~/repo && git checkout on a non-spawning repo."
hooks/cleanup-worktrees.sh -- Added /tmp/ cross-repo clone cleanup section that scans for /tmp/{repo-name}-* directories with .git older than 7 days and removes them.
settings.json -- Registered cross-repo-isolation.sh in the Bash PreToolUse matcher.

Test Plan

echo '{"tool_input":{"command":"cd ~/westside-playground && git checkout -b my-feature"},"cwd":"/home/ldraney/pal-e-platform/.claude/worktrees/agent-abc123"}' | bash hooks/cross-repo-isolation.sh -- fires BLOCKED with safe alternative
Same with $HOME/ and /home/ldraney/ path variants -- all blocked
Same with git switch -- blocked
Spawning repo (cd ~/pal-e-platform from pal-e-platform CWD) -- silent exit 0, allowed
/tmp/ clone path (cd /tmp/westside-app-branch) -- silent exit 0, allowed
Non-git commands (ls ~/repo) -- silent exit 0, allowed
Empty command -- silent exit 0
settings.json validates as valid JSON

Review Checklist

Hook is lightweight (string pattern matching only, no filesystem checks)
Hook follows existing PreToolUse conventions (jq output, deny/allow pattern)
Hook registered in settings.json under Bash matcher
agents/dev.md updated with cross-repo isolation section and constraint
cleanup-worktrees.sh handles /tmp/ clone cleanup
No overlap with #184 scope (freshness/cleanup gaps)

No pal-e-docs notes modified (Dev agent boundary)

Closes #205

Parent: forgejo_admin/pal-e-platform#188
Adjacent (no overlap): claude-custom#184

## Summary Prevents parallel Dev agents from clobbering shared checkouts when working on repos other than the spawning repo. Adds a PreToolUse hook that detects `cd ~/repo && git checkout` patterns on non-spawning repos and blocks with guidance to clone to `/tmp/{repo}-{branch}` instead. Incident 2026-03-26: three parallel agents all ran `cd ~/westside-playground && git checkout` on the same shared directory, destroying each other's work. ## Changes - **`hooks/cross-repo-isolation.sh`** (NEW) -- PreToolUse Bash hook that pattern-matches `cd ~/repo && git checkout/switch` commands, identifies the spawning repo from CWD (including worktree paths), and blocks cross-repo checkouts with a deny + safe alternative message. Lightweight string matching only, no filesystem checks. - **`agents/dev.md`** -- Added "Cross-Repo Isolation" section with safe/unsafe patterns and rules. Added constraint line: "Never `cd ~/repo && git checkout` on a non-spawning repo." - **`hooks/cleanup-worktrees.sh`** -- Added `/tmp/` cross-repo clone cleanup section that scans for `/tmp/{repo-name}-*` directories with `.git` older than 7 days and removes them. - **`settings.json`** -- Registered `cross-repo-isolation.sh` in the Bash PreToolUse matcher. ## Test Plan - `echo '{"tool_input":{"command":"cd ~/westside-playground && git checkout -b my-feature"},"cwd":"/home/ldraney/pal-e-platform/.claude/worktrees/agent-abc123"}' | bash hooks/cross-repo-isolation.sh` -- fires BLOCKED with safe alternative - Same with `$HOME/` and `/home/ldraney/` path variants -- all blocked - Same with `git switch` -- blocked - Spawning repo (`cd ~/pal-e-platform` from pal-e-platform CWD) -- silent exit 0, allowed - `/tmp/` clone path (`cd /tmp/westside-app-branch`) -- silent exit 0, allowed - Non-git commands (`ls ~/repo`) -- silent exit 0, allowed - Empty command -- silent exit 0 - `settings.json` validates as valid JSON ## Review Checklist - [x] Hook is lightweight (string pattern matching only, no filesystem checks) - [x] Hook follows existing PreToolUse conventions (jq output, deny/allow pattern) - [x] Hook registered in settings.json under Bash matcher - [x] agents/dev.md updated with cross-repo isolation section and constraint - [x] cleanup-worktrees.sh handles /tmp/ clone cleanup - [x] No overlap with #184 scope (freshness/cleanup gaps) ## Related Notes - No pal-e-docs notes modified (Dev agent boundary) ## Related Closes #205 - Parent: `forgejo_admin/pal-e-platform#188` - Adjacent (no overlap): `claude-custom#184`

forgejo_admin added 1 commit

2026-03-28 18:43:48 +00:00

feat: cross-repo isolation hook and dev agent profile update 3b4eca9cfc

Prevents parallel Dev agents from clobbering shared checkouts when
working on repos other than the spawning repo. Adds PreToolUse hook
that detects cd ~/repo && git checkout patterns and blocks with
guidance to clone to /tmp/{repo}-{branch} instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 18:44:35 +00:00

Author

Contributor

QA Review -- PR #215

Files Reviewed

hooks/cross-repo-isolation.sh (NEW, 83 lines)
agents/dev.md (24 lines added)
hooks/cleanup-worktrees.sh (35 lines added, 5 removed)
settings.json (4 lines added)

Findings

Correctness: PASS

Hook correctly pattern-matches cd ~/repo && git checkout/switch commands
Spawning repo detection handles worktree paths (~/repo/.claude/worktrees/agent-xxx)
Quick-exit guards (empty command, no cd, no git checkout) ensure sub-50ms for non-matching commands
/tmp/ clone paths are correctly excluded (don't match ~/, $HOME/, or /home/user/ patterns)
Cleanup script properly globs /tmp/{repo-name}-* with .git check to avoid false positives

SOP Compliance: PASS

Hook follows existing PreToolUse conventions (jq JSON output, deny/allow pattern, set -euo pipefail)
Hook registered in settings.json under Bash matcher
agents/dev.md updated with section and constraint
No overlap with #184 scope (confirmed: #184 covers freshness/cleanup gaps, not cross-repo isolation)
PR body follows template with all required sections

Edge Cases Considered:

git clone ~/repo /tmp/repo-branch && cd /tmp/repo-branch && git checkout -b branch -- ALLOWED (correct: /tmp/ path doesn't match home dir patterns)
CWD inside worktree (~/repo/.claude/worktrees/agent-xxx) -- spawning repo correctly resolved via sed strip
Multiple cd targets in one command -- first cross-repo match triggers deny (acceptable: commands with multiple cd targets to different repos are pathological)

Nits: None

VERDICT: APPROVED

## QA Review -- PR #215 ### Files Reviewed - `hooks/cross-repo-isolation.sh` (NEW, 83 lines) - `agents/dev.md` (24 lines added) - `hooks/cleanup-worktrees.sh` (35 lines added, 5 removed) - `settings.json` (4 lines added) ### Findings **Correctness: PASS** - Hook correctly pattern-matches `cd ~/repo && git checkout/switch` commands - Spawning repo detection handles worktree paths (`~/repo/.claude/worktrees/agent-xxx`) - Quick-exit guards (empty command, no cd, no git checkout) ensure sub-50ms for non-matching commands - `/tmp/` clone paths are correctly excluded (don't match `~/`, `$HOME/`, or `/home/user/` patterns) - Cleanup script properly globs `/tmp/{repo-name}-*` with `.git` check to avoid false positives **SOP Compliance: PASS** - Hook follows existing PreToolUse conventions (jq JSON output, deny/allow pattern, `set -euo pipefail`) - Hook registered in `settings.json` under Bash matcher - `agents/dev.md` updated with section and constraint - No overlap with #184 scope (confirmed: #184 covers freshness/cleanup gaps, not cross-repo isolation) - PR body follows template with all required sections **Edge Cases Considered:** - `git clone ~/repo /tmp/repo-branch && cd /tmp/repo-branch && git checkout -b branch` -- ALLOWED (correct: /tmp/ path doesn't match home dir patterns) - CWD inside worktree (`~/repo/.claude/worktrees/agent-xxx`) -- spawning repo correctly resolved via sed strip - Multiple cd targets in one command -- first cross-repo match triggers deny (acceptable: commands with multiple cd targets to different repos are pathological) **Nits: None** VERDICT: APPROVED

forgejo_admin commented

2026-03-28 18:47:54 +00:00

Author

Contributor

PR #215 Review

DOMAIN REVIEW

Tech stack: Bash hooks for Claude Code harness (PreToolUse pattern matching, SessionStart cleanup), agent profile markdown, JSON configuration. This is shell scripting + configuration management.

hooks/cross-repo-isolation.sh (new, 83 lines)

Correctness of pattern matching: The hook correctly uses a two-stage filter -- first a quick exit if cd and git checkout|switch are not both present, then a targeted grep for cd targets. This is sound and performant.
Path normalization: The sed chain handles ~/repo, $HOME/repo, and /home/*/repo forms. The first sed "s|^~/|$HOME/|" expands tilde, and the second "s|^\$HOME/|$HOME/|" expands literal $HOME. Both are correct in double-quoted context. The grep also captures /home/*/repo directly. All three variants from the test plan are covered.
Spawning repo detection via CWD: The hook strips .claude/worktrees/... suffix to resolve the root repo name. This correctly handles both direct repo CWD and worktree CWD. Solid.
Early exit on first match: The exit 0 inside the while loop means only the first cross-repo cd target triggers the block. If a command has multiple cd targets (unlikely but possible with chained commands), only the first is checked. This is acceptable for the pattern being caught (cd ~/repo && git checkout).
set -euo pipefail: Correct. Matches convention used by 12 other hooks in this repo.
jq output format: Follows the existing permissionDecision: "deny" convention established in block-upstream.sh, block-main-commits.sh, and others. Correct.
Potential bypass: A multi-line command or one that uses pushd instead of cd would bypass this hook. The hook header acknowledges it is "lightweight: pattern match on command string only" which is the right tradeoff for sub-50ms execution. The pushd bypass is a marginal edge case -- agents don't typically use pushd.

agents/dev.md updates

The "Cross-Repo Isolation" section is placed correctly between "Branch Naming Convention" and "Constraints". Clear safe/unsafe examples. The constraint line addition (Never cd ~/repo && git checkout on a non-spawning repo) is properly placed in the existing constraints list.
Documentation matches hook behavior accurately. The safe pattern shows git clone to /tmp/, which is exactly what the hook's deny message recommends.

hooks/cleanup-worktrees.sh changes

The new /tmp/ cleanup loop iterates over REPO_DIRS and glob-matches /tmp/{repo-name}-* directories. Uses the same MAX_AGE_DAYS threshold and stat -c %Y age check as the existing worktree cleanup. Consistent.
errors array scope: The new code appends to the existing errors array (initialized at line 42), which is correct. Error entries from both worktree cleanup and /tmp/ cleanup will appear in the same errors summary section.
Summary block rebuild: The original if [ ${#cleaned[@]} -gt 0 ]; then...else exit 0 is replaced with if [ ${#cleaned[@]} -gt 0 ] || [ ${#tmp_cleaned[@]} -gt 0 ]; then. This correctly ensures the hook reports when either type of cleanup happened.
Silent exit preserved: The else branch still does exit 0 when nothing was cleaned. Behavior preserved.

settings.json changes

The new hook is registered under the Bash PreToolUse matcher, which is correct -- it intercepts bash commands. Placed after check-ruff-before-commit.sh. JSON structure is valid (trailing comma on the preceding entry, new entry with closing bracket).
Convention compliance: Issue #166 established that matchers must use separate entries, not pipe-separated. This hook is registered as a standalone entry under the existing Bash matcher array -- correct.

BLOCKERS

None found.

No new functionality that requires automated test coverage. This is a PreToolUse hook with shell pattern matching. The test plan in the PR body describes manual echo/pipe verification for all code paths (blocked, allowed, edge cases). This is appropriate for shell hooks in this repo -- no other hooks in this codebase have automated test suites.
No unvalidated user input. The hook reads from a structured JSON input (tool_input.command) and only does string matching.
No secrets or credentials in code.
No DRY violations in auth/security paths.

NITS

cross-repo-isolation.sh line 36 -- grep character class for repo names: The regex [a-zA-Z0-9._-]+ does not include the @ character. If any repo checkout path ever contained @ (unlikely for local checkouts, but Forgejo clone URLs can), it would not be captured. This is fine for the current REPO_DIRS list but worth noting.
cleanup-worktrees.sh -- glob expansion when no matches exist: The pattern /tmp/"${repo_name}"-* will remain as a literal string if no matches exist. The next line [ -d "$tmp_clone/.git" ] || continue handles this correctly (the literal glob string is not a directory), but adding shopt -s nullglob before the loop and unsetting it after would be cleaner. Minor -- the current code works correctly.
agents/dev.md safe pattern placeholder: The safe pattern shows git clone ~/other-repo /tmp/other-repo-{branch} with {branch} as a placeholder. This is clear, but the hook's deny message also uses {branch} as a placeholder. Consistent, which is good.
Comment typo in hook header: Line 15 says "Does NOT fire for:" with two bullet points. The second bullet says "Commands that don't include git checkout/switch after a cd to ~/" -- this is slightly misleading since it also does not fire for /tmp/ paths. The comment could mention /tmp/ paths explicitly, though the code makes this clear.

SOP COMPLIANCE

Branch named after issue: 205-cross-repo-isolation matches issue #205
PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present
Related references parent: forgejo_admin/pal-e-platform#188 and adjacent claude-custom#184 noted with no-overlap clarification
No secrets committed
No unnecessary file changes -- all 4 files are directly relevant to the feature
Commit messages are descriptive (PR title follows conventional commits: feat:)

PROCESS OBSERVATIONS

Deployment frequency: This hook is purely additive -- it blocks a new class of unsafe operations without modifying any existing hook behavior. Low change failure risk.
Incident-driven: The PR body documents the specific incident (2026-03-26, three agents clobbering westside-playground). This is good post-incident engineering practice.
Scope boundary: The PR explicitly notes no overlap with #184 (worktree isolation enforcement gaps). Clean scope separation.
Cleanup integration: Rather than creating a separate cleanup mechanism, the /tmp/ clone cleanup was integrated into the existing cleanup-worktrees.sh SessionStart hook. This reduces operational complexity -- one hook handles all stale workspace cleanup.

VERDICT: APPROVED

## PR #215 Review ### DOMAIN REVIEW **Tech stack**: Bash hooks for Claude Code harness (PreToolUse pattern matching, SessionStart cleanup), agent profile markdown, JSON configuration. This is shell scripting + configuration management. **`hooks/cross-repo-isolation.sh` (new, 83 lines)** 1. **Correctness of pattern matching**: The hook correctly uses a two-stage filter -- first a quick exit if `cd` and `git checkout|switch` are not both present, then a targeted grep for cd targets. This is sound and performant. 2. **Path normalization**: The sed chain handles `~/repo`, `$HOME/repo`, and `/home/*/repo` forms. The first sed `"s|^~/|$HOME/|"` expands tilde, and the second `"s|^\$HOME/|$HOME/|"` expands literal `$HOME`. Both are correct in double-quoted context. The grep also captures `/home/*/repo` directly. All three variants from the test plan are covered. 3. **Spawning repo detection via CWD**: The hook strips `.claude/worktrees/...` suffix to resolve the root repo name. This correctly handles both direct repo CWD and worktree CWD. Solid. 4. **Early exit on first match**: The `exit 0` inside the while loop means only the first cross-repo cd target triggers the block. If a command has multiple cd targets (unlikely but possible with chained commands), only the first is checked. This is acceptable for the pattern being caught (`cd ~/repo && git checkout`). 5. **`set -euo pipefail`**: Correct. Matches convention used by 12 other hooks in this repo. 6. **jq output format**: Follows the existing `permissionDecision: "deny"` convention established in `block-upstream.sh`, `block-main-commits.sh`, and others. Correct. 7. **Potential bypass**: A multi-line command or one that uses `pushd` instead of `cd` would bypass this hook. The hook header acknowledges it is "lightweight: pattern match on command string only" which is the right tradeoff for sub-50ms execution. The `pushd` bypass is a marginal edge case -- agents don't typically use `pushd`. **`agents/dev.md` updates** 8. The "Cross-Repo Isolation" section is placed correctly between "Branch Naming Convention" and "Constraints". Clear safe/unsafe examples. The constraint line addition (`Never cd ~/repo && git checkout on a non-spawning repo`) is properly placed in the existing constraints list. 9. Documentation matches hook behavior accurately. The safe pattern shows `git clone` to `/tmp/`, which is exactly what the hook's deny message recommends. **`hooks/cleanup-worktrees.sh` changes** 10. The new `/tmp/` cleanup loop iterates over `REPO_DIRS` and glob-matches `/tmp/{repo-name}-*` directories. Uses the same `MAX_AGE_DAYS` threshold and `stat -c %Y` age check as the existing worktree cleanup. Consistent. 11. **`errors` array scope**: The new code appends to the existing `errors` array (initialized at line 42), which is correct. Error entries from both worktree cleanup and `/tmp/` cleanup will appear in the same errors summary section. 12. **Summary block rebuild**: The original `if [ ${#cleaned[@]} -gt 0 ]; then...else exit 0` is replaced with `if [ ${#cleaned[@]} -gt 0 ] || [ ${#tmp_cleaned[@]} -gt 0 ]; then`. This correctly ensures the hook reports when either type of cleanup happened. 13. **Silent exit preserved**: The `else` branch still does `exit 0` when nothing was cleaned. Behavior preserved. **`settings.json` changes** 14. The new hook is registered under the `Bash` PreToolUse matcher, which is correct -- it intercepts bash commands. Placed after `check-ruff-before-commit.sh`. JSON structure is valid (trailing comma on the preceding entry, new entry with closing bracket). 15. Convention compliance: Issue #166 established that matchers must use separate entries, not pipe-separated. This hook is registered as a standalone entry under the existing `Bash` matcher array -- correct. ### BLOCKERS **None found.** - No new functionality that requires automated test coverage. This is a PreToolUse hook with shell pattern matching. The test plan in the PR body describes manual echo/pipe verification for all code paths (blocked, allowed, edge cases). This is appropriate for shell hooks in this repo -- no other hooks in this codebase have automated test suites. - No unvalidated user input. The hook reads from a structured JSON input (tool_input.command) and only does string matching. - No secrets or credentials in code. - No DRY violations in auth/security paths. ### NITS 1. **`cross-repo-isolation.sh` line 36 -- grep character class for repo names**: The regex `[a-zA-Z0-9._-]+` does not include the `@` character. If any repo checkout path ever contained `@` (unlikely for local checkouts, but Forgejo clone URLs can), it would not be captured. This is fine for the current REPO_DIRS list but worth noting. 2. **`cleanup-worktrees.sh` -- glob expansion when no matches exist**: The pattern `/tmp/"${repo_name}"-*` will remain as a literal string if no matches exist. The next line `[ -d "$tmp_clone/.git" ] || continue` handles this correctly (the literal glob string is not a directory), but adding `shopt -s nullglob` before the loop and unsetting it after would be cleaner. Minor -- the current code works correctly. 3. **`agents/dev.md` safe pattern placeholder**: The safe pattern shows `git clone ~/other-repo /tmp/other-repo-{branch}` with `{branch}` as a placeholder. This is clear, but the hook's deny message also uses `{branch}` as a placeholder. Consistent, which is good. 4. **Comment typo in hook header**: Line 15 says "Does NOT fire for:" with two bullet points. The second bullet says "Commands that don't include git checkout/switch after a cd to ~/<repo>" -- this is slightly misleading since it also does not fire for `/tmp/` paths. The comment could mention `/tmp/` paths explicitly, though the code makes this clear. ### SOP COMPLIANCE - [x] Branch named after issue: `205-cross-repo-isolation` matches issue #205 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present - [x] Related references parent: `forgejo_admin/pal-e-platform#188` and adjacent `claude-custom#184` noted with no-overlap clarification - [x] No secrets committed - [x] No unnecessary file changes -- all 4 files are directly relevant to the feature - [x] Commit messages are descriptive (PR title follows conventional commits: `feat:`) ### PROCESS OBSERVATIONS - **Deployment frequency**: This hook is purely additive -- it blocks a new class of unsafe operations without modifying any existing hook behavior. Low change failure risk. - **Incident-driven**: The PR body documents the specific incident (2026-03-26, three agents clobbering westside-playground). This is good post-incident engineering practice. - **Scope boundary**: The PR explicitly notes no overlap with #184 (worktree isolation enforcement gaps). Clean scope separation. - **Cleanup integration**: Rather than creating a separate cleanup mechanism, the `/tmp/` clone cleanup was integrated into the existing `cleanup-worktrees.sh` SessionStart hook. This reduces operational complexity -- one hook handles all stale workspace cleanup. ### VERDICT: APPROVED