refactor(apply.sh): extract functions + expand clean-code skill

apply.sh: - refactor entire script into main() + 14 named functions - fix BSD sed portability: \s → [[:space:]] - fix set -e traps: replace [[ ]] && cmd with if blocks - write_version_file in both scan and update mode - print_summary only in scan mode - keep 3 justified comments, remove all section headers clean-code.instructions.md: - add Newspaper Readability section - add Single Level of Abstraction (SLAB) section - add Declarative and Imperative Layers section - rewrite Comments section (Allowed / Forbidden / The test) - add guard clause guidance to Functions section - add "always use braces" rule to Formatting - add "avoid optical illusions" rule to Formatting - replace Bash/Python examples with TypeScript
2026-03-09 09:35:27 +02:00 · 2026-03-09 09:35:27 +02:00 · 407c0a560c
commit 407c0a560c
parent a90ac30d7e
3 changed files with 269 additions and 105 deletions
--- a/.ai/.gitignore
+++ b/.ai/.gitignore
@ -0,0 +1 @@
+.ai
--- a/.ai/instructions/skills/clean-code.instructions.md
+++ b/.ai/instructions/skills/clean-code.instructions.md
@ -20,6 +20,94 @@ Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and
 - 0–2 arguments preferred; use a parameter object when more are needed
 - **No side effects** — a function named `checkPassword` must not also start a session
 - No flag arguments: `delete(file, true)` means the function already does two things — split it
+- **Guard clauses** — use early return for validation/error cases at the top of a function to avoid nesting the main flow in an `else` block. When both branches share code after the split, use `if/else` instead — it reads more clearly and avoids duplication
+
+## Newspaper Readability
+
+Code should read like a good newspaper article: the headline (function/class name) tells you everything, the opening paragraph gives the high-level story, and the details follow below in order. A reader should understand the whole without jumping around.
+
+- **High-level first** — the top-level function appears at the top of the file; it calls lower-level helpers that follow below it in the order they are called
+- **One thought per "paragraph"** — one function = one clear theme; if you need a heading to separate sections inside a function, extract a function instead
+- **Consistent vocabulary** — use the same terms throughout: pick one word per concept and stick to it
+- **Smooth flow** — control flow reads top-to-bottom; avoid hidden state changes and surprising side effects that break the narrative
+
+### Newspaper Test
+Ask: *"Can someone read this function for 30 seconds and describe what it does without reading any of the helpers it calls?"*  
+If no → the function name or its level of abstraction is wrong.
+
+## Single Level of Abstraction (SLAB)
+
+One function = one technical level. High-level orchestration and low-level mechanics must not appear in the same function.
+
+**Levels of abstraction (examples):**
+- High: business flow — `processOrder`, `setupProject`, `handleUpdateMode`
+- Mid: coordination — `applyDiscount`, `resolveDocsDir`
+- Low: mechanics — `saveToDB`, `readFile`, arithmetic, string manipulation
+
+**Bad — mixed levels:**
+```typescript
+function processOrder(order: Order): void {
+  if (order.total < 100) {                     // high-level policy
+    const discount = order.total * 0.1;        // low-level calc
+    order.total -= discount;                   // mutation
+    db.save('orders', order);                  // external call
+  }
+}
+```
+
+**Good — single level:**
+```typescript
+function processOrder(order: Order): void {
+  const validated  = validateOrder(order);
+  const discounted = applyDiscount(validated);
+  saveOrder(discounted);
+}
+// Each helper owns its own level of detail
+```
+
+**AI prompts to enforce SLAB:**
+- *"Refactor this function to a single level of abstraction — high-level calls only, extract all low-level logic."*
+- *"Review this code: does each function stay at one abstraction level? Flag any that mix levels."*
+- *"Generate [feature] newspaper-style: headline function first, detail functions below in call order."*
+
+## Declarative and Imperative Layers
+
+Well-structured code has a clear layer boundary between *what* and *how*.
+
+- **Top layer — declarative**: reads like a description of intent. No loops, no conditionals on mechanics, no string manipulation. A non-programmer should be able to read it and understand the flow.
+- **Middle layer — still mostly declarative**: coordinates steps and error paths, calls lower helpers. May have simple conditionals about *what* to do, not *how*.
+- **Bottom layer — imperative**: the actual mechanics. `grep`, `awk`, `find`, SQL, arithmetic, string parsing. This is the only layer where implementation details belong.
+
+```typescript
+// Top — declarative (what happens)
+async function runCli(args: CliArgs): Promise<void> {
+  const config  = await loadConfig();
+  if (args.updateOnly) {
+    await handleUpdateMode(args.prevCommit);
+  } else {
+    await scanProjects(config.devRoot);
+    printSummary();
+  }
+  writeVersionFile(config.devRoot);
+}
+
+// Middle — declarative (what each project needs)
+async function setupProject(project: Project): Promise<void> {
+  ensureSymlink(project);
+  ensureGitignore(project);
+  const docsDir = await resolveDocsDir(project);
+  if (docsDir) await ensureDocFiles(docsDir);
+}
+
+// Bottom — imperative (how a symlink is actually created)
+function ensureSymlink(project: Project): void {
+  const linkPath = path.join(project.path, '.ai');
+  if (fs.existsSync(linkPath) && fs.readlinkSync(linkPath) === AI_TARGET) return;
+  fs.symlinkSync(AI_TARGET, linkPath);
+}
+```
+
+**Test:** Can you cover the bottom-layer functions and still understand the program from reading only the top two layers? If yes, the layering is correct.

 ## Classes

@ -37,10 +125,25 @@ Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and

 ## Comments

- Good code needs no comments — if you need to explain *what*, rename instead
- Comments explain *why*, not *what*: acceptable for non-obvious decisions, legal notices
- TODOs are temporary; remove before merging
- Never commit commented-out code
+**The default is no comments.** If you feel the urge to write one, first try to eliminate the need by renaming or extracting a function.
+
+A comment is only justified when the code cannot speak for itself — meaning the *why* is non-obvious and cannot be expressed in the language itself:
+
+**Allowed:**
+- A bug or quirk in a dependency forces unusual code: `# stripe returns 402 for both expired cards and fraud — we treat both as declined`
+- Non-obvious language or platform behaviour: `# BASH_SOURCE[0] is empty when piped through bash`
+- Timing or ordering constraint that isn't visible from the code: `# capture before exec replaces the process`
+- A trap for the next developer about a hidden coupling: `# sets got_new=1 via dynamic scope — caller must declare 'local got_new'`
+
+**Forbidden:**
+- Describing what the function name already says: `# load docs folders` above `load_docs_folders()`
+- Section dividers that replace missing structure: `# ── Config ──────────`
+- Repeating the parameter contract in prose: `# Sets RESOLVED_DOCS_DIR; returns 1 if user declines`
+- Any comment that answers "what" instead of "why"
+- TODO comments left in merged code
+- Commented-out code
+
+**The test:** Cover the function name and read only the comment. If someone could have written that comment without reading the code, it adds no value — delete it.

 ## Formatting

@ -48,6 +151,23 @@ Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and
 - Consistent indentation throughout the codebase — agree once, automate with linting
 - No deep nesting — if you're 4 levels deep, extract a function
 - Team standard beats personal preference every time
+- **Always use braces for `if`, `for`, `while`** — even for single-line bodies:
+  ```typescript
+  // Bad
+  if (docsDir) await ensureDocFiles(docsDir);
+
+  // Good
+  if (docsDir) {
+    await ensureDocFiles(docsDir);
+  }
+  ```
+  Single-line bodies invite accidents when a second line is added later.
+- **Avoid optical illusions** — code that looks like it does one thing but does another is the most dangerous kind of bug. Common sources:
+  - Side effects hidden in expressions: `if (user = getUser())` or `list.sort()` that returns void
+  - Boolean logic that reads as simpler than it is: `!a || !b` — write `!(a && b)` or extract `isInvalid(a, b)`
+  - Chained calls hiding control flow: `items.filter(...).map(...).forEach(save)` — if `save` throws, the chain silently stops
+  - Formatting that suggests grouping: unbraced `if` followed by two lines where only the first is conditional
+  - Names that imply the wrong type or behaviour: `getUserList()` returning a `Promise`, `isReady` that triggers a network call

 ## General Principles

@ -74,5 +194,5 @@ The same principles apply to configuration:
 - **Single responsibility**: one chart does one thing; don't bundle unrelated services
 - **No magic values**: named values in `values.yaml`, never hardcoded in templates
 - **DRY**: use template helpers (`_helpers.tpl`) to avoid repeating label blocks and name patterns
- **Comments**: explain *why* a value is set a certain way, not what it does — the YAML speaks for itself
+- **Comments**: only when a value's *why* is non-obvious — a workaround for a known bug, a constraint imposed by an external system, or a counter-intuitive default that will surprise the next person
 - **Defaults that make sense**: a default of `false` or `""` that always needs overriding is not a useful default
--- a/apply.sh
+++ b/apply.sh
@ -5,12 +5,11 @@ REPO_URL="https://gitea.nikos-dev.keskikuja.site/niko/ai-superpower.git"
 REPO_NAME="ai-superpower"
 RAW_URL="https://gitea.nikos-dev.keskikuja.site/niko/ai-superpower/raw/branch/main/apply.sh"

-# ── Bootstrap mode (curl | bash) ─────────────────────────────────────────────
-# BASH_SOURCE[0] is empty when piped through bash
+# BASH_SOURCE[0] is empty when piped through bash — this is the bootstrap entry point
 if [[ -z "${BASH_SOURCE[0]:-}" ]]; then
  DEV_ROOT="$PWD"   # capture before exec replaces the process
  TARGET="$DEV_ROOT/$REPO_NAME"
-  EXTRA_FLAGS="${1:-}"   # pass through --update if provided
+  EXTRA_FLAGS="${1:-}"
  if [[ -d "$TARGET" ]]; then
    PREV_COMMIT="$(git -C "$TARGET" rev-parse --short HEAD 2>/dev/null || echo "")"
    echo "\u2192 updating $REPO_NAME ..."
@ -20,11 +19,9 @@ if [[ -z "${BASH_SOURCE[0]:-}" ]]; then
    echo "\u2192 cloning $REPO_NAME into $TARGET ..."
    git clone "$REPO_URL" "$TARGET" || { echo "\u2717 git clone failed"; exit 1; }
  fi
-  # Pass DEV_ROOT and PREV_COMMIT explicitly
  exec bash "$TARGET/apply.sh" --bootstrapped "$DEV_ROOT" ${EXTRA_FLAGS:+"$EXTRA_FLAGS"} "$PREV_COMMIT"
 fi

-# ── Guard — block direct invocation ──────────────────────────────────────────
 if [[ "${1:-}" != "--bootstrapped" ]]; then
  echo "✗ do not run this script directly."
  echo "   run from your dev root folder:"
@ -35,64 +32,73 @@ if [[ "${1:-}" != "--bootstrapped" ]]; then
  exit 1
 fi

-# ── Local mode ────────────────────────────────────────────────────────────────
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-DEV_ROOT="${2:?DEV_ROOT not passed — re-run via curl}"
 AI_TARGET="$SCRIPT_DIR/.ai"
 TEMPLATE_MARKER="<!-- ai-superpower:template"
-
-# Read docs folder alternatives from config.yaml
 DOCS_FOLDERS=()
+RESOLVED_DOCS_DIR=""
+
+projects_found=0
+projects_skipped_no_docs=0
+projects_templated=0
+doc_files_refreshed=0
+
+load_docs_folders() {
  while IFS= read -r folder; do
    DOCS_FOLDERS+=("$folder")
-done < <(grep '^\s*-\s' "$SCRIPT_DIR/config.yaml" 2>/dev/null | sed 's/^\s*-\s*//')
-[[ ${#DOCS_FOLDERS[@]} -eq 0 ]] && DOCS_FOLDERS=("docs")
-# ── --update mode: instructions only, skip project setup ────────────────────
-# git pull already ran in bootstrap; compare commits and report changes
-if [[ "${3:-}" == "--update" ]]; then
-  COMMIT="$(git -C "$SCRIPT_DIR" rev-parse --short HEAD 2>/dev/null || echo "unknown")"
-  PREV_COMMIT="${4:-}"
+done < <(grep '^\ *-\ ' "$SCRIPT_DIR/config.yaml" 2>/dev/null | sed 's/^[[:space:]]*-[[:space:]]*//')
+  if [[ ${#DOCS_FOLDERS[@]} -eq 0 ]]; then DOCS_FOLDERS=("docs"); fi
+}
+
+current_commit() {
+  git -C "$SCRIPT_DIR" rev-parse --short HEAD 2>/dev/null || echo "unknown"
+}
+
+print_version_line() {
+  local prev="$1" current="$2"
+  if   [[ -z "$prev" ]];            then echo "  version:   $current (first run)"
+  elif [[ "$prev" == "$current" ]]; then echo "  version:   $current (no change)"
+  else                                   echo "  version:   $prev → $current"
+  fi
+}
+
+handle_update_mode() {
+  local prev_commit="$1"
+  local commit
+  commit="$(current_commit)"
  echo ""
-  if [[ -z "$PREV_COMMIT" || "$PREV_COMMIT" == "$COMMIT" ]]; then
-    echo "  version:   $COMMIT (already up to date)"
+  if [[ -z "$prev_commit" || "$prev_commit" == "$commit" ]]; then
+    echo "  version:   $commit (already up to date)"
  else
-    echo "  version:   $PREV_COMMIT → $COMMIT"
+    echo "  version:   $prev_commit → $commit"
    echo "  new commits:"
-    git -C "$SCRIPT_DIR" log --oneline "${PREV_COMMIT}..HEAD" | sed 's/^/    /'
+    git -C "$SCRIPT_DIR" log --oneline "${prev_commit}..HEAD" | sed 's/^/    /'
  fi
  echo ""
-  exit 0
-fi
-# Counters
-cnt_found=0
-cnt_no_docs=0
-cnt_templated=0
-cnt_refreshed=0
+}

-setup_project() {
+ensure_symlink() {
  local project="$1"
-  local name
-  name="$(basename "$project")"
-  echo ""
-  echo "▸ $name"
-
-  # FR-3: symlink
  local ai_link="$project/.ai"
  if [[ -L "$ai_link" && "$(readlink "$ai_link")" == "$AI_TARGET" ]]; then
-    : # correct — skip silently
-  else
+    return
+  fi
  ln -sfn "$AI_TARGET" "$ai_link"
  echo "  ✓  .ai symlinked"
-  fi
+}

-  # FR-4: .gitignore
+ensure_gitignore() {
+  local project="$1"
  local gitignore="$project/.gitignore"
  if ! grep -qxF '.ai' "$gitignore" 2>/dev/null; then
    echo '.ai' >> "$gitignore"
    echo "  ✓  .ai added to .gitignore"
  fi
+}

-  # FR-5 + FR-6: docs — find existing folder or use default
+resolve_docs_dir() {
+  local project="$1"
+  RESOLVED_DOCS_DIR=""
  local docs=""
  for folder in "${DOCS_FOLDERS[@]}"; do
    if [[ -d "$project/$folder" ]]; then
@ -100,7 +106,7 @@ setup_project() {
      break
    fi
  done
-  [[ -z "$docs" ]] && docs="$project/${DOCS_FOLDERS[0]}"
+  if [[ -z "$docs" ]]; then docs="$project/${DOCS_FOLDERS[0]}"; fi

  if [[ ! -d "$docs" ]]; then
    if [[ -t 0 ]] || [[ -e /dev/tty ]]; then
@ -111,71 +117,108 @@ setup_project() {
        echo "  ✓  created docs/"
      else
        echo "  –  skipped"
-        (( cnt_no_docs++ )) || true
-        return
+        (( projects_skipped_no_docs++ )) || true
+        return 1
      fi
    else
      echo "  ⚠  no docs/ folder — skipping context setup"
-      (( cnt_no_docs++ )) || true
-      return
+      (( projects_skipped_no_docs++ )) || true
+      return 1
    fi
  fi
-
-  local got_template=0
-
-  if [[ ! -f "$docs/ai-context.md" ]]; then
-    cp "$SCRIPT_DIR/templates/ai-context.md" "$docs/ai-context.md"
-    echo "  ✓  created docs/ai-context.md"
-    got_template=1
-  elif head -1 "$docs/ai-context.md" | grep -qF "$TEMPLATE_MARKER"; then
-    cp "$SCRIPT_DIR/templates/ai-context.md" "$docs/ai-context.md"
-    echo "  ✓  refreshed docs/ai-context.md (was factory template)"
-    (( cnt_refreshed++ )) || true
-  fi
-
-  if [[ ! -f "$docs/architecture.md" ]]; then
-    cp "$SCRIPT_DIR/templates/architecture.md" "$docs/architecture.md"
-    echo "  ✓  created docs/architecture.md"
-    got_template=1
-  elif head -1 "$docs/architecture.md" | grep -qF "$TEMPLATE_MARKER"; then
-    cp "$SCRIPT_DIR/templates/architecture.md" "$docs/architecture.md"
-    echo "  ✓  refreshed docs/architecture.md (was factory template)"
-    (( cnt_refreshed++ )) || true
-  fi
-
-  (( got_template )) && (( cnt_templated++ )) || true
+  RESOLVED_DOCS_DIR="$docs"
 }

-# ── Scan and run ──────────────────────────────────────────────────────────────
-echo "→ scanning $DEV_ROOT for projects ..."
+# sets got_new=1 via dynamic scope — caller must declare 'local got_new' before calling
+ensure_doc_file() {
+  local docs_dir="$1" filename="$2"
+  local target="$docs_dir/$filename"
+  if [[ ! -f "$target" ]]; then
+    cp "$SCRIPT_DIR/templates/$filename" "$target"
+    echo "  ✓  created $filename"
+    got_new=1
+  elif head -1 "$target" | grep -qF "$TEMPLATE_MARKER"; then
+    cp "$SCRIPT_DIR/templates/$filename" "$target"
+    echo "  ✓  refreshed $filename (was factory template)"
+    (( doc_files_refreshed++ )) || true
+  fi
+}

+ensure_doc_files() {
+  local docs_dir="$1"
+  local got_new=0
+  ensure_doc_file "$docs_dir" "ai-context.md"
+  ensure_doc_file "$docs_dir" "architecture.md"
+  (( got_new )) && (( projects_templated++ )) || true
+}
+
+setup_project() {
+  local project="$1"
+  echo ""
+  echo "▸ $(basename "$project")"
+  ensure_symlink   "$project"
+  ensure_gitignore "$project"
+  resolve_docs_dir "$project" || return 0
+  ensure_doc_files "$RESOLVED_DOCS_DIR"
+}
+
+update_mode=false
+prev_commit=""
+
+parse_args() {
+  for arg in "$@"; do
+    case "$arg" in
+      --update) update_mode=true ;;
+      *) if [[ -n "$arg" ]]; then prev_commit="$arg"; fi ;;
+    esac
+  done
+}
+
+scan_projects() {
+  local dev_root="$1"
+  echo "→ scanning $dev_root for projects ..."
  while IFS= read -r gitdir; do
    project="$(cd "$(dirname "$gitdir")" && pwd)"
-  [[ -f "$project/.ai-superpower" ]] && continue
+    if [[ -f "$project/.ai-superpower" ]]; then continue; fi
    setup_project "$project"
-  (( cnt_found++ )) || true
-done < <(find "$DEV_ROOT" -mindepth 2 -maxdepth 4 -name ".git" -type d 2>/dev/null || true)
+    (( projects_found++ )) || true
+  done < <(find "$dev_root" -mindepth 2 -maxdepth 4 -name ".git" -type d 2>/dev/null || true)
+}

-# ── Summary ───────────────────────────────────────────────────────────────────
-COMMIT="$(git -C "$SCRIPT_DIR" rev-parse --short HEAD 2>/dev/null || echo "unknown")"
-PREV_COMMIT="$(grep '^commit:' "$DEV_ROOT/.ai-superpower.version" 2>/dev/null | awk '{print $2}' || echo "")"
+print_summary() {
+  local dev_root="$1"
+  local commit saved_commit
+  commit="$(current_commit)"
+  saved_commit="$(grep '^commit:' "$dev_root/.ai-superpower.version" 2>/dev/null | awk '{print $2}' || echo "")"

  echo ""
  echo "────────────────────────────────────────"
-if [[ -n "$PREV_COMMIT" && "$PREV_COMMIT" != "$COMMIT" ]]; then
-  echo "  version:   $PREV_COMMIT → $COMMIT"
-elif [[ -n "$PREV_COMMIT" ]]; then
-  echo "  version:   $COMMIT (no change)"
-else
-  echo "  version:   $COMMIT (first run)"
-fi
-echo "  projects:  $cnt_found found"
-(( cnt_no_docs   > 0 )) && echo "  no docs/:  $cnt_no_docs project(s) skipped (no docs/ folder)" || true
-(( cnt_templated > 0 )) && echo "  templated: $cnt_templated project(s) received new docs templates" || true
-(( cnt_refreshed > 0 )) && echo "  refreshed: $cnt_refreshed template file(s) updated (were still factory default)" || true
+  print_version_line "$saved_commit" "$commit"
+  echo "  projects:  $projects_found found"
+  (( projects_skipped_no_docs > 0 )) && echo "  no docs/:  $projects_skipped_no_docs project(s) skipped (no docs/ folder)" || true
+  (( projects_templated       > 0 )) && echo "  templated: $projects_templated project(s) received new docs templates" || true
+  (( doc_files_refreshed      > 0 )) && echo "  refreshed: $doc_files_refreshed template file(s) updated (were still factory default)" || true
  echo "────────────────────────────────────────"
-(( cnt_found > 0 )) && echo "✅ done" || echo "⚠  no projects found — are you in the right directory?"
+  (( projects_found > 0 )) && echo "✅ done" || echo "⚠  no projects found — are you in the right directory?"
+}

-# FR-8: write version file
+write_version_file() {
+  local dev_root="$1"
  printf '# Written by apply.sh on every run — shows when it was last run and which version of ai-superpower was used.\ndate: %s\ncommit: %s\n' \
-  "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" "$COMMIT" > "$DEV_ROOT/.ai-superpower.version"
+    "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" "$(current_commit)" > "$dev_root/.ai-superpower.version"
+}
+
+main() {
+  local dev_root="${2:?DEV_ROOT not passed — re-run via curl}"
+  parse_args "${@:3}"
+  load_docs_folders
+  if [[ "$update_mode" == true ]]; then
+    handle_update_mode "$prev_commit"
+  else
+    scan_projects "$dev_root"
+    print_summary "$dev_root"
+  fi
+  write_version_file "$dev_root"
+}
+
+main "$@"