Skip to content

Commit 041d745

Browse files
go-to-kgo-to-k
authored
chore(hooks): scope integ-local-gate to PRs that touch local-execution code (#845)
1 parent 998e1a2 commit 041d745

3 files changed

Lines changed: 129 additions & 7 deletions

File tree

‎.claude/hooks/integ-local-gate.sh‎

Lines changed: 76 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,22 @@
22
# integ-local-gate.sh
33
#
44
# PreToolUse hook. Blocks `gh pr merge` (including --auto) and
5-
# `git merge` unless the `integ-local` markgate marker is fresh for
6-
# the current content state. The gate's scope (see .markgate.yml)
7-
# covers every code path that participates in the `cdkd local *`
8-
# family (Lambda RIE containers, ECS task emulation, HTTP server,
9-
# container pool, etc.); editing any of them invalidates the marker
10-
# and forces a successful Docker-based `/run-integ local-*` run
11-
# before the PR can be merged.
5+
# `git merge` when the merged PR actually touches local-execution code
6+
# AND the `integ-local` markgate marker is not fresh. The gate's scope
7+
# (see .markgate.yml) covers every code path that participates in the
8+
# `cdkd local *` family (Lambda RIE containers, ECS task emulation,
9+
# HTTP server, container pool, etc.); editing any of them invalidates
10+
# the marker and forces a successful Docker-based `/run-integ local-*`
11+
# run before the PR can be merged.
12+
#
13+
# IMPORTANT — PR-diff scope guard (see below): for `gh pr merge <N>`
14+
# the hook first checks whether the PR's file list actually touches
15+
# local-execution scope. A PR that touches NO local code passes
16+
# through even when the marker is stale, mirroring integ-destroy-gate
17+
# / integ-broad-gate (which already scope-check). Without this guard a
18+
# stale marker (14d TTL expiry, or an unrelated src/local change
19+
# already on main) would block EVERY merge, including pure
20+
# src/provisioning PRs — the over-fire this guard fixes.
1221
#
1322
# This is the structural counterpart for local-execution changes,
1423
# mirroring `integ-destroy-gate.sh` for deletion logic.
@@ -108,6 +117,66 @@ fi
108117

109118
cd "$target_dir" 2>/dev/null || exit 0
110119

120+
# --- PR-diff scope check (mirrors integ-destroy-gate.sh / integ-broad-gate.sh) ---
121+
# The markgate scope (.markgate.yml) is file-level, but markgate `verify`
122+
# cannot tell whether THIS PR's diff actually touches local-execution code.
123+
# Without this guard a stale `integ-local` marker (14d TTL expiry, or an
124+
# unrelated src/local change already on main) blocks EVERY merge — including
125+
# PRs that touch no local code at all (e.g. a pure src/provisioning fix).
126+
# The sibling gates integ-destroy / integ-broad already scope-check their
127+
# diff and pass non-matching PRs through; integ-local must do the same.
128+
#
129+
# Only applies to `gh pr merge <N>` where we can fetch the PR's file list.
130+
# `git merge` and number-less `gh pr merge` fall through to the
131+
# unconditional verify below (conservative — those are rarer and we
132+
# cannot cheaply enumerate the incoming diff).
133+
LOCAL_SCOPE_REGEX='^src/local/|^src/cli/commands/local-[A-Za-z0-9_-]*\.ts$|^tests/integration/local-'
134+
135+
if printf '%s' "$cmd" | grep -qE 'gh([[:space:]]+-C[[:space:]]+[^[:space:]]+)?[[:space:]]+pr[[:space:]]+merge'; then
136+
pr_number=""
137+
args="${cmd#*merge}"
138+
# shellcheck disable=SC2086
139+
set -- $args
140+
while [ $# -gt 0 ]; do
141+
case "$1" in
142+
--*=*) shift; continue ;;
143+
--auto|--admin|--delete-branch|--squash|--merge|--rebase) shift; continue ;;
144+
-*) shift; [ $# -gt 0 ] && shift; continue ;;
145+
*)
146+
if printf '%s' "$1" | grep -qE '^[0-9]+$'; then
147+
pr_number="$1"
148+
break
149+
fi
150+
shift
151+
;;
152+
esac
153+
done
154+
155+
if [ -n "$pr_number" ]; then
156+
# Pass-through on any gh error so an unrelated infra outage does not
157+
# block merges (mirrors integ-broad-gate.sh / pr-review-gate.sh).
158+
pr_json=$(gh pr view "$pr_number" --json files 2>/dev/null) || {
159+
printf 'integ-local-gate: gh pr view %s failed; allowing merge (infra fail-open)\n' "$pr_number" >&2
160+
exit 0
161+
}
162+
paths=$(printf '%s' "$pr_json" | jq -r '.files[].path' 2>/dev/null || echo "")
163+
touches_local=0
164+
while IFS= read -r f; do
165+
[ -z "$f" ] && continue
166+
if printf '%s' "$f" | grep -qE "$LOCAL_SCOPE_REGEX"; then
167+
touches_local=1
168+
break
169+
fi
170+
done <<EOF_FILES
171+
$paths
172+
EOF_FILES
173+
# No local-execution file in the PR diff -> this gate does not apply.
174+
if [ "$touches_local" -eq 0 ]; then
175+
exit 0
176+
fi
177+
fi
178+
fi
179+
111180
# Prefer the `.mise.toml`-pinned version via `mise exec --` so the repo's
112181
# canonical markgate wins over an older PATH binary; see check-gate.sh for
113182
# the schema-bump rationale (0.3.0 markers are silently invisible to 0.3.1).

‎.claude/hooks/integ-local-gate.test.sh‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,57 @@ run_case "gh issue body quoting 'gh pr merge' passes through" 0 \
142142
run_case "echo body quoting 'git merge' passes through" 0 \
143143
"$(printf '{"cwd":"%s","tool_input":{"command":"echo \"reminder: git merge origin/main when ready\""}}' "$fixture_repo")"
144144

145+
# --- PR-DIFF SCOPE-CHECK cases (mirrors integ-destroy / integ-broad gates) ---
146+
#
147+
# For `gh pr merge <N>` WITH a PR number, the gate fetches the PR's file
148+
# list via `gh pr view <N> --json files` and passes through when no file
149+
# touches local-execution scope (src/local/** / src/cli/commands/local-*.ts
150+
# / tests/integration/local-*). A stub `gh` on PATH returns a controlled
151+
# file list; the real markgate (stale in this checkout) drives the
152+
# fires-when-relevant case to exit 2.
153+
GH_STUB_DIR="$TMPDIR/ghstub"
154+
mkdir -p "$GH_STUB_DIR"
155+
GH_FILES_PAYLOAD="$TMPDIR/gh-files.json"
156+
cat > "$GH_STUB_DIR/gh" <<'GH_EOF'
157+
#!/usr/bin/env bash
158+
if [ "${1:-} ${2:-}" = "pr view" ]; then
159+
if [ "${GH_STUB_FAIL:-}" = "1" ]; then exit 1; fi
160+
cat "$GH_FILES_PAYLOAD"
161+
exit 0
162+
fi
163+
exit 0
164+
GH_EOF
165+
chmod +x "$GH_STUB_DIR/gh"
166+
export GH_FILES_PAYLOAD
167+
OLD_PATH="$PATH"
168+
export PATH="$GH_STUB_DIR:$PATH"
169+
170+
# 17. PR with a number whose files are all NON-local -> scope check
171+
# passes the merge through (exit 0) even though integ-local is stale.
172+
printf '{"files":[{"path":"src/provisioning/cloud-control-provider.ts"},{"path":"docs/changelog-cdkd.md"}]}' > "$GH_FILES_PAYLOAD"
173+
GH_STUB_FAIL="" run_case "gh pr merge <N> non-local files passes through" 0 \
174+
"$(printf '{"cwd":"%s","tool_input":{"command":"gh pr merge 999 --squash --delete-branch"}}' "$fixture_repo")"
175+
176+
# 18. PR with a number whose files include a local-execution file ->
177+
# scope check engages, the (stale) marker blocks the merge (exit 2).
178+
printf '{"files":[{"path":"src/local/docker-runner.ts"},{"path":"docs/changelog-cdkd.md"}]}' > "$GH_FILES_PAYLOAD"
179+
GH_STUB_FAIL="" run_case "gh pr merge <N> with src/local file gate fires" 2 \
180+
"$(printf '{"cwd":"%s","tool_input":{"command":"gh pr merge 999 --squash"}}' "$fixture_repo")"
181+
182+
# 19. PR with a number whose files include a tests/integration/local-*
183+
# fixture -> scope check engages, stale marker blocks (exit 2).
184+
printf '{"files":[{"path":"tests/integration/local-invoke/verify.sh"}]}' > "$GH_FILES_PAYLOAD"
185+
GH_STUB_FAIL="" run_case "gh pr merge <N> with tests/integration/local- file gate fires" 2 \
186+
"$(printf '{"cwd":"%s","tool_input":{"command":"gh pr merge 999"}}' "$fixture_repo")"
187+
188+
# 20. `gh pr view` failure -> fail-open (exit 0), an infra outage must
189+
# not block merges (mirrors integ-broad-gate.sh).
190+
printf '{"files":[{"path":"src/local/docker-runner.ts"}]}' > "$GH_FILES_PAYLOAD"
191+
GH_STUB_FAIL="1" run_case "gh pr view failure fails open" 0 \
192+
"$(printf '{"cwd":"%s","tool_input":{"command":"gh pr merge 999 --squash"}}' "$fixture_repo")"
193+
194+
export PATH="$OLD_PATH"
195+
145196
echo
146197
echo "Pass: $pass Fail: $fail"
147198
if [[ "$fail" -gt 0 ]]; then

‎.claude/rules/hooks.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ All eleven produce actionable error messages with the exact replacement command.
4848

4949
The seven markgate-backed gate hooks (`check-gate.sh`, `verify-pr-gate.sh`, `integ-destroy-gate.sh`, `integ-broad-gate.sh`, `integ-local-gate.sh`, `integ-schema-migration-gate.sh`, `pr-review-gate.sh`) are all **cwd-aware** post-#559. Each reads the PreToolUse payload's `cwd` field plus parses leading `cd <path>` and the last `git -C <path>` / `gh -C <path>` flag from the command, then `cd`s to that resolved target dir before invoking `markgate verify`. This restores markgate's per-worktree marker isolation — pre-#559 the hooks always landed in the main tree via `git rev-parse --git-common-dir`'s parent, forcing every parallel agent to converge on the main tree's marker store (the actual root cause of the cross-agent edit-race documented in memory rule `feedback_cross_agent_main_tree_contention.md`).
5050

51+
**PR-diff scope guards (integ-destroy / integ-broad / integ-local).** The three integ gates that fire on `gh pr merge` first check whether the merged PR's diff actually touches their scope before consulting the marker — `integ-destroy-gate` greps the changed files against its delete-logic patterns, `integ-broad-gate` against `CROSS_CUTTING_REGEX`, and `integ-local-gate` against `^src/local/|^src/cli/commands/local-*\.ts$|^tests/integration/local-` (via `gh pr view <N> --json files`). A PR that touches none of a gate's scope passes that gate through even when its marker is stale. This matters because the integ markers carry a 14d TTL and are file-scoped: without the per-gate diff guard an expired (or unrelated-change-invalidated) marker would block EVERY merge, including PRs touching none of that gate's code. `integ-local-gate` gained this guard to match its siblings (it previously verified the marker unconditionally, so a stale `integ-local` blocked even pure `src/provisioning` merges); `git merge` and number-less `gh pr merge` still fall through to the unconditional verify since the incoming diff cannot be cheaply enumerated.
52+
5153
**Convention shift (post-#559)**: `markgate set <gate>` must be run from the same worktree (cwd) where the gated command (`git commit` / `gh pr create` / `gh pr merge`) will eventually be invoked. Concurrent agents in different worktrees no longer collide because each worktree has its own markgate state dir (`<worktree>/.git/worktrees/<name>/markgate/` for side worktrees, `<main>/.git/markgate/` for the main tree). The `.markgate-pr-review-sha` sentinel is already per-worktree by construction (each worktree has its own root), so no change there.
5254

5355
**Backward compat**: a user who does everything from the main tree (the historical default) sees no behavior change — `tool_input.cwd` resolves to the main tree, the hook reads the main tree's markgate state. Agents who previously relied on the "set marker from main tree, merge from anywhere" pattern need to update their flow to set markers from the worktree they merge from; the `/review-pr` skill was updated accordingly.

0 commit comments

Comments
 (0)