Skip to content

refactor(ci): add safety guards and consolidate directory removal in cache restoration#230

Closed
Copilot wants to merge 5 commits intomk/dev/fix_external_app_rebuild_pipelinefrom
copilot/sub-pr-228
Closed

refactor(ci): add safety guards and consolidate directory removal in cache restoration#230
Copilot wants to merge 5 commits intomk/dev/fix_external_app_rebuild_pipelinefrom
copilot/sub-pr-228

Conversation

Copy link

Copilot AI commented Feb 9, 2026

Summary

Addresses review feedback on the cache restoration fix by adding safety guards to prevent accidental deletion and consolidating duplicate logic.

Changes:

  • Safety guards for rm -rf: Validates path is non-empty, not root, and under $GITHUB_WORKSPACE/(apps-external|apps)/ before deletion
  • Consolidated logic: Extracted directory removal into safe_remove_app_dir() function, eliminating duplication between JFrog and GitHub cache paths
  • Code quality: Removed trailing whitespace, improved regex pattern clarity
# Function to safely remove app directory before cache restoration
safe_remove_app_dir() {
  local app_path="$1"
  
  # Safety checks before removing directory
  if [ -z "$app_path" ]; then
    echo "❌ Error: app_path is empty, refusing to delete"
    exit 1
  fi
  
  if [ "$app_path" = "/" ]; then
    echo "❌ Error: app_path is root directory, refusing to delete"
    exit 1
  fi
  
  # Verify path is under expected workspace directories
  local pattern="^${GITHUB_WORKSPACE}/(apps-external|apps)/"
  if [[ ! "$app_path" =~ $pattern ]]; then
    echo "❌ Error: app_path '$app_path' is not under expected workspace directory"
    exit 1
  fi
  
  # Remove directory if it exists
  if [ -d "$app_path" ]; then
    echo "🗑️  Removing existing directory: $app_path"
    rm -rf "$app_path"
  fi
}

TODO

  • Add safety guards to directory removal
  • Extract duplicate logic into function
  • Remove whitespace issues

Checklist


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

printminion-co and others added 2 commits February 5, 2026 14:48
The 'Restore cached apps' step was extracting tarballs into directories
that already existed from git submodule checkout. This caused files to
be extracted into the wrong locations (e.g., LICENSE files ending up in
apps-external/ instead of apps-external/mail/).

The fix removes the existing directory before extracting the cached
tarball, ensuring clean restoration for both JFrog and GitHub cache
sources.

Fixes issue where mail app LICENSES folder landed in wrong directory
during first pipeline run, but worked correctly after manual re-run.

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
Copilot AI and others added 3 commits February 9, 2026 13:17
Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com>
Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com>
Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix cache restoration by removing existing app directories refactor(ci): add safety guards and consolidate directory removal in cache restoration Feb 9, 2026
@printminion-co printminion-co force-pushed the mk/dev/fix_external_app_rebuild_pipeline branch from 10310f7 to bc7ad3f Compare February 9, 2026 14:25
@printminion-co printminion-co deleted the copilot/sub-pr-228 branch February 9, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants