Merge branch 'fix/remaining-findings' — 10 HIGH/MED/LOW fixes

README.md

@@ -27,7 +27,9 @@ cd KeiSeiKit
 5. Generates agent `.md` files in-place with `AGENT_ROOT=~/.claude/agents assemble --in-place`
 6. Copies the three hooks and six skills
 
-After install, the only remaining step is merging `settings-snippet.json` into your `~/.claude/settings.json` to activate the hooks.
+After install, the only remaining step is merging `settings-snippet.json` into your `~/.claude/settings.json` to activate the hooks. You can do this automatically with `./install.sh --activate-hooks` or answer `y` at the end-of-install TTY prompt.
+
+> **Re-install disclaimer (v0.1.1):** `install.sh` is idempotent for clean state but **overwrites kit-owned `_blocks/`, `_templates/`, `_assembler/`, `hooks/`, and `skills/` on re-run** — local modifications under those directories are backed up to `<dir>.bak-TIMESTAMP/` (or, for shared hook files, to `<file>.bak-TIMESTAMP`). User-owned `_manifests/*.toml` are never overwritten.
 
 ## What you get
 
@@ -38,6 +40,8 @@ After install, the only remaining step is merging `settings-snippet.json` into y
 | Hooks | 3 | `assemble-agents` (PostToolUse), `assemble-validate` (PreToolUse Bash), `no-hand-edit-agents` (PreToolUse Edit/Write) |
 | Skills | 6 | `new-agent`, `research`, `test-gen`, `pr-review`, `refactor`, `debug-deep` |
 
+Of the 34 blocks, the **8 base blocks** (`baseline`, `evidence-grading`, `memory-protocol`, `rule-pre-dev-gate`, `rule-test-first`, `rule-error-budget`, `rule-double-audit`, `rule-math-first`) are referenced directly by the 14 shipped manifests. The remaining **26 blocks** (`stack-*`, `deploy-*`, `api-*`, `scraper-*`, `domain-*`) are a library consumed by the `/new-agent` wizard: when you compose a project specialist, the wizard picks the appropriate stack / deploy / API / scraper / domain blocks and emits a manifest that references them. The kit's generic 14 agents do not import them by default.
+
 ## Creating a new agent
 
 Run the wizard in Claude Code:

_assembler/src/validator.rs

@@ -34,5 +34,120 @@ pub fn validate(m: &Manifest, blocks_dir: &Path) -> Result<(), String> {
         return Err("role must not be empty".into());
     }
 
+    check_no_placeholders(m)?;
+
     Ok(())
 }
+
+/// Reject manifests that still carry `{{PLACEHOLDER}}` tokens — the wizard
+/// should have substituted them. Emitted literally into generated .md they
+/// produce broken agents. Matches `{{...}}` conservatively (not single braces).
+fn check_no_placeholders(m: &Manifest) -> Result<(), String> {
+    let check = |field: &str, value: &str| -> Result<(), String> {
+        if contains_placeholder(value) {
+            Err(format!(
+                "Unsubstituted template placeholder in field '{field}': {value}. Did the wizard skip a substitution?"
+            ))
+        } else {
+            Ok(())
+        }
+    };
+
+    check("name", &m.name)?;
+    check("description", &m.description)?;
+    check("model", &m.model)?;
+    check("role", &m.role)?;
+    for (i, t) in m.tools.iter().enumerate() {
+        check(&format!("tools[{i}]"), t)?;
+    }
+    for (i, b) in m.blocks.iter().enumerate() {
+        check(&format!("blocks[{i}]"), b)?;
+    }
+    for (i, d) in m.domain_in.iter().enumerate() {
+        check(&format!("domain_in[{i}]"), d)?;
+    }
+    for (i, d) in m.forbidden_domain.iter().enumerate() {
+        check(&format!("forbidden_domain[{i}]"), d)?;
+    }
+    for (i, h) in m.handoff.iter().enumerate() {
+        check(&format!("handoff[{i}].target"), &h.target)?;
+        check(&format!("handoff[{i}].trigger"), &h.trigger)?;
+    }
+    for (i, o) in m.output_extra_fields.iter().enumerate() {
+        check(&format!("output_extra_fields[{i}]"), o)?;
+    }
+    if let Some(v) = &m.memory_project {
+        check("memory_project", v)?;
+    }
+    if let Some(v) = &m.project_claudemd {
+        check("project_claudemd", v)?;
+    }
+    if let Some(r) = &m.references {
+        for (i, e) in r.extra.iter().enumerate() {
+            check(&format!("references.extra[{i}]"), e)?;
+        }
+    }
+    Ok(())
+}
+
+fn contains_placeholder(s: &str) -> bool {
+    // Look for a `{{` with a matching `}}` later in the same string.
+    if let Some(start) = s.find("{{") {
+        if s[start + 2..].contains("}}") {
+            return true;
+        }
+    }
+    false
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::manifest::{Handoff, Manifest};
+
+    fn base() -> Manifest {
+        Manifest {
+            name: "test".into(),
+            description: "d".into(),
+            tools: vec!["Read".into()],
+            model: "opus".into(),
+            role: "r".into(),
+            blocks: vec!["baseline".into(), "evidence-grading".into(), "memory-protocol".into()],
+            domain_in: vec!["x".into()],
+            forbidden_domain: vec!["y".into()],
+            handoff: vec![Handoff { target: "a".into(), trigger: "b".into() }],
+            output_extra_fields: vec![],
+            memory_project: None,
+            project_claudemd: None,
+            references: None,
+        }
+    }
+
+    #[test]
+    fn rejects_placeholder_in_memory_project() {
+        let mut m = base();
+        m.memory_project = Some("{{MEMORY_PROJECT}}".into());
+        let err = check_no_placeholders(&m).unwrap_err();
+        assert!(err.contains("memory_project"), "err = {err}");
+        assert!(err.contains("{{MEMORY_PROJECT}}"), "err = {err}");
+    }
+
+    #[test]
+    fn accepts_single_braces() {
+        let mut m = base();
+        m.description = "hello {world}".into();
+        assert!(check_no_placeholders(&m).is_ok());
+    }
+
+    #[test]
+    fn accepts_empty_manifest() {
+        assert!(check_no_placeholders(&base()).is_ok());
+    }
+
+    #[test]
+    fn rejects_placeholder_in_role() {
+        let mut m = base();
+        m.role = "do {{THING}}".into();
+        assert!(check_no_placeholders(&m).is_err());
+    }
+}

hooks/assemble-agents.sh

@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 # PostToolUse(Edit|Write) — auto-regenerate agent .md files.
 #
 # Trigger logic:
@@ -27,9 +27,15 @@ case "$FILE" in
         "$ASSEMBLER" --in-place "$FILE" 2>&1 | sed 's/^/[assemble-agents] /'
         ;;
     */agents/_blocks/*.md)
-        # Block changed → rebuild everything (block is shared)
+        # Block changed → rebuild everything (block is shared).
+        # Always surface FAIL/ERROR lines; truncate only the OK tail.
         echo "[assemble-agents] block changed, rebuilding all agents..."
-        "$ASSEMBLER" --in-place 2>&1 | sed 's/^/[assemble-agents] /' | head -40
+        OUTPUT=$("$ASSEMBLER" --in-place 2>&1 || true)
+        FAILS=$(printf '%s\n' "$OUTPUT" | grep -E '^(FAIL|ERROR)' || true)
+        if [ -n "$FAILS" ]; then
+            printf '%s\n' "$FAILS" | sed 's/^/[assemble-agents] /'
+        fi
+        printf '%s\n' "$OUTPUT" | sed 's/^/[assemble-agents] /' | head -40
         ;;
     *)
         exit 0

hooks/assemble-validate.sh

@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 # PreToolUse(Bash) — validate all agent manifests before `git commit` in ~/.claude.
 #
 # Trigger: Bash command contains "git commit" AND current directory is under ~/.claude.

hooks/no-hand-edit-agents.sh

@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 # PreToolUse(Edit|Write) — block hand-editing generated agent .md files.
 #
 # Generated files start with: <!-- GENERATED by _assembler ...

install.sh

@@ -10,15 +10,75 @@ AGENTS_DIR="$HOME_DIR/.claude/agents"
 HOOKS_DIR="$HOME_DIR/.claude/hooks"
 SKILLS_DIR="$HOME_DIR/.claude/skills"
 
-say() { printf '\033[1;36m[install]\033[0m %s\n' "$*"; }
-warn() { printf '\033[1;33m[warn]\033[0m %s\n' "$*"; }
-err() { printf '\033[1;31m[error]\033[0m %s\n' "$*" >&2; }
+# --- flag parsing ----------------------------------------------------------
+ACTIVATE_HOOKS=0
+for arg in "$@"; do
+  case "$arg" in
+    --activate-hooks) ACTIVATE_HOOKS=1 ;;
+    --help|-h)
+      cat <<EOF
+Usage: ./install.sh [--activate-hooks]
+
+  --activate-hooks   jq-merge settings-snippet.json into ~/.claude/settings.json
+                     non-interactively. Without this flag, a TTY prompt asks
+                     at the end; non-TTY runs print manual instructions.
+EOF
+      exit 0
+      ;;
+  esac
+done
+
+# ANSI on iff stdout is a TTY and NO_COLOR is unset (respect no-color.org).
+if [ -t 1 ] && [ "${NO_COLOR:-}" = "" ]; then
+  COLOR=1
+else
+  COLOR=0
+fi
+if [ "$COLOR" = "1" ]; then
+  say()  { printf '\033[1;36m[install]\033[0m %s\n' "$*"; }
+  warn() { printf '\033[1;33m[warn]\033[0m %s\n' "$*"; }
+  err()  { printf '\033[1;31m[error]\033[0m %s\n' "$*" >&2; }
+else
+  say()  { printf '[install] %s\n' "$*"; }
+  warn() { printf '[warn] %s\n' "$*"; }
+  err()  { printf '[error] %s\n' "$*" >&2; }
+fi
+
+# --- rollback bookkeeping ---------------------------------------------------
+# Every successful backup_dir / per-file backup appends a "ORIGINAL|BACKUP"
+# pair to BACKUP_PAIRS. On ERR the trap walks the list in reverse and atomically
+# swaps BACKUP back onto ORIGINAL. A boolean guard makes rollback idempotent.
+BACKUP_PAIRS=()
+ROLLED_BACK=0
+
+rollback() {
+  [ "$ROLLED_BACK" = "1" ] && return 0
+  ROLLED_BACK=1
+  if [ "${#BACKUP_PAIRS[@]}" -eq 0 ]; then
+    err "install failed at line ${BASH_LINENO[0]:-?}; no backups to restore"
+    return 0
+  fi
+  warn "install failed — rolling back ${#BACKUP_PAIRS[@]} backup(s)"
+  local i pair orig bak
+  for (( i=${#BACKUP_PAIRS[@]}-1; i>=0; i-- )); do
+    pair="${BACKUP_PAIRS[$i]}"
+    orig="${pair%%|*}"
+    bak="${pair#*|}"
+    if [ -e "$bak" ]; then
+      rm -rf "$orig"
+      mv "$bak" "$orig"
+      say "  restored $orig from $bak"
+    fi
+  done
+  err "install failed at line ${BASH_LINENO[0]:-?}; rolled back"
+}
+trap rollback ERR
 
 # Backup a populated target directory to a timestamped sibling before clobber.
 # No-op if target is absent or contains no regular files (recursively). This
 # means freshly-mkdir'd scaffolds are NOT backed up — only real user content.
-# Only called on $AGENTS_DIR/_blocks, _templates, _assembler, $HOOKS_DIR,
-# $SKILLS_DIR — never on $KIT_DIR source.
+# Only called on $AGENTS_DIR/_blocks, _templates, _assembler, $SKILLS_DIR —
+# never on $KIT_DIR source. (hooks are now per-file; see backup_file.)
 backup_dir() {
   local target="$1"
   [ -d "$target" ] || return 0
@@ -28,9 +88,73 @@ backup_dir() {
   fi
   local backup="${target}.bak-$(date +%s)"
   cp -a "$target" "$backup"
+  BACKUP_PAIRS+=("$target|$backup")
   say "backed up existing $target to $backup"
 }
 
+# Per-file backup for shared directories like $HOOKS_DIR, where other kits
+# may drop sibling files we must not touch. Only the specific file is moved
+# aside to <file>.bak-TIMESTAMP.
+backup_file() {
+  local target="$1"
+  [ -f "$target" ] || return 0
+  local backup="${target}.bak-$(date +%s)"
+  mv "$target" "$backup"
+  BACKUP_PAIRS+=("$target|$backup")
+  say "backed up existing $target to $backup"
+}
+
+# Activate KeiSeiKit hooks by merging settings-snippet.json into the user's
+# settings.json. Idempotent:
+#   - If settings.json is absent, copy snippet verbatim (minus _comment key).
+#   - If present, concatenate the snippet's PostToolUse / PreToolUse entries
+#     onto existing arrays, then de-dupe by the nested hooks[].command field
+#     so re-runs do not stack duplicate entries.
+#   - .hooks itself (the root object key) is merged with `*` — snippet wins on
+#     scalar keys, arrays are unioned then de-duped.
+# Requires jq (already checked earlier in prerequisites). Writes atomically
+# via a tmpfile in the same dir.
+activate_hooks() {
+  local snippet="$KIT_DIR/settings-snippet.json"
+  local target="$HOME_DIR/.claude/settings.json"
+  local tmp
+  [ -f "$snippet" ] || { warn "no snippet at $snippet"; return 0; }
+  if [ ! -f "$target" ]; then
+    # Strip _comment, keep the rest. Create atomically.
+    tmp="$(mktemp "$target.XXXXXX")"
+    jq 'del(._comment)' "$snippet" > "$tmp"
+    mv "$tmp" "$target"
+    say "created $target from snippet (no prior settings.json)"
+    return 0
+  fi
+  # Merge: walk each matcher-group in PostToolUse / PreToolUse, append hooks,
+  # unique_by command. jq filter is written for readability, not golf.
+  tmp="$(mktemp "$target.XXXXXX")"
+  jq --slurpfile snip "$snippet" '
+    . as $orig
+    | ($snip[0] | del(._comment)) as $add
+    | reduce ($add.hooks | keys[]) as $phase ($orig;
+        .hooks[$phase] = (
+          ((.hooks[$phase] // []) + ($add.hooks[$phase] // []))
+          | group_by(.matcher)
+          | map({
+              matcher: .[0].matcher,
+              hooks: (map(.hooks // []) | add | unique_by(.command))
+            })
+        )
+      )
+  ' "$target" > "$tmp"
+  # Only replace if jq produced non-empty valid JSON
+  if [ -s "$tmp" ] && jq -e . "$tmp" >/dev/null 2>&1; then
+    mv "$tmp" "$target"
+    say "merged hooks into $target (idempotent)"
+  else
+    rm -f "$tmp"
+    err "jq-merge produced invalid output; $target unchanged"
+    return 1
+  fi
+}
+
 # --- prerequisites ----------------------------------------------------------
 say "checking prerequisites"
 if ! command -v cargo >/dev/null 2>&1; then
@@ -97,7 +221,12 @@ done
 say "  copied $copied, skipped $skipped (already present)"
 
 # --- copy template ---------------------------------------------------------
-if compgen -G "$KIT_DIR/_templates/*.template" >/dev/null; then
+# bash-3.2-portable glob detection: iterate, break on first hit.
+has_templates=0
+for t in "$KIT_DIR/_templates/"*.template; do
+  [ -f "$t" ] && { has_templates=1; break; }
+done
+if [ "$has_templates" = "1" ]; then
   say "copying specialist template"
   backup_dir "$AGENTS_DIR/_templates"
   cp -f "$KIT_DIR/_templates/"*.template "$AGENTS_DIR/_templates/"
@@ -113,9 +242,12 @@ if [[ -f "$KIT_DIR/_assembler/.gitignore" ]]; then
 fi
 
 # --- copy hooks (refresh; hooks are logic, not config) ---------------------
+# $HOOKS_DIR is shared with other kits — back up each KeiSeiKit-owned hook
+# individually rather than the whole directory, so foreign hooks are not
+# dragged into .bak-TIMESTAMP snapshots on every re-run.
 say "copying hooks -> $HOOKS_DIR/"
-backup_dir "$HOOKS_DIR"
 for h in assemble-agents.sh assemble-validate.sh no-hand-edit-agents.sh; do
+  backup_file "$HOOKS_DIR/$h"
   cp -f "$KIT_DIR/hooks/$h" "$HOOKS_DIR/$h"
   chmod +x "$HOOKS_DIR/$h"
 done
@@ -134,8 +266,13 @@ if [[ -d "$KIT_DIR/skills" ]]; then
 fi
 
 # --- build assembler -------------------------------------------------------
-say "building Rust assembler (cargo build --release)"
-( cd "$AGENTS_DIR/_assembler" && cargo build --release )
+# Prefer offline build (fresh-clone on a no-network machine should still work
+# if the registry cache is warm). Fall back to online fetch on failure.
+say "building Rust assembler (cargo build --release, offline first)"
+if ! ( cd "$AGENTS_DIR/_assembler" && cargo build --release --offline ) 2>/tmp/keiseikit-cargo-offline.log; then
+  say "offline build failed — fetching deps from crates.io"
+  ( cd "$AGENTS_DIR/_assembler" && cargo build --release )
+fi
 if [[ ! -x "$AGENTS_DIR/_assembler/target/release/assemble" ]]; then
   err "build succeeded but binary not found at $AGENTS_DIR/_assembler/target/release/assemble"
   exit 2
@@ -145,27 +282,39 @@ fi
 say "generating agent .md files (--in-place)"
 AGENT_ROOT="$AGENTS_DIR" "$AGENTS_DIR/_assembler/target/release/assemble" --in-place
 
+# --- activate hooks (flag, or interactive prompt on TTY) -------------------
+SETTINGS_FILE="$HOME_DIR/.claude/settings.json"
+DID_ACTIVATE=0
+if [ "$ACTIVATE_HOOKS" = "1" ]; then
+  say "activating hooks (--activate-hooks)"
+  activate_hooks && DID_ACTIVATE=1
+elif [ ! -f "$SETTINGS_FILE" ]; then
+  # No existing settings — merge is trivial, do it unconditionally.
+  say "no existing settings.json; installing snippet"
+  activate_hooks && DID_ACTIVATE=1
+elif [ -t 0 ] && [ -t 1 ]; then
+  if [ "$COLOR" = "1" ]; then
+    printf '\033[1;36m[install]\033[0m activate hooks now? [y/N] '
+  else
+    printf '[install] activate hooks now? [y/N] '
+  fi
+  read -r reply
+  case "$reply" in
+    y|Y|yes|YES) activate_hooks && DID_ACTIVATE=1 ;;
+    *) say "skipping hook activation" ;;
+  esac
+fi
+
 # --- done -----------------------------------------------------------------
 echo
 say "install complete"
 echo
-cat <<EOF
+if [ "$DID_ACTIVATE" = "1" ]; then
+  cat <<EOF
 ==========================================================================
-  NEXT STEP: merge settings-snippet.json into ~/.claude/settings.json
+  Hooks activated. Settings merged into $SETTINGS_FILE
 ==========================================================================
 
-  KeiSeiKit ships 3 hooks (assemble-agents, assemble-validate, no-hand-edit).
-  To activate them, merge entries from:
-      $KIT_DIR/settings-snippet.json
-  into your:
-      $HOME_DIR/.claude/settings.json
-
-  If you have no settings.json yet, you can simply copy the snippet:
-      cp $KIT_DIR/settings-snippet.json $HOME_DIR/.claude/settings.json
-
-  Otherwise, open both files and append the PostToolUse / PreToolUse
-  entries to the matching arrays in your existing settings.json.
-
   To verify install:
       ls $AGENTS_DIR/*.md   # should show ~14 generated agents
       $AGENTS_DIR/_assembler/target/release/assemble --validate
@@ -175,3 +324,28 @@ cat <<EOF
 
 ==========================================================================
 EOF
+else
+  cat <<EOF
+==========================================================================
+  NEXT STEP: merge settings-snippet.json into ~/.claude/settings.json
+==========================================================================
+
+  KeiSeiKit ships 3 hooks (assemble-agents, assemble-validate, no-hand-edit).
+  To activate them, merge entries from:
+      $KIT_DIR/settings-snippet.json
+  into your:
+      $SETTINGS_FILE
+
+  Or re-run with automatic activation:
+      ./install.sh --activate-hooks
+
+  To verify install:
+      ls $AGENTS_DIR/*.md   # should show ~14 generated agents
+      $AGENTS_DIR/_assembler/target/release/assemble --validate
+
+  To create a new project-specialist agent:
+      /new-agent
+
+==========================================================================
+EOF
+fi