Skip to content

fix(strip): make strip_for_main.sh work mid-merge + align safety lists#4

Merged
devonartis merged 1 commit into
developfrom
fix/strip-script-mid-merge
Apr 10, 2026
Merged

fix(strip): make strip_for_main.sh work mid-merge + align safety lists#4
devonartis merged 1 commit into
developfrom
fix/strip-script-mid-merge

Conversation

@devonartis

Copy link
Copy Markdown
Owner

Summary

Fixes two issues that surfaced during Task 28 (develop → main strip merge) of the M-sec plan.

Issue 1 — documented merge flow was impossible

scripts/strip_for_main.sh header comment says:

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 with "ERROR: uncommitted changes detected".

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 — it's the merge's own state)
  • have strip_path use git rm -rf --ignore-unmatch so modify/delete conflicts get deleted AND staged as resolved in one 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 in every mode, merge state, and flag combo. develop is the source of dev artifacts — the script must never touch it.

Issue 2 — two defense layers had drifted

scripts/strip_for_main.sh (pre-merge automation) and .githooks/pre-commit (per-commit safety net on main) are supposed to enforce the same "never on main" list. Audit found:

Path strip_for_main.sh pre-commit
adr/ ❌ missing
.vscode/ ❌ missing ❌ missing

.vscode/settings.json is in develop's tree and carries editor-specific settings (e.g. Snyk IDE prefs) — it should never ship to main. adr/ was already in the strip list but the pre-commit hook wouldn't catch a manual stray commit.

Both lists now agree: 12 strip paths in each. Added an explicit comment in pre-commit telling future editors to keep both in sync.

Local verification

  • bash -n scripts/strip_for_main.sh — syntax OK
  • bash -n .githooks/pre-commit — syntax OK
  • ./scripts/strip_for_main.sh --dry-run from develop — 12 paths listed (was 11; new .vscode entry visible)
  • go build ./cmd/broker ./cmd/aactl — OK
  • go test -short ./... — 15/15 packages PASS
  • golangci-lint run ./... — clean

Test plan after merge

  • Merge this PR to develop via CI gates
  • Retry Task 28 (develop → main strip merge) using the fixed script
  • Verify main gets the merge commit with all 12 strip paths applied
  • Continue Phase D (branch protection on main, README badges, observation)

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
@devonartis devonartis merged commit 6d68c19 into develop Apr 10, 2026
16 checks passed
@devonartis devonartis deleted the fix/strip-script-mid-merge branch April 10, 2026 09:52
devonartis added a commit that referenced this pull request Apr 10, 2026
Brings M-sec CI/build/gates v1 from develop to main. Represents the
full Phase A–C output of the plan at
.plans/specs/2026-04-10-ci-build-gates-msec-plan.md (30 tasks across
feature/ci-msec + 1 task on fix/strip-script-mid-merge).

New on main:
  - .github/workflows/ci.yml     — 13 parallel gates + gate-parity +
                                   gates-passed aggregator, triggers
                                   on PR to develop and push to
                                   develop/main
  - .github/workflows/codeql.yml — Go SAST (workflow_dispatch only
                                   until public flip — see TD-VUL-006)
  - .github/workflows/scorecard.yml — OpenSSF Scorecard (same —
                                      workflow_dispatch only)
  - .github/workflows/nightly.yml — L4 regression (scheduled + manual)
  - .github/workflows/contribution-policy.yml — Decision 014
                                                auto-close
  - .github/dependabot.yml       — github-actions/gomod/docker weekly
  - .github/CODEOWNERS           — @devonartis
  - .github/MAINTAINERS          — contribution-policy allowlist
  - scripts/smoke/core-contract.sh — L2.5 live-broker contract smoke
  - scripts/test-gate-parity.sh  — local/CI gate list alignment
  - .gosec.yml                   — documented G117/G304/G101 excludes
  - .golangci.yml                — security-aware linter set

Modified on main:
  - scripts/gates.sh             — extended from 4 to 13 gates
  - scripts/strip_for_main.sh    — mid-merge support (fix from PR #4)
  - .githooks/pre-commit         — aligned FORBIDDEN list with strip
  - .gitignore                   — ignore coverage.out, sbom.spdx.json
  - go.mod                       — toolchain go1.25.7 → go1.25.9
                                   (resolves TD-VUL-001..004)
  - CHANGELOG.md                 — Unreleased entries for M-sec + strip fix
  - Production code fixes flagged by new gates:
      internal/keystore/parseKey — defensive ed25519.PublicKey assertion
      internal/mutauth/sweep     — log auto-revoke failures
      cmd/aactl/client           — propagate json/io errors
      internal/store/sql_store   — documented #nosec G202
      internal/admin/test        — doc comment on spec compliance test
  - gofmt normalization across 24 drifted files

Stripped by scripts/strip_for_main.sh (mid-merge mode, first use of
the fix from PR #4):
  MEMORY.md, MEMORY_ARCHIVE.md, FLOW.md, TECH-DEBT.md, AGENTS.md,
  CLAUDE.md, COWORK_SESSION.md, COWORK_DOCS_AUDIT.md, .active_module,
  .plans/, .claude/, .agents/, audit/, adr/, .vscode/, generate_pdf.py

Verification:
  - ./scripts/strip_for_main.sh (mid-merge): 16 paths stripped
  - go build ./cmd/broker ./cmd/aactl: OK
  - go test -short ./...: 15/15 packages PASS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant