Fix MCP cleanup trap scoping issue causing exit errors

[fletchgqc]

Fixes #72

Summary

This PR fixes the bash "unbound variable" errors that occur when ClaudeBox exits with MCP servers configured. The issue was caused by EXIT traps not having access to local variables from the function where they're defined.

Changes Made

• Simplified cleanup_mcp_files() to only use mcp_temp_files array
• Added user_mcp_file to mcp_temp_files array for proper cleanup tracking
• Added parameter expansion safety (${mcp_temp_files[@]:-}) to handle empty arrays
• Removed direct variable access in cleanup function that caused scoping issues

Technical Details

The core issue was that EXIT traps execute in a different context where function-local variables are not accessible:

run_claudebox_container() {
    local user_mcp_file=""      # Function-scoped
    local project_mcp_file=""   # Function-scoped

    cleanup_mcp_files() {
        # Trap runs when script exits - function scope is gone!
        if [[ -n "$user_mcp_file" ]]; then  # ERROR: unbound variable
            rm -f "$user_mcp_file"
        fi
    }
    trap cleanup_mcp_files EXIT
}

The fix ensures all temporary files are tracked in the mcp_temp_files array which is accessible to the EXIT trap, eliminating the scoping issue entirely.

Testing

• ✅ Bash syntax validation passes (bash -n lib/docker.sh)
• ✅ No more "unbound variable" errors on exit
• ✅ All MCP temp files still cleaned up properly
• ✅ Exit code 0 instead of 1

Impact

• Fixes user-visible errors on every ClaudeBox exit with MCP servers
• Resolves exit code 1 that can break scripts and automation
• Improves reliability of the cleanup process
• Fully backwards compatible - only improves the cleanup mechanism

The fix eliminates a bash scoping anti-pattern and makes the cleanup process more predictable and maintainable.

Summary by Sourcery

Fix trap scoping issue in cleanup_mcp_files by consolidating MCP temp files into a single array accessible to the EXIT trap, eliminating unbound variable errors and restoring exit code 0.

Bug Fixes:

• Avoid bash "unbound variable" errors on exit by removing direct access to function-scoped variables in the cleanup trap
• Restore exit code 0 by ensuring cleanup_mcp_files always runs without error

Enhancements:

• Simplify cleanup_mcp_files to iterate solely over mcp_temp_files with safe parameter expansion
• Add user and project MCP config files to mcp_temp_files array for consistent tracking and removal

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR refactors the MCP cleanup process by consolidating all temporary file paths into a single shared array and updating the EXIT trap to safely iterate over that array, removing direct variable references and eliminating unbound variable errors on exit.

Sequence diagram for MCP temp file cleanup on EXIT trap

sequenceDiagram
participant BashScript
participant "cleanup_mcp_files()"
BashScript->>"cleanup_mcp_files()": EXIT trap triggered
"cleanup_mcp_files()"->>"mcp_temp_files array": Iterate over files
"cleanup_mcp_files()"->>BashScript: Remove each temp file if exists

Loading

Class diagram for MCP temp file tracking and cleanup

classDiagram
class BashScript {
  +mcp_temp_files : array
  +user_mcp_file : string
  +project_mcp_file : string
  +cleanup_mcp_files()
}
BashScript : cleanup_mcp_files() uses mcp_temp_files
BashScript : user_mcp_file added to mcp_temp_files
BashScript : project_mcp_file (tracked via temp_project_file in mcp_temp_files)

Loading

File-Level Changes

Change	Details	Files
Refactor cleanup_mcp_files to use only the shared mcp_temp_files array	Replaced loop target with ${mcp_temp_files[@]:-} to guard against empty arrays Removed explicit checks and removals for user_mcp_file and project_mcp_file	lib/docker.sh
Ensure all MCP temp files are tracked via the mcp_temp_files array	Added mcp_temp_files+=("$user_mcp_file") when generating the user MCP config Added comment noting that temp_project_file is already included in the array	lib/docker.sh

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#72	Simplify cleanup to only use the mcp_temp_files array for tracking and removing temporary MCP files.	✅
#72	Remove direct access to user_mcp_file and project_mcp_file variables in the cleanup function to avoid unbound variable errors due to scoping issues.	✅
#72	Add parameter expansion safety to the cleanup function to handle cases where the mcp_temp_files array may be empty.	✅

Possibly linked issues

• MCP cleanup causes exit error with unbound variable #72: The PR resolves the issue by modifying the cleanup trap to use a global array for temporary files instead of inaccessible local variables, adding parameter expansion safety.
• MCP cleanup causes exit error with unbound variable #72: The PR fixes the 'user_mcp_file: unbound variable' error by adding user_mcp_file to the mcp_temp_files array for cleanup.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• Ensure mcp_temp_files is explicitly initialized (e.g. mcp_temp_files=()) before use to avoid potential unbound array warnings under set -u.
• Consider extending the trap to include INT and TERM so that cleanup runs on all termination paths, not just EXIT.
• You may want to move the cleanup_mcp_files function and its trap setup outside run_claudebox_container to avoid redefining them on every invocation and keep cleanup logic centralized.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- Ensure mcp_temp_files is explicitly initialized (e.g. mcp_temp_files=()) before use to avoid potential unbound array warnings under set -u.
- Consider extending the trap to include INT and TERM so that cleanup runs on all termination paths, not just EXIT.
- You may want to move the cleanup_mcp_files function and its trap setup outside run_claudebox_container to avoid redefining them on every invocation and keep cleanup logic centralized.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}