{"id":"d89d7268-e3a3-4653-9f56-118c1ae1381b","entityType":"agent","slug":"clawhub-athola-nm-pensive-shell-review","name":"shell-review","canonicalUrl":"https://www.xpersona.co/agent/clawhub-athola-nm-pensive-shell-review","canonicalPath":"/agent/clawhub-athola-nm-pensive-shell-review","generatedAt":"2026-10-10T10:56:43.418Z","source":"CLAWHUB","claimStatus":"UNCLAIMED","verificationTier":"NONE","summary":{"evidence":{"source":"editorial-content","verified":true,"confidence":"high","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":null},"description":"Audits shell scripts for correctness, portability, and common pitfalls Skill: shell-review Owner: athola Summary: Audits shell scripts for correctness, portability, and common pitfalls Tags: latest:1.9.19 Version history: v1.9.19 | 2026-08-26T13:19:27.339Z | user Release v1.9.19 v1.9.17 | 2026-07-30T05:39:38.105Z | user Release v1.9.17 v1.9.16 | 2026-07-14T19:56:22.538Z | user Release v1.9.16 v1.9.14 | 2026-06-30T18:04:36.162Z | user Release v1.9.14 v1.9.13 | 2026-06-27T16:22:33.599Z |","descriptionLabel":"Technical summary","evidenceSummary":"Capability contract not published. No trust telemetry is available yet. 1.6K downloads reported by the source. Last updated 10/10/2026.","installCommand":"clawhub skill install s17emme0e2m3cpf7k2jvp3a84984b8z9:nm-pensive-shell-review","sourceUrl":"https://clawhub.ai/athola/nm-pensive-shell-review","homepage":"https://clawhub.ai/athola/skills/nm-pensive-shell-review","primaryLinks":[{"label":"View on ClawHub","url":"https://clawhub.ai/athola/nm-pensive-shell-review","kind":"source"},{"label":"Homepage","url":"https://clawhub.ai/athola/skills/nm-pensive-shell-review","kind":"homepage"}],"safetyScore":84,"overallRank":62,"popularityScore":64,"trustScore":null,"claimedByName":null,"isOwner":false,"seoDescription":"Audits shell scripts for correctness, portability, and common pitfalls Skill: shell-review Owner: athola Summary: Audits shell scripts for correctness, portabil"},"coverage":{"evidence":{"source":"public-profile","verified":false,"confidence":"medium","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":null},"protocols":[{"protocol":"OPENCLEW","label":"OpenClaw","status":"self-declared","notes":"Declared in the public agent profile."}],"capabilities":[],"verifiedCount":0,"selfDeclaredCount":1,"capabilityMatrix":{"rows":[{"key":"OPENCLEW","type":"protocol","support":"unknown","confidenceSource":"profile","notes":"Listed on profile"}],"flattenedTokens":"protocol:OPENCLEW|unknown|profile"}},"adoption":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"medium","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":null},"stars":null,"forks":null,"downloads":1589,"packageName":null,"latestVersion":"1.9.19","tractionLabel":"1.6K downloads"},"release":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"medium","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":null},"lastUpdatedAt":"2026-10-10T07:32:00.205Z","lastCrawledAt":"2026-10-10T07:32:00.205Z","lastIndexedAt":null,"nextCrawlAt":"2026-10-11T07:32:00.205Z","lastVerifiedAt":null,"highlights":[{"version":"1.9.19","createdAt":"2026-08-26T13:19:27.339Z","changelog":"Release v1.9.19","fileCount":7,"zipByteSize":11231},{"version":"1.9.17","createdAt":"2026-07-30T05:39:38.105Z","changelog":"Release v1.9.17","fileCount":7,"zipByteSize":11479},{"version":"1.9.16","createdAt":"2026-07-14T19:56:22.538Z","changelog":"Release v1.9.16","fileCount":7,"zipByteSize":11453},{"version":"1.9.14","createdAt":"2026-06-30T18:04:36.162Z","changelog":"Release v1.9.14","fileCount":7,"zipByteSize":11397},{"version":"1.9.13","createdAt":"2026-06-27T16:22:33.599Z","changelog":"Release v1.9.13","fileCount":7,"zipByteSize":11251},{"version":"1.9.12","createdAt":"2026-06-19T03:17:47.637Z","changelog":"Release v1.9.12","fileCount":7,"zipByteSize":11068},{"version":"1.0.3","createdAt":"2026-06-18T15:16:22.749Z","changelog":"Release v1.9.12","fileCount":7,"zipByteSize":11251},{"version":"1.0.2","createdAt":"2026-05-09T02:19:29.003Z","changelog":"Release v1.9.5","fileCount":6,"zipByteSize":7484}]},"execution":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"low","updatedAt":null,"emptyReason":"No published capability contract is available yet."},"installCommand":"clawhub skill install s17emme0e2m3cpf7k2jvp3a84984b8z9:nm-pensive-shell-review","setupComplexity":"low","setupSteps":["Setup complexity is classified as HIGH. You must provision dedicated cloud infrastructure or an isolated VM. Do not run this directly on your local workstation.","Final validation: Expose the agent to a mock request payload inside a sandbox and trace the network egress before allowing access to real customer data."],"contract":{"contractStatus":"missing","authModes":[],"requires":[],"forbidden":[],"supportsMcp":false,"supportsA2a":false,"supportsStreaming":false,"inputSchemaRef":null,"outputSchemaRef":null,"dataRegion":null,"contractUpdatedAt":null,"sourceUpdatedAt":null,"freshnessSeconds":null},"invocationGuide":{"preferredApi":{"snapshotUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/snapshot","contractUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/contract","trustUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/trust"},"curlExamples":["curl -s \"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/snapshot\"","curl -s \"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/contract\"","curl -s \"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/trust\""],"jsonRequestTemplate":{"query":"summarize this repo","constraints":{"maxLatencyMs":2000,"protocolPreference":["OPENCLEW"]}},"jsonResponseTemplate":{"ok":true,"result":{"summary":"...","confidence":0.9},"meta":{"source":"CLAWHUB","generatedAt":"2026-10-10T10:56:43.415Z"}},"retryPolicy":{"maxAttempts":3,"backoffMs":[500,1500,3500],"retryableConditions":["HTTP_429","HTTP_503","NETWORK_TIMEOUT"]}},"endpoints":{"dossierUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/dossier","snapshotUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/snapshot","contractUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/contract","trustUrl":"https://www.xpersona.co/api/v1/agents/clawhub-athola-nm-pensive-shell-review/trust"}},"reliability":{"evidence":{"source":"runtime-metrics","verified":false,"confidence":"low","updatedAt":null,"emptyReason":"No trust, reliability, or runtime telemetry is available."},"trust":{"status":"unavailable","handshakeStatus":"UNKNOWN","verificationFreshnessHours":null,"reputationScore":null,"p95LatencyMs":null,"successRate30d":null,"fallbackRate":null,"attempts30d":null,"trustUpdatedAt":null,"trustConfidence":"unknown","sourceUpdatedAt":null,"freshnessSeconds":null},"decisionGuardrails":{"doNotUseIf":["Contract metadata is missing or unavailable for deterministic execution."],"safeUseWhen":[],"riskFlags":["missing_or_unavailable_contract","trust_data_unavailable","schema_references_missing"],"operationalConfidence":"low"},"executionMetrics":{"observedLatencyMsP50":null,"observedLatencyMsP95":null,"estimatedCostUsd":null,"uptime30d":null,"rateLimitRpm":null,"rateLimitBurst":null,"lastVerifiedAt":null,"verificationSource":null},"runtimeMetrics":{"successRate":null,"avgLatencyMs":null,"avgCostUsd":null,"hallucinationRate":null,"retryRate":null,"disputeRate":null,"p50Latency":null,"p95Latency":null,"lastUpdated":null}},"benchmarks":{"evidence":{"source":"no-benchmark-data","verified":false,"confidence":"low","updatedAt":null,"emptyReason":"No benchmark suites or observed failure patterns are available."},"suites":[],"failurePatterns":[]},"artifacts":{"evidence":{"source":"CLAWHUB","verified":false,"confidence":"high","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":null},"readme":"Skill: shell-review\n\nOwner: athola\n\nSummary: Audits shell scripts for correctness, portability, and common pitfalls\n\nTags: latest:1.9.19\n\nVersion history:\n\nv1.9.19 | 2026-08-26T13:19:27.339Z | user\n\nRelease v1.9.19\n\nv1.9.17 | 2026-07-30T05:39:38.105Z | user\n\nRelease v1.9.17\n\nv1.9.16 | 2026-07-14T19:56:22.538Z | user\n\nRelease v1.9.16\n\nv1.9.14 | 2026-06-30T18:04:36.162Z | user\n\nRelease v1.9.14\n\nv1.9.13 | 2026-06-27T16:22:33.599Z | user\n\nRelease v1.9.13\n\nv1.9.12 | 2026-06-19T03:17:47.637Z | user\n\nRelease v1.9.12\n\nv1.0.3 | 2026-06-18T15:16:22.749Z | user\n\nRelease v1.9.12\n\nv1.0.2 | 2026-05-09T02:19:29.003Z | user\n\nRelease v1.9.5\n\nv1.0.1 | 2026-05-06T14:20:44.887Z | user\n\nRelease v1.9.4\n\nv1.0.0 | 2026-04-15T15:01:53.545Z | auto\n\n- Initial release of the shell-review skill: audit shell scripts for correctness, safety, and portability.\n- Provides a clear workflow for mapping context, checking exit codes, assessing portability, and verifying safety patterns.\n- Documents required TodoWrite items and structured output format for review findings.\n- Includes guidance for usage in CI/CD, pre-commit hooks, and automation scripts, with explicit usage and exclusion criteria.\n- Integrates proof-of-work evidence logging for auditability.\n\nArchive index:\n\nArchive v1.9.19: 7 files, 11231 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (1890b), SKILL.md (4009b), _meta.json (143b)\n\nFile v1.9.19:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (portability, maintainability)\n- Minor issues (style, documentation)\n\n## Output Format\n\n```markdown\n## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n\n## Exit Criteria\n\n- [ ] Exit code propagation verified (pipelines checked for pipefail or\n  capture-and-check)\n- [ ] Portability issues documented (Bash-isms in `#!/bin/sh` scripts flagged)\n- [ ] Safety patterns verified (no echo, braced vars, `:?` expansion, cd in\n  subshells, no basename/dirname)\n- [ ] Structure patterns verified (library/executable distinction, main call,\n  preamble, depcheck, shfmt formatting)\n- [ ] Evidence logged with file:line references via `imbue:proof-of-work`\n\nFile v1.9.19:_meta.json\n\n{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.19\",\n  \"publishedAt\": 1787750367339\n}\n\nFile v1.9.19:modules/exit-codes.md\n\n---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |\n\nFile v1.9.19:modules/portability.md\n\n---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent\n\nFile v1.9.19:modules/safety-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nDetection for non-canonical form:\n\n```sh\nrg -n '\\[ -[zn].*__\\w+_loaded' scripts/\n```\n\n## set -e / set -u in libraries\n\nFiles meant to be sourced must not enable `set -e` or `set -u`\nbecause the flags leak to the caller and can exit the caller's\nsession on unrelated commands.\n\nDetection:\n\n```sh\nrg -n '^set -[eu]' scripts/logging.sh\n```\n\n## printf over echo\n\nFor data output and multi-line messages, prefer `printf` with a\nfixed format string. Never build the format string from untrusted\ntext.\n\n```sh\n# Bad — echo interprets escape sequences inconsistently\necho \"Processing ${file}\"\n\n# Good — fixed format, no interpretation surprises\nprintf 'Processing %s\\n' \"${file}\"\n```\n\nFor logging through `log()`, pass the message as an argument.\n`log()` uses `printf` internally.\n\n## Checklist\n\n- [ ] No raw `echo` calls (use `log()` or `printf`)\n- [ ] All variables in braced form `${VAR}`\n- [ ] Unset required variables caught with `:?` expansion\n- [ ] Every `cd` is wrapped in a subshell\n- [ ] External files sourced via `${0%/*}` relative path\n- [ ] No `basename`/`dirname`; use param expansion\n- [ ] Library guard uses `case \"${__lib_loaded:-NULL}\" in`\n- [ ] Library scripts have no `set -e` or `set -u`\n\nFile v1.9.19:modules/structure-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: structure-patterns\ndescription: Library vs executable structure, main(), preamble, depcheck(), platform detection\ntags: [structure, main, library, depcheck, platform, readonly, shfmt]\n---\n\n# Shell Script Structure Patterns\n\n## Library vs executable\n\nA script is a **library** if it has no `main()` function (e.g.\n`scripts/logging.sh`). A script is an **executable** if it defines\n`main()` and ends with `main \"${@}\"`.\n\nLibraries signal their presence by setting a `__`-prefixed guard\nvariable (e.g. `__logging_loaded=1`). Callers verify it was sourced\nwith the canonical form:\n\n```sh\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nSee also: `safety-patterns.md` → \"Library loading check form\".\n\n| Property | Library | Executable |\n|----------|---------|------------|\n| Execute bit | No | Yes |\n| `main()` | No | Required |\n| Last line | — | `main \"${@}\"` |\n| `usage()` | Not needed | Recommended |\n| `set -e`/`set -u` | Never | Allowed |\n| `__`-prefixed globals | Yes (guards) | Allowed |\n\nDetection: library with execute bit\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  rg -q 'main\\(\\)' \"${f}\" || printf 'Library with +x: %s\\n' \"${f}\"\ndone\n```\n\nDetection: executable missing `main \"${@}\"` as last line\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  last=\"$(tail -1 \"${f}\")\"\n  case \"${last}\" in\n    'main \"${@}\"') : ;;\n    *) printf 'Missing main call: %s\\n' \"${f}\" ;;\n  esac\ndone\n```\n\n## Preamble for executable scripts\n\nEvery executable script must start with this preamble (using\n`scripts/shellcheck.sh` as the canonical example):\n\n```sh\n#!/bin/sh\nset -eu\n\nMYDIR=\"${0%/*}\"\nreadonly MYDIR\n\n# shellcheck source=scripts/logging.sh\n. \"${MYDIR%/}/logging.sh\"\n```\n\n- `#!/bin/sh`: POSIX dialect; no Bash extensions\n- `set -eu`: exit on error (`-e`), error on unset (`-u`)\n- `MYDIR=\"${0%/*}\"`: script directory without `dirname`\n- `readonly MYDIR`: marks the variable immutable\n- `# shellcheck source=…`: lets shellcheck follow the source\n- `. \"${MYDIR%/}/logging.sh\"`: loads `log()` and `banner()`\n\n## All functionality in functions; no top-down execution\n\nScripts must never execute logic at top level. Every statement\nbelongs inside a named function. The only top-level calls are:\n\n1. `set -eu` (preamble)\n2. Variable declarations (`readonly`, assignments)\n3. Source statements (`. lib.sh`)\n4. `main \"${@}\"` on the last line\n\nDetection:\n\n```sh\n# Top-level commands outside function definitions\n# (rough heuristic — awk parses function body depth)\nawk '/^[a-z_][a-z_0-9]*\\(\\)/{depth++} /^\\}/{depth--}\n     depth==0 && /^\\s*[a-z]/ && !/^(readonly|MYDIR|LOG|\\.|\\s*#)/{print NR\": \"$0}' script.sh\n```\n\n## depcheck() for external dependencies\n\nAny script relying on tools beyond POSIX must define `depcheck()`.\nRequired tools use `log 5` (critical); optional tools use `log 3`\n(notice). Dependency lists allow the check logic to stay unchanged\nwhen tools are added.\n\n```sh\nREQUIRED_DEPENDENCIES=\"shellcheck shfmt\"\n\ndepcheck() {\n  _dc_missing=\"\"\n  for _dc_util in ${REQUIRED_DEPENDENCIES}; do\n    command -v \"${_dc_util}\" >/dev/null 2>&1 ||\n      _dc_missing=\"${_dc_missing:+\"${_dc_missing} \"}${_dc_util}\"\n  done\n  case \"${#_dc_missing}\" in\n    0) return 0 ;;\n  esac\n  log 5 \"Required utilities not found: ${_dc_missing}\"\n  return 1\n}\n```\n\nBuilding `_dc_missing` with `${_dc_missing:+\"${_dc_missing} \"}${_dc_util}`\nis the POSIX way to append a space-separated word without leaving a\nleading space. It avoids arrays, which are a Bash extension.\n\n## usage() function\n\nScripts that accept flags must define `usage()`. The first output\nline must use `log \"Usage: …\"`. Subsequent lines use `printf`.\n\n```sh\nusage() {\n  log \"Usage: scripts/myscript.sh [-h] [-x|-t] [ARGS]\"\n  printf '  -h       Show this help and exit (exit 0)\\n'\n  printf '  -x, -t   Enable xtrace for debugging\\n'\n  printf '  ARGS     Files or patterns to process\\n'\n}\n```\n\nThe `usage` and `help` case patterns must accept both spellings and\nany case:\n\n```sh\ncase \"${1}\" in\n  *[uU][sS][aA][gG][eE] | *[hH][eE][lL][pP] | -h)\n    usage\n    exit 0\n    ;;\nesac\n```\n\n## xtrace support (-x / -t flags)\n\nEvery executable script must support a flag to enable `xtrace`\nfor debugging. The preferred flags are `-x` and `-t`.\n\n```sh\nXTRACE=0\n\n# …inside main() after arg parsing:\ncase \"${XTRACE}\" in\n  1) set -x ;;\nesac\n```\n\n## readonly for non-modified globals\n\nGlobal variables that do not change during execution must be\nmarked `readonly`. Group these declarations near the top of the\nscript, after the preamble.\n\n```sh\nreadonly MYDIR\nreadonly VERSION=\"1.0.0\"\nreadonly CONFIG_FILE=\"${MYDIR%/}/../.config\"\n```\n\n## Platform detection\n\nWhen commands differ by OS, assign the command and its arguments\nto separate variables using `uname -s` and distribution files.\n\n```sh\nKERNEL=\"$(uname -s)\"\ncase \"${KERNEL}\" in\n  *BSD | [Ll]inux)\n    . /etc/os-release\n    case \"${ID}\" in\n      freebsd) INSTALLER=\"pkg\"; INSTALLER_ARG=\"install\" ;;\n      ubuntu | debian) INSTALLER=\"apt-get\"; INSTALLER_ARG=\"install -yqq\" ;;\n      alpine) INSTALLER=\"apk\"; INSTALLER_ARG=\"add\" ;;\n      fedora | centos | rhel)\n        for _pm_cmd in dnf yum; do\n          command -v \"${_pm_cmd}\" >/dev/null 2>&1 && INSTALLER=\"${_pm_cmd}\"\n        done\n        INSTALLER_ARG=\"install\"\n        ;;\n    esac\n    ;;\n  Darwin)\n    INSTALLER=\"brew\"\n    INSTALLER_ARG=\"install\"\n    ;;\nesac\n\n\"${INSTALLER:?No known installer selected}\" ${INSTALLER_ARG} \"${PACKAGES}\"\n```\n\nNote: `${INSTALLER_ARG}` is intentionally unquoted here so its\nspace-separated arguments expand into multiple words.\n\n## case over test / [ ]\n\nPrefer `case` statements over `test`/`[ ]` for branching. `case`\nis faster (no subprocess), cleaner, and handles patterns natively.\n\n```sh\n# Preferred\ncase \"${answer}\" in\n  [yY] | [yY][eE][sS]) confirm ;;\n  *) abort ;;\nesac\n\n# Avoid\nif [ \"${answer}\" = \"y\" ] || [ \"${answer}\" = \"Y\" ]; then\n  confirm\nfi\n```\n\n## Formatting: shfmt -p -i 2 -ci\n\nAll scripts must be formatted with:\n\n```sh\nshfmt -p -i 2 -ci -w script.sh\n```\n\n- `-p` POSIX mode (no Bash extensions)\n- `-i 2` two-space indent\n- `-ci` indent `case` label bodies\n\nRun from the repository root:\n\n```sh\n# Check all scripts\nshfmt -p -i 2 -ci -d scripts/\n\n# Apply formatting in-place\nshfmt -p -i 2 -ci -w scripts/*.sh\n```\n\n## Checklist\n\n- [ ] Library: no execute bit, no `main()`, `__`-guard present\n- [ ] Executable: starts with preamble, ends with `main \"${@}\"`\n- [ ] No top-level logic (only declarations, source, `main \"${@}\"`)\n- [ ] `depcheck()` present when external tools are required\n- [ ] `usage()` present and accepts `-h` / `usage`/`help` variants\n- [ ] `-x`/`-t` flags supported and enable xtrace\n- [ ] Non-modified globals are `readonly`\n- [ ] Platform branching uses `uname -s` + INSTALLER pattern\n- [ ] `case` used instead of `[ ]` for branching\n- [ ] `shfmt -p -i 2 -ci -d` reports no diff\n\nFile v1.9.19:skill-card.md\n\n## Description:\n\nAudits shell scripts for correctness, portability, and common pitfalls.\n\nThis skill is ready for commercial/non-commercial use.\n\n## Publisher:\n\n[athola](https://clawhub.ai/user/athola)\n\n### License/Terms of Use:\n\nMIT-0\n\n## Use Case:\n\nDevelopers and engineers use this skill to review shell scripts used in CI/CD, hooks, wrappers, build automation, and pre-commit workflows for exit-code handling, portability, safety, and structure issues.\n\n### Deployment Geography for Use:\n\nGlobal\n\n## Known Risks and Mitigations:\n\nRisk: Broad shell-related triggers may activate the skill in contexts where script review was not intended.\n\nMitigation: Install it only for agents expected to review shell scripts, and confirm activation context before applying recommendations.\n\nRisk: Package-manager and xtrace snippets could be copied into CI or secret-handling scripts without enough review.\n\nMitigation: Treat snippets as review examples; inspect and adapt commands before use, and avoid xtrace where secrets may appear in logs.\n\n## Reference(s):\n\n- [ClawHub skill page](https://clawhub.ai/athola/skills/nm-pensive-shell-review)\n- [ClawHub metadata homepage](https://github.com/athola/claude-night-market/tree/master/plugins/pensive)\n\n## Skill Output:\n\n**Output Type(s):** [markdown, shell commands, guidance]\n\n**Output Format:** [Markdown review report with shell command examples and file:line findings]\n\n**Output Parameters:** [1D]\n\n**Other Properties Related to Output:** [Produces a recommendation such as Approve, Approve with actions, or Block.]\n\n## Skill Version(s):\n\n1.9.19 (source: server release evidence)\n\n## Ethical Considerations:\n\nUsers should evaluate whether this skill is appropriate for their environment, review any generated or modified files before relying on them, and apply their organization's safety, security, and compliance requirements before deployment.\n\nArchive v1.9.17: 7 files, 11479 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (2605b), SKILL.md (4009b), _meta.json (143b)\n\nFile v1.9.17:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (portability, maintainability)\n- Minor issues (style, documentation)\n\n## Output Format\n\n```markdown\n## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n\n## Exit Criteria\n\n- [ ] Exit code propagation verified (pipelines checked for pipefail or\n  capture-and-check)\n- [ ] Portability issues documented (Bash-isms in `#!/bin/sh` scripts flagged)\n- [ ] Safety patterns verified (no echo, braced vars, `:?` expansion, cd in\n  subshells, no basename/dirname)\n- [ ] Structure patterns verified (library/executable distinction, main call,\n  preamble, depcheck, shfmt formatting)\n- [ ] Evidence logged with file:line references via `imbue:proof-of-work`\n\nFile v1.9.17:_meta.json\n\n{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.17\",\n  \"publishedAt\": 1785389978105\n}\n\nFile v1.9.17:modules/exit-codes.md\n\n---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |\n\nFile v1.9.17:modules/portability.md\n\n---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent\n\nFile v1.9.17:modules/safety-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nDetection for non-canonical form:\n\n```sh\nrg -n '\\[ -[zn].*__\\w+_loaded' scripts/\n```\n\n## set -e / set -u in libraries\n\nFiles meant to be sourced must not enable `set -e` or `set -u`\nbecause the flags leak to the caller and can exit the caller's\nsession on unrelated commands.\n\nDetection:\n\n```sh\nrg -n '^set -[eu]' scripts/logging.sh\n```\n\n## printf over echo\n\nFor data output and multi-line messages, prefer `printf` with a\nfixed format string. Never build the format string from untrusted\ntext.\n\n```sh\n# Bad — echo interprets escape sequences inconsistently\necho \"Processing ${file}\"\n\n# Good — fixed format, no interpretation surprises\nprintf 'Processing %s\\n' \"${file}\"\n```\n\nFor logging through `log()`, pass the message as an argument.\n`log()` uses `printf` internally.\n\n## Checklist\n\n- [ ] No raw `echo` calls (use `log()` or `printf`)\n- [ ] All variables in braced form `${VAR}`\n- [ ] Unset required variables caught with `:?` expansion\n- [ ] Every `cd` is wrapped in a subshell\n- [ ] External files sourced via `${0%/*}` relative path\n- [ ] No `basename`/`dirname`; use param expansion\n- [ ] Library guard uses `case \"${__lib_loaded:-NULL}\" in`\n- [ ] Library scripts have no `set -e` or `set -u`\n\nFile v1.9.17:modules/structure-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: structure-patterns\ndescription: Library vs executable structure, main(), preamble, depcheck(), platform detection\ntags: [structure, main, library, depcheck, platform, readonly, shfmt]\n---\n\n# Shell Script Structure Patterns\n\n## Library vs executable\n\nA script is a **library** if it has no `main()` function (e.g.\n`scripts/logging.sh`). A script is an **executable** if it defines\n`main()` and ends with `main \"${@}\"`.\n\nLibraries signal their presence by setting a `__`-prefixed guard\nvariable (e.g. `__logging_loaded=1`). Callers verify it was sourced\nwith the canonical form:\n\n```sh\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nSee also: `safety-patterns.md` → \"Library loading check form\".\n\n| Property | Library | Executable |\n|----------|---------|------------|\n| Execute bit | No | Yes |\n| `main()` | No | Required |\n| Last line | — | `main \"${@}\"` |\n| `usage()` | Not needed | Recommended |\n| `set -e`/`set -u` | Never | Allowed |\n| `__`-prefixed globals | Yes (guards) | Allowed |\n\nDetection: library with execute bit\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  rg -q 'main\\(\\)' \"${f}\" || printf 'Library with +x: %s\\n' \"${f}\"\ndone\n```\n\nDetection: executable missing `main \"${@}\"` as last line\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  last=\"$(tail -1 \"${f}\")\"\n  case \"${last}\" in\n    'main \"${@}\"') : ;;\n    *) printf 'Missing main call: %s\\n' \"${f}\" ;;\n  esac\ndone\n```\n\n## Preamble for executable scripts\n\nEvery executable script must start with this preamble (using\n`scripts/shellcheck.sh` as the canonical example):\n\n```sh\n#!/bin/sh\nset -eu\n\nMYDIR=\"${0%/*}\"\nreadonly MYDIR\n\n# shellcheck source=scripts/logging.sh\n. \"${MYDIR%/}/logging.sh\"\n```\n\n- `#!/bin/sh`: POSIX dialect; no Bash extensions\n- `set -eu`: exit on error (`-e`), error on unset (`-u`)\n- `MYDIR=\"${0%/*}\"`: script directory without `dirname`\n- `readonly MYDIR`: marks the variable immutable\n- `# shellcheck source=…`: lets shellcheck follow the source\n- `. \"${MYDIR%/}/logging.sh\"`: loads `log()` and `banner()`\n\n## All functionality in functions; no top-down execution\n\nScripts must never execute logic at top level. Every statement\nbelongs inside a named function. The only top-level calls are:\n\n1. `set -eu` (preamble)\n2. Variable declarations (`readonly`, assignments)\n3. Source statements (`. lib.sh`)\n4. `main \"${@}\"` on the last line\n\nDetection:\n\n```sh\n# Top-level commands outside function definitions\n# (rough heuristic — awk parses function body depth)\nawk '/^[a-z_][a-z_0-9]*\\(\\)/{depth++} /^\\}/{depth--}\n     depth==0 && /^\\s*[a-z]/ && !/^(readonly|MYDIR|LOG|\\.|\\s*#)/{print NR\": \"$0}' script.sh\n```\n\n## depcheck() for external dependencies\n\nAny script relying on tools beyond POSIX must define `depcheck()`.\nRequired tools use `log 5` (critical); optional tools use `log 3`\n(notice). Dependency lists allow the check logic to stay unchanged\nwhen tools are added.\n\n```sh\nREQUIRED_DEPENDENCIES=\"shellcheck shfmt\"\n\ndepcheck() {\n  _dc_missing=\"\"\n  for _dc_util in ${REQUIRED_DEPENDENCIES}; do\n    command -v \"${_dc_util}\" >/dev/null 2>&1 ||\n      _dc_missing=\"${_dc_missing:+\"${_dc_missing} \"}${_dc_util}\"\n  done\n  case \"${#_dc_missing}\" in\n    0) return 0 ;;\n  esac\n  log 5 \"Required utilities not found: ${_dc_missing}\"\n  return 1\n}\n```\n\nBuilding `_dc_missing` with `${_dc_missing:+\"${_dc_missing} \"}${_dc_util}`\nis the POSIX way to append a space-separated word without leaving a\nleading space. It avoids arrays, which are a Bash extension.\n\n## usage() function\n\nScripts that accept flags must define `usage()`. The first output\nline must use `log \"Usage: …\"`. Subsequent lines use `printf`.\n\n```sh\nusage() {\n  log \"Usage: scripts/myscript.sh [-h] [-x|-t] [ARGS]\"\n  printf '  -h       Show this help and exit (exit 0)\\n'\n  printf '  -x, -t   Enable xtrace for debugging\\n'\n  printf '  ARGS     Files or patterns to process\\n'\n}\n```\n\nThe `usage` and `help` case patterns must accept both spellings and\nany case:\n\n```sh\ncase \"${1}\" in\n  *[uU][sS][aA][gG][eE] | *[hH][eE][lL][pP] | -h)\n    usage\n    exit 0\n    ;;\nesac\n```\n\n## xtrace support (-x / -t flags)\n\nEvery executable script must support a flag to enable `xtrace`\nfor debugging. The preferred flags are `-x` and `-t`.\n\n```sh\nXTRACE=0\n\n# …inside main() after arg parsing:\ncase \"${XTRACE}\" in\n  1) set -x ;;\nesac\n```\n\n## readonly for non-modified globals\n\nGlobal variables that do not change during execution must be\nmarked `readonly`. Group these declarations near the top of the\nscript, after the preamble.\n\n```sh\nreadonly MYDIR\nreadonly VERSION=\"1.0.0\"\nreadonly CONFIG_FILE=\"${MYDIR%/}/../.config\"\n```\n\n## Platform detection\n\nWhen commands differ by OS, assign the command and its arguments\nto separate variables using `uname -s` and distribution files.\n\n```sh\nKERNEL=\"$(uname -s)\"\ncase \"${KERNEL}\" in\n  *BSD | [Ll]inux)\n    . /etc/os-release\n    case \"${ID}\" in\n      freebsd) INSTALLER=\"pkg\"; INSTALLER_ARG=\"install\" ;;\n      ubuntu | debian) INSTALLER=\"apt-get\"; INSTALLER_ARG=\"install -yqq\" ;;\n      alpine) INSTALLER=\"apk\"; INSTALLER_ARG=\"add\" ;;\n      fedora | centos | rhel)\n        for _pm_cmd in dnf yum; do\n          command -v \"${_pm_cmd}\" >/dev/null 2>&1 && INSTALLER=\"${_pm_cmd}\"\n        done\n        INSTALLER_ARG=\"install\"\n        ;;\n    esac\n    ;;\n  Darwin)\n    INSTALLER=\"brew\"\n    INSTALLER_ARG=\"install\"\n    ;;\nesac\n\n\"${INSTALLER:?No known installer selected}\" ${INSTALLER_ARG} \"${PACKAGES}\"\n```\n\nNote: `${INSTALLER_ARG}` is intentionally unquoted here so its\nspace-separated arguments expand into multiple words.\n\n## case over test / [ ]\n\nPrefer `case` statements over `test`/`[ ]` for branching. `case`\nis faster (no subprocess), cleaner, and handles patterns natively.\n\n```sh\n# Preferred\ncase \"${answer}\" in\n  [yY] | [yY][eE][sS]) confirm ;;\n  *) abort ;;\nesac\n\n# Avoid\nif [ \"${answer}\" = \"y\" ] || [ \"${answer}\" = \"Y\" ]; then\n  confirm\nfi\n```\n\n## Formatting: shfmt -p -i 2 -ci\n\nAll scripts must be formatted with:\n\n```sh\nshfmt -p -i 2 -ci -w script.sh\n```\n\n- `-p` POSIX mode (no Bash extensions)\n- `-i 2` two-space indent\n- `-ci` indent `case` label bodies\n\nRun from the repository root:\n\n```sh\n# Check all scripts\nshfmt -p -i 2 -ci -d scripts/\n\n# Apply formatting in-place\nshfmt -p -i 2 -ci -w scripts/*.sh\n```\n\n## Checklist\n\n- [ ] Library: no execute bit, no `main()`, `__`-guard present\n- [ ] Executable: starts with preamble, ends with `main \"${@}\"`\n- [ ] No top-level logic (only declarations, source, `main \"${@}\"`)\n- [ ] `depcheck()` present when external tools are required\n- [ ] `usage()` present and accepts `-h` / `usage`/`help` variants\n- [ ] `-x`/`-t` flags supported and enable xtrace\n- [ ] Non-modified globals are `readonly`\n- [ ] Platform branching uses `uname -s` + INSTALLER pattern\n- [ ] `case` used instead of `[ ]` for branching\n- [ ] `shfmt -p -i 2 -ci -d` reports no diff\n\nFile v1.9.17:skill-card.md\n\n## Description: <br>\nAudits shell scripts for correctness, portability, and common pitfalls. <br>\n\nThis skill is ready for commercial/non-commercial use. <br>\n\n## Publisher: <br>\n[athola](https://clawhub.ai/user/athola) <br>\n\n### License/Terms of Use: <br>\nMIT-0 <br>\n\n\n## Use Case: <br>\nDevelopers and engineers use this skill to review shell, Bash, POSIX, CI, hook, wrapper, and build scripts before committing or shipping changes. It focuses the review on exit-code handling, portability, safety patterns, structure, and evidence-backed findings. <br>\n\n### Deployment Geography for Use: <br>\nGlobal <br>\n\n## Known Risks and Mitigations: <br>\nRisk: The skill includes shell and package-manager command examples that could be mistaken for commands to run automatically. <br>\nMitigation: Treat command snippets as review aids and approve any actual command execution separately. <br>\nRisk: Some review guidance references format-changing commands such as shfmt -w that can modify files. <br>\nMitigation: Review proposed write operations before execution and prefer dry runs or diffs when available. <br>\nRisk: Shell review guidance can miss project-specific behavior or test expectations. <br>\nMitigation: Confirm findings against file:line evidence, run shellcheck where applicable, and use the project's existing tests before accepting changes. <br>\n\n\n## Reference(s): <br>\n- [ClawHub skill page](https://clawhub.ai/athola/skills/nm-pensive-shell-review) <br>\n- [Pensive plugin homepage](https://github.com/athola/claude-night-market/tree/master/plugins/pensive) <br>\n- [Exit code patterns](modules/exit-codes.md) <br>\n- [Shell portability](modules/portability.md) <br>\n- [Shell safety patterns](modules/safety-patterns.md) <br>\n- [Shell structure patterns](modules/structure-patterns.md) <br>\n\n\n## Skill Output: <br>\n**Output Type(s):** [text, markdown, code, shell commands, guidance] <br>\n**Output Format:** [Markdown review report with findings, file references, command snippets, suggested fixes, and an approval recommendation.] <br>\n**Output Parameters:** [1D] <br>\n**Other Properties Related to Output:** [The skill produces review guidance and proposed changes; it does not need to write files itself.] <br>\n\n## Skill Version(s): <br>\n1.9.17 (source: ClawHub release evidence; artifact frontmatter reports 1.9.8) <br>\n\n## Ethical Considerations: <br>\nUsers should evaluate whether this skill is appropriate for their environment, review any generated or modified files before relying on them, and apply their organization's safety, security, and compliance requirements before deployment. <br>\n\nArchive v1.9.16: 7 files, 11453 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (2590b), SKILL.md (4009b), _meta.json (143b)\n\nFile v1.9.16:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (portability, maintainability)\n- Minor issues (style, documentation)\n\n## Output Format\n\n```markdown\n## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n\n## Exit Criteria\n\n- [ ] Exit code propagation verified (pipelines checked for pipefail or\n  capture-and-check)\n- [ ] Portability issues documented (Bash-isms in `#!/bin/sh` scripts flagged)\n- [ ] Safety patterns verified (no echo, braced vars, `:?` expansion, cd in\n  subshells, no basename/dirname)\n- [ ] Structure patterns verified (library/executable distinction, main call,\n  preamble, depcheck, shfmt formatting)\n- [ ] Evidence logged with file:line references via `imbue:proof-of-work`\n\nFile v1.9.16:_meta.json\n\n{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.16\",\n  \"publishedAt\": 1784058982538\n}\n\nFile v1.9.16:modules/exit-codes.md\n\n---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |\n\nFile v1.9.16:modules/portability.md\n\n---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent\n\nFile v1.9.16:modules/safety-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nDetection for non-canonical form:\n\n```sh\nrg -n '\\[ -[zn].*__\\w+_loaded' scripts/\n```\n\n## set -e / set -u in libraries\n\nFiles meant to be sourced must not enable `set -e` or `set -u`\nbecause the flags leak to the caller and can exit the caller's\nsession on unrelated commands.\n\nDetection:\n\n```sh\nrg -n '^set -[eu]' scripts/logging.sh\n```\n\n## printf over echo\n\nFor data output and multi-line messages, prefer `printf` with a\nfixed format string. Never build the format string from untrusted\ntext.\n\n```sh\n# Bad — echo interprets escape sequences inconsistently\necho \"Processing ${file}\"\n\n# Good — fixed format, no interpretation surprises\nprintf 'Processing %s\\n' \"${file}\"\n```\n\nFor logging through `log()`, pass the message as an argument.\n`log()` uses `printf` internally.\n\n## Checklist\n\n- [ ] No raw `echo` calls (use `log()` or `printf`)\n- [ ] All variables in braced form `${VAR}`\n- [ ] Unset required variables caught with `:?` expansion\n- [ ] Every `cd` is wrapped in a subshell\n- [ ] External files sourced via `${0%/*}` relative path\n- [ ] No `basename`/`dirname`; use param expansion\n- [ ] Library guard uses `case \"${__lib_loaded:-NULL}\" in`\n- [ ] Library scripts have no `set -e` or `set -u`\n\nFile v1.9.16:modules/structure-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: structure-patterns\ndescription: Library vs executable structure, main(), preamble, depcheck(), platform detection\ntags: [structure, main, library, depcheck, platform, readonly, shfmt]\n---\n\n# Shell Script Structure Patterns\n\n## Library vs executable\n\nA script is a **library** if it has no `main()` function (e.g.\n`scripts/logging.sh`). A script is an **executable** if it defines\n`main()` and ends with `main \"${@}\"`.\n\nLibraries signal their presence by setting a `__`-prefixed guard\nvariable (e.g. `__logging_loaded=1`). Callers verify it was sourced\nwith the canonical form:\n\n```sh\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nSee also: `safety-patterns.md` → \"Library loading check form\".\n\n| Property | Library | Executable |\n|----------|---------|------------|\n| Execute bit | No | Yes |\n| `main()` | No | Required |\n| Last line | — | `main \"${@}\"` |\n| `usage()` | Not needed | Recommended |\n| `set -e`/`set -u` | Never | Allowed |\n| `__`-prefixed globals | Yes (guards) | Allowed |\n\nDetection: library with execute bit\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  rg -q 'main\\(\\)' \"${f}\" || printf 'Library with +x: %s\\n' \"${f}\"\ndone\n```\n\nDetection: executable missing `main \"${@}\"` as last line\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  last=\"$(tail -1 \"${f}\")\"\n  case \"${last}\" in\n    'main \"${@}\"') : ;;\n    *) printf 'Missing main call: %s\\n' \"${f}\" ;;\n  esac\ndone\n```\n\n## Preamble for executable scripts\n\nEvery executable script must start with this preamble (using\n`scripts/shellcheck.sh` as the canonical example):\n\n```sh\n#!/bin/sh\nset -eu\n\nMYDIR=\"${0%/*}\"\nreadonly MYDIR\n\n# shellcheck source=scripts/logging.sh\n. \"${MYDIR%/}/logging.sh\"\n```\n\n- `#!/bin/sh`: POSIX dialect; no Bash extensions\n- `set -eu`: exit on error (`-e`), error on unset (`-u`)\n- `MYDIR=\"${0%/*}\"`: script directory without `dirname`\n- `readonly MYDIR`: marks the variable immutable\n- `# shellcheck source=…`: lets shellcheck follow the source\n- `. \"${MYDIR%/}/logging.sh\"`: loads `log()` and `banner()`\n\n## All functionality in functions; no top-down execution\n\nScripts must never execute logic at top level. Every statement\nbelongs inside a named function. The only top-level calls are:\n\n1. `set -eu` (preamble)\n2. Variable declarations (`readonly`, assignments)\n3. Source statements (`. lib.sh`)\n4. `main \"${@}\"` on the last line\n\nDetection:\n\n```sh\n# Top-level commands outside function definitions\n# (rough heuristic — awk parses function body depth)\nawk '/^[a-z_][a-z_0-9]*\\(\\)/{depth++} /^\\}/{depth--}\n     depth==0 && /^\\s*[a-z]/ && !/^(readonly|MYDIR|LOG|\\.|\\s*#)/{print NR\": \"$0}' script.sh\n```\n\n## depcheck() for external dependencies\n\nAny script relying on tools beyond POSIX must define `depcheck()`.\nRequired tools use `log 5` (critical); optional tools use `log 3`\n(notice). Dependency lists allow the check logic to stay unchanged\nwhen tools are added.\n\n```sh\nREQUIRED_DEPENDENCIES=\"shellcheck shfmt\"\n\ndepcheck() {\n  _dc_missing=\"\"\n  for _dc_util in ${REQUIRED_DEPENDENCIES}; do\n    command -v \"${_dc_util}\" >/dev/null 2>&1 ||\n      _dc_missing=\"${_dc_missing:+\"${_dc_missing} \"}${_dc_util}\"\n  done\n  case \"${#_dc_missing}\" in\n    0) return 0 ;;\n  esac\n  log 5 \"Required utilities not found: ${_dc_missing}\"\n  return 1\n}\n```\n\nBuilding `_dc_missing` with `${_dc_missing:+\"${_dc_missing} \"}${_dc_util}`\nis the POSIX way to append a space-separated word without leaving a\nleading space. It avoids arrays, which are a Bash extension.\n\n## usage() function\n\nScripts that accept flags must define `usage()`. The first output\nline must use `log \"Usage: …\"`. Subsequent lines use `printf`.\n\n```sh\nusage() {\n  log \"Usage: scripts/myscript.sh [-h] [-x|-t] [ARGS]\"\n  printf '  -h       Show this help and exit (exit 0)\\n'\n  printf '  -x, -t   Enable xtrace for debugging\\n'\n  printf '  ARGS     Files or patterns to process\\n'\n}\n```\n\nThe `usage` and `help` case patterns must accept both spellings and\nany case:\n\n```sh\ncase \"${1}\" in\n  *[uU][sS][aA][gG][eE] | *[hH][eE][lL][pP] | -h)\n    usage\n    exit 0\n    ;;\nesac\n```\n\n## xtrace support (-x / -t flags)\n\nEvery executable script must support a flag to enable `xtrace`\nfor debugging. The preferred flags are `-x` and `-t`.\n\n```sh\nXTRACE=0\n\n# …inside main() after arg parsing:\ncase \"${XTRACE}\" in\n  1) set -x ;;\nesac\n```\n\n## readonly for non-modified globals\n\nGlobal variables that do not change during execution must be\nmarked `readonly`. Group these declarations near the top of the\nscript, after the preamble.\n\n```sh\nreadonly MYDIR\nreadonly VERSION=\"1.0.0\"\nreadonly CONFIG_FILE=\"${MYDIR%/}/../.config\"\n```\n\n## Platform detection\n\nWhen commands differ by OS, assign the command and its arguments\nto separate variables using `uname -s` and distribution files.\n\n```sh\nKERNEL=\"$(uname -s)\"\ncase \"${KERNEL}\" in\n  *BSD | [Ll]inux)\n    . /etc/os-release\n    case \"${ID}\" in\n      freebsd) INSTALLER=\"pkg\"; INSTALLER_ARG=\"install\" ;;\n      ubuntu | debian) INSTALLER=\"apt-get\"; INSTALLER_ARG=\"install -yqq\" ;;\n      alpine) INSTALLER=\"apk\"; INSTALLER_ARG=\"add\" ;;\n      fedora | centos | rhel)\n        for _pm_cmd in dnf yum; do\n          command -v \"${_pm_cmd}\" >/dev/null 2>&1 && INSTALLER=\"${_pm_cmd}\"\n        done\n        INSTALLER_ARG=\"install\"\n        ;;\n    esac\n    ;;\n  Darwin)\n    INSTALLER=\"brew\"\n    INSTALLER_ARG=\"install\"\n    ;;\nesac\n\n\"${INSTALLER:?No known installer selected}\" ${INSTALLER_ARG} \"${PACKAGES}\"\n```\n\nNote: `${INSTALLER_ARG}` is intentionally unquoted here so its\nspace-separated arguments expand into multiple words.\n\n## case over test / [ ]\n\nPrefer `case` statements over `test`/`[ ]` for branching. `case`\nis faster (no subprocess), cleaner, and handles patterns natively.\n\n```sh\n# Preferred\ncase \"${answer}\" in\n  [yY] | [yY][eE][sS]) confirm ;;\n  *) abort ;;\nesac\n\n# Avoid\nif [ \"${answer}\" = \"y\" ] || [ \"${answer}\" = \"Y\" ]; then\n  confirm\nfi\n```\n\n## Formatting: shfmt -p -i 2 -ci\n\nAll scripts must be formatted with:\n\n```sh\nshfmt -p -i 2 -ci -w script.sh\n```\n\n- `-p` POSIX mode (no Bash extensions)\n- `-i 2` two-space indent\n- `-ci` indent `case` label bodies\n\nRun from the repository root:\n\n```sh\n# Check all scripts\nshfmt -p -i 2 -ci -d scripts/\n\n# Apply formatting in-place\nshfmt -p -i 2 -ci -w scripts/*.sh\n```\n\n## Checklist\n\n- [ ] Library: no execute bit, no `main()`, `__`-guard present\n- [ ] Executable: starts with preamble, ends with `main \"${@}\"`\n- [ ] No top-level logic (only declarations, source, `main \"${@}\"`)\n- [ ] `depcheck()` present when external tools are required\n- [ ] `usage()` present and accepts `-h` / `usage`/`help` variants\n- [ ] `-x`/`-t` flags supported and enable xtrace\n- [ ] Non-modified globals are `readonly`\n- [ ] Platform branching uses `uname -s` + INSTALLER pattern\n- [ ] `case` used instead of `[ ]` for branching\n- [ ] `shfmt -p -i 2 -ci -d` reports no diff\n\nFile v1.9.16:skill-card.md\n\n## Description: <br>\nAudits shell scripts for correctness, portability, and common pitfalls. <br>\n\nThis skill is ready for commercial/non-commercial use. <br>\n\n## Publisher: <br>\n[athola](https://clawhub.ai/user/athola) <br>\n\n### License/Terms of Use: <br>\nMIT-0 <br>\n\n\n## Use Case: <br>\nDevelopers and engineers use this skill to review shell scripts used in CI/CD pipelines, hooks, wrappers, build automation, and pre-commit workflows. It focuses the review on exit-code propagation, portability, safety patterns, script structure, and evidence-backed findings. <br>\n\n### Deployment Geography for Use: <br>\nGlobal <br>\n\n## Known Risks and Mitigations: <br>\nRisk: Broad shell and CI activation terms may cause the skill to be invoked during general shell-script discussions. <br>\nMitigation: Confirm the review target and scope before applying recommendations. <br>\nRisk: The skill may suggest copyable commands, including commands that format files in place or invoke package managers. <br>\nMitigation: Inspect commands before running them and prefer read-only checks or dry runs before applying changes. <br>\nRisk: Review guidance may be incomplete or incorrect for a specific repository or shell dialect. <br>\nMitigation: Validate findings with ShellCheck, relevant tests, and human review before relying on the recommendation. <br>\n\n\n## Reference(s): <br>\n- [ClawHub skill page](https://clawhub.ai/athola/skills/nm-pensive-shell-review) <br>\n- [Pensive source homepage](https://github.com/athola/claude-night-market/tree/master/plugins/pensive) <br>\n- [Exit Code Patterns](modules/exit-codes.md) <br>\n- [Shell Portability](modules/portability.md) <br>\n- [Shell Safety Patterns](modules/safety-patterns.md) <br>\n- [Shell Script Structure Patterns](modules/structure-patterns.md) <br>\n\n\n## Skill Output: <br>\n**Output Type(s):** [Text, Markdown, Shell commands, Guidance] <br>\n**Output Format:** [Markdown review findings with script lists, issue sections, suggested fixes, and an approval recommendation.] <br>\n**Output Parameters:** [1D] <br>\n**Other Properties Related to Output:** [May include copyable discovery, verification, and formatting commands; inspect commands before running them.] <br>\n\n## Skill Version(s): <br>\n1.9.16 (source: ClawHub release evidence; artifact frontmatter reports 1.9.8) <br>\n\n## Ethical Considerations: <br>\nUsers should evaluate whether this skill is appropriate for their environment, review any generated or modified files before relying on them, and apply their organization's safety, security, and compliance requirements before deployment. <br>\n\nArchive v1.9.14: 7 files, 11397 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (2403b), SKILL.md (4009b), _meta.json (143b)\n\nFile v1.9.14:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (portability, maintainability)\n- Minor issues (style, documentation)\n\n## Output Format\n\n```markdown\n## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n\n## Exit Criteria\n\n- [ ] Exit code propagation verified (pipelines checked for pipefail or\n  capture-and-check)\n- [ ] Portability issues documented (Bash-isms in `#!/bin/sh` scripts flagged)\n- [ ] Safety patterns verified (no echo, braced vars, `:?` expansion, cd in\n  subshells, no basename/dirname)\n- [ ] Structure patterns verified (library/executable distinction, main call,\n  preamble, depcheck, shfmt formatting)\n- [ ] Evidence logged with file:line references via `imbue:proof-of-work`\n\nFile v1.9.14:_meta.json\n\n{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.14\",\n  \"publishedAt\": 1782842676162\n}\n\nFile v1.9.14:modules/exit-codes.md\n\n---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |\n\nFile v1.9.14:modules/portability.md\n\n---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent\n\nFile v1.9.14:modules/safety-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nDetection for non-canonical form:\n\n```sh\nrg -n '\\[ -[zn].*__\\w+_loaded' scripts/\n```\n\n## set -e / set -u in libraries\n\nFiles meant to be sourced must not enable `set -e` or `set -u`\nbecause the flags leak to the caller and can exit the caller's\nsession on unrelated commands.\n\nDetection:\n\n```sh\nrg -n '^set -[eu]' scripts/logging.sh\n```\n\n## printf over echo\n\nFor data output and multi-line messages, prefer `printf` with a\nfixed format string. Never build the format string from untrusted\ntext.\n\n```sh\n# Bad — echo interprets escape sequences inconsistently\necho \"Processing ${file}\"\n\n# Good — fixed format, no interpretation surprises\nprintf 'Processing %s\\n' \"${file}\"\n```\n\nFor logging through `log()`, pass the message as an argument.\n`log()` uses `printf` internally.\n\n## Checklist\n\n- [ ] No raw `echo` calls (use `log()` or `printf`)\n- [ ] All variables in braced form `${VAR}`\n- [ ] Unset required variables caught with `:?` expansion\n- [ ] Every `cd` is wrapped in a subshell\n- [ ] External files sourced via `${0%/*}` relative path\n- [ ] No `basename`/`dirname`; use param expansion\n- [ ] Library guard uses `case \"${__lib_loaded:-NULL}\" in`\n- [ ] Library scripts have no `set -e` or `set -u`\n\nFile v1.9.14:modules/structure-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: structure-patterns\ndescription: Library vs executable structure, main(), preamble, depcheck(), platform detection\ntags: [structure, main, library, depcheck, platform, readonly, shfmt]\n---\n\n# Shell Script Structure Patterns\n\n## Library vs executable\n\nA script is a **library** if it has no `main()` function (e.g.\n`scripts/logging.sh`). A script is an **executable** if it defines\n`main()` and ends with `main \"${@}\"`.\n\nLibraries signal their presence by setting a `__`-prefixed guard\nvariable (e.g. `__logging_loaded=1`). Callers verify it was sourced\nwith the canonical form:\n\n```sh\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nSee also: `safety-patterns.md` → \"Library loading check form\".\n\n| Property | Library | Executable |\n|----------|---------|------------|\n| Execute bit | No | Yes |\n| `main()` | No | Required |\n| Last line | — | `main \"${@}\"` |\n| `usage()` | Not needed | Recommended |\n| `set -e`/`set -u` | Never | Allowed |\n| `__`-prefixed globals | Yes (guards) | Allowed |\n\nDetection: library with execute bit\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  rg -q 'main\\(\\)' \"${f}\" || printf 'Library with +x: %s\\n' \"${f}\"\ndone\n```\n\nDetection: executable missing `main \"${@}\"` as last line\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  last=\"$(tail -1 \"${f}\")\"\n  case \"${last}\" in\n    'main \"${@}\"') : ;;\n    *) printf 'Missing main call: %s\\n' \"${f}\" ;;\n  esac\ndone\n```\n\n## Preamble for executable scripts\n\nEvery executable script must start with this preamble (using\n`scripts/shellcheck.sh` as the canonical example):\n\n```sh\n#!/bin/sh\nset -eu\n\nMYDIR=\"${0%/*}\"\nreadonly MYDIR\n\n# shellcheck source=scripts/logging.sh\n. \"${MYDIR%/}/logging.sh\"\n```\n\n- `#!/bin/sh`: POSIX dialect; no Bash extensions\n- `set -eu`: exit on error (`-e`), error on unset (`-u`)\n- `MYDIR=\"${0%/*}\"`: script directory without `dirname`\n- `readonly MYDIR`: marks the variable immutable\n- `# shellcheck source=…`: lets shellcheck follow the source\n- `. \"${MYDIR%/}/logging.sh\"`: loads `log()` and `banner()`\n\n## All functionality in functions; no top-down execution\n\nScripts must never execute logic at top level. Every statement\nbelongs inside a named function. The only top-level calls are:\n\n1. `set -eu` (preamble)\n2. Variable declarations (`readonly`, assignments)\n3. Source statements (`. lib.sh`)\n4. `main \"${@}\"` on the last line\n\nDetection:\n\n```sh\n# Top-level commands outside function definitions\n# (rough heuristic — awk parses function body depth)\nawk '/^[a-z_][a-z_0-9]*\\(\\)/{depth++} /^\\}/{depth--}\n     depth==0 && /^\\s*[a-z]/ && !/^(readonly|MYDIR|LOG|\\.|\\s*#)/{print NR\": \"$0}' script.sh\n```\n\n## depcheck() for external dependencies\n\nAny script relying on tools beyond POSIX must define `depcheck()`.\nRequired tools use `log 5` (critical); optional tools use `log 3`\n(notice). Dependency lists allow the check logic to stay unchanged\nwhen tools are added.\n\n```sh\nREQUIRED_DEPENDENCIES=\"shellcheck shfmt\"\n\ndepcheck() {\n  _dc_missing=\"\"\n  for _dc_util in ${REQUIRED_DEPENDENCIES}; do\n    command -v \"${_dc_util}\" >/dev/null 2>&1 ||\n      _dc_missing=\"${_dc_missing:+\"${_dc_missing} \"}${_dc_util}\"\n  done\n  case \"${#_dc_missing}\" in\n    0) return 0 ;;\n  esac\n  log 5 \"Required utilities not found: ${_dc_missing}\"\n  return 1\n}\n```\n\nBuilding `_dc_missing` with `${_dc_missing:+\"${_dc_missing} \"}${_dc_util}`\nis the POSIX way to append a space-separated word without leaving a\nleading space. It avoids arrays, which are a Bash extension.\n\n## usage() function\n\nScripts that accept flags must define `usage()`. The first output\nline must use `log \"Usage: …\"`. Subsequent lines use `printf`.\n\n```sh\nusage() {\n  log \"Usage: scripts/myscript.sh [-h] [-x|-t] [ARGS]\"\n  printf '  -h       Show this help and exit (exit 0)\\n'\n  printf '  -x, -t   Enable xtrace for debugging\\n'\n  printf '  ARGS     Files or patterns to process\\n'\n}\n```\n\nThe `usage` and `help` case patterns must accept both spellings and\nany case:\n\n```sh\ncase \"${1}\" in\n  *[uU][sS][aA][gG][eE] | *[hH][eE][lL][pP] | -h)\n    usage\n    exit 0\n    ;;\nesac\n```\n\n## xtrace support (-x / -t flags)\n\nEvery executable script must support a flag to enable `xtrace`\nfor debugging. The preferred flags are `-x` and `-t`.\n\n```sh\nXTRACE=0\n\n# …inside main() after arg parsing:\ncase \"${XTRACE}\" in\n  1) set -x ;;\nesac\n```\n\n## readonly for non-modified globals\n\nGlobal variables that do not change during execution must be\nmarked `readonly`. Group these declarations near the top of the\nscript, after the preamble.\n\n```sh\nreadonly MYDIR\nreadonly VERSION=\"1.0.0\"\nreadonly CONFIG_FILE=\"${MYDIR%/}/../.config\"\n```\n\n## Platform detection\n\nWhen commands differ by OS, assign the command and its arguments\nto separate variables using `uname -s` and distribution files.\n\n```sh\nKERNEL=\"$(uname -s)\"\ncase \"${KERNEL}\" in\n  *BSD | [Ll]inux)\n    . /etc/os-release\n    case \"${ID}\" in\n      freebsd) INSTALLER=\"pkg\"; INSTALLER_ARG=\"install\" ;;\n      ubuntu | debian) INSTALLER=\"apt-get\"; INSTALLER_ARG=\"install -yqq\" ;;\n      alpine) INSTALLER=\"apk\"; INSTALLER_ARG=\"add\" ;;\n      fedora | centos | rhel)\n        for _pm_cmd in dnf yum; do\n          command -v \"${_pm_cmd}\" >/dev/null 2>&1 && INSTALLER=\"${_pm_cmd}\"\n        done\n        INSTALLER_ARG=\"install\"\n        ;;\n    esac\n    ;;\n  Darwin)\n    INSTALLER=\"brew\"\n    INSTALLER_ARG=\"install\"\n    ;;\nesac\n\n\"${INSTALLER:?No known installer selected}\" ${INSTALLER_ARG} \"${PACKAGES}\"\n```\n\nNote: `${INSTALLER_ARG}` is intentionally unquoted here so its\nspace-separated arguments expand into multiple words.\n\n## case over test / [ ]\n\nPrefer `case` statements over `test`/`[ ]` for branching. `case`\nis faster (no subprocess), cleaner, and handles patterns natively.\n\n```sh\n# Preferred\ncase \"${answer}\" in\n  [yY] | [yY][eE][sS]) confirm ;;\n  *) abort ;;\nesac\n\n# Avoid\nif [ \"${answer}\" = \"y\" ] || [ \"${answer}\" = \"Y\" ]; then\n  confirm\nfi\n```\n\n## Formatting: shfmt -p -i 2 -ci\n\nAll scripts must be formatted with:\n\n```sh\nshfmt -p -i 2 -ci -w script.sh\n```\n\n- `-p` POSIX mode (no Bash extensions)\n- `-i 2` two-space indent\n- `-ci` indent `case` label bodies\n\nRun from the repository root:\n\n```sh\n# Check all scripts\nshfmt -p -i 2 -ci -d scripts/\n\n# Apply formatting in-place\nshfmt -p -i 2 -ci -w scripts/*.sh\n```\n\n## Checklist\n\n- [ ] Library: no execute bit, no `main()`, `__`-guard present\n- [ ] Executable: starts with preamble, ends with `main \"${@}\"`\n- [ ] No top-level logic (only declarations, source, `main \"${@}\"`)\n- [ ] `depcheck()` present when external tools are required\n- [ ] `usage()` present and accepts `-h` / `usage`/`help` variants\n- [ ] `-x`/`-t` flags supported and enable xtrace\n- [ ] Non-modified globals are `readonly`\n- [ ] Platform branching uses `uname -s` + INSTALLER pattern\n- [ ] `case` used instead of `[ ]` for branching\n- [ ] `shfmt -p -i 2 -ci -d` reports no diff\n\nFile v1.9.14:skill-card.md\n\n## Description: <br>\nAudits shell scripts for correctness, portability, and common pitfalls. <br>\n\nThis skill is ready for commercial/non-commercial use. <br>\n\n## Publisher: <br>\n[athola](https://clawhub.ai/user/athola) <br>\n\n### License/Terms of Use: <br>\nMIT-0 <br>\n\n\n## Use Case: <br>\nDevelopers and engineers use this skill to review shell scripts in CI, hooks, wrappers, and build automation for exit-code handling, portability, safety, and maintainability issues. <br>\n\n### Deployment Geography for Use: <br>\nGlobal <br>\n\n## Known Risks and Mitigations: <br>\nRisk: Command examples may be copied into a local shell and some examples can install packages or modify files in place. <br>\nMitigation: Review each suggested command before running it, avoid package-manager commands unless intentionally installing dependencies, and prefer diff or dry-run checks before in-place formatting. <br>\nRisk: Shell review guidance can miss project-specific execution context or produce recommendations that are not appropriate for a particular script. <br>\nMitigation: Validate findings against the script's shebang, target platforms, CI environment, and tests before applying changes. <br>\n\n\n## Reference(s): <br>\n- [ClawHub Skill Page](https://clawhub.ai/athola/skills/nm-pensive-shell-review) <br>\n- [Clawdis Homepage](https://github.com/athola/claude-night-market/tree/master/plugins/pensive) <br>\n- [Exit Code Patterns](modules/exit-codes.md) <br>\n- [Shell Portability](modules/portability.md) <br>\n- [Shell Safety Patterns](modules/safety-patterns.md) <br>\n- [Shell Script Structure Patterns](modules/structure-patterns.md) <br>\n\n\n## Skill Output: <br>\n**Output Type(s):** [Text, Markdown, Shell commands, Guidance] <br>\n**Output Format:** [Markdown review report with file references, issue categories, recommendations, and inline shell command examples.] <br>\n**Output Parameters:** [1D] <br>\n**Other Properties Related to Output:** [May suggest tools such as shellcheck, grep, rg, and shfmt; command examples should be reviewed before execution.] <br>\n\n## Skill Version(s): <br>\n1.9.14 (source: server release evidence) <br>\n\n## Ethical Considerations: <br>\nUsers should evaluate whether this skill is appropriate for their environment, review any generated or modified files before relying on them, and apply their organization's safety, security, and compliance requirements before deployment. <br>\n\nArchive v1.9.13: 7 files, 11251 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (2023b), SKILL.md (4009b), _meta.json (143b)\n\nFile v1.9.13:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (portability, maintainability)\n- Minor issues (style, documentation)\n\n## Output Format\n\n```markdown\n## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n\n## Exit Criteria\n\n- [ ] Exit code propagation verified (pipelines checked for pipefail or\n  capture-and-check)\n- [ ] Portability issues documented (Bash-isms in `#!/bin/sh` scripts flagged)\n- [ ] Safety patterns verified (no echo, braced vars, `:?` expansion, cd in\n  subshells, no basename/dirname)\n- [ ] Structure patterns verified (library/executable distinction, main call,\n  preamble, depcheck, shfmt formatting)\n- [ ] Evidence logged with file:line references via `imbue:proof-of-work`\n\nFile v1.9.13:_meta.json\n\n{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.13\",\n  \"publishedAt\": 1782577353599\n}\n\nFile v1.9.13:modules/exit-codes.md\n\n---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |\n\nFile v1.9.13:modules/portability.md\n\n---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent\n\nFile v1.9.13:modules/safety-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nDetection for non-canonical form:\n\n```sh\nrg -n '\\[ -[zn].*__\\w+_loaded' scripts/\n```\n\n## set -e / set -u in libraries\n\nFiles meant to be sourced must not enable `set -e` or `set -u`\nbecause the flags leak to the caller and can exit the caller's\nsession on unrelated commands.\n\nDetection:\n\n```sh\nrg -n '^set -[eu]' scripts/logging.sh\n```\n\n## printf over echo\n\nFor data output and multi-line messages, prefer `printf` with a\nfixed format string. Never build the format string from untrusted\ntext.\n\n```sh\n# Bad — echo interprets escape sequences inconsistently\necho \"Processing ${file}\"\n\n# Good — fixed format, no interpretation surprises\nprintf 'Processing %s\\n' \"${file}\"\n```\n\nFor logging through `log()`, pass the message as an argument.\n`log()` uses `printf` internally.\n\n## Checklist\n\n- [ ] No raw `echo` calls (use `log()` or `printf`)\n- [ ] All variables in braced form `${VAR}`\n- [ ] Unset required variables caught with `:?` expansion\n- [ ] Every `cd` is wrapped in a subshell\n- [ ] External files sourced via `${0%/*}` relative path\n- [ ] No `basename`/`dirname`; use param expansion\n- [ ] Library guard uses `case \"${__lib_loaded:-NULL}\" in`\n- [ ] Library scripts have no `set -e` or `set -u`\n\nFile v1.9.13:modules/structure-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: structure-patterns\ndescription: Library vs executable structure, main(), preamble, depcheck(), platform detection\ntags: [structure, main, library, depcheck, platform, readonly, shfmt]\n---\n\n# Shell Script Structure Patterns\n\n## Library vs executable\n\nA script is a **library** if it has no `main()` function (e.g.\n`scripts/logging.sh`). A script is an **executable** if it defines\n`main()` and ends with `main \"${@}\"`.\n\nLibraries signal their presence by setting a `__`-prefixed guard\nvariable (e.g. `__logging_loaded=1`). Callers verify it was sourced\nwith the canonical form:\n\n```sh\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nSee also: `safety-patterns.md` → \"Library loading check form\".\n\n| Property | Library | Executable |\n|----------|---------|------------|\n| Execute bit | No | Yes |\n| `main()` | No | Required |\n| Last line | — | `main \"${@}\"` |\n| `usage()` | Not needed | Recommended |\n| `set -e`/`set -u` | Never | Allowed |\n| `__`-prefixed globals | Yes (guards) | Allowed |\n\nDetection: library with execute bit\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  rg -q 'main\\(\\)' \"${f}\" || printf 'Library with +x: %s\\n' \"${f}\"\ndone\n```\n\nDetection: executable missing `main \"${@}\"` as last line\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  last=\"$(tail -1 \"${f}\")\"\n  case \"${last}\" in\n    'main \"${@}\"') : ;;\n    *) printf 'Missing main call: %s\\n' \"${f}\" ;;\n  esac\ndone\n```\n\n## Preamble for executable scripts\n\nEvery executable script must start with this preamble (using\n`scripts/shellcheck.sh` as the canonical example):\n\n```sh\n#!/bin/sh\nset -eu\n\nMYDIR=\"${0%/*}\"\nreadonly MYDIR\n\n# shellcheck source=scripts/logging.sh\n. \"${MYDIR%/}/logging.sh\"\n```\n\n- `#!/bin/sh`: POSIX dialect; no Bash extensions\n- `set -eu`: exit on error (`-e`), error on unset (`-u`)\n- `MYDIR=\"${0%/*}\"`: script directory without `dirname`\n- `readonly MYDIR`: marks the variable immutable\n- `# shellcheck source=…`: lets shellcheck follow the source\n- `. \"${MYDIR%/}/logging.sh\"`: loads `log()` and `banner()`\n\n## All functionality in functions; no top-down execution\n\nScripts must never execute logic at top level. Every statement\nbelongs inside a named function. The only top-level calls are:\n\n1. `set -eu` (preamble)\n2. Variable declarations (`readonly`, assignments)\n3. Source statements (`. lib.sh`)\n4. `main \"${@}\"` on the last line\n\nDetection:\n\n```sh\n# Top-level commands outside function definitions\n# (rough heuristic — awk parses function body depth)\nawk '/^[a-z_][a-z_0-9]*\\(\\)/{depth++} /^\\}/{depth--}\n     depth==0 && /^\\s*[a-z]/ && !/^(readonly|MYDIR|LOG|\\.|\\s*#)/{print NR\": \"$0}' script.sh\n```\n\n## depcheck() for external dependencies\n\nAny script relying on tools beyond POSIX must define `depcheck()`.\nRequired tools use `log 5` (critical); optional tools use `log 3`\n(notice). Dependency lists allow the check logic to stay unchanged\nwhen tools are added.\n\n```sh\nREQUIRED_DEPENDENCIES=\"shellcheck shfmt\"\n\ndepcheck() {\n  _dc_missing=\"\"\n  for _dc_util in ${REQUIRED_DEPENDENCIES}; do\n    command -v \"${_dc_util}\" >/dev/null 2>&1 ||\n      _dc_missing=\"${_dc_missing:+\"${_dc_missing} \"}${_dc_util}\"\n  done\n  case \"${#_dc_missing}\" in\n    0) return 0 ;;\n  esac\n  log 5 \"Required utilities not found: ${_dc_missing}\"\n  return 1\n}\n```\n\nBuilding `_dc_missing` with `${_dc_missing:+\"${_dc_missing} \"}${_dc_util}`\nis the POSIX way to append a space-separated word without leaving a\nleading space. It avoids arrays, which are a Bash extension.\n\n## usage() function\n\nScripts that accept flags must define `usage()`. The first output\nline must use `log \"Usage: …\"`. Subsequent lines use `printf`.\n\n```sh\nusage() {\n  log \"Usage: scripts/myscript.sh [-h] [-x|-t] [ARGS]\"\n  printf '  -h       Show this help and exit (exit 0)\\n'\n  printf '  -x, -t   Enable xtrace for debugging\\n'\n  printf '  ARGS     Files or patterns to process\\n'\n}\n```\n\nThe `usage` and `help` case patterns must accept both spellings and\nany case:\n\n```sh\ncase \"${1}\" in\n  *[uU][sS][aA][gG][eE] | *[hH][eE][lL][pP] | -h)\n    usage\n    exit 0\n    ;;\nesac\n```\n\n## xtrace support (-x / -t flags)\n\nEvery executable script must support a flag to enable `xtrace`\nfor debugging. The preferred flags are `-x` and `-t`.\n\n```sh\nXTRACE=0\n\n# …inside main() after arg parsing:\ncase \"${XTRACE}\" in\n  1) set -x ;;\nesac\n```\n\n## readonly for non-modified globals\n\nGlobal variables that do not change during execution must be\nmarked `readonly`. Group these declarations near the top of the\nscript, after the preamble.\n\n```sh\nreadonly MYDIR\nreadonly VERSION=\"1.0.0\"\nreadonly CONFIG_FILE=\"${MYDIR%/}/../.config\"\n```\n\n## Platform detection\n\nWhen commands differ by OS, assign the command and its arguments\nto separate variables using `uname -s` and distribution files.\n\n```sh\nKERNEL=\"$(uname -s)\"\ncase \"${KERNEL}\" in\n  *BSD | [Ll]inux)\n    . /etc/os-release\n    case \"${ID}\" in\n      freebsd) INSTALLER=\"pkg\"; INSTALLER_ARG=\"install\" ;;\n      ubuntu | debian) INSTALLER=\"apt-get\"; INSTALLER_ARG=\"install -yqq\" ;;\n      alpine) INSTALLER=\"apk\"; INSTALLER_ARG=\"add\" ;;\n      fedora | centos | rhel)\n        for _pm_cmd in dnf yum; do\n          command -v \"${_pm_cmd}\" >/dev/null 2>&1 && INSTALLER=\"${_pm_cmd}\"\n        done\n        INSTALLER_ARG=\"install\"\n        ;;\n    esac\n    ;;\n  Darwin)\n    INSTALLER=\"brew\"\n    INSTALLER_ARG=\"install\"\n    ;;\nesac\n\n\"${INSTALLER:?No known installer selected}\" ${INSTALLER_ARG} \"${PACKAGES}\"\n```\n\nNote: `${INSTALLER_ARG}` is intentionally unquoted here so its\nspace-separated arguments expand into multiple words.\n\n## case over test / [ ]\n\nPrefer `case` statements over `test`/`[ ]` for branching. `case`\nis faster (no subprocess), cleaner, and handles patterns natively.\n\n```sh\n# Preferred\ncase \"${answer}\" in\n  [yY] | [yY][eE][sS]) confirm ;;\n  *) abort ;;\nesac\n\n# Avoid\nif [ \"${answer}\" = \"y\" ] || [ \"${answer}\" = \"Y\" ]; then\n  confirm\nfi\n```\n\n## Formatting: shfmt -p -i 2 -ci\n\nAll scripts must be formatted with:\n\n```sh\nshfmt -p -i 2 -ci -w script.sh\n```\n\n- `-p` POSIX mode (no Bash extensions)\n- `-i 2` two-space indent\n- `-ci` indent `case` label bodies\n\nRun from the repository root:\n\n```sh\n# Check all scripts\nshfmt -p -i 2 -ci -d scripts/\n\n# Apply formatting in-place\nshfmt -p -i 2 -ci -w scripts/*.sh\n```\n\n## Checklist\n\n- [ ] Library: no execute bit, no `main()`, `__`-guard present\n- [ ] Executable: starts with preamble, ends with `main \"${@}\"`\n- [ ] No top-level logic (only declarations, source, `main \"${@}\"`)\n- [ ] `depcheck()` present when external tools are required\n- [ ] `usage()` present and accepts `-h` / `usage`/`help` variants\n- [ ] `-x`/`-t` flags supported and enable xtrace\n- [ ] Non-modified globals are `readonly`\n- [ ] Platform branching uses `uname -s` + INSTALLER pattern\n- [ ] `case` used instead of `[ ]` for branching\n- [ ] `shfmt -p -i 2 -ci -d` reports no diff\n\nFile v1.9.13:skill-card.md\n\n## Description: <br>\nAudits shell scripts for correctness, portability, and common pitfalls. <br>\n\nThis skill is ready for commercial/non-commercial use. <br>\n\n## Publisher: <br>\n[athola](https://clawhub.ai/user/athola) <br>\n\n### License/Terms of Use: <br>\nMIT-0 <br>\n\n\n## Use Case: <br>\nDevelopers and engineers use this skill to review shell scripts used in CI/CD pipelines, hooks, wrappers, and build automation for reliability, portability, and safety issues. <br>\n\n### Deployment Geography for Use: <br>\nGlobal <br>\n\n## Known Risks and Mitigations: <br>\nRisk: The skill may activate on broad shell or CI-related wording outside the intended review context. <br>\nMitigation: Use it when reviewing shell scripts and confirm the target files before acting on findings. <br>\nRisk: Suggested package-manager, ShellCheck, or formatting commands may change local tools or files. <br>\nMitigation: Review proposed commands before execution and inspect any resulting file changes before relying on them. <br>\n\n\n## Reference(s): <br>\n- [ClawHub skill page](https://clawhub.ai/athola/skills/nm-pensive-shell-review) <br>\n- [Source homepage from package metadata](https://github.com/athola/claude-night-market/tree/master/plugins/pensive) <br>\n\n\n## Skill Output: <br>\n**Output Type(s):** [markdown, guidance, shell commands] <br>\n**Output Format:** [Markdown review with file references, categorized findings, recommendations, and shell command examples.] <br>\n**Output Parameters:** [1D] <br>\n**Other Properties Related to Output:** [Findings are organized around scripts reviewed, exit-code behavior, portability, safety patterns, structure, and an approval recommendation.] <br>\n\n## Skill Version(s): <br>\n1.9.13 (source: server release metadata) <br>\n\n## Ethical Considerations: <br>\nUsers should evaluate whether this skill is appropriate for their environment, review any generated or modified files before relying on them, and apply their organization's safety, security, and compliance requirements before deployment. <br>\n\nArchive v1.9.12: 7 files, 11068 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (1622b), SKILL.md (4009b), _meta.json (143b)\n\nFile v1.9.12:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (portability, maintainability)\n- Minor issues (style, documentation)\n\n## Output Format\n\n```markdown\n## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block\n```\n\n## Exit Criteria\n\n- [ ] Exit code propagation verified (pipelines checked for pipefail or\n  capture-and-check)\n- [ ] Portability issues documented (Bash-isms in `#!/bin/sh` scripts flagged)\n- [ ] Safety patterns verified (no echo, braced vars, `:?` expansion, cd in\n  subshells, no basename/dirname)\n- [ ] Structure patterns verified (library/executable distinction, main call,\n  preamble, depcheck, shfmt formatting)\n- [ ] Evidence logged with file:line references via `imbue:proof-of-work`\n\nFile v1.9.12:_meta.json\n\n{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.12\",\n  \"publishedAt\": 1781839067637\n}\n\nFile v1.9.12:modules/exit-codes.md\n\n---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |\n\nFile v1.9.12:modules/portability.md\n\n---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent\n\nFile v1.9.12:modules/safety-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nDetection for non-canonical form:\n\n```sh\nrg -n '\\[ -[zn].*__\\w+_loaded' scripts/\n```\n\n## set -e / set -u in libraries\n\nFiles meant to be sourced must not enable `set -e` or `set -u`\nbecause the flags leak to the caller and can exit the caller's\nsession on unrelated commands.\n\nDetection:\n\n```sh\nrg -n '^set -[eu]' scripts/logging.sh\n```\n\n## printf over echo\n\nFor data output and multi-line messages, prefer `printf` with a\nfixed format string. Never build the format string from untrusted\ntext.\n\n```sh\n# Bad — echo interprets escape sequences inconsistently\necho \"Processing ${file}\"\n\n# Good — fixed format, no interpretation surprises\nprintf 'Processing %s\\n' \"${file}\"\n```\n\nFor logging through `log()`, pass the message as an argument.\n`log()` uses `printf` internally.\n\n## Checklist\n\n- [ ] No raw `echo` calls (use `log()` or `printf`)\n- [ ] All variables in braced form `${VAR}`\n- [ ] Unset required variables caught with `:?` expansion\n- [ ] Every `cd` is wrapped in a subshell\n- [ ] External files sourced via `${0%/*}` relative path\n- [ ] No `basename`/`dirname`; use param expansion\n- [ ] Library guard uses `case \"${__lib_loaded:-NULL}\" in`\n- [ ] Library scripts have no `set -e` or `set -u`\n\nFile v1.9.12:modules/structure-patterns.md\n\n---\nparent_skill: pensive:shell-review\nmodule: structure-patterns\ndescription: Library vs executable structure, main(), preamble, depcheck(), platform detection\ntags: [structure, main, library, depcheck, platform, readonly, shfmt]\n---\n\n# Shell Script Structure Patterns\n\n## Library vs executable\n\nA script is a **library** if it has no `main()` function (e.g.\n`scripts/logging.sh`). A script is an **executable** if it defines\n`main()` and ends with `main \"${@}\"`.\n\nLibraries signal their presence by setting a `__`-prefixed guard\nvariable (e.g. `__logging_loaded=1`). Callers verify it was sourced\nwith the canonical form:\n\n```sh\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;\n  *) printf 'logging.sh not loaded\\n' >&2; exit 1 ;;\nesac\n```\n\nSee also: `safety-patterns.md` → \"Library loading check form\".\n\n| Property | Library | Executable |\n|----------|---------|------------|\n| Execute bit | No | Yes |\n| `main()` | No | Required |\n| Last line | — | `main \"${@}\"` |\n| `usage()` | Not needed | Recommended |\n| `set -e`/`set -u` | Never | Allowed |\n| `__`-prefixed globals | Yes (guards) | Allowed |\n\nDetection: library with execute bit\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  rg -q 'main\\(\\)' \"${f}\" || printf 'Library with +x: %s\\n' \"${f}\"\ndone\n```\n\nDetection: executable missing `main \"${@}\"` as last line\n\n```sh\nfind scripts/ -name \"*.sh\" -perm /u+x | while IFS= read -r f; do\n  last=\"$(tail -1 \"${f}\")\"\n  case \"${last}\" in\n    'main \"${@}\"') : ;;\n    *) printf 'Missing main call: %s\\n' \"${f}\" ;;\n  esac\ndone\n```\n\n## Preamble for executable scripts\n\nEvery executable script must start with this preamble (using\n`scripts/shellcheck.sh` as the canonical example):\n\n```sh\n#!/bin/sh\nset -eu\n\nMYDIR=\"${0%/*}\"\nreadonly MYDIR\n\n# shellcheck source=scripts/logging.sh\n. \"${MYDIR%/}/logging.sh\"\n```\n\n- `#!/bin/sh`: POSIX dialect; no Bash extensions\n- `set -eu`: exit on error (`-e`), error on unset (`-u`)\n- `MYDIR=\"${0%/*}\"`: script directory without `dirname`\n- `readonly MYDIR`: marks the variable immutable\n- `# shellcheck source=…`: lets shellcheck follow the source\n- `. \"${MYDIR%/}/logging.sh\"`: loads `log()` and `banner()`\n\n## All functionality in functions; no top-down execution\n\nScripts must never execute logic at top level. Every statement\nbelongs inside a named function. The only top-level calls are:\n\n1. `set -eu` (preamble)\n2. Variable declarations (`readonly`, assignments)\n3. Source statements (`. lib.sh`)\n4. `main \"${@}\"` on the last line\n\nDetection:\n\n```sh\n# Top-level commands outside function definitions\n# (rough heuristic — awk parses function body depth)\nawk '/^[a-z_][a-z_0-9]*\\(\\)/{depth++} /^\\}/{depth--}\n     depth==0 && /^\\s*[a-z]/ && !/^(readonly|MYDIR|LOG|\\.|\\s*#)/{print NR\": \"$0}' script.sh\n```\n\n## depcheck() for external dependencies\n\nAny script relying on tools beyond POSIX must define `depcheck()`.\nRequired tools use `log 5` (critical); optional tools use `log 3`\n(notice). Dependency lists allow the check logic to stay unchanged\nwhen tools are added.\n\n```sh\nREQUIRED_DEPENDENCIES=\"shellcheck shfmt\"\n\ndepcheck() {\n  _dc_missing=\"\"\n  for _dc_util in ${REQUIRED_DEPENDENCIES}; do\n    command -v \"${_dc_util}\" >/dev/null 2>&1 ||\n      _dc_missing=\"${_dc_missing:+\"${_dc_missing} \"}${_dc_util}\"\n  done\n  case \"${#_dc_missing}\" in\n    0) return 0 ;;\n  esac\n  log 5 \"Required utilities not found: ${_dc_missing}\"\n  return 1\n}\n```\n\nBuilding `_dc_missing` with `${_dc_missing:+\"${_dc_missing} \"}${_dc_util}`\nis the POSIX way to append a space-separated word without leaving a\nleading space. It avoids arrays, which are a Bash extension.\n\n## usage() function\n\nScripts that accept flags must define `usage()`. The first output\nline must use `log \"Usage: …\"`. Subsequent lines use `printf`.\n\n```sh\nusage() {\n  log \"Usage: scripts/myscript.sh [-h] [-x|-t] [ARGS]\"\n  printf '  -h       Show this help and exit (exit 0)\\n'\n  printf '  -x, -t   Enable xtrace for debugging\\n'\n  printf '  ARGS     Files or patterns to process\\n'\n}\n```\n\nThe `usage` and `help` case patterns must accept both spellings and\nany case:\n\n```sh\ncase \"${1}\" in\n  *[uU][sS][aA][gG][eE] | *[hH][eE][lL][pP] | -h)\n    usage\n    exit 0\n    ;;\nesac\n```\n\n## xtrace support (-x / -t flags)\n\nEvery executable script must support a flag to enable `xtrace`\nfor debugging. The preferred flags are `-x` and `-t`.\n\n```sh\nXTRACE=0\n\n# …inside main() after arg parsing:\ncase \"${XTRACE}\" in\n  1) set -x ;;\nesac\n```\n\n## readonly for non-modified globals\n\nGlobal variables that do not change during execution must be\nmarked `readonly`. Group these declarations near the top of the\nscript, after the preamble.\n\n```sh\nreadonly MYDIR\nreadonly VERSION=\"1.0.0\"\nreadonly CONFIG_FILE=\"${MYDIR%/}/../.config\"\n```\n\n## Platform detection\n\nWhen commands differ by OS, assign the command and its arguments\nto separate variables using `uname -s` and distribution files.\n\n```sh\nKERNEL=\"$(uname -s)\"\ncase \"${KERNEL}\" in\n  *BSD | [Ll]inux)\n    . /etc/os-release\n    case \"${ID}\" in\n      freebsd) INSTALLER=\"pkg\"; INSTALLER_ARG=\"install\" ;;\n      ubuntu | debian) INSTALLER=\"apt-get\"; INSTALLER_ARG=\"install -yqq\" ;;\n      alpine) INSTALLER=\"apk\"; INSTALLER_ARG=\"add\" ;;\n      fedora | centos | rhel)\n        for _pm_cmd in dnf yum; do\n          command -v \"${_pm_cmd}\" >/dev/null 2>&1 && INSTALLER=\"${_pm_cmd}\"\n        done\n        INSTALLER_ARG=\"install\"\n        ;;\n    esac\n    ;;\n  Darwin)\n    INSTALLER=\"brew\"\n    INSTALLER_ARG=\"install\"\n    ;;\nesac\n\n\"${INSTALLER:?No known installer selected}\" ${INSTALLER_ARG} \"${PACKAGES}\"\n```\n\nNote: `${INSTALLER_ARG}` is intentionally unquoted here so its\nspace-separated arguments expand into multiple words.\n\n## case over test / [ ]\n\nPrefer `case` statements over `test`/`[ ]` for branching. `case`\nis faster (no subprocess), cleaner, and handles patterns natively.\n\n```sh\n# Preferred\ncase \"${answer}\" in\n  [yY] | [yY][eE][sS]) confirm ;;\n  *) abort ;;\nesac\n\n# Avoid\nif [ \"${answer}\" = \"y\" ] || [ \"${answer}\" = \"Y\" ]; then\n  confirm\nfi\n```\n\n## Formatting: shfmt -p -i 2 -ci\n\nAll scripts must be formatted with:\n\n```sh\nshfmt -p -i 2 -ci -w script.sh\n```\n\n- `-p` POSIX mode (no Bash extensions)\n- `-i 2` two-space indent\n- `-ci` indent `case` label bodies\n\nRun from the repository root:\n\n```sh\n# Check all scripts\nshfmt -p -i 2 -ci -d scripts/\n\n# Apply formatting in-place\nshfmt -p -i 2 -ci -w scripts/*.sh\n```\n\n## Checklist\n\n- [ ] Library: no execute bit, no `main()`, `__`-guard present\n- [ ] Executable: starts with preamble, ends with `main \"${@}\"`\n- [ ] No top-level logic (only declarations, source, `main \"${@}\"`)\n- [ ] `depcheck()` present when external tools are required\n- [ ] `usage()` present and accepts `-h` / `usage`/`help` variants\n- [ ] `-x`/`-t` flags supported and enable xtrace\n- [ ] Non-modified globals are `readonly`\n- [ ] Platform branching uses `uname -s` + INSTALLER pattern\n- [ ] `case` used instead of `[ ]` for branching\n- [ ] `shfmt -p -i 2 -ci -d` reports no diff\n\nFile v1.9.12:skill-card.md\n\n## Description: <br>\nAudits shell scripts for correctness, portability, and common pitfalls. <br>\n\nThis skill is ready for commercial/non-commercial use. <br>\n\n## Publisher: <br>\n[athola](https://clawhub.ai/user/athola) <br>\n\n### License/Terms of Use: <br>\nMIT-0 <br>\n\n\n## Use Case: <br>\nDevelopers and engineers use this skill to review shell scripts used in CI, hooks, wrappers, build automation, and pre-commit workflows for correctness, portability, safety, and maintainability issues. <br>\n\n### Deployment Geography for Use: <br>\nGlobal <br>\n\n## Known Risks and Mitigations: <br>\nRisk: Review before execution as proposals could introduce incorrect or misleading guidance into skills. <br>\nMitigation: Review and scan skill before deployment. <br>\n\n## Reference(s): <br>\n- [Pensive source plugin homepage](https://github.com/athola/claude-night-market/tree/master/plugins/pensive) <br>\n\n\n## Skill Output: <br>\n**Output Type(s):** [analysis, markdown, shell commands, guidance] <br>\n**Output Format:** [Markdown review findings with inline shell commands and recommendations] <br>\n**Output Parameters:** [1D] <br>\n**Other Properties Related to Output:** [May include approve, approve-with-actions, or block recommendations based on reviewed shell script issues.] <br>\n\n## Skill Version(s): <br>\n1.9.12 (source: ClawHub release evidence) <br>\n\n## Ethical Considerations: <br>\nUsers should evaluate whether this skill is appropriate for their environment, review any generated or modified files before relying on them, and apply their organization's safety, security, and compliance requirements before deployment. <br>\n\nArchive v1.0.3: 7 files, 11251 bytes\n\nFiles: modules/exit-codes.md (2713b), modules/portability.md (2667b), modules/safety-patterns.md (4257b), modules/structure-patterns.md (6946b), skill-card.md (2147b), SKILL.md (4009b), _meta.json (142b)\n\nFile v1.0.3:SKILL.md\n\n---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`\n\nArchive v1.0.2: 6 files, 7484 bytes\n\nFiles: modules/exit-codes.md (2711b), modules/portability.md (2667b), modules/safety-patterns.md (3197b), skill-card.md (2318b), SKILL.md (3437b), _meta.json (142b)\n\nArchive v1.0.1: 5 files, 6259 bytes\n\nFiles: modules/exit-codes.md (2711b), modules/portability.md (2667b), modules/safety-patterns.md (3197b), SKILL.md (3437b), _meta.json (142b)\n\nArchive v1.0.0: 5 files, 6261 bytes\n\nFiles: modules/exit-codes.md (2711b), modules/portability.md (2667b), modules/safety-patterns.md (3197b), SKILL.md (3437b), _meta.json (142b)","readmeExcerpt":"Skill: shell-review Owner: athola Summary: Audits shell scripts for correctness, portability, and common pitfalls Tags: latest:1.9.19 Version history: v1.9.19 | 2026-08-26T13:19:27.339Z | user Release v1.9.19 v1.9.17 | 2026-07-30T05:39:38.105Z | user Release v1.9.17 v1.9.16 | 2026-07-14T19:56:22.538Z | user Release v1.9.16 v1.9.14 | 2026-06-30T18:04:36.162Z | user Release v1.9.14 v1.9.13 | 2026-06-27T16:22:33.599Z | ","codeSnippets":[],"executableExamples":[{"language":"bash","snippet":"/shell-review path/to/script.sh"},{"language":"bash","snippet":"# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10"},{"language":"markdown","snippet":"## Summary\nShell script review findings\n\n## Scripts Reviewed\n- [list with line counts]\n\n## Exit Code Issues\n### [E1] Pipeline masks failure\n- Location: script.sh:42\n- Pattern: `cmd | grep` loses exit code\n- Fix: Use pipefail or capture separately\n\n## Portability Issues\n[cross-platform concerns]\n\n## Safety Issues\n[unquoted variables, missing set flags]\n\n## Recommendation\nApprove / Approve with actions / Block"},{"language":"bash","snippet":"# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi"},{"language":"bash","snippet":"set -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi"},{"language":"bash","snippet":"# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi"}],"parameters":null,"dependencies":[],"permissions":[],"extractedFiles":[{"path":"SKILL.md","content":"---\nname: shell-review\ndescription: Audits shell scripts for correctness, portability, and common pitfalls\nversion: 1.9.8\ntriggers:\n  - shell\n  - bash\n  - posix\n  - scripting\n  - ci\n  - hooks\n  - reviewing shell scripts or before committing shell changes\nmetadata: {\"openclaw\": {\"homepage\": \"https://github.com/athola/claude-night-market/tree/master/plugins/pensive\", \"emoji\": \"\\ud83e\\udd9e\", \"requires\": {\"config\": [\"night-market.pensive:shared\", \"night-market.imbue:proof-of-work\"]}}}\nsource: claude-night-market\nsource_plugin: pensive\n---\n\n> **Night Market Skill** — ported from [claude-night-market/pensive](https://github.com/athola/claude-night-market/tree/master/plugins/pensive). For the full experience with agents, hooks, and commands, install the Claude Code plugin.\n\n\n## Table of Contents\n\n- [Quick Start](#quick-start)\n- [When to Use](#when-to-use)\n- [Required TodoWrite Items](#required-todowrite-items)\n- [Workflow](#workflow)\n- [Output Format](#output-format)\n\n# Shell Script Review\n\nAudit shell scripts for correctness, safety, and portability.\n\n## Verification\n\nAfter review, run `shellcheck <script>` to verify fixes address identified issues.\n\n## Testing\n\nRun `pytest plugins/pensive/tests/skills/test_shell_review.py -v` to validate review patterns.\n\n## Quick Start\n\n```bash\n/shell-review path/to/script.sh\n```\n\n## When To Use\n\n- CI/CD pipeline scripts\n- Git hook scripts\n- Wrapper scripts (run-*.sh)\n- Build automation scripts\n- Pre-commit hook implementations\n\n## When NOT To Use\n\n- Non-shell scripts (Python, JS, etc.)\n- One-liner commands that don't need review\n\n## Required TodoWrite Items\n\n1. `shell-review:context-mapped`\n2. `shell-review:exit-codes-checked`\n3. `shell-review:portability-checked`\n4. `shell-review:safety-patterns-verified`\n5. `shell-review:structure-checked`\n6. `shell-review:evidence-logged`\n\n## Workflow\n\n### Step 1: Map Context (`shell-review:context-mapped`)\n\nIdentify shell scripts:\n```bash\n# Find shell scripts\nfind . -not -path \"*/.venv/*\" -not -path \"*/__pycache__/*\" \\\n  -not -path \"*/node_modules/*\" -not -path \"*/.git/*\" \\\n  -name \"*.sh\" -type f | head -20\n# Check shebangs\nrg -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n# fallback: grep -l \"^#!/\" scripts/ hooks/ 2>/dev/null | head -10\n```\n\nDocument:\n- Script purpose and trigger context\n- Integration points (make, pre-commit, CI)\n- Expected inputs and outputs\n\n### Step 2: Exit Code Audit (`shell-review:exit-codes-checked`)\n\n@include modules/exit-codes.md\n\n### Step 3: Portability Check (`shell-review:portability-checked`)\n\n@include modules/portability.md\n\n### Step 4: Safety Patterns (`shell-review:safety-patterns-verified`)\n\n@include modules/safety-patterns.md\n\n### Step 5: Structure Patterns (`shell-review:structure-checked`)\n\n@include modules/structure-patterns.md\n\n### Step 6: Evidence Log (`shell-review:evidence-logged`)\n\nUse `imbue:proof-of-work` to record findings with file:line references.\n\nSummarize:\n- Critical issues (failures masked, security risks)\n- Major issues (p"},{"path":"_meta.json","content":"{\n  \"ownerId\": \"kn7d107jg9jv602h9ytsegydq184a42s\",\n  \"slug\": \"nm-pensive-shell-review\",\n  \"version\": \"1.9.19\",\n  \"publishedAt\": 1787750367339\n}"},{"path":"modules/exit-codes.md","content":"---\nparent_skill: pensive:shell-review\nmodule: exit-codes\ndescription: Exit code propagation patterns and pipeline pitfalls\ntags: [exit-codes, pipelines, error-handling, pipefail]\n---\n\n# Exit Code Patterns\n\n## Critical: Pipeline Exit Codes\n\nThe default bash behavior is that a pipeline's exit code equals the **last** command's exit code. This masks failures:\n\n```bash\n# BAD - grep always succeeds if it finds lines, hiding make failure\nif (make typecheck 2>&1 | grep -v \"^make\\[\"); then\n    echo \"Passed\"  # WRONG - runs even when make fails!\nfi\n```\n\n### Fix 1: Use pipefail\n\n```bash\nset -o pipefail\n\n# Now pipeline fails if ANY command fails\nif make typecheck 2>&1 | grep -v \"^make\\[\"; then\n    echo \"Passed\"\nfi\n```\n\n### Fix 2: Capture Output and Exit Code Separately\n\n```bash\n# Capture output, preserve exit code\nlocal output\nlocal exit_code=0\noutput=$(make typecheck 2>&1) || exit_code=$?\n\n# Filter output for display\necho \"$output\" | grep -v \"^make\\[\" || true\n\n# Check actual exit code\nif [ \"$exit_code\" -eq 0 ]; then\n    echo \"Passed\"\nelse\n    echo \"Failed\"\n    return 1\nfi\n```\n\n### Fix 3: Use PIPESTATUS (Bash-specific)\n\n```bash\nmake typecheck 2>&1 | grep -v \"^make\\[\"\nif [ \"${PIPESTATUS[0]}\" -ne 0 ]; then\n    echo \"Make failed\"\n    exit 1\nfi\n```\n\n## Detection Commands\n\nFind pipeline patterns that may mask failures:\n```bash\n# Commands piped to grep/head/tail (common culprits)\ngrep -n \"| grep\" scripts/*.sh\ngrep -n \"| head\" scripts/*.sh\ngrep -n \"| tail\" scripts/*.sh\n\n# Pipelines in if conditions\ngrep -n \"if.*|\" scripts/*.sh\n\n# Subshells with pipelines\ngrep -n \"\\$(.*|\" scripts/*.sh\n```\n\n## set -e Pitfalls\n\n`set -e` (exit on error) has exceptions that can surprise:\n\n```bash\nset -e\n\n# These do NOT trigger exit:\ncmd || true           # Explicit fallback\nif cmd; then ...      # Part of condition\ncmd && other          # Part of AND/OR list\nwhile cmd; do ...     # Loop condition\n\n# This DOES trigger exit:\ncmd                   # Standalone command that fails\n```\n\n## Subshell Exit Codes\n\n```bash\n# BAD - subshell exit code lost\n(cd /tmp && failing_command)\necho \"This runs even if failing_command failed\"\n\n# GOOD - check subshell result\nif ! (cd /tmp && failing_command); then\n    echo \"Failed\"\n    exit 1\nfi\n\n# GOOD - use || to handle failure\n(cd /tmp && failing_command) || { echo \"Failed\"; exit 1; }\n```\n\n## Common Patterns to Flag\n\n| Pattern | Risk | Fix |\n|---------|------|-----|\n| `cmd \\| grep` in `if` | Exit code from grep | pipefail or capture |\n| `$(cmd \\| filter)` | Exit code from filter | PIPESTATUS or capture |\n| `cmd \\| head -1` | Loses cmd failure | pipefail |\n| `cmd 2>&1 \\| tee log` | May hide failure | pipefail |\n| `set -e` and pipes | Inconsistent behavior | Explicit checks |"},{"path":"modules/portability.md","content":"---\nparent_skill: pensive:shell-review\nmodule: portability\ndescription: POSIX vs Bash compatibility and cross-platform considerations\ntags: [posix, bash, portability, cross-platform]\n---\n\n# Shell Portability\n\n## Shebang Lines\n\n```bash\n#!/bin/sh          # POSIX shell (most portable)\n#!/bin/bash        # Bash (most features)\n#!/usr/bin/env bash  # Bash via env (handles non-standard paths)\n```\n\nIf using Bash features, use `#!/usr/bin/env bash` for portability across systems where bash may not be at `/bin/bash`.\n\n## Bash-Only Features\n\nThese require `#!/bin/bash` or `#!/usr/bin/env bash`:\n\n| Feature | Bash | POSIX Alternative |\n|---------|------|-------------------|\n| `[[ ... ]]` | Yes | `[ ... ]` |\n| `(( ... ))` | Yes | `$(( ... ))` or `[ ... ]` |\n| Arrays | Yes | Use files or positional params |\n| `${var:offset:len}` | Yes | `expr` or external tools |\n| `${var//pat/rep}` | Yes | `sed` |\n| `<<<` here-string | Yes | `echo \"$var\" \\|` |\n| `<(cmd)` process sub | Yes | Temp files or pipes |\n| `source file` | Yes | `. file` |\n| `function name { }` | Yes | `name() { }` |\n| `local -n` nameref | Bash 4.3+ | Workarounds |\n\n## Detection Commands\n\n```bash\n# Find Bash-isms in #!/bin/sh scripts\ngrep -l \"^#!/bin/sh\" scripts/*.sh | while read f; do\n    # Check for [[ ]]\n    grep -n \"\\[\\[\" \"$f\" && echo \"  ^ $f uses [[ ]]\"\n    # Check for arrays\n    grep -n \"=(\" \"$f\" && echo \"  ^ $f uses arrays\"\ndone\n\n# Find all shebang types\ngrep -h \"^#!\" scripts/*.sh | sort -u\n```\n\n## Common Portability Fixes\n\n### Test Brackets\n\n```bash\n# BAD - Bash only\nif [[ -f \"$file\" && \"$var\" == \"value\" ]]; then\n\n# GOOD - POSIX\nif [ -f \"$file\" ] && [ \"$var\" = \"value\" ]; then\n```\n\n### String Comparison\n\n```bash\n# BAD - Bash only (== works but not standard)\nif [ \"$a\" == \"$b\" ]; then\n\n# GOOD - POSIX\nif [ \"$a\" = \"$b\" ]; then\n```\n\n### Arithmetic\n\n```bash\n# BAD - Bash only\n((count++))\nif (( count > 10 )); then\n\n# GOOD - POSIX\ncount=$((count + 1))\nif [ \"$count\" -gt 10 ]; then\n```\n\n### Local Variables\n\n```bash\n# BAD - 'local' is not POSIX (but widely supported)\nlocal var=\"value\"\n\n# GOOD - explicitly use in functions only, document assumption\n# Most modern shells support 'local', acceptable if documented\n```\n\n## macOS vs Linux\n\n```bash\n# sed -i differs\n# Linux: sed -i 's/a/b/' file\n# macOS: sed -i '' 's/a/b/' file\n\n# Portable approach\nsed 's/a/b/' file > file.tmp && mv file.tmp file\n\n# Or detect platform\ncase \"$(uname -s)\" in\n    Darwin*) SED_INPLACE=\"sed -i ''\" ;;\n    *)       SED_INPLACE=\"sed -i\" ;;\nesac\n```\n\n## Recommendation\n\n1. Use `#!/usr/bin/env bash` and document Bash requirement\n2. Or use `#!/bin/sh` and avoid ALL Bash-isms\n3. Don't mix - pick one and be consistent"},{"path":"modules/safety-patterns.md","content":"---\nparent_skill: pensive:shell-review\nmodule: safety-patterns\ndescription: POSIX safety rules: no echo, braced vars, :? expansion, cd subshells\ntags: [safety, posix, quoting, expansion, cd]\n---\n\n# Shell Safety Patterns\n\n## No echo: use log() or printf\n\nAll output must go through `log()` from `scripts/logging.sh` or via\n`printf(1)`. The only exception is `usage()` body lines (after\nthe first), where `printf` is used directly.\n\nDetection:\n\n```sh\n# Bare echo calls in non-comment lines\nrg -n '^\\s*echo\\s' scripts/ .githooks/ plugins/*/hooks/\n# fallback: grep -rn '^\\s*echo\\s' scripts/ .githooks/\n```\n\nFix: replace `echo \"msg\"` with `log \"msg\"` or `printf '%s\\n' \"msg\"`.\n\n## Braced variable references\n\nEvery variable reference must use the braced form `${VAR}`, not\nbare `$VAR`. This avoids surprises with adjacent text and is\nrequired for consistent ShellCheck compliance.\n\nDetection:\n\n```sh\nrg -n '\\$[A-Za-z_][A-Za-z_0-9]*[^}]' scripts/\n```\n\nFix: `$VAR` → `${VAR}`, `$1` → `${1}`, `$@` → `\"${@}\"`.\n\n## :? expansion instead of branching on unset\n\nNever branch on an unset variable before triggering an exit-path.\nUse `${VAR:?message}` so the shell emits the message and exits\nimmediately when the variable is unset or empty.\n\n```sh\n# Bad — branches on unset, then exits\nif [ -z \"${DIR}\" ]; then\n  log 4 \"DIR is unset\"\n  exit 1\nfi\n\n# Good — parameter expansion handles it\nprocess_dir \"${DIR:?DIR must be set}\"\n```\n\nDetection:\n\n```sh\nrg -n '\\[ -z.*\\$\\{?\\w' scripts/   # [ -z \"$VAR\" ] before exit\n```\n\n## cd inside a subshell\n\nEvery `cd` must be wrapped in a subshell so that the change of\ndirectory does not persist and a failed `cd` cannot leave the\nscript in the wrong directory.\n\n```sh\n# Bad — cd leaks to caller scope; fails silently without set -e\ncd \"${build_dir}\"\nmake clean\n\n# Good — scoped and guarded\n(cd \"${build_dir:?No build dir}\" && make clean)\n```\n\nDetection:\n\n```sh\nrg -n '^\\s*cd\\s+[^(]' scripts/     # cd not wrapped in (\n```\n\n## Source relative to script location\n\nExternal files must be sourced relative to the script's own\nlocation, not the caller's working directory.\n\n```sh\n# Bad — breaks when invoked from any other directory\n. ./logging.sh\n\n# Good — always resolves from the script's directory\nMYDIR=\"${0%/*}\"\n. \"${MYDIR%/}/logging.sh\"\n```\n\nUse `${0%/*}` (POSIX parameter expansion) instead of `dirname \"$0\"`.\n\n## No basename or dirname\n\nUse POSIX parameter expansion instead of the external commands\n`basename` and `dirname`.\n\n| Command | Expansion |\n|---------|-----------|\n| `basename \"$path\"` | `\"${path##*/}\"` |\n| `dirname \"$path\"` | `\"${path%/*}\"` |\n| `basename \"$path\" .ext` | `f=\"${path##*/}\"; \"${f%.ext}\"` |\n\nDetection:\n\n```sh\nrg -n '\\bbasename\\b|\\bdirname\\b' scripts/\n```\n\n## Library loading check form\n\nWhen a script must verify a library was sourced, use the canonical\n`case` form, not `[ -z … ]` or `[ -n … ]`:\n\n```sh\n# Required form — distinguishes unset/empty/loaded\ncase \"${__logging_loaded:-NULL}\" in\n  1) : ;;    # loaded\n  *) printf 'logging.sh not loaded\\"}],"languages":[],"docsSourceLabel":"CLAWHUB","editorialOverview":"Audits shell scripts for correctness, portability, and common pitfalls Skill: shell-review Owner: athola Summary: Audits shell scripts for correctness, portability, and common pitfalls Tags: latest:1.9.19 Version history: v1.9.19 | 2026-08-26T13:19:27.339Z | user Release v1.9.19 v1.9.17 | 2026-07-30T05:39:38.105Z | user Release v1.9.17 v1.9.16 | 2026-07-14T19:56:22.538Z | user Release v1.9.16 v1.9.14 | 2026-06-30T18:04:36.162Z | user Release v1.9.14 v1.9.13 | 2026-06-27T16:22:33.599Z |","editorialQuality":{"score":100,"threshold":65,"status":"ready","wordCount":1303,"uniquenessScore":47,"reasons":[]}},"media":{"evidence":{"source":"no-media","verified":false,"confidence":"low","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":"No screenshots, media assets, or demo links are available."},"primaryImageUrl":null,"mediaAssetCount":0,"assets":[],"demoUrl":null},"ownerResources":{"evidence":{"source":"unclaimed","verified":false,"confidence":"low","updatedAt":"2026-10-10T07:32:00.205Z","emptyReason":"This page has not been claimed by the agent owner."},"hasCustomPage":false,"customPageUpdatedAt":null,"customLinks":[],"structuredLinks":{"docsUrl":null,"demoUrl":null,"supportUrl":null,"pricingUrl":null,"statusUrl":null},"customPage":null},"relatedAgents":{"evidence":{"source":"protocol-neighbors","verified":false,"confidence":"medium","updatedAt":"2026-10-10T10:56:43.418Z","emptyReason":null},"items":[{"id":"8ebccd8e-3863-4187-8355-c3f14e1f9edf","entityType":"agent","canonicalPath":"/agent/iofficeai-aionui","slug":"iofficeai-aionui","name":"AionUi","description":"Free, local, open-source 24/7 Cowork app and OpenClaw for Gemini CLI, Claude Code, Codex, OpenCode, Qwen Code, Goose CLI, Auggie, and more | 🌟 Star if you like it!","url":"https://github.com/iOfficeAI/AionUi","homepage":"https://www.aionui.com","source":"GITHUB_REPOS","protocols":["MCP","OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-10-09T19:11:12.944Z","createdAt":"2026-02-25T03:38:16.584Z","downloads":null},{"id":"b917f68a-ebff-438e-84f8-3f4b2494c0bc","entityType":"agent","canonicalPath":"/agent/activepieces-activepieces","slug":"activepieces-activepieces","name":"activepieces","description":"AI Agents & MCPs & AI Workflow Automation • (~400 MCP servers for AI agents) • AI Automation / AI Agent with MCPs • AI Workflows & AI Agents • MCPs for AI Agents","url":"https://github.com/activepieces/activepieces","homepage":"https://www.activepieces.com","source":"GITHUB_REPOS","protocols":["OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-04-15T02:22:12.426Z","createdAt":"2026-02-25T03:38:12.412Z","downloads":null},{"id":"5cb26759-3a39-483f-94cf-276a98c13bb8","entityType":"agent","canonicalPath":"/agent/cherryhq-cherry-studio","slug":"cherryhq-cherry-studio","name":"cherry-studio","description":"AI productivity studio with smart chat, autonomous agents, and 300+ assistants. Unified access to frontier LLMs","url":"https://github.com/CherryHQ/cherry-studio","homepage":"https://cherry-ai.com","source":"GITHUB_REPOS","protocols":["MCP","OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-04-11T14:38:40.986Z","createdAt":"2026-02-25T03:38:19.379Z","downloads":null},{"id":"6f6582d0-5d76-4f0f-b81d-86520247950b","entityType":"agent","canonicalPath":"/agent/copilotkit-copilotkit","slug":"copilotkit-copilotkit","name":"CopilotKit","description":"The Frontend for Agents & Generative UI. React + Angular","url":"https://github.com/CopilotKit/CopilotKit","homepage":"https://docs.copilotkit.ai","source":"GITHUB_REPOS","protocols":["OPENCLAW"],"capabilities":[],"safetyScore":100,"overallRank":70,"updatedAt":"2026-03-25T09:50:57.846Z","createdAt":"2026-02-25T03:39:14.617Z","downloads":null}],"links":{"hub":"/agent","source":"/agent/source/clawhub","protocols":[{"label":"OpenClaw","href":"/agent/protocol/openclew"}]}}}