diff --git a/.claude/skills/delegate-issue/SKILL.md b/.claude/skills/delegate-issue/SKILL.md new file mode 100644 index 0000000..a275d28 --- /dev/null +++ b/.claude/skills/delegate-issue/SKILL.md @@ -0,0 +1,116 @@ +--- +name: delegate-issue +description: Delegate well-scoped coding issues to a SWE-AF node as a cheap sub-harness via implement_issue, then collect and merge the returned branches. Use when the user asks to offload, delegate, or fan out scoped tasks to SWE-AF / swe-planner / swe-fast, or when several independent, fully-specified changes could run in parallel without burning main-harness tokens. +--- + +# Delegate scoped issues to SWE-AF + +You are the main harness. SWE-AF is the sub-harness: it implements one +fully-scoped issue per call on an isolated branch, using cheap models, and +hands the branch back. You keep planning, merging, review, and CI. + +## When to delegate (and when not to) + +Delegate an issue only when ALL of these hold: + +- The change is **fully specified**: you can name the files, the behavior, and + the acceptance criteria without the sub-harness needing to plan anything. +- It is **independent** of your other in-flight edits (no shared files with + another delegation or with your own uncommitted work). +- The repo is reachable by the SWE-AF node (same machine or shared volume) and + has at least one commit. + +Do NOT delegate: vague feature requests (use `swe-planner.build` instead), +changes to files you are editing yourself right now, or anything whose spec +you'd have to guess. Garbage spec in → garbage branch out. + +## Preflight (once per session) + +```bash +# Control plane up and the node registered? +curl -s ${AGENTFIELD_SERVER:-http://localhost:8080}/api/v1/nodes | grep -o '"swe-[a-z-]*"' | sort -u +``` + +Pick the node (`swe-planner`, `swe-fast`, or the `-go` twins — the +`implement_issue` surface is identical). If an `X-API-Key` is configured for +the control plane, add `-H "X-API-Key: $AGENTFIELD_API_KEY"` to every curl. + +## Compose the issue + +Write the spec from YOUR context — the whole point is that the sub-harness +skips planning. Required: `title`, `description`. Strongly recommended: +`acceptance_criteria` (verifier checks these), `files_to_modify` / +`files_to_create`, `testing_strategy`. Set `needs_deeper_qa: true` only for +risky/interface-heavy changes (doubles the review cost per iteration). + +Commit (or at least be aware of) your local state first: the issue branch is +created from the **committed** base, so your uncommitted edits are invisible +to it. + +## Fire the delegation (async, never sync) + +```bash +curl -s -X POST ${AGENTFIELD_SERVER:-http://localhost:8080}/api/v1/execute/async/swe-planner.implement_issue \ + -H "Content-Type: application/json" \ + -d @issue.json +``` + +with `issue.json` shaped like: + +```json +{ + "input": { + "issue": { + "title": "...", + "description": "...", + "acceptance_criteria": ["..."], + "files_to_modify": ["..."], + "testing_strategy": "..." + }, + "repo_path": "/abs/path/to/checkout", + "base_branch": "main", + "config": { "models": { "default": "haiku" } } + } +} +``` + +Capture `execution_id` from the 202 response. Track every in-flight +delegation in your todo list (one item per execution_id). + +**Cap fan-out at 3 concurrent delegations per repo** unless the user +explicitly asks for more — each one is a paid multi-agent run, and an +unbounded loop of delegations is a surprise bill. Never re-fire a delegation +just because it is slow; poll it. + +## Track progress + +```bash +# Terminal status + result +curl -s $SERVER/api/v1/executions/ +# Live agent notes (what the coder/reviewer are doing) +curl -s $SERVER/api/v1/executions//notes +``` + +Poll on a backoff (30–60s is plenty; a typical issue lands in 10–30 min). +Batch-check several: `POST /api/v1/executions/batch-status` with +`{"execution_ids": [...]}`. + +## Collect the result + +The execution's `result` is an IssueBuildResult: + +- `success: true` → `branch` holds the implementation + (`issue/-`). Review the diff yourself + (`git diff main...`), run YOUR test gate, then merge: + `git merge --no-ff ` (or cherry-pick), and delete the branch. +- `success: false` with a `branch` → partial work was salvaged. Triage: + read `verification` / `debt_items` / `error_message`, then either fix + forward on the branch yourself, re-delegate with a sharpened spec, or drop + the branch (`git branch -D`). +- `success: false` with `branch: ""` → nothing usable was produced; the + sub-harness already cleaned up. Sharpen the spec before retrying — do not + retry verbatim. + +After merging all accepted branches, run the project's full test suite once +yourself before reporting done. The sub-harness verifier is a per-issue +check, not your integration gate. diff --git a/.gitignore b/.gitignore index 46bd037..21e1379 100644 --- a/.gitignore +++ b/.gitignore @@ -39,8 +39,9 @@ _worktrees/ # Root runtime artifacts for this node (example artifacts are versioned separately) /.artifacts/ -# Claude Code -.claude/ +# Claude Code — local state stays ignored; versioned skills ship with the repo +.claude/* +!.claude/skills/ # Python packaging / dist dist/ diff --git a/README.md b/README.md index 39e8bee..1210ab2 100644 --- a/README.md +++ b/README.md @@ -623,6 +623,118 @@ Configuration on `BuildConfig`: | `ci_wait_seconds` | `1500` | Wall-clock cap per `gh pr checks` watch (25 min). | | `ci_poll_seconds` | `30` | Poll interval for `gh pr checks`. | +## Use SWE-AF as a Sub-Harness (Issue-Level Builds) + +The full `build` pipeline is **feature-level**: it plans, decomposes, and +verifies a whole feature, which takes hours. When the caller is itself a +coding harness — Claude Code, Codex, OpenCode — it has already done the +planning. For that case SWE-AF exposes an **issue-level** entry point, +`implement_issue`, that skips every planning agent and runs just the coding +loop on an isolated branch. Delegating well-scoped issues to SWE-AF on cheap +or open-weight models keeps the main harness's token budget for the work that +needs it. + +Rule of thumb for the two prompt shapes: + +| Prompt shape | Entry point | +| --- | --- | +| "Implement X feature" (needs decomposition) | `swe-planner.build` / `swe-fast.build` | +| "Change this code in this file, like this" (fully scoped, context supplied) | `swe-planner.implement_issue` / `swe-fast.implement_issue` | + +Each call creates its own git worktree and an `issue/-` branch +off `base_branch` (default: the current branch), implements the issue with the +coder → reviewer loop (a QA + synthesizer path when `needs_deeper_qa` is set), +optionally runs one verifier pass against the acceptance criteria, removes the +worktree, and returns the branch. The caller's checkout, current branch, and +`git status` are untouched — so a main harness can fan out several issues +against the same `repo_path` concurrently and merge the returned branches +itself. Nothing is pushed and no PR is opened unless `enable_github_pr` is set: +the caller owns merge and CI. Typical cost is 4–8 LLM calls (vs hundreds for a +feature-level build). + +```bash +# Delegate one scoped issue (async; returns an execution_id immediately) +curl -X POST http://localhost:8080/api/v1/execute/async/swe-planner.implement_issue \ + -H "Content-Type: application/json" \ + -d @- <<'JSON' +{ + "input": { + "issue": { + "title": "Add retry with exponential backoff to fetch_user", + "description": "In src/api/client.py, wrap fetch_user's HTTP call in a retry helper: 3 attempts, 0.5s base delay, doubling. Reuse the existing logger for retry warnings.", + "acceptance_criteria": [ + "fetch_user retries up to 3 times on ConnectionError", + "tests cover the retry-then-succeed path" + ], + "files_to_modify": ["src/api/client.py"], + "testing_strategy": "pytest tests/api/test_client.py" + }, + "repo_path": "/workspaces/my-project", + "base_branch": "main", + "config": { "models": { "default": "haiku" } } + }, + "webhook": { "url": "https://my-harness.example/hooks/swe-af" } +} +JSON + +# Poll instead of (or in addition to) the webhook +curl http://localhost:8080/api/v1/executions/ +# Progress notes while it runs +curl http://localhost:8080/api/v1/executions//notes +``` + +The result's `branch` field is the deliverable: + +```json +{ + "success": true, + "outcome": "completed", + "branch": "issue/a1b2c3d4-add-retry-with-exponential-backoff", + "base_branch": "main", + "commits": [""], + "files_changed": ["src/api/client.py", "tests/api/test_client.py"], + "iterations": 1, + "verification": { "passed": true, "criteria_results": ["..."] }, + "debt_items": [], + "pr_url": "" +} +``` + +`issue` fields: `title` + `description` (required), `acceptance_criteria`, +`files_to_create` / `files_to_modify`, `testing_strategy`, `needs_deeper_qa` +(routes through QA + reviewer + synthesizer), `estimated_complexity`, `name`. +`additional_context` (top-level) is appended to the description. + +`config` keys (full schema: [`swe_af/issue/schemas.py`](swe_af/issue/schemas.py)): + +| Key | Default | Description | +| --- | --- | --- | +| `runtime` / `models` | as in `build` | Same runtime + flat role map; valid role keys: `default`, `coder`, `code_reviewer`, `qa`, `qa_synthesizer`, `verifier`, `git` | +| `max_coding_iterations` | `3` | Inner-loop budget (the feature-level default is 5) | +| `verify` | `true` | One verifier pass against the acceptance criteria | +| `enable_github_pr` | `false` | Push the branch and open a PR (needs an `origin` remote) | +| `agent_timeout_seconds` | `1800` | Per-agent timeout | +| `agent_max_turns` | `50` | Tool-use turn budget per agent | +| `keep_worktree` | `false` | Leave the worktree in place for debugging | + +Notes for main-harness authors: + +- `repo_path` must be a checkout the SWE-AF node can reach (same machine, or + the shared `workspaces` volume in the Docker setup) with at least one commit. +- Uncommitted changes in the caller's tree are **not** visible to the issue + branch — it is created from the committed base state. +- A failed build with commits still returns the branch (`success: false`) so + the caller can triage; a build that produced no commits deletes its branch + and returns `branch: ""`. +- Cap your fan-out: each delegation is a paid multi-agent run. A handful of + concurrent issues per repo is the sweet spot — the node also bounds its own + concurrency. +- Available identically on `swe-fast.implement_issue` and, in the Go port, on + `swe-planner-go` / `swe-fast-go`. + +A ready-made Claude Code skill for this flow ships in +[`.claude/skills/delegate-issue/`](.claude/skills/delegate-issue/SKILL.md). + ## API Reference
@@ -634,6 +746,9 @@ Core async endpoints (returns an `execution_id` immediately): # Full build: plan -> execute -> verify POST /api/v1/execute/async/swe-planner.build +# Issue-level build (sub-harness entry): coding loop only, no planning +POST /api/v1/execute/async/swe-planner.implement_issue + # Plan only POST /api/v1/execute/async/swe-planner.plan diff --git a/go/internal/config/issueconfig.go b/go/internal/config/issueconfig.go new file mode 100644 index 0000000..1ebce6d --- /dev/null +++ b/go/internal/config/issueconfig.go @@ -0,0 +1,114 @@ +package config + +// This file ports swe_af/issue/schemas.py::IssueBuildConfig — configuration for +// a single issue-level build run (the implement_issue reasoner). Like +// FastBuildConfig it has no legacy-key scan in Python (only +// ConfigDict(extra="forbid")), plus a model-key validation restricted to the +// roles the issue-level path can invoke. + +import ( + "fmt" + "sort" + "strings" +) + +// issueValidModelKeys is {"default"} | set(ISSUE_MODEL_ROLE_KEYS): the roles the +// issue-level path can invoke ("git" covers the optional PR step). +var issueValidModelKeys = map[string]struct{}{ + "default": {}, + "coder": {}, + "code_reviewer": {}, + "qa": {}, + "qa_synthesizer": {}, + "verifier": {}, + "git": {}, +} + +// IssueBuildConfig ports issue/schemas.py::IssueBuildConfig. +type IssueBuildConfig struct { + Runtime string `json:"runtime"` + Models map[string]string `json:"models"` + + MaxCodingIterations int `json:"max_coding_iterations"` + AgentMaxTurns int `json:"agent_max_turns"` + AgentTimeoutSeconds int `json:"agent_timeout_seconds"` + PermissionMode string `json:"permission_mode"` + Verify bool `json:"verify"` + EnableGithubPR bool `json:"enable_github_pr"` + GithubPRBase string `json:"github_pr_base"` + BranchPrefix string `json:"branch_prefix"` + KeepWorktree bool `json:"keep_worktree"` +} + +// defaultIssueBuildConfig seeds every non-zero Pydantic default. +func defaultIssueBuildConfig() IssueBuildConfig { + return IssueBuildConfig{ + Runtime: DefaultRuntime(), + Models: nil, + MaxCodingIterations: 3, + AgentMaxTurns: 50, + AgentTimeoutSeconds: 1800, + PermissionMode: "", + Verify: true, + EnableGithubPR: false, + GithubPRBase: "", + BranchPrefix: "issue/", + KeepWorktree: false, + } +} + +// LoadIssueBuildConfig constructs an IssueBuildConfig from a raw input map: +// strict decode (extra="forbid") then issue-role model-key validation. +func LoadIssueBuildConfig(raw map[string]any) (*IssueBuildConfig, error) { + if raw == nil { + raw = map[string]any{} + } + cfg := defaultIssueBuildConfig() + if err := strictDecode(raw, &cfg); err != nil { + return nil, err + } + + var unknown []string + for k := range cfg.Models { + if _, ok := issueValidModelKeys[k]; !ok { + unknown = append(unknown, fmt.Sprintf("%q", k)) + } + } + if len(unknown) > 0 { + sort.Strings(unknown) + valid := make([]string, 0, len(issueValidModelKeys)) + for k := range issueValidModelKeys { + valid = append(valid, k) + } + sort.Strings(valid) + return nil, fmt.Errorf( + "Unknown model keys for implement_issue: %s. Valid keys: %s", + strings.Join(unknown, ", "), strings.Join(valid, ", "), + ) + } + + return &cfg, nil +} + +// ToExecutionRaw builds the raw map handed to LoadExecutionConfig — the Go +// equivalent of constructing ExecutionConfig(...) inside +// _implement_issue_impl: the coding loop runs with advisor/replanning/ +// integration-testing/CI/learning all disabled. +func (c *IssueBuildConfig) ToExecutionRaw() map[string]any { + raw := map[string]any{ + "runtime": c.Runtime, + "max_coding_iterations": c.MaxCodingIterations, + "agent_max_turns": c.AgentMaxTurns, + "agent_timeout_seconds": c.AgentTimeoutSeconds, + "permission_mode": c.PermissionMode, + "enable_learning": false, + "enable_replanning": false, + "enable_issue_advisor": false, + "enable_integration_testing": false, + "check_ci": false, + } + if c.Models != nil { + raw["models"] = c.Models + } + return raw +} diff --git a/go/internal/config/issueconfig_test.go b/go/internal/config/issueconfig_test.go new file mode 100644 index 0000000..6cdaf8c --- /dev/null +++ b/go/internal/config/issueconfig_test.go @@ -0,0 +1,82 @@ +package config + +import ( + "strings" + "testing" +) + +func TestIssueBuildConfigDefaults(t *testing.T) { + cfg, err := LoadIssueBuildConfig(nil) + if err != nil { + t.Fatalf("LoadIssueBuildConfig(nil): %v", err) + } + if cfg.MaxCodingIterations != 3 { + t.Errorf("MaxCodingIterations = %d, want 3", cfg.MaxCodingIterations) + } + if cfg.AgentMaxTurns != 50 { + t.Errorf("AgentMaxTurns = %d, want 50", cfg.AgentMaxTurns) + } + if cfg.AgentTimeoutSeconds != 1800 { + t.Errorf("AgentTimeoutSeconds = %d, want 1800", cfg.AgentTimeoutSeconds) + } + if !cfg.Verify { + t.Error("Verify default should be true") + } + if cfg.EnableGithubPR { + t.Error("EnableGithubPR default should be false") + } + if cfg.BranchPrefix != "issue/" { + t.Errorf("BranchPrefix = %q", cfg.BranchPrefix) + } + if cfg.KeepWorktree { + t.Error("KeepWorktree default should be false") + } +} + +func TestIssueBuildConfigModelKeyValidation(t *testing.T) { + if _, err := LoadIssueBuildConfig(map[string]any{ + "models": map[string]any{"default": "haiku", "coder": "sonnet"}, + }); err != nil { + t.Errorf("valid model keys rejected: %v", err) + } + _, err := LoadIssueBuildConfig(map[string]any{ + "models": map[string]any{"pm": "haiku"}, + }) + if err == nil || !strings.Contains(err.Error(), "Unknown model keys") { + t.Errorf("unknown model key accepted: %v", err) + } +} + +func TestIssueBuildConfigStrictDecode(t *testing.T) { + if _, err := LoadIssueBuildConfig(map[string]any{"max_tasks": 5}); err == nil { + t.Error("unknown config key accepted (extra=forbid)") + } +} + +func TestIssueBuildConfigToExecutionRaw(t *testing.T) { + cfg, err := LoadIssueBuildConfig(map[string]any{ + "runtime": "claude_code", + "models": map[string]any{"default": "haiku", "coder": "sonnet"}, + "max_coding_iterations": 2, + }) + if err != nil { + t.Fatalf("LoadIssueBuildConfig: %v", err) + } + execCfg, err := LoadExecutionConfig(cfg.ToExecutionRaw()) + if err != nil { + t.Fatalf("LoadExecutionConfig(ToExecutionRaw): %v", err) + } + if execCfg.MaxCodingIterations != 2 { + t.Errorf("MaxCodingIterations = %d", execCfg.MaxCodingIterations) + } + if execCfg.CoderModel() != "sonnet" { + t.Errorf("CoderModel = %q", execCfg.CoderModel()) + } + if execCfg.CodeReviewerModel() != "haiku" { + t.Errorf("CodeReviewerModel = %q", execCfg.CodeReviewerModel()) + } + if execCfg.EnableIssueAdvisor || execCfg.EnableReplanning || execCfg.CheckCI || + execCfg.EnableIntegrationTesting || execCfg.EnableLearning { + t.Error("issue-level execution config must disable advisor/replanning/CI/integration/learning") + } +} diff --git a/go/internal/issue/build.go b/go/internal/issue/build.go new file mode 100644 index 0000000..ef4436c --- /dev/null +++ b/go/internal/issue/build.go @@ -0,0 +1,448 @@ +package issue + +// build.go ports swe_af/issue/build.py — the implement_issue orchestrator. + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/Agent-Field/SWE-AF/go/internal/afx" + "github.com/Agent-Field/SWE-AF/go/internal/coding" + "github.com/Agent-Field/SWE-AF/go/internal/config" + "github.com/Agent-Field/SWE-AF/go/internal/schemas" +) + +// CallFn dispatches to a reasoner by target with the same keyword args Python +// passes to app.call. Structurally identical to coding.CallFn / fast.CallFn, so +// the node's single agent.Call + envelope-unwrap closure satisfies all three. +type CallFn func(ctx context.Context, target string, kwargs map[string]any) (map[string]any, error) + +// Noter matches the SDK's *agent.Agent.Note method; tests supply a recorder. +type Noter interface { + Note(ctx context.Context, message string, tags ...string) +} + +// Handler is the registration signature, matching the roles/fast packages. +type Handler func(ctx context.Context, deps *Deps, input map[string]any) (any, error) + +// Handlers returns the name→handler map, keyed by the EXACT Python reasoner +// name. The wiring registers it on BOTH nodes (Python includes issue_router in +// swe_af/app.py and swe_af/fast/app.py). +func Handlers() map[string]Handler { + return map[string]Handler{"implement_issue": ImplementIssue} +} + +// Deps carries the seams implement_issue needs. +type Deps struct { + Call CallFn + Note Noter + NodeID string +} + +func (d *Deps) note(ctx context.Context, msg string, tags ...string) { + if d != nil && d.Note != nil { + d.Note.Note(ctx, msg, tags...) + } +} + +type implementInput struct { + Issue map[string]any `json:"issue"` + RepoPath string `json:"repo_path"` + BaseBranch string `json:"base_branch"` + ArtifactsDir string `json:"artifacts_dir"` + AdditionalContext string `json:"additional_context"` + Config map[string]any `json:"config"` +} + +var completedOutcomes = map[string]bool{ + "completed": true, + "completed_with_debt": true, +} + +func newBuildID() string { + b := make([]byte, 4) + if _, err := rand.Read(b); err != nil { + return fmt.Sprintf("%08x", time.Now().UnixNano()&0xffffffff) + } + return hex.EncodeToString(b) +} + +// ImplementIssue implements one fully-scoped issue on an isolated branch (the +// sub-harness entry point). Ports build.implement_issue/_implement_issue_impl: +// setup/validation errors return an error (they are the caller's to fix); +// failures after branch creation come back as a structured result. +func ImplementIssue(ctx context.Context, deps *Deps, input map[string]any) (any, error) { + in, err := afx.Bind[implementInput](input) + if err != nil { + return nil, err + } + if in.RepoPath == "" { + return nil, fmt.Errorf("implement_issue: repo_path is required") + } + if in.ArtifactsDir == "" { + in.ArtifactsDir = ".artifacts" + } + + cfg, err := config.LoadIssueBuildConfig(in.Config) + if err != nil { + return nil, err + } + spec, err := ParseSpec(in.Issue) + if err != nil { + return nil, err + } + planned := spec.ToPlannedIssue(in.AdditionalContext) + plannedName, _ := planned["name"].(string) + + // --- Setup (validation errors raise: they are the caller's to fix) ------ + repoPath, err := filepath.Abs(in.RepoPath) + if err != nil { + return nil, err + } + if err := ensureIssueReadyRepo(repoPath); err != nil { + return nil, err + } + baseRef, baseSHA, err := resolveBase(repoPath, in.BaseBranch) + if err != nil { + return nil, err + } + excludes := []string{".worktrees/"} + if !filepath.IsAbs(in.ArtifactsDir) { + top := topPathSegment(in.ArtifactsDir) + if top != "" && top != "." && top != ".." { + excludes = append(excludes, top+"/") + } + } + if err := ensureLocalExcludes(repoPath, excludes); err != nil { + return nil, err + } + if isDirty(repoPath) { + deps.note(ctx, + "Caller repo has uncommitted changes; the issue branch is created "+ + "from the committed base state and will not see them", + "issue_build", "warning", "dirty_tree", + ) + } + + buildID := newBuildID() + branch := cfg.BranchPrefix + buildID + "-" + plannedName + worktreePath := filepath.Join(repoPath, ".worktrees", buildID+"-"+plannedName) + var absArtifacts string + if filepath.IsAbs(in.ArtifactsDir) { + absArtifacts = filepath.Join(in.ArtifactsDir, "issue-builds", buildID) + } else { + absArtifacts = filepath.Join(repoPath, in.ArtifactsDir, "issue-builds", buildID) + } + if err := os.MkdirAll(absArtifacts, 0o755); err != nil { + return nil, err + } + + if err := addWorktree(repoPath, worktreePath, branch, baseSHA); err != nil { + return nil, err + } + deps.note(ctx, + fmt.Sprintf("Issue build %s: %s on %s (base %s @ %.12s)", + buildID, plannedName, branch, baseRef, baseSHA), + "issue_build", "start", + ) + + planned["worktree_path"] = worktreePath + planned["branch_name"] = branch + + execCfg, err := config.LoadExecutionConfig(cfg.ToExecutionRaw()) + if err != nil { + removeWorktree(repoPath, worktreePath) + deleteBranch(repoPath, branch) + return nil, err + } + dagState := &schemas.DAGState{ + RepoPath: worktreePath, + ArtifactsDir: absArtifacts, + BuildID: buildID, + AllIssues: []map[string]any{planned}, + } + + // --- Execute (failures after branch creation → structured result) ------ + outcomeValue := "error" + loopSummary := "" + errorMessage := "" + iterations := 0 + var iterationHistory []map[string]any + var debtItems []map[string]any + var verification map[string]any + prURL := "" + var commits []string + var filesChanged []string + stat := "" + + noteFn := func(msg string, tags []string) { deps.note(ctx, msg, tags...) } + callFn := coding.CallFn(deps.Call) + + loopResult, loopErr := coding.RunCodingLoop( + ctx, planned, dagState, callFn, deps.NodeID, execCfg, noteFn, nil, + ) + if loopErr != nil && ctx.Err() != nil { + // Context cancellation propagates (Python does not catch CancelledError). + removeWorktree(repoPath, worktreePath) + return nil, loopErr + } + if loopErr != nil { + // Fatal harness errors become a structured failure (Python's generic + // `except Exception` path). + errorMessage = loopErr.Error() + deps.note(ctx, fmt.Sprintf("Issue build failed: %v", loopErr), "issue_build", "error") + commits = newCommits(repoPath, baseSHA, branch) + filesChanged = changedFiles(repoPath, baseSHA, branch) + stat = diffStat(repoPath, baseSHA, branch) + } else { + outcomeValue = string(loopResult.Outcome) + loopSummary = loopResult.ResultSummary + if loopSummary == "" { + loopSummary = loopResult.ErrorMessage + } + errorMessage = loopResult.ErrorMessage + iterations = loopResult.Attempts + iterationHistory = loopResult.IterationHistory + debtItems = loopResult.DebtItems + + if _, err := scrubTrackedJunk(worktreePath, plannedName); err != nil { + deps.note(ctx, fmt.Sprintf("Junk scrub failed: %v", err), + "issue_build", "error") + } + if _, err := commitAll( + worktreePath, + fmt.Sprintf("chore(%s): checkpoint uncommitted issue work", plannedName), + ); err != nil { + deps.note(ctx, fmt.Sprintf("Checkpoint commit failed: %v", err), + "issue_build", "error") + } + commits = newCommits(repoPath, baseSHA, branch) + filesChanged = changedFiles(repoPath, baseSHA, branch) + stat = diffStat(repoPath, baseSHA, branch) + + codingOK := completedOutcomes[outcomeValue] + if cfg.Verify && codingOK && len(commits) > 0 { + verification = runVerification(ctx, deps, cfg, execCfg, spec, planned, + worktreePath, absArtifacts, loopSummary) + } + if cfg.EnableGithubPR && codingOK && len(commits) > 0 { + prURL = maybeCreatePR(ctx, deps, cfg, execCfg, spec, planned, + repoPath, worktreePath, branch, baseRef, absArtifacts, + loopSummary, debtItems) + } + } + + if !cfg.KeepWorktree { + removeWorktree(repoPath, worktreePath) + } + if len(commits) == 0 { + // A branch with zero commits is pure noise for the caller. + deleteBranch(repoPath, branch) + } + + codingOK := completedOutcomes[outcomeValue] + verifyOK := verification == nil || isTruthyBool(verification["passed"]) + success := codingOK && len(commits) > 0 && verifyOK + + summary := fmt.Sprintf("%s: %s, %d commit(s), %d file(s) changed", + plannedName, outcomeValue, len(commits), len(filesChanged)) + if len(commits) > 0 { + summary += " on " + branch + } else { + summary += "; no commits produced" + } + if verification != nil { + if isTruthyBool(verification["passed"]) { + summary += "; verification passed" + } else { + summary += "; verification FAILED" + } + } + if loopSummary != "" { + summary += " — " + loopSummary + } + + completeTag := "complete" + if !success { + completeTag = "failed" + } + deps.note(ctx, summary, "issue_build", completeTag) + + resultBranch := branch + if len(commits) == 0 { + resultBranch = "" + } + finalError := errorMessage + if success { + finalError = "" + } + if commits == nil { + commits = []string{} + } + if filesChanged == nil { + filesChanged = []string{} + } + if iterationHistory == nil { + iterationHistory = []map[string]any{} + } + if debtItems == nil { + debtItems = []map[string]any{} + } + + return map[string]any{ + "success": success, + "outcome": outcomeValue, + "summary": summary, + "build_id": buildID, + "branch": resultBranch, + "base_branch": baseRef, + "base_sha": baseSHA, + "commits": commits, + "files_changed": filesChanged, + "diff_stat": stat, + "iterations": iterations, + "iteration_history": iterationHistory, + "debt_items": debtItems, + "verification": anyOrNil(verification), + "pr_url": prURL, + "error_message": finalError, + }, nil +} + +// runVerification ports build._run_verification — one verifier pass whose +// unavailability reports passed=false, never success. +func runVerification( + ctx context.Context, + deps *Deps, + cfg *config.IssueBuildConfig, + execCfg *config.ExecutionConfig, + spec *Spec, + planned map[string]any, + worktreePath, artifactsDir, loopSummary string, +) map[string]any { + callCtx, cancel := context.WithTimeout(ctx, time.Duration(cfg.AgentTimeoutSeconds)*time.Second) + defer cancel() + + prd := map[string]any{ + "validated_description": spec.Title + "\n\n" + spec.Description, + "acceptance_criteria": toAnySlice(spec.AcceptanceCriteria), + "must_have": toAnySlice(spec.AcceptanceCriteria), + "nice_to_have": []any{}, + "out_of_scope": []any{}, + } + result, err := deps.Call(callCtx, deps.NodeID+".run_verifier", map[string]any{ + "prd": prd, + "repo_path": worktreePath, + "artifacts_dir": artifactsDir, + "completed_issues": []any{map[string]any{ + "issue_name": planned["name"], + "result_summary": loopSummary, + }}, + "failed_issues": []any{}, + "skipped_issues": []any{}, + "model": execCfg.VerifierModel(), + "permission_mode": cfg.PermissionMode, + "ai_provider": execCfg.AIProvider(), + }) + if err != nil { + deps.note(ctx, fmt.Sprintf("Verifier unavailable: %v", err), + "issue_build", "verify", "error") + return map[string]any{ + "passed": false, + "summary": fmt.Sprintf("Verification unavailable: %v", err), + } + } + return result +} + +// maybeCreatePR ports build._maybe_create_pr — best-effort PR creation. +func maybeCreatePR( + ctx context.Context, + deps *Deps, + cfg *config.IssueBuildConfig, + execCfg *config.ExecutionConfig, + spec *Spec, + planned map[string]any, + repoPath, worktreePath, branch, baseRef, artifactsDir, loopSummary string, + debtItems []map[string]any, +) string { + if remoteURL(repoPath) == "" { + deps.note(ctx, "No origin remote; skipping PR creation", + "issue_build", "github_pr", "skip") + return "" + } + baseForPR := cfg.GithubPRBase + if baseForPR == "" { + baseForPR = defaultRemoteBranch(repoPath) + } + if baseForPR == "" { + baseForPR = baseRef + if baseForPR == "HEAD" { + baseForPR = "main" + } + } + buildSummary := loopSummary + if buildSummary == "" { + buildSummary = spec.Title + } + debtAny := make([]any, len(debtItems)) + for i, d := range debtItems { + debtAny[i] = d + } + + callCtx, cancel := context.WithTimeout(ctx, time.Duration(cfg.AgentTimeoutSeconds)*time.Second) + defer cancel() + result, err := deps.Call(callCtx, deps.NodeID+".run_github_pr", map[string]any{ + "repo_path": worktreePath, + "integration_branch": branch, + "base_branch": baseForPR, + "goal": spec.Title, + "build_summary": buildSummary, + "completed_issues": []any{map[string]any{ + "issue_name": planned["name"], + "result_summary": loopSummary, + }}, + "accumulated_debt": debtAny, + "artifacts_dir": artifactsDir, + "model": execCfg.GitModel(), + "permission_mode": cfg.PermissionMode, + "ai_provider": execCfg.AIProvider(), + }) + if err != nil { + deps.note(ctx, fmt.Sprintf("PR creation failed (non-fatal): %v", err), + "issue_build", "github_pr", "error") + return "" + } + if url, ok := result["pr_url"].(string); ok { + return url + } + return "" +} + +// topPathSegment returns the first path component of a relative path — the +// pattern added to .git/info/exclude for the artifacts dir. +func topPathSegment(p string) string { + p = strings.Trim(p, "/") + if i := strings.IndexByte(p, '/'); i >= 0 { + return p[:i] + } + return p +} + +func isTruthyBool(v any) bool { + b, ok := v.(bool) + return ok && b +} + +func anyOrNil(m map[string]any) any { + if m == nil { + return nil + } + return m +} diff --git a/go/internal/issue/build_test.go b/go/internal/issue/build_test.go new file mode 100644 index 0000000..0af584a --- /dev/null +++ b/go/internal/issue/build_test.go @@ -0,0 +1,625 @@ +package issue + +// Contract tests for the issue-level build orchestrator, mirroring +// tests/issue/test_implement_issue.py: +// +// C1. Branch contains the implementation; caller's tree/branch untouched. +// C2. Concurrent calls yield independent branches. +// C3. No planning roles invoked; call count bounded. +// C4. Default config pushes nothing and opens no PR. +// C5. Verifier failure surfaces (success=false) instead of claiming success. +// C6. Exhaustion / blocking review / coder crash surface debt or failure. +// +// Everything is real (temp git repo, worktrees, the real coding loop) except +// the scripted CallFn. + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/Agent-Field/SWE-AF/go/internal/fatal" +) + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +func gitT(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +func initRepo(t *testing.T) string { + t.Helper() + repo := filepath.Join(t.TempDir(), "repo") + if err := os.MkdirAll(repo, 0o755); err != nil { + t.Fatal(err) + } + gitT(t, repo, "init", "-q") + gitT(t, repo, "checkout", "-q", "-b", "main") + gitT(t, repo, "config", "user.email", "test@example.com") + gitT(t, repo, "config", "user.name", "Test") + if err := os.WriteFile(filepath.Join(repo, "README.md"), []byte("# Test\n"), 0o644); err != nil { + t.Fatal(err) + } + gitT(t, repo, "add", "README.md") + gitT(t, repo, "commit", "-q", "-m", "initial commit") + return repo +} + +var planningTargets = map[string]bool{ + "run_product_manager": true, "run_architect": true, "run_tech_lead": true, + "run_sprint_planner": true, "run_issue_writer": true, "run_environment_scout": true, +} + +type recorder struct { + mu sync.Mutex + targets []string +} + +func (r *recorder) add(target string) { + r.mu.Lock() + defer r.mu.Unlock() + r.targets = append(r.targets, target) +} + +func (r *recorder) names() []string { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]string, len(r.targets)) + for i, t := range r.targets { + out[i] = t[strings.LastIndex(t, ".")+1:] + } + return out +} + +type scriptOpts struct { + coderCommits bool + coderWrites bool + reviewerReplies []map[string]any + verifierReply map[string]any + coderErr error +} + +// scriptedCallFn mirrors tests/issue/conftest.make_call_fn: the fake coder +// writes (and by default commits) one file per iteration in the worktree. +func scriptedCallFn(t *testing.T, rec *recorder, opts scriptOpts) CallFn { + t.Helper() + if opts.reviewerReplies == nil { + opts.reviewerReplies = []map[string]any{ + {"approved": true, "blocking": false, "summary": "LGTM"}, + } + } + reviews := 0 + var mu sync.Mutex + + return func(ctx context.Context, target string, kwargs map[string]any) (map[string]any, error) { + rec.add(target) + name := target[strings.LastIndex(target, ".")+1:] + switch name { + case "run_coder": + if opts.coderErr != nil { + return nil, opts.coderErr + } + worktree, _ := kwargs["worktree_path"].(string) + iteration := 1 + if v, ok := kwargs["iteration"].(int); ok { + iteration = v + } + if opts.coderWrites { + path := filepath.Join(worktree, "feature.py") + if err := os.WriteFile(path, []byte(fmt.Sprintf("VALUE = %d\n", iteration)), 0o644); err != nil { + return nil, err + } + if opts.coderCommits { + gitT(t, worktree, "add", "feature.py") + gitT(t, worktree, "commit", "-q", "-m", fmt.Sprintf("feat: iteration %d", iteration)) + } + } + files := []any{} + if opts.coderWrites { + files = []any{"feature.py"} + } + return map[string]any{ + "files_changed": files, + "summary": fmt.Sprintf("iteration %d", iteration), + "complete": true, + }, nil + case "run_code_reviewer": + mu.Lock() + idx := reviews + if idx >= len(opts.reviewerReplies) { + idx = len(opts.reviewerReplies) - 1 + } + reviews++ + mu.Unlock() + out := map[string]any{} + for k, v := range opts.reviewerReplies[idx] { + out[k] = v + } + return out, nil + case "run_qa": + return map[string]any{"passed": true, "summary": "qa ok"}, nil + case "run_qa_synthesizer": + return map[string]any{"action": "approve", "summary": "synth ok"}, nil + case "run_verifier": + if opts.verifierReply != nil { + return opts.verifierReply, nil + } + return map[string]any{"passed": true, "summary": "verified"}, nil + } + return nil, fmt.Errorf("unexpected call target: %s", target) + } +} + +func runImplement(t *testing.T, repo string, callFn CallFn, input map[string]any) map[string]any { + t.Helper() + deps := &Deps{Call: callFn, NodeID: "test-node"} + base := map[string]any{"issue": map[string]any{ + "title": "Add retry helper", + "description": "Add a retry helper with exponential backoff.", + "acceptance_criteria": []any{"retry() retries up to 3 times"}, + }, "repo_path": repo} + for k, v := range input { + base[k] = v + } + raw, err := ImplementIssue(context.Background(), deps, base) + if err != nil { + t.Fatalf("ImplementIssue: %v", err) + } + result, ok := raw.(map[string]any) + if !ok { + t.Fatalf("result type %T", raw) + } + return result +} + +// --------------------------------------------------------------------------- +// C1 — happy path +// --------------------------------------------------------------------------- + +func TestCreatesBranchAndLeavesCallerUntouched(t *testing.T) { + repo := initRepo(t) + headBefore := gitT(t, repo, "rev-parse", "HEAD") + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), nil) + + if result["success"] != true { + t.Fatalf("success = %v (result: %v)", result["success"], result["summary"]) + } + if result["outcome"] != "completed" { + t.Errorf("outcome = %v", result["outcome"]) + } + branch, _ := result["branch"].(string) + if !strings.HasPrefix(branch, "issue/") { + t.Fatalf("branch = %q", branch) + } + if n := len(result["commits"].([]string)); n != 1 { + t.Errorf("commits = %d, want 1", n) + } + if v, _ := result["verification"].(map[string]any); v == nil || v["passed"] != true { + t.Errorf("verification = %v", result["verification"]) + } + + // Caller repo untouched. + if got := gitT(t, repo, "rev-parse", "--abbrev-ref", "HEAD"); got != "main" { + t.Errorf("caller branch = %q", got) + } + if got := gitT(t, repo, "rev-parse", "HEAD"); got != headBefore { + t.Errorf("caller HEAD moved: %q", got) + } + if got := gitT(t, repo, "status", "--porcelain"); got != "" { + t.Errorf("caller status dirty: %q", got) + } + + // The branch carries the implementation; the worktree is gone. + if got := gitT(t, repo, "show", branch+":feature.py"); got == "" { + t.Error("feature.py missing on branch") + } + entries, _ := os.ReadDir(filepath.Join(repo, ".worktrees")) + if len(entries) != 0 { + t.Errorf("worktrees not cleaned: %v", entries) + } +} + +func TestUncommittedCoderWorkGetsCheckpointCommit(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: false, coderWrites: true}), + map[string]any{"config": map[string]any{"verify": false}}) + + if result["success"] != true { + t.Fatalf("success = %v", result["success"]) + } + branch := result["branch"].(string) + msg := gitT(t, repo, "log", "-1", "--format=%s", branch) + if !strings.Contains(msg, "checkpoint") { + t.Errorf("checkpoint commit message = %q", msg) + } +} + +func TestBytecodeJunkNeverLandsOnBranch(t *testing.T) { + // The real coder runs tests in the worktree, generating __pycache__, and + // a sloppy model may even commit it. Neither may reach the branch. + repo := initRepo(t) + rec := &recorder{} + inner := scriptedCallFn(t, rec, scriptOpts{coderCommits: false, coderWrites: true}) + callFn := func(ctx context.Context, target string, kwargs map[string]any) (map[string]any, error) { + if strings.HasSuffix(target, "run_coder") { + worktree, _ := kwargs["worktree_path"].(string) + pycache := filepath.Join(worktree, "pkg", "__pycache__") + if err := os.MkdirAll(pycache, 0o755); err != nil { + return nil, err + } + if err := os.WriteFile(filepath.Join(pycache, "x.cpython-312.pyc"), []byte{0}, 0o644); err != nil { + return nil, err + } + } + return inner(ctx, target, kwargs) + } + result := runImplement(t, repo, callFn, + map[string]any{"config": map[string]any{"verify": false}}) + + if result["success"] != true { + t.Fatalf("success = %v", result["success"]) + } + for _, f := range result["files_changed"].([]string) { + if strings.Contains(f, "__pycache__") || strings.HasSuffix(f, ".pyc") { + t.Errorf("junk in files_changed: %s", f) + } + } + tracked := gitT(t, repo, "ls-tree", "-r", "--name-only", result["branch"].(string)) + if strings.Contains(tracked, "__pycache__") { + t.Errorf("junk tracked on branch:\n%s", tracked) + } +} + +func TestScrubUntracksAgentCommittedJunk(t *testing.T) { + repo := initRepo(t) + _, baseSHA, err := resolveBase(repo, "") + if err != nil { + t.Fatal(err) + } + worktree := filepath.Join(repo, ".worktrees", "wt-junk") + if err := addWorktree(repo, worktree, "issue/junk", baseSHA); err != nil { + t.Fatal(err) + } + pycache := filepath.Join(worktree, "pkg", "__pycache__") + if err := os.MkdirAll(pycache, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pycache, "m.cpython-312.pyc"), []byte{0}, 0o644); err != nil { + t.Fatal(err) + } + gitT(t, worktree, "add", "-A", ".") + gitT(t, worktree, "commit", "-q", "-m", "agent committed junk") + + sha, err := scrubTrackedJunk(worktree, "junk-issue") + if err != nil || sha == "" { + t.Fatalf("scrub: sha=%q err=%v", sha, err) + } + if tracked := gitT(t, worktree, "ls-files"); strings.Contains(tracked, "__pycache__") { + t.Errorf("junk still tracked:\n%s", tracked) + } + if again, err := scrubTrackedJunk(worktree, "junk-issue"); err != nil || again != "" { + t.Errorf("scrub not idempotent: sha=%q err=%v", again, err) + } +} + +func TestBaseBranchOverride(t *testing.T) { + repo := initRepo(t) + gitT(t, repo, "checkout", "-q", "-b", "dev") + if err := os.WriteFile(filepath.Join(repo, "dev.txt"), []byte("dev\n"), 0o644); err != nil { + t.Fatal(err) + } + gitT(t, repo, "add", "dev.txt") + gitT(t, repo, "commit", "-q", "-m", "dev work") + gitT(t, repo, "checkout", "-q", "main") + + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), + map[string]any{"base_branch": "dev", "config": map[string]any{"verify": false}}) + + if result["success"] != true || result["base_branch"] != "dev" { + t.Fatalf("result = %v", result) + } + if got := gitT(t, repo, "show", result["branch"].(string)+":dev.txt"); got != "dev" { + t.Errorf("dev.txt on branch = %q", got) + } +} + +// --------------------------------------------------------------------------- +// C2 — parallel fan-out +// --------------------------------------------------------------------------- + +func TestConcurrentBuildsOnSameRepo(t *testing.T) { + repo := initRepo(t) + results := make([]map[string]any, 2) + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + rec := &recorder{} + deps := &Deps{ + Call: scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), + NodeID: "test-node", + } + raw, err := ImplementIssue(context.Background(), deps, map[string]any{ + "issue": map[string]any{ + "title": fmt.Sprintf("Issue %d", i), + "description": "concurrent", + }, + "repo_path": repo, + "config": map[string]any{"verify": false}, + }) + if err != nil { + t.Errorf("ImplementIssue[%d]: %v", i, err) + return + } + results[i] = raw.(map[string]any) + }(i) + } + wg.Wait() + + if results[0] == nil || results[1] == nil { + t.Fatal("missing results") + } + if results[0]["success"] != true || results[1]["success"] != true { + t.Fatalf("successes: %v / %v", results[0]["summary"], results[1]["summary"]) + } + if results[0]["branch"] == results[1]["branch"] { + t.Errorf("branches collide: %v", results[0]["branch"]) + } + if got := gitT(t, repo, "rev-parse", "--abbrev-ref", "HEAD"); got != "main" { + t.Errorf("caller branch = %q", got) + } +} + +// --------------------------------------------------------------------------- +// C3 — call budget +// --------------------------------------------------------------------------- + +func TestNoPlanningAgentsAndBoundedCalls(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), nil) + + names := rec.names() + for _, n := range names { + if planningTargets[n] { + t.Errorf("planning agent invoked: %s", n) + } + } + // 1 iteration: coder + reviewer, then one verifier pass. + if len(names) != 3 { + t.Errorf("call count = %d (%v), want 3", len(names), names) + } + if result["iterations"] != 1 { + t.Errorf("iterations = %v", result["iterations"]) + } +} + +func TestFlaggedPathUsesQAAndSynthesizer(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), + map[string]any{ + "issue": map[string]any{ + "title": "Risky change", "description": "d", "needs_deeper_qa": true, + }, + "config": map[string]any{"verify": false}, + }) + + if result["success"] != true { + t.Fatalf("success = %v", result["success"]) + } + names := strings.Join(rec.names(), ",") + if !strings.Contains(names, "run_qa") || !strings.Contains(names, "run_qa_synthesizer") { + t.Errorf("flagged path roles missing: %s", names) + } +} + +// --------------------------------------------------------------------------- +// C4 — no side effects by default +// --------------------------------------------------------------------------- + +func TestNoPRAndNoPushByDefault(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), nil) + + for _, n := range rec.names() { + if n == "run_github_pr" { + t.Error("run_github_pr invoked with enable_github_pr=false") + } + } + if result["pr_url"] != "" { + t.Errorf("pr_url = %v", result["pr_url"]) + } + if got := gitT(t, repo, "remote"); got != "" { + t.Errorf("unexpected remote: %q", got) + } +} + +func TestPRSkippedWithoutRemoteEvenWhenEnabled(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderCommits: true, coderWrites: true}), + map[string]any{"config": map[string]any{"verify": false, "enable_github_pr": true}}) + + if result["success"] != true || result["pr_url"] != "" { + t.Fatalf("result = %v / %v", result["success"], result["pr_url"]) + } + for _, n := range rec.names() { + if n == "run_github_pr" { + t.Error("run_github_pr invoked without a remote") + } + } +} + +// --------------------------------------------------------------------------- +// C5 — verification +// --------------------------------------------------------------------------- + +func TestVerifierFailureFailsBuildButKeepsBranch(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{ + coderCommits: true, coderWrites: true, + verifierReply: map[string]any{"passed": false, "summary": "AC not met"}, + }), nil) + + if result["success"] != false { + t.Errorf("success = %v, want false", result["success"]) + } + if result["outcome"] != "completed" { + t.Errorf("outcome = %v", result["outcome"]) + } + if branch, _ := result["branch"].(string); !strings.HasPrefix(branch, "issue/") { + t.Errorf("branch = %v (partial work should be kept)", result["branch"]) + } +} + +// --------------------------------------------------------------------------- +// C6 — failure modes +// --------------------------------------------------------------------------- + +func TestExhaustionCompletesWithDebt(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{ + coderCommits: true, coderWrites: true, + reviewerReplies: []map[string]any{ + {"approved": false, "blocking": false, "summary": "needs polish"}, + }, + }), + map[string]any{"config": map[string]any{"verify": false, "max_coding_iterations": 2}}) + + if result["outcome"] != "completed_with_debt" { + t.Errorf("outcome = %v", result["outcome"]) + } + if result["success"] != true { + t.Errorf("success = %v", result["success"]) + } + if result["iterations"] != 2 { + t.Errorf("iterations = %v", result["iterations"]) + } +} + +func TestBlockingReviewFailsButSalvagesCommits(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{ + coderCommits: true, coderWrites: true, + reviewerReplies: []map[string]any{ + {"approved": false, "blocking": true, "summary": "introduces data loss"}, + }, + }), + map[string]any{"config": map[string]any{"verify": false}}) + + if result["success"] != false || result["outcome"] != "failed_unrecoverable" { + t.Fatalf("result = %v / %v", result["success"], result["outcome"]) + } + if branch, _ := result["branch"].(string); !strings.HasPrefix(branch, "issue/") { + t.Errorf("branch = %v (commits should be salvaged)", result["branch"]) + } +} + +func TestNoCommitsDeletesBranch(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{coderWrites: false}), + map[string]any{"config": map[string]any{"verify": false}}) + + if result["success"] != false || result["branch"] != "" { + t.Fatalf("result = %v / %v", result["success"], result["branch"]) + } + if got := gitT(t, repo, "branch", "--list", "issue/*"); got != "" { + t.Errorf("stray issue branches: %q", got) + } +} + +func TestFatalCoderErrorReturnsStructuredFailureAndCleansUp(t *testing.T) { + repo := initRepo(t) + rec := &recorder{} + result := runImplement(t, repo, + scriptedCallFn(t, rec, scriptOpts{ + coderWrites: true, + coderErr: &fatal.FatalHarnessError{OriginalMessage: "credit balance too low"}, + }), + map[string]any{"config": map[string]any{"verify": false}}) + + if result["success"] != false || result["outcome"] != "error" { + t.Fatalf("result = %v / %v", result["success"], result["outcome"]) + } + if msg, _ := result["error_message"].(string); !strings.Contains(msg, "credit balance") { + t.Errorf("error_message = %q", msg) + } + if got := gitT(t, repo, "branch", "--list", "issue/*"); got != "" { + t.Errorf("stray issue branches: %q", got) + } + entries, _ := os.ReadDir(filepath.Join(repo, ".worktrees")) + if len(entries) != 0 { + t.Errorf("worktrees not cleaned: %v", entries) + } +} + +// --------------------------------------------------------------------------- +// Setup validation +// --------------------------------------------------------------------------- + +func TestSetupValidationErrors(t *testing.T) { + deps := &Deps{Call: func(context.Context, string, map[string]any) (map[string]any, error) { + return nil, fmt.Errorf("must not be called") + }, NodeID: "test-node"} + issueMap := map[string]any{"title": "t", "description": "d"} + + if _, err := ImplementIssue(context.Background(), deps, map[string]any{ + "issue": issueMap, "repo_path": filepath.Join(t.TempDir(), "nope"), + }); err == nil || !strings.Contains(err.Error(), "does not exist") { + t.Errorf("missing repo: err = %v", err) + } + + plain := t.TempDir() + if _, err := ImplementIssue(context.Background(), deps, map[string]any{ + "issue": issueMap, "repo_path": plain, + }); err == nil || !strings.Contains(err.Error(), "not a git repository") { + t.Errorf("non-repo: err = %v", err) + } + + repo := initRepo(t) + if _, err := ImplementIssue(context.Background(), deps, map[string]any{ + "issue": issueMap, "repo_path": repo, "base_branch": "ghost", + }); err == nil || !strings.Contains(err.Error(), "base branch") { + t.Errorf("unknown base: err = %v", err) + } + + if _, err := ImplementIssue(context.Background(), deps, map[string]any{ + "issue": issueMap, + }); err == nil || !strings.Contains(err.Error(), "repo_path is required") { + t.Errorf("missing repo_path: err = %v", err) + } +} diff --git a/go/internal/issue/exec.go b/go/internal/issue/exec.go new file mode 100644 index 0000000..1061576 --- /dev/null +++ b/go/internal/issue/exec.go @@ -0,0 +1,38 @@ +package issue + +import ( + "bytes" + "errors" + "os/exec" + "strings" +) + +// runGit executes `git -C repoPath args...` and returns (stdout, detail, +// exitCode). detail is stderr when present, else stdout, else the exec error — +// mirroring git_ops._git's error-detail selection. +func runGit(repoPath string, args ...string) (string, string, int) { + cmd := exec.Command("git", append([]string{"-C", repoPath}, args...)...) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + + code := 0 + if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + code = exitErr.ExitCode() + } else { + code = -1 + } + } + + out := strings.TrimSpace(stdout.String()) + detail := strings.TrimSpace(stderr.String()) + if detail == "" { + detail = out + } + if detail == "" && err != nil { + detail = err.Error() + } + return out, detail, code +} diff --git a/go/internal/issue/gitops.go b/go/internal/issue/gitops.go new file mode 100644 index 0000000..f0741f5 --- /dev/null +++ b/go/internal/issue/gitops.go @@ -0,0 +1,269 @@ +package issue + +// gitops.go ports swe_af/issue/git_ops.py — deterministic (non-LLM) git +// operations for the issue-level build path. The full pipeline delegates git +// work to LLM agents; the issue-level path owns exactly one worktree + one +// branch off a known base, so plain git commands are sufficient and keep the +// LLM budget for coding. + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" +) + +// GitOpsError mirrors git_ops.GitOpsError — a git failure the caller must fix. +type GitOpsError struct{ Msg string } + +func (e *GitOpsError) Error() string { return e.Msg } + +func gitOpsErrf(format string, args ...any) *GitOpsError { + return &GitOpsError{Msg: fmt.Sprintf(format, args...)} +} + +func ensureIssueReadyRepo(repoPath string) error { + info, err := os.Stat(repoPath) + if err != nil || !info.IsDir() { + return gitOpsErrf("repo_path does not exist or is not a directory: %s", repoPath) + } + if _, _, code := runGit(repoPath, "rev-parse", "--git-dir"); code != 0 { + return gitOpsErrf("repo_path is not a git repository: %s", repoPath) + } + if _, _, code := runGit(repoPath, "rev-parse", "--verify", "HEAD"); code != 0 { + return gitOpsErrf( + "repository has no commits; issue-level builds require an existing " + + "base commit (use the feature-level build for empty repos)", + ) + } + return nil +} + +func currentBranch(repoPath string) (string, error) { + out, detail, code := runGit(repoPath, "rev-parse", "--abbrev-ref", "HEAD") + if code != 0 { + return "", gitOpsErrf("git rev-parse --abbrev-ref HEAD failed: %s", detail) + } + return out, nil +} + +// resolveBase ports git_ops.resolve_base: (base_ref, base_sha), defaulting to +// the caller's current branch (or literal "HEAD" when detached). +func resolveBase(repoPath, baseBranch string) (string, string, error) { + ref := baseBranch + if ref == "" { + var err error + if ref, err = currentBranch(repoPath); err != nil { + return "", "", err + } + } + sha, _, code := runGit(repoPath, "rev-parse", "--verify", ref+"^{commit}") + if code != 0 { + return "", "", gitOpsErrf("base branch/ref not found: %s", ref) + } + return ref, sha, nil +} + +func isDirty(repoPath string) bool { + out, _, code := runGit(repoPath, "status", "--porcelain") + return code == 0 && out != "" +} + +// addWorktree creates an isolated worktree on a new branch off baseSHA, +// retrying briefly because concurrent `git worktree add` calls contend on the +// repo lock — the exact scenario a fan-out caller creates. +func addWorktree(repoPath, worktreePath, branch, baseSHA string) error { + if err := os.MkdirAll(filepath.Dir(worktreePath), 0o755); err != nil { + return gitOpsErrf("mkdir for worktree failed: %v", err) + } + const attempts = 3 + var lastDetail string + for attempt := 1; attempt <= attempts; attempt++ { + _, detail, code := runGit(repoPath, "worktree", "add", "-b", branch, worktreePath, baseSHA) + if code == 0 { + return nil + } + lastDetail = detail + if attempt < attempts { + time.Sleep(time.Duration(attempt) * 500 * time.Millisecond) + } + } + return gitOpsErrf("git worktree add failed after %d attempts: %s", attempts, lastDetail) +} + +func hasCommitIdentity(repoPath string) bool { + out, _, code := runGit(repoPath, "config", "user.email") + return code == 0 && out != "" +} + +// junkPathspecs lists bytecode/cache junk that must never be versioned on the +// issue branch. The coder runs tests inside the worktree, so these appear as +// a side effect and an indiscriminate `git add` (ours or the coder's) would +// sweep them in. Ports git_ops._JUNK_PATHSPECS. +var junkPathspecs = []string{"*__pycache__*", "*.pyc", "*.pyo"} + +// commitIndex commits whatever is staged. Returns the sha, or "" when the +// index is clean. Ports git_ops._commit_index. +func commitIndex(worktreePath, message string) (string, error) { + args := []string{} + if !hasCommitIdentity(worktreePath) { + args = append(args, + "-c", "user.name=SWE-AF", + "-c", "user.email=swe-af@agentfield.local", + ) + } + args = append(args, "commit", "-m", message) + if out, detail, code := runGit(worktreePath, args...); code != 0 { + lower := strings.ToLower(out + "\n" + detail) + if strings.Contains(lower, "nothing to commit") || + strings.Contains(lower, "nothing added to commit") { + return "", nil + } + return "", gitOpsErrf("git commit failed: %s", detail) + } + sha, detail, code := runGit(worktreePath, "rev-parse", "HEAD") + if code != 0 { + return "", gitOpsErrf("git rev-parse HEAD failed: %s", detail) + } + return sha, nil +} + +// scrubTrackedJunk untracks bytecode junk an agent committed on the issue +// branch. Returns the scrub commit sha, or "" when the branch was already +// clean. History stays append-only. Ports git_ops.scrub_tracked_junk. +func scrubTrackedJunk(worktreePath, issueName string) (string, error) { + listArgs := append([]string{"ls-files", "--"}, junkPathspecs...) + out, _, code := runGit(worktreePath, listArgs...) + if code != 0 || out == "" { + return "", nil + } + rmArgs := append([]string{"rm", "-r", "--cached", "-q", "--ignore-unmatch", "--"}, junkPathspecs...) + if _, detail, code := runGit(worktreePath, rmArgs...); code != 0 { + return "", gitOpsErrf("git rm --cached failed: %s", detail) + } + return commitIndex(worktreePath, + fmt.Sprintf("chore(%s): untrack bytecode caches", issueName)) +} + +// commitAll commits any uncommitted changes inside the (isolated) worktree. +// `add -A` is safe here precisely because the worktree belongs to this build +// alone; bytecode junk is excluded. Returns the new commit sha, or "" when +// there was nothing to commit. +func commitAll(worktreePath, message string) (string, error) { + if !isDirty(worktreePath) { + return "", nil + } + addArgs := []string{"add", "-A", "--", "."} + for _, p := range junkPathspecs { + addArgs = append(addArgs, ":(exclude)"+p) + } + if _, detail, code := runGit(worktreePath, addArgs...); code != 0 { + return "", gitOpsErrf("git add -A failed: %s", detail) + } + return commitIndex(worktreePath, message) +} + +// newCommits returns commits on branch since baseSHA, oldest first; empty when +// the branch is gone. +func newCommits(repoPath, baseSHA, branch string) []string { + out, _, code := runGit(repoPath, "rev-list", "--reverse", baseSHA+".."+branch) + if code != 0 || out == "" { + return nil + } + return strings.Fields(out) +} + +func changedFiles(repoPath, baseSHA, branch string) []string { + out, _, code := runGit(repoPath, "diff", "--name-only", baseSHA+".."+branch) + if code != 0 || out == "" { + return nil + } + return strings.Split(out, "\n") +} + +func diffStat(repoPath, baseSHA, branch string) string { + out, _, code := runGit(repoPath, "diff", "--stat", baseSHA+".."+branch) + if code != 0 { + return "" + } + return out +} + +// removeWorktree removes the worktree (the branch survives). Best-effort. +func removeWorktree(repoPath, worktreePath string) { + _, _, code := runGit(repoPath, "worktree", "remove", "--force", worktreePath) + if code != 0 { + if _, err := os.Stat(worktreePath); err == nil { + _ = os.RemoveAll(worktreePath) + runGit(repoPath, "worktree", "prune") + } + } +} + +// deleteBranch deletes a branch (used only when it holds no commits). +func deleteBranch(repoPath, branch string) { + runGit(repoPath, "branch", "-D", branch) +} + +func remoteURL(repoPath string) string { + out, _, code := runGit(repoPath, "remote", "get-url", "origin") + if code != 0 { + return "" + } + return out +} + +func defaultRemoteBranch(repoPath string) string { + out, _, code := runGit(repoPath, "symbolic-ref", "refs/remotes/origin/HEAD") + if code != 0 { + return "" + } + const prefix = "refs/remotes/origin/" + if strings.HasPrefix(out, prefix) { + return out[len(prefix):] + } + return "" +} + +// ensureLocalExcludes adds patterns to the repo-local ignore file +// (.git/info/exclude) so issue-build bookkeeping (.artifacts, .worktrees) +// never shows up in the caller's `git status` — unlike .gitignore, this file +// is not versioned and never shows in a diff. +func ensureLocalExcludes(repoPath string, patterns []string) error { + gitDir, detail, code := runGit(repoPath, "rev-parse", "--git-dir") + if code != 0 { + return gitOpsErrf("git rev-parse --git-dir failed: %s", detail) + } + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(repoPath, gitDir) + } + excludePath := filepath.Join(gitDir, "info", "exclude") + if err := os.MkdirAll(filepath.Dir(excludePath), 0o755); err != nil { + return gitOpsErrf("mkdir .git/info failed: %v", err) + } + existing := map[string]struct{}{} + if data, err := os.ReadFile(excludePath); err == nil { + for _, line := range strings.Split(string(data), "\n") { + existing[strings.TrimSpace(line)] = struct{}{} + } + } + var toAdd []string + for _, p := range patterns { + if _, ok := existing[p]; !ok { + toAdd = append(toAdd, p) + } + } + if len(toAdd) == 0 { + return nil + } + f, err := os.OpenFile(excludePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + return gitOpsErrf("open .git/info/exclude failed: %v", err) + } + defer f.Close() + if _, err := f.WriteString(strings.Join(toAdd, "\n") + "\n"); err != nil { + return gitOpsErrf("write .git/info/exclude failed: %v", err) + } + return nil +} diff --git a/go/internal/issue/spec.go b/go/internal/issue/spec.go new file mode 100644 index 0000000..e9d454d --- /dev/null +++ b/go/internal/issue/spec.go @@ -0,0 +1,119 @@ +// Package issue is the issue-level build entry point (sub-harness surface) — a +// 1:1 behavioural port of swe_af/issue/*. A main coding harness (Claude Code, +// Codex, OpenCode) delegates one fully-scoped issue; no planning agents run. +// Deterministic git setup creates an isolated worktree + issue/- +// branch, the existing coding loop implements the issue, an optional single +// verifier pass checks the acceptance criteria, and the branch is returned as +// the deliverable. PR/CI stay off by default — the caller owns merge and CI. +package issue + +import ( + "bytes" + "encoding/json" + "fmt" + "regexp" + "strings" +) + +// Spec ports issue/schemas.py::IssueSpec — the caller-provided, fully-scoped +// issue (extra="forbid", title and description required and non-empty). +type Spec struct { + Title string `json:"title"` + Description string `json:"description"` + Name string `json:"name"` + AcceptanceCriteria []string `json:"acceptance_criteria"` + FilesToCreate []string `json:"files_to_create"` + FilesToModify []string `json:"files_to_modify"` + TestingStrategy string `json:"testing_strategy"` + EstimatedComplexity string `json:"estimated_complexity"` + NeedsDeeperQA bool `json:"needs_deeper_qa"` +} + +// ParseSpec strict-decodes the issue map (mirrors IssueSpec.model_validate). +func ParseSpec(raw map[string]any) (*Spec, error) { + spec := Spec{EstimatedComplexity: "small"} + b, err := json.Marshal(raw) + if err != nil { + return nil, fmt.Errorf("issue: marshal spec: %w", err) + } + dec := json.NewDecoder(bytes.NewReader(b)) + dec.DisallowUnknownFields() + if err := dec.Decode(&spec); err != nil { + return nil, fmt.Errorf("issue: invalid issue spec: %w", err) + } + if strings.TrimSpace(spec.Title) == "" { + return nil, fmt.Errorf("issue: title must be a non-empty string") + } + if strings.TrimSpace(spec.Description) == "" { + return nil, fmt.Errorf("issue: description must be a non-empty string") + } + return &spec, nil +} + +var ( + slugNonAlnum = regexp.MustCompile(`[^0-9a-zA-Z]+`) + slugDashes = regexp.MustCompile(`-+`) +) + +// Slugify ports issue/schemas.py::slugify — a kebab-case slug safe for git +// branch names and file paths (max 40 chars, "issue" fallback). +func Slugify(text string) string { + slug := slugNonAlnum.ReplaceAllString(strings.ToLower(strings.TrimSpace(text)), "-") + slug = strings.Trim(slug, "-") + slug = slugDashes.ReplaceAllString(slug, "-") + if len(slug) > 40 { + slug = slug[:40] + } + slug = strings.Trim(slug, "-") + if slug == "" { + return "issue" + } + return slug +} + +// ToPlannedIssue maps the spec onto the internal PlannedIssue dict shape +// consumed by the coding loop (ports IssueSpec.to_planned_issue). +func (s *Spec) ToPlannedIssue(additionalContext string) map[string]any { + description := s.Description + if additionalContext != "" { + description = fmt.Sprintf( + "%s\n\n## Additional context from the caller\n\n%s", + description, additionalContext, + ) + } + name := s.Name + if name == "" { + name = s.Title + } + return map[string]any{ + "name": Slugify(name), + "title": s.Title, + "description": description, + "acceptance_criteria": toAnySlice(s.AcceptanceCriteria), + "depends_on": []any{}, + "provides": []any{}, + "estimated_complexity": s.EstimatedComplexity, + "files_to_create": toAnySlice(s.FilesToCreate), + "files_to_modify": toAnySlice(s.FilesToModify), + "testing_strategy": s.TestingStrategy, + "sequence_number": 1, + "guidance": map[string]any{ + "needs_new_tests": true, + "estimated_scope": s.EstimatedComplexity, + "touches_interfaces": false, + "needs_deeper_qa": s.NeedsDeeperQA, + "testing_guidance": s.TestingStrategy, + "review_focus": "", + "risk_rationale": "", + }, + "target_repo": "", + } +} + +func toAnySlice(in []string) []any { + out := make([]any, len(in)) + for i, v := range in { + out[i] = v + } + return out +} diff --git a/go/internal/issue/spec_test.go b/go/internal/issue/spec_test.go new file mode 100644 index 0000000..b310292 --- /dev/null +++ b/go/internal/issue/spec_test.go @@ -0,0 +1,86 @@ +package issue + +import ( + "strings" + "testing" +) + +func TestSlugify(t *testing.T) { + cases := []struct{ in, want string }{ + {"Add retry helper!", "add-retry-helper"}, + {" --Weird__ Name-- ", "weird-name"}, + {"!!!", "issue"}, + {strings.Repeat("x", 100), strings.Repeat("x", 40)}, + } + for _, c := range cases { + if got := Slugify(c.in); got != c.want { + t.Errorf("Slugify(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseSpecValidation(t *testing.T) { + if _, err := ParseSpec(map[string]any{"title": "", "description": "d"}); err == nil { + t.Error("empty title accepted") + } + if _, err := ParseSpec(map[string]any{"title": "t", "description": " "}); err == nil { + t.Error("blank description accepted") + } + if _, err := ParseSpec(map[string]any{"description": "d"}); err == nil { + t.Error("missing title accepted") + } + if _, err := ParseSpec(map[string]any{"title": "t", "description": "d", "surprise": 1}); err == nil { + t.Error("unknown field accepted (extra=forbid)") + } + spec, err := ParseSpec(map[string]any{"title": "t", "description": "d"}) + if err != nil { + t.Fatalf("valid spec rejected: %v", err) + } + if spec.EstimatedComplexity != "small" { + t.Errorf("EstimatedComplexity default = %q, want small", spec.EstimatedComplexity) + } +} + +func TestToPlannedIssue(t *testing.T) { + spec, err := ParseSpec(map[string]any{ + "title": "Add retry helper", + "description": "Backoff retry in utils.", + "acceptance_criteria": []any{"retries 3 times"}, + "testing_strategy": "pytest tests/test_utils.py", + "needs_deeper_qa": true, + }) + if err != nil { + t.Fatalf("ParseSpec: %v", err) + } + planned := spec.ToPlannedIssue("Use httpx, not requests.") + + if planned["name"] != "add-retry-helper" { + t.Errorf("name = %v", planned["name"]) + } + if planned["sequence_number"] != 1 { + t.Errorf("sequence_number = %v", planned["sequence_number"]) + } + desc, _ := planned["description"].(string) + if !strings.Contains(desc, "Backoff retry") || !strings.Contains(desc, "Use httpx") { + t.Errorf("description missing base or additional context: %q", desc) + } + guidance, _ := planned["guidance"].(map[string]any) + if guidance == nil || guidance["needs_deeper_qa"] != true { + t.Errorf("guidance.needs_deeper_qa = %v", guidance) + } + if guidance["testing_guidance"] != "pytest tests/test_utils.py" { + t.Errorf("guidance.testing_guidance = %v", guidance["testing_guidance"]) + } +} + +func TestToPlannedIssueSlugifiesExplicitName(t *testing.T) { + spec, err := ParseSpec(map[string]any{ + "title": "T", "description": "d", "name": "My Fancy Name!", + }) + if err != nil { + t.Fatalf("ParseSpec: %v", err) + } + if got := spec.ToPlannedIssue("")["name"]; got != "my-fancy-name" { + t.Errorf("name = %v, want my-fancy-name", got) + } +} diff --git a/go/internal/node/node_test.go b/go/internal/node/node_test.go index e4d7f3a..e8c685b 100644 --- a/go/internal/node/node_test.go +++ b/go/internal/node/node_test.go @@ -56,6 +56,10 @@ var pythonOrchestrators = []string{"build", "plan", "execute", "resolve", "resum // plus fast_plan_tasks / fast_execute_tasks / fast_verify. var pythonFastReasoners = []string{"build", "fast_plan_tasks", "fast_execute_tasks", "fast_verify"} +// pythonIssueReasoners is the issue-level entry point (swe_af/issue/build.py), +// registered on BOTH nodes via the shared issue_router. +var pythonIssueReasoners = []string{"implement_issue"} + func TestRegisterPlannerExactSurface(t *testing.T) { n, err := BuildAgent("swe-planner-go", "8005", "Autonomous SWE planning pipeline") if err != nil { @@ -63,8 +67,10 @@ func TestRegisterPlannerExactSurface(t *testing.T) { } n.RegisterPlanner() - // swe-planner-go surface = 25 roles + 5 orchestrators = 30 unique names. + // swe-planner-go surface = 25 roles + 5 orchestrators + implement_issue + // = 31 unique names. want := append(append([]string(nil), pythonRoleSurface...), pythonOrchestrators...) + want = append(want, pythonIssueReasoners...) assertSurface(t, "swe-planner-go", n.RegisteredNames(), want) } @@ -75,10 +81,12 @@ func TestRegisterFastExactSurface(t *testing.T) { } n.RegisterFast() - // swe-fast-go surface = 25 roles + 4 fast reasoners = 29 unique names. - // It must NOT contain plan/execute/resolve/resume_build (those live only on - // swe-planner-go) — assertSurface's extra-name check enforces that. + // swe-fast-go surface = 25 roles + 4 fast reasoners + implement_issue + // = 30 unique names. It must NOT contain plan/execute/resolve/resume_build + // (those live only on swe-planner-go) — assertSurface's extra-name check + // enforces that. want := append(append([]string(nil), pythonRoleSurface...), pythonFastReasoners...) + want = append(want, pythonIssueReasoners...) assertSurface(t, "swe-fast-go", n.RegisteredNames(), want) } diff --git a/go/internal/node/register.go b/go/internal/node/register.go index 2734eee..e1ab3d4 100644 --- a/go/internal/node/register.go +++ b/go/internal/node/register.go @@ -38,6 +38,7 @@ import ( "github.com/Agent-Field/SWE-AF/go/internal/roles/planning" "github.com/Agent-Field/SWE-AF/go/internal/fast" + "github.com/Agent-Field/SWE-AF/go/internal/issue" ) const ( @@ -46,18 +47,21 @@ const ( ) // RegisterPlanner registers the full swe-planner surface: 25 role reasoners + -// 5 orchestrators (30 total). Ports swe_af/app.py. +// 5 orchestrators + the issue-level entry point (31 total). Ports swe_af/app.py. func (n *Node) RegisterPlanner() { n.registerRoles() n.registerOrchestrators() + n.registerIssueReasoner() } // RegisterFast registers the swe-fast surface: the same 25 role reasoners + the -// 4 fast reasoners (29 total). Ports swe_af/fast/app.py. It deliberately does -// NOT register the orchestrators — fast/app.py only defines its own build. +// 4 fast reasoners + the issue-level entry point (30 total). Ports +// swe_af/fast/app.py. It deliberately does NOT register the orchestrators — +// fast/app.py only defines its own build. func (n *Node) RegisterFast() { n.registerRoles() n.registerFastReasoners() + n.registerIssueReasoner() } // --------------------------------------------------------------------------- @@ -174,6 +178,30 @@ func (n *Node) registerFastReasoners() { } } +// --------------------------------------------------------------------------- +// Issue-level entry point (both nodes) +// --------------------------------------------------------------------------- + +// registerIssueReasoner wires implement_issue — the sub-harness entry point a +// main coding harness delegates fully-scoped issues to. Python includes the +// swe-issue-tagged issue_router in BOTH apps; the Go port mirrors that on both +// nodes under the -go tag convention. +func (n *Node) registerIssueReasoner() { + deps := &issue.Deps{ + Call: newCallFn(n.App), + Note: n.App, + NodeID: n.NodeID, + } + tag := agent.WithReasonerTags("swe-issue-go") + for name, h := range issue.Handlers() { + opts := []agent.ReasonerOption{tag} + if s, ok := issueSchemas[name]; ok { + opts = append(opts, agent.WithInputSchema(s)) + } + regHandler(n, name, deps, h, opts...) + } +} + // --------------------------------------------------------------------------- // Registration helper // --------------------------------------------------------------------------- @@ -250,6 +278,15 @@ var orchestratorSchemas = map[string]json.RawMessage{ `"git_config":{"type":"object"}}}`), } +// issueSchemas maps the issue-level reasoner to its input schema. +var issueSchemas = map[string]json.RawMessage{ + // implement_issue(issue, repo_path, base_branch="", artifacts_dir=".artifacts", + // additional_context="", config=None) + "implement_issue": schema(`{"type":"object","additionalProperties":true,"required":["issue","repo_path"],"properties":{` + + `"issue":{"type":"object"},"repo_path":{"type":"string"},"base_branch":{"type":"string"},` + + `"artifacts_dir":{"type":"string"},"additional_context":{"type":"string"},"config":{"type":"object"}}}`), +} + // fastSchemas maps the 4 fast reasoner names to their input schemas. var fastSchemas = map[string]json.RawMessage{ // build(goal, repo_path="", repo_url="", artifacts_dir=".artifacts", diff --git a/swe_af/app.py b/swe_af/app.py index 3dbed3c..cd75c0a 100644 --- a/swe_af/app.py +++ b/swe_af/app.py @@ -17,6 +17,7 @@ load_dotenv() # surface HAX_API_KEY (and friends) before Agent() is constructed +from swe_af.issue import issue_router from swe_af.reasoners import router from swe_af.reasoners.pipeline import _assign_sequence_numbers, _compute_levels, _validate_file_conflicts from swe_af.reasoners.schemas import PlanResult, ReviewResult @@ -59,6 +60,7 @@ def __init__(self, message: str, *, result=None, error_details=None) -> None: ) app.include_router(router) +app.include_router(issue_router) # --------------------------------------------------------------------------- diff --git a/swe_af/fast/app.py b/swe_af/fast/app.py index cb1252e..a2cb0c3 100644 --- a/swe_af/fast/app.py +++ b/swe_af/fast/app.py @@ -39,6 +39,11 @@ from swe_af.reasoners import router as _execution_router # noqa: E402 app.include_router(_execution_router) +# Issue-level entry point (implement_issue) — shared with swe-planner so a +# main harness can delegate scoped issues to either node. +from swe_af.issue import issue_router as _issue_router # noqa: E402 +app.include_router(_issue_router) + def _repo_name_from_url(url: str) -> str: """Extract repo name from a GitHub URL.""" diff --git a/swe_af/issue/__init__.py b/swe_af/issue/__init__.py new file mode 100644 index 0000000..86f962e --- /dev/null +++ b/swe_af/issue/__init__.py @@ -0,0 +1,42 @@ +"""swe_af.issue — issue-level build entry point (sub-harness surface). + +Registers ``implement_issue`` on an AgentRouter that is included by BOTH node +apps (``swe_af.app`` / swe-planner and ``swe_af.fast.app`` / swe-fast), so a +main harness can delegate a fully-scoped issue to whichever node it is +pointed at: + + POST /api/v1/execute/async/swe-planner.implement_issue + POST /api/v1/execute/async/swe-fast.implement_issue + +Deliberately imports only ``swe_af.execution.*`` (schemas, coding loop, +envelope) — never ``swe_af.reasoners`` — so importing this package pulls in +no planning agents and no LLM role implementations. Role reasoners +(run_coder, run_code_reviewer, run_verifier, …) are reached at runtime via +``call_fn`` targets on the current node. +""" + +from __future__ import annotations + +from agentfield import AgentRouter +from swe_af.runtime.codex_harness_patch import apply_codex_harness_patch + +apply_codex_harness_patch() + +issue_router = AgentRouter(tags=["swe-issue"]) + +from . import git_ops # noqa: E402, F401 — deterministic git helpers +from . import build # noqa: E402, F401 — registers implement_issue +from .schemas import ( # noqa: E402 + IssueBuildConfig, + IssueBuildResult, + IssueSpec, +) + +__all__ = [ + "issue_router", + "build", + "git_ops", + "IssueBuildConfig", + "IssueBuildResult", + "IssueSpec", +] diff --git a/swe_af/issue/build.py b/swe_af/issue/build.py new file mode 100644 index 0000000..0564afe --- /dev/null +++ b/swe_af/issue/build.py @@ -0,0 +1,403 @@ +"""Issue-level build orchestrator — the sub-harness entry point. + +``implement_issue`` is the issue-level twin of ``build``: the caller +(typically a bigger coding harness such as Claude Code delegating a +well-scoped task) supplies a fully-scoped issue, so **no planning agents run +at all**. The flow is: + + deterministic git setup (worktree + branch off base, no LLM) + → run_coding_loop (coder → reviewer, or QA/reviewer/synthesizer) + → deterministic checkpoint commit of anything left uncommitted + → optional single verifier pass against the acceptance criteria + → optional GitHub PR (off by default — the caller owns merge/CI) + → worktree cleanup (the branch is the deliverable) + +Concurrent calls against the same ``repo_path`` are safe: every call gets its +own worktree and its own ``issue/-`` branch, and the caller's +checkout and current branch are never touched. +""" + +from __future__ import annotations + +import asyncio +import os +import uuid +from typing import Callable + +from swe_af.execution.coding_loop import run_coding_loop +from swe_af.execution.envelope import unwrap_call_result as _unwrap +from swe_af.execution.schemas import DAGState, ExecutionConfig, IssueOutcome +from swe_af.issue import git_ops, issue_router +from swe_af.issue.schemas import IssueBuildConfig, IssueBuildResult, IssueSpec + +_COMPLETED_OUTCOMES = ("completed", "completed_with_debt") + + +async def _run_verification( + *, + call_fn: Callable, + node_id: str, + spec: IssueSpec, + planned: dict, + worktree_path: str, + artifacts_dir: str, + exec_config: ExecutionConfig, + cfg: IssueBuildConfig, + loop_summary: str, + note: Callable, +) -> dict: + """One verifier pass. Unavailability reports passed=False, never success.""" + prd = { + "validated_description": f"{spec.title}\n\n{spec.description}", + "acceptance_criteria": list(spec.acceptance_criteria), + "must_have": list(spec.acceptance_criteria), + "nice_to_have": [], + "out_of_scope": [], + } + try: + result = await asyncio.wait_for( + call_fn( + f"{node_id}.run_verifier", + prd=prd, + repo_path=worktree_path, + artifacts_dir=artifacts_dir, + completed_issues=[ + {"issue_name": planned["name"], "result_summary": loop_summary} + ], + failed_issues=[], + skipped_issues=[], + model=exec_config.verifier_model, + permission_mode=cfg.permission_mode, + ai_provider=exec_config.ai_provider, + ), + timeout=cfg.agent_timeout_seconds, + ) + if isinstance(result, dict): + return result + return {"passed": False, "summary": f"Unexpected verifier output: {result!r}"} + except Exception as e: # noqa: BLE001 — verification must not sink the build + note( + f"Verifier unavailable: {e}", + tags=["issue_build", "verify", "error"], + ) + return {"passed": False, "summary": f"Verification unavailable: {e}"} + + +async def _maybe_create_pr( + *, + call_fn: Callable, + node_id: str, + cfg: IssueBuildConfig, + exec_config: ExecutionConfig, + spec: IssueSpec, + planned: dict, + repo_path: str, + worktree_path: str, + branch: str, + base_ref: str, + artifacts_dir: str, + loop_summary: str, + debt_items: list[dict], + note: Callable, +) -> str: + remote = await asyncio.to_thread(git_ops.remote_url, repo_path) + if not remote: + note( + "No origin remote; skipping PR creation", + tags=["issue_build", "github_pr", "skip"], + ) + return "" + base_for_pr = ( + cfg.github_pr_base + or await asyncio.to_thread(git_ops.default_remote_branch, repo_path) + or (base_ref if base_ref != "HEAD" else "main") + ) + try: + result = await asyncio.wait_for( + call_fn( + f"{node_id}.run_github_pr", + repo_path=worktree_path, + integration_branch=branch, + base_branch=base_for_pr, + goal=spec.title, + build_summary=loop_summary or spec.title, + completed_issues=[ + {"issue_name": planned["name"], "result_summary": loop_summary} + ], + accumulated_debt=debt_items, + artifacts_dir=artifacts_dir, + model=exec_config.git_model, + permission_mode=cfg.permission_mode, + ai_provider=exec_config.ai_provider, + ), + timeout=cfg.agent_timeout_seconds, + ) + return result.get("pr_url", "") if isinstance(result, dict) else "" + except Exception as e: # noqa: BLE001 — PR creation is best-effort + note( + f"PR creation failed (non-fatal): {e}", + tags=["issue_build", "github_pr", "error"], + ) + return "" + + +async def _implement_issue_impl( + *, + issue: dict, + repo_path: str, + base_branch: str, + artifacts_dir: str, + additional_context: str, + config: dict | None, + call_fn: Callable, + note_fn: Callable | None, + node_id: str, +) -> dict: + def note(message: str, tags: list[str] | None = None) -> None: + if note_fn: + note_fn(message, tags=tags or []) + + cfg = IssueBuildConfig(**(config or {})) + spec = IssueSpec.model_validate(issue) + planned = spec.to_planned_issue(additional_context=additional_context) + + # --- Setup (validation errors raise: they are the caller's to fix) ------ + repo_path = os.path.abspath(repo_path) + await asyncio.to_thread(git_ops.ensure_issue_ready_repo, repo_path) + base_ref, base_sha = await asyncio.to_thread( + git_ops.resolve_base, repo_path, base_branch + ) + excludes = [".worktrees/"] + if not os.path.isabs(artifacts_dir): + top = artifacts_dir.strip("/").split("/")[0] + if top and top not in (".", ".."): + excludes.append(f"{top}/") + await asyncio.to_thread(git_ops.ensure_local_excludes, repo_path, excludes) + if await asyncio.to_thread(git_ops.is_dirty, repo_path): + note( + "Caller repo has uncommitted changes; the issue branch is created " + "from the committed base state and will not see them", + tags=["issue_build", "warning", "dirty_tree"], + ) + + build_id = uuid.uuid4().hex[:8] + branch = f"{cfg.branch_prefix}{build_id}-{planned['name']}" + worktree_path = os.path.join( + repo_path, ".worktrees", f"{build_id}-{planned['name']}" + ) + if os.path.isabs(artifacts_dir): + abs_artifacts = os.path.join(artifacts_dir, "issue-builds", build_id) + else: + abs_artifacts = os.path.join(repo_path, artifacts_dir, "issue-builds", build_id) + os.makedirs(abs_artifacts, exist_ok=True) + + await asyncio.to_thread( + git_ops.add_worktree, repo_path, worktree_path, branch, base_sha + ) + note( + f"Issue build {build_id}: {planned['name']} on {branch} " + f"(base {base_ref} @ {base_sha[:12]})", + tags=["issue_build", "start"], + ) + + planned["worktree_path"] = worktree_path + planned["branch_name"] = branch + + exec_config = ExecutionConfig( + runtime=cfg.runtime, + models=cfg.models, + max_coding_iterations=cfg.max_coding_iterations, + agent_max_turns=cfg.agent_max_turns, + agent_timeout_seconds=cfg.agent_timeout_seconds, + permission_mode=cfg.permission_mode, + enable_learning=False, + enable_replanning=False, + enable_issue_advisor=False, + enable_integration_testing=False, + check_ci=False, + ) + dag_state = DAGState( + repo_path=worktree_path, + artifacts_dir=abs_artifacts, + build_id=build_id, + all_issues=[planned], + ) + + # --- Execute (failures after branch creation return a structured result) - + outcome_value = "error" + loop_summary = "" + error_message = "" + iterations = 0 + iteration_history: list[dict] = [] + debt_items: list[dict] = [] + verification: dict | None = None + pr_url = "" + commits: list[str] = [] + files_changed: list[str] = [] + stat = "" + + try: + loop_result = await run_coding_loop( + issue=planned, + dag_state=dag_state, + call_fn=call_fn, + node_id=node_id, + config=exec_config, + note_fn=note_fn, + memory_fn=None, + ) + outcome_value = loop_result.outcome.value + loop_summary = loop_result.result_summary or loop_result.error_message + error_message = loop_result.error_message + iterations = loop_result.attempts + iteration_history = loop_result.iteration_history + debt_items = loop_result.debt_items + + await asyncio.to_thread( + git_ops.scrub_tracked_junk, worktree_path, planned["name"] + ) + await asyncio.to_thread( + git_ops.commit_all, + worktree_path, + f"chore({planned['name']}): checkpoint uncommitted issue work", + ) + commits = await asyncio.to_thread( + git_ops.new_commits, repo_path, base_sha, branch + ) + files_changed = await asyncio.to_thread( + git_ops.changed_files, repo_path, base_sha, branch + ) + stat = await asyncio.to_thread(git_ops.diff_stat, repo_path, base_sha, branch) + + coding_ok = loop_result.outcome in ( + IssueOutcome.COMPLETED, + IssueOutcome.COMPLETED_WITH_DEBT, + ) + if cfg.verify and coding_ok and commits: + verification = await _run_verification( + call_fn=call_fn, + node_id=node_id, + spec=spec, + planned=planned, + worktree_path=worktree_path, + artifacts_dir=abs_artifacts, + exec_config=exec_config, + cfg=cfg, + loop_summary=loop_summary, + note=note, + ) + if cfg.enable_github_pr and coding_ok and commits: + pr_url = await _maybe_create_pr( + call_fn=call_fn, + node_id=node_id, + cfg=cfg, + exec_config=exec_config, + spec=spec, + planned=planned, + repo_path=repo_path, + worktree_path=worktree_path, + branch=branch, + base_ref=base_ref, + artifacts_dir=abs_artifacts, + loop_summary=loop_summary, + debt_items=debt_items, + note=note, + ) + except Exception as e: # noqa: BLE001 — surface as a structured failure + error_message = error_message or str(e) + note(f"Issue build failed: {e}", tags=["issue_build", "error"]) + # Salvage whatever the coder committed before the failure. + commits = await asyncio.to_thread( + git_ops.new_commits, repo_path, base_sha, branch + ) + files_changed = await asyncio.to_thread( + git_ops.changed_files, repo_path, base_sha, branch + ) + stat = await asyncio.to_thread(git_ops.diff_stat, repo_path, base_sha, branch) + finally: + if not cfg.keep_worktree: + await asyncio.to_thread(git_ops.remove_worktree, repo_path, worktree_path) + if not commits: + # A branch with zero commits is pure noise for the caller. + await asyncio.to_thread(git_ops.delete_branch, repo_path, branch) + + coding_ok = outcome_value in _COMPLETED_OUTCOMES + verify_ok = True if verification is None else bool(verification.get("passed")) + success = coding_ok and bool(commits) and verify_ok + + summary = ( + f"{planned['name']}: {outcome_value}, {len(commits)} commit(s), " + f"{len(files_changed)} file(s) changed" + ) + summary += f" on {branch}" if commits else "; no commits produced" + if verification is not None: + summary += ( + "; verification passed" + if verification.get("passed") + else "; verification FAILED" + ) + if loop_summary: + summary += f" — {loop_summary}" + + note(summary, tags=["issue_build", "complete" if success else "failed"]) + + return IssueBuildResult( + success=success, + outcome=outcome_value, + summary=summary, + build_id=build_id, + branch=branch if commits else "", + base_branch=base_ref, + base_sha=base_sha, + commits=commits, + files_changed=files_changed, + diff_stat=stat, + iterations=iterations, + iteration_history=iteration_history, + debt_items=debt_items, + verification=verification, + pr_url=pr_url, + error_message="" if success else error_message, + ).model_dump() + + +@issue_router.reasoner() +async def implement_issue( + issue: dict, + repo_path: str, + base_branch: str = "", + artifacts_dir: str = ".artifacts", + additional_context: str = "", + config: dict | None = None, +) -> dict: + """Implement one fully-scoped issue on an isolated branch (sub-harness entry). + + Args: + issue: IssueSpec dict — title + description required; optionally + acceptance_criteria, files_to_create/modify, testing_strategy, + needs_deeper_qa, estimated_complexity, name. + repo_path: Existing local checkout (must have at least one commit). + base_branch: Base ref for the issue branch (default: current branch). + artifacts_dir: Where build artifacts go (relative to repo_path unless + absolute). + additional_context: Extra caller context appended to the description. + config: IssueBuildConfig overrides as a dict. + + Returns an IssueBuildResult dict; ``branch`` carries the deliverable. + """ + agent = issue_router.app + + async def call_fn(target: str, **kwargs): + return _unwrap(await agent.call(target, **kwargs), target) + + return await _implement_issue_impl( + issue=issue, + repo_path=repo_path, + base_branch=base_branch, + artifacts_dir=artifacts_dir, + additional_context=additional_context, + config=config, + call_fn=call_fn, + note_fn=agent.note, + node_id=agent.node_id, + ) diff --git a/swe_af/issue/git_ops.py b/swe_af/issue/git_ops.py new file mode 100644 index 0000000..ff2d21a --- /dev/null +++ b/swe_af/issue/git_ops.py @@ -0,0 +1,227 @@ +"""Deterministic (non-LLM) git operations for the issue-level build path. + +The full pipeline delegates git work to LLM agents (run_git_init, run_merger, +run_workspace_cleanup) because it juggles many branches and conflict +resolution. The issue-level path owns exactly one worktree + one branch off a +known base, so plain git commands are sufficient — and keep the LLM budget +for coding. + +All functions are synchronous; callers wrap them in ``asyncio.to_thread``. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import time + + +class GitOpsError(RuntimeError): + """A git operation failed in a way the caller must handle.""" + + +def _git(repo_path: str, *args: str, check: bool = True) -> subprocess.CompletedProcess: + proc = subprocess.run( + ["git", "-C", repo_path, *args], + capture_output=True, + text=True, + ) + if check and proc.returncode != 0: + detail = proc.stderr.strip() or proc.stdout.strip() or f"exit {proc.returncode}" + raise GitOpsError(f"git {' '.join(args)} failed: {detail}") + return proc + + +def ensure_issue_ready_repo(repo_path: str) -> None: + """Validate that repo_path is a git repository with at least one commit.""" + if not os.path.isdir(repo_path): + raise GitOpsError(f"repo_path does not exist or is not a directory: {repo_path}") + if _git(repo_path, "rev-parse", "--git-dir", check=False).returncode != 0: + raise GitOpsError(f"repo_path is not a git repository: {repo_path}") + if _git(repo_path, "rev-parse", "--verify", "HEAD", check=False).returncode != 0: + raise GitOpsError( + "repository has no commits; issue-level builds require an existing " + "base commit (use the feature-level build for empty repos)" + ) + + +def current_branch(repo_path: str) -> str: + """Current branch name, or "HEAD" when detached.""" + return _git(repo_path, "rev-parse", "--abbrev-ref", "HEAD").stdout.strip() + + +def resolve_base(repo_path: str, base_branch: str = "") -> tuple[str, str]: + """Resolve the base ref for the issue branch. + + Returns (base_ref, base_sha). Falls back to the caller's current branch + (or literal "HEAD" when detached) so "work off what's checked out" is the + zero-config behaviour. + """ + ref = base_branch or current_branch(repo_path) + proc = _git(repo_path, "rev-parse", "--verify", f"{ref}^{{commit}}", check=False) + if proc.returncode != 0: + raise GitOpsError(f"base branch/ref not found: {ref}") + return ref, proc.stdout.strip() + + +def is_dirty(repo_path: str) -> bool: + return bool(_git(repo_path, "status", "--porcelain").stdout.strip()) + + +def add_worktree( + repo_path: str, + worktree_path: str, + branch: str, + base_sha: str, + attempts: int = 3, +) -> None: + """Create an isolated worktree on a new branch off base_sha. + + Retries a few times because concurrent ``git worktree add`` calls against + the same repository can briefly contend on the repo lock — the exact + scenario a fan-out caller creates. + """ + os.makedirs(os.path.dirname(worktree_path), exist_ok=True) + last_detail = "" + for attempt in range(1, attempts + 1): + proc = _git( + repo_path, "worktree", "add", "-b", branch, worktree_path, base_sha, + check=False, + ) + if proc.returncode == 0: + return + last_detail = proc.stderr.strip() or proc.stdout.strip() + if attempt < attempts: + time.sleep(0.5 * attempt) + raise GitOpsError(f"git worktree add failed after {attempts} attempts: {last_detail}") + + +def _has_commit_identity(repo_path: str) -> bool: + return bool(_git(repo_path, "config", "user.email", check=False).stdout.strip()) + + +# Bytecode/cache junk that must never be versioned on the issue branch. The +# coder runs tests inside the worktree, so these appear as a side effect and +# an indiscriminate `git add` (ours or the coder's) would sweep them in. +_JUNK_PATHSPECS: tuple[str, ...] = ("*__pycache__*", "*.pyc", "*.pyo") + + +def _commit_index(worktree_path: str, message: str) -> str: + """Commit whatever is staged. Returns the sha, or "" when index is clean.""" + identity: list[str] = [] + if not _has_commit_identity(worktree_path): + identity = [ + "-c", "user.name=SWE-AF", + "-c", "user.email=swe-af@agentfield.local", + ] + proc = _git(worktree_path, *identity, "commit", "-m", message, check=False) + if proc.returncode != 0: + out = f"{proc.stdout}\n{proc.stderr}".lower() + if "nothing to commit" in out or "nothing added to commit" in out: + return "" + detail = proc.stderr.strip() or proc.stdout.strip() + raise GitOpsError(f"git commit failed: {detail}") + return _git(worktree_path, "rev-parse", "HEAD").stdout.strip() + + +def scrub_tracked_junk(worktree_path: str, issue_name: str) -> str: + """Untrack bytecode junk an agent committed on the issue branch. + + Returns the scrub commit sha, or "" when the branch was already clean. + History stays append-only: the agent's commit is not rewritten. + """ + listed = _git(worktree_path, "ls-files", "--", *_JUNK_PATHSPECS, check=False) + if listed.returncode != 0 or not listed.stdout.strip(): + return "" + _git( + worktree_path, "rm", "-r", "--cached", "-q", "--ignore-unmatch", + "--", *_JUNK_PATHSPECS, + ) + return _commit_index( + worktree_path, f"chore({issue_name}): untrack bytecode caches" + ) + + +def commit_all(worktree_path: str, message: str) -> str: + """Commit any uncommitted changes inside the (isolated) worktree. + + ``add -A`` is safe here precisely because the worktree belongs to this + build alone; bytecode junk is excluded. Returns the new commit sha, or + "" when there was nothing to commit. + """ + if not is_dirty(worktree_path): + return "" + excludes = [f":(exclude){p}" for p in _JUNK_PATHSPECS] + _git(worktree_path, "add", "-A", "--", ".", *excludes) + return _commit_index(worktree_path, message) + + +def new_commits(repo_path: str, base_sha: str, branch: str) -> list[str]: + """Commits on branch since base_sha, oldest first. [] if branch is gone.""" + proc = _git(repo_path, "rev-list", "--reverse", f"{base_sha}..{branch}", check=False) + if proc.returncode != 0: + return [] + return [line for line in proc.stdout.splitlines() if line.strip()] + + +def changed_files(repo_path: str, base_sha: str, branch: str) -> list[str]: + proc = _git(repo_path, "diff", "--name-only", f"{base_sha}..{branch}", check=False) + if proc.returncode != 0: + return [] + return [line for line in proc.stdout.splitlines() if line.strip()] + + +def diff_stat(repo_path: str, base_sha: str, branch: str) -> str: + proc = _git(repo_path, "diff", "--stat", f"{base_sha}..{branch}", check=False) + return proc.stdout.strip() if proc.returncode == 0 else "" + + +def remove_worktree(repo_path: str, worktree_path: str) -> None: + """Remove the worktree (the branch survives). Best-effort, never raises.""" + proc = _git(repo_path, "worktree", "remove", "--force", worktree_path, check=False) + if proc.returncode != 0 and os.path.isdir(worktree_path): + shutil.rmtree(worktree_path, ignore_errors=True) + _git(repo_path, "worktree", "prune", check=False) + + +def delete_branch(repo_path: str, branch: str) -> None: + """Delete a branch (used only when it holds no commits). Best-effort.""" + _git(repo_path, "branch", "-D", branch, check=False) + + +def ensure_local_excludes(repo_path: str, patterns: list[str]) -> None: + """Add patterns to the repo-local ignore file (``.git/info/exclude``). + + Keeps issue-build bookkeeping (.artifacts, .worktrees) out of the + caller's ``git status`` without touching the working tree — unlike + ``.gitignore``, this file is not versioned and never shows in a diff. + """ + git_dir = _git(repo_path, "rev-parse", "--git-dir").stdout.strip() + if not os.path.isabs(git_dir): + git_dir = os.path.join(repo_path, git_dir) + exclude_path = os.path.join(git_dir, "info", "exclude") + os.makedirs(os.path.dirname(exclude_path), exist_ok=True) + existing: set[str] = set() + if os.path.exists(exclude_path): + with open(exclude_path, "r", encoding="utf-8") as f: + existing = {line.strip() for line in f} + to_add = [p for p in patterns if p not in existing] + if to_add: + with open(exclude_path, "a", encoding="utf-8") as f: + f.write("\n".join(to_add) + "\n") + + +def remote_url(repo_path: str) -> str: + proc = _git(repo_path, "remote", "get-url", "origin", check=False) + return proc.stdout.strip() if proc.returncode == 0 else "" + + +def default_remote_branch(repo_path: str) -> str: + """Default branch of origin (e.g. "main"), or "" when unknown.""" + proc = _git(repo_path, "symbolic-ref", "refs/remotes/origin/HEAD", check=False) + if proc.returncode != 0: + return "" + ref = proc.stdout.strip() + prefix = "refs/remotes/origin/" + return ref[len(prefix):] if ref.startswith(prefix) else "" diff --git a/swe_af/issue/schemas.py b/swe_af/issue/schemas.py new file mode 100644 index 0000000..c93f5ac --- /dev/null +++ b/swe_af/issue/schemas.py @@ -0,0 +1,162 @@ +"""Pydantic schemas for the issue-level build entry point (``implement_issue``). + +The caller — typically a bigger coding harness (Claude Code, Codex, OpenCode) +delegating a well-scoped task — owns planning. ``IssueSpec`` is the wire shape +it sends; ``to_planned_issue()`` maps it 1:1 onto the internal ``PlannedIssue`` +dict the coding loop consumes, so no planning agents run at all. +""" + +from __future__ import annotations + +import re +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from swe_af.execution.schemas import ( + ROLE_TO_MODEL_FIELD, + _default_runtime, +) + +# Roles the issue-level path can invoke. "git" covers the optional PR step. +ISSUE_MODEL_ROLE_KEYS: tuple[str, ...] = ( + "coder", + "code_reviewer", + "qa", + "qa_synthesizer", + "verifier", + "git", +) +_ALLOWED_ISSUE_MODEL_KEYS: set[str] = set(ISSUE_MODEL_ROLE_KEYS) | {"default"} + +ISSUE_MODEL_FIELDS: list[str] = [ + ROLE_TO_MODEL_FIELD[role] for role in ISSUE_MODEL_ROLE_KEYS +] + + +def slugify(text: str, max_length: int = 40) -> str: + """Kebab-case slug safe for git branch names and file paths.""" + slug = re.sub(r"[^0-9a-zA-Z]+", "-", text.strip().lower()).strip("-") + slug = re.sub(r"-+", "-", slug)[:max_length].strip("-") + return slug or "issue" + + +class IssueSpec(BaseModel): + """A caller-provided, fully-scoped issue for ``implement_issue``.""" + + model_config = ConfigDict(extra="forbid") + + title: str + description: str + name: str = "" # kebab-case slug; derived from title when empty + acceptance_criteria: list[str] = [] + files_to_create: list[str] = [] + files_to_modify: list[str] = [] + testing_strategy: str = "" + estimated_complexity: str = "small" + needs_deeper_qa: bool = False # True => coder → QA + reviewer → synthesizer + + @field_validator("title", "description") + @classmethod + def _non_empty(cls, v: str) -> str: + if not v.strip(): + raise ValueError("must be a non-empty string") + return v + + def to_planned_issue(self, additional_context: str = "") -> dict: + """Map onto the internal ``PlannedIssue`` dict shape (see + ``swe_af.reasoners.schemas.PlannedIssue``) consumed by the coding loop. + + Built as a plain dict on purpose: importing ``swe_af.reasoners`` + would load every planning/execution agent module into the process. + """ + description = self.description + if additional_context: + description = ( + f"{description}\n\n## Additional context from the caller\n\n" + f"{additional_context}" + ) + return { + "name": slugify(self.name or self.title), + "title": self.title, + "description": description, + "acceptance_criteria": list(self.acceptance_criteria), + "depends_on": [], + "provides": [], + "estimated_complexity": self.estimated_complexity, + "files_to_create": list(self.files_to_create), + "files_to_modify": list(self.files_to_modify), + "testing_strategy": self.testing_strategy, + "sequence_number": 1, + "guidance": { + "needs_new_tests": True, + "estimated_scope": self.estimated_complexity, + "touches_interfaces": False, + "needs_deeper_qa": self.needs_deeper_qa, + "testing_guidance": self.testing_strategy, + "review_focus": "", + "risk_rationale": "", + }, + "target_repo": "", + } + + +class IssueBuildConfig(BaseModel): + """Configuration for a single issue-level build run.""" + + model_config = ConfigDict(extra="forbid") + + runtime: Literal["claude_code", "open_code", "codex"] = Field( + default_factory=_default_runtime + ) + models: dict[str, str] | None = None + max_coding_iterations: int = 3 + agent_max_turns: int = 50 + agent_timeout_seconds: int = 1800 + permission_mode: str = "" + verify: bool = True # single verifier pass against the acceptance criteria + enable_github_pr: bool = False # the caller owns merge/PR/CI by default + github_pr_base: str = "" + branch_prefix: str = "issue/" + keep_worktree: bool = False # leave the worktree in place for debugging + + @field_validator("models") + @classmethod + def _validate_issue_model_keys( + cls, v: dict[str, str] | None + ) -> dict[str, str] | None: + if v is None: + return v + unknown = sorted(k for k in v if k not in _ALLOWED_ISSUE_MODEL_KEYS) + if unknown: + raise ValueError( + f"Unknown model keys for implement_issue: " + f"{', '.join(repr(k) for k in unknown)}. " + f"Valid keys: {', '.join(sorted(_ALLOWED_ISSUE_MODEL_KEYS))}" + ) + return v + + +class IssueBuildResult(BaseModel): + """Top-level result returned by ``implement_issue``. + + The branch is the deliverable: the caller merges (or discards) it. The + worktree used to produce it is removed unless ``keep_worktree`` was set. + """ + + success: bool + outcome: str # IssueOutcome value, or "error" for unexpected failures + summary: str + build_id: str = "" + branch: str = "" # empty when no commits were produced (branch deleted) + base_branch: str = "" + base_sha: str = "" + commits: list[str] = [] + files_changed: list[str] = [] + diff_stat: str = "" + iterations: int = 0 + iteration_history: list[dict] = [] + debt_items: list[dict] = [] + verification: dict | None = None + pr_url: str = "" + error_message: str = "" diff --git a/tests/issue/__init__.py b/tests/issue/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/issue/conftest.py b/tests/issue/conftest.py new file mode 100644 index 0000000..7fc6963 --- /dev/null +++ b/tests/issue/conftest.py @@ -0,0 +1,116 @@ +"""Shared fixtures for the issue-level build tests. + +Everything is real except ``call_fn``: tests run against a real temp git +repository, real worktrees, and the real coding loop — only the AI agent +dispatch is scripted (same philosophy as tests/test_coding_loop.py). +""" + +from __future__ import annotations + +import os +import subprocess + +import pytest + +os.environ.setdefault("AGENTFIELD_SERVER", "http://localhost:9999") + + +def run_git(cwd: str, *args: str) -> str: + proc = subprocess.run( + ["git", "-C", cwd, *args], + check=True, + capture_output=True, + text=True, + ) + return proc.stdout.strip() + + +@pytest.fixture +def git_repo(tmp_path) -> str: + """A real git repository on branch ``main`` with one commit.""" + repo = tmp_path / "repo" + repo.mkdir() + repo_s = str(repo) + subprocess.run(["git", "init", "-q"], cwd=repo_s, check=True) + run_git(repo_s, "checkout", "-q", "-b", "main") + run_git(repo_s, "config", "user.email", "test@example.com") + run_git(repo_s, "config", "user.name", "Test") + (repo / "README.md").write_text("# Test repo\n") + run_git(repo_s, "add", "README.md") + run_git(repo_s, "commit", "-q", "-m", "initial commit") + return repo_s + + +PLANNING_TARGETS = { + "run_product_manager", + "run_architect", + "run_tech_lead", + "run_sprint_planner", + "run_issue_writer", + "run_environment_scout", +} + + +def make_call_fn( + recorded: list, + *, + coder_commits: bool = True, + coder_writes: bool = True, + reviewer_responses: list[dict] | None = None, + verifier_response: dict | None = None, + coder_exception: Exception | None = None, +): + """Build a scripted async ``call_fn`` that mimics the role reasoners. + + The fake coder writes (and by default commits) one file per iteration in + the worktree it was pointed at, which is exactly what the real coder role + does. ``reviewer_responses`` is consumed one per iteration; the last entry + repeats once exhausted. + """ + reviewer_responses = reviewer_responses or [ + {"approved": True, "blocking": False, "summary": "LGTM"} + ] + review_calls = {"n": 0} + + async def call_fn(target: str, **kwargs): + recorded.append((target, kwargs)) + name = target.split(".", 1)[1] + + if name == "run_coder": + if coder_exception is not None: + raise coder_exception + worktree = kwargs["worktree_path"] + iteration = kwargs.get("iteration", 1) + if coder_writes: + path = os.path.join(worktree, "feature.py") + with open(path, "w") as f: + f.write(f"VALUE = {iteration}\n") + if coder_commits: + run_git(worktree, "add", "feature.py") + run_git(worktree, "commit", "-q", "-m", f"feat: iteration {iteration}") + return { + "files_changed": ["feature.py"] if coder_writes else [], + "summary": f"iteration {iteration}", + "complete": True, + } + + if name == "run_code_reviewer": + idx = min(review_calls["n"], len(reviewer_responses) - 1) + review_calls["n"] += 1 + return dict(reviewer_responses[idx]) + + if name == "run_qa": + return {"passed": True, "summary": "qa ok", "test_failures": []} + + if name == "run_qa_synthesizer": + return {"action": "approve", "summary": "synth ok"} + + if name == "run_verifier": + return dict( + verifier_response + or {"passed": True, "criteria_results": [], "summary": "verified"} + ) + + raise AssertionError(f"unexpected call target: {target}") + + return call_fn diff --git a/tests/issue/test_git_ops.py b/tests/issue/test_git_ops.py new file mode 100644 index 0000000..bbefaac --- /dev/null +++ b/tests/issue/test_git_ops.py @@ -0,0 +1,161 @@ +"""Tests for the deterministic git helpers used by the issue-level build.""" + +from __future__ import annotations + +import os +import subprocess + +import pytest + +from swe_af.issue import git_ops +from tests.issue.conftest import run_git + + +class TestEnsureIssueReadyRepo: + def test_ok(self, git_repo: str) -> None: + git_ops.ensure_issue_ready_repo(git_repo) # must not raise + + def test_missing_dir(self, tmp_path) -> None: + with pytest.raises(git_ops.GitOpsError, match="does not exist"): + git_ops.ensure_issue_ready_repo(str(tmp_path / "nope")) + + def test_not_a_repo(self, tmp_path) -> None: + with pytest.raises(git_ops.GitOpsError, match="not a git repository"): + git_ops.ensure_issue_ready_repo(str(tmp_path)) + + def test_no_commits(self, tmp_path) -> None: + empty = tmp_path / "empty" + empty.mkdir() + subprocess.run(["git", "init", "-q"], cwd=str(empty), check=True) + with pytest.raises(git_ops.GitOpsError, match="no commits"): + git_ops.ensure_issue_ready_repo(str(empty)) + + +class TestResolveBase: + def test_defaults_to_current_branch(self, git_repo: str) -> None: + ref, sha = git_ops.resolve_base(git_repo) + assert ref == "main" + assert sha == run_git(git_repo, "rev-parse", "HEAD") + + def test_named_ref(self, git_repo: str) -> None: + run_git(git_repo, "branch", "other") + ref, sha = git_ops.resolve_base(git_repo, "other") + assert ref == "other" + assert len(sha) == 40 + + def test_missing_ref(self, git_repo: str) -> None: + with pytest.raises(git_ops.GitOpsError, match="base branch"): + git_ops.resolve_base(git_repo, "ghost") + + +class TestWorktreeLifecycle: + def test_add_commit_inspect_remove(self, git_repo: str) -> None: + _, base_sha = git_ops.resolve_base(git_repo) + worktree = os.path.join(git_repo, ".worktrees", "wt1") + + git_ops.add_worktree(git_repo, worktree, "issue/test-1", base_sha) + assert os.path.isdir(worktree) + + # Nothing to commit yet. + assert git_ops.commit_all(worktree, "noop") == "" + + with open(os.path.join(worktree, "new.txt"), "w") as f: + f.write("hi\n") + sha = git_ops.commit_all(worktree, "add new.txt") + assert len(sha) == 40 + + assert git_ops.new_commits(git_repo, base_sha, "issue/test-1") == [sha] + assert git_ops.changed_files(git_repo, base_sha, "issue/test-1") == ["new.txt"] + assert "new.txt" in git_ops.diff_stat(git_repo, base_sha, "issue/test-1") + + git_ops.remove_worktree(git_repo, worktree) + assert not os.path.isdir(worktree) + # Branch survives worktree removal — it is the deliverable. + assert run_git(git_repo, "rev-parse", "issue/test-1") == sha + + def test_commit_identity_fallback(self, git_repo: str, monkeypatch) -> None: + _, base_sha = git_ops.resolve_base(git_repo) + worktree = os.path.join(git_repo, ".worktrees", "wt2") + git_ops.add_worktree(git_repo, worktree, "issue/test-2", base_sha) + with open(os.path.join(worktree, "x.txt"), "w") as f: + f.write("x\n") + + monkeypatch.setattr(git_ops, "_has_commit_identity", lambda _: False) + sha = git_ops.commit_all(worktree, "identity fallback") + author = run_git(worktree, "log", "-1", "--format=%ae") + assert sha + assert author == "swe-af@agentfield.local" + + def test_delete_branch_and_missing_branch_queries(self, git_repo: str) -> None: + _, base_sha = git_ops.resolve_base(git_repo) + run_git(git_repo, "branch", "issue/tmp") + git_ops.delete_branch(git_repo, "issue/tmp") + assert run_git(git_repo, "branch", "--list", "issue/tmp") == "" + # Queries against a deleted branch degrade to empty results. + assert git_ops.new_commits(git_repo, base_sha, "issue/tmp") == [] + assert git_ops.changed_files(git_repo, base_sha, "issue/tmp") == [] + + +class TestJunkHygiene: + def _worktree_with_junk(self, git_repo: str, *, commit_junk: bool) -> str: + _, base_sha = git_ops.resolve_base(git_repo) + worktree = os.path.join(git_repo, ".worktrees", "wt-junk") + git_ops.add_worktree(git_repo, worktree, "issue/junk", base_sha) + pycache = os.path.join(worktree, "pkg", "__pycache__") + os.makedirs(pycache) + with open(os.path.join(pycache, "mod.cpython-312.pyc"), "wb") as f: + f.write(b"\x00") + with open(os.path.join(worktree, "real.py"), "w") as f: + f.write("x = 1\n") + if commit_junk: + run_git(worktree, "add", "-A", ".") + run_git(worktree, "commit", "-q", "-m", "agent committed junk") + return worktree + + def test_commit_all_excludes_bytecode_junk(self, git_repo: str) -> None: + worktree = self._worktree_with_junk(git_repo, commit_junk=False) + sha = git_ops.commit_all(worktree, "checkpoint") + assert sha + tracked = run_git(worktree, "ls-files") + assert "real.py" in tracked + assert "__pycache__" not in tracked + + def test_scrub_untracks_agent_committed_junk(self, git_repo: str) -> None: + worktree = self._worktree_with_junk(git_repo, commit_junk=True) + assert "__pycache__" in run_git(worktree, "ls-files") + sha = git_ops.scrub_tracked_junk(worktree, "junk-issue") + assert sha + assert "__pycache__" not in run_git(worktree, "ls-files") + # Idempotent: nothing left to scrub. + assert git_ops.scrub_tracked_junk(worktree, "junk-issue") == "" + + +class TestLocalExcludes: + def test_adds_patterns_and_masks_status(self, git_repo: str) -> None: + os.makedirs(os.path.join(git_repo, ".artifacts"), exist_ok=True) + with open(os.path.join(git_repo, ".artifacts", "x.json"), "w") as f: + f.write("{}") + git_ops.ensure_local_excludes(git_repo, [".artifacts/", ".worktrees/"]) + assert run_git(git_repo, "status", "--porcelain") == "" + + def test_idempotent(self, git_repo: str) -> None: + git_ops.ensure_local_excludes(git_repo, [".artifacts/"]) + git_ops.ensure_local_excludes(git_repo, [".artifacts/"]) + exclude = os.path.join(git_repo, ".git", "info", "exclude") + with open(exclude) as f: + lines = [line.strip() for line in f if line.strip()] + assert lines.count(".artifacts/") == 1 + + +class TestRemotes: + def test_no_remote(self, git_repo: str) -> None: + assert git_ops.remote_url(git_repo) == "" + assert git_ops.default_remote_branch(git_repo) == "" + + def test_with_remote(self, git_repo: str, tmp_path) -> None: + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "-q", "--bare", str(origin)], check=True + ) + run_git(git_repo, "remote", "add", "origin", str(origin)) + assert git_ops.remote_url(git_repo) == str(origin) diff --git a/tests/issue/test_implement_issue.py b/tests/issue/test_implement_issue.py new file mode 100644 index 0000000..2fe3c88 --- /dev/null +++ b/tests/issue/test_implement_issue.py @@ -0,0 +1,368 @@ +"""Contract tests for the issue-level build orchestrator. + +Validation contract (each test names the item it covers): + + C1. Given a checkout and a scoped issue, the result branch contains commits + implementing it; the caller's working tree and current branch are + untouched. + C2. N concurrent calls on the same repo_path yield N independent branches. + C3. No planning roles are ever invoked; total agent calls stay within the + configured budget. + C4. Default config pushes nothing anywhere and opens no PR. + C5. Verifier failure surfaces (success=False) instead of claiming success. + C6. Iteration exhaustion / blocking reviews surface debt or failure. + +Everything is real except the scripted ``call_fn`` (see conftest). +""" + +from __future__ import annotations + +import asyncio +import os + +import pytest + +from swe_af.issue.build import _implement_issue_impl +from swe_af.issue.git_ops import GitOpsError +from tests.issue.conftest import PLANNING_TARGETS, make_call_fn, run_git + +ISSUE = { + "title": "Add retry helper", + "description": "Add a retry helper with exponential backoff to utils.", + "acceptance_criteria": ["retry() retries up to 3 times"], + "files_to_create": ["feature.py"], +} + + +def _run(git_repo: str, call_fn, *, issue: dict | None = None, config: dict | None = None, **kwargs) -> dict: + return asyncio.run( + _implement_issue_impl( + issue=issue or ISSUE, + repo_path=git_repo, + base_branch=kwargs.pop("base_branch", ""), + artifacts_dir=kwargs.pop("artifacts_dir", ".artifacts"), + additional_context=kwargs.pop("additional_context", ""), + config=config, + call_fn=call_fn, + note_fn=kwargs.pop("note_fn", None), + node_id="test-node", + **kwargs, + ) + ) + + +class TestHappyPath: + """C1 — branch with commits, caller untouched.""" + + def test_creates_branch_and_leaves_caller_untouched(self, git_repo: str) -> None: + head_before = run_git(git_repo, "rev-parse", "HEAD") + recorded: list = [] + result = _run(git_repo, make_call_fn(recorded)) + + assert result["success"] is True + assert result["outcome"] == "completed" + assert result["branch"].startswith("issue/") + assert len(result["commits"]) == 1 + assert "feature.py" in result["files_changed"] + assert result["base_branch"] == "main" + assert result["verification"] is not None + assert result["verification"]["passed"] is True + assert result["error_message"] == "" + + # Caller repo untouched: same branch, same HEAD, clean status (the + # artifacts dir exists but is masked via .git/info/exclude). + assert run_git(git_repo, "rev-parse", "--abbrev-ref", "HEAD") == "main" + assert run_git(git_repo, "rev-parse", "HEAD") == head_before + assert run_git(git_repo, "status", "--porcelain") == "" + assert os.path.isdir(os.path.join(git_repo, ".artifacts", "issue-builds")) + + # The branch is the deliverable and carries the implementation. + assert run_git(git_repo, "show", f"{result['branch']}:feature.py") + + # The worktree was cleaned up. + worktrees = os.path.join(git_repo, ".worktrees") + assert not os.path.isdir(worktrees) or os.listdir(worktrees) == [] + + def test_uncommitted_coder_work_gets_checkpoint_commit(self, git_repo: str) -> None: + recorded: list = [] + call_fn = make_call_fn(recorded, coder_commits=False) + result = _run(git_repo, call_fn, config={"verify": False}) + + assert result["success"] is True + assert len(result["commits"]) == 1 + message = run_git(git_repo, "log", "-1", "--format=%s", result["branch"]) + assert "checkpoint" in message + + def test_bytecode_junk_never_lands_on_branch(self, git_repo: str) -> None: + # The real coder runs pytest in the worktree, generating __pycache__, + # and a sloppy model may even commit it. Neither may reach the branch. + recorded: list = [] + base_call_fn = make_call_fn(recorded, coder_commits=False) + + async def call_fn(target, **kwargs): + if target.endswith("run_coder"): + worktree = kwargs["worktree_path"] + pycache = os.path.join(worktree, "taskstore", "__pycache__") + os.makedirs(pycache, exist_ok=True) + with open(os.path.join(pycache, "x.cpython-312.pyc"), "wb") as f: + f.write(b"\x00") + return await base_call_fn(target, **kwargs) + + result = _run(git_repo, call_fn, config={"verify": False}) + + assert result["success"] is True + assert not any("__pycache__" in f or f.endswith(".pyc") + for f in result["files_changed"]) + tracked = run_git(git_repo, "ls-tree", "-r", "--name-only", result["branch"]) + assert "__pycache__" not in tracked + + def test_base_branch_override(self, git_repo: str) -> None: + run_git(git_repo, "checkout", "-q", "-b", "dev") + with open(os.path.join(git_repo, "dev.txt"), "w") as f: + f.write("dev\n") + run_git(git_repo, "add", "dev.txt") + run_git(git_repo, "commit", "-q", "-m", "dev work") + run_git(git_repo, "checkout", "-q", "main") + + recorded: list = [] + result = _run( + git_repo, make_call_fn(recorded), + base_branch="dev", config={"verify": False}, + ) + + assert result["success"] is True + assert result["base_branch"] == "dev" + # Branch includes dev's history, not just main's. + assert run_git(git_repo, "show", f"{result['branch']}:dev.txt") == "dev" + + +class TestParallelFanOut: + """C2 — concurrent calls yield independent branches.""" + + def test_two_concurrent_builds_on_same_repo(self, git_repo: str) -> None: + recorded_a: list = [] + recorded_b: list = [] + + async def _both() -> tuple[dict, dict]: + return await asyncio.gather( + _implement_issue_impl( + issue={**ISSUE, "title": "Issue A"}, + repo_path=git_repo, + base_branch="", + artifacts_dir=".artifacts", + additional_context="", + config={"verify": False}, + call_fn=make_call_fn(recorded_a), + note_fn=None, + node_id="test-node", + ), + _implement_issue_impl( + issue={**ISSUE, "title": "Issue B"}, + repo_path=git_repo, + base_branch="", + artifacts_dir=".artifacts", + additional_context="", + config={"verify": False}, + call_fn=make_call_fn(recorded_b), + note_fn=None, + node_id="test-node", + ), + ) + + result_a, result_b = asyncio.run(_both()) + + assert result_a["success"] and result_b["success"] + assert result_a["branch"] != result_b["branch"] + branches = run_git(git_repo, "branch", "--list", "issue/*") + assert result_a["branch"].split("/", 1)[1] in branches + assert result_b["branch"].split("/", 1)[1] in branches + assert run_git(git_repo, "rev-parse", "--abbrev-ref", "HEAD") == "main" + + +class TestCallBudget: + """C3 — no planning agents, bounded call count.""" + + def test_no_planning_agents_and_bounded_calls(self, git_repo: str) -> None: + recorded: list = [] + result = _run(git_repo, make_call_fn(recorded)) + + targets = {t.split(".", 1)[1] for t, _ in recorded} + assert targets & PLANNING_TARGETS == set() + assert targets <= {"run_coder", "run_code_reviewer", "run_verifier"} + # 1 iteration: coder + reviewer, then one verifier pass. + assert len(recorded) == 3 + assert result["iterations"] == 1 + + def test_flagged_path_uses_qa_and_synthesizer(self, git_repo: str) -> None: + recorded: list = [] + result = _run( + git_repo, + make_call_fn(recorded), + issue={**ISSUE, "needs_deeper_qa": True}, + config={"verify": False}, + ) + + assert result["success"] is True + targets = [t.split(".", 1)[1] for t, _ in recorded] + assert "run_qa" in targets + assert "run_qa_synthesizer" in targets + assert set(targets) & PLANNING_TARGETS == set() + + +class TestNoSideEffectsByDefault: + """C4 — nothing pushed, no PR.""" + + def test_no_pr_and_no_push_targets(self, git_repo: str) -> None: + recorded: list = [] + result = _run(git_repo, make_call_fn(recorded)) + + targets = {t.split(".", 1)[1] for t, _ in recorded} + assert "run_github_pr" not in targets + assert result["pr_url"] == "" + # No remotes were ever configured, so nothing could have been pushed. + assert run_git(git_repo, "remote") == "" + + def test_pr_skipped_without_remote_even_when_enabled(self, git_repo: str) -> None: + recorded: list = [] + notes: list = [] + result = _run( + git_repo, + make_call_fn(recorded), + config={"verify": False, "enable_github_pr": True}, + note_fn=lambda msg, tags=None: notes.append(msg), + ) + + assert result["success"] is True + assert result["pr_url"] == "" + targets = {t.split(".", 1)[1] for t, _ in recorded} + assert "run_github_pr" not in targets + assert any("skipping PR" in n for n in notes) + + +class TestVerification: + """C5 — verifier failure surfaces.""" + + def test_verifier_failure_fails_build_but_keeps_branch(self, git_repo: str) -> None: + recorded: list = [] + call_fn = make_call_fn( + recorded, + verifier_response={ + "passed": False, + "criteria_results": [ + {"criterion": "retry() retries", "passed": False, "evidence": "no test"} + ], + "summary": "AC not met", + }, + ) + result = _run(git_repo, call_fn) + + assert result["success"] is False + assert result["outcome"] == "completed" + assert result["verification"]["passed"] is False + # Partial work is still handed to the caller for triage. + assert result["branch"].startswith("issue/") + assert len(result["commits"]) == 1 + + def test_verify_disabled_skips_verifier(self, git_repo: str) -> None: + recorded: list = [] + result = _run(git_repo, make_call_fn(recorded), config={"verify": False}) + + targets = {t.split(".", 1)[1] for t, _ in recorded} + assert "run_verifier" not in targets + assert result["verification"] is None + assert result["success"] is True + + +class TestFailureModes: + """C6 — exhaustion, blocking review, coder crash.""" + + def test_exhaustion_completes_with_debt(self, git_repo: str) -> None: + recorded: list = [] + call_fn = make_call_fn( + recorded, + reviewer_responses=[ + {"approved": False, "blocking": False, "summary": "needs polish"} + ], + ) + result = _run( + git_repo, call_fn, + config={"verify": False, "max_coding_iterations": 2}, + ) + + assert result["outcome"] == "completed_with_debt" + assert result["success"] is True + assert result["iterations"] == 2 + assert len(result["iteration_history"]) == 2 + + def test_blocking_review_fails_but_salvages_commits(self, git_repo: str) -> None: + recorded: list = [] + call_fn = make_call_fn( + recorded, + reviewer_responses=[ + {"approved": False, "blocking": True, "summary": "introduces data loss"} + ], + ) + result = _run(git_repo, call_fn, config={"verify": False}) + + assert result["success"] is False + assert result["outcome"] == "failed_unrecoverable" + assert result["error_message"] + # The coder committed before the block: the branch is kept for triage. + assert result["branch"].startswith("issue/") + assert len(result["commits"]) == 1 + + def test_no_commits_deletes_branch(self, git_repo: str) -> None: + recorded: list = [] + call_fn = make_call_fn(recorded, coder_writes=False) + result = _run(git_repo, call_fn, config={"verify": False}) + + assert result["success"] is False + assert result["branch"] == "" + assert result["commits"] == [] + assert run_git(git_repo, "branch", "--list", "issue/*") == "" + + def test_unexpected_error_returns_structured_failure_and_cleans_up( + self, git_repo: str + ) -> None: + from swe_af.execution.fatal_error import FatalHarnessError + + recorded: list = [] + call_fn = make_call_fn( + recorded, coder_exception=FatalHarnessError("credit balance too low") + ) + result = _run(git_repo, call_fn, config={"verify": False}) + + assert result["success"] is False + assert result["outcome"] == "error" + assert "credit balance" in result["error_message"] + assert result["branch"] == "" + assert run_git(git_repo, "branch", "--list", "issue/*") == "" + worktrees = os.path.join(git_repo, ".worktrees") + assert not os.path.isdir(worktrees) or os.listdir(worktrees) == [] + + +class TestSetupValidation: + """Setup errors raise — they are the caller's to fix.""" + + def test_missing_repo_raises(self, tmp_path) -> None: + with pytest.raises(GitOpsError, match="does not exist"): + _run(str(tmp_path / "nope"), make_call_fn([])) + + def test_non_git_dir_raises(self, tmp_path) -> None: + plain = tmp_path / "plain" + plain.mkdir() + with pytest.raises(GitOpsError, match="not a git repository"): + _run(str(plain), make_call_fn([])) + + def test_repo_without_commits_raises(self, tmp_path) -> None: + import subprocess + + empty = tmp_path / "empty" + empty.mkdir() + subprocess.run(["git", "init", "-q"], cwd=str(empty), check=True) + with pytest.raises(GitOpsError, match="no commits"): + _run(str(empty), make_call_fn([])) + + def test_unknown_base_branch_raises(self, git_repo: str) -> None: + with pytest.raises(GitOpsError, match="base branch"): + _run(git_repo, make_call_fn([]), base_branch="does-not-exist") diff --git a/tests/issue/test_schemas.py b/tests/issue/test_schemas.py new file mode 100644 index 0000000..444fe0d --- /dev/null +++ b/tests/issue/test_schemas.py @@ -0,0 +1,120 @@ +"""Tests for the issue-level build schemas.""" + +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from swe_af.issue.schemas import ( + IssueBuildConfig, + IssueBuildResult, + IssueSpec, + slugify, +) + + +class TestSlugify: + def test_basic(self) -> None: + assert slugify("Add retry helper!") == "add-retry-helper" + + def test_collapses_and_trims(self) -> None: + assert slugify(" --Weird__ Name-- ") == "weird-name" + + def test_empty_falls_back(self) -> None: + assert slugify("!!!") == "issue" + + def test_truncates(self) -> None: + assert len(slugify("x" * 100)) <= 40 + + +class TestIssueSpec: + def test_requires_title_and_description(self) -> None: + with pytest.raises(ValidationError): + IssueSpec(title="", description="d") + with pytest.raises(ValidationError): + IssueSpec(title="t", description=" ") + with pytest.raises(ValidationError): + IssueSpec(description="d") # type: ignore[call-arg] + + def test_rejects_unknown_fields(self) -> None: + with pytest.raises(ValidationError): + IssueSpec(title="t", description="d", surprise=1) # type: ignore[call-arg] + + def test_to_planned_issue_maps_onto_planned_issue_schema(self) -> None: + # The dict must satisfy the internal PlannedIssue model 1:1. + from swe_af.reasoners.schemas import PlannedIssue + + spec = IssueSpec( + title="Add retry helper", + description="Backoff retry in utils.", + acceptance_criteria=["retries 3 times"], + files_to_modify=["utils.py"], + testing_strategy="pytest tests/test_utils.py", + needs_deeper_qa=True, + ) + planned = spec.to_planned_issue() + model = PlannedIssue.model_validate(planned) + + assert model.name == "add-retry-helper" + assert model.guidance is not None + assert model.guidance.needs_deeper_qa is True + assert model.guidance.testing_guidance == "pytest tests/test_utils.py" + assert model.files_to_modify == ["utils.py"] + assert model.sequence_number == 1 + + def test_additional_context_appended(self) -> None: + spec = IssueSpec(title="T", description="Base description.") + planned = spec.to_planned_issue(additional_context="Use httpx, not requests.") + assert "Base description." in planned["description"] + assert "Use httpx, not requests." in planned["description"] + + def test_explicit_name_is_slugified(self) -> None: + spec = IssueSpec(title="T", description="d", name="My Fancy Name!") + assert spec.to_planned_issue()["name"] == "my-fancy-name" + + +class TestIssueBuildConfig: + def test_defaults(self) -> None: + cfg = IssueBuildConfig() + assert cfg.max_coding_iterations == 3 + assert cfg.verify is True + assert cfg.enable_github_pr is False + assert cfg.branch_prefix == "issue/" + assert cfg.keep_worktree is False + + def test_valid_model_keys(self) -> None: + cfg = IssueBuildConfig( + models={"default": "haiku", "coder": "sonnet", "verifier": "haiku"} + ) + assert cfg.models["coder"] == "sonnet" + + def test_unknown_model_key_rejected(self) -> None: + with pytest.raises(ValidationError, match="Unknown model keys"): + IssueBuildConfig(models={"pm": "haiku"}) + + def test_unknown_config_key_rejected(self) -> None: + with pytest.raises(ValidationError): + IssueBuildConfig(max_tasks=5) # type: ignore[call-arg] + + def test_models_flow_into_execution_config(self) -> None: + # The subset of role keys accepted here must remain valid for the + # ExecutionConfig that drives the coding loop. + from swe_af.execution.schemas import ExecutionConfig + + cfg = IssueBuildConfig( + runtime="claude_code", + models={"default": "haiku", "coder": "sonnet"}, + ) + exec_config = ExecutionConfig(runtime=cfg.runtime, models=cfg.models) + assert exec_config.coder_model == "sonnet" + assert exec_config.code_reviewer_model == "haiku" + assert exec_config.verifier_model == "haiku" + + +class TestIssueBuildResult: + def test_minimal_result(self) -> None: + result = IssueBuildResult(success=False, outcome="error", summary="boom") + data = result.model_dump() + assert data["branch"] == "" + assert data["commits"] == [] + assert data["verification"] is None