diff --git a/CLAUDE.md b/CLAUDE.md index e53b65f..bb0d45e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -182,6 +182,8 @@ On session start, run `/engage` to detect platform, resolve identity, load conte If the session was started with `--channels`, a Discord watcher will push notifications. See `docs/discord-watcher.md` for the addressing convention, signature format, and echo-filter rules. +The MCP fleet writes a unified structured-event log to `~/.claude/logs/mcp.jsonl`. Rotation policy and operational commands: `docs/operations/log-rotation.md`. + --- ## MANDATORY: Post-Compaction Rules Confirmation diff --git a/assets/logrotate/cc-mcp-logs b/assets/logrotate/cc-mcp-logs new file mode 100644 index 0000000..b769e06 --- /dev/null +++ b/assets/logrotate/cc-mcp-logs @@ -0,0 +1,27 @@ +# /etc/logrotate.d/cc-mcp-logs — MCP fleet structured-log rotation policy +# +# Rotates the unified Claude Code MCP log aggregator at +# ~/.claude/logs/mcp.jsonl when it reaches 100 MB OR daily, whichever +# comes first. Keeps 14 rotations, gzip-compressed except for the most +# recent (delaycompress) so a live grep/tail of the previous rotation +# stays human-readable for debugging. +# +# Templated: the {{HOME}} marker is replaced with the installing user's +# home directory at install time by `install --with-logrotate`. +# +# copytruncate is load-bearing — MCP servers do not handle SIGHUP, so a +# rename-and-reopen rotation would orphan the original file descriptor +# and silently drop subsequent log writes. With copytruncate, the log +# file's inode is preserved; a small write window during the copy may +# duplicate or interleave but cannot lose entries. + +{{HOME}}/.claude/logs/mcp.jsonl { + size 100M + daily + rotate 14 + compress + delaycompress + copytruncate + missingok + notifempty +} diff --git a/config/settings.template.json b/config/settings.template.json index 6125a70..ba3682b 100644 --- a/config/settings.template.json +++ b/config/settings.template.json @@ -62,7 +62,7 @@ "hooks": [ { "type": "command", - "command": "~/.local/bin/hooks/nerf/session-start-compact.sh" + "command": "~/.claude/scripts/hooks/nerf/session-start-compact.sh" } ] } @@ -73,7 +73,7 @@ "hooks": [ { "type": "command", - "command": "~/.local/bin/hooks/nerf/pre-compact.sh" + "command": "~/.claude/scripts/hooks/nerf/pre-compact.sh" } ] } diff --git a/docs/operations/log-rotation.md b/docs/operations/log-rotation.md new file mode 100644 index 0000000..0459cd0 --- /dev/null +++ b/docs/operations/log-rotation.md @@ -0,0 +1,167 @@ +# MCP Log Rotation + +The Claude Code kit's MCP fleet writes a unified structured-event log to +`~/.claude/logs/mcp.jsonl`. At ~12 events/sec sustained, the file accrues +roughly 290k events / day and grows on the order of 50 MB / day. Without +rotation, the aggregator reaches multi-GB sizes within weeks — slow to +grep, slow to tail, and an eventual disk-fill risk on developer machines. + +The kit ships a `logrotate(8)` policy that lands at +`/etc/logrotate.d/cc-mcp-logs` during install. This doc describes the +policy, the operational commands, and the macOS escape hatch. + +## Scope and platform + +**Linux only.** `logrotate` is in every standard Linux distro and the kit +relies on the system-wide `cron.daily` (or systemd timer) hook to invoke +it. The `install --with-logrotate` flag is a no-op on macOS / non-Linux +hosts — the installer detects `uname` and skips the step with a notice. + +**macOS users:** macOS ships `newsyslog(8)` instead of `logrotate`. The +cc-workflow kit does not currently include a `newsyslog` config; the +recommended workaround is one of: + +- Manual rotation: `mv ~/.claude/logs/mcp.jsonl{,.$(date +%Y%m%d)} && + : > ~/.claude/logs/mcp.jsonl` on a personal cron / launchd schedule. +- Native `newsyslog` config under `/etc/newsyslog.d/cc-mcp-logs.conf` + with `B` (binary mode, do not insert "logfile turned over" markers) + and a size threshold. PRs adding this template welcomed. + +If macOS support becomes load-bearing, a separate issue should track it. + +## Policy + +``` +~/.claude/logs/mcp.jsonl { + size 100M + daily + rotate 14 + compress + delaycompress + copytruncate + missingok + notifempty +} +``` + +| Directive | Effect | +| --------------- | ------------------------------------------------------------------------------------- | +| `size 100M` | Rotate when file exceeds 100 MB, regardless of cadence. | +| `daily` | Also rotate every day at the system's `cron.daily` time, even if under the size cap. | +| `rotate 14` | Keep 14 historical files; the 15th oldest is unlinked. | +| `compress` | Gzip rotated files (`.1.gz`, `.2.gz`, ...). | +| `delaycompress` | Skip compressing the most-recent rotation (so `.1` stays plaintext for grep/tail). | +| `copytruncate` | Copy contents to the rotated path, then truncate the live file in place. | +| `missingok` | Do not error if the log file does not exist yet (fresh installs). | +| `notifempty` | Do not rotate an empty file. | + +### Why `copytruncate` (load-bearing) + +The conventional `logrotate` workflow is rename-and-reopen: + +1. `mv mcp.jsonl mcp.jsonl.1` +2. Send SIGHUP (or equivalent) to the writing process so it reopens its + log file descriptor at the original path. + +The kit's MCP servers do **not** handle SIGHUP. They open their log +once at startup and hold the file descriptor for the lifetime of the +process. A rename-and-reopen rotation would orphan the file descriptor — +the OS would keep `mcp.jsonl.1` open under the writing process while the +new `mcp.jsonl` sat on disk receiving zero events. Silent log loss. + +`copytruncate` solves this by preserving the inode: the rotation runs +`cp mcp.jsonl mcp.jsonl.1; : > mcp.jsonl`. The MCP server's open file +descriptor still points at the same inode, which has been truncated to +zero. Subsequent writes append to the (now empty) file. There is a small +window during the copy where new events may write into both the source +(before truncate) and the rotated copy (since the copy started); these +are duplicates, not losses, and the timestamp ordering is preserved. + +For comparison, the alternative — adding SIGHUP handling to every MCP +server — is a fleet-wide refactor of process lifecycle code. Not worth +it for a log rotation policy. + +## Install / uninstall + +```bash +# Install (interactive prompt) +./install + +# Install non-interactively +./install --with-logrotate + +# Skip rotation install (still installs the rest of the kit) +./install --without-logrotate + +# Check whether rotation is in place +./install --check +``` + +`./install --with-logrotate` does the following: + +1. Detects platform; bails out cleanly on non-Linux. +2. Renders `assets/logrotate/cc-mcp-logs` with the current user's home. +3. Drops it to `/etc/logrotate.d/cc-mcp-logs` via `sudo install -m 0644`. +4. Runs `sudo logrotate -d /etc/logrotate.d/cc-mcp-logs` for syntax / + target validation, surfaces stderr. + +## Operational commands + +```bash +# Validate config without rotating (dry-run, prints what would happen) +sudo logrotate -d /etc/logrotate.d/cc-mcp-logs + +# Force-rotate now, ignoring size and cadence thresholds +sudo logrotate -f /etc/logrotate.d/cc-mcp-logs + +# Inspect last rotation time +ls -lh ~/.claude/logs/mcp.jsonl* + +# Tail the most-recent rotation (uncompressed thanks to delaycompress) +tail -F ~/.claude/logs/mcp.jsonl.1 +``` + +## Disabling temporarily + +For short-term high-volume debugging where you want every event in +one file: + +```bash +# Move the config aside; logrotate will skip it +sudo mv /etc/logrotate.d/cc-mcp-logs{,.disabled} + +# ...debug session here... + +# Re-enable +sudo mv /etc/logrotate.d/cc-mcp-logs{.disabled,} +``` + +Or comment out the entire stanza in place. The kit's `--check` mode +reports the config as missing while it is `.disabled`. + +## Removing the policy + +```bash +sudo rm /etc/logrotate.d/cc-mcp-logs +``` + +Existing `.gz` archives under `~/.claude/logs/` are unaffected. To +reclaim disk space: + +```bash +rm ~/.claude/logs/mcp.jsonl.*.gz +``` + +## Verification on a Linux test box + +The full operator-side verification list (per cc-workflow#540): + +1. `./install --with-logrotate` succeeds on a fresh checkout. +2. `sudo logrotate -d /etc/logrotate.d/cc-mcp-logs` reports no errors. +3. `sudo logrotate -f /etc/logrotate.d/cc-mcp-logs` produces + `mcp.jsonl.1` (uncompressed); MCP servers continue writing to + `mcp.jsonl` without restart; no log lines lost. +4. After a second forced run: `mcp.jsonl.1.gz` exists and `mcp.jsonl.1` + is the most-recent rotation (delaycompress confirmed). +5. `./install --check` reports the rotation config as present and + surfaces the last-rotation mtime. diff --git a/install b/install index 3bf7da0..bdeb3a6 100755 --- a/install +++ b/install @@ -5,17 +5,20 @@ # into the correct locations on the local machine. # # Usage: -# ./install Install everything -# ./install --dry-run Show what would be done without doing it -# ./install --check Show drift between repo and installed versions -# ./install --skills Install skills only -# ./install --scripts Install scripts only (also builds packages from src/) -# ./install --config Install config files only (smart-merges settings.json) -# ./install --mcps Install MCP servers only (via mcps.json manifest) -# ./install --crystallizer Install context-crystallizer only -# ./install --no-mcps Install everything except MCP servers -# ./install --prune Remove installed files whose source no longer exists -# ./install --yes Non-interactive (assume yes for --prune confirmations) +# ./install Install everything +# ./install --dry-run Show what would be done without doing it +# ./install --check Show drift between repo and installed versions +# ./install --skills Install skills only +# ./install --scripts Install scripts only (also builds packages from src/) +# ./install --config Install config files only (smart-merges settings.json) +# ./install --mcps Install MCP servers only (via mcps.json manifest) +# ./install --crystallizer Install context-crystallizer only +# ./install --no-mcps Install everything except MCP servers +# ./install --with-logrotate Install logrotate policy for ~/.claude/logs/mcp.jsonl +# (Linux only; no-op elsewhere). Implies non-interactive. +# ./install --without-logrotate Skip logrotate install (suppresses the prompt). +# ./install --prune Remove installed files whose source no longer exists +# ./install --yes Non-interactive (assume yes for --prune confirmations) # # Recursion: scripts/ is walked recursively. Files at scripts//path are # mirrored to ~/.local/bin//path with executable bit preserved. Excluded @@ -39,6 +42,11 @@ REPO_DIR="$(cd "$(dirname "$0")" && pwd)" SKILLS_DIR="$HOME/.claude/skills" SCRIPTS_DIR="$HOME/.local/bin" CLAUDE_DIR="$HOME/.claude" +# Cellar: kit-owned scripts directory (Homebrew/Nix pattern). Wiped and +# recreated on every install — this is what eliminates orphan rot. Anything +# that landed here in a prior install is fair game for deletion. SCRIPTS_DIR +# becomes a symlink farm pointing into here for entries that need PATH. +CELLAR_DIR="$HOME/.claude/scripts" DRY_RUN=false CHECK_MODE=false PRUNE_MODE=false @@ -48,6 +56,8 @@ INSTALL_SCRIPTS=true INSTALL_CONFIG=true INSTALL_MCPS=true INSTALL_CRYSTALLIZER=true +# Logrotate is a tri-state: "prompt" by default, forced on/off by flag. +LOGROTATE_MODE=prompt # Subtree names excluded from the recursive scripts/ walk. SCRIPTS_EXCLUDE_NAMES=(tests fixtures __pycache__ .pytest_cache) @@ -102,6 +112,14 @@ while [[ $# -gt 0 ]]; do INSTALL_MCPS=false shift ;; + --with-logrotate) + LOGROTATE_MODE=on + shift + ;; + --without-logrotate) + LOGROTATE_MODE=off + shift + ;; --prune) PRUNE_MODE=true shift @@ -219,6 +237,238 @@ do_check() { fi } +# --- Cellar + symlink-farm helpers -------------------------------------------- +# Cellar = $CELLAR_DIR (kit-owned). Wiped and recreated each install. +# Symlink farm = $SCRIPTS_DIR — only top-level Cellar entries (recommendation +# B from #560: subtrees like hooks/, vox-providers/ stay Cellar-only and are +# invoked by absolute path). + +# Resolve a symlink's target to a normalized absolute path. Uses readlink -f +# when available (GNU), falls back to a portable cd-and-pwd dance for BSD. +# Result printed to stdout. Empty string if the path can't be resolved. +resolve_symlink_target() { + local link="$1" target + # Plain readlink (no -f) gives the literal target text. + target=$(readlink "$link" 2>/dev/null || true) + [[ -z "$target" ]] && return 0 + # Make absolute relative to the link's directory if needed. + if [[ "$target" != /* ]]; then + target="$(cd "$(dirname "$link")" 2>/dev/null && pwd)/$target" + fi + # Normalize ~ (readlink can't do this for us if user's HOME has tilde + # in raw target — uncommon but harmless to handle). + target="${target/#\~/$HOME}" + echo "$target" +} + +# Wipe and recreate the Cellar from $REPO_DIR/scripts/. Preserves directory +# structure and executable bits. Excludes ci/ and noise subtrees. +# Honors $DRY_RUN. +cellar_deploy() { + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would wipe and redeploy $CELLAR_DIR from $REPO_DIR/scripts/" + return 0 + fi + # Wipe — this is the structural orphan-killer. Anything that was in + # Cellar from a prior install but is no longer in the repo dies here. + rm -rf "$CELLAR_DIR" + mkdir -p "$CELLAR_DIR" + while IFS= read -r rel; do + [[ "$rel" == ci/* ]] && continue + local src="$REPO_DIR/scripts/$rel" + local dest="$CELLAR_DIR/$rel" + mkdir -p "$(dirname "$dest")" + cp "$src" "$dest" + # Preserve exec bit (top-level scripts always +x for backwards compat). + if [[ "$rel" != */* ]]; then + chmod +x "$dest" + elif [[ -x "$src" ]]; then + chmod +x "$dest" + fi + done < <(enumerate_scripts) + info "Cellar redeployed: $CELLAR_DIR ($(find "$CELLAR_DIR" -type f | wc -l) files)" +} + +# Drop a single file (skill helper or built artifact) into the Cellar at +# $CELLAR_DIR/. Preserves +x. +cellar_install_file() { + local src="$1" name="$2" + local dest="$CELLAR_DIR/$name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) $src → $dest (Cellar)" + return 0 + fi + mkdir -p "$(dirname "$dest")" + cp "$src" "$dest" + chmod +x "$dest" +} + +# Drop a skill helper into a per-skill Cellar subdir at +# $CELLAR_DIR/skills//. Preserves +x. Distinct from +# cellar_install_file() so two skills shipping same-named helpers (e.g. both +# with helper.sh) cannot silently overwrite each other in the Cellar. +cellar_install_skill_helper() { + local src="$1" skill_name="$2" helper_name="$3" + local dest="$CELLAR_DIR/skills/$skill_name/$helper_name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) $src → $dest (Cellar/skills)" + return 0 + fi + mkdir -p "$(dirname "$dest")" + cp "$src" "$dest" + chmod +x "$dest" +} + +# Create or refresh a symlink at $SCRIPTS_DIR/ pointing to +# $CELLAR_DIR/skills//. Bare-name target preserves +# PATH-based skill invocation (e.g. `slackbot-send`, `file-opener`). If a +# symlink with this name already exists pointing into a *different* skill's +# Cellar subdir, warn loudly — this is the namespace-collision case the +# per-skill Cellar layout was designed to surface. +farm_symlink_skill_helper() { + local skill_name="$1" helper_name="$2" + local target="$CELLAR_DIR/skills/$skill_name/$helper_name" + local link="$SCRIPTS_DIR/$helper_name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) symlink $link → $target" + return 0 + fi + if [[ -L "$link" ]]; then + local existing + existing=$(resolve_symlink_target "$link") + # Existing symlink into a *different* skill's helper → collision. + if [[ "$existing" == "$CELLAR_DIR/skills/"* && "$existing" != "$target" ]]; then + warn "Helper name collision for '$helper_name': $existing already farmed; overwriting with $target" + fi + fi + mkdir -p "$(dirname "$link")" + ln -sf "$target" "$link" +} + +# If $SCRIPTS_DIR/ exists as a plain file (not a symlink), it's a user +# customization or a leftover from the pre-Cellar layout. Back it up to +# .bak before we replace it with a symlink. No-op if it's already a +# symlink (or doesn't exist). +safeguard_user_file() { + local name="$1" + local path="$SCRIPTS_DIR/$name" + # Symlink → already farm-managed, leave alone (will be overwritten below). + [[ -L "$path" ]] && return 0 + # No plain file → nothing to safeguard. + [[ -e "$path" ]] || return 0 + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would back up plain file $path → ${path}.bak" + return 0 + fi + cp "$path" "${path}.bak" + rm -f "$path" + warn "Backed up user-customized $path → ${path}.bak" +} + +# Create or refresh a single symlink at $SCRIPTS_DIR/ pointing into +# the Cellar. The target is $CELLAR_DIR/. Caller is responsible for +# safeguarding any plain-file occupant first via safeguard_user_file(). +farm_symlink() { + local name="$1" + local target="$CELLAR_DIR/$name" + local link="$SCRIPTS_DIR/$name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) symlink $link → $target" + return 0 + fi + mkdir -p "$(dirname "$link")" + ln -sf "$target" "$link" +} + +# Walk $SCRIPTS_DIR and remove symlinks that point into the Cellar but whose +# target no longer exists. This is the structural reaper that catches both +# removed-from-repo cases (e.g. discord-bot in PR #283) and rename cases. +# Foreign symlinks (target NOT under $CELLAR_DIR) are NEVER touched. +reap_stale_cellar_symlinks() { + [[ -d "$SCRIPTS_DIR" ]] || return 0 + local removed=0 + while IFS= read -r link; do + [[ -L "$link" ]] || continue + local target + target=$(resolve_symlink_target "$link") + # Foreign symlink (target outside Cellar) → leave alone. + [[ "$target" == "$CELLAR_DIR"/* || "$target" == "$CELLAR_DIR" ]] || continue + # Target exists → live link, skip. + [[ -e "$target" ]] && continue + # Target missing under Cellar → stale. Reap it. + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would remove stale symlink: $link → $target" + else + rm -f "$link" + info "Reaped stale symlink: $link → $target" + fi + removed=$((removed + 1)) + done < <(find "$SCRIPTS_DIR" -maxdepth 1 -type l 2>/dev/null) + if [[ $removed -eq 0 ]]; then + skip "No stale symlinks under $SCRIPTS_DIR (Cellar reaper)" + fi +} + +# Enumerate the basenames that should appear in $SCRIPTS_DIR as symlinks +# pointing into $CELLAR_DIR. Granularity B: only top-level entries from +# scripts/ (subtrees like hooks/, vox-providers/, testing/ stay Cellar-only). +# ci/ is excluded as before. +# +# Uses `find ... | sed` rather than GNU's `-printf '%f\n'` because BSD/macOS +# find lacks -printf and silently emits nothing — which would empty the +# symlink farm on macOS and silently break PATH discoverability (AC #2). +# Same fix-pattern enumerate_scripts uses above. +enumerate_farm_targets() { + (cd "$REPO_DIR/scripts" && find . -maxdepth 1 -type f | sed 's|^\./||' | sort) +} + +# In a $HOME/.claude/settings.json, rewrite any hook command path of the +# shape "~/.local/bin/hooks/..." to "~/.claude/scripts/hooks/...". Walks +# every hooks.[].hooks[].command and applies a literal-string replace. +# Idempotent. Modifies $1 in place via tmpfile rotation. +rewrite_old_hook_paths_in_settings() { + local target="$1" + [[ -f "$target" ]] || return 0 + command -v jq &>/dev/null || return 0 + # jq walk: for any string ending up under hooks.*.hooks.*.command, + # rewrite leading "~/.local/bin/hooks/" → "~/.claude/scripts/hooks/". + local rewritten + rewritten=$(jq ' + if .hooks then + .hooks |= with_entries( + if .key == "_comment" then . else + .value |= map( + .hooks |= map( + if .command and (.command | startswith("~/.local/bin/hooks/")) then + .command = ("~/.claude/scripts/hooks/" + (.command | ltrimstr("~/.local/bin/hooks/"))) + else . end + ) + ) + end + ) + else . end + ' "$target") + echo "$rewritten" >"$target" +} + +# Detect any settings.json hook command paths of the old form. Prints the +# offending event names (one per line) for use in --check drift reporting. +detect_old_hook_paths_in_settings() { + local target="$1" + [[ -f "$target" ]] || return 0 + command -v jq &>/dev/null || return 0 + jq -r ' + .hooks // {} | to_entries[] | + select(.key != "_comment") | + . as $e | + .value | + map(.hooks // []) | add // [] | + map(.command // "") | + map(select(startswith("~/.local/bin/hooks/"))) | + if length > 0 then $e.key else empty end + ' "$target" 2>/dev/null +} + merge_settings() { local template="$1" target="$2" @@ -230,10 +480,28 @@ merge_settings() { cp "$target" "${target}.bak" + # Migrate any pre-Cellar hook command paths (~/.local/bin/hooks/...) to + # the new Cellar location (~/.claude/scripts/hooks/...) BEFORE merge so + # the union step compares apples to apples (template uses new paths). + local old_path_events + old_path_events=$(detect_old_hook_paths_in_settings "$target") + if [[ -n "$old_path_events" ]]; then + rewrite_old_hook_paths_in_settings "$target" + while IFS= read -r ev; do + [[ -z "$ev" ]] && continue + info "hooks.$ev — migrated old path(s) ~/.local/bin/hooks/... → ~/.claude/scripts/hooks/..." + done <<<"$old_path_events" + fi + # Deep merge: template into local, preserving user customizations # Rules: # 1. Top-level keys (scalars AND objects like statusLine): added only if ABSENT locally - # 2. hooks: add missing event keys from template; leave existing alone + # 2. hooks: + # - Event keys missing locally: added from template + # - Event keys present in both: matcher arrays unioned by .matcher value + # (template matcher entries whose .matcher is not in the local array + # are appended; existing local entries are left untouched, preserving + # user customizations to the per-matcher hooks list) # 3. permissions.allow: union of both arrays (deduplicated) # 4. enabledPlugins: add missing keys from template; leave existing alone # 5. _comment keys: stripped from result (template-only documentation) @@ -250,12 +518,26 @@ merge_settings() { # Capture local keys before entering reduce (.[1] | keys) as $local_keys | - # Merge hooks: add missing event keys + # Merge hooks: add missing event keys AND union matcher arrays for shared keys ((.[0].hooks // {}) | to_entries | map( select(.key != "_comment") )) as $tpl_hooks | + ((.[1].hooks // {}) | to_entries | map( + select(.key != "_comment") + )) as $local_hooks | ((.[1].hooks // {}) | keys) as $local_hook_keys | + # New event keys (template only) — add wholesale ($tpl_hooks | map(select(.key | IN($local_hook_keys[]) | not))) as $new_hooks | + # Shared event keys — union their matcher arrays by .matcher value + ($tpl_hooks | map(select(.key | IN($local_hook_keys[]))) | map({ + key: .key, + value: ( + (.value // []) as $tpl_arr | + (($local_hooks | from_entries)[.key] // []) as $local_arr | + ($local_arr | map(.matcher)) as $local_matchers | + $local_arr + ($tpl_arr | map(select(.matcher as $m | $local_matchers | index($m) | not))) + ) + }) | from_entries) as $merged_shared_hooks | # Merge permissions.allow: union ((.[0].permissions.allow // []) + (.[1].permissions.allow // []) | unique) as $merged_perms | @@ -268,7 +550,7 @@ merge_settings() { # Build result: start with local, add missing pieces .[1] | .permissions.allow = $merged_perms - | .hooks = ((.hooks // {}) + ($new_hooks | from_entries)) + | .hooks = ((.hooks // {}) + ($new_hooks | from_entries) + $merged_shared_hooks) | .enabledPlugins = ((.enabledPlugins // {}) + ($new_plugins | from_entries)) | reduce ($tpl_defaults[] | select(.key | IN($local_keys[]) | not)) as $s (.; .[$s.key] = $s.value) | del(._comment) @@ -279,10 +561,24 @@ merge_settings() { # Report what changed - # Report new hooks + # Report new hooks (and new matchers within shared event keys) for hook_event in $(jq -r '.hooks // {} | keys[] | select(. != "_comment")' "$template"); do if jq -e ".hooks.\"${hook_event}\"" "${target}.bak" &>/dev/null; then - skip "hooks.$hook_event — already present (skipped)" + # Event already exists locally — diff matcher arrays and report any + # template matchers that were appended by the union-merge. + local added_matchers + added_matchers=$(jq -r --arg ev "$hook_event" --slurpfile bak "${target}.bak" ' + (($bak[0].hooks[$ev] // []) | map(.matcher)) as $local_ms | + (.hooks[$ev] // []) | map(.matcher) | map(select(. as $m | $local_ms | index($m) | not)) | .[] + ' "$target") + if [[ -z "$added_matchers" ]]; then + skip "hooks.$hook_event — already present (skipped)" + else + while IFS= read -r m; do + [[ -z "$m" ]] && continue + info "hooks.$hook_event — matcher \"$m\" added" + done <<<"$added_matchers" + fi else info "hooks.$hook_event — added" fi @@ -309,6 +605,103 @@ merge_settings() { fi } +# --- Logrotate helpers -------------------------------------------------------- +# Constants for the logrotate policy. Source template lives under +# assets/logrotate/cc-mcp-logs and contains a {{HOME}} marker that must be +# rendered to the installing user's home before dropping into /etc. +LOGROTATE_SRC="$REPO_DIR/assets/logrotate/cc-mcp-logs" +LOGROTATE_DEST="/etc/logrotate.d/cc-mcp-logs" +LOGROTATE_LOGS_DIR="$HOME/.claude/logs" + +# True iff this host has a logrotate(8) we can drive. Linux + logrotate on PATH. +logrotate_supported() { + [[ "$(uname -s)" == "Linux" ]] && command -v logrotate &>/dev/null +} + +# Render the templated config to stdout: substitute {{HOME}} with $HOME. +render_logrotate_template() { + # Use a literal sed delimiter that cannot appear in a Linux home path. + # Any character is fine; we use '|' since absolute paths contain '/'. + sed "s|{{HOME}}|$HOME|g" "$LOGROTATE_SRC" +} + +# Install the rendered config to /etc/logrotate.d/cc-mcp-logs and run a +# dry-run validation. Returns non-zero on failure. +install_logrotate_config() { + local tmp + tmp="$(mktemp)" + render_logrotate_template >"$tmp" + + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would install logrotate config to $LOGROTATE_DEST" + info "(dry-run) Would run: sudo logrotate -d $LOGROTATE_DEST" + rm -f "$tmp" + return 0 + fi + + if sudo install -m 0644 "$tmp" "$LOGROTATE_DEST"; then + info "Installed $LOGROTATE_DEST" + else + warn "Failed to install $LOGROTATE_DEST (sudo install)" + rm -f "$tmp" + return 1 + fi + rm -f "$tmp" + + # Dry-run validation: logrotate -d prints what it would do without rotating. + if sudo logrotate -d "$LOGROTATE_DEST" >/dev/null 2>&1; then + ok "logrotate -d validation passed" + else + warn "logrotate -d reported errors — re-run manually to diagnose:" + warn " sudo logrotate -d $LOGROTATE_DEST" + return 1 + fi +} + +# Report logrotate status for --check mode. +# Returns 0 if all-in-sync, 1 if any drift. +check_logrotate_status() { + if ! logrotate_supported; then + info "logrotate (skipped — non-Linux or logrotate not installed)" + return 0 + fi + local d=0 + if [[ ! -f "$LOGROTATE_DEST" ]]; then + drift "logrotate config $LOGROTATE_DEST — NOT INSTALLED" + return 1 + fi + # Compare installed vs rendered template; report drift if differs. + local rendered + rendered="$(mktemp)" + render_logrotate_template >"$rendered" + if ! sudo diff -q "$rendered" "$LOGROTATE_DEST" &>/dev/null; then + drift "logrotate config $LOGROTATE_DEST — DIFFERS from repo template" + d=1 + else + info "logrotate config $LOGROTATE_DEST (in sync)" + fi + rm -f "$rendered" + # Last-rotation mtime: any mcp.jsonl.1 / mcp.jsonl.1.gz / mcp.jsonl.N[.gz] + # under the logs dir indicates the rotation has fired at least once. + if [[ -d "$LOGROTATE_LOGS_DIR" ]]; then + local newest + newest=$(find "$LOGROTATE_LOGS_DIR" -maxdepth 1 \ + \( -name 'mcp.jsonl.[0-9]*' -o -name 'mcp.jsonl.[0-9]*.gz' \) \ + -type f -printf '%T@ %p\n' 2>/dev/null | sort -nr | head -1) + if [[ -n "$newest" ]]; then + local newest_path newest_mtime + newest_path="${newest#* }" + newest_mtime=$(stat -c '%y' "$newest_path" 2>/dev/null || echo 'unknown') + info "last rotation: $newest_path ($newest_mtime)" + else + info "no rotated files yet under $LOGROTATE_LOGS_DIR — rotation has not fired" + fi + else + info "$LOGROTATE_LOGS_DIR does not exist yet — no rotation history" + fi + return $d +} + # --- Prune mode --------------------------------------------------------------- # Walk previously-installed files (manifest + current source set), remove the # ones whose source under scripts/ no longer exists. Backed-up to .bak @@ -389,7 +782,7 @@ if [[ "$CHECK_MODE" == true ]]; then total=$((total + 1)) do_check "$src" "$dest" "$skill_name" || drifted=$((drifted + 1)) fi - # Check non-SKILL.md files: .md files in skill dir, others in scripts dir + # Check non-SKILL.md files: .md files in skill dir, helpers in Cellar. for helper in "$skill_dir"/*; do [[ -f "$helper" ]] || continue helper_name="$(basename "$helper")" @@ -398,7 +791,7 @@ if [[ "$CHECK_MODE" == true ]]; then if [[ "$helper_name" == *.md ]]; then do_check "$helper" "$SKILLS_DIR/$skill_name/$helper_name" "$skill_name/$helper_name" || drifted=$((drifted + 1)) else - do_check "$helper" "$SCRIPTS_DIR/$helper_name" "$skill_name/$helper_name" || drifted=$((drifted + 1)) + do_check "$helper" "$CELLAR_DIR/$helper_name" "$skill_name/$helper_name" || drifted=$((drifted + 1)) fi done # Check content subdirectories (e.g., tours/) @@ -415,17 +808,19 @@ if [[ "$CHECK_MODE" == true ]]; then done echo "" - echo "Scripts" + echo "Scripts (Cellar: $CELLAR_DIR)" echo "──────────────────────────────────────────" while IFS= read -r rel; do [[ "$rel" == ci/* ]] && continue src="$REPO_DIR/scripts/$rel" - dest="$SCRIPTS_DIR/$rel" + dest="$CELLAR_DIR/$rel" total=$((total + 1)) do_check "$src" "$dest" "$rel" || drifted=$((drifted + 1)) done < <(enumerate_scripts) - # Orphans: installed files whose source under scripts/ no longer exists. - # (Detected via union of source set + previous-install manifest.) + # Legacy orphan detector (manifest-based) — kept for backwards compat. + # Cellar wipe handles structural orphan rot at install time, but a user + # running --check on a host that hasn't installed since the migration may + # still see orphans in $SCRIPTS_DIR; surface them. while IFS= read -r orphan; do [[ -z "$orphan" ]] && continue total=$((total + 1)) @@ -433,6 +828,60 @@ if [[ "$CHECK_MODE" == true ]]; then drifted=$((drifted + 1)) done < <(enumerate_orphans) + echo "" + echo "Symlink farm ($SCRIPTS_DIR → Cellar)" + echo "──────────────────────────────────────────" + # Top-level Cellar entries should be reachable via $SCRIPTS_DIR/. + # Either as a symlink pointing into Cellar, or — pre-migration — as a + # plain file (which will be backed up + replaced on the next install). + while IFS= read -r name; do + [[ -z "$name" ]] && continue + total=$((total + 1)) + link="$SCRIPTS_DIR/$name" + if [[ -L "$link" ]]; then + tgt=$(resolve_symlink_target "$link") + if [[ "$tgt" == "$CELLAR_DIR/$name" && -e "$tgt" ]]; then + info "$name (symlink → Cellar)" + elif [[ ! -e "$tgt" ]]; then + drift "$name — DANGLING SYMLINK ($link → $tgt)" + drifted=$((drifted + 1)) + else + drift "$name — symlink points outside Cellar ($tgt)" + drifted=$((drifted + 1)) + fi + elif [[ -e "$link" ]]; then + drift "$name — plain file (will be backed up to .bak on next ./install)" + drifted=$((drifted + 1)) + else + drift "$name — NOT INSTALLED in $SCRIPTS_DIR" + drifted=$((drifted + 1)) + fi + done < <(enumerate_farm_targets) + # Stale symlinks under $SCRIPTS_DIR pointing into Cellar but with missing + # target — orphan detection at the symlink-farm level. + stale_count=0 + while IFS= read -r link; do + [[ -L "$link" ]] || continue + tgt=$(resolve_symlink_target "$link") + [[ "$tgt" == "$CELLAR_DIR"/* || "$tgt" == "$CELLAR_DIR" ]] || continue + if [[ ! -e "$tgt" ]]; then + # Skip if this symlink is for a name we just reported above — + # avoid double-counting. enumerate_farm_targets gives current + # expected names; a stale link is one for a name no longer + # expected. + link_name="$(basename "$link")" + if ! enumerate_farm_targets | grep -Fxq "$link_name"; then + total=$((total + 1)) + drift "stale symlink $link → $tgt (target gone from Cellar)" + drifted=$((drifted + 1)) + stale_count=$((stale_count + 1)) + fi + fi + done < <(find "$SCRIPTS_DIR" -maxdepth 1 -type l 2>/dev/null) + if [[ $stale_count -eq 0 ]]; then + info "no stale symlinks pointing into Cellar" + fi + echo "" echo "Config" echo "──────────────────────────────────────────" @@ -446,14 +895,30 @@ if [[ "$CHECK_MODE" == true ]]; then echo "" echo "Settings" echo "──────────────────────────────────────────" - # Check for missing hooks + # Check for missing hooks AND missing matchers within existing event arrays for hook_event in $(jq -r '.hooks // {} | keys[] | select(. != "_comment")' "$REPO_DIR/config/settings.template.json"); do total=$((total + 1)) if ! jq -e ".hooks.\"${hook_event}\"" "$CLAUDE_DIR/settings.json" &>/dev/null; then drift "settings.json — missing hook: $hook_event" drifted=$((drifted + 1)) else - info "settings.json hook $hook_event (present)" + # Event present — but does the local array cover every + # template matcher? Report any missing matchers as drift. + missing_matchers=$(jq -r \ + --arg ev "$hook_event" \ + --slurpfile localf "$CLAUDE_DIR/settings.json" ' + (($localf[0].hooks[$ev] // []) | map(.matcher)) as $local_ms | + (.hooks[$ev] // []) | map(.matcher) | map(select(. as $m | $local_ms | index($m) | not)) | .[] + ' "$REPO_DIR/config/settings.template.json") + if [[ -n "$missing_matchers" ]]; then + while IFS= read -r m; do + [[ -z "$m" ]] && continue + drift "settings.json — missing matcher \"$m\" in event $hook_event" + done <<<"$missing_matchers" + drifted=$((drifted + 1)) + else + info "settings.json hook $hook_event (present)" + fi fi done # Check for missing plugins @@ -466,6 +931,22 @@ if [[ "$CHECK_MODE" == true ]]; then info "settings.json plugin $plugin (present)" fi done + + # Old-path hook commands: any leftover ~/.local/bin/hooks/... entries + # from before the Cellar migration. These get rewritten by the next + # ./install (merge_settings); surface them here so a --check user + # knows they're carrying drift. + old_path_events=$(detect_old_hook_paths_in_settings "$CLAUDE_DIR/settings.json") + if [[ -n "$old_path_events" ]]; then + while IFS= read -r ev; do + [[ -z "$ev" ]] && continue + total=$((total + 1)) + drift "settings.json — hooks.$ev has old ~/.local/bin/hooks/... command path (will be rewritten on next ./install)" + drifted=$((drifted + 1)) + done <<<"$old_path_events" + else + info "settings.json hook paths (no legacy ~/.local/bin/hooks/... entries)" + fi fi # Crystallizer: compare repo files vs ~/.claude/context-crystallizer/ @@ -511,7 +992,7 @@ if [[ "$CHECK_MODE" == true ]]; then pkg_name="$(basename "$pkg_dir")" artifact_name="${pkg_name//_/-}" src="$REPO_DIR/dist/$artifact_name" - dest="$SCRIPTS_DIR/$artifact_name" + dest="$CELLAR_DIR/$artifact_name" if [[ -f "$src" ]]; then total=$((total + 1)) do_check "$src" "$dest" "$artifact_name" || drifted=$((drifted + 1)) @@ -520,6 +1001,15 @@ if [[ "$CHECK_MODE" == true ]]; then fi fi + # Logrotate + echo "" + echo "Logrotate" + echo "──────────────────────────────────────────" + total=$((total + 1)) + if ! check_logrotate_status; then + drifted=$((drifted + 1)) + fi + # External dependencies # shellcheck source=scripts/ci/check-deps.sh source "$REPO_DIR/scripts/ci/check-deps.sh" @@ -539,19 +1029,62 @@ if [[ "$CHECK_MODE" == true ]]; then exit 0 fi +# --- Install scripts ---------------------------------------------------------- +# Cellar + symlink-farm layout (closes #560): +# 1. Wipe and redeploy $CELLAR_DIR from scripts/ — kills orphans structurally. +# 2. Reap stale symlinks under $SCRIPTS_DIR (target was under Cellar but the +# file is now gone — catches removed-from-repo scripts like discord-bot). +# 3. Recreate symlinks for top-level Cellar entries (granularity B from #560: +# subtrees like hooks/, vox-providers/, testing/ stay Cellar-only). +# +# MUST run before INSTALL_SKILLS: cellar_deploy wipes $CELLAR_DIR before +# deploying scripts/, so skill helpers (which live under $CELLAR_DIR/skills/) +# would be obliterated if INSTALL_SKILLS ran first. Skills then layer onto the +# freshly-deployed Cellar without conflict. +if [[ "$INSTALL_SCRIPTS" == true ]]; then + echo "" + echo "Scripts → $CELLAR_DIR (Cellar) + $SCRIPTS_DIR (symlinks)" + echo "──────────────────────────────────────────" + # 1. Cellar: wipe + redeploy. + cellar_deploy + # 2. Reap stale symlinks: any symlink under $SCRIPTS_DIR pointing into the + # Cellar but whose target no longer exists. Foreign symlinks left alone. + reap_stale_cellar_symlinks + # 3. Symlink farm: top-level Cellar entries only. Per-name safeguard for + # user-customized plain files in $SCRIPTS_DIR (back up to .bak first). + while IFS= read -r name; do + [[ -z "$name" ]] && continue + safeguard_user_file "$name" + farm_symlink "$name" + done < <(enumerate_farm_targets) + # Refresh manifest (still used by --prune for legacy compat). + write_manifest +fi + # --- Install skills ----------------------------------------------------------- if [[ "$INSTALL_SKILLS" == true ]]; then echo "" - echo "Skills → $SKILLS_DIR" + echo "Skills → $SKILLS_DIR (helpers go to $CELLAR_DIR/skills// + symlink farm)" echo "──────────────────────────────────────────" + # Skill helpers ride the Cellar + symlink-farm pattern same as scripts/, + # but namespaced under $CELLAR_DIR/skills//: two skills + # shipping same-named helpers cannot silently clobber each other. + # When --skills runs in isolation, INSTALL_SCRIPTS hasn't created the + # Cellar — ensure it exists so we can write into it. + [[ "$DRY_RUN" != true && "$INSTALL_SCRIPTS" != true ]] && mkdir -p "$CELLAR_DIR" for skill_dir in "$REPO_DIR"/skills/*/; do skill_name="$(basename "$skill_dir")" # Install SKILL.md src="$skill_dir/SKILL.md" dest="$SKILLS_DIR/$skill_name/SKILL.md" [[ -f "$src" ]] && do_copy "$src" "$dest" - # Install non-SKILL.md files: .md files stay in the skill dir, - # everything else goes to ~/.local/bin/ as executable scripts + # Install non-SKILL.md files: .md files stay in the skill dir; + # everything else (helper scripts) goes into the Cellar at + # $CELLAR_DIR/skills// (per-skill namespace, so + # two skills shipping same-named helpers cannot silently clobber each + # other), with a top-level bare-name symlink in $SCRIPTS_DIR for + # PATH discoverability. farm_symlink_skill_helper warns on cross-skill + # name collisions at symlink time. for helper in "$skill_dir"/*; do [[ -f "$helper" ]] || continue helper_name="$(basename "$helper")" @@ -559,10 +1092,9 @@ if [[ "$INSTALL_SKILLS" == true ]]; then if [[ "$helper_name" == *.md ]]; then do_copy "$helper" "$SKILLS_DIR/$skill_name/$helper_name" else - do_copy "$helper" "$SCRIPTS_DIR/$helper_name" - if [[ "$DRY_RUN" != true ]]; then - chmod +x "$SCRIPTS_DIR/$helper_name" - fi + cellar_install_skill_helper "$helper" "$skill_name" "$helper_name" + safeguard_user_file "$helper_name" + farm_symlink_skill_helper "$skill_name" "$helper_name" fi done # Install content subdirectories (e.g., tours/) into the skill dir @@ -578,34 +1110,6 @@ if [[ "$INSTALL_SKILLS" == true ]]; then done fi -# --- Install scripts ---------------------------------------------------------- -if [[ "$INSTALL_SCRIPTS" == true ]]; then - echo "" - echo "Scripts → $SCRIPTS_DIR" - echo "──────────────────────────────────────────" - # Skip ci/ — internal tooling, not for installation. Everything else under - # scripts/ (including nested subdirs like vox-providers/, wavebus/) gets - # mirrored into ~/.local/bin/, preserving relative paths. - while IFS= read -r rel; do - [[ "$rel" == ci/* ]] && continue - src="$REPO_DIR/scripts/$rel" - dest="$SCRIPTS_DIR/$rel" - do_copy "$src" "$dest" - # Preserve the source's executable bit. Top-level scripts have - # always been chmod'd +x unconditionally; preserve that behavior - # for backward compatibility while honoring source bits in subdirs. - if [[ "$DRY_RUN" != true ]]; then - if [[ "$rel" != */* ]]; then - chmod +x "$dest" - elif [[ -x "$src" ]]; then - chmod +x "$dest" - fi - fi - done < <(enumerate_scripts) - # Refresh manifest of what we just installed — used by --prune later. - write_manifest -fi - # --- Install config ----------------------------------------------------------- if [[ "$INSTALL_CONFIG" == true ]]; then echo "" @@ -676,7 +1180,7 @@ fi # --- Build and install packages ----------------------------------------------- if [[ "$INSTALL_SCRIPTS" == true && -d "$REPO_DIR/src" ]]; then echo "" - echo "Packages → $SCRIPTS_DIR" + echo "Packages → $CELLAR_DIR (Cellar) + $SCRIPTS_DIR (symlinks)" echo "──────────────────────────────────────────" # Build from source if [[ "$DRY_RUN" == true ]]; then @@ -684,19 +1188,19 @@ if [[ "$INSTALL_SCRIPTS" == true && -d "$REPO_DIR/src" ]]; then else "$REPO_DIR/scripts/ci/build.sh" fi - # Install built artifacts + # Install built artifacts: Cellar copy + top-level symlink farm entry. for pkg_dir in "$REPO_DIR"/src/*/; do [[ -f "$pkg_dir/__main__.py" ]] || continue pkg_name="$(basename "$pkg_dir")" artifact_name="${pkg_name//_/-}" src="$REPO_DIR/dist/$artifact_name" - dest="$SCRIPTS_DIR/$artifact_name" if [[ "$DRY_RUN" == true ]]; then - info "(dry-run) $src → $dest" + info "(dry-run) $src → $CELLAR_DIR/$artifact_name (+ symlink at $SCRIPTS_DIR/$artifact_name)" else if [[ -f "$src" ]]; then - do_copy "$src" "$dest" - chmod +x "$dest" + cellar_install_file "$src" "$artifact_name" + safeguard_user_file "$artifact_name" + farm_symlink "$artifact_name" else warn "Expected artifact not found: $src" fi @@ -735,6 +1239,52 @@ if [[ "$INSTALL_MCPS" == true && -f "$REPO_DIR/mcps.json" ]]; then fi fi +# --- Install logrotate policy ------------------------------------------------- +# Only fires on Linux with logrotate(8) on PATH. macOS and other platforms +# print a single notice and skip — no error, no prompt. The mode is set by +# --with-logrotate / --without-logrotate; absent either flag the user is +# prompted (unless --yes was passed, which is taken as consent). +if [[ "$INSTALL_CONFIG" == true || "$LOGROTATE_MODE" != "prompt" ]]; then + echo "" + echo "Logrotate (~/.claude/logs/mcp.jsonl)" + echo "──────────────────────────────────────────" + if ! logrotate_supported; then + skip "Skipping — logrotate(8) is Linux-only and not present here ($(uname -s))." + skip "macOS users: see docs/operations/log-rotation.md for newsyslog guidance." + else + do_logrotate=false + case "$LOGROTATE_MODE" in + on) + do_logrotate=true + ;; + off) + skip "Skipping — --without-logrotate." + ;; + prompt) + if [[ "$ASSUME_YES" == true ]]; then + do_logrotate=true + info "--yes given — installing logrotate policy." + elif [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would prompt to install logrotate policy." + elif [[ -t 0 ]]; then + read -r -p "Install logrotate policy for ~/.claude/logs/mcp.jsonl? [Y/n] " reply || reply="" + case "$reply" in + n | N | no | NO) skip "Skipped by user." ;; + *) do_logrotate=true ;; + esac + else + # Non-interactive (no tty) and no flag — default to skip with a notice. + skip "Non-interactive shell and no --with-logrotate / --without-logrotate flag — skipping." + info "Re-run with ./install --with-logrotate to enable rotation." + fi + ;; + esac + if [[ "$do_logrotate" == true ]]; then + install_logrotate_config || warn "logrotate install reported issues — see above." + fi + fi +fi + # --- Summary ------------------------------------------------------------------ echo "" echo "──────────────────────────────────────────" diff --git a/scripts/install-remote.sh b/scripts/install-remote.sh index 2be1114..4113843 100755 --- a/scripts/install-remote.sh +++ b/scripts/install-remote.sh @@ -14,6 +14,8 @@ set -euo pipefail # curl ... | bash -s -- --check # curl ... | bash -s -- --version v1.0.0 # curl ... | bash -s -- --no-mcps +# curl ... | bash -s -- --with-logrotate +# curl ... | bash -s -- --without-logrotate OWNER="Wave-Engineering" REPO="claudecode-workflow" @@ -22,9 +24,18 @@ BASE_URL="https://github.com/${OWNER}/${REPO}/releases" SKILLS_DIR="$HOME/.claude/skills" SCRIPTS_DIR="$HOME/.local/bin" CLAUDE_DIR="$HOME/.claude" +# Cellar: kit-owned scripts directory (Homebrew/Nix pattern). Wiped and +# recreated on every install — eliminates orphan rot. SCRIPTS_DIR becomes a +# symlink farm pointing into here for entries that need PATH. Ported from +# install per cc-workflow#560. +CELLAR_DIR="$HOME/.claude/scripts" VERSION="" NO_MCPS=false +DRY_RUN=false +# Logrotate is a tri-state: "prompt" by default, forced on/off by flag. +# Ported from install per cc-workflow#540. +LOGROTATE_MODE=prompt TMPDIR_CLEANUP="" # --------------------------------------------------------------------------- @@ -36,6 +47,7 @@ ok() { printf ' \033[1;32m[+]\033[0m %s\n' "$*"; } warn() { printf ' \033[1;33m[!]\033[0m %s\n' "$*"; } fail() { printf ' \033[1;31m[!]\033[0m %s\n' "$*"; } skip() { printf ' \033[0;37m[-]\033[0m %s\n' "$*"; } +drift() { printf ' \033[0;33m[~]\033[0m %s\n' "$*"; } die() { fail "$*" exit 1 @@ -97,7 +109,163 @@ resolve_url() { } # --------------------------------------------------------------------------- -# Settings smart-merge (ported from install.sh) +# Cellar + symlink-farm helpers (ported from install — see #560) +# --------------------------------------------------------------------------- +# Cellar = $CELLAR_DIR (kit-owned). Wiped and recreated each install. +# Symlink farm = $SCRIPTS_DIR — only top-level Cellar entries; subtrees like +# hooks/, vox-providers/ stay Cellar-only and are invoked by absolute path. + +# Resolve a symlink's target to a normalized absolute path. Portable across +# GNU and BSD readlink (no -f). +resolve_symlink_target() { + local link="$1" target + target=$(readlink "$link" 2>/dev/null || true) + [[ -z "$target" ]] && return 0 + if [[ "$target" != /* ]]; then + target="$(cd "$(dirname "$link")" 2>/dev/null && pwd)/$target" + fi + target="${target/#\~/$HOME}" + echo "$target" +} + +# Enumerate the basenames that should appear in $SCRIPTS_DIR as symlinks +# pointing into $CELLAR_DIR. Top-level scripts/ entries only. Uses +# `find ... | sed` rather than GNU's `-printf '%f\n'` because BSD/macOS +# find lacks -printf and silently emits nothing — same fix-pattern as +# install. CRITICAL for macOS portability. +enumerate_farm_targets() { + local src="$1" + (cd "$src" && find . -maxdepth 1 -type f | sed 's|^\./||' | sort) +} + +# Wipe and recreate the Cellar from the release tarball's scripts/ tree. +# Preserves directory structure and executable bits. Honors $DRY_RUN. +cellar_deploy() { + local src_scripts="$1" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would wipe and redeploy $CELLAR_DIR from tarball scripts/" + return 0 + fi + # Wipe — structural orphan-killer. Anything from a prior install that is + # no longer in the release tarball dies here. + rm -rf "$CELLAR_DIR" + mkdir -p "$CELLAR_DIR" + # BSD/macOS-portable: find + sed instead of -printf '%P\n'. + while IFS= read -r rel; do + [[ -z "$rel" ]] && continue + [[ "$rel" == ci/* ]] && continue + [[ "$rel" == */tests/* ]] && continue + [[ "$rel" == */fixtures/* ]] && continue + [[ "$rel" == */__pycache__/* ]] && continue + [[ "$rel" == */.pytest_cache/* ]] && continue + local s="$src_scripts/$rel" + local d="$CELLAR_DIR/$rel" + mkdir -p "$(dirname "$d")" + cp "$s" "$d" + # Preserve exec bit (top-level scripts always +x for backwards compat). + if [[ "$rel" != */* ]]; then + chmod +x "$d" + elif [[ -x "$s" ]]; then + chmod +x "$d" + fi + done < <(cd "$src_scripts" && find . -type f | sed 's|^\./||' | sort) + info "Cellar redeployed: $CELLAR_DIR ($(find "$CELLAR_DIR" -type f | wc -l) files)" +} + +# Drop a skill helper into a per-skill Cellar subdir at +# $CELLAR_DIR/skills//. Preserves +x. Distinct +# from a flat Cellar drop so two skills shipping same-named helpers cannot +# silently overwrite each other. +cellar_install_skill_helper() { + local src="$1" skill_name="$2" helper_name="$3" + local dest="$CELLAR_DIR/skills/$skill_name/$helper_name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) $src → $dest (Cellar/skills)" + return 0 + fi + mkdir -p "$(dirname "$dest")" + cp "$src" "$dest" + chmod +x "$dest" +} + +# If $SCRIPTS_DIR/ exists as a plain file (not a symlink), back it up +# before replacing with a symlink. Pre-Cellar layouts have plain files here. +safeguard_user_file() { + local name="$1" + local path="$SCRIPTS_DIR/$name" + [[ -L "$path" ]] && return 0 + [[ -e "$path" ]] || return 0 + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would back up plain file $path → ${path}.bak" + return 0 + fi + cp "$path" "${path}.bak" + rm -f "$path" + warn "Backed up user-customized $path → ${path}.bak" +} + +# Create or refresh a symlink at $SCRIPTS_DIR/ pointing into the +# Cellar at $CELLAR_DIR/. Caller must safeguard plain-file occupants +# first. +farm_symlink() { + local name="$1" + local target="$CELLAR_DIR/$name" + local link="$SCRIPTS_DIR/$name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) symlink $link → $target" + return 0 + fi + mkdir -p "$(dirname "$link")" + ln -sf "$target" "$link" +} + +# Create or refresh a symlink at $SCRIPTS_DIR/ pointing into +# the per-skill Cellar subdir. Warn loudly on cross-skill name collision. +farm_symlink_skill_helper() { + local skill_name="$1" helper_name="$2" + local target="$CELLAR_DIR/skills/$skill_name/$helper_name" + local link="$SCRIPTS_DIR/$helper_name" + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) symlink $link → $target" + return 0 + fi + if [[ -L "$link" ]]; then + local existing + existing=$(resolve_symlink_target "$link") + if [[ "$existing" == "$CELLAR_DIR/skills/"* && "$existing" != "$target" ]]; then + warn "Helper name collision for '$helper_name': $existing already farmed; overwriting with $target" + fi + fi + mkdir -p "$(dirname "$link")" + ln -sf "$target" "$link" +} + +# Walk $SCRIPTS_DIR and remove symlinks pointing into the Cellar whose +# target no longer exists. Foreign symlinks are NEVER touched. +reap_stale_cellar_symlinks() { + [[ -d "$SCRIPTS_DIR" ]] || return 0 + local removed=0 + while IFS= read -r link; do + [[ -L "$link" ]] || continue + local target + target=$(resolve_symlink_target "$link") + [[ "$target" == "$CELLAR_DIR"/* || "$target" == "$CELLAR_DIR" ]] || continue + [[ -e "$target" ]] && continue + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would remove stale symlink: $link → $target" + else + rm -f "$link" + info "Reaped stale symlink: $link → $target" + fi + removed=$((removed + 1)) + done < <(find "$SCRIPTS_DIR" -maxdepth 1 -type l 2>/dev/null) + if [[ $removed -eq 0 ]]; then + skip "No stale symlinks under $SCRIPTS_DIR (Cellar reaper)" + fi +} + +# --------------------------------------------------------------------------- +# Settings smart-merge (ported from install — see #556 for hook union-merge) # --------------------------------------------------------------------------- merge_settings() { @@ -108,7 +276,11 @@ merge_settings() { # Deep merge: template into local, preserving user customizations # Rules: # 1. Top-level keys (scalars AND objects): added only if ABSENT locally - # 2. hooks: add missing event keys from template; leave existing alone + # 2. hooks: + # - Event keys missing locally: added from template + # - Event keys present in both: matcher arrays unioned by .matcher + # value (template matcher entries whose .matcher is not in the + # local array are appended; existing local entries left untouched) # 3. permissions.allow: union of both arrays (deduplicated) # 4. enabledPlugins: add missing keys from template; leave existing alone # 5. _comment keys: stripped from result (template-only documentation) @@ -125,12 +297,26 @@ merge_settings() { # Capture local keys before entering reduce (.[1] | keys) as $local_keys | - # Merge hooks: add missing event keys + # Merge hooks: add missing event keys AND union matcher arrays for shared keys ((.[0].hooks // {}) | to_entries | map( select(.key != "_comment") )) as $tpl_hooks | + ((.[1].hooks // {}) | to_entries | map( + select(.key != "_comment") + )) as $local_hooks | ((.[1].hooks // {}) | keys) as $local_hook_keys | + # New event keys (template only) — add wholesale ($tpl_hooks | map(select(.key | IN($local_hook_keys[]) | not))) as $new_hooks | + # Shared event keys — union their matcher arrays by .matcher value + ($tpl_hooks | map(select(.key | IN($local_hook_keys[]))) | map({ + key: .key, + value: ( + (.value // []) as $tpl_arr | + (($local_hooks | from_entries)[.key] // []) as $local_arr | + ($local_arr | map(.matcher)) as $local_matchers | + $local_arr + ($tpl_arr | map(select(.matcher as $m | $local_matchers | index($m) | not))) + ) + }) | from_entries) as $merged_shared_hooks | # Merge permissions.allow: union ((.[0].permissions.allow // []) + (.[1].permissions.allow // []) | unique) as $merged_perms | @@ -143,7 +329,7 @@ merge_settings() { # Build result: start with local, add missing pieces .[1] | .permissions.allow = $merged_perms - | .hooks = ((.hooks // {}) + ($new_hooks | from_entries)) + | .hooks = ((.hooks // {}) + ($new_hooks | from_entries) + $merged_shared_hooks) | .enabledPlugins = ((.enabledPlugins // {}) + ($new_plugins | from_entries)) | reduce ($tpl_defaults[] | select(.key | IN($local_keys[]) | not)) as $s (.; .[$s.key] = $s.value) | del(._comment) @@ -153,9 +339,25 @@ merge_settings() { echo "$merged" >"$target" # Report what changed + + # Report new hooks (and new matchers within shared event keys) for hook_event in $(jq -r '.hooks // {} | keys[] | select(. != "_comment")' "$template"); do if jq -e ".hooks.\"${hook_event}\"" "${target}.bak" &>/dev/null; then - skip "hooks.$hook_event -- already present (skipped)" + # Event already exists locally — diff matcher arrays and report + # any template matchers appended by the union-merge. + local added_matchers + added_matchers=$(jq -r --arg ev "$hook_event" --slurpfile bak "${target}.bak" ' + (($bak[0].hooks[$ev] // []) | map(.matcher)) as $local_ms | + (.hooks[$ev] // []) | map(.matcher) | map(select(. as $m | $local_ms | index($m) | not)) | .[] + ' "$target") + if [[ -z "$added_matchers" ]]; then + skip "hooks.$hook_event -- already present (skipped)" + else + while IFS= read -r m; do + [[ -z "$m" ]] && continue + info "hooks.$hook_event -- matcher \"$m\" added" + done <<<"$added_matchers" + fi else info "hooks.$hook_event -- added" fi @@ -180,6 +382,142 @@ merge_settings() { fi } +# --------------------------------------------------------------------------- +# Logrotate helpers (ported from install — see #540) +# --------------------------------------------------------------------------- +# The release tarball ships assets/logrotate/cc-mcp-logs containing a +# {{HOME}} marker which is rendered to the installing user's home before +# dropping into /etc/logrotate.d. Linux-only; macOS no-ops politely. + +LOGROTATE_DEST="/etc/logrotate.d/cc-mcp-logs" +LOGROTATE_LOGS_DIR="$HOME/.claude/logs" +# LOGROTATE_SRC is set per-invocation from the release_dir. + +# True iff this host has a logrotate(8) we can drive. Linux + logrotate on PATH. +logrotate_supported() { + [[ "$(uname -s)" == "Linux" ]] && command -v logrotate &>/dev/null +} + +# Render the templated config to stdout: substitute {{HOME}} with $HOME. +render_logrotate_template() { + local src="$1" + sed "s|{{HOME}}|$HOME|g" "$src" +} + +# Install the rendered config to /etc/logrotate.d/cc-mcp-logs and run a +# dry-run validation. Returns non-zero on failure. +install_logrotate_config() { + local src="$1" + local tmp + tmp="$(mktemp)" + render_logrotate_template "$src" >"$tmp" + + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would install logrotate config to $LOGROTATE_DEST" + info "(dry-run) Would run: sudo logrotate -d $LOGROTATE_DEST" + rm -f "$tmp" + return 0 + fi + + if sudo install -m 0644 "$tmp" "$LOGROTATE_DEST"; then + info "Installed $LOGROTATE_DEST" + else + warn "Failed to install $LOGROTATE_DEST (sudo install)" + rm -f "$tmp" + return 1 + fi + rm -f "$tmp" + + if sudo logrotate -d "$LOGROTATE_DEST" >/dev/null 2>&1; then + ok "logrotate -d validation passed" + else + warn "logrotate -d reported errors -- re-run manually to diagnose:" + warn " sudo logrotate -d $LOGROTATE_DEST" + return 1 + fi +} + +# Remove the installed logrotate config (used by --uninstall). +uninstall_logrotate_config() { + if ! logrotate_supported; then + skip "logrotate (skipped -- non-Linux or logrotate not installed)" + return 0 + fi + if [[ ! -f "$LOGROTATE_DEST" ]]; then + skip "$LOGROTATE_DEST not installed" + return 0 + fi + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would remove $LOGROTATE_DEST" + return 0 + fi + if sudo rm -f "$LOGROTATE_DEST"; then + ok "Removed $LOGROTATE_DEST" + else + warn "Failed to remove $LOGROTATE_DEST" + return 1 + fi +} + +# Report logrotate status for --check mode. Returns 0 if all-in-sync, +# 1 if any drift. Takes the rendered-source path as $1 (or empty string if +# the source isn't available — e.g. if the tarball didn't ship it). +check_logrotate_status() { + local src="$1" + if ! logrotate_supported; then + info "logrotate (skipped -- non-Linux or logrotate not installed)" + return 0 + fi + if [[ -z "$src" || ! -f "$src" ]]; then + # No source to compare against — best-effort: report install status. + if [[ -f "$LOGROTATE_DEST" ]]; then + info "logrotate config $LOGROTATE_DEST installed (no source available for drift check)" + else + drift "logrotate config $LOGROTATE_DEST -- NOT INSTALLED" + return 1 + fi + return 0 + fi + local d=0 + if [[ ! -f "$LOGROTATE_DEST" ]]; then + drift "logrotate config $LOGROTATE_DEST -- NOT INSTALLED" + return 1 + fi + local rendered + rendered="$(mktemp)" + render_logrotate_template "$src" >"$rendered" + if ! sudo diff -q "$rendered" "$LOGROTATE_DEST" &>/dev/null; then + drift "logrotate config $LOGROTATE_DEST -- DIFFERS from release template" + d=1 + else + info "logrotate config $LOGROTATE_DEST (in sync)" + fi + rm -f "$rendered" + # Last-rotation mtime: any rotated mcp.jsonl.* indicates the rotation has + # fired at least once. Use find + stat (no -printf — BSD portability). + if [[ -d "$LOGROTATE_LOGS_DIR" ]]; then + local newest="" + while IFS= read -r f; do + [[ -z "$f" ]] && continue + if [[ -z "$newest" || "$f" -nt "$newest" ]]; then + newest="$f" + fi + done < <(find "$LOGROTATE_LOGS_DIR" -maxdepth 1 \ + \( -name 'mcp.jsonl.[0-9]*' -o -name 'mcp.jsonl.[0-9]*.gz' \) \ + -type f 2>/dev/null) + if [[ -n "$newest" ]]; then + local newest_mtime + newest_mtime=$(stat -c '%y' "$newest" 2>/dev/null || stat -f '%Sm' "$newest" 2>/dev/null || echo 'unknown') + info "last rotation: $newest ($newest_mtime)" + else + info "no rotated files yet under $LOGROTATE_LOGS_DIR -- rotation has not fired" + fi + else + info "$LOGROTATE_LOGS_DIR does not exist yet -- no rotation history" + fi + return $d +} + # --------------------------------------------------------------------------- # Install # --------------------------------------------------------------------------- @@ -209,9 +547,33 @@ do_install() { local release_dir="$tmpdir" + # --- Install scripts (Cellar + symlink-farm, ported from install — #560) --- + # Order matters: cellar_deploy wipes $CELLAR_DIR before deploying, so + # skill helpers (which live under $CELLAR_DIR/skills/) would be obliterated + # if skills ran first. Scripts go FIRST, then skills layer on top. + if [[ -d "$release_dir/scripts" ]]; then + echo "Scripts -> $CELLAR_DIR (Cellar) + $SCRIPTS_DIR (symlinks)" + echo "--------------------------------------------" + mkdir -p "$SCRIPTS_DIR" + # 1. Cellar: wipe + redeploy from tarball scripts/. + cellar_deploy "$release_dir/scripts" + # 2. Reap stale symlinks pointing into the Cellar with missing targets. + reap_stale_cellar_symlinks + # 3. Symlink farm: top-level Cellar entries only. + while IFS= read -r name; do + [[ -z "$name" ]] && continue + safeguard_user_file "$name" + farm_symlink "$name" + ok "$name (symlink -> Cellar)" + done < <(enumerate_farm_targets "$release_dir/scripts") + echo "" + fi + # --- Install skills --- - echo "Skills -> $SKILLS_DIR" + echo "Skills -> $SKILLS_DIR (helpers go to $CELLAR_DIR/skills// + symlink farm)" echo "--------------------------------------------" + # Ensure Cellar exists (cellar_deploy may not have run if no scripts/). + mkdir -p "$CELLAR_DIR" for skill_dir in "$release_dir"/skills/*/; do [[ -d "$skill_dir" ]] || continue local skill_name @@ -223,7 +585,9 @@ do_install() { cp "$skill_dir/SKILL.md" "$SKILLS_DIR/$skill_name/SKILL.md" fi - # Install other files in the skill dir + # Install other files in the skill dir: .md files in the skill dir; + # everything else into Cellar at $CELLAR_DIR/skills// + # with a top-level symlink for PATH discoverability (#560). for helper in "$skill_dir"/*; do [[ -f "$helper" ]] || continue local helper_name @@ -233,9 +597,9 @@ do_install() { mkdir -p "$SKILLS_DIR/$skill_name" cp "$helper" "$SKILLS_DIR/$skill_name/$helper_name" else - mkdir -p "$SCRIPTS_DIR" - cp "$helper" "$SCRIPTS_DIR/$helper_name" - chmod +x "$SCRIPTS_DIR/$helper_name" + cellar_install_skill_helper "$helper" "$skill_name" "$helper_name" + safeguard_user_file "$helper_name" + farm_symlink_skill_helper "$skill_name" "$helper_name" fi done @@ -257,50 +621,49 @@ do_install() { done echo "" - # --- Install scripts --- - echo "Scripts -> $SCRIPTS_DIR" - echo "--------------------------------------------" - mkdir -p "$SCRIPTS_DIR" - # Recursive walk: scripts/ -> $SCRIPTS_DIR/, preserving nested - # subdirs like vox-providers/. ci/ and well-known noise subtrees are - # excluded. - while IFS= read -r rel; do - [[ "$rel" == ci/* ]] && continue - local src_path="$release_dir/scripts/$rel" - local dest_path="$SCRIPTS_DIR/$rel" - mkdir -p "$(dirname "$dest_path")" - cp "$src_path" "$dest_path" - # Preserve executable bit: top-level scripts always +x (legacy); - # nested files only +x if the source had it. - if [[ "$rel" != */* ]] || [[ -x "$src_path" ]]; then - chmod +x "$dest_path" - fi - ok "$rel" - done < <( - cd "$release_dir" && find scripts -type f \ - -not -path '*/tests/*' \ - -not -path '*/fixtures/*' \ - -not -path '*/__pycache__/*' \ - -not -path '*/.pytest_cache/*' \ - -printf '%P\n' | sort - ) - echo "" - # --- Install pre-built packages --- if [[ -d "$release_dir/dist" ]]; then - echo "Packages -> $SCRIPTS_DIR" + echo "Packages -> $CELLAR_DIR (Cellar) + $SCRIPTS_DIR (symlinks)" echo "--------------------------------------------" for pkg in "$release_dir"/dist/*; do [[ -f "$pkg" ]] || continue local pkg_name pkg_name="$(basename "$pkg")" - cp "$pkg" "$SCRIPTS_DIR/$pkg_name" - chmod +x "$SCRIPTS_DIR/$pkg_name" + # Cellar drop, then symlink farm entry (matches install — #560). + if [[ "$DRY_RUN" != true ]]; then + mkdir -p "$CELLAR_DIR" + cp "$pkg" "$CELLAR_DIR/$pkg_name" + chmod +x "$CELLAR_DIR/$pkg_name" + fi + safeguard_user_file "$pkg_name" + farm_symlink "$pkg_name" ok "$pkg_name" done echo "" fi + # --- Sanity check: prebuilt binaries present in symlink farm --- + # If the release tarball ever ships without a dist/ tree (release-pipeline + # regression), prebuilt binaries silently won't appear on PATH. Reuse the + # expected_scripts list from do_check() so the two stay in lockstep. + # Skipped on --dry-run (would always fail — nothing was actually farmed). + if [[ "$DRY_RUN" != true ]]; then + local expected_prebuilt=(discord-status-post slackbot-send job-fetch file-opener vox) + local missing_prebuilt=() + for prebuilt in "${expected_prebuilt[@]}"; do + if [[ ! -L "$SCRIPTS_DIR/$prebuilt" && ! -x "$SCRIPTS_DIR/$prebuilt" ]]; then + missing_prebuilt+=("$prebuilt") + fi + done + if [[ ${#missing_prebuilt[@]} -gt 0 ]]; then + warn "Expected prebuilt binaries missing from $SCRIPTS_DIR:" + for p in "${missing_prebuilt[@]}"; do + warn " - $p" + done + warn "The release tarball may be missing its dist/ tree -- file an issue." + fi + fi + # --- Install config --- echo "Config -> $CLAUDE_DIR" echo "--------------------------------------------" @@ -359,6 +722,49 @@ do_install() { echo "" fi + # --- Install logrotate policy (ported from install — #540) --- + # Linux-only; macOS no-ops politely. Tri-state: --with-logrotate forces on, + # --without-logrotate forces off, default prompts (or skips if no tty). + echo "Logrotate (~/.claude/logs/mcp.jsonl)" + echo "--------------------------------------------" + local logrotate_src="$release_dir/assets/logrotate/cc-mcp-logs" + if ! logrotate_supported; then + skip "Skipping -- logrotate(8) is Linux-only and not present here ($(uname -s))." + skip "macOS users: see docs/operations/log-rotation.md for newsyslog guidance." + elif [[ ! -f "$logrotate_src" ]]; then + skip "Skipping -- assets/logrotate/cc-mcp-logs not found in release tarball." + else + local do_logrotate=false + case "$LOGROTATE_MODE" in + on) + do_logrotate=true + ;; + off) + skip "Skipping -- --without-logrotate." + ;; + prompt) + if [[ "$DRY_RUN" == true ]]; then + info "(dry-run) Would prompt to install logrotate policy." + elif [[ -t 0 ]]; then + read -r -p "Install logrotate policy for ~/.claude/logs/mcp.jsonl? [Y/n] " reply || reply="" + case "$reply" in + n | N | no | NO) skip "Skipped by user." ;; + *) do_logrotate=true ;; + esac + else + # Non-interactive (no tty, e.g. curl | bash) and no flag — + # default to skip with a notice. + skip "Non-interactive shell and no --with-logrotate / --without-logrotate flag -- skipping." + info "Re-run with --with-logrotate to enable rotation." + fi + ;; + esac + if [[ "$do_logrotate" == true ]]; then + install_logrotate_config "$logrotate_src" || warn "logrotate install reported issues -- see above." + fi + fi + echo "" + # Verify install dir is on PATH if ! echo "$PATH" | tr ':' '\n' | grep -qx "$SCRIPTS_DIR"; then warn "${SCRIPTS_DIR} is not on your PATH" @@ -404,7 +810,6 @@ do_check() { # Skills echo "Skills" echo "--------------------------------------------" - # We check what's installed without downloading local skill_count=0 if [[ -d "$SKILLS_DIR" ]]; then for skill_dir in "$SKILLS_DIR"/*/; do @@ -424,13 +829,38 @@ do_check() { fi echo "" - # Scripts - echo "Scripts" + # Scripts (Cellar + symlink farm, ported from install — #560) + echo "Scripts (Cellar: $CELLAR_DIR)" + echo "--------------------------------------------" + if [[ ! -d "$CELLAR_DIR" ]]; then + drift "Cellar -- NOT INSTALLED at $CELLAR_DIR" + issues=$((issues + 1)) + else + local cellar_count + cellar_count=$(find "$CELLAR_DIR" -type f 2>/dev/null | wc -l) + info "$cellar_count file(s) in Cellar" + fi + echo "" + + echo "Symlink farm ($SCRIPTS_DIR -> Cellar)" echo "--------------------------------------------" local expected_scripts=(discord-status-post slackbot-send job-fetch file-opener vox) for script_name in "${expected_scripts[@]}"; do - if [[ -x "$SCRIPTS_DIR/$script_name" ]]; then - ok "$script_name" + local link="$SCRIPTS_DIR/$script_name" + if [[ -L "$link" ]]; then + local tgt + tgt=$(resolve_symlink_target "$link") + if [[ "$tgt" == "$CELLAR_DIR/$script_name" && -e "$tgt" ]]; then + ok "$script_name (symlink -> Cellar)" + elif [[ ! -e "$tgt" ]]; then + drift "$script_name -- DANGLING SYMLINK ($link -> $tgt)" + issues=$((issues + 1)) + else + drift "$script_name -- symlink points outside Cellar ($tgt)" + issues=$((issues + 1)) + fi + elif [[ -x "$link" ]]; then + drift "$script_name -- plain file (will be backed up to .bak on next install)" else fail "$script_name not found" issues=$((issues + 1)) @@ -454,6 +884,19 @@ do_check() { fail "settings.json not found" issues=$((issues + 1)) fi + + # Settings drift detection requires the template; run from a checkout + # (./install --check) for full settings diff. + echo "" + + # Logrotate (ported from install — #540) + echo "Logrotate" + echo "--------------------------------------------" + # In --check we don't have the rendered template handy (no tarball + # extraction). Pass empty src; check_logrotate_status reports install-only. + if ! check_logrotate_status ""; then + issues=$((issues + 1)) + fi echo "" # MCPs @@ -514,9 +957,28 @@ do_uninstall() { fi echo "" - # Remove scripts (known script names from skills) - echo "Scripts" + # Remove Cellar (Cellar + symlink farm — #560) + echo "Scripts (Cellar)" echo "--------------------------------------------" + if [[ -d "$CELLAR_DIR" ]]; then + rm -rf "$CELLAR_DIR" + ok "Removed $CELLAR_DIR" + else + skip "No Cellar directory found" + fi + # Remove symlinks from $SCRIPTS_DIR pointing into the (now-gone) Cellar. + if [[ -d "$SCRIPTS_DIR" ]]; then + while IFS= read -r link; do + [[ -L "$link" ]] || continue + local tgt + tgt=$(resolve_symlink_target "$link") + if [[ "$tgt" == "$CELLAR_DIR"/* || "$tgt" == "$CELLAR_DIR" ]]; then + rm -f "$link" + ok "Removed symlink $(basename "$link")" + fi + done < <(find "$SCRIPTS_DIR" -maxdepth 1 -type l 2>/dev/null) + fi + # Legacy plain-file uninstall: known script names from pre-Cellar layouts. local known_scripts=( discord-status-post slackbot-send job-fetch file-opener vox worktree-manager cc-inspector discord-lock @@ -525,7 +987,7 @@ do_uninstall() { for script_name in "${known_scripts[@]}"; do if [[ -f "$SCRIPTS_DIR/$script_name" ]]; then rm -f "$SCRIPTS_DIR/$script_name" - ok "Removed $script_name" + ok "Removed legacy plain file $script_name" fi done echo "" @@ -542,6 +1004,12 @@ do_uninstall() { info "settings.json preserved (not removed)" echo "" + # Remove logrotate config (#540) + echo "Logrotate" + echo "--------------------------------------------" + uninstall_logrotate_config + echo "" + # Uninstall MCPs if [[ "$NO_MCPS" == false ]]; then echo "MCPs" @@ -603,8 +1071,20 @@ while [[ $# -gt 0 ]]; do NO_MCPS=true shift ;; + --with-logrotate) + LOGROTATE_MODE=on + shift + ;; + --without-logrotate) + LOGROTATE_MODE=off + shift + ;; + --dry-run) + DRY_RUN=true + shift + ;; *) - die "Unknown flag: $1 (use --uninstall, --check, --version , or --no-mcps)" + die "Unknown flag: $1 (use --uninstall, --check, --version , --no-mcps, --with-logrotate, --without-logrotate, --dry-run)" ;; esac done diff --git a/scripts/precheck-asking-detector.sh b/scripts/precheck-asking-detector.sh index fb822b7..3910fbb 100755 --- a/scripts/precheck-asking-detector.sh +++ b/scripts/precheck-asking-detector.sh @@ -53,14 +53,23 @@ if [[ -z "$LAST_ASSISTANT_TEXT" || "$LAST_ASSISTANT_TEXT" == "null" ]]; then exit 0 fi -# Two alternations: +# Three alternations: # 1. Interrogative — trigger word (shall/should/can/may/do you want me to/ # ready for|to) within ~40 chars of "precheck", ending with "?" within # ~80 chars. Catches direct asking like "Shall I run /precheck?". # 2. Deferral — "let me know" idiom near "precheck", no "?" required. # Catches "Let me know when I should run precheck." which is asking by # deferral. -# Both case-insensitive. Negative cases the regex must NOT match: past-tense +# 3. Inverted — "precheck" appears BEFORE the trigger word (broadened +# trigger set: should/shall/need/ready/appropriate), within the same +# sentence (no sentence-terminating punctuation between), ending with +# "?". Catches "Is /precheck something I should run?", "Would /precheck +# be appropriate here?", "Precheck — should I do that now?". The +# sentence-boundary character class ([^.?!\n]) is the false-positive +# guard for cases like "/precheck completed. Should I commit now?" — +# the period severs precheck from the trigger word so the inverted +# pattern declines to fire. Issue cc-workflow#545. +# All case-insensitive. Negative cases the regex must NOT match: past-tense # precheck mentions, different-command questions ("Shall I run /scp?"), # distant trigger-and-precheck pairs, and ordinary checklist text. # @@ -69,9 +78,11 @@ fi # kill-switch exists for those cases. PATTERN_INTERROGATIVE='(?i)\b(shall|should|can|may|do you want me to|ready (for|to))\b.{0,40}/?precheck.{0,80}\?' PATTERN_DEFERRAL='(?i)\blet me know\b.{0,40}/?precheck' +PATTERN_INVERTED='(?i)/?precheck[^.?!\n]{0,40}\b(should|shall|need|ready|appropriate)\b[^.?!\n]{0,40}\?' if printf '%s' "$LAST_ASSISTANT_TEXT" | grep -Pq "$PATTERN_INTERROGATIVE" 2>/dev/null || - printf '%s' "$LAST_ASSISTANT_TEXT" | grep -Pq "$PATTERN_DEFERRAL" 2>/dev/null; then + printf '%s' "$LAST_ASSISTANT_TEXT" | grep -Pq "$PATTERN_DEFERRAL" 2>/dev/null || + printf '%s' "$LAST_ASSISTANT_TEXT" | grep -Pq "$PATTERN_INVERTED" 2>/dev/null; then cat <<'JSON' {"decision":"block","reason":"Per CLAUDE.md MANDATORY Pre-Commit Gate: don't ask whether to run /precheck — run it. The checklist that /precheck presents is the approval gate; the start of /precheck is unilateral. Continue this turn by invoking /precheck now."} JSON diff --git a/scripts/wavebus/changelog-aggregate b/scripts/wavebus/changelog-aggregate new file mode 100755 index 0000000..754101c --- /dev/null +++ b/scripts/wavebus/changelog-aggregate @@ -0,0 +1,281 @@ +#!/usr/bin/env bash +# wavebus: aggregate per-issue CHANGELOG fragments into the target repo's +# CHANGELOG.md under "## Unreleased". +# +# Usage: changelog-aggregate [] +# +# Scans /flight-*/issue-*/CHANGELOG.fragment.md in deterministic +# order (numeric flight then numeric issue). Each fragment is a markdown +# document with H3 category headings (### Breaking, ### Features, ### Fixes, +# ### Docs, ### Chore) followed by bullet lines (lines starting with "- "). +# +# Lines outside a recognized category heading are ignored (so fragments may +# carry an H2 of their own or arbitrary prose; only category-bullets are +# merged). Within each category, identical bullets are deduplicated (first +# occurrence wins). Categories are emitted in the canonical order: +# +# Breaking, Features, Fixes, Docs, Chore +# +# The merged content is written under "## Unreleased" in +# /CHANGELOG.md, creating that heading at the top of the +# file (after any leading H1/intro) if absent. If the heading already exists, +# its existing body is replaced wholesale by the freshly-aggregated content. +# +# Empty fragment dir (no fragments anywhere under ) -> no-op, +# exit 0, CHANGELOG.md untouched. +# +# Exit codes: +# 0 success (including no-op when zero fragments) +# 1 usage error +# 2 wave-root missing or not a directory +# 3 target-repo-path missing or not a directory + +set -euo pipefail + +if [[ $# -lt 2 || $# -gt 3 ]]; then + echo "usage: changelog-aggregate []" >&2 + exit 1 +fi + +wave_root="$1" +target_repo="$2" +# wave_id is currently unused by the merge logic; accepted for forward-compat +# (callers may want to stamp the section heading with the wave id later). +wave_id="${3:-}" +: "$wave_id" # silence shellcheck SC2034 — accepted but unused + +if [[ ! -d "$wave_root" ]]; then + echo "error: wave-root is not a directory: $wave_root" >&2 + exit 2 +fi + +if [[ ! -d "$target_repo" ]]; then + echo "error: target-repo-path is not a directory: $target_repo" >&2 + exit 3 +fi + +# --------------------------------------------------------------------------- +# Discover fragments in deterministic order: numeric flight, then numeric +# issue. Globbing alone gives lexical order which would mis-order flight-10 +# vs flight-2; sort by the numeric component instead. +# --------------------------------------------------------------------------- + +mapfile -t fragments < <( + find "$wave_root" -path "*/flight-*/issue-*/CHANGELOG.fragment.md" -type f 2>/dev/null | + while IFS= read -r path; do + flight="" + issue="" + # Walk path components; pull out the numeric ids from flight-N + # and issue-N segments. POSIX-portable bash extraction (no gawk + # match-with-array required, so this works on mawk too). + IFS='/' read -r -a parts <<<"$path" + for part in "${parts[@]}"; do + if [[ "$part" =~ ^flight-([0-9]+)$ ]]; then + flight="${BASH_REMATCH[1]}" + elif [[ "$part" =~ ^issue-([0-9]+)$ ]]; then + issue="${BASH_REMATCH[1]}" + fi + done + printf '%010d\t%010d\t%s\n' "$flight" "$issue" "$path" + done | + sort -k1,1n -k2,2n | + cut -f3- +) + +if [[ ${#fragments[@]} -eq 0 ]]; then + # No-op: no fragments anywhere under wave-root. + echo "changelog-aggregate: no fragments found under $wave_root (no-op)" + exit 0 +fi + +# --------------------------------------------------------------------------- +# Parse each fragment, accumulating bullets per category. We use one tmpfile +# per canonical category so dedup can be done with awk '!seen[$0]++' at the +# end. Categories are case-insensitive on the heading match but emitted with +# canonical capitalization. +# --------------------------------------------------------------------------- + +work_dir="$(mktemp -d)" +trap 'rm -rf "$work_dir"' EXIT + +# Canonical order and display names. +declare -a CATEGORIES=("Breaking" "Features" "Fixes" "Docs" "Chore") + +# Initialize per-category tmpfiles. +for cat in "${CATEGORIES[@]}"; do + : >"$work_dir/$cat" +done + +# Map a heading text (lowercased) to a canonical category, or empty if not +# recognized. +canonical_category() { + local raw="$1" + # Strip leading/trailing whitespace and trailing punctuation. + raw="${raw#"${raw%%[![:space:]]*}"}" + raw="${raw%"${raw##*[![:space:]]}"}" + local lower + lower="$(printf '%s' "$raw" | tr '[:upper:]' '[:lower:]')" + case "$lower" in + breaking | "breaking changes" | "breaking change") echo "Breaking" ;; + features | feature | added | new) echo "Features" ;; + fixes | fixed | "bug fixes" | bugfix | bugfixes) echo "Fixes" ;; + docs | documentation) echo "Docs" ;; + chore | chores | maintenance) echo "Chore" ;; + *) echo "" ;; + esac +} + +for frag in "${fragments[@]}"; do + current="" + # shellcheck disable=SC2094 # reading line-by-line + while IFS= read -r line || [[ -n "$line" ]]; do + # H3 heading -> category switch. + if [[ "$line" =~ ^###[[:space:]]+(.+)$ ]]; then + heading="${BASH_REMATCH[1]}" + current="$(canonical_category "$heading")" + continue + fi + # Any other heading resets current (so an H2 in a fragment won't + # accidentally absorb bullets into the previous H3's category). + if [[ "$line" =~ ^#{1,2}[[:space:]] ]]; then + current="" + continue + fi + # Bullet under a recognized category. + if [[ -n "$current" && "$line" =~ ^-[[:space:]] ]]; then + printf '%s\n' "$line" >>"$work_dir/$current" + fi + done <"$frag" +done + +# Dedupe each category (first occurrence wins, preserving order). +for cat in "${CATEGORIES[@]}"; do + if [[ -s "$work_dir/$cat" ]]; then + awk '!seen[$0]++' "$work_dir/$cat" >"$work_dir/$cat.dedup" + mv "$work_dir/$cat.dedup" "$work_dir/$cat" + fi +done + +# If, after parsing, every category is empty (fragments existed but contained +# no recognizable bullets), treat as no-op too. +any_content=0 +for cat in "${CATEGORIES[@]}"; do + if [[ -s "$work_dir/$cat" ]]; then + any_content=1 + break + fi +done +if [[ $any_content -eq 0 ]]; then + echo "changelog-aggregate: fragments found but no recognized category bullets (no-op)" + exit 0 +fi + +# --------------------------------------------------------------------------- +# Build the new "## Unreleased" body. +# --------------------------------------------------------------------------- + +new_section="$work_dir/unreleased.md" +{ + echo "## Unreleased" + echo "" + for cat in "${CATEGORIES[@]}"; do + if [[ -s "$work_dir/$cat" ]]; then + echo "### $cat" + echo "" + cat "$work_dir/$cat" + echo "" + fi + done +} >"$new_section" + +# --------------------------------------------------------------------------- +# Splice into target CHANGELOG.md. +# --------------------------------------------------------------------------- + +changelog="$target_repo/CHANGELOG.md" + +if [[ ! -f "$changelog" ]]; then + # Brand-new CHANGELOG.md: just emit the section. + { + echo "# Changelog" + echo "" + cat "$new_section" + } >"$changelog" + echo "changelog-aggregate: wrote $(wc -l <"$new_section") lines under '## Unreleased' (new CHANGELOG.md at $changelog)" + exit 0 +fi + +# Existing CHANGELOG.md. Look for an existing "## Unreleased" heading. If +# present, replace from that line up to (but not including) the next H2 (or +# EOF) with the freshly built section. If absent, insert the new section +# after the file's leading H1 + blank-line block (if any), else at the top. + +if grep -q -E '^## Unreleased[[:space:]]*$' "$changelog"; then + # Replace existing block. + awk -v new_section_path="$new_section" ' + BEGIN { state = "before" } + state == "before" { + if ($0 ~ /^## Unreleased[[:space:]]*$/) { + # Emit the new section in place of this line + everything until next H2/EOF. + while ((getline line < new_section_path) > 0) print line; + close(new_section_path); + state = "skip" + next + } + print + next + } + state == "skip" { + # We are skipping the old Unreleased body. Stop skipping when we hit the next H2. + if ($0 ~ /^## /) { print; state = "after"; next } + next + } + state == "after" { print } + ' "$changelog" >"$work_dir/changelog.new" +else + # Insert new section. Strategy: place it after the first H1+blank-line + # (typical CHANGELOG.md preamble) if present, else at the top. + awk -v new_section_path="$new_section" ' + BEGIN { inserted = 0; saw_h1 = 0; just_after_h1 = 0 } + !inserted { + if (!saw_h1 && $0 ~ /^# /) { + print + saw_h1 = 1 + just_after_h1 = 1 + next + } + if (just_after_h1) { + # The blank line directly after the H1 is the natural splice point. + if ($0 == "") { + print + while ((getline line < new_section_path) > 0) print line; + close(new_section_path); + inserted = 1 + next + } + # H1 not followed by blank line -> splice now anyway. + while ((getline line < new_section_path) > 0) print line; + close(new_section_path); + inserted = 1 + print + next + } + # No H1 yet -> splice at the very top before printing the first line. + while ((getline line < new_section_path) > 0) print line; + close(new_section_path); + inserted = 1 + print + next + } + { print } + END { + if (!inserted) { + while ((getline line < new_section_path) > 0) print line; + close(new_section_path); + } + } + ' "$changelog" >"$work_dir/changelog.new" +fi + +mv "$work_dir/changelog.new" "$changelog" +echo "changelog-aggregate: merged ${#fragments[@]} fragments into $changelog" diff --git a/skills/nextwave/SKILL.md b/skills/nextwave/SKILL.md index 5044220..20d3dc4 100644 --- a/skills/nextwave/SKILL.md +++ b/skills/nextwave/SKILL.md @@ -26,7 +26,7 @@ CC sub-agents do not have the `Agent`/`Task` tool. Only the top-level session ca - Spec: `spec_validate_structure`, `spec_get`, `spec_acceptance_criteria` - Commutativity + merge (used by Prime post-flight): `commutativity_verify`, `pr_create`, `pr_wait_ci`, `pr_merge` - CI trust (auto mode): `wave_ci_trust_level` -- Bus primitives (bash): `scripts/wavebus/wave-init`, `scripts/wavebus/flight-finalize`, `scripts/wavebus/wave-cleanup` +- Bus primitives (bash): `scripts/wavebus/wave-init`, `scripts/wavebus/flight-finalize`, `scripts/wavebus/wave-cleanup`, `scripts/wavebus/changelog-aggregate` - Notifications: `mcp__disc-server__disc_send` — post wave status to `#wave-status` (`1487386934094462986`) - Observability: `mcp-log` — emit lifecycle events to `~/.claude/logs/mcp.jsonl` for temporal correlation against sdlc-server `tool_call` events. See `docs/mcp-logging-standard.md`. @@ -230,14 +230,16 @@ For each flight M in the plan, in order: The Orchestrator issues one `Agent` tool-use block containing N `Agent` calls — one per issue in flight M — **in the same assistant message**. This is the only place in the skill where parallelism actually happens. Do NOT issue the calls in separate messages; do NOT use `run_in_background`; do NOT shell out. +**Non-negotiable — `run_in_background` is forbidden here.** The Orchestrator MUST omit the parameter entirely (the default routes synchronously). Passing `run_in_background: true` is a contract break, not a tuning knob. The collection step (3c) below assumes every same-block Agent call returns synchronously with its canonical PASS/FAIL line; an async/background return for any one of the N calls would split the batch and break deterministic collection. This is a **harness contract**: same-block Agent calls without the explicit `run_in_background: true` flag MUST all route synchronously, with no internal heuristic backgrounding any subset. If a harness ever returns an "Async agent launched" surface for a call the Orchestrator did not flag as background, that is a harness bug — file it; do NOT add a polling fallback here. (The explicit case — when a caller intentionally passes `run_in_background: true` — still routes async; the contract targets the implicit case only. See cc-workflow#490 for the source incident.) + Each Agent call: `subagent_type: general-purpose`, prompt = the Flight stub below (filled in), and for same-repo waves `isolation: "worktree"` (for cross-repo, omit — worktrees were pre-created in Step 1.5 and are referenced by path in the prompt). ### 3c. Collect returns. -Each Flight's last message is exactly one line matching `^(/tmp/wavemachine/[^\s]+/results\.md) (PASS|FAIL)$`. Orchestrator: +Each Flight's last message is exactly one line matching `^(/tmp/wavemachine/[^\s]+/results\.md) (PASS|FAIL)$`. Per the Step 3b harness contract, all N returns are synchronous and arrive together in the same tool-result block — no polling, no waiting on bus DONE sentinels for collection, no fallback path for "one of N came back async." Orchestrator: - Parses each line. Malformed → record as FAIL, note "malformed return". -- Verifies the `DONE` sentinel (`/tmp/wavemachine/.../issue-/DONE`) exists and contains `PASS` or `FAIL` matching the returned line. Mismatch → FAIL. +- Verifies the `DONE` sentinel (`/tmp/wavemachine/.../issue-/DONE`) exists and contains `PASS` or `FAIL` matching the returned line. Mismatch → FAIL. (The DONE sentinel is a cross-check on the Flight's self-report, not a collection mechanism.) - Reads each `results.md` into context (small, summary + checklist). ### 3c.5. Reviewer pass (Orchestrator dispatches — runs before the gate). @@ -324,7 +326,12 @@ One `Agent` call, `subagent_type: general-purpose`. Pass the wave's `kahuna_bran > - `STRONG` / `MEDIUM` → `pr_merge(skip_train=true)` for all. > - `WEAK` / `ORACLE_REQUIRED` → sequential merge via the merge queue (no skip). > Single-issue flights skip commutativity entirely. -> 5. Merge all flight PRs via `pr_merge`. On merge, call `wave_close_issue(X)` and `wave_record_mr(issue_number=X, mr_ref=)` per issue. Call `wave_flight_done(M)` after all merges land. +> 5. Merge all flight PRs via `pr_merge`. On merge, per issue X: +> a. `wave_close_issue(X)` +> b. `wave_record_mr(issue_number=X, mr_ref=)` +> c. Read `**Plan:** #M` from the story issue's `## Metadata` section (via `spec_get(issue_ref=X)`). If `M` is present and not `N/A`, call `plan_mark_story_done({plan_ref: M, story_id: X})`. The handler is `warn_only: true` — a failure is logged to the merge report but does NOT abort the merge sequence. (The handler ships in `mcp-server-sdlc`; until it lands, the call surfaces as a warn-only logged failure per the same contract.) +> +> Call `wave_flight_done(M)` after all merges land. Then fire-and-forget the auto-updating Discord embed: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored — Discord is informational, never a gate). > 6. `git checkout main && git pull` in the target repo. > 7. Write `/flight-/merge-report.md` (per-issue PR URL, CI status, merge strategy, reviewer-pass summary per issue from the Step 3c.5 dispatch, anomalies). > @@ -360,8 +367,9 @@ After every flight has merged: > - `SPEC CURRENT` → note in report; move on. > - `SPEC STALE` → mechanical fix: update the issue with corrected paths/names; list changes in the report. > - `SPEC BROKEN` → leave the issue alone; flag for user attention in the report. - > 4. `wave_complete()` (marks the current wave complete — takes no args; the server uses the active wave from state). - > 5. Write `/merge-report.md` (issues closed, PR URLs, flight breakdown, drift findings, deferred items, next-wave preview). + > 4. **CHANGELOG aggregation (mechanical — no gate).** Run `scripts/wavebus/changelog-aggregate wave-` to merge per-issue `CHANGELOG.fragment.md` files into the target repo's `CHANGELOG.md` under `## Unreleased`. If the aggregator wrote to `CHANGELOG.md`, commit on a fresh `chore/wave--changelog` branch in the target repo, push, open a PR (`pr_create`) targeting `` if set else `main`, wait for CI (`pr_wait_ci`), then `pr_merge`. No human gate — content was already approved at each flight's Step 3d gate; this step is purely mechanical aggregation. If the aggregator reports `no fragments found`, skip the commit/PR step entirely (no-op). + > 5. `wave_complete()` (marks the current wave complete — takes no args; the server uses the active wave from state). + > 6. Write `/merge-report.md` (issues closed, PR URLs, flight breakdown, drift findings, CHANGELOG aggregation result + PR URL if applicable, deferred items, next-wave preview). > > Final message — exactly one line: > @@ -376,7 +384,7 @@ After every flight has merged: ## Step 5 — Cleanup -- **Success path** (Prime(post-wave) returned `PASS`): call `scripts/wavebus/wave-cleanup `. Report "bus cleaned" in the final summary. Then remove any worktrees the wave created — both same-repo (CC `isolation: "worktree"`) and cross-repo (orchestrator-created). +- **Success path** (Prime(post-wave) returned `PASS`): call `scripts/wavebus/wave-cleanup `. This rm -rf's the wave subtree, including every per-issue `CHANGELOG.fragment.md` written by the flights — Prime(post-wave) Step 4 already merged their content into the target repo's `CHANGELOG.md` under `## Unreleased` and opened the chore PR, so the fragments are intentionally ephemeral and disappear with the bus. Report "bus cleaned" in the final summary. Then remove any worktrees the wave created — both same-repo (CC `isolation: "worktree"`) and cross-repo (orchestrator-created). **Worktree-removal contract.** CC's worktree-isolation mechanism (and Flight sub-agents in general) leaves a git lock file on each Flight's worktree (`lock reason: "claude agent (pid ...)"`). A single `git worktree remove --force` is NOT enough — git refuses to remove a locked working tree. The cleanup MUST unlock first (no-op if unlocked) and then force-remove: @@ -432,6 +440,8 @@ This prompt is what each Flight sub-agent receives. Preserve the SPEC EXECUTOR b > - Tests exercise REAL code paths. Mocks only for true external boundaries (network/fs/APIs). Every new function needs coverage. Assert meaningful outcomes. >2 mocks in one test = wrong approach. > - Report: what was implemented, files, test results, review findings, AC status, concerns. Do NOT report success on failing tests or unmet AC. > +> **CHANGELOG fragment rule.** Do NOT edit `CHANGELOG.md` directly. If the issue introduces a user-visible change worth a changelog entry, write `CHANGELOG.fragment.md` in your issue dir at `/flight-/issue-/CHANGELOG.fragment.md`. The fragment is a markdown document with H3 category headings (`### Breaking`, `### Features`, `### Fixes`, `### Docs`, `### Chore`) followed by bullet lines. Prime(post-wave) aggregates every fragment in the wave into the target repo's `CHANGELOG.md` under `## Unreleased` (deterministic order: numeric flight, then numeric issue; identical bullets deduplicated). This unblocks `flight_partition` parallelism for waves whose stories all touch `CHANGELOG.md` — see `pattern_changelog_fragment_aggregation.md`. +> > **After implementation, run the mechanical half of `/precheck` in the worktree:** > > 1. `./scripts/ci/validate.sh` (or the project's equivalent) diff --git a/skills/wavemachine/SKILL.md b/skills/wavemachine/SKILL.md index 8a86388..252ae2c 100644 --- a/skills/wavemachine/SKILL.md +++ b/skills/wavemachine/SKILL.md @@ -81,7 +81,7 @@ Once pre-flight passes: The first command writes a self-contained HTML snapshot of current wave state to `.status-panel.html` at the repo root. The second command opens it in the operator's default browser. Do not skip either half — the panel is the visual contract this skill maintains with the human, and a /wavemachine launch without an open panel is malformed. (See "Status Panel Lifecycle" below for what the panel does and does NOT do after launch.) 3. **Detect CI trust** by calling `wave_ci_trust_level()` once. (The value is also cached by each `/nextwave auto` iteration; calling it here is informational — it shapes the start announcement.) 4. **Pre-wave kahuna bootstrap** (see "Pre-Wave Kahuna Bootstrap" below). Runs exactly once per Plan on first `/wavemachine` invocation. On resume invocations the wave state already carries `kahuna_branch` and this step is a no-op. -5. **Post to Discord.** `disc_send` to `#wave-status` (`1487386934094462986`): `"🌊 **Wavemachine started** — , waves pending. Agent: **** "`. Resolve identity from `/tmp/claude-agent-.json`. If `disc_send` fails, log and continue — Discord is informational, not a gate. +5. **Post to Discord.** `disc_send` to `#wave-status` (`1487386934094462986`): `"🌊 **Wavemachine started** — , waves pending. Agent: **** "`. Resolve identity from `/tmp/claude-agent-.json`. If `disc_send` fails, log and continue — Discord is informational, not a gate. Then fire-and-forget the auto-updating embed card: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored). 6. **Emit observability event.** `mcp-log wavemachine_start plan= waves= kahuna=` — timestamps autopilot start in the fleet logfile so post-mortem can correlate with sdlc-server tool_call events. ## Status Panel Lifecycle @@ -97,7 +97,7 @@ This is a deliberate design choice: a static file has zero runtime cost, no auth In both cases the regeneration is **fire-and-forget** — we do NOT block the loop on it, we do NOT open a new browser tab (the operator's tab from launch is still pointed at the file), and a regeneration failure is logged but does not abort the loop. The operator refreshes when they want fresh data; the file is current within ~1 second of each lifecycle event. -**What this is NOT.** This is not a live dashboard. If you want live-updating telemetry, look at `scripts/discord-status-post` (the embed it posts to `#wave-status` is PATCHed in place on every call) or the Discord notifications in the "Announcements" section below. The HTML panel is for at-a-glance overview; Discord is for the live timeline. +**What this is NOT.** This is not a live dashboard. For live-updating telemetry, `/wavemachine` invokes `scripts/discord-status-post` as a fire-and-forget call at every state-change point (launch, post-`wave_complete`, post-`wave_flight_done`, and all five terminal exits) — the embed it posts to `#wave-status` is PATCHed in place on every call, so subscribers get a single auto-updating card rather than a stream of plain-text posts. The HTML panel is for at-a-glance overview; the Discord embed is for the live timeline. ## Pre-Wave Kahuna Bootstrap @@ -152,6 +152,9 @@ loop: - "OK" → run `generate-status-panel` (fire-and-forget; auto-regen on wave_complete per "Status Panel Lifecycle"), + then fire-and-forget + `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` + (PATCH the embed in place; failures logged and ignored), then loop back to step 1 - "BLOCKED" → stop; announce abort with the blocker detail - "FAIL" → stop; announce abort with the failure detail @@ -275,6 +278,7 @@ In every interrupt case: - Run `wave-status wavemachine-stop` immediately to clear the flag (the statusline must match reality). - **Leave the in-flight wave's bus tree in place** (`/tmp/wavemachine//wave-/`). Do NOT call `wave-cleanup` on an interrupted wave — the partial state is forensic evidence for the human. - Regenerate `.status-panel.html` synchronously before announcing so the attachment captures the interrupted state: `generate-status-panel`. +- Fire-and-forget the embed update: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored). - Announce the interrupt to `#wave-status` (`1487386934094462986`) with the panel attached: `disc_send(channel_id="1487386934094462986", message="⏸ **Wavemachine interrupted** — , wave mid-flight, bus preserved at . Agent: **** ", attach_path=".status-panel.html")`. - Report to the user: which wave was interrupted, the bus root path, what was merged successfully before the interrupt, how to resume (re-run `/wavemachine` after reviewing the bus). @@ -303,6 +307,7 @@ Before each terminal-event `disc_send` that includes `attach_path`, make sure th - This announcement runs AFTER the trust-score gate's all-green path (see "Trust-Score Gate and Auto-Merge"). In KAHUNA mode, the gate has already auto-merged kahuna→main and posted its own `✅ **Kahuna gate passed**` notification — this announcement closes out the wavemachine session. - In legacy non-KAHUNA mode (no `kahuna_branch` in wave state), the gate is skipped and this announcement runs directly when `wave_next_pending()` returns null. - Regenerate `.status-panel.html` synchronously before posting so the attachment is current: `generate-status-panel`. +- Fire-and-forget the embed update: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored). - Discord `#wave-status`: `disc_send(channel_id="1487386934094462986", message="✅ **Wavemachine complete** — , all waves merged. Run /dod to verify. Agent: **** ", attach_path=".status-panel.html")` - `mcp-log wavemachine_complete plan= status=OK waves_merged=` - Vox (conversational, brief): name, team, project, "wavemachine complete, all waves merged". @@ -310,6 +315,7 @@ Before each terminal-event `disc_send` that includes `attach_path`, make sure th **On gate-blocked completion** (KAHUNA mode, one or more trust signals failed): - Regenerate `.status-panel.html` synchronously before posting so the attachment captures the gate-blocked state: `generate-status-panel`. +- Fire-and-forget the embed update: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored). - Per Procedure C / "Trust-Score Gate and Auto-Merge" any-red path: `disc_send(channel_id="1487386934094462986", message="🛑 **Kahuna gate blocked** — , Plan #: . MR open for review. Agent: **** ", attach_path=".status-panel.html")` - Vox alert: "Kahuna gate blocked for Plan . signals red. Ready for your review." - `mcp-log --level warn wavemachine_complete plan= status=BLOCKED reason="kahuna gate blocked: "` @@ -318,6 +324,7 @@ Before each terminal-event `disc_send` that includes `attach_path`, make sure th **On circuit-breaker trip** (`wave_health_check` non-HEALTHY): - Regenerate `.status-panel.html` synchronously before posting: `generate-status-panel`. +- Fire-and-forget the embed update: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored). - Discord `#wave-status`: `disc_send(channel_id="1487386934094462986", message="🛑 **Wavemachine aborted (circuit breaker)** — : . Agent: **** ", attach_path=".status-panel.html")` - `mcp-log --level error wavemachine_complete plan= status=ABORTED reason="circuit breaker: "` - Call `wave_waiting("wavemachine aborted (circuit breaker): ")` so the plan is explicitly marked paused. @@ -325,6 +332,7 @@ Before each terminal-event `disc_send` that includes `attach_path`, make sure th **On per-wave BLOCKED or FAIL** (from `/nextwave auto` return): - Regenerate `.status-panel.html` synchronously before posting: `generate-status-panel`. +- Fire-and-forget the embed update: `./scripts/discord-status-post --channel-id 1487386934094462986 --state-dir .claude/status` (background, non-blocking; failures logged and ignored). - Discord `#wave-status`: `disc_send(channel_id="1487386934094462986", message="🛑 **Wavemachine aborted** — , wave : . Agent: **** ", attach_path=".status-panel.html")` - `mcp-log --level error wavemachine_complete plan= status=ABORTED wave= reason=""` - Call `wave_waiting("wavemachine aborted: ")`. diff --git a/src/wave_status/__main__.py b/src/wave_status/__main__.py index 26b5ae1..75ac9ec 100644 --- a/src/wave_status/__main__.py +++ b/src/wave_status/__main__.py @@ -86,6 +86,37 @@ def _regenerate_dashboard(root: Path) -> None: pass # discord-status-post not installed or timed out — skip silently +def _safe_regenerate_dashboard(root: Path) -> None: + """Run ``_regenerate_dashboard`` but never raise to the caller [#495]. + + The state mutation has already persisted by the time this is called. + Dashboard regeneration is a derived-view refresh — if it crashes (bad + legacy state.json, missing field, library bug, etc.), we still want + the JSON success envelope to print so MCP wrappers and downstream + sub-agents see ``{ok: true}`` instead of a non-zero exit. + + The exception detail is written to stderr so a human running the CLI + interactively can still see what failed. + """ + try: + _regenerate_dashboard(root) + except Exception as exc: # noqa: BLE001 — best-effort; envelope must print + print( + f"warning: dashboard regeneration failed: {exc}", + file=sys.stderr, + ) + + +def _print_envelope(state: dict) -> None: + """Print the canonical ``{ok, state}`` JSON envelope to stdout [#495]. + + Used by the mutation subcommands so callers (sdlc-server MCP wrappers, + Prime sub-agents) get a parseable success payload rather than the empty + stdout that pre-#495 mutation handlers produced. + """ + print(json.dumps({"ok": True, "state": state})) + + # --------------------------------------------------------------------------- # Subcommand handlers # --------------------------------------------------------------------------- @@ -141,15 +172,17 @@ def _cmd_planning(args: argparse.Namespace) -> None: def _cmd_flight(args: argparse.Namespace) -> None: """Handle ``flight ``.""" root = get_project_root() - flight(args.n, root) - _regenerate_dashboard(root) + result = flight(args.n, root) + _safe_regenerate_dashboard(root) + _print_envelope(result) def _cmd_flight_done(args: argparse.Namespace) -> None: """Handle ``flight-done ``.""" root = get_project_root() - flight_done(args.n, root) - _regenerate_dashboard(root) + result = flight_done(args.n, root) + _safe_regenerate_dashboard(root) + _print_envelope(result) def _cmd_review(args: argparse.Namespace) -> None: @@ -170,8 +203,9 @@ def _cmd_waiting(args: argparse.Namespace) -> None: """Handle ``waiting [msg]``.""" root = get_project_root() msg = args.msg if args.msg else "" - waiting(root, msg=msg) - _regenerate_dashboard(root) + result = waiting(root, msg=msg) + _safe_regenerate_dashboard(root) + _print_envelope(result) def _cmd_waiting_ci(args: argparse.Namespace) -> None: @@ -185,15 +219,17 @@ def _cmd_waiting_ci(args: argparse.Namespace) -> None: def _cmd_close_issue(args: argparse.Namespace) -> None: """Handle ``close-issue ``.""" root = get_project_root() - close_issue(args.n, root) - _regenerate_dashboard(root) + result = close_issue(args.n, root) + _safe_regenerate_dashboard(root) + _print_envelope(result) def _cmd_record_mr(args: argparse.Namespace) -> None: """Handle ``record-mr ``.""" root = get_project_root() - record_mr(args.issue, args.mr, root) - _regenerate_dashboard(root) + result = record_mr(args.issue, args.mr, root) + _safe_regenerate_dashboard(root) + _print_envelope(result) def _cmd_defer(args: argparse.Namespace) -> None: @@ -243,8 +279,9 @@ def _cmd_wavemachine_start(args: argparse.Namespace) -> None: def _cmd_wavemachine_stop(args: argparse.Namespace) -> None: """Handle ``wavemachine-stop``.""" root = get_project_root() - wavemachine_stop(root) - _regenerate_dashboard(root) + result = wavemachine_stop(root) + _safe_regenerate_dashboard(root) + _print_envelope(result) def _cmd_show(args: argparse.Namespace) -> None: diff --git a/src/wave_status/state.py b/src/wave_status/state.py index 30ef0fd..9a20bef 100644 --- a/src/wave_status/state.py +++ b/src/wave_status/state.py @@ -248,10 +248,27 @@ def _plan_default_repo(plan_data: dict) -> str | None: def _issue_repo(plan_data: dict, issue: dict) -> str | None: - """Resolve the effective repo for *issue* — per-issue overrides plan-level.""" + """Resolve the effective repo for *issue*. + + Resolution order: + + 1. Per-issue ``repo`` override — wins. + 2. Per-issue qualified ``ref`` field (e.g. ``owner/name#N``) — when no + per-issue ``repo`` is set, parse the repo prefix off the ref so a + cross-repo plan written from a different orchestrator repo doesn't + silently substitute the orchestrator slug into ``state.json`` keys + (issue #521). + 3. Plan-level default ``repo``. + 4. ``None``. + """ per_issue = issue.get("repo") if isinstance(per_issue, str) and per_issue: return per_issue + ref = issue.get("ref") + if isinstance(ref, str) and "#" in ref: + repo_part, _, _ = ref.rpartition("#") + if repo_part: + return repo_part return _plan_default_repo(plan_data) @@ -297,7 +314,20 @@ def _all_issue_refs(plan_data: dict, default_repo: str | None = None) -> set[str for phase in plan_data.get("phases", []): for wave in phase.get("waves", []): for issue in wave.get("issues", []): - repo = issue.get("repo") if isinstance(issue.get("repo"), str) and issue.get("repo") else plan_default + # Per-issue repo wins; then a qualified ``ref`` field; then + # the plan-level default. Mirrors ``_issue_repo()`` so + # collision detection sees the same key shape ``init`` and + # ``extend_init`` actually write. + per_issue_repo = issue.get("repo") + if isinstance(per_issue_repo, str) and per_issue_repo: + repo = per_issue_repo + else: + ref = issue.get("ref") + if isinstance(ref, str) and "#" in ref: + repo_part, _, _ = ref.rpartition("#") + repo = repo_part or plan_default + else: + repo = plan_default refs.add(_compose_issue_key(issue["number"], repo)) return refs diff --git a/tests/regression/test_precheck_asking_detector.sh b/tests/regression/test_precheck_asking_detector.sh index d528c24..7932882 100755 --- a/tests/regression/test_precheck_asking_detector.sh +++ b/tests/regression/test_precheck_asking_detector.sh @@ -87,6 +87,12 @@ assert_blocks "let_me_know_when" "Let me know when I should run precheck." assert_blocks "do_you_want" "Do you want me to run /precheck?" assert_blocks "may_i_run" "May I run /precheck?" +# Inverted phrasings — issue #545. Trigger word follows "precheck" instead +# of preceding it. Caught by PATTERN_INVERTED. +assert_blocks "inverted_is_precheck" "Is /precheck something I should run?" +assert_blocks "inverted_appropriate" "Would /precheck be appropriate here?" +assert_blocks "inverted_precheck_first" "Precheck — should I do that now?" + # --------------------------------------------------------------------------- # Negative cases — hook MUST NOT block # --------------------------------------------------------------------------- @@ -114,6 +120,13 @@ assert_passes "checklist_text" "Validation green, trivy 0, reviewer clean. Ready assert_passes "distant_words" \ "Should we, given everything that has happened in this very long sentence with lots of detail, also do a precheck?" +# Issue #545 false-positive guard: a past-tense precheck mention followed by +# a separate question about the *next* step must not block. The +# sentence-terminating period severs precheck from "should" so PATTERN_INVERTED +# declines to fire. CRITICAL — narrow PATTERN_INVERTED if this regresses. +assert_passes "negative_past_tense_with_question" \ + "/precheck completed. Should I commit now?" + # --------------------------------------------------------------------------- # Disable env var # --------------------------------------------------------------------------- diff --git a/tests/test_changelog_aggregate.py b/tests/test_changelog_aggregate.py new file mode 100644 index 0000000..9905af0 --- /dev/null +++ b/tests/test_changelog_aggregate.py @@ -0,0 +1,262 @@ +"""Tests for scripts/wavebus/changelog-aggregate. + +Exercises the real bash script via subprocess.run() against a tmp-dir wave +root and a tmp-dir target repo. No mocking of the script under test. + +Pattern mirrors tests/test_wavebus_scripts.py — keep the same shape so the +two test files read consistently. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "scripts" / "wavebus" / "changelog-aggregate" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _run(argv: list[str]) -> subprocess.CompletedProcess: + return subprocess.run(argv, capture_output=True, text=True) + + +def _seed_fragment( + wave_root: Path, + flight: int, + issue: int, + body: str, +) -> Path: + issue_dir = wave_root / f"flight-{flight}" / f"issue-{issue}" + issue_dir.mkdir(parents=True, exist_ok=True) + frag = issue_dir / "CHANGELOG.fragment.md" + frag.write_text(body) + return frag + + +@pytest.fixture() +def wave_root(tmp_path: Path) -> Path: + root = tmp_path / "wave-1" + root.mkdir() + return root + + +@pytest.fixture() +def target_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + return repo + + +# --------------------------------------------------------------------------- +# Argument validation +# --------------------------------------------------------------------------- + + +def test_usage_error_no_args() -> None: + result = _run([str(SCRIPT)]) + assert result.returncode == 1, result.stderr + assert "usage:" in result.stderr + + +def test_usage_error_too_many_args(tmp_path: Path) -> None: + result = _run([str(SCRIPT), str(tmp_path), str(tmp_path), "wave-1", "extra"]) + assert result.returncode == 1, result.stderr + + +def test_rejects_missing_wave_root(target_repo: Path, tmp_path: Path) -> None: + bogus = tmp_path / "does-not-exist" + result = _run([str(SCRIPT), str(bogus), str(target_repo)]) + assert result.returncode == 2, result.stderr + + +def test_rejects_missing_target_repo(wave_root: Path, tmp_path: Path) -> None: + bogus = tmp_path / "no-such-repo" + result = _run([str(SCRIPT), str(wave_root), str(bogus)]) + assert result.returncode == 3, result.stderr + + +# --------------------------------------------------------------------------- +# Empty / no-op cases +# --------------------------------------------------------------------------- + + +def test_empty_wave_root_is_noop(wave_root: Path, target_repo: Path) -> None: + """No fragments anywhere -> exit 0, no CHANGELOG.md created.""" + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + assert not (target_repo / "CHANGELOG.md").exists() + assert "no fragments" in result.stdout.lower() + + +def test_fragments_with_no_recognized_categories_is_noop( + wave_root: Path, target_repo: Path +) -> None: + """Fragments exist but contain no recognized H3 category bullets -> no-op.""" + _seed_fragment( + wave_root, + 1, + 100, + "## Just an H2\n\nSome prose, no category headings.\n", + ) + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + assert not (target_repo / "CHANGELOG.md").exists() + + +# --------------------------------------------------------------------------- +# Basic merge: 3 fragments, 2 categories, deterministic order +# --------------------------------------------------------------------------- + + +def test_basic_merge_three_fragments_two_categories( + wave_root: Path, target_repo: Path +) -> None: + _seed_fragment( + wave_root, + 1, + 101, + "### Features\n- Added A (#101)\n\n### Fixes\n- Fixed X (#101)\n", + ) + _seed_fragment( + wave_root, + 1, + 102, + "### Features\n- Added B (#102)\n", + ) + _seed_fragment( + wave_root, + 2, + 103, + "### Fixes\n- Fixed Y (#103)\n", + ) + + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + + content = (target_repo / "CHANGELOG.md").read_text() + # Heading present + assert "## Unreleased" in content + # Both categories present in canonical order + feat_idx = content.index("### Features") + fix_idx = content.index("### Fixes") + assert feat_idx < fix_idx, "Features should come before Fixes" + # Numeric flight / issue ordering preserved within Features: + # flight-1/issue-101 (Added A) before flight-1/issue-102 (Added B) + a_idx = content.index("Added A (#101)") + b_idx = content.index("Added B (#102)") + assert a_idx < b_idx + # And X (flight 1) before Y (flight 2) within Fixes + x_idx = content.index("Fixed X (#101)") + y_idx = content.index("Fixed Y (#103)") + assert x_idx < y_idx + + +# --------------------------------------------------------------------------- +# Dedup: identical bullet across fragments collapses to one line +# --------------------------------------------------------------------------- + + +def test_duplicate_bullet_collapses_to_one_line( + wave_root: Path, target_repo: Path +) -> None: + _seed_fragment(wave_root, 1, 200, "### Features\n- Same bullet\n") + _seed_fragment(wave_root, 1, 201, "### Features\n- Same bullet\n") + _seed_fragment(wave_root, 2, 202, "### Features\n- Same bullet\n") + + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + + content = (target_repo / "CHANGELOG.md").read_text() + occurrences = content.count("- Same bullet") + assert occurrences == 1, f"expected one occurrence, got {occurrences}" + + +# --------------------------------------------------------------------------- +# Splice into an existing CHANGELOG.md +# --------------------------------------------------------------------------- + + +def test_splice_replaces_existing_unreleased_block( + wave_root: Path, target_repo: Path +) -> None: + _seed_fragment(wave_root, 1, 300, "### Features\n- New entry\n") + + (target_repo / "CHANGELOG.md").write_text( + "# Changelog\n\n" + "## Unreleased\n\n" + "### Fixes\n\n" + "- Stale entry that should disappear\n\n" + "## v1.0.0\n\n" + "- Initial release\n" + ) + + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + + content = (target_repo / "CHANGELOG.md").read_text() + assert "Stale entry that should disappear" not in content + assert "New entry" in content + # Earlier release section preserved + assert "## v1.0.0" in content + assert "Initial release" in content + # Unreleased still appears exactly once + assert content.count("## Unreleased") == 1 + + +def test_splice_inserts_when_unreleased_absent( + wave_root: Path, target_repo: Path +) -> None: + _seed_fragment(wave_root, 1, 400, "### Features\n- Brand new entry\n") + + (target_repo / "CHANGELOG.md").write_text( + "# Changelog\n\n## v1.0.0\n\n- Initial release\n" + ) + + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + + content = (target_repo / "CHANGELOG.md").read_text() + assert "## Unreleased" in content + assert "Brand new entry" in content + # Unreleased should land before the prior release + assert content.index("## Unreleased") < content.index("## v1.0.0") + # H1 still at the top + assert content.startswith("# Changelog") + + +# --------------------------------------------------------------------------- +# Numeric flight ordering: flight-2 before flight-10 +# --------------------------------------------------------------------------- + + +def test_flight_ordering_is_numeric_not_lexical( + wave_root: Path, target_repo: Path +) -> None: + _seed_fragment(wave_root, 2, 500, "### Features\n- From flight 2\n") + _seed_fragment(wave_root, 10, 501, "### Features\n- From flight 10\n") + + result = _run([str(SCRIPT), str(wave_root), str(target_repo)]) + assert result.returncode == 0, result.stderr + + content = (target_repo / "CHANGELOG.md").read_text() + # flight 2 must precede flight 10 even though "10" sorts before "2" lexically + assert content.index("From flight 2") < content.index("From flight 10") + + +# --------------------------------------------------------------------------- +# Wave-id arg accepted (forward-compat) but ignored +# --------------------------------------------------------------------------- + + +def test_wave_id_arg_accepted(wave_root: Path, target_repo: Path) -> None: + _seed_fragment(wave_root, 1, 600, "### Features\n- With wave id\n") + result = _run([str(SCRIPT), str(wave_root), str(target_repo), "wave-1"]) + assert result.returncode == 0, result.stderr + assert "With wave id" in (target_repo / "CHANGELOG.md").read_text() diff --git a/tests/test_install.py b/tests/test_install.py index 9b1e683..74b2141 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -511,30 +511,56 @@ def test_recursive_install_with_exclusions(self, tmp_path: Path) -> None: f"stdout: {result.stdout}\nstderr: {result.stderr}" ) + # Cellar + symlink-farm contract (#560, recommendation B): + # - Cellar at ~/.claude/scripts/ holds the full recursive tree + # (top-level + nested), preserving exec bits and structure. + # - ~/.local/bin/ holds symlinks ONLY for top-level Cellar entries. + cellar = home / ".claude" / "scripts" bin_dir = home / ".local" / "bin" - # Flat + nested + doubly-nested all landed. - assert (bin_dir / "foo").exists(), "flat top-level script missing" - assert (bin_dir / "sub" / "bar").exists(), "single-nested script missing" - assert (bin_dir / "sub" / "baz" / "qux").exists(), ( - "double-nested script missing" + + # Cellar mirrors the source tree end-to-end (top-level + nested). + assert (cellar / "foo").exists(), "flat top-level script missing in Cellar" + assert (cellar / "sub" / "bar").exists(), ( + "single-nested script missing in Cellar" + ) + assert (cellar / "sub" / "baz" / "qux").exists(), ( + "double-nested script missing in Cellar" ) - # Executable bits preserved. - assert os.access(str(bin_dir / "foo"), os.X_OK) - assert os.access(str(bin_dir / "sub" / "bar"), os.X_OK) - assert os.access(str(bin_dir / "sub" / "baz" / "qux"), os.X_OK) - # Non-executable doc copied, NOT marked executable. - readme = bin_dir / "sub" / "README.md" - assert readme.exists(), "non-executable doc not copied" + # Executable bits preserved end-to-end in the Cellar. + assert os.access(str(cellar / "foo"), os.X_OK) + assert os.access(str(cellar / "sub" / "bar"), os.X_OK) + assert os.access(str(cellar / "sub" / "baz" / "qux"), os.X_OK) + # Non-executable doc copied to Cellar, NOT marked executable. + readme = cellar / "sub" / "README.md" + assert readme.exists(), "non-executable doc not copied to Cellar" assert not os.access(str(readme), os.X_OK), ( - "non-executable doc was incorrectly chmod'd +x" + "non-executable doc was incorrectly chmod'd +x in Cellar" ) - # Excluded subtree not copied. - assert not (bin_dir / "excluded" / "__pycache__" / "y").exists(), ( + # Excluded subtree not copied to Cellar either. + assert not (cellar / "excluded" / "__pycache__" / "y").exists(), ( "excluded __pycache__ subtree was copied" ) + # Symlink farm: only top-level entries are symlinked into ~/.local/bin/. + # Nested entries (sub/, sub/baz/, etc.) stay Cellar-only per + # recommendation B from #560. + assert (bin_dir / "foo").is_symlink(), ( + "top-level script should be a symlink in the farm" + ) + assert not (bin_dir / "sub").exists(), ( + "subtree directories should NOT be mirrored into ~/.local/bin/ " + "(granularity B per #560)" + ) + def test_prune_removes_orphan(self, tmp_path: Path) -> None: - """--prune removes installed files whose source no longer exists.""" + """--prune (legacy, manifest-based) removes ~/.local/bin/ files + whose source under scripts/ no longer exists. + + Note: under #560's Cellar layout, structural orphan rot is killed + by the cellar-wipe at install time, so --prune is effectively a + backwards-compat code path. We exercise it with a synthetic + manifest entry to confirm it still functions. + """ repo = self._build_synthetic_repo(tmp_path) home = tmp_path / "home" (home / ".local" / "bin").mkdir(parents=True) @@ -553,7 +579,9 @@ def test_prune_removes_orphan(self, tmp_path: Path) -> None: ).returncode assert rc == 0, "first install failed" - # Plant an orphan (the manifest will pick it up since we'll add it). + # Plant an orphan (manifest-tracked plain file under ~/.local/bin/). + # The legacy --prune walks the manifest and removes plain files + # whose source under scripts/ no longer exists. bin_dir = home / ".local" / "bin" orphan = bin_dir / "ghost" orphan.write_text("#!/bin/bash\necho gone\n") @@ -576,14 +604,24 @@ def test_prune_removes_orphan(self, tmp_path: Path) -> None: assert not orphan.exists(), "orphan was not pruned" # Backup retained. assert (bin_dir / "ghost.bak").exists(), "orphan backup missing" - # Real files still present. - assert (bin_dir / "foo").exists(), "prune removed live top-level script" - assert (bin_dir / "sub" / "bar").exists(), ( - "prune removed live nested script" + # Cellar live files still present (top-level + nested). + cellar = home / ".claude" / "scripts" + assert (cellar / "foo").exists(), "prune removed live top-level Cellar entry" + assert (cellar / "sub" / "bar").exists(), ( + "prune removed live nested Cellar entry" + ) + # Symlink farm top-level still present. + assert (bin_dir / "foo").is_symlink(), ( + "prune removed live top-level symlink-farm entry" ) def test_check_reports_nested_drift(self, tmp_path: Path) -> None: - """--check reports drift for missing nested files (not just top-level).""" + """--check reports drift for missing nested files in the Cellar. + + Under #560 the Cellar (~/.claude/scripts/) holds the full recursive + tree (top-level + nested). Deleting a nested Cellar entry should + surface as drift — the tree no longer matches the source. + """ repo = self._build_synthetic_repo(tmp_path) home = tmp_path / "home" (home / ".local" / "bin").mkdir(parents=True) @@ -601,8 +639,8 @@ def test_check_reports_nested_drift(self, tmp_path: Path) -> None: ).returncode assert rc == 0, "install failed" - # Delete a nested file; --check should flag the drift. - target = home / ".local" / "bin" / "sub" / "bar" + # Delete a nested Cellar file; --check should flag the drift. + target = home / ".claude" / "scripts" / "sub" / "bar" assert target.exists() target.unlink() @@ -615,7 +653,7 @@ def test_check_reports_nested_drift(self, tmp_path: Path) -> None: ) assert result.returncode == 0, "check exited non-zero" assert "sub/bar" in result.stdout, ( - f"--check did not report missing nested file:\n{result.stdout}" + f"--check did not report missing nested Cellar file:\n{result.stdout}" ) assert "out of sync" in result.stdout.lower(), ( f"--check did not flag drift:\n{result.stdout}" diff --git a/tests/test_install_cellar.py b/tests/test_install_cellar.py new file mode 100644 index 0000000..92897a8 --- /dev/null +++ b/tests/test_install_cellar.py @@ -0,0 +1,573 @@ +"""Cellar + symlink-farm tests for the ``install`` script (issue #560). + +Covers the layout migration where kit-managed scripts move from direct +deployment in ``~/.local/bin/`` to a wipe-and-recreate Cellar at +``~/.claude/scripts/``, with ``~/.local/bin/`` becoming a symlink farm +pointing into the Cellar (Homebrew/Nix pattern). + +This file is intentionally self-contained — like +``tests/test_install_merge_hooks_union.py`` it does NOT depend on +``tests/test_install.py`` or ``tests/test_install_merge.py`` (which have +known pre-existing failures from the ``install.sh`` -> ``install`` rename). +It writes synthetic fixtures, invokes ``./install`` with ``HOME`` +overridden, and asserts the post-install on-disk shape. + +Acceptance criteria from #560 are mapped 1:1 to test functions: + +- AC1 — Cellar wipe + redeploy: ``test_cellar_wipe_and_redeploy`` +- AC2 — Symlink farm granularity B: ``test_symlink_farm_granularity_b`` +- AC3 — Removed-from-repo orphan-free re-deploy: + ``test_removed_from_repo_no_orphan`` +- AC4 — Hook path migration in settings.json: + ``test_settings_hook_path_migration`` +- AC5 — Matcher union-merge (re-confirms #556 still holds after #560): + ``test_matcher_union_merge_still_works`` +- AC6 — User-customized plain file backup: + ``test_user_customized_plain_file_backup`` +- AC7 — ``--check`` reports stale symlinks AND old-path hooks: + ``test_check_reports_stale_symlinks``, + ``test_check_reports_old_path_hooks`` +- Bonus — Foreign symlinks (target outside Cellar) NOT touched: + ``test_foreign_symlinks_untouched`` +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +_REPO_DIR = Path(__file__).resolve().parent.parent +_INSTALL_SCRIPT = str(_REPO_DIR / "install") + + +# --------------------------------------------------------------------------- +# Skip conditions +# --------------------------------------------------------------------------- + +_HAS_BASH = shutil.which("bash") is not None +_HAS_JQ = shutil.which("jq") is not None +_HAS_PYTHON3 = shutil.which("python3") is not None + +_SKIP_NO_BASH = pytest.mark.skipif(not _HAS_BASH, reason="bash not available") +_SKIP_NO_JQ = pytest.mark.skipif(not _HAS_JQ, reason="jq not available") +_SKIP_NO_PYTHON3 = pytest.mark.skipif( + not _HAS_PYTHON3, reason="python3 not available (zipapp build)" +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_env(home: Path) -> dict[str, str]: + env = os.environ.copy() + env["HOME"] = str(home) + return env + + +def _read_json(path: Path) -> dict: + return json.loads(path.read_text()) + + +def _write_json(path: Path, data: dict) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(data, indent=2)) + + +def _build_synthetic_repo(tmp_path: Path) -> Path: + """Materialize a minimal repo at ``tmp_path/repo`` exercising + the Cellar + symlink-farm code path without dragging in skills, + crystallizer, MCPs, or built packages. + + Layout:: + + repo/ + ├── install (real script) + ├── config/ + │ └── settings.template.json (with new ~/.claude/scripts/ paths) + └── scripts/ + ├── foo (top-level — gets a symlink) + ├── bar (top-level — gets a symlink) + ├── ci/ + │ └── check-deps.sh (stub: no deps) + └── hooks/nerf/ + └── pre-compact.sh (subtree — Cellar only, no symlink) + """ + repo = tmp_path / "repo" + scripts = repo / "scripts" + scripts.mkdir(parents=True) + + # Top-level scripts (these get symlinks). + (scripts / "foo").write_text("#!/bin/bash\necho foo\n") + os.chmod(scripts / "foo", 0o755) + (scripts / "bar").write_text("#!/bin/bash\necho bar\n") + os.chmod(scripts / "bar", 0o755) + + # Subtree (Cellar-only, no symlink under recommendation B). + nerf = scripts / "hooks" / "nerf" + nerf.mkdir(parents=True) + (nerf / "pre-compact.sh").write_text("#!/bin/bash\necho pre-compact\n") + os.chmod(nerf / "pre-compact.sh", 0o755) + + # Stub ci/check-deps.sh so the post-install dep gate is a no-op. + ci_dir = scripts / "ci" + ci_dir.mkdir() + (ci_dir / "check-deps.sh").write_text( + "#!/usr/bin/env bash\ncheck_deps() { return 0; }\n" + ) + (ci_dir / "check-deps.sh").chmod(0o755) + + # Minimal settings template using the NEW Cellar hook path. + config_dir = repo / "config" + config_dir.mkdir() + template = { + "hooks": { + "PreCompact": [ + { + "matcher": "*", + "hooks": [ + { + "type": "command", + "command": "~/.claude/scripts/hooks/nerf/pre-compact.sh", + } + ], + } + ], + }, + } + (config_dir / "settings.template.json").write_text(json.dumps(template, indent=2)) + + # Drop the real install script in. + shutil.copy(_INSTALL_SCRIPT, repo / "install") + os.chmod(repo / "install", 0o755) + return repo + + +def _build_sandbox_home(tmp_path: Path) -> Path: + home = tmp_path / "home" + (home / ".local" / "bin").mkdir(parents=True) + (home / ".claude" / "skills").mkdir(parents=True) + return home + + +def _run_install(repo: Path, args: list[str], home: Path) -> tuple[int, str, str]: + result = subprocess.run( + ["bash", str(repo / "install")] + args, + capture_output=True, + text=True, + env=_make_env(home), + timeout=120, + ) + return result.returncode, result.stdout, result.stderr + + +# --------------------------------------------------------------------------- +# AC1 — Cellar wipe + redeploy +# --------------------------------------------------------------------------- + +@_SKIP_NO_BASH +def test_cellar_wipe_and_redeploy(tmp_path: Path) -> None: + """``~/.claude/scripts/`` is the canonical install location and the + entire tree is wiped and recreated on every ``./install``.""" + repo = _build_synthetic_repo(tmp_path) + home = _build_sandbox_home(tmp_path) + + # Plant a stale file in the Cellar BEFORE install — verify it disappears. + cellar = home / ".claude" / "scripts" + cellar.mkdir(parents=True, exist_ok=True) + stale = cellar / "leftover-from-old-install" + stale.write_text("#!/bin/bash\necho stale\n") + + rc, out, err = _run_install(repo, ["--scripts"], home) + assert rc == 0, f"install --scripts failed: stdout={out}\nstderr={err}" + + # Cellar exists and contains current sources. + assert (cellar / "foo").exists(), "Cellar missing top-level: foo" + assert (cellar / "bar").exists(), "Cellar missing top-level: bar" + assert (cellar / "hooks" / "nerf" / "pre-compact.sh").exists(), ( + "Cellar missing nested hook: hooks/nerf/pre-compact.sh" + ) + + # Stale file from before install was wiped. + assert not stale.exists(), ( + f"Cellar wipe did not delete stale file: {stale}" + ) + + # Cellar files have exec bits preserved. + assert os.access(str(cellar / "foo"), os.X_OK), "Cellar foo not executable" + assert os.access(str(cellar / "hooks" / "nerf" / "pre-compact.sh"), os.X_OK), ( + "Cellar hook script not executable" + ) + + +# --------------------------------------------------------------------------- +# AC2 — Symlink farm: granularity B (top-level only) +# --------------------------------------------------------------------------- + +@_SKIP_NO_BASH +def test_symlink_farm_granularity_b(tmp_path: Path) -> None: + """``~/.local/bin/`` contains symlinks for top-level Cellar entries + only. Subtrees (hooks/, vox-providers/, etc.) stay Cellar-only.""" + repo = _build_synthetic_repo(tmp_path) + home = _build_sandbox_home(tmp_path) + + rc, _out, err = _run_install(repo, ["--scripts"], home) + assert rc == 0, f"install failed: {err}" + + bin_dir = home / ".local" / "bin" + cellar = home / ".claude" / "scripts" + + # Top-level entries are symlinks pointing into the Cellar. + for name in ("foo", "bar"): + link = bin_dir / name + assert link.is_symlink(), f"{name} should be a symlink in {bin_dir}" + target = os.readlink(str(link)) + # Target may be absolute or relative — resolve via os.path.realpath. + resolved = Path(os.path.realpath(str(link))) + assert resolved == cellar / name, ( + f"{name} symlink target: expected {cellar / name}, got {resolved}" + ) + + # Subtree entries are NOT symlinked (granularity B). + assert not (bin_dir / "hooks").exists(), ( + "hooks/ subtree should NOT be mirrored into ~/.local/bin/ " + "(recommendation B from #560)" + ) + + +# --------------------------------------------------------------------------- +# AC3 — Removed-from-repo orphan-free re-deploy +# --------------------------------------------------------------------------- + +@_SKIP_NO_BASH +def test_removed_from_repo_no_orphan(tmp_path: Path) -> None: + """Running ``./install`` after a script is removed from the repo + deletes both the Cellar entry AND the corresponding ``~/.local/bin/`` + symlink — without any ``--prune`` flag.""" + repo = _build_synthetic_repo(tmp_path) + home = _build_sandbox_home(tmp_path) + + # First install: foo and bar both present. + rc, _, err = _run_install(repo, ["--scripts"], home) + assert rc == 0, f"first install failed: {err}" + + cellar = home / ".claude" / "scripts" + bin_dir = home / ".local" / "bin" + assert (cellar / "foo").exists() + assert (bin_dir / "foo").is_symlink() + + # Remove foo from the repo. + (repo / "scripts" / "foo").unlink() + + # Second install: foo should be gone from BOTH Cellar and bin/. + rc, _, err = _run_install(repo, ["--scripts"], home) + assert rc == 0, f"second install failed: {err}" + + assert not (cellar / "foo").exists(), ( + "removed-from-repo script still in Cellar after re-install" + ) + assert not (bin_dir / "foo").exists(), ( + "removed-from-repo symlink still in ~/.local/bin/ — orphan rot" + ) + # bar is unaffected. + assert (cellar / "bar").exists() + assert (bin_dir / "bar").is_symlink() + + +# --------------------------------------------------------------------------- +# AC4 — Settings hook path migration +# --------------------------------------------------------------------------- + +@_SKIP_NO_BASH +@_SKIP_NO_JQ +def test_settings_hook_path_migration(tmp_path: Path) -> None: + """``~/.claude/settings.json`` hook command paths get rewritten from + ``~/.local/bin/hooks/...`` to ``~/.claude/scripts/hooks/...`` during + ``merge_settings()``.""" + repo = _build_synthetic_repo(tmp_path) + home = _build_sandbox_home(tmp_path) + + # Pre-existing user settings with the OLD path shape. + settings = home / ".claude" / "settings.json" + _write_json(settings, { + "hooks": { + "PreCompact": [ + { + "matcher": "*", + "hooks": [ + { + "type": "command", + "command": "~/.local/bin/hooks/nerf/pre-compact.sh", + } + ], + } + ], + }, + }) + + rc, _out, err = _run_install(repo, ["--config"], home) + assert rc == 0, f"install --config failed: {err}" + + merged = _read_json(settings) + cmd = merged["hooks"]["PreCompact"][0]["hooks"][0]["command"] + assert cmd == "~/.claude/scripts/hooks/nerf/pre-compact.sh", ( + f"hook path not migrated: got {cmd}" + ) + + # Backup retains the original. + bak = settings.with_suffix(".json.bak") + assert bak.exists(), "settings.json.bak not created during migration" + bak_data = _read_json(bak) + bak_cmd = bak_data["hooks"]["PreCompact"][0]["hooks"][0]["command"] + assert bak_cmd == "~/.local/bin/hooks/nerf/pre-compact.sh", ( + "settings.json.bak should preserve the pre-migration path" + ) + + +# --------------------------------------------------------------------------- +# AC5 — Matcher union-merge still works (re-confirm #556 wave-1 contract) +# --------------------------------------------------------------------------- + +@_SKIP_NO_BASH +@_SKIP_NO_JQ +def test_matcher_union_merge_still_works(tmp_path: Path) -> None: + """``merge_settings()`` continues to union-merge new matcher entries + into existing event arrays (regression test for #556 logic now sharing + code path with the #560 path-rewrite).""" + repo = _build_synthetic_repo(tmp_path) + home = _build_sandbox_home(tmp_path) + + # Override the synthetic template to have TWO matchers in a single + # event. The user has only one — merge must add the second. + template = { + "hooks": { + "SessionStart": [ + { + "matcher": "*", + "hooks": [ + {"type": "command", "command": "default-session-start.sh"} + ], + }, + { + "matcher": "compact", + "hooks": [ + {"type": "command", "command": "session-start-compact.sh"} + ], + }, + ], + }, + } + (repo / "config" / "settings.template.json").write_text( + json.dumps(template, indent=2) + ) + + settings = home / ".claude" / "settings.json" + _write_json(settings, { + "hooks": { + "SessionStart": [ + { + "matcher": "*", + "hooks": [ + {"type": "command", "command": "user-session-start.sh"} + ], + } + ], + }, + }) + + rc, _out, err = _run_install(repo, ["--config"], home) + assert rc == 0, f"install --config failed: {err}" + + merged = _read_json(settings) + matchers = sorted(e["matcher"] for e in merged["hooks"]["SessionStart"]) + assert matchers == ["*", "compact"], ( + f"matcher union merge failed: got {matchers}" + ) + # User customization on '*' preserved. + star = next(e for e in merged["hooks"]["SessionStart"] if e["matcher"] == "*") + assert any(h["command"] == "user-session-start.sh" for h in star["hooks"]), ( + "user customization on shared matcher was overwritten" + ) + + +# --------------------------------------------------------------------------- +# AC6 — User-customized plain file gets backed up before symlink replacement +# --------------------------------------------------------------------------- + +@_SKIP_NO_BASH +def test_user_customized_plain_file_backup(tmp_path: Path) -> None: + """If ``~/.local/bin/