diff --git a/go/bin/swe-pro-darwin-arm64 b/go/bin/swe-pro-darwin-arm64 index 3891b7ea..fcc99041 100755 Binary files a/go/bin/swe-pro-darwin-arm64 and b/go/bin/swe-pro-darwin-arm64 differ diff --git a/go/bin/swe-pro-linux-amd64 b/go/bin/swe-pro-linux-amd64 index 8080c43d..a0465f30 100755 Binary files a/go/bin/swe-pro-linux-amd64 and b/go/bin/swe-pro-linux-amd64 differ diff --git a/go/internal/node/discovery_surface_test.go b/go/internal/node/discovery_surface_test.go new file mode 100644 index 00000000..6b44e22f --- /dev/null +++ b/go/internal/node/discovery_surface_test.go @@ -0,0 +1,200 @@ +package node + +import ( + "strings" + "testing" + + "github.com/Agent-Field/SWE-AF/go/internal/pro" +) + +// roleAreas is the independent checklist of the pipeline area each internal role +// reasoner belongs to. Like pythonRoleSurface it is written from the roles' +// jobs — NOT read back out of register.go — so a role that moves package +// without its description following fails here. +var roleAreas = map[string]string{ + // planning roles + "run_product_manager": "planning", + "run_environment_scout": "planning", + "run_architect": "planning", + "run_tech_lead": "planning", + "run_sprint_planner": "planning", + // coding roles + "run_coder": "coding", + "run_qa": "coding", + "run_code_reviewer": "coding", + "run_qa_synthesizer": "coding", + // git/workspace roles + "run_git_init": "gitops", + "run_workspace_setup": "gitops", + "run_workspace_cleanup": "gitops", + "run_merger": "gitops", + "run_integration_tester": "gitops", + "run_repo_finalize": "gitops", + "run_github_pr": "gitops", + // advisor/verify roles + "run_retry_advisor": "advisor", + "run_issue_advisor": "advisor", + "run_replanner": "advisor", + "run_issue_writer": "advisor", + "run_verifier": "advisor", + "generate_fix_issues": "advisor", + // CI/resolve roles + "run_ci_watcher": "ci", + "run_ci_fixer": "ci", + "run_pr_resolver": "ci", +} + +// wantEntrypoints is the exact set of swe-planner reasoners a caller may start a +// run from. execute is deliberately absent (its plan_result comes from plan) and +// so is pro_execute (an execute_fn_target). A caller-facing utility landing on +// this node — e.g. get_workspace_handle — joins this list when it does. +var wantEntrypoints = []string{"build", "implement_issue", "plan", "resolve", "resume_build"} + +// TestRoleReasonersAreMarkedInternal: every role reasoner must carry the +// "internal" tag AND a description saying an orchestrator drives it. Without +// both, a coding agent discovering the node sees a bare name like +// run_product_manager and invokes it directly — which fails, because the stage +// has no orchestrator context. +func TestRoleReasonersAreMarkedInternal(t *testing.T) { + t.Setenv("SWE_PRO_ENGINE", "") + n, err := BuildAgent("swe-planner", "8005", "Autonomous SWE planning pipeline") + if err != nil { + t.Fatalf("BuildAgent: %v", err) + } + n.RegisterPlanner() + meta := n.RegisteredMeta() + + for _, name := range pythonRoleSurface { + area, ok := roleAreas[name] + if !ok { + t.Errorf("role %q has no area in roleAreas — extend the checklist", name) + continue + } + m, ok := meta[name] + if !ok { + t.Errorf("role %q not registered", name) + continue + } + if !hasTag(m.Tags, tagInternal) { + t.Errorf("role %q tags = %v, want the %q marker", name, m.Tags, tagInternal) + } + if !hasTag(m.Tags, tagPlanner) { + t.Errorf("role %q tags = %v, lost the %q group tag", name, m.Tags, tagPlanner) + } + if m.Description == "" { + t.Errorf("role %q has no description — discovery shows a bare name", name) + continue + } + if want := "Internal " + area + " pipeline stage"; !strings.HasPrefix(m.Description, want) { + t.Errorf("role %q description = %q, want it to start with %q", name, m.Description, want) + } + if !strings.Contains(m.Description, "do not call directly") { + t.Errorf("role %q description = %q, want the do-not-call-directly warning", name, m.Description) + } + } +} + +// TestEntrypointTagIsExactSet: the "entrypoint" tag is what `af ls --entrypoints` +// and GET /api/v1/discovery/capabilities filter on, so the tagged set must be +// exactly the reasoners a caller can legitimately start from — no internal stage +// leaking in, no real entry point missing. +func TestEntrypointTagIsExactSet(t *testing.T) { + t.Setenv("SWE_PRO_ENGINE", "") + n, err := BuildAgent("swe-planner", "8005", "Autonomous SWE planning pipeline") + if err != nil { + t.Fatalf("BuildAgent: %v", err) + } + n.RegisterPlanner() + + assertSurface(t, "swe-planner[entrypoint]", entrypointNames(n), wantEntrypoints) + + // Every entry point must also say what it is for — the tag routes a caller + // to it, the description tells them whether to pick it. + meta := n.RegisteredMeta() + for _, name := range wantEntrypoints { + if meta[name].Description == "" { + t.Errorf("entrypoint %q has no description", name) + } + } + + // The fast node exposes only its own build plus implement_issue. + f, err := BuildAgent("swe-fast", "8006", "fast desc") + if err != nil { + t.Fatalf("BuildAgent: %v", err) + } + f.RegisterFast() + assertSurface(t, "swe-fast[entrypoint]", entrypointNames(f), []string{"build", "implement_issue"}) +} + +// TestProExecuteIsInternal: pro_execute is reached through +// config.execute_fn_target, never started by a caller — it must be tagged +// internal and must not appear as an entry point. +func TestProExecuteIsInternal(t *testing.T) { + t.Setenv(pro.EnvEnabled, "1") + fakeEngineBin(t) + + n, err := BuildAgent("swe-planner", "8005", "Autonomous SWE planning pipeline") + if err != nil { + t.Fatalf("BuildAgent: %v", err) + } + n.RegisterPlanner() + + m, ok := n.RegisteredMeta()["pro_execute"] + if !ok { + t.Fatal("pro_execute not registered with the engine enabled") + } + if !hasTag(m.Tags, tagInternal) { + t.Errorf("pro_execute tags = %v, want the %q marker", m.Tags, tagInternal) + } + if hasTag(m.Tags, tagEntrypoint) { + t.Errorf("pro_execute tags = %v, must not be an entry point", m.Tags) + } + if m.Description == "" { + t.Error("pro_execute lost its description") + } + assertSurface(t, "swe-planner[pro][entrypoint]", entrypointNames(n), wantEntrypoints) +} + +// TestExecuteDescribesItsPlanResultInput: execute's plan_result is the one input +// on the surface a caller cannot write by hand — the description has to say so, +// since execute is (correctly) not tagged as an entry point but is still visible. +func TestExecuteDescribesItsPlanResultInput(t *testing.T) { + t.Setenv("SWE_PRO_ENGINE", "") + n, err := BuildAgent("swe-planner", "8005", "Autonomous SWE planning pipeline") + if err != nil { + t.Fatalf("BuildAgent: %v", err) + } + n.RegisterPlanner() + + got := n.RegisteredMeta()["execute"].Description + for _, want := range []string{"plan_result comes from a prior plan call", "prefer build"} { + if !strings.Contains(got, want) { + t.Errorf("execute description = %q, want it to contain %q", got, want) + } + } +} + +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +// entrypointNames returns the names of every reasoner registered with the +// "entrypoint" tag. +func entrypointNames(n *Node) []string { + var out []string + for name, m := range n.RegisteredMeta() { + if hasTag(m.Tags, tagEntrypoint) { + out = append(out, name) + } + } + return out +} + +func hasTag(tags []string, want string) bool { + for _, t := range tags { + if t == want { + return true + } + } + return false +} diff --git a/go/internal/node/node.go b/go/internal/node/node.go index d3cebc56..b253dc70 100644 --- a/go/internal/node/node.go +++ b/go/internal/node/node.go @@ -52,6 +52,20 @@ type Node struct { // surface. RegisterReasoner is a pure insert keyed by name, so this slice // equals the agent's reasoner set (the test also guards against duplicates). registered []string + + // meta records the resolved discovery metadata of every reasoner passed + // through regHandler, keyed by name. The SDK keeps its own reasoner map + // unexported, so this is the only way the surface tests can assert what a + // caller actually sees on the control plane. + meta map[string]ReasonerMeta +} + +// ReasonerMeta is the discovery-facing metadata a reasoner registers with: the +// tags and the description `af ls` and GET /api/v1/discovery/capabilities show +// a caller deciding which reasoner to invoke. +type ReasonerMeta struct { + Tags []string + Description string } // RegisteredNames returns a copy of the reasoner names registered on this node, @@ -61,6 +75,18 @@ func (n *Node) RegisteredNames() []string { return append([]string(nil), n.registered...) } +// RegisteredMeta returns a copy of the registration metadata keyed by reasoner +// name. Used by the surface tests to assert the entrypoint tagging and the +// internal-stage markers a discovering caller routes on. +func (n *Node) RegisteredMeta() map[string]ReasonerMeta { + out := make(map[string]ReasonerMeta, len(n.meta)) + for name, m := range n.meta { + m.Tags = append([]string(nil), m.Tags...) + out[name] = m + } + return out +} + // BuildAgent constructs the SWE-AF agent from the environment exactly as the // Python entry points do (app.py:51-59 / fast/app.py:24-31): // diff --git a/go/internal/node/register.go b/go/internal/node/register.go index f0db852c..598ab4aa 100644 --- a/go/internal/node/register.go +++ b/go/internal/node/register.go @@ -22,6 +22,18 @@ package node // fast_router tags=["swe-fast"]). The five orchestrators carry ["swe-planner"] // to group them with the node in the control-plane UI (design §8). // +// Two further tags are load-bearing for discovery rather than grouping: +// +// - "entrypoint" marks a reasoner a caller may legitimately start from — +// build, implement_issue, plan, resolve, resume_build. `af ls --entrypoints` +// and GET /api/v1/discovery/capabilities filter on it. execute is NOT one: +// its plan_result input is produced by plan, not hand-written. +// - "internal" marks the pipeline stages an orchestrator drives and nothing +// else should call — every role reasoner plus pro_execute. Each also carries +// a description saying so, because a discovering coding agent that sees a +// bare name (run_product_manager) will otherwise invoke it directly and get +// a failure that reads like a broken node. +// // Running this alongside the Python node against one control plane therefore // needs an explicit NODE_ID on one of them; docker-compose.go.yml does that. @@ -45,8 +57,10 @@ import ( ) const ( - tagPlanner = "swe-planner" - tagFast = "swe-fast" + tagPlanner = "swe-planner" + tagFast = "swe-fast" + tagEntrypoint = "entrypoint" + tagInternal = "internal" ) // RegisterPlanner registers the full swe-planner surface: 25 role reasoners + @@ -74,12 +88,24 @@ func (n *Node) RegisterFast() { // Role reasoners (identical on both nodes) // --------------------------------------------------------------------------- +// internalRoleOpts is the single source of the registration metadata every role +// reasoner carries: the swe-planner group tag, the "internal" marker, and the +// one-line description that tells a discovering caller this stage is driven by +// an orchestrator. area names the role package's domain (planning, coding, +// gitops, advisor, ci) — the only part that varies across the 25. +func internalRoleOpts(area string) []agent.ReasonerOption { + return []agent.ReasonerOption{ + agent.WithReasonerTags(tagPlanner, tagInternal), + agent.WithDescription("Internal " + area + " pipeline stage invoked by the orchestrators " + + "(build/plan/execute) — do not call directly."), + } +} + // registerRoles wires the 25 execution/planning role reasoners, each backed by // its package handler and threaded with the Deps built from the agent. All are -// tagged ["swe-planner"] (Python groups them under the swe-planner router). +// tagged ["swe-planner","internal"] (Python groups them under the swe-planner +// router) and described per internalRoleOpts. func (n *Node) registerRoles() { - tag := agent.WithReasonerTags(tagPlanner) - planningDeps := &planning.Deps{ Harness: n.App, App: n.App, @@ -88,18 +114,21 @@ func (n *Node) registerRoles() { NodeID: n.NodeID, AgentFieldServer: n.AgentFieldServer, } + planningOpts := internalRoleOpts("planning") for name, h := range planning.Handlers() { - regHandler(n, name, planningDeps, h, tag) + regHandler(n, name, planningDeps, h, planningOpts...) } codingDeps := &coding.Deps{Harness: n.App, AI: n.App, Note: n.App} + codingOpts := internalRoleOpts("coding") for name, h := range coding.Handlers() { - regHandler(n, name, codingDeps, h, tag) + regHandler(n, name, codingDeps, h, codingOpts...) } gitopsDeps := &gitops.Deps{App: n.App} + gitopsOpts := internalRoleOpts("gitops") for name, h := range gitops.Handlers() { - regHandler(n, name, gitopsDeps, h, tag) + regHandler(n, name, gitopsDeps, h, gitopsOpts...) } advisorDeps := &advisor.Deps{ @@ -110,13 +139,15 @@ func (n *Node) registerRoles() { NodeID: n.NodeID, AgentFieldServer: n.AgentFieldServer, } + advisorOpts := internalRoleOpts("advisor") for name, h := range advisor.Handlers() { - regHandler(n, name, advisorDeps, h, tag) + regHandler(n, name, advisorDeps, h, advisorOpts...) } ciDeps := &ci.Deps{App: n.App} + ciOpts := internalRoleOpts("ci") for name, h := range ci.Handlers() { - regHandler(n, name, ciDeps, h, tag) + regHandler(n, name, ciDeps, h, ciOpts...) } } @@ -154,13 +185,13 @@ func (n *Node) registerOrchestrators() { handlers["resume_build"] = orch.ResumeBuildHandler // Python registers the orchestrators via @app.reasoner(): only `build` - // carries tags (["entrypoint"]) plus an explicit routing description; the - // others get their docstring summaries as descriptions — keep the - // registration payload identical. + // carries an explicit routing description, the others get their docstring + // summaries. The "entrypoint" tag goes on every orchestrator a caller may + // legitimately start from (orchestratorEntrypoints). for name, h := range handlers { var opts []agent.ReasonerOption - if name == "build" { - opts = append(opts, agent.WithReasonerTags("entrypoint")) + if orchestratorEntrypoints[name] { + opts = append(opts, agent.WithReasonerTags(tagEntrypoint)) } if d, ok := orchestratorDescriptions[name]; ok { opts = append(opts, agent.WithDescription(d)) @@ -172,6 +203,19 @@ func (n *Node) registerOrchestrators() { } } +// orchestratorEntrypoints is the set of orchestrators a caller may start a run +// from, and therefore the ones tagged "entrypoint" for discovery. plan, resolve +// and resume_build are advanced but legitimate entries (a goal, a PR URL and a +// checkpointed repo respectively). execute is deliberately absent: its +// plan_result input is only producible by a prior plan call, so surfacing it as +// an entry point invites hand-written garbage. +var orchestratorEntrypoints = map[string]bool{ + "build": true, + "plan": true, + "resolve": true, + "resume_build": true, +} + // orchestratorDescriptions mirrors the Python side: build's explicit // description= kwarg, and the docstring first paragraphs the Python SDK // auto-registers for the other orchestrators (swe_af/app.py). @@ -181,8 +225,10 @@ var orchestratorDescriptions = map[string]string{ "repo_url; returns a verified feature branch (optionally a draft PR). " + "Typical wall-clock 25-60 min. For one well-scoped change with known files, " + "prefer implement_issue.", - "plan": "Run the full planning pipeline.", - "execute": "Execute a planned DAG with self-healing replanning.", + "plan": "Run the full planning pipeline.", + "execute": "Execute a planned DAG with self-healing replanning. Input plan_result comes " + + "from a prior plan call — not a hand-written object; prefer build unless you are " + + "resuming a custom pipeline.", "resolve": "Update an existing PR: merge base, fix CI, address review comments, push.", "resume_build": "Resume a crashed build from the last checkpoint.", } @@ -208,7 +254,7 @@ func (n *Node) registerFastReasoners() { var opts []agent.ReasonerOption if name == "build" { opts = append(opts, - agent.WithReasonerTags("entrypoint"), + agent.WithReasonerTags(tagEntrypoint), agent.WithDescription( "Fast-mode build: one planning pass into a small task list, then code and "+ "verify with tight timeouts. Same goal/repo_path interface as "+ @@ -239,7 +285,7 @@ func (n *Node) registerIssueReasoner() { Note: n.App, NodeID: n.NodeID, } - tag := agent.WithReasonerTags("swe-issue-go", "entrypoint") + tag := agent.WithReasonerTags("swe-issue-go", tagEntrypoint) for name, h := range issue.Handlers() { opts := []agent.ReasonerOption{tag, agent.WithDescription( "Issue-level build (sub-harness entry): implements ONE fully-scoped issue " + @@ -270,7 +316,10 @@ func (n *Node) registerProReasoners() { } for name, h := range pro.Handlers() { opts := []agent.ReasonerOption{ - agent.WithReasonerTags(tagPlanner), + // "internal": pro_execute is an execute_fn_target, reached by + // build/execute routing per-issue coding through it — not a surface a + // caller starts a run from. + agent.WithReasonerTags(tagPlanner, tagInternal), agent.WithDescription( "Pro-engine executor: implements ONE fully-scoped issue via the " + "bundled pro coding engine. Matches the execute_fn_target contract — " + @@ -290,9 +339,9 @@ func (n *Node) registerProReasoners() { // regHandler adapts a package handler (func(ctx, *Deps, input) (any, error)) to // the SDK's HandlerFunc (func(ctx, input) (any, error)) by capturing deps, then -// registers it under name and records the name on the node. D is inferred from -// deps; the package Handler types are assignable to the parameter's unnamed func -// type. +// registers it under name and records the name plus its resolved discovery +// metadata on the node. D is inferred from deps; the package Handler types are +// assignable to the parameter's unnamed func type. func regHandler[D any]( n *Node, name string, @@ -301,11 +350,27 @@ func regHandler[D any]( opts ...agent.ReasonerOption, ) { n.registered = append(n.registered, name) + n.recordMeta(name, opts) n.App.RegisterReasoner(name, func(ctx context.Context, input map[string]any) (any, error) { return h(ctx, deps, input) }, opts...) } +// recordMeta resolves opts the same way RegisterReasoner does — by applying them +// to a zero agent.Reasoner — and keeps the tags/description under name. The SDK +// exposes no reader for its registered reasoners, so this mirror is what lets +// the surface tests assert what a caller discovers. +func (n *Node) recordMeta(name string, opts []agent.ReasonerOption) { + var r agent.Reasoner + for _, opt := range opts { + opt(&r) + } + if n.meta == nil { + n.meta = make(map[string]ReasonerMeta) + } + n.meta[name] = ReasonerMeta{Tags: r.Tags, Description: r.Description} +} + // --------------------------------------------------------------------------- // Input schemas — derived from the Python reasoner signatures so the // control-plane UI reasoner cards show the real fields (the SDK default is a diff --git a/go/internal/pro/adapter.go b/go/internal/pro/adapter.go index 5152987a..5346df54 100644 --- a/go/internal/pro/adapter.go +++ b/go/internal/pro/adapter.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "os" + "strconv" "strings" "github.com/Agent-Field/SWE-AF/go/internal/afx" @@ -78,7 +79,6 @@ func ProExecute(ctx context.Context, deps *Deps, input map[string]any) (any, err // Optional env-driven dispatch overrides: cost ceiling, sub-agent model // pools and reasoning-effort variant. Unset keeps the engine's defaults. for env, kw := range map[string]string{ - EnvMaxCost: "max_cost", EnvModelsHigh: "high", EnvModelsLow: "low", EnvVariant: "variant", @@ -87,6 +87,17 @@ func ProExecute(ctx context.Context, deps *Deps, input map[string]any) (any, err kwargs[kw] = v } } + // max_cost is a number in the engine's contract — code_task rejects string + // scalars rather than coercing them, so forwarding the raw env string + // would fail every dispatch. An unparseable ceiling is a configuration + // error surfaced per-dispatch, not a limit to drop silently. + if v := os.Getenv(EnvMaxCost); v != "" { + cost, err := strconv.ParseFloat(v, 64) + if err != nil { + return nil, fmt.Errorf("pro_execute: %s=%q is not a number", EnvMaxCost, v) + } + kwargs["max_cost"] = cost + } name, _ := in.Issue["name"].(string) deps.note(ctx, fmt.Sprintf("pro engine: dispatching issue %q", name), "pro") diff --git a/go/internal/pro/adapter_test.go b/go/internal/pro/adapter_test.go index c0067cfb..b223fda1 100644 --- a/go/internal/pro/adapter_test.go +++ b/go/internal/pro/adapter_test.go @@ -137,8 +137,28 @@ func TestProExecuteMaxCostForwarded(t *testing.T) { map[string]any{"issue": sampleIssue(), "repo_path": "/tmp/repo"}); err != nil { t.Fatal(err) } - if rec.kwargs["max_cost"] != "2.50" { - t.Errorf("max_cost = %v, want 2.50", rec.kwargs["max_cost"]) + // The engine's contract types max_cost as a number and rejects string + // scalars, so the ceiling must cross the wire as a float, not the raw + // env string. + if rec.kwargs["max_cost"] != 2.50 { + t.Errorf("max_cost = %#v, want float64 2.5", rec.kwargs["max_cost"]) + } +} + +func TestProExecuteMaxCostUnparseable(t *testing.T) { + t.Setenv(EnvMaxCost, "five dollars") + rec := &callRec{res: map[string]any{"status": "pass"}} + deps := &Deps{Call: rec.call, EngineNode: "swe-pro"} + _, err := ProExecute(context.Background(), deps, + map[string]any{"issue": sampleIssue(), "repo_path": "/tmp/repo"}) + if err == nil { + t.Fatal("want an error for an unparseable cost ceiling, got nil") + } + if !strings.Contains(err.Error(), EnvMaxCost) || !strings.Contains(err.Error(), "five dollars") { + t.Errorf("error should name the env var and value: %v", err) + } + if rec.target != "" { + t.Errorf("nothing may be dispatched on a bad ceiling; called %q", rec.target) } } diff --git a/swe_af/app.py b/swe_af/app.py index a9915644..0a7f548c 100644 --- a/swe_af/app.py +++ b/swe_af/app.py @@ -21,6 +21,7 @@ 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 +from swe_af.surface import TAG_ENTRYPOINT from agentfield import Agent @@ -494,7 +495,7 @@ def _is_empty_build(success: bool, ever_completed: int, ever_merged: int) -> boo @app.reasoner( - tags=["entrypoint"], + tags=[TAG_ENTRYPOINT], description=( "Feature-level build: plans a PRD → architecture → issue DAG, then codes, " "reviews, merges and verifies end-to-end. Give it a goal plus repo_path or " @@ -1431,7 +1432,7 @@ async def build( clear_scoped_credentials(_scope_id) -@app.reasoner() +@app.reasoner(tags=[TAG_ENTRYPOINT]) async def plan( goal: str, repo_path: str, @@ -1659,7 +1660,13 @@ async def execute( build_id: str = "", workspace_manifest: dict | None = None, ) -> dict: - """Execute a planned DAG with self-healing replanning. + """Execute a planned DAG with self-healing replanning. Input plan_result + comes from a prior plan call — not a hand-written object; prefer build + unless you are resuming a custom pipeline. + + Deliberately not tagged ``entrypoint`` (see ``swe_af.surface``); the first + paragraph is published as the reasoner description, so keep the plan_result + warning inside it. Args: plan_result: Output from the ``plan`` reasoner. @@ -1708,7 +1715,7 @@ async def execute_fn(issue, dag_state): return state.model_dump() -@app.reasoner() +@app.reasoner(tags=[TAG_ENTRYPOINT]) async def resolve( pr_url: str, pr_number: int, @@ -2107,7 +2114,7 @@ async def _post_thread_replies_and_resolve( return results -@app.reasoner() +@app.reasoner(tags=[TAG_ENTRYPOINT]) async def resume_build( repo_path: str, artifacts_dir: str = ".artifacts", diff --git a/swe_af/fast/__init__.py b/swe_af/fast/__init__.py index 78d875ee..b1020e27 100644 --- a/swe_af/fast/__init__.py +++ b/swe_af/fast/__init__.py @@ -18,6 +18,7 @@ from agentfield import AgentRouter from swe_af.runtime.codex_harness_patch import apply_codex_harness_patch +from swe_af.surface import internal_role apply_codex_harness_patch() @@ -30,7 +31,7 @@ # --------------------------------------------------------------------------- -@fast_router.reasoner() +@internal_role(fast_router, "gitops") async def run_git_init( repo_path: str, goal: str, @@ -50,7 +51,7 @@ async def run_git_init( ) -@fast_router.reasoner() +@internal_role(fast_router, "coding") async def run_coder( issue: dict, worktree_path: str, @@ -73,7 +74,7 @@ async def run_coder( ) -@fast_router.reasoner() +@internal_role(fast_router, "advisor") async def run_verifier( prd: dict, repo_path: str, @@ -95,7 +96,7 @@ async def run_verifier( ) -@fast_router.reasoner() +@internal_role(fast_router, "gitops") async def run_repo_finalize( repo_path: str, artifacts_dir: str = "", @@ -111,7 +112,7 @@ async def run_repo_finalize( ) -@fast_router.reasoner() +@internal_role(fast_router, "gitops") async def run_github_pr( repo_path: str, integration_branch: str, @@ -136,7 +137,7 @@ async def run_github_pr( ) -@fast_router.reasoner() +@internal_role(fast_router, "ci") async def run_ci_watcher( repo_path: str, pr_number: int, @@ -151,7 +152,7 @@ async def run_ci_watcher( ) -@fast_router.reasoner() +@internal_role(fast_router, "ci") async def run_ci_fixer( repo_path: str, pr_number: int, diff --git a/swe_af/fast/app.py b/swe_af/fast/app.py index c91adc97..66571b63 100644 --- a/swe_af/fast/app.py +++ b/swe_af/fast/app.py @@ -21,6 +21,7 @@ from swe_af.execution.schemas import _workspace_root from swe_af.fast import fast_router from swe_af.fast.schemas import FastBuildConfig, FastBuildResult, fast_resolve_models +from swe_af.surface import TAG_ENTRYPOINT NODE_ID = os.getenv("NODE_ID", "swe-fast") @@ -62,7 +63,7 @@ def _runtime_to_provider(runtime: str) -> str: @app.reasoner( - tags=["entrypoint"], + tags=[TAG_ENTRYPOINT], description=( "Fast-mode build: one planning pass into a small task list, then code and " "verify with tight timeouts. Same goal/repo_path interface as " diff --git a/swe_af/issue/build.py b/swe_af/issue/build.py index 4907c545..291d7d32 100644 --- a/swe_af/issue/build.py +++ b/swe_af/issue/build.py @@ -29,6 +29,7 @@ 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 +from swe_af.surface import TAG_ENTRYPOINT _COMPLETED_OUTCOMES = ("completed", "completed_with_debt") @@ -362,7 +363,7 @@ def note(message: str, tags: list[str] | None = None) -> None: @issue_router.reasoner( - tags=["entrypoint"], + tags=[TAG_ENTRYPOINT], description=( "Issue-level build (sub-harness entry): implements ONE fully-scoped issue " "on an isolated branch of a local repo — no planning agents, ~4-8 LLM " diff --git a/swe_af/reasoners/execution_agents.py b/swe_af/reasoners/execution_agents.py index ed0fc429..e65300e0 100644 --- a/swe_af/reasoners/execution_agents.py +++ b/swe_af/reasoners/execution_agents.py @@ -85,6 +85,7 @@ workspace_setup_task_prompt, ) from swe_af.tools.web_search import maybe_apply_coder_guardrail +from swe_af.surface import internal_role from . import router @@ -122,7 +123,7 @@ def _build_issue_results(failed_issues: list[dict]): # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "advisor") async def run_retry_advisor( issue: dict, error_message: str, @@ -203,7 +204,7 @@ async def run_retry_advisor( ).model_dump() -@router.reasoner() +@internal_role(router, "advisor") async def run_issue_advisor( issue: dict, original_issue: dict, @@ -316,7 +317,7 @@ async def _invoke_advisor( return fallback.model_dump() -@router.reasoner() +@internal_role(router, "advisor") async def run_replanner( dag_state: dict, failed_issues: list[dict], @@ -442,7 +443,7 @@ async def _invoke_replanner( return fallback.model_dump() -@router.reasoner() +@internal_role(router, "advisor") async def run_issue_writer( issue: dict, prd_summary: str, @@ -523,7 +524,7 @@ class IssueWriterOutput(BaseModel): } -@router.reasoner() +@internal_role(router, "advisor") async def run_verifier( prd: dict, repo_path: str, @@ -596,7 +597,7 @@ async def run_verifier( # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "gitops") async def run_git_init( repo_path: str, goal: str, @@ -679,7 +680,7 @@ async def run_git_init( ).model_dump() -@router.reasoner() +@internal_role(router, "gitops") async def run_workspace_setup( repo_path: str, integration_branch: str, @@ -746,7 +747,7 @@ class WorkspaceSetupResult(BaseModel): return {"workspaces": [], "success": False} -@router.reasoner() +@internal_role(router, "gitops") async def run_merger( repo_path: str, integration_branch: str, @@ -819,7 +820,7 @@ async def run_merger( ).model_dump() -@router.reasoner() +@internal_role(router, "gitops") async def run_integration_tester( repo_path: str, integration_branch: str, @@ -894,7 +895,7 @@ async def run_integration_tester( ).model_dump() -@router.reasoner() +@internal_role(router, "gitops") async def run_workspace_cleanup( repo_path: str, worktrees_dir: str, @@ -961,7 +962,7 @@ class WorkspaceCleanupResult(BaseModel): # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "coding") async def run_coder( issue: dict, worktree_path: str, @@ -1057,7 +1058,7 @@ async def run_coder( ).model_dump() -@router.reasoner() +@internal_role(router, "coding") async def run_qa( worktree_path: str, coder_result: dict, @@ -1131,7 +1132,7 @@ async def run_qa( ).model_dump() -@router.reasoner() +@internal_role(router, "coding") async def run_code_reviewer( worktree_path: str, coder_result: dict, @@ -1214,7 +1215,7 @@ async def run_code_reviewer( ).model_dump() -@router.reasoner() +@internal_role(router, "coding") async def run_qa_synthesizer( qa_result: dict, review_result: dict, @@ -1310,7 +1311,7 @@ async def run_qa_synthesizer( # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "advisor") async def generate_fix_issues( failed_criteria: list[dict], dag_state: dict, @@ -1414,7 +1415,7 @@ class FixGeneratorOutput(BaseModel): # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "gitops") async def run_repo_finalize( repo_path: str, artifacts_dir: str = "", @@ -1472,7 +1473,7 @@ async def run_repo_finalize( # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "gitops") async def run_github_pr( repo_path: str, integration_branch: str, @@ -1545,7 +1546,7 @@ async def run_github_pr( # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "ci") async def run_ci_watcher( repo_path: str, pr_number: int, @@ -1597,7 +1598,7 @@ async def run_ci_watcher( return result.model_dump() -@router.reasoner() +@internal_role(router, "ci") async def run_ci_fixer( repo_path: str, pr_number: int, @@ -1686,7 +1687,7 @@ async def run_ci_fixer( ).model_dump() -@router.reasoner() +@internal_role(router, "ci") async def run_pr_resolver( repo_path: str, pr_number: int, diff --git a/swe_af/reasoners/pipeline.py b/swe_af/reasoners/pipeline.py index 87e2860c..0580113f 100644 --- a/swe_af/reasoners/pipeline.py +++ b/swe_af/reasoners/pipeline.py @@ -26,6 +26,7 @@ ReviewResult, ) from swe_af.runtime.providers import runtime_to_harness_adapter +from swe_af.surface import internal_role from . import router @@ -158,7 +159,7 @@ def _assign_sequence_numbers(issues: list[dict], levels: list[list[str]]) -> lis # --------------------------------------------------------------------------- -@router.reasoner() +@internal_role(router, "planning") async def run_product_manager( goal: str, repo_path: str, @@ -252,7 +253,7 @@ async def _invoke_pm(prior_user_responses: list[dict] | None) -> PRD | None: return parsed.model_dump() -@router.reasoner() +@internal_role(router, "planning") async def run_environment_scout( prd: dict, repo_path: str, @@ -369,7 +370,7 @@ async def _invoke_scout(prior_user_responses: list[dict] | None) -> ScoutResult return parsed.model_dump(exclude={"scoped_credentials"}) -@router.reasoner() +@internal_role(router, "planning") async def run_architect( prd: dict, repo_path: str, @@ -435,7 +436,7 @@ async def run_architect( return result.parsed.model_dump() -@router.reasoner() +@internal_role(router, "planning") async def run_tech_lead( prd: dict, repo_path: str, @@ -501,7 +502,7 @@ async def run_tech_lead( return review -@router.reasoner() +@internal_role(router, "planning") async def run_sprint_planner( prd: dict, architecture: dict, diff --git a/swe_af/surface.py b/swe_af/surface.py new file mode 100644 index 00000000..52c2e32f --- /dev/null +++ b/swe_af/surface.py @@ -0,0 +1,58 @@ +"""Discovery surface metadata — the tags and descriptions callers route on. + +Two tags are load-bearing on both nodes, on top of the routers' grouping tags +(``swe-planner`` / ``swe-fast`` / ``swe-issue``): + +* ``entrypoint`` marks a reasoner a caller may legitimately start a run from — + ``build``, ``implement_issue``, ``plan``, ``resolve``, ``resume_build``. + ``af ls --entrypoints`` and ``GET /api/v1/discovery/capabilities`` filter on + it. ``execute`` is deliberately NOT one: its ``plan_result`` input is + produced by a prior ``plan`` call, not written by hand. +* ``internal`` marks the pipeline stages an orchestrator drives and nothing + else should call — every ``run_*`` role reasoner plus ``generate_fix_issues``. + Each also carries :data:`INTERNAL_ROLE_DESCRIPTION`, because a coding agent + that discovers a bare name like ``run_product_manager`` will otherwise invoke + it directly and get a failure that reads like a broken node. + +:func:`internal_role` is the single place both pieces of metadata are attached, +so the 25 role registrations share one string instead of 25 copies. The Go port +mirrors this file in ``go/internal/node/register.go`` (``internalRoleOpts`` and +``orchestratorEntrypoints``) — the two nodes register under the same identity, +so their tags and descriptions must stay identical. +""" + +from __future__ import annotations + +from typing import Callable + +#: Tag marking a reasoner a caller may start a run from. +TAG_ENTRYPOINT = "entrypoint" + +#: Tag marking an orchestrator-driven pipeline stage. +TAG_INTERNAL = "internal" + +#: Description every internal pipeline stage registers with. ``area`` is the +#: stage's domain: planning, coding, gitops, advisor or ci. +INTERNAL_ROLE_DESCRIPTION = ( + "Internal {area} pipeline stage invoked by the orchestrators " + "(build/plan/execute) — do not call directly." +) + + +def internal_role_description(area: str) -> str: + """Return the one-line description for an internal *area* pipeline stage.""" + return INTERNAL_ROLE_DESCRIPTION.format(area=area) + + +def internal_role(router, area: str) -> Callable[[Callable], Callable]: + """Register one internal pipeline stage on *router*. + + Drop-in replacement for ``@router.reasoner()``: adds the ``internal`` tag + (``AgentRouter.reasoner`` merges it with the router's own group tag) and the + shared do-not-call-directly description, which takes the place of the + docstring summary the SDK would otherwise publish. + """ + return router.reasoner( + tags=[TAG_INTERNAL], + description=internal_role_description(area), + ) diff --git a/tests/test_reasoner_surface.py b/tests/test_reasoner_surface.py new file mode 100644 index 00000000..c4a552cb --- /dev/null +++ b/tests/test_reasoner_surface.py @@ -0,0 +1,164 @@ +"""Discovery-surface tests: which reasoners a caller may start, and which are internal. + +The control plane publishes each reasoner's tags and description +(``af ls --entrypoints`` / ``GET /api/v1/discovery/capabilities``), and a coding +agent picks what to invoke from exactly that payload. These tests pin it: + +* every ``run_*`` role reasoner is tagged ``internal`` and describes itself as + an orchestrator-driven stage — a bare, undescribed ``run_product_manager`` is + what led callers to invoke it directly and get a context-less failure; +* the ``entrypoint`` set is exactly the reasoners a caller can legitimately + start a run from. + +Each node's surface is collected in a subprocess, like ``test_node_id_isolation``: +both apps include the same shared routers, and an ``AgentRouter`` hands its +entries to the first app that includes them — so in-process the surface depends +on which app another test imported (or reloaded) first. A fresh interpreter is +what the control plane actually sees at node startup. + +The Go node registers the same surface under the same identity — the mirror of +this module is ``go/internal/node/discovery_surface_test.go``. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys + +import pytest + +from swe_af.surface import TAG_ENTRYPOINT, TAG_INTERNAL, internal_role_description + +# Independent checklist of the pipeline area each internal role belongs to. +# Written from the role's job — NOT read back out of the registrations — so a +# role that changes area without its description following fails here. +ROLE_AREAS: dict[str, str] = { + # planning + "run_product_manager": "planning", + "run_environment_scout": "planning", + "run_architect": "planning", + "run_tech_lead": "planning", + "run_sprint_planner": "planning", + # coding + "run_coder": "coding", + "run_qa": "coding", + "run_code_reviewer": "coding", + "run_qa_synthesizer": "coding", + # git / workspace + "run_git_init": "gitops", + "run_workspace_setup": "gitops", + "run_workspace_cleanup": "gitops", + "run_merger": "gitops", + "run_integration_tester": "gitops", + "run_repo_finalize": "gitops", + "run_github_pr": "gitops", + # advisor / verify + "run_retry_advisor": "advisor", + "run_issue_advisor": "advisor", + "run_replanner": "advisor", + "run_issue_writer": "advisor", + "run_verifier": "advisor", + "generate_fix_issues": "advisor", + # CI / resolve + "run_ci_watcher": "ci", + "run_ci_fixer": "ci", + "run_pr_resolver": "ci", +} + +# The exact set of swe-planner reasoners a caller may start a run from. +# ``execute`` is deliberately absent: its ``plan_result`` input is produced by a +# prior ``plan`` call, not written by hand. +PLANNER_ENTRYPOINTS = {"build", "implement_issue", "plan", "resolve", "resume_build"} + +# The fast node has no plan/execute/resolve/resume_build — its own build plus +# the shared issue-level entry point. +FAST_ENTRYPOINTS = {"build", "implement_issue"} + +_DUMP = ( + "import json, importlib; " + "m = importlib.import_module('{module}'); " + "print(json.dumps({{r['id']: {{'tags': r.get('tags') or [], " + "'description': r.get('description', '')}} for r in m.app.reasoners}}))" +) + + +def _node_surface(module: str) -> dict[str, dict]: + """Registered metadata of *module*'s agent, collected in a fresh interpreter.""" + env = {k: v for k, v in os.environ.items() if k != "NODE_ID"} + env["AGENTFIELD_SERVER"] = "http://localhost:9999" + result = subprocess.run( + [sys.executable, "-c", _DUMP.format(module=module)], + env=env, + capture_output=True, + text=True, + ) + assert result.returncode == 0, f"collecting {module} surface failed: {result.stderr}" + return json.loads(result.stdout) + + +@pytest.fixture(scope="module") +def planner_surface() -> dict[str, dict]: + return _node_surface("swe_af.app") + + +@pytest.fixture(scope="module") +def fast_surface() -> dict[str, dict]: + return _node_surface("swe_af.fast.app") + + +def test_role_reasoners_are_marked_internal(planner_surface: dict[str, dict]) -> None: + """Every role reasoner carries the ``internal`` tag and the shared warning.""" + for name, area in ROLE_AREAS.items(): + meta = planner_surface.get(name) + assert meta is not None, f"role {name!r} is not registered" + + tags = meta["tags"] + assert TAG_INTERNAL in tags, f"role {name!r} tags={tags} lost the internal marker" + assert "swe-planner" in tags, f"role {name!r} tags={tags} lost the group tag" + assert TAG_ENTRYPOINT not in tags, f"role {name!r} must not be an entry point" + + assert meta["description"] == internal_role_description(area), ( + f"role {name!r} description={meta['description']!r} — expected the " + f"shared {area} pipeline-stage warning" + ) + + +def test_internal_tag_is_exactly_the_roles(planner_surface: dict[str, dict]) -> None: + """Nothing but the role reasoners hides behind the ``internal`` marker.""" + tagged = {name for name, meta in planner_surface.items() if TAG_INTERNAL in meta["tags"]} + assert tagged == set(ROLE_AREAS) + + +def test_entrypoint_tag_is_exact_set(planner_surface: dict[str, dict]) -> None: + """Only the reasoners a caller can start a run from carry ``entrypoint``.""" + tagged = {name for name, meta in planner_surface.items() if TAG_ENTRYPOINT in meta["tags"]} + assert tagged == PLANNER_ENTRYPOINTS + + # The tag routes a caller to the reasoner; the description tells them + # whether to pick it. Both are required. + for name in PLANNER_ENTRYPOINTS: + assert planner_surface[name]["description"], f"entrypoint {name!r} has no description" + + +def test_execute_is_not_an_entrypoint_and_says_where_plan_result_comes_from( + planner_surface: dict[str, dict], +) -> None: + """execute stays discoverable but tells callers not to hand-write plan_result.""" + meta = planner_surface["execute"] + assert TAG_ENTRYPOINT not in meta["tags"] + assert "plan_result comes from a prior plan call" in meta["description"] + assert "prefer build" in meta["description"] + + +def test_fast_node_surface_is_marked_the_same_way(fast_surface: dict[str, dict]) -> None: + """The fast node registers the same roles, marked internal the same way.""" + entrypoints = {name for name, meta in fast_surface.items() if TAG_ENTRYPOINT in meta["tags"]} + assert entrypoints == FAST_ENTRYPOINTS + + for name, area in ROLE_AREAS.items(): + meta = fast_surface.get(name) + assert meta is not None, f"role {name!r} is not registered on swe-fast" + assert TAG_INTERNAL in meta["tags"], f"role {name!r} tags={meta['tags']} on swe-fast" + assert meta["description"] == internal_role_description(area)