Merge pull request #11 from mojwang/feat/signal-safe-cleanup

feat: implement signal-safe cleanup and temp file handling

Author: mojwang

.gitignore

@@ -40,6 +40,13 @@ logs/
 tmp/
 temp/
 .tmp/
+tests/tmp.*
+tests/failing_test.*
+tests/interrupt_test.*
+tests/timeout_cleanup.*
+tests/trap_test.*
+tests/syntax_test.*
+tests/test_syntax_error.sh
 
 # Backup files
 *.backup

CHANGELOG.md

@@ -16,7 +16,6 @@
 ### Features
 
 * Add support for community MCP servers ([47cc153](https://github.com/mojwang/macbook-dev-setup/commit/47cc153a6927d7d537d1f71093427062d32ae9e1))
-
 # [2.6.0](https://github.com/mojwang/macbook-dev-setup/compare/v2.5.0...v2.6.0) (2025-07-26)
 
 

CLAUDE.md

@@ -118,6 +118,12 @@ MCP servers installed:
 - **memory**: In-memory key-value storage for temporary data
 - **git**: Tools to read, search, and manipulate Git repositories
 - **fetch**: Web content fetching and conversion for efficient LLM usage
+- **sequentialthinking**: Dynamic problem-solving through iterative thought processes
+- **context7**: Documentation retrieval for any library (community server)
+- **playwright**: Browser automation and testing (community server)
+- **figma**: Figma design context integration (community server)
+- **semgrep**: Code analysis and pattern matching (community server)
+- **exa**: Enhanced search capabilities (community server)
 
 ### Git Commit Helpers
 ```bash
@@ -152,14 +158,27 @@ The `--sync` flag detects and installs new packages added to configuration files
 
 ### Script Organization
 - **Main Scripts**: `setup.sh` (production) and `setup-validate.sh` (validation/dry-run)
+- **Core Libraries** in `/lib/`:
+  - `common.sh`: Shared functions and utilities
+  - `signal-safety.sh`: Signal handling and cleanup framework
+  - `backup-manager.sh`: Centralized backup management
+  - `config.sh`: Configuration management
 - **Component Scripts** in `/scripts/`:
   - `install-homebrew.sh`: Installs Homebrew package manager
   - `install-packages.sh`: Installs packages from Brewfile
   - `setup-dotfiles.sh`: Deploys dotfiles with automatic backups
   - `setup-applications.sh`: Installs macOS desktop applications
   - `setup-macos.sh`: Configures macOS system preferences
   - `setup-git-hooks.sh`: Configures conventional commit hooks
+  - `setup-claude-mcp.sh`: Installs and configures MCP servers
+  - `setup-terminal-fonts.sh`: Configures terminal fonts
+  - `setup-warp.sh`: Warp terminal specific optimizations
   - `commit-helper.sh`: Interactive conventional commit creator
+  - `cleanup-artifacts.sh`: Periodic maintenance and cleanup
+  - `health-check.sh`: System health verification
+  - `update.sh`: Update all tools and dependencies
+  - `uninstall.sh`: Clean removal with backups
+  - `rollback.sh`: Restore from previous backups
 
 ### Key Configuration Files
 - `homebrew/Brewfile`: Package definitions (formulae, casks, VS Code extensions)
@@ -195,6 +214,107 @@ The `--sync` flag detects and installs new packages added to configuration files
 - Timeout protection (30s) for potentially hanging commands
 - Git is configured with security-conscious settings (fsckObjects enabled)
 
+## Security and Signal Safety
+
+### Signal-Safe Cleanup Requirements
+All scripts that perform system modifications MUST implement signal-safe cleanup:
+
+1. **Source the signal-safety library** at the beginning of the script:
+   ```bash
+   source "$ROOT_DIR/lib/signal-safety.sh"
+   ```
+
+2. **Implement a cleanup function** specific to your script's needs:
+   ```bash
+   cleanup_yourscript() {
+       # Clean up temporary files
+       rm -f "${TEMP_FILE:-}" 2>/dev/null || true
+       
+       # Kill any background processes
+       [[ -n "${CHILD_PID:-}" ]] && kill "$CHILD_PID" 2>/dev/null || true
+       
+       # Remove partial installations
+       [[ -d "${WORK_DIR:-}" ]] && rm -rf "$WORK_DIR" 2>/dev/null || true
+       
+       # Call default cleanup
+       default_cleanup
+   }
+   ```
+
+3. **Register the cleanup function** immediately after defining it:
+   ```bash
+   setup_cleanup "cleanup_yourscript"
+   ```
+
+### Critical Cleanup Areas
+When implementing cleanup, ensure these artifacts are handled:
+
+- **Package Managers**: npm node_modules, Python venvs, Ruby gems
+- **Build Artifacts**: Compiled binaries, object files, cache directories
+- **Temporary Files**: Config backups, download files, lock files
+- **Background Processes**: Any spawned child processes or daemons
+- **Partial Installations**: Incomplete setups that could cause issues
+
+### Security Best Practices
+
+1. **Never leave sensitive data** in temporary files or logs
+2. **Use secure temp directories**: `mktemp -d` with proper permissions
+3. **Validate all inputs** before using in commands
+4. **Avoid eval** unless absolutely necessary
+5. **Quote all variables** to prevent injection: `"$var"` not `$var`
+6. **Set restrictive permissions** on created files: `umask 077`
+7. **Clean up secrets** from memory/disk after use
+
+### Testing Signal Safety
+Always test your cleanup implementation:
+
+```bash
+# Start your script and interrupt it
+./your-script.sh &
+PID=$!
+sleep 2
+kill -INT $PID
+# Verify no artifacts remain
+```
+
+## Testing Framework
+
+### Running Tests
+```bash
+# Run all tests
+./tests/run_tests.sh
+
+# Run specific test suites
+./tests/run_tests.sh unit        # Unit tests only
+./tests/run_tests.sh integration # Integration tests only
+./tests/run_tests.sh ci          # CI-specific tests
+
+# Run tests in parallel (faster)
+./tests/run_tests_parallel.sh
+
+# Run with custom parallelism
+TEST_JOBS=8 ./tests/run_tests.sh
+```
+
+### Test Organization
+- **Unit Tests** (`tests/unit/`): Test individual functions and components
+- **Integration Tests** (`tests/integration/`): Test script interactions
+- **CI Tests** (`tests/ci/`): Tests specific to CI environment
+- **Stress Tests** (`tests/stress/`): Performance and load testing
+- **Performance Tests** (`tests/performance/`): Benchmark and optimization tests
+
+### Writing Tests
+Use the test framework's built-in functions:
+```bash
+it "should describe what it tests"
+assert_equals "expected" "actual" "Test description"
+assert_true "[[ condition ]]" "Condition should be true"
+assert_false "[[ condition ]]" "Condition should be false"
+assert_contains "haystack" "needle" "Should contain substring"
+assert_file_exists "/path/to/file" "File should exist"
+assert_dir_exists "/path/to/dir" "Directory should exist"
+```
+
 ## Setup Script Maintenance
 
 When adding new functionality or capabilities to this project, always check if the setup script needs updating:
@@ -207,3 +327,33 @@ When adding new functionality or capabilities to this project, always check if t
 6. **Update documentation**: Document new features in this file if needed
 
 After any changes, run the setup script to ensure everything works correctly.
+
+## Important Behavioral Guidelines
+
+- Do only what has been asked; nothing more, nothing less
+- Never create files unless absolutely necessary for the task
+- Always prefer editing existing files over creating new ones
+- Never proactively create documentation files (*.md) or README files unless explicitly requested
+- Never commit changes unless explicitly asked to
+- When blocked, ask for clarification rather than making assumptions
+
+## Repository Standards
+
+### Version Management
+- Current version is tracked in `VERSION` file
+- Semantic versioning is enforced via CI/CD
+- Changelog follows Keep a Changelog format
+- Automatic releases via semantic-release
+
+### CI/CD Pipeline
+- GitHub Actions for automated testing
+- Semantic release for version management
+- Branch protection on main branch
+- All PRs require passing tests
+- Claude Code Review integration for AI-assisted reviews
+
+### Code Quality
+- ShellCheck for shell script linting
+- Consistent error handling patterns
+- Comprehensive test coverage
+- Security scanning in CI pipeline

VERSION

@@ -1 +1 @@
-2.8.0
+2.8.0

config/global-claude.md

@@ -24,6 +24,8 @@ This file provides baseline guidance to Claude Code across all projects. Project
 - Provide dry-run/preview modes for all destructive operations
 - Detailed error handling with recovery options
 - Optional logging for troubleshooting
+- Signal-safe cleanup handlers for all system-modifying scripts
+- Atomic file operations to prevent partial writes
 
 ## File Organization
 
@@ -70,6 +72,16 @@ This file provides baseline guidance to Claude Code across all projects. Project
 - Follow conventional commit message formats when they exist in the project
 - Keep commits focused and atomic - one logical change per commit
 
+## Security Practices
+
+- Always quote variables in shell scripts: `"$var"` not `$var`
+- Validate and sanitize all user inputs
+- Use `mktemp` for temporary files with restrictive permissions
+- Never store secrets in code or logs
+- Clean up sensitive data from memory/disk after use
+- Implement proper signal handling for cleanup on interruption
+- Use secure defaults and fail closed, not open
+
 ## Important Behavioral Guidelines
 
 - Do only what has been asked; nothing more, nothing less

docs/SIGNAL_SAFETY.md

@@ -0,0 +1,160 @@
+# Signal Safety and Cleanup Guide
+
+This document explains how scripts in this repository handle signals and cleanup to ensure no temporary files or resources are left behind, even when interrupted.
+
+## The Problem
+
+When a script is interrupted (e.g., via Ctrl+C), it receives a SIGINT signal. If the script only uses `trap cleanup EXIT`, the cleanup function won't run because:
+- `EXIT` trap only triggers on normal script termination
+- SIGINT causes immediate termination unless explicitly handled
+- This can leave temporary files, directories, and other resources behind
+
+## The Solution
+
+All scripts that create temporary resources should use signal-aware traps:
+
+```bash
+# ❌ BAD: Only handles normal exit
+trap cleanup EXIT
+
+# ✅ GOOD: Handles interrupts and other signals
+trap cleanup EXIT INT TERM HUP
+```
+
+### Signal Explanation
+
+- **EXIT**: Normal script termination
+- **INT**: Interrupt signal (Ctrl+C)
+- **TERM**: Termination request (kill command)
+- **HUP**: Terminal hangup (closing terminal)
+
+## Implementation Guide
+
+### Basic Pattern
+
+```bash
+#!/bin/bash
+
+# Define cleanup function
+cleanup() {
+    rm -f "$temp_file"
+    rm -rf "$temp_dir"
+}
+
+# Set up signal-safe trap
+trap cleanup EXIT INT TERM HUP
+
+# Create temporary resources
+temp_file=$(mktemp /tmp/myapp.XXXXXX)
+temp_dir=$(mktemp -d /tmp/myapp_dir.XXXXXX)
+
+# Your script logic here
+```
+
+### Using the Signal Safety Library
+
+For more complex scripts, use the provided library:
+
+```bash
+#!/bin/bash
+
+source "$(dirname "$0")/../lib/signal-safety.sh"
+
+# Define custom cleanup
+cleanup() {
+    echo "Cleaning up..."
+    # Your cleanup code here
+    default_cleanup  # Cleans registered temp resources
+}
+
+# Setup signal handling
+setup_cleanup "cleanup"
+
+# Use safe temp file creation
+temp_file=$(safe_mktemp "myapp.XXXXXX")
+temp_dir=$(safe_mktemp_dir "myapp_work.XXXXXX")
+```
+
+### Preventing Multiple Cleanup Calls
+
+The signal-safety library ensures cleanup only runs once:
+
+```bash
+CLEANUP_DONE=false
+
+safe_cleanup() {
+    if [[ "$CLEANUP_DONE" == "false" ]]; then
+        CLEANUP_DONE=true
+        cleanup
+    fi
+}
+
+trap safe_cleanup EXIT INT TERM HUP
+```
+
+## Testing Signal Handling
+
+Run the signal handling tests:
+
+```bash
+./tests/test_signal_handling.sh
+```
+
+This test suite verifies:
+- Cleanup on SIGINT (Ctrl+C)
+- Cleanup on SIGTERM
+- Proper trap inheritance in subshells
+- Prevention of multiple cleanup calls
+
+## CI/CD Considerations
+
+GitHub Actions and other CI systems may forcefully terminate jobs. Our scripts handle this by:
+
+1. Using proper signal traps in all scripts
+2. Running periodic cleanup tests in CI
+3. Providing a cleanup utility for safety
+
+## Cleanup Utility
+
+For additional safety, a cleanup utility is provided:
+
+```bash
+# Dry run (default) - shows what would be cleaned
+./scripts/cleanup-artifacts.sh
+
+# Actually clean up
+./scripts/cleanup-artifacts.sh --execute
+
+# Add to crontab for automatic cleanup
+0 3 * * 0 /path/to/cleanup-artifacts.sh --execute
+```
+
+## Best Practices
+
+1. **Always use signal-aware traps** when creating temporary resources
+2. **Use /tmp for temporary files** via `mktemp /tmp/prefix.XXXXXX`
+3. **Test signal handling** when modifying scripts that create resources
+4. **Clean up in reverse order** of resource creation
+5. **Use the signal-safety library** for complex scripts
+
+## Verification
+
+To verify no artifacts are left behind:
+
+```bash
+# Run CI cleanup verification
+./tests/test_ci_cleanup.sh
+
+# Check for common artifacts
+find /tmp -name "test_*" -o -name "tmp.*" | wc -l
+find ~ -maxdepth 1 -name ".test-setup-backups-*" | wc -l
+```
+
+## Updated Scripts
+
+The following scripts have been updated with proper signal handling:
+- `lib/common.sh` - Main library with signal-safe trap
+- `tests/test_backup_system.sh` - Handles test backup cleanup
+- `scripts/rollback.sh` - Cleans up temporary requirements file
+- `tests/test_error_recovery.sh` - Cleans up test artifacts
+- All scripts using the common library inherit signal safety