best-practices/scripting.md

# Scripting Conventions

## Structure

- All scripts live in `scripts/` and run from the repository root
- Scripts should be idempotent and safe to re-run
- Exit non-zero on failure so `&&` chains work naturally

## Verification Scripts

- Automated checks confirming milestone or feature outcomes
- Use colour output (green/red) for pass/fail indicators
- Should be non-destructive and environment-resilient
- Avoid needing sudo — test from the accessible side of a connection instead
- Check for default/insecure credentials and print remediation instructions on failure
- Use `curl --resolve` to bypass DNS/proxy layers when testing direct connectivity
- **Never use `set -e` in verification scripts.** A verify script's job is to run ALL checks and report a summary. `set -e` exits on the first failure, hiding remaining issues. Use explicit conditional checks and a pass/fail counter instead. Note: `((var++))` under `set -e` is a classic bash trap — pre-increment of 0 evaluates to falsy, triggering errexit. Use `var=$((var + 1))`.

## Automation Triggers

If you run the same 3+ commands in sequence more than once, it should become a script. Look for:
- Repeated command sequences in conversation history
- Steps requiring careful ordering
- Multi-step manual processes that are error-prone

## Error Handling by Tool Purpose

Not all scripts need the same error handling strategy:
- **Destructive scripts** (deploy, configure, delete) should use `set -euo pipefail` — fail fast on any error.
- **Reporting/read-only scripts** (status dashboards, aggregation, monitoring) should start without `set -e` — complex data collection from multiple sources is hard to debug under errexit. Use explicit conditional checks instead.
- **The choice depends on the tool's purpose.** A script that writes to production needs strict error handling. A script that reads from 10 sources and aggregates results needs resilience.

## Dryrun Mode

Every script that modifies state should support `--dryrun` / `-n`:
- Makes the script self-documenting about its side effects
- Enables safe testing and review before execution
- Enables test harnesses that verify output without executing changes
- Dryrun output should show exactly what would happen, not a summary

## Shell Gotchas

- `((PASS++))` fails under `set -e` when PASS=0 — the expression evaluates to 0 (false), triggering errexit. Use `PASS=$((PASS + 1))` instead.
- `set -e` silently terminates complex pipelines and subshells with no output — makes debugging extremely difficult. Also kills command substitutions that capture non-zero exit codes (e.g., `result=$(grep "pattern" file)` exits if grep finds nothing).
- `grep` interprets option-like strings (starting with `-`) as flags — use `--` terminator before patterns or input that may start with dashes.
- Always quote variables in conditionals and file paths
- Use `trap` for cleanup of temp files and credentials
- **Use `git diff --numstat` for binary file detection** instead of `file`. The `file` command is unreliable (marks shell scripts as "executable"), while `git diff --numstat` shows `-` for binary files using git's robust binary detection heuristics.
- **Use `cat -A` to diagnose invisible character issues.** Reveals non-printing characters like em dashes, zero-width spaces, and smart quotes that look identical to correct characters but break YAML parsers, config files, and frontmatter. Essential when a file looks correct but tooling rejects it.
- `grep` returns exit code 1 when no lines match — under `set -e`, this kills the script even when zero matches is a valid outcome. Append `|| true` to `grep` commands in pipelines where empty results are expected.
- `while read` in a pipeline creates a subshell — variables modified inside the loop (counters, accumulators) are lost after the loop ends. Use process substitution (`while read line; do ...; done < <(command)`) or here-strings to keep the loop in the current shell.
- **Order matters in sed/regex transformation pipelines.** Process more specific patterns before general ones. For example, if both `![[image.png]]` and `[[page]]` are valid patterns, process the image embed first — otherwise the general wikilink regex matches the inner `[[image.png]]` and the `!` prefix is left orphaned.
- **Bare `except: pass` swallows `SystemExit` in Python.** `sys.exit(0)` inside a bare `except: pass` block is captured as a `SystemExit` exception and silently swallowed — the script continues instead of exiting. Use specific exception types in except clauses, or use `break`/`return` for loop early-exit, or re-raise after checking `isinstance(e, SystemExit)`. Applies to any script with loops that short-circuit on a condition inside exception handling.
- **Never use `GROUPS` (or other reserved names) as a bash variable.** `GROUPS` is pre-set by bash completion and session initialisation with numeric group IDs — assignment appears to succeed but the pre-existing value often persists in sourcing contexts, producing bizarre "array iterates over 1000, 24, 27..." bugs. Other reserved/built-in names to avoid: `UID`, `EUID`, `PWD`, `OLDPWD`, `SHLVL`, `RANDOM`, `SECONDS`, `LINENO`, `PIPESTATUS`, `IFS`. Prefix project variables (`PROJECT_GROUPS`, `TEMPLATE_SLUGS`).
- **Inline `VAR=val cmd "$VAR"` expands the pre-existing value, not the new one.** Bash inline env-var assignment sets `VAR` for the child process, but `$VAR` in argument position is expanded by the **calling** shell using its existing (often empty) value. The trap:
  ```bash
  CP_TOKEN=$(get_token) curl -H "Authorization: Bearer $CP_TOKEN" ...   # sends empty header
  ```
  The Authorization header is empty because `$CP_TOKEN` is expanded before `CP_TOKEN=$(get_token)` takes effect. **Fixes:**
  ```bash
  export CP_TOKEN=$(get_token)                                              # set on a prior line
  curl -H "Authorization: Bearer $CP_TOKEN" ...
  # — or —
  curl -H "Authorization: Bearer $(get_token)" ...                          # inline substitution at use site
  ```
  Doesn't affect Python subprocesses launched with `env=...` because `os.environ` reads at runtime, not at command-parse time. Costs ~30 minutes per occurrence; common in CLI-tool authentication wrappers. See [Secrets Management](secrets-management.md) "Never Source .env Files" for the related safe-parser pattern when handling `.env`-style files.
- **`git mv <src> <dest>/<sub>` won't create missing parent directories.** `git mv admin static/admin` fails with `renaming 'admin' failed: No such file or directory` when `static/` doesn't exist yet, even though `admin/` does. Create the parent first: `mkdir static && git mv admin static/admin`.

## Grep All Consumers Before Removing or Keeping a Field

Before deleting — or deciding to keep — a config field, env var, or interactive prompt, grep every consumer across the tree (`grep -r VAR_NAME .` / `~`). Two outcomes: (1) dead fields accumulate silently when nothing reads them — the grep proves they're unused and safe to remove; (2) for fields you keep or rename, the grep enumerates every downstream file needing a matching edit (templates, status lines, other scripts), so you don't ship a half-applied rename. Auditing consumers is cheaper than shipping a change that leaves orphaned references.

## JSON Construction in Scripts

Use Python (not shell) for constructing JSON payloads. Multi-line prompts with quotes, backticks, and special characters break shell-based JSON construction (printf/sed/heredocs). Python's `json.dump` handles escaping correctly every time. For scripts that need to construct and submit JSON payloads, write the construction logic in Python even if the rest of the script is bash. For long agent prompts, `--prompt-file` with temp files is cleaner than heredocs — writing prompts to `/tmp/*.md` files avoids shell escaping issues and enables review before submission.

**Build payloads in a variable first — never nest `$(...)` with mixed quoting.** Patterns like `response=$(octopus_post "/endpoint" "$(python3 -c "...$var")")` silently mangle variable values inside the inner substitution. Always assign the JSON body to a variable, then pass the variable:

```bash
body=$(python3 -c "import json; print(json.dumps({'name': '$name'}))")
response=$(octopus_post "/endpoint" "$body")
```

**Helper functions must send status messages to stderr.** If a helper both returns a value on stdout and prints progress (`echo`, `ok`, `info`), the captured stdout will contain the status text mixed with the return value. Send status to stderr: `ok "msg" >&2`, so callers capturing stdout get only the return value.

## Markdown-to-PDF Pipeline (pandoc / ODT / LibreOffice)

Key gotchas when building branded PDFs from markdown via pandoc + ODT + LibreOffice:

1. **Pandoc treats standalone `---` lines as YAML frontmatter delimiters.** Replace horizontal rules with `* * *` after the frontmatter block so pandoc doesn't misparse the document.
2. **Avoid Unicode in YAML frontmatter string values.** Em-dashes and other non-ASCII characters in frontmatter cause pandoc parse failures — use plain ASCII or escape.
3. **Never use shell `sed` on ODF XML.** ODF uses namespaces (`fo:`, `style:`) that sed can't target reliably. Use Python `xml.etree.ElementTree` with namespace registration.
4. **`--toc` leaves the TOC body empty in headless builds.** Pandoc creates the TOC element but leaves `<text:index-body>` empty, and LibreOffice headless doesn't populate it. Build TOC entries directly in XML.
5. **Prefer direct XML manipulation over python-uno.** python-uno socket servers are unreliable in headless builds.
6. **Cover-page page-number suppression requires a master-page chain.** Use a `CoverPage` master with `next-style-name = "Standard"`, not a single page style.
7. **Prefer Python over shell `sed` for markdown compilation with footnotes/references.** Escaping and multi-line handling is far more reliable.