From 58d647dcd3eab46e842b5763fddaf8fd32d77a8e Mon Sep 17 00:00:00 2001
From: mario-valente <mario.neto@blip.ai>
Date: Tue, 28 Jul 2026 10:33:51 -0300
Subject: [PATCH] fix(skills-init): clean stale dest before cloning
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CloneGit never removed a pre-existing ref.Dest before starting. A prior
attempt killed between the clone/checkout and applySubPath's final
os.Rename leaves the full clone sitting in dest — non-empty, but not yet
reduced to the subPath. Every subsequent retry then fails git's own
pre-flight "already exists and is not an empty directory" check before
touching the network, permanently, even though the container is documented
to retry from scratch on failure.

Reproduced against a real Agent on AKS: skills-init crash-looped forever
after the first (interrupted) attempt left a full-repo clone behind.

Fix: os.RemoveAll(ref.Dest) at the top of CloneGit, before any clone
attempt, so every invocation genuinely starts from scratch as intended.
---
 .../skillsinit/clonegit_stale_dest_test.go    | 52 +++++++++++++++++++
 go/core/internal/skillsinit/git.go            | 11 ++++
 2 files changed, 63 insertions(+)
 create mode 100644 go/core/internal/skillsinit/clonegit_stale_dest_test.go

diff --git a/go/core/internal/skillsinit/clonegit_stale_dest_test.go b/go/core/internal/skillsinit/clonegit_stale_dest_test.go
new file mode 100644
index 000000000..5f25fda86
--- /dev/null
+++ b/go/core/internal/skillsinit/clonegit_stale_dest_test.go
@@ -0,0 +1,52 @@
+package skillsinit
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// Test_CloneGit_cleansStaleDest guards against a regression of the bug where
+// a prior failed attempt (e.g. killed between the clone/checkout and
+// applySubPath's final rename) leaves ref.Dest non-empty on disk. Without
+// cleanup, every subsequent retry fails git's own pre-flight
+// "already exists and is not an empty directory" check forever, even though
+// the container is documented to retry from scratch on failure.
+func Test_CloneGit_cleansStaleDest(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+
+	origin := t.TempDir()
+	runIn(t, origin, "init", "--initial-branch=main")
+	runIn(t, origin, "config", "user.email", "test@example.com")
+	runIn(t, origin, "config", "user.name", "test")
+	require.NoError(t, os.WriteFile(filepath.Join(origin, "README.md"), []byte("hi"), 0o644))
+	runIn(t, origin, "add", "README.md")
+	runIn(t, origin, "commit", "-m", "init")
+
+	dest := filepath.Join(t.TempDir(), "dest")
+	// Simulate the leftover from an interrupted prior attempt: a non-empty
+	// dest that was never cleaned up.
+	require.NoError(t, os.MkdirAll(dest, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dest, "stale.txt"), []byte("leftover"), 0o644))
+
+	err := CloneGit(GitRef{URL: origin, Ref: "main", Dest: dest})
+	require.NoError(t, err, "CloneGit must clean a stale dest and retry successfully")
+
+	_, err = os.Stat(filepath.Join(dest, "README.md"))
+	require.NoError(t, err, "dest should contain the freshly cloned content")
+	_, err = os.Stat(filepath.Join(dest, "stale.txt"))
+	require.True(t, os.IsNotExist(err), "stale leftover content must not survive the clone")
+}
+
+func runIn(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	cmd := exec.Command("git", args...)
+	cmd.Dir = dir
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err, "git %v: %s", args, out)
+}
diff --git a/go/core/internal/skillsinit/git.go b/go/core/internal/skillsinit/git.go
index 519a4d90d..28749b1a8 100644
--- a/go/core/internal/skillsinit/git.go
+++ b/go/core/internal/skillsinit/git.go
@@ -18,7 +18,18 @@ import (
 //
 // SubPath, if set, rewrites the destination so the final layout matches the
 // requested in-repo subdirectory.
+//
+// ref.Dest is removed before cloning: a prior failed attempt (e.g. killed
+// between the clone/checkout and applySubPath's final rename) can leave a
+// non-empty dest behind, which would otherwise make every subsequent retry
+// fail git's own "already exists and is not an empty directory" pre-flight
+// check forever, even though the container is expected to retry from
+// scratch on failure.
 func CloneGit(ref GitRef) error {
+	if err := os.RemoveAll(ref.Dest); err != nil {
+		return fmt.Errorf("clean stale dest %q: %w", ref.Dest, err)
+	}
+
 	if ref.Full {
 		if err := runGit("clone", "--", ref.URL, ref.Dest); err != nil {
 			return err