Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go/internal/fast/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/Agent-Field/SWE-AF/go/internal/config"
"github.com/Agent-Field/SWE-AF/go/internal/harnessx"
"github.com/Agent-Field/SWE-AF/go/internal/schemas"
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
)

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -169,7 +170,7 @@ func Build(ctx context.Context, deps *Deps, input map[string]any) (any, error) {
repoPath := in.RepoPath
// Auto-derive repo_path from repo_url when not specified.
if effectiveRepoURL != "" && repoPath == "" {
repoPath = "/workspaces/" + repoNameFromURL(effectiveRepoURL)
repoPath = filepath.Join(workspace.Root(), repoNameFromURL(effectiveRepoURL))
}
if repoPath == "" {
return nil, errors.New("Either repo_path or repo_url must be provided")
Expand Down
14 changes: 10 additions & 4 deletions go/internal/orch/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/Agent-Field/SWE-AF/go/internal/envelope"
"github.com/Agent-Field/SWE-AF/go/internal/hitl"
"github.com/Agent-Field/SWE-AF/go/internal/schemas"
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
)

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

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

if repoPath == "" {
Expand Down Expand Up @@ -491,7 +492,11 @@ func prepareSingleRepo(ctx context.Context, deps *Deps, cfg *config.BuildConfig,
switch {
case cfg.RepoURL != "" && !pathExists(gitDir):
deps.Note(ctx, fmt.Sprintf("Cloning %s → %s", cfg.RepoURL, repoPath), "build", "clone")
_ = os.MkdirAll(repoPath, 0o755)
// Create only the parent; git clone creates the leaf itself. Pre-creating
// the destination leaf makes git refuse it as "already exists and is not
// an empty directory" on Windows, where it cannot re-open the dir the node
// just made (issue #107).
_ = os.MkdirAll(filepath.Dir(repoPath), 0o755)
r := runGit(ctx, "", "clone", cfg.RepoURL, repoPath)
if r.ExitCode != 0 {
errMsg := strings.TrimSpace(r.Stderr)
Expand Down Expand Up @@ -524,7 +529,8 @@ func prepareSingleRepo(ctx context.Context, deps *Deps, cfg *config.BuildConfig,
deps.Note(ctx, fmt.Sprintf("Reset to origin/%s failed — re-cloning", defaultBranch),
"build", "clone", "reclone")
_ = os.RemoveAll(repoPath)
_ = os.MkdirAll(repoPath, 0o755)
// Parent-only: git clone re-creates the leaf (issue #107).
_ = os.MkdirAll(filepath.Dir(repoPath), 0o755)
clone := runGit(ctx, "", "clone", cfg.RepoURL, repoPath)
if clone.ExitCode != 0 {
return fmt.Errorf("git re-clone failed: %s", strings.TrimSpace(clone.Stderr))
Expand Down
7 changes: 5 additions & 2 deletions go/internal/orch/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import (
"context"
"errors"
"fmt"
"path/filepath"
"strings"
"sync"
"testing"

"github.com/Agent-Field/agentfield/sdk/go/agent"

"github.com/Agent-Field/SWE-AF/go/internal/workspace"
)

// buildHandler routes mock reasoner responses by target suffix. Overridable
Expand Down Expand Up @@ -203,8 +206,8 @@ func TestBuildIsolationConcurrent(t *testing.T) {
func TestBuildScopedPathIncludesBuildID(t *testing.T) {
repoURL := "https://github.com/example/my-repo.git"
name := deriveRepoName(repoURL)
a := fmt.Sprintf("/workspaces/%s-%s", name, newBuildID())
b := fmt.Sprintf("/workspaces/%s-%s", name, newBuildID())
a := filepath.Join(workspace.Root(), fmt.Sprintf("%s-%s", name, newBuildID()))
b := filepath.Join(workspace.Root(), fmt.Sprintf("%s-%s", name, newBuildID()))
if a == b {
t.Fatal("two builds for the same repo must produce different workspace paths")
}
Expand Down
9 changes: 7 additions & 2 deletions go/internal/orch/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"errors"
"fmt"
"os"
"path/filepath"
"strings"
"time"

"github.com/Agent-Field/SWE-AF/go/internal/config"
"github.com/Agent-Field/SWE-AF/go/internal/workspace"
)

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

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

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

// ---- 1. Clone ----------------------------------------------------------
_ = os.MkdirAll(repoPath, 0o755)
// Create only the parent; git clone creates the leaf. Pre-creating the leaf
// makes git refuse it as "already exists and is not an empty directory" on
// Windows, where it cannot re-open the node-created dir (issue #107).
_ = os.MkdirAll(filepath.Dir(repoPath), 0o755)
clone := runGit(ctx, "", "clone", in.RepoURL, repoPath)
if clone.ExitCode != 0 {
errMsg := strings.TrimSpace(clone.Stderr)
Expand Down
59 changes: 59 additions & 0 deletions go/internal/workspace/workspace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Package workspace resolves the base directory into which SWE-AF clones build
// repositories when the caller supplies only a repo_url (no explicit
// repo_path). It exists so that default never resolves to a drive-relative
// path on Windows.
//
// A hardcoded "/workspaces" base is drive-relative on Windows: it carries no
// drive letter, so the Windows Go node's spawn context resolves it
// unpredictably. os.MkdirAll appears to succeed, but the following `git clone`
// fails with "destination path ... already exists and is not an empty
// directory" — git's is_empty_dir reports "not empty" when it cannot open the
// directory it was handed from that context. Rooting the default under an
// absolute drive-letter base (%LOCALAPPDATA%) removes the ambiguity while
// keeping byte-identical "/workspaces" behavior on every other platform
// (Docker parity).
//
// Ref: https://github.com/Agent-Field/SWE-AF/issues/107
package workspace

import (
"os"
"path/filepath"
"runtime"
)

// tempDir is indirected so tests can drive the LOCALAPPDATA-empty Windows
// fallback deterministically. Production always uses os.TempDir.
var tempDir = os.TempDir

// Root returns the base directory under which build repositories are cloned.
// See rootFor for the resolution rules.
func Root() string {
return rootFor(runtime.GOOS, os.Getenv)
}

// rootFor is the testable core of Root. Resolution order:
//
// 1. SWE_WORKSPACE_ROOT, when non-empty, on every platform — the explicit
// operator override.
// 2. On Windows: %LOCALAPPDATA%\agentfield\workspaces. When LOCALAPPDATA is
// empty, fall back to <os.TempDir()>\agentfield\workspaces so the base is
// still an absolute drive-letter path (never drive-relative "/workspaces").
// 3. Everywhere else: exactly "/workspaces" — byte-identical to the historical
// default (Docker parity).
//
// goos and getenv are injected so tests can exercise every branch without
// touching the real runtime environment.
func rootFor(goos string, getenv func(string) string) string {
if root := getenv("SWE_WORKSPACE_ROOT"); root != "" {
return root
}
if goos == "windows" {
base := getenv("LOCALAPPDATA")
if base == "" {
base = tempDir()
}
return filepath.Join(base, "agentfield", "workspaces")
}
return "/workspaces"
}
72 changes: 72 additions & 0 deletions go/internal/workspace/workspace_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package workspace

import (
"path/filepath"
"testing"
)

// fakeEnv builds a getenv closure from a fixed map so each table case drives an
// isolated environment without mutating the real process env.
func fakeEnv(vars map[string]string) func(string) string {
return func(k string) string { return vars[k] }
}

// TestRootFor covers the full resolution matrix from issue #107: the override
// wins everywhere, Windows never yields a drive-relative "/workspaces", and
// non-Windows stays byte-identical to the historical default.
func TestRootFor(t *testing.T) {
const fakeTemp = `C:\FakeTemp`
orig := tempDir
tempDir = func() string { return fakeTemp }
t.Cleanup(func() { tempDir = orig })

cases := []struct {
name string
goos string
env map[string]string
want string
}{
{
name: "windows override wins over LOCALAPPDATA",
goos: "windows",
env: map[string]string{
"SWE_WORKSPACE_ROOT": `E:\builds`,
"LOCALAPPDATA": `C:\Users\me\AppData\Local`,
},
want: `E:\builds`,
},
{
name: "windows LOCALAPPDATA default",
goos: "windows",
env: map[string]string{"LOCALAPPDATA": `C:\Users\me\AppData\Local`},
want: filepath.Join(`C:\Users\me\AppData\Local`, "agentfield", "workspaces"),
},
{
name: "windows neither falls back to tempdir",
goos: "windows",
env: map[string]string{},
want: filepath.Join(fakeTemp, "agentfield", "workspaces"),
},
{
name: "linux default is exactly /workspaces",
goos: "linux",
env: map[string]string{},
want: "/workspaces",
},
{
name: "linux override wins",
goos: "linux",
env: map[string]string{"SWE_WORKSPACE_ROOT": "/mnt/scratch/ws"},
want: "/mnt/scratch/ws",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := rootFor(tc.goos, fakeEnv(tc.env))
if got != tc.want {
t.Fatalf("rootFor(%q, ...) = %q, want %q", tc.goos, got, tc.want)
}
})
}
}
29 changes: 21 additions & 8 deletions swe_af/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def __init__(self, message: str, *, result=None, error_details=None) -> None:
_default_planning_model,
_default_runtime,
_derive_repo_name as _repo_name_from_url,
_workspace_root,
)

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

git_dir = os.path.join(dest, ".git")
if spec.repo_url and not os.path.exists(git_dir):
os.makedirs(dest, exist_ok=True)
# Parent-only (workspace_root already exists): git clone creates the
# leaf. Pre-creating it makes git refuse it as "already exists and is
# not an empty directory" on Windows (issue #107).
os.makedirs(os.path.dirname(dest) or ".", exist_ok=True)
cmd = ["git", "clone", spec.repo_url, dest]
if spec.branch:
cmd += ["--branch", spec.branch]
Expand Down Expand Up @@ -516,7 +520,9 @@ async def build(
This is the single entry point. Pass a goal, get working code.

If ``repo_url`` is provided and ``repo_path`` is empty, the repo is cloned
into ``/workspaces/<repo-name>`` automatically (useful in Docker).
into a build-scoped directory under the workspace root automatically
(``/workspaces`` in Docker, ``%LOCALAPPDATA%\\agentfield\\workspaces`` on
Windows, or ``$SWE_WORKSPACE_ROOT`` when set — see ``_workspace_root``).
"""
cfg = BuildConfig(**config) if config else BuildConfig()

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

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

if not repo_path:
raise ValueError("Either repo_path or repo_url must be provided")
Expand All @@ -560,7 +566,10 @@ async def build(
git_dir = os.path.join(repo_path, ".git")
if cfg.repo_url and not os.path.exists(git_dir):
app.note(f"Cloning {cfg.repo_url} → {repo_path}", tags=["build", "clone"])
os.makedirs(repo_path, exist_ok=True)
# Create only the parent; git clone creates the leaf itself.
# Pre-creating the leaf makes git refuse it as "already exists and is
# not an empty directory" on Windows (issue #107).
os.makedirs(os.path.dirname(repo_path) or ".", exist_ok=True)
clone_result = subprocess.run(
["git", "clone", cfg.repo_url, repo_path],
capture_output=True,
Expand Down Expand Up @@ -614,7 +623,8 @@ async def build(
)
import shutil
shutil.rmtree(repo_path, ignore_errors=True)
os.makedirs(repo_path, exist_ok=True)
# Parent-only: git clone re-creates the leaf (issue #107).
os.makedirs(os.path.dirname(repo_path) or ".", exist_ok=True)
clone_result = subprocess.run(
["git", "clone", cfg.repo_url, repo_path],
capture_output=True, text=True,
Expand Down Expand Up @@ -1748,15 +1758,18 @@ async def resolve(

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

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

# ---- 1. Clone -----------------------------------------------------------
os.makedirs(repo_path, exist_ok=True)
# Parent-only: git clone creates the leaf itself; pre-creating it makes git
# refuse it as "already exists and is not an empty directory" on Windows
# (issue #107).
os.makedirs(os.path.dirname(repo_path) or ".", exist_ok=True)
clone = subprocess.run(
["git", "clone", repo_url, repo_path],
capture_output=True, text=True,
Expand Down
29 changes: 29 additions & 0 deletions swe_af/execution/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import logging
import os
import re
import tempfile
from enum import Enum
from typing import Any, Literal

Expand Down Expand Up @@ -73,6 +74,34 @@ def _derive_repo_name(url: str) -> str:
return name


def _workspace_root() -> str:
"""Base directory into which builds clone repositories by default.

Resolution order:
1. ``SWE_WORKSPACE_ROOT`` env var, when set, on every platform.
2. On Windows (``os.name == "nt"``):
``%LOCALAPPDATA%\\agentfield\\workspaces`` — an absolute drive-letter
path. Falls back to ``<tempdir>\\agentfield\\workspaces`` when
LOCALAPPDATA is unset.
3. Everywhere else: exactly ``/workspaces`` (Docker parity).

A hardcoded ``/workspaces`` base is *drive-relative* on Windows (no drive
letter), which the node's spawn context resolves unpredictably: makedirs
appears to succeed but ``git clone`` then fails with "destination path ...
already exists and is not an empty directory". Rooting the default under an
absolute base avoids that.

Ref: https://github.com/Agent-Field/SWE-AF/issues/107
"""
root = os.environ.get("SWE_WORKSPACE_ROOT")
if root:
return root
if os.name == "nt":
base = os.environ.get("LOCALAPPDATA") or tempfile.gettempdir()
return os.path.join(base, "agentfield", "workspaces")
return "/workspaces"


# ---------------------------------------------------------------------------
# Multi-repo models
# ---------------------------------------------------------------------------
Expand Down
Loading
Loading