Skip to content

Fix temp directory leak in Azure fork deploy#110

Merged
ewega merged 2 commits intomainfrom
copilot/fix-temp-directory-leak
Mar 4, 2026
Merged

Fix temp directory leak in Azure fork deploy#110
ewega merged 2 commits intomainfrom
copilot/fix-temp-directory-leak

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 4, 2026

findRepoRoot() creates a temp dir when --repo-url is set but the caller runDeployAzure never cleaned it up, leaking ~100MB+ of cloned source per deploy. The local deploy path (deploy_local.go) already handles this correctly via defer os.RemoveAll.

Changes

  • cmd/deploy_azure.go: Added defer os.RemoveAll(repoRoot) in runDeployAzure immediately after findRepoRoot(), guarded by azureRepoURL != "" to avoid deleting a pre-existing on-disk repo root returned by the directory-walk branch.
repoRoot, err := findRepoRoot()
if err != nil {
    return err
}
if azureRepoURL != "" {
    defer os.RemoveAll(repoRoot)
}
Original prompt

This section details on the original issue you should resolve

<issue_title>Fix temp directory leak in Azure fork deploy</issue_title>
<issue_description>## Problem

findRepoRoot() in cmd/deploy_azure.go:377 creates a temporary directory with os.MkdirTemp, clones a DevLake repo into it, and returns the path. However, the caller (runDeployAzure) never removes the temporary directory after use.

go func findRepoRoot() (string, error) { if azureRepoURL != "" { tmpDir, err := os.MkdirTemp("", "devlake-clone-*") // ... return tmpDir, nil // caller never cleans up tmpDir } }

By contrast, deployLocalFork_clone in deploy_local.go:304-309 correctly uses defer os.RemoveAll(tmpDir).

Each Azure fork deploy leaks ~100MB+ of cloned source code in the system temp directory.

Fix

Either:

  1. Return a cleanup function alongside the path: func findRepoRoot() (string, func(), error)
  2. Or have runDeployAzure defer cleanup after calling findRepoRoot

Acceptance Criteria

  • Temp directory is removed after Azure fork deploy completes (success or failure)
  • Existing local fork deploy still works (deploy_local.go already handles this correctly)
  • go build ./... and go test ./... pass</issue_description>

<agent_instructions>The fix pattern already exists in cmd/deploy_local.go:304-309 which uses defer os.RemoveAll(tmpDir). Apply the same pattern to cmd/deploy_azure.go. Either return a cleanup function from findRepoRoot() or have runDeployAzure defer cleanup after calling it. Option 2 (defer in caller) is simpler — add defer os.RemoveAll(repoRoot) right after the findRepoRoot() call in runDeployAzure, guarded by a check that repoRoot came from a temp dir (azureRepoURL != ""). Build with go build ./... and test with go test ./....</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@ewega **Agent context:** The fix pattern already exists in `cmd/deploy_local.go:304-309` which uses `defer os.RemoveAll(tmpDir)`. Apply the same pattern to `cmd/deploy_azure.go`. Either return a cleanup function from `findRepoRoot()` or have `runDeployAzure` defer cleanup after calling it. Option 2 (defer in caller) is simpler. Build with `go build ./...` and test with `go test ./...`.

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

…URL is set

Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix temporary directory leak in Azure fork deploy Fix temp directory leak in Azure fork deploy Mar 4, 2026
Copilot finished work on behalf of ewega March 4, 2026 12:04
@ewega ewega marked this pull request as ready for review March 4, 2026 12:10
Copilot AI review requested due to automatic review settings March 4, 2026 12:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a temp-directory disk leak in the Azure deploy flow when --repo-url is used, aligning behavior with the local deploy path by cleaning up the cloned repo directory after the command finishes.

Changes:

  • Add deferred cleanup (os.RemoveAll) for the repo root returned by findRepoRoot() when --repo-url is set.

Comment on lines 182 to +188
repoRoot, err := findRepoRoot()
if err != nil {
return err
}
if azureRepoURL != "" {
defer os.RemoveAll(repoRoot)
}
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The new deferred cleanup only runs after findRepoRoot() succeeds. If --repo-url is set and gitclone.Clone() fails inside findRepoRoot(), the temp directory created by os.MkdirTemp is still leaked because the caller never receives repoRoot. To meet the “success or failure” cleanup requirement, ensure the temp dir is removed on clone error as well (e.g., remove it before returning the error, or return a cleanup func from findRepoRoot).

Copilot uses AI. Check for mistakes.
@ewega ewega merged commit 2dc0a76 into main Mar 4, 2026
11 checks passed
@ewega ewega deleted the copilot/fix-temp-directory-leak branch March 4, 2026 13:42
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.

Fix temp directory leak in Azure fork deploy

3 participants