From 9c73e68528e4f296d7ac149272c9fc08c1216768 Mon Sep 17 00:00:00 2001 From: 404prefrontalcortexnotfound <106208474+404prefrontalcortexnotfound@users.noreply.github.com> Date: Thu, 9 Apr 2026 05:29:50 +1000 Subject: [PATCH] fix(bash): sed replacement escaping, BSD portability, dead cleanup in update-agent-context.sh (#2090) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(bash): sed replacement escaping, BSD portability, dead cleanup code Three bugs in update-agent-context.sh: 1. **sed escaping targets wrong side** (line 318-320): The escaping function escapes regex pattern characters (`[`, `.`, `*`, `^`, `$`, `+`, `{`, `}`, `|`) but these variables are used as sed *replacement* strings, not patterns. Only `&` (insert matched text), `\` (escape char), and `|` (our sed delimiter) are special in the replacement context. Also adds escaping for `project_name` which was used unescaped. 2. **BSD sed newline insertion fails on macOS** (line 364-366): Uses bash variable expansion to insert a literal newline into a sed replacement string. This works on GNU sed (Linux) but fails silently on BSD sed (macOS). Replaced with portable awk approach that works on both platforms. 3. **cleanup() removes non-existent files** (line 125-126): The cleanup trap attempts `rm -f /tmp/agent_update_*_$$` and `rm -f /tmp/manual_additions_$$` but the script never creates files matching these patterns — all temp files use `mktemp`. The wildcard with `$$` (PID) in /tmp could theoretically match unrelated files. Fixes #154 (macOS sed failure) Fixes #293 (sed expression errors) Related: #338 (shellcheck findings) * fix: restore forge case and revert copilot path change Address PR review feedback: - Restore forge) case in update_specific_agent since src/specify_cli/integrations/forge/__init__.py still exists - Revert COPILOT_FILE path from .github/agents/ back to .github/ to stay consistent with Python integration and tests - Restore FORGE_FILE variable, comments, and usage strings Co-Authored-By: Claude Opus 4.6 (1M context) * refactor: extract repeated sed escaping into _esc_sed helper Address Gemini review feedback — the inline sed escaping pattern appeared 7 times in create_new_agent_file(). Extract to a single helper function for maintainability and readability. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: restore combined AGENTS_FILE label in update_all_existing_agents Gemini correctly identified that splitting AGENTS_FILE updates into individual calls is redundant — _update_if_new deduplicates by realpath, so only the first call logs. Restore the combined label and add back missing Pi reference. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: remove pre-escaped && in JS/TS commands now that _esc_sed handles it The old code manually pre-escaped & as \& in get_commands_for_language because the broken escaping function didn't handle &. Now that _esc_sed properly escapes replacement-side specials, the pre-escaping causes double-escaping: && becomes \&\& in generated files. Found by blind audit. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: split awk && mv to let set -e catch awk failures Under set -e, the left side of && does not trigger errexit on failure. Split into two statements so awk failures are fatal instead of silent. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: guard empty _CLEANUP_FILES array for Bash 3.2 compatibility On Bash 3.2, the ${arr[@]+"${arr[@]}"} pattern expands to a single empty string when the array is empty, causing rm to target .bak and .tmp in the current directory. Use explicit length check instead, which also avoids the word-splitting risk of unquoted expansion. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Bo Bobson Co-authored-by: Claude Opus 4.6 (1M context) --- scripts/bash/update-agent-context.sh | 48 ++++++++++++++++++---------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/scripts/bash/update-agent-context.sh b/scripts/bash/update-agent-context.sh index b0ef4b422..fce379b34 100644 --- a/scripts/bash/update-agent-context.sh +++ b/scripts/bash/update-agent-context.sh @@ -117,13 +117,19 @@ log_warning() { echo "WARNING: $1" >&2 } +# Track temporary files for cleanup on interrupt +_CLEANUP_FILES=() + # Cleanup function for temporary files cleanup() { local exit_code=$? # Disarm traps to prevent re-entrant loop trap - EXIT INT TERM - rm -f /tmp/agent_update_*_$$ - rm -f /tmp/manual_additions_$$ + if [ ${#_CLEANUP_FILES[@]} -gt 0 ]; then + for f in "${_CLEANUP_FILES[@]}"; do + rm -f "$f" "$f.bak" "$f.tmp" + done + fi exit $exit_code } @@ -268,7 +274,7 @@ get_commands_for_language() { echo "cargo test && cargo clippy" ;; *"JavaScript"*|*"TypeScript"*) - echo "npm test \\&\\& npm run lint" + echo "npm test && npm run lint" ;; *) echo "# Add commands for $lang" @@ -281,10 +287,15 @@ get_language_conventions() { echo "$lang: Follow standard conventions" } +# Escape sed replacement-side specials for | delimiter. +# & and \ are replacement-side specials; | is our sed delimiter. +_esc_sed() { printf '%s\n' "$1" | sed 's/[\\&|]/\\&/g'; } + create_new_agent_file() { local target_file="$1" local temp_file="$2" - local project_name="$3" + local project_name + project_name=$(_esc_sed "$3") local current_date="$4" if [[ ! -f "$TEMPLATE_FILE" ]]; then @@ -307,18 +318,19 @@ create_new_agent_file() { # Replace template placeholders local project_structure project_structure=$(get_project_structure "$NEW_PROJECT_TYPE") + project_structure=$(_esc_sed "$project_structure") local commands commands=$(get_commands_for_language "$NEW_LANG") - + local language_conventions language_conventions=$(get_language_conventions "$NEW_LANG") - - # Perform substitutions with error checking using safer approach - # Escape special characters for sed by using a different delimiter or escaping - local escaped_lang=$(printf '%s\n' "$NEW_LANG" | sed 's/[\[\.*^$()+{}|]/\\&/g') - local escaped_framework=$(printf '%s\n' "$NEW_FRAMEWORK" | sed 's/[\[\.*^$()+{}|]/\\&/g') - local escaped_branch=$(printf '%s\n' "$CURRENT_BRANCH" | sed 's/[\[\.*^$()+{}|]/\\&/g') + + local escaped_lang=$(_esc_sed "$NEW_LANG") + local escaped_framework=$(_esc_sed "$NEW_FRAMEWORK") + commands=$(_esc_sed "$commands") + language_conventions=$(_esc_sed "$language_conventions") + local escaped_branch=$(_esc_sed "$CURRENT_BRANCH") # Build technology stack and recent change strings conditionally local tech_stack @@ -361,17 +373,18 @@ create_new_agent_file() { fi done - # Convert \n sequences to actual newlines - newline=$(printf '\n') - sed -i.bak2 "s/\\\\n/${newline}/g" "$temp_file" + # Convert literal \n sequences to actual newlines (portable — works on BSD + GNU) + awk '{gsub(/\\n/,"\n")}1' "$temp_file" > "$temp_file.tmp" + mv "$temp_file.tmp" "$temp_file" - # Clean up backup files - rm -f "$temp_file.bak" "$temp_file.bak2" + # Clean up backup files from sed -i.bak + rm -f "$temp_file.bak" # Prepend Cursor frontmatter for .mdc files so rules are auto-included if [[ "$target_file" == *.mdc ]]; then local frontmatter_file frontmatter_file=$(mktemp) || return 1 + _CLEANUP_FILES+=("$frontmatter_file") printf '%s\n' "---" "description: Project Development Guidelines" "globs: [\"**/*\"]" "alwaysApply: true" "---" "" > "$frontmatter_file" cat "$temp_file" >> "$frontmatter_file" mv "$frontmatter_file" "$temp_file" @@ -395,6 +408,7 @@ update_existing_agent_file() { log_error "Failed to create temporary file" return 1 } + _CLEANUP_FILES+=("$temp_file") # Process the file in one pass local tech_stack=$(format_technology_stack "$NEW_LANG" "$NEW_FRAMEWORK") @@ -519,6 +533,7 @@ update_existing_agent_file() { if ! head -1 "$temp_file" | grep -q '^---'; then local frontmatter_file frontmatter_file=$(mktemp) || { rm -f "$temp_file"; return 1; } + _CLEANUP_FILES+=("$frontmatter_file") printf '%s\n' "---" "description: Project Development Guidelines" "globs: [\"**/*\"]" "alwaysApply: true" "---" "" > "$frontmatter_file" cat "$temp_file" >> "$frontmatter_file" mv "$frontmatter_file" "$temp_file" @@ -571,6 +586,7 @@ update_agent_file() { log_error "Failed to create temporary file" return 1 } + _CLEANUP_FILES+=("$temp_file") if create_new_agent_file "$target_file" "$temp_file" "$project_name" "$current_date"; then if mv "$temp_file" "$target_file"; then