Skip to content

fix(bash): sed replacement escaping, BSD portability, dead cleanup code #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
404prefrontalcortexnotfound wants to merge 7 commits into from
+32 −16
Closed

fix(bash): sed replacement escaping, BSD portability, dead cleanup code #1

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8f97d7e
fix(bash): sed replacement escaping, BSD portability, dead cleanup code
Apr 4, 2026
1661ddb
fix: restore forge case and revert copilot path change
Apr 7, 2026
3280cd7
refactor: extract repeated sed escaping into _esc_sed helper
Apr 7, 2026
e4933f9
fix: restore combined AGENTS_FILE label in update_all_existing_agents
Apr 7, 2026
e26a1e6
fix: remove pre-escaped && in JS/TS commands now that _esc_sed handle…
Apr 7, 2026
414d016
fix: split awk && mv to let set -e catch awk failures
Apr 7, 2026
aac7864
fix: guard empty _CLEANUP_FILES array for Bash 3.2 compatibility
Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions scripts/bash/update-agent-context.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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"
Expand All @@ -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'; }
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The _esc_sed function uses printf '%s\n' which appends a trailing newline to the output. While Bash command substitution $(...) strips trailing newlines, this behavior can be problematic if the input string itself contains intended trailing newlines or if the function is used in a context where newlines are not stripped. More importantly, if the input contains internal newlines, the resulting string will break the sed commands later in the script (e.g., line 367) because sed replacement strings cannot contain unescaped literal newlines. It is safer to use printf '%s' to avoid adding unnecessary characters.

Suggested change
_esc_sed() { printf '%s\n' "$1" | sed 's/[\\&|]/\\&/g'; }
_esc_sed() { printf '%s' "$1" | sed 's/[\\&|]/\\&/g'; }

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The _esc_sed function correctly handles \, &, and the | delimiter, which are the primary special characters in a sed replacement string. However, it does not escape literal newline characters. If an input variable (e.g., from a plan file) were to contain a newline, the sed command in create_new_agent_file would fail due to an unterminated s command. While current inputs are expected to be single-line, adding a substitution to escape newlines as \n (backslash followed by a real newline) would make this utility more robust against multi-line data.


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
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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")
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down