fix: separate check from format commands to prevent accidental modifications

osterman · osterman · commit 338d91275d22 · 2025-09-07T16:08:07.000-05:00
BREAKING CHANGE: Renamed 'fix' command to 'format' for clarity

- Check commands are now read-only (idempotent) - they only report issues
- Format commands modify files - clearly separated and labeled
- Added format-all with explicit DANGEROUS warning and 5-second delay
- Protected golden snapshots in tests/snapshots/ from all hooks
- Clear documentation explaining check vs format distinction

This prevents developers from accidentally formatting the entire codebase
when they just wanted to check for issues. Check commands are safe to run
anytime, while format commands explicitly modify files.
diff --git a/.atmos.d/dev.yaml b/.atmos.d/dev.yaml
@@ -46,42 +46,86 @@ commands:
 
         echo "✅ Development environment setup complete!"
         echo ""
-        echo "Run 'atmos dev check' to run pre-commit hooks on staged files"
-        echo "Run 'atmos dev check-pr' to run pre-commit hooks on PR changes"
-        echo "Run 'atmos dev check-all' to run pre-commit hooks on all files"
+        echo "Run 'atmos dev check' to check staged files (read-only)"
+        echo "Run 'atmos dev check-pr' to check PR changes (read-only)"
+        echo "Run 'atmos dev format' to auto-format staged files"
+        echo "Run 'atmos dev format-pr' to auto-format PR changes"
 
   - name: check
-    description: Run pre-commit hooks on staged files (for local development)
+    description: Check staged files for issues (read-only, no modifications)
     steps:
-      - pre-commit run
+      - |
+        echo "🔍 Checking staged files (read-only)..."
+        # Run checks without modifying files
+        STAGED_GO_FILES=$(git diff --cached --name-only --diff-filter=ACM -- '*.go' | grep -v -E '^(vendor/|tests/test-cases/|tests/testdata/|tests/snapshots/)' || true)
+        if [ -n "$STAGED_GO_FILES" ]; then
+          echo "Checking Go formatting..."
+          echo "$STAGED_GO_FILES" | xargs -I {} gofumpt -d {} 2>/dev/null | head -20 || true
+          echo "Running golangci-lint..."
+          golangci-lint run --new-from-rev=origin/main --config=.golangci.yml $STAGED_GO_FILES || true
+        fi
+        echo "💡 To auto-fix issues, run: atmos dev format"
 
   - name: check-pr
-    description: Run pre-commit hooks on PR changes (compares with main branch)
+    description: Check PR changes for issues (read-only, no modifications)
     steps:
       - |
-        if [ -z "${GITHUB_BASE_REF:-}" ]; then
-          # Local development - compare with main branch
-          echo "🔍 Checking files changed from main branch..."
-          pre-commit run --from-ref origin/main --to-ref HEAD
-        else
-          # CI environment - use GitHub PR refs
-          echo "🔍 Checking files changed in PR..."
-          pre-commit run --from-ref "origin/${GITHUB_BASE_REF}" --to-ref HEAD
+        echo "🔍 Checking PR changes (read-only)..."
+        BASE_REF="${GITHUB_BASE_REF:-main}"
+        # Get changed Go files
+        CHANGED_GO_FILES=$(git diff origin/$BASE_REF...HEAD --name-only -- '*.go' | grep -v -E '^(vendor/|tests/test-cases/|tests/testdata/|tests/snapshots/)' || true)
+        if [ -n "$CHANGED_GO_FILES" ]; then
+          echo "Checking Go formatting..."
+          echo "$CHANGED_GO_FILES" | xargs -I {} gofumpt -d {} 2>/dev/null | head -20 || true
+          echo "Running golangci-lint..."
+          golangci-lint run --new-from-rev=origin/$BASE_REF --config=.golangci.yml || true
         fi
+        echo "💡 To auto-fix issues, run: atmos dev format-pr"
 
   - name: check-all
-    description: Run pre-commit hooks on all files (use sparingly)
+    description: Check all files for issues (read-only, no modifications)
     steps:
-      - pre-commit run --all-files
+      - |
+        echo "🔍 Checking all files (read-only)..."
+        echo "⚠️  This may take a while..."
+        # Check formatting without modifying
+        echo "Checking Go formatting..."
+        find . -name '*.go' -not -path './vendor/*' -not -path './tests/test-cases/*' -not -path './tests/testdata/*' -not -path './tests/snapshots/*' | xargs gofumpt -d 2>/dev/null | head -50 || true
+        echo "Running comprehensive linting..."
+        golangci-lint run --config=.golangci.yml || true
+        echo "💡 To auto-fix issues, run: atmos dev format-all (⚠️  DANGEROUS)"
+
+  - name: format
+    description: Auto-format staged files
+    steps:
+      - |
+        echo "🔧 Auto-formatting staged files..."
+        pre-commit run || true
+        echo "✅ Formatting applied to staged files"
 
-  - name: fix
-    description: Auto-fix Go formatting issues
+  - name: format-pr
+    description: Auto-format PR changes
     steps:
       - |
-        echo "🔧 Auto-fixing Go formatting issues..."
-        gofumpt -w .
-        go mod tidy
-        echo "✅ Formatting fixes applied"
+        echo "🔧 Auto-formatting PR changes..."
+        BASE_REF="${GITHUB_BASE_REF:-main}"
+        pre-commit run --from-ref origin/$BASE_REF --to-ref HEAD || true
+        echo "✅ Formatting applied to PR changes"
+
+  - name: format-all
+    description: ⚠️  DANGEROUS - Auto-format ALL files (excludes golden snapshots)
+    steps:
+      - |
+        echo "⚠️  WARNING: This will modify many files!"
+        echo "   Excludes: vendor/, tests/test-cases/, tests/testdata/, tests/snapshots/"
+        echo "   Golden snapshots and fixtures are protected."
+        echo ""
+        echo "   Press Ctrl+C to cancel, or wait 5 seconds to continue..."
+        sleep 5
+        echo "🔧 Auto-formatting all files..."
+        # Run pre-commit on all files with our exclude patterns
+        pre-commit run --all-files || true
+        echo "✅ Formatting complete. Review changes with: git diff"
 
   - name: lint
     description: Run golangci-lint
@@ -119,10 +163,17 @@ commands:
         echo "Available Atmos dev commands:"
         echo ""
         echo "  atmos dev setup       - Set up local development environment"
-        echo "  atmos dev check       - Run pre-commit hooks on staged files (local dev)"
-        echo "  atmos dev check-pr    - Run pre-commit hooks on PR changes"
-        echo "  atmos dev check-all   - Run pre-commit hooks on all files"
-        echo "  atmos dev fix         - Auto-fix Go formatting issues"
+        echo ""
+        echo "  Checking (read-only, no modifications):"
+        echo "  atmos dev check       - Check staged files for issues"
+        echo "  atmos dev check-pr    - Check PR changes for issues"
+        echo "  atmos dev check-all   - Check all files for issues"
+        echo ""
+        echo "  Formatting (modifies files):"
+        echo "  atmos dev format      - Auto-format staged files"
+        echo "  atmos dev format-pr   - Auto-format PR changes"
+        echo "  atmos dev format-all  - ⚠️  DANGEROUS: Auto-format ALL files"
+        echo ""
         echo "  atmos dev lint        - Run golangci-lint"
         echo "  atmos dev test        - Run tests"
         echo "  atmos dev build       - Build the Atmos binary"
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -5,7 +5,7 @@ repos:
     hooks:
       - id: go-fumpt
         args: [-w]
-        exclude: ^(vendor/|tests/test-cases/|tests/testdata/)
+        exclude: ^(vendor/|tests/test-cases/|tests/testdata/|tests/snapshots/)
 
       - id: go-build-mod
         name: Check Go build
@@ -26,16 +26,16 @@ repos:
         types: [go]
         pass_filenames: false
         require_serial: true
-        exclude: ^(vendor/|tests/test-cases/|tests/testdata/)
+        exclude: ^(vendor/|tests/test-cases/|tests/testdata/|tests/snapshots/)
 
   # General file hygiene
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.5.0
     hooks:
       - id: trailing-whitespace
-        exclude: ^(vendor/|tests/test-cases/|tests/testdata/|.*\.md)
+        exclude: ^(vendor/|tests/test-cases/|tests/testdata/|tests/snapshots/|.*\.md)
       - id: end-of-file-fixer
-        exclude: ^(vendor/|tests/test-cases/|tests/testdata/)
+        exclude: ^(vendor/|tests/test-cases/|tests/testdata/|tests/snapshots/)
       - id: check-yaml
         exclude: ^(examples/|tests/test-cases/|vendor/)
         args: [--allow-multiple-documents]
diff --git a/docs/development.md b/docs/development.md
@@ -28,15 +28,22 @@ We use Atmos custom commands for development (dogfooding our own tool). This ens
 ### Available Commands
 
 ```bash
-atmos dev help         # Show all available dev commands
-atmos dev check        # Run pre-commit hooks on staged files (local dev)
-atmos dev check-pr     # Run pre-commit hooks on PR changes
-atmos dev check-all    # Run pre-commit hooks on all files
+# Checking Commands (Read-only, no modifications)
+atmos dev check        # Check staged files for issues
+atmos dev check-pr     # Check PR changes for issues  
+atmos dev check-all    # Check all files for issues
 atmos dev lint         # Run golangci-lint
+
+# Formatting Commands (Modifies files)
+atmos dev format       # Auto-format staged files
+atmos dev format-pr    # Auto-format PR changes
+atmos dev format-all   # ⚠️ DANGEROUS: Auto-format ALL files
+
+# Build and Test Commands
 atmos dev test         # Run tests
 atmos dev build        # Build the Atmos binary
 atmos dev quick        # Quick build and test
-atmos dev fix          # Auto-fix Go formatting issues
+atmos dev help         # Show all available dev commands
 ```
 
 ### Alternative Make Commands
@@ -53,12 +60,23 @@ make lint              # Run golangci-lint on changed files
 
 We use pre-commit hooks to ensure code quality. The following hooks run automatically on `git commit`:
 
-### Running Checks
+### Checking vs Formatting
 
-Different commands for different contexts:
-- **`atmos dev check`** - Checks only staged files (best for local development before commit)
+**Important distinction:**
+- **Check commands** are read-only and will NOT modify any files. They only report issues.
+- **Format commands** WILL modify files to fix issues automatically.
+
+#### Check Commands (Safe, read-only)
+- **`atmos dev check`** - Checks only staged files (best before committing)
 - **`atmos dev check-pr`** - Checks files changed from main branch (best for PR reviews)
-- **`atmos dev check-all`** - Checks all files in the repository (use sparingly, can be slow)
+- **`atmos dev check-all`** - Checks all files in the repository
+
+#### Format Commands (Modifies files)
+- **`atmos dev format`** - Auto-formats only staged files
+- **`atmos dev format-pr`** - Auto-formats files changed from main branch
+- **`atmos dev format-all`** - ⚠️ **DANGEROUS**: Auto-formats ALL files (use with extreme caution)
+
+**Note:** Golden snapshots and test fixtures are always protected from formatting.
 
 ### Go-specific Hooks
 - **go-fumpt**: Enforces consistent Go formatting (stricter than gofmt)
