From 6952692dc9552dba97fe3e5c03b965adb85e1d12 Mon Sep 17 00:00:00 2001 From: Divine Date: Fri, 10 Apr 2026 05:46:54 -0400 Subject: [PATCH] fix(strip): make strip_for_main.sh work mid-merge + align safety lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues surfaced when attempting Task 28 (develop → main strip merge) of the M-sec plan: 1. strip_for_main.sh could never actually run the documented flow. The header comment said: git merge develop --no-commit ./scripts/strip_for_main.sh git add -A && git commit But the script's dirty-tree guard (git diff --quiet HEAD) refused to run on the dirty post-merge tree, so step 2 always aborted. Fix: detect mid-merge state via $GIT_DIR/MERGE_HEAD presence. When IN_MERGE=1, skip the dirty-tree guard (the dirty tree is expected) and have strip_path use 'git rm -rf --ignore-unmatch' so modify/delete conflicts are resolved (deleted AND staged) in a single step. Outside a merge, the guard still fires — stripping on top of unrelated edits would mix commits. Critical invariant preserved: the 'refuses to run on develop' guard still fires no matter what mode or merge state. develop is the source of dev artifacts; the script must never touch it. 2. Two defense layers had drifted: - strip_for_main.sh had 'adr', .githooks/pre-commit didn't - neither had '.vscode' (editor settings, Snyk IDE prefs etc.) Both lists now agree: 12 strip paths in each. Added an explicit comment in pre-commit telling future editors to keep both in sync. Verified locally: - go build ./cmd/broker ./cmd/aactl: OK - go test -short ./...: 15/15 packages PASS - golangci-lint run ./...: clean - ./scripts/strip_for_main.sh --dry-run (from develop): 12 paths listed including new .vscode entry --- .githooks/pre-commit | 9 +++- CHANGELOG.md | 16 ++++++ scripts/strip_for_main.sh | 108 ++++++++++++++++++++++++++++++++------ 3 files changed, 115 insertions(+), 18 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index ca8602c..3dd723b 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -20,7 +20,12 @@ if [[ "$BRANCH" != "main" ]]; then exit 0 fi -# Files/dirs that must NEVER land on main +# Files/dirs that must NEVER land on main. +# Keep this list in sync with the strip_path calls in scripts/strip_for_main.sh. +# The two lists exist for defense in depth: +# - strip_for_main.sh is the automated pre-merge step (runs once per merge) +# - this hook is the last-chance safety net (runs on every commit to main) +# If either catches a stray dev file, the other should too. FORBIDDEN=( "MEMORY.md" "MEMORY_ARCHIVE.md" @@ -35,6 +40,8 @@ FORBIDDEN=( ".claude" ".agents" "audit" + "adr" + ".vscode" "generate_pdf.py" ) diff --git a/CHANGELOG.md b/CHANGELOG.md index aca0b6e..f37c32d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed — strip_for_main.sh mid-merge support + two drift fixes + +- **`scripts/strip_for_main.sh`** — the documented `git merge develop + --no-commit` → strip flow could never actually work because the + script's dirty-tree guard refused to run mid-merge. Added merge-state + detection (`$GIT_DIR/MERGE_HEAD` presence); when mid-merge the strip + uses `git rm -rf --ignore-unmatch` so modify/delete conflicts get + deleted AND staged as resolved in one step. The "absolute refusal to + run on develop" guard is preserved regardless of merge state. +- **`scripts/strip_for_main.sh` + `.githooks/pre-commit`** — added + `.vscode/` (editor settings, often carry per-user Snyk / IDE prefs) + to both strip lists, and `adr/` to the pre-commit FORBIDDEN list + (it was already in the strip script). The two defense layers now + agree. A note in pre-commit tells future editors to keep both lists + in sync. + ### Added — CI/build/gates (M-sec v1) - **`.gosec.yml`** — explicit gosec configuration with documented rule diff --git a/scripts/strip_for_main.sh b/scripts/strip_for_main.sh index 35d1610..83255f5 100755 --- a/scripts/strip_for_main.sh +++ b/scripts/strip_for_main.sh @@ -14,17 +14,41 @@ set -euo pipefail # # --dry-run Print what would be removed without actually removing anything # +# What this script protects: +# develop is private and messy; main is the public branch that ships +# outside the door. This script is the automated safety net that keeps +# private material off main. It never modifies develop — it only runs +# on main (or a detached HEAD for testing) to ensure the main tree is +# stripped before a merge commit lands. +# # Safe merge flow (develop → main): # 1. Commit/push all work on develop # 2. git checkout main -# 3. git merge develop --no-commit -# 4. ./scripts/strip_for_main.sh -# 5. git add -A +# 3. git merge develop --no-commit (may conflict on modify/delete) +# 4. ./scripts/strip_for_main.sh (resolves strip-target conflicts +# by deleting + staging together; +# see "Mid-merge mode" below) +# 5. git add -A (stage any remaining clean merges) # 6. git commit -m "merge: develop → main (stripped dev files)" # # Safety checks: -# - Refuses to run on the develop branch (must be on main or detached) -# - Refuses to run with uncommitted changes unless --dry-run +# - ABSOLUTE: refuses to run on the develop branch. No exceptions. +# develop is the source of truth for dev artifacts; stripping there +# would destroy work. See guard below. +# - Outside a merge: refuses to run with uncommitted changes. Mixing +# strips with unrelated edits in one commit would destroy the audit +# trail of what-got-stripped-when. +# - Mid-merge (.git/MERGE_HEAD exists): the dirty-tree check is skipped +# because a --no-commit merge always leaves a dirty tree. The +# "refuses on develop" check is still enforced — we can never be +# mid-merge ON develop (develop is the source, not the target). +# +# Mid-merge mode: +# When .git/MERGE_HEAD exists, strip_path uses `git rm -rf` so modify/ +# delete conflicts (a file deleted on main but modified on develop) are +# resolved by keeping the deletion AND staging the resolution in one +# step. Without this, the script would delete the file but git would +# still see it as conflicted, blocking the commit. # # What it removes: # - Internal tracking: MEMORY.md, MEMORY_ARCHIVE.md, FLOW.md, TECH-DEBT.md, @@ -55,24 +79,44 @@ if [[ "${1:-}" == "--dry-run" ]]; then else echo "=== strip_for_main.sh ===" fi + +# Detect mid-merge state. MERGE_HEAD exists only between `git merge --no-commit` +# (or a conflict pause) and the eventual merge commit. When this is set we +# know the dirty working tree is expected from the merge, not from random edits. +GIT_DIR=$(git rev-parse --git-dir 2>/dev/null || echo ".git") +IN_MERGE=0 +if [[ -f "$GIT_DIR/MERGE_HEAD" ]]; then + IN_MERGE=1 + echo "(mid-merge mode: MERGE_HEAD present)" +fi echo "" -# Safety: refuse real runs on develop (dry-run is fine — it doesn't touch files) +# Safety: ABSOLUTE refusal to run on develop. This guard exists no matter +# what mode or flag is passed, and no matter whether a merge is in progress. +# develop is the source of dev artifacts; stripping there would destroy +# work. A mid-merge on develop is also impossible in the documented flow +# (merges go FROM develop INTO main, not the other way), so reaching this +# condition means something has gone badly wrong and we must abort. BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "unknown") if [[ $DRY_RUN -eq 0 && "$BRANCH" == "develop" ]]; then echo "ERROR: refusing to run on 'develop' branch." - echo "This script strips dev files — run it only on 'main' (or a detached HEAD for a" - echo "temporary test). On develop you'd lose your tracked dev files." + echo "This script strips dev files — run it only on 'main' (or a detached HEAD" + echo "for a temporary test). On develop you'd lose your tracked dev files." echo "" echo "Safe flow: checkout main, merge develop --no-commit, then run this script." echo "To preview from develop: ./scripts/strip_for_main.sh --dry-run" exit 1 fi -# Safety: refuse real runs with uncommitted changes -if [[ $DRY_RUN -eq 0 ]] && ! git diff --quiet HEAD -- 2>/dev/null; then - echo "ERROR: uncommitted changes detected. Commit or stash first." - echo "(Running the strip with dirty state would mix strips with other edits.)" +# Safety: refuse dirty tree UNLESS we're in the middle of a merge. A merge +# always leaves a dirty tree (staged adds, conflicted modifies), and the +# documented flow runs the strip right after `git merge --no-commit`. Outside +# a merge, a dirty tree means unrelated edits — stripping would mix them in. +if [[ $DRY_RUN -eq 0 && $IN_MERGE -eq 0 ]] && ! git diff --quiet HEAD -- 2>/dev/null; then + echo "ERROR: uncommitted changes detected (and we are NOT mid-merge)." + echo "Commit or stash first. Running the strip with a dirty state would" + echo "mix strips with unrelated edits in one commit, destroying the audit" + echo "trail of what-got-stripped-when." echo "" echo "Override: pass --dry-run to preview without stripping." exit 1 @@ -80,15 +124,44 @@ fi STRIPPED=0 +# strip_path removes $1 from the working tree. Behavior: +# - dry run : only prints what would be removed +# - mid-merge : uses `git rm -rf` so modify/delete conflicts are +# resolved (deleted AND staged as resolved) in one +# step. This is what makes the merge flow work. +# - normal run : uses `rm -rf` (classic behavior — works on tracked +# and untracked paths alike; any resulting deletions +# get staged by the caller's `git add -A`). strip_path() { local path="$1" - if [[ -e "$path" ]]; then - if [[ $DRY_RUN -eq 1 ]]; then + if [[ ! -e "$path" ]] && [[ $IN_MERGE -eq 0 ]]; then + return 0 + fi + if [[ $DRY_RUN -eq 1 ]]; then + if [[ -e "$path" ]]; then echo " would remove: $path" - else - rm -rf "$path" - echo " removed: $path" + STRIPPED=$((STRIPPED + 1)) + fi + return 0 + fi + if [[ $IN_MERGE -eq 1 ]]; then + # --ignore-unmatch: quiet no-op if path doesn't exist (covers the case + # where the path was already deleted on main and develop also deleted + # it, leaving nothing to strip). + if git rm -rf --ignore-unmatch "$path" >/dev/null 2>&1; then + if [[ -e "$path" ]]; then + # git rm failed to touch it (e.g. untracked in merge state) — fall + # back to plain rm and stage later via git add -A. + rm -rf "$path" + echo " removed (untracked): $path" + else + echo " removed (git rm): $path" + fi + STRIPPED=$((STRIPPED + 1)) fi + else + rm -rf "$path" + echo " removed: $path" STRIPPED=$((STRIPPED + 1)) fi } @@ -110,6 +183,7 @@ strip_path ".claude" strip_path ".agents" strip_path "audit" strip_path "adr" +strip_path ".vscode" # editor-specific settings (Snyk IDE prefs, etc.) # Internal reports and utilities strip_path "generate_pdf.py"