--- license: Apache-2.0 name: script-refactorer description: | Modernizes legacy scripts (bash, Node, Python) by replacing deprecated APIs, adding error handling, converting callbacks to async/await, and improving maintainability. Turns brittle one-off scripts into reliable, documented tools. category: Code Quality & Testing tags: - refactoring - scripts - modernization - cleanup - maintenance allowed-tools: - Read - Bash(*) - Glob - Grep - Write - Edit pairs-with: - skill: refactoring-surgeon reason: General refactoring principles applied to scripts - skill: error-handling-patterns reason: Robust error handling is the primary script improvement - skill: beautiful-cli-design reason: Good CLI ergonomics for script interfaces - skill: devops-automator reason: Scripts often live inside CI/CD pipelines --- # Script Refactorer Modernizes legacy scripts into reliable, maintainable tools. Replaces deprecated patterns, adds error handling and input validation, converts callback spaghetti to async/await, and turns undocumented one-off hacks into proper CLI tools. ## Activation Triggers **Use this skill when:** - A bash/Node/Python script works but is fragile, undocumented, or hard to modify - Scripts use deprecated Node.js APIs (e.g., `new Buffer()`, `url.parse()`, `fs.exists()`) - Callback-heavy Node.js code needs async/await conversion - Scripts lack error handling and fail silently or with cryptic messages - A script needs proper CLI argument parsing, help text, and exit codes - Legacy Python 2 patterns need updating to modern Python 3 - Shell scripts lack `set -euo pipefail` and proper quoting **Do NOT use this skill for:** - Rewriting scripts from scratch in a different language (use the appropriate language skill) - Building new CLI tools (use beautiful-cli-design) - Large application refactoring (use refactor-architect for planning, refactoring-surgeon for execution) - Performance optimization of scripts (use performance-profiling) ## Core Capabilities ### 1. Bash Script Modernization **Before** -- the kind of script that makes you wince: ```bash #!/bin/bash # deploy.sh - "works on my machine" cd /opt/app git pull npm install npm run build pm2 restart app echo "done" ``` **After** -- a script you can trust: ```bash #!/usr/bin/env bash set -euo pipefail # deploy.sh — Deploy application with rollback safety # Usage: deploy.sh [--branch ] [--skip-build] [--dry-run] readonly SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" readonly APP_DIR="/opt/app" readonly LOG_FILE="/var/log/deploy-$(date +%Y%m%d-%H%M%S).log" # Defaults BRANCH="main" SKIP_BUILD=false DRY_RUN=false usage() { cat < Branch to deploy (default: main) --skip-build Skip npm install and build steps --dry-run Show what would happen without doing it -h, --help Show this help message Environment: APP_DIR Application directory (default: /opt/app) PM2_APP_NAME PM2 process name (default: app) EOF exit 0 } log() { echo "[$(date '+%H:%M:%S')] $*" | tee -a "$LOG_FILE"; } die() { log "FATAL: $*" >&2; exit 1; } parse_args() { while [[ $# -gt 0 ]]; do case "$1" in --branch) BRANCH="${2:?--branch requires a value}"; shift 2 ;; --skip-build) SKIP_BUILD=true; shift ;; --dry-run) DRY_RUN=true; shift ;; -h|--help) usage ;; *) die "Unknown option: $1" ;; esac done } preflight_checks() { command -v git >/dev/null 2>&1 || die "git is not installed" command -v npm >/dev/null 2>&1 || die "npm is not installed" command -v pm2 >/dev/null 2>&1 || die "pm2 is not installed" [[ -d "$APP_DIR/.git" ]] || die "$APP_DIR is not a git repository" } main() { parse_args "$@" preflight_checks local prev_commit prev_commit=$(git -C "$APP_DIR" rev-parse HEAD) log "Current commit: ${prev_commit:0:8}" log "Deploying branch: $BRANCH" if $DRY_RUN; then log "DRY RUN — would deploy $BRANCH to $APP_DIR" exit 0 fi # Capture state for rollback trap 'log "Deploy failed. Rolling back to ${prev_commit:0:8}"; git -C "$APP_DIR" checkout "$prev_commit" 2>/dev/null' ERR git -C "$APP_DIR" fetch origin "$BRANCH" git -C "$APP_DIR" checkout "$BRANCH" git -C "$APP_DIR" pull origin "$BRANCH" if ! $SKIP_BUILD; then log "Installing dependencies..." (cd "$APP_DIR" && npm ci --production) log "Building..." (cd "$APP_DIR" && npm run build) fi log "Restarting application..." pm2 restart "${PM2_APP_NAME:-app}" log "Deploy complete: $(git -C "$APP_DIR" rev-parse --short HEAD)" } main "$@" ``` **Key transformations:** - `set -euo pipefail` -- fail on errors, undefined variables, pipe failures - `#!/usr/bin/env bash` -- portable shebang - Proper argument parsing with `--help` - Preflight checks for required commands - Trap-based rollback on failure - Logging with timestamps - Readonly variables for constants - Quoted variables everywhere (no word-splitting bugs) ### 2. Node.js Deprecated API Replacement | Deprecated | Replacement | Since | |------------|-------------|-------| | `new Buffer(data)` | `Buffer.from(data)` | Node 6 | | `Buffer(size)` | `Buffer.alloc(size)` | Node 6 | | `url.parse(str)` | `new URL(str)` | Node 10 | | `url.resolve(from, to)` | `new URL(to, from).href` | Node 10 | | `fs.exists(path, cb)` | `fs.access(path)` or `fs.stat(path)` | Node 1 | | `path.exists()` | `fs.existsSync()` | Node 1 | | `require('domain')` | `AsyncLocalStorage` | Node 13 | | `crypto.createCipher()` | `crypto.createCipheriv()` | Node 10 | | `process.binding()` | N/A (internal only) | Node 10 | | `util.puts/print/debug` | `console.log/error` | Node 0.12 | | `sys` module | `util` module | Ancient | | `querystring.parse()` | `URLSearchParams` | Node 10 | | `os.tmpDir()` | `os.tmpdir()` | Node 4 | ### 3. Callback to Async/Await Conversion **Before** -- callback pyramid: ```javascript const fs = require('fs'); const path = require('path'); function processFiles(dir, callback) { fs.readdir(dir, function(err, files) { if (err) return callback(err); let results = []; let pending = files.length; if (!pending) return callback(null, results); files.forEach(function(file) { const filePath = path.join(dir, file); fs.stat(filePath, function(err, stat) { if (err) return callback(err); if (stat.isDirectory()) { processFiles(filePath, function(err, res) { if (err) return callback(err); results = results.concat(res); if (!--pending) callback(null, results); }); } else { fs.readFile(filePath, 'utf8', function(err, content) { if (err) return callback(err); results.push({ path: filePath, content: content }); if (!--pending) callback(null, results); }); } }); }); }); } ``` **After** -- clean async/await: ```javascript import { readdir, stat, readFile } from 'node:fs/promises'; import { join } from 'node:path'; async function processFiles(dir) { const entries = await readdir(dir, { withFileTypes: true }); const results = await Promise.all( entries.map(async (entry) => { const filePath = join(dir, entry.name); if (entry.isDirectory()) { return processFiles(filePath); } const content = await readFile(filePath, 'utf8'); return [{ path: filePath, content }]; }) ); return results.flat(); } ``` **Conversion checklist:** 1. Replace `require()` with `import` (ESM) or keep `require` if CommonJS is required 2. Use `node:` prefix for built-in modules (`node:fs`, `node:path`) 3. Replace `fs.readFile(path, cb)` with `await readFile(path)` from `node:fs/promises` 4. Replace manual pending counters with `Promise.all()` or `Promise.allSettled()` 5. Replace `try/catch` around callbacks with `try/catch` around `await` 6. Use `for await...of` for streams instead of `.on('data', cb)` 7. Add `{ withFileTypes: true }` to `readdir` to avoid extra `stat` calls ### 4. Python Script Modernization **Common upgrades:** ```python # BEFORE: Python 2/early 3 patterns import os import sys import json # String formatting msg = "Hello %s, you have %d items" % (name, count) # File handling f = open('data.json', 'r') data = json.load(f) f.close() # Path construction config_path = os.path.join(os.path.dirname(__file__), '..', 'config', 'settings.json') # Subprocess import subprocess output = subprocess.Popen(['ls', '-la'], stdout=subprocess.PIPE).communicate()[0] # Type checking if type(x) == str: pass ``` ```python # AFTER: Modern Python 3.10+ from pathlib import Path import json import subprocess import sys # f-strings msg = f"Hello {name}, you have {count} items" # Context manager with Path('data.json').open() as f: data = json.load(f) # pathlib config_path = Path(__file__).parent.parent / 'config' / 'settings.json' # subprocess.run result = subprocess.run(['ls', '-la'], capture_output=True, text=True, check=True) output = result.stdout # isinstance if isinstance(x, str): pass ``` ### 5. Signal Handling and Exit Codes ```bash # Bash: proper signal handling cleanup() { log "Caught signal, cleaning up..." rm -f "$LOCK_FILE" exit 130 # 128 + SIGINT(2) } trap cleanup INT TERM # Exit code conventions # 0 = success # 1 = general error # 2 = misuse of shell command (bad args) # 126 = command not executable # 127 = command not found # 130 = terminated by Ctrl+C (128+2) # 143 = terminated by SIGTERM (128+15) ``` ```javascript // Node.js: graceful shutdown process.on('SIGINT', async () => { console.log('\nReceived SIGINT. Cleaning up...'); await cleanup(); process.exit(130); }); process.on('SIGTERM', async () => { console.log('Received SIGTERM. Shutting down gracefully...'); await cleanup(); process.exit(143); }); process.on('uncaughtException', (err) => { console.error('Uncaught exception:', err.message); process.exit(1); }); process.on('unhandledRejection', (reason) => { console.error('Unhandled rejection:', reason); process.exit(1); }); ``` ## Decision Points ### When to refactor vs. rewrite | Signal | Action | |--------|--------| | Script is < 100 lines and basically works | Refactor in place | | Script works but has no error handling | Add error handling, keep logic | | Script uses APIs deprecated 2+ major versions ago | Replace deprecated APIs | | Script is > 300 lines with tangled control flow | Consider splitting into modules first | | Script is in a dead language/runtime | Rewrite (but that's a different skill) | | Script is a critical production tool | Refactor incrementally with tests | ### When to convert CJS to ESM - **Convert**: New Node.js projects, anything using `type: "module"` in package.json, libraries targeting modern consumers - **Keep CJS**: Scripts embedded in legacy build systems, anything that must `require()` conditionally, tools with no plans to publish as packages ### Language-specific argument parsers | Language | Recommended | Why | |----------|-------------|-----| | Bash | Manual `case` loop or `getopts` | No dependencies, portable | | Node.js | `parseArgs` (built-in since Node 18.3) | Zero dependencies | | Python | `argparse` (stdlib) | Battle-tested, auto-generates help | | Go | `flag` (stdlib) or `cobra` | Standard practice | ## Anti-Patterns ### 1. Gold-plating a throwaway script **Symptom**: Adding full argument parsing, logging, and tests to a one-time data migration script **Fix**: If the script runs once and is deleted, minimal error handling is enough. Save the polish for scripts that live in the repo. ### 2. Replacing `bash` with Node.js for file operations **Symptom**: Rewriting `cp`, `mv`, `find`, `grep` in JavaScript **Fix**: Shell is the right tool for file manipulation. Only move to Node.js when the logic (not the I/O) is complex. ### 3. Adding types without adding tests **Symptom**: Converting a .js script to .ts and calling it "modernized" **Fix**: Types catch shape errors. Tests catch logic errors. You need both. ### 4. Breaking the interface **Symptom**: Renaming flags, changing exit codes, or altering output format during a refactor **Fix**: Refactoring means behavior-preserving changes. If callers depend on the interface, maintain backward compatibility or coordinate the migration. ### 5. Ignoring the shebang **Symptom**: `#!/bin/bash` on a script that uses bash 4+ features, deployed to macOS (bash 3.2) **Fix**: Use `#!/usr/bin/env bash` and test on all target platforms. If you need bash 4+, document it. ### 6. Silent failures in pipelines **Symptom**: `cat file.txt | grep pattern | wc -l` where `file.txt` doesn't exist and the script continues **Fix**: `set -o pipefail` in bash. In Node.js, check `process.exitCode` after spawning child processes. ### 7. Mixing refactoring with feature additions **Symptom**: "While I'm in here, let me also add support for..." **Fix**: One commit for the refactor (behavior-preserving). A separate commit for new features. This keeps the refactor reviewable and revertable. ## Quality Checklist - [ ] Script has a clear, documented purpose (comment header or `--help`) - [ ] All arguments are validated with usage errors on bad input - [ ] `--help` flag works and shows all options - [ ] Exit codes follow conventions (0 = success, 1 = error, 2 = bad args) - [ ] No deprecated API calls remain - [ ] Error messages include context (what failed, what was expected) - [ ] Signals (SIGINT, SIGTERM) are handled with cleanup - [ ] Temporary files are cleaned up on exit (trap-based cleanup) - [ ] No hardcoded paths, credentials, or magic numbers - [ ] Variables are properly quoted (bash) or validated (Node.js/Python) - [ ] `set -euo pipefail` is present (bash scripts) - [ ] Callback chains have been converted to async/await where applicable - [ ] The script's external interface (flags, exit codes, output format) is unchanged unless intentionally migrated - [ ] A smoke test or example invocation is documented