mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(bash): sed replacement escaping, BSD portability, dead cleanup in update-agent-context.sh (#2090)
* 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) <noreply@anthropic.com>
* 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) <noreply@anthropic.com>
* 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) <noreply@anthropic.com>
* 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) <noreply@anthropic.com>
* 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) <noreply@anthropic.com>
* 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) <noreply@anthropic.com>
---------
Co-authored-by: Bo Bobson <bo@noneofyourbusiness.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
404prefrontalcortexnotfound
committed by
GitHub
GitHub
parent
8472e44215
commit
9c73e68528
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user