Skip to content

Commit 6dfc4e5

Browse files
AbirAbbasclaude
andauthored
fix(go/windows): root the default clone workspace under an absolute base (#110)
* fix(go): root the default workspace under an absolute base on Windows (#107) A build given only repo_url derived its clone target under a hardcoded "/workspaces" base. On the Windows Go node that is a drive-relative path (no drive letter), which the node's spawn context resolves unpredictably: MkdirAll appears to succeed but `git clone` then fails with "destination path ... already exists and is not an empty directory". Add internal/workspace.Root(): SWE_WORKSPACE_ROOT override on every platform, else %LOCALAPPDATA%\agentfield\workspaces on Windows (temp-dir fallback when LOCALAPPDATA is empty), else exactly "/workspaces" for Docker parity. Wire it into the three default-derivation sites (orch build single/multi-repo, orch resolve, fast build) via filepath.Join so the name/suffix patterns stay byte-identical. Ref: #107 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(go): create only the parent dir before git clone (#107) git clone refuses a destination it did not create with "already exists and is not an empty directory" when it cannot re-open a node-created leaf dir from the Windows spawn context. Pre-creating the clone target's leaf therefore reintroduces the very failure issue #107 is about. In the orch build and resolve clone paths (including the reset-failed re-clone), create only filepath.Dir(repoPath) and let git clone create the leaf. The fresh-init default case and the fast node keep creating the leaf — they git-init in place rather than cloning. Ref: #107 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(py): mirror the Windows workspace-root fix on the Python node (#107) The Python node runs on Windows too and derived the same drive-relative "/workspaces/..." defaults. Add _workspace_root() in execution/schemas.py (SWE_WORKSPACE_ROOT override; else %LOCALAPPDATA%\agentfield\workspaces on nt with a temp-dir fallback; else "/workspaces") and use it at every default-derivation site: build single/multi-repo, resolve, and fast build. Also stop pre-creating the clone destination leaf before git clone in the single-repo, re-clone, resolve, and multi-repo clone_repos paths — create only the parent so git clone owns the leaf, matching the Go node. Update the build-isolation source assertions to the os.path.join form and add _workspace_root unit tests (override, nt LOCALAPPDATA/tempdir, posix). Ref: #107 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent e89c3d9 commit 6dfc4e5

10 files changed

Lines changed: 274 additions & 20 deletions

File tree

go/internal/fast/build.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/Agent-Field/SWE-AF/go/internal/config"
2626
"github.com/Agent-Field/SWE-AF/go/internal/harnessx"
2727
"github.com/Agent-Field/SWE-AF/go/internal/schemas"
28+
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
2829
)
2930

3031
// ---------------------------------------------------------------------------
@@ -169,7 +170,7 @@ func Build(ctx context.Context, deps *Deps, input map[string]any) (any, error) {
169170
repoPath := in.RepoPath
170171
// Auto-derive repo_path from repo_url when not specified.
171172
if effectiveRepoURL != "" && repoPath == "" {
172-
repoPath = "/workspaces/" + repoNameFromURL(effectiveRepoURL)
173+
repoPath = filepath.Join(workspace.Root(), repoNameFromURL(effectiveRepoURL))
173174
}
174175
if repoPath == "" {
175176
return nil, errors.New("Either repo_path or repo_url must be provided")

go/internal/orch/build.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/Agent-Field/SWE-AF/go/internal/envelope"
2020
"github.com/Agent-Field/SWE-AF/go/internal/hitl"
2121
"github.com/Agent-Field/SWE-AF/go/internal/schemas"
22+
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
2223
)
2324

2425
// Handlers is the name→handler registration surface consumed by node wiring.
@@ -73,7 +74,7 @@ func Build(ctx context.Context, deps *Deps, input map[string]any) (any, error) {
7374
// Auto-derive repo_path from repo_url, build-scoped.
7475
if cfg.RepoURL != "" && repoPath == "" {
7576
repoName := deriveRepoName(cfg.RepoURL)
76-
repoPath = fmt.Sprintf("/workspaces/%s-%s", repoName, buildID)
77+
repoPath = filepath.Join(workspace.Root(), fmt.Sprintf("%s-%s", repoName, buildID))
7778
}
7879

7980
// Multi-repo: derive repo_path from the primary repo.
@@ -83,7 +84,7 @@ func Build(ctx context.Context, deps *Deps, input map[string]any) (any, error) {
8384
primary = &cfg.Repos[0]
8485
}
8586
repoName := deriveRepoName(primary.RepoURL)
86-
repoPath = fmt.Sprintf("/workspaces/%s-%s", repoName, buildID)
87+
repoPath = filepath.Join(workspace.Root(), fmt.Sprintf("%s-%s", repoName, buildID))
8788
}
8889

8990
if repoPath == "" {
@@ -491,7 +492,11 @@ func prepareSingleRepo(ctx context.Context, deps *Deps, cfg *config.BuildConfig,
491492
switch {
492493
case cfg.RepoURL != "" && !pathExists(gitDir):
493494
deps.Note(ctx, fmt.Sprintf("Cloning %s → %s", cfg.RepoURL, repoPath), "build", "clone")
494-
_ = os.MkdirAll(repoPath, 0o755)
495+
// Create only the parent; git clone creates the leaf itself. Pre-creating
496+
// the destination leaf makes git refuse it as "already exists and is not
497+
// an empty directory" on Windows, where it cannot re-open the dir the node
498+
// just made (issue #107).
499+
_ = os.MkdirAll(filepath.Dir(repoPath), 0o755)
495500
r := runGit(ctx, "", "clone", cfg.RepoURL, repoPath)
496501
if r.ExitCode != 0 {
497502
errMsg := strings.TrimSpace(r.Stderr)
@@ -524,7 +529,8 @@ func prepareSingleRepo(ctx context.Context, deps *Deps, cfg *config.BuildConfig,
524529
deps.Note(ctx, fmt.Sprintf("Reset to origin/%s failed — re-cloning", defaultBranch),
525530
"build", "clone", "reclone")
526531
_ = os.RemoveAll(repoPath)
527-
_ = os.MkdirAll(repoPath, 0o755)
532+
// Parent-only: git clone re-creates the leaf (issue #107).
533+
_ = os.MkdirAll(filepath.Dir(repoPath), 0o755)
528534
clone := runGit(ctx, "", "clone", cfg.RepoURL, repoPath)
529535
if clone.ExitCode != 0 {
530536
return fmt.Errorf("git re-clone failed: %s", strings.TrimSpace(clone.Stderr))

go/internal/orch/build_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"path/filepath"
78
"strings"
89
"sync"
910
"testing"
1011

1112
"github.com/Agent-Field/agentfield/sdk/go/agent"
13+
14+
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
1215
)
1316

1417
// buildHandler routes mock reasoner responses by target suffix. Overridable
@@ -203,8 +206,8 @@ func TestBuildIsolationConcurrent(t *testing.T) {
203206
func TestBuildScopedPathIncludesBuildID(t *testing.T) {
204207
repoURL := "https://github.com/example/my-repo.git"
205208
name := deriveRepoName(repoURL)
206-
a := fmt.Sprintf("/workspaces/%s-%s", name, newBuildID())
207-
b := fmt.Sprintf("/workspaces/%s-%s", name, newBuildID())
209+
a := filepath.Join(workspace.Root(), fmt.Sprintf("%s-%s", name, newBuildID()))
210+
b := filepath.Join(workspace.Root(), fmt.Sprintf("%s-%s", name, newBuildID()))
208211
if a == b {
209212
t.Fatal("two builds for the same repo must produce different workspace paths")
210213
}

go/internal/orch/resolve.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import (
66
"errors"
77
"fmt"
88
"os"
9+
"path/filepath"
910
"strings"
1011
"time"
1112

1213
"github.com/Agent-Field/SWE-AF/go/internal/config"
14+
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
1315
)
1416

1517
// resolveInput mirrors the Python resolve() signature (param names + defaults).
@@ -58,13 +60,16 @@ func ResolveHandler(ctx context.Context, deps *Deps, input map[string]any) (any,
5860

5961
buildID := newBuildID()
6062
repoName := deriveRepoName(in.RepoURL)
61-
repoPath := fmt.Sprintf("/workspaces/%s-resolve-%s", repoName, buildID)
63+
repoPath := filepath.Join(workspace.Root(), fmt.Sprintf("%s-resolve-%s", repoName, buildID))
6264

6365
deps.Note(ctx, fmt.Sprintf("Resolve starting (build_id=%s) — PR #%d", buildID, in.PRNumber),
6466
"resolve", "start")
6567

6668
// ---- 1. Clone ----------------------------------------------------------
67-
_ = os.MkdirAll(repoPath, 0o755)
69+
// Create only the parent; git clone creates the leaf. Pre-creating the leaf
70+
// makes git refuse it as "already exists and is not an empty directory" on
71+
// Windows, where it cannot re-open the node-created dir (issue #107).
72+
_ = os.MkdirAll(filepath.Dir(repoPath), 0o755)
6873
clone := runGit(ctx, "", "clone", in.RepoURL, repoPath)
6974
if clone.ExitCode != 0 {
7075
errMsg := strings.TrimSpace(clone.Stderr)

go/internal/workspace/workspace.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Package workspace resolves the base directory into which SWE-AF clones build
2+
// repositories when the caller supplies only a repo_url (no explicit
3+
// repo_path). It exists so that default never resolves to a drive-relative
4+
// path on Windows.
5+
//
6+
// A hardcoded "/workspaces" base is drive-relative on Windows: it carries no
7+
// drive letter, so the Windows Go node's spawn context resolves it
8+
// unpredictably. os.MkdirAll appears to succeed, but the following `git clone`
9+
// fails with "destination path ... already exists and is not an empty
10+
// directory" — git's is_empty_dir reports "not empty" when it cannot open the
11+
// directory it was handed from that context. Rooting the default under an
12+
// absolute drive-letter base (%LOCALAPPDATA%) removes the ambiguity while
13+
// keeping byte-identical "/workspaces" behavior on every other platform
14+
// (Docker parity).
15+
//
16+
// Ref: https://github.com/Agent-Field/SWE-AF/issues/107
17+
package workspace
18+
19+
import (
20+
"os"
21+
"path/filepath"
22+
"runtime"
23+
)
24+
25+
// tempDir is indirected so tests can drive the LOCALAPPDATA-empty Windows
26+
// fallback deterministically. Production always uses os.TempDir.
27+
var tempDir = os.TempDir
28+
29+
// Root returns the base directory under which build repositories are cloned.
30+
// See rootFor for the resolution rules.
31+
func Root() string {
32+
return rootFor(runtime.GOOS, os.Getenv)
33+
}
34+
35+
// rootFor is the testable core of Root. Resolution order:
36+
//
37+
// 1. SWE_WORKSPACE_ROOT, when non-empty, on every platform — the explicit
38+
// operator override.
39+
// 2. On Windows: %LOCALAPPDATA%\agentfield\workspaces. When LOCALAPPDATA is
40+
// empty, fall back to <os.TempDir()>\agentfield\workspaces so the base is
41+
// still an absolute drive-letter path (never drive-relative "/workspaces").
42+
// 3. Everywhere else: exactly "/workspaces" — byte-identical to the historical
43+
// default (Docker parity).
44+
//
45+
// goos and getenv are injected so tests can exercise every branch without
46+
// touching the real runtime environment.
47+
func rootFor(goos string, getenv func(string) string) string {
48+
if root := getenv("SWE_WORKSPACE_ROOT"); root != "" {
49+
return root
50+
}
51+
if goos == "windows" {
52+
base := getenv("LOCALAPPDATA")
53+
if base == "" {
54+
base = tempDir()
55+
}
56+
return filepath.Join(base, "agentfield", "workspaces")
57+
}
58+
return "/workspaces"
59+
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package workspace
2+
3+
import (
4+
"path/filepath"
5+
"testing"
6+
)
7+
8+
// fakeEnv builds a getenv closure from a fixed map so each table case drives an
9+
// isolated environment without mutating the real process env.
10+
func fakeEnv(vars map[string]string) func(string) string {
11+
return func(k string) string { return vars[k] }
12+
}
13+
14+
// TestRootFor covers the full resolution matrix from issue #107: the override
15+
// wins everywhere, Windows never yields a drive-relative "/workspaces", and
16+
// non-Windows stays byte-identical to the historical default.
17+
func TestRootFor(t *testing.T) {
18+
const fakeTemp = `C:\FakeTemp`
19+
orig := tempDir
20+
tempDir = func() string { return fakeTemp }
21+
t.Cleanup(func() { tempDir = orig })
22+
23+
cases := []struct {
24+
name string
25+
goos string
26+
env map[string]string
27+
want string
28+
}{
29+
{
30+
name: "windows override wins over LOCALAPPDATA",
31+
goos: "windows",
32+
env: map[string]string{
33+
"SWE_WORKSPACE_ROOT": `E:\builds`,
34+
"LOCALAPPDATA": `C:\Users\me\AppData\Local`,
35+
},
36+
want: `E:\builds`,
37+
},
38+
{
39+
name: "windows LOCALAPPDATA default",
40+
goos: "windows",
41+
env: map[string]string{"LOCALAPPDATA": `C:\Users\me\AppData\Local`},
42+
want: filepath.Join(`C:\Users\me\AppData\Local`, "agentfield", "workspaces"),
43+
},
44+
{
45+
name: "windows neither falls back to tempdir",
46+
goos: "windows",
47+
env: map[string]string{},
48+
want: filepath.Join(fakeTemp, "agentfield", "workspaces"),
49+
},
50+
{
51+
name: "linux default is exactly /workspaces",
52+
goos: "linux",
53+
env: map[string]string{},
54+
want: "/workspaces",
55+
},
56+
{
57+
name: "linux override wins",
58+
goos: "linux",
59+
env: map[string]string{"SWE_WORKSPACE_ROOT": "/mnt/scratch/ws"},
60+
want: "/mnt/scratch/ws",
61+
},
62+
}
63+
64+
for _, tc := range cases {
65+
t.Run(tc.name, func(t *testing.T) {
66+
got := rootFor(tc.goos, fakeEnv(tc.env))
67+
if got != tc.want {
68+
t.Fatalf("rootFor(%q, ...) = %q, want %q", tc.goos, got, tc.want)
69+
}
70+
})
71+
}
72+
}

swe_af/app.py

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ def __init__(self, message: str, *, result=None, error_details=None) -> None:
4747
_default_planning_model,
4848
_default_runtime,
4949
_derive_repo_name as _repo_name_from_url,
50+
_workspace_root,
5051
)
5152

5253
NODE_ID = os.getenv("NODE_ID", "swe-planner")
@@ -141,7 +142,10 @@ async def _clone_single(spec: WorkspaceRepo) -> tuple[str, str]: # type: ignore
141142

142143
git_dir = os.path.join(dest, ".git")
143144
if spec.repo_url and not os.path.exists(git_dir):
144-
os.makedirs(dest, exist_ok=True)
145+
# Parent-only (workspace_root already exists): git clone creates the
146+
# leaf. Pre-creating it makes git refuse it as "already exists and is
147+
# not an empty directory" on Windows (issue #107).
148+
os.makedirs(os.path.dirname(dest) or ".", exist_ok=True)
145149
cmd = ["git", "clone", spec.repo_url, dest]
146150
if spec.branch:
147151
cmd += ["--branch", spec.branch]
@@ -516,7 +520,9 @@ async def build(
516520
This is the single entry point. Pass a goal, get working code.
517521
518522
If ``repo_url`` is provided and ``repo_path`` is empty, the repo is cloned
519-
into ``/workspaces/<repo-name>`` automatically (useful in Docker).
523+
into a build-scoped directory under the workspace root automatically
524+
(``/workspaces`` in Docker, ``%LOCALAPPDATA%\\agentfield\\workspaces`` on
525+
Windows, or ``$SWE_WORKSPACE_ROOT`` when set — see ``_workspace_root``).
520526
"""
521527
cfg = BuildConfig(**config) if config else BuildConfig()
522528

@@ -535,13 +541,13 @@ async def build(
535541
# concurrent builds from sharing git state, artifacts, or worktrees.
536542
if cfg.repo_url and not repo_path:
537543
repo_name = _repo_name_from_url(cfg.repo_url)
538-
repo_path = f"/workspaces/{repo_name}-{build_id}"
544+
repo_path = os.path.join(_workspace_root(), f"{repo_name}-{build_id}")
539545

540546
# Multi-repo: derive repo_path from primary repo; _clone_repos handles cloning later
541547
if not repo_path and len(cfg.repos) > 1:
542548
primary = next((r for r in cfg.repos if r.role == "primary"), cfg.repos[0])
543549
repo_name = _repo_name_from_url(primary.repo_url)
544-
repo_path = f"/workspaces/{repo_name}-{build_id}"
550+
repo_path = os.path.join(_workspace_root(), f"{repo_name}-{build_id}")
545551

546552
if not repo_path:
547553
raise ValueError("Either repo_path or repo_url must be provided")
@@ -560,7 +566,10 @@ async def build(
560566
git_dir = os.path.join(repo_path, ".git")
561567
if cfg.repo_url and not os.path.exists(git_dir):
562568
app.note(f"Cloning {cfg.repo_url}{repo_path}", tags=["build", "clone"])
563-
os.makedirs(repo_path, exist_ok=True)
569+
# Create only the parent; git clone creates the leaf itself.
570+
# Pre-creating the leaf makes git refuse it as "already exists and is
571+
# not an empty directory" on Windows (issue #107).
572+
os.makedirs(os.path.dirname(repo_path) or ".", exist_ok=True)
564573
clone_result = subprocess.run(
565574
["git", "clone", cfg.repo_url, repo_path],
566575
capture_output=True,
@@ -614,7 +623,8 @@ async def build(
614623
)
615624
import shutil
616625
shutil.rmtree(repo_path, ignore_errors=True)
617-
os.makedirs(repo_path, exist_ok=True)
626+
# Parent-only: git clone re-creates the leaf (issue #107).
627+
os.makedirs(os.path.dirname(repo_path) or ".", exist_ok=True)
618628
clone_result = subprocess.run(
619629
["git", "clone", cfg.repo_url, repo_path],
620630
capture_output=True, text=True,
@@ -1748,15 +1758,18 @@ async def resolve(
17481758

17491759
build_id = uuid.uuid4().hex[:8]
17501760
repo_name = _repo_name_from_url(repo_url)
1751-
repo_path = f"/workspaces/{repo_name}-resolve-{build_id}"
1761+
repo_path = os.path.join(_workspace_root(), f"{repo_name}-resolve-{build_id}")
17521762

17531763
app.note(
17541764
f"Resolve starting (build_id={build_id}) — PR #{pr_number}",
17551765
tags=["resolve", "start"],
17561766
)
17571767

17581768
# ---- 1. Clone -----------------------------------------------------------
1759-
os.makedirs(repo_path, exist_ok=True)
1769+
# Parent-only: git clone creates the leaf itself; pre-creating it makes git
1770+
# refuse it as "already exists and is not an empty directory" on Windows
1771+
# (issue #107).
1772+
os.makedirs(os.path.dirname(repo_path) or ".", exist_ok=True)
17601773
clone = subprocess.run(
17611774
["git", "clone", repo_url, repo_path],
17621775
capture_output=True, text=True,

swe_af/execution/schemas.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import logging
66
import os
77
import re
8+
import tempfile
89
from enum import Enum
910
from typing import Any, Literal
1011

@@ -73,6 +74,34 @@ def _derive_repo_name(url: str) -> str:
7374
return name
7475

7576

77+
def _workspace_root() -> str:
78+
"""Base directory into which builds clone repositories by default.
79+
80+
Resolution order:
81+
1. ``SWE_WORKSPACE_ROOT`` env var, when set, on every platform.
82+
2. On Windows (``os.name == "nt"``):
83+
``%LOCALAPPDATA%\\agentfield\\workspaces`` — an absolute drive-letter
84+
path. Falls back to ``<tempdir>\\agentfield\\workspaces`` when
85+
LOCALAPPDATA is unset.
86+
3. Everywhere else: exactly ``/workspaces`` (Docker parity).
87+
88+
A hardcoded ``/workspaces`` base is *drive-relative* on Windows (no drive
89+
letter), which the node's spawn context resolves unpredictably: makedirs
90+
appears to succeed but ``git clone`` then fails with "destination path ...
91+
already exists and is not an empty directory". Rooting the default under an
92+
absolute base avoids that.
93+
94+
Ref: https://github.com/Agent-Field/SWE-AF/issues/107
95+
"""
96+
root = os.environ.get("SWE_WORKSPACE_ROOT")
97+
if root:
98+
return root
99+
if os.name == "nt":
100+
base = os.environ.get("LOCALAPPDATA") or tempfile.gettempdir()
101+
return os.path.join(base, "agentfield", "workspaces")
102+
return "/workspaces"
103+
104+
76105
# ---------------------------------------------------------------------------
77106
# Multi-repo models
78107
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)