diff --git a/lab/internal/onboarding/bootstrap_planner_test.go b/lab/internal/onboarding/bootstrap_planner_test.go new file mode 100644 index 0000000..03a83f7 --- /dev/null +++ b/lab/internal/onboarding/bootstrap_planner_test.go @@ -0,0 +1,158 @@ +package onboarding + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/DanielNetzer/forgecell/lab/internal/harness" + "github.com/DanielNetzer/forgecell/lab/internal/resources" +) + +// prepareExplicitPlanner runs a fresh-Lab init --harness coding with an explicit +// Planner over the given candidates. +func prepareExplicitPlanner(t *testing.T, coding, planner string, candidates ...Candidate) (Outcome, string, error) { + t.Helper() + root, run := repoFixture(t) + dir := filepath.Join(t.TempDir(), "lab") + out, err := Prepare(context.Background(), InitOptions{Cwd: root, LabDir: dir, Launcher: "/installed/forgecell", Preferred: coding, Planner: planner, Candidates: candidates, Runner: run}) + return out, dir, err +} + +// assertNothingSaved checks a refused Prepare returned no proposal and wrote no +// assays and no resource configuration. +func assertNothingSaved(t *testing.T, out Outcome, dir string) { + t.Helper() + if out.Proposal != nil { + t.Fatalf("refusal returned a proposal: %+v", out.Proposal) + } + if entries, e := os.ReadDir(filepath.Join(dir, "assays")); e == nil && len(entries) > 0 || e != nil && !os.IsNotExist(e) { + t.Fatalf("refusal wrote assays: %v %v", entries, e) + } + if _, e := os.Stat(filepath.Join(dir, resources.ConfigFile)); !os.IsNotExist(e) { + t.Fatalf("refusal wrote %s: %v", resources.ConfigFile, e) + } +} + +func allReady() []Candidate { + return []Candidate{bootstrapCandidate("cursor", "/bin/cursor-agent", "ready"), bootstrapCandidate("claude-code", "/bin/claude", "ready"), bootstrapCandidate("codex", "/bin/codex", "ready")} +} + +func TestBootstrapExplicitPlannerCodex(t *testing.T) { + out, dir, err := prepareExplicitPlanner(t, "cursor", "codex", allReady()...) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out.Note, "the planner you chose") { + t.Fatalf("note does not say the planner was chosen: %s", out.Note) + } + assertPlannedBy(t, out, dir, "codex", "/bin/codex") +} + +func TestBootstrapExplicitPlannerClaudeCode(t *testing.T) { + candidates := []Candidate{bootstrapCandidate("cursor", "/bin/cursor-agent", "ready"), bootstrapCandidate("codex", "/bin/codex", "ready"), bootstrapCandidate("claude-code", "/bin/claude", "ready")} + out, dir, err := prepareExplicitPlanner(t, "cursor", "claude-code", candidates...) + if err != nil { + t.Fatal(err) + } + assertPlannedBy(t, out, dir, "claude-code", "/bin/claude") +} + +func TestBootstrapExplicitPlannerRefusesUnreadyOrUnableToPlan(t *testing.T) { + cursorReason := harness.AdapterCapabilities("cursor").Analysis.Reason + for name, tc := range map[string]struct { + planner string + candidates []Candidate + want []string + }{ + "cursor plans": {"cursor", allReady(), []string{"cannot be used for read-only ticket analysis", cursorReason, "no proposal saved"}}, + "codex signed out": {"codex", []Candidate{bootstrapCandidate("cursor", "/bin/cursor-agent", "ready"), bootstrapCandidate("claude-code", "/bin/claude", "ready"), bootstrapCandidate("codex", "/bin/codex", "login-required")}, []string{"cannot bind harnesses.analysis to codex", "codex is login-required.", "no proposal saved"}}, + "claude no binary": {"claude-code", []Candidate{bootstrapCandidate("cursor", "/bin/cursor-agent", "ready"), bootstrapCandidate("claude-code", "", "ready")}, []string{"cannot bind harnesses.analysis to claude-code", "no proposal saved"}}, + "not discovered": {"codex", []Candidate{bootstrapCandidate("cursor", "/bin/cursor-agent", "ready"), bootstrapCandidate("claude-code", "/bin/claude", "ready")}, []string{"no such harness was discovered", "no proposal saved"}}, + "unknown harness": {"other", allReady(), []string{"no such harness was discovered", "no proposal saved"}}, + "coding plans self": {"codex", allReady(), nil}, + } { + t.Run(name, func(t *testing.T) { + coding := "cursor" + if name == "coding plans self" { + coding = "claude-code" + tc.want = []string{"claude-code can do read-only ticket analysis itself", "no proposal saved"} + } + out, dir, err := prepareExplicitPlanner(t, coding, tc.planner, tc.candidates...) + if err == nil { + t.Fatalf("planner %s was accepted: %+v", tc.planner, out) + } + for _, want := range tc.want { + if !strings.Contains(err.Error(), want) { + t.Fatalf("refusal lacks %q: %v", want, err) + } + } + assertNothingSaved(t, out, dir) + }) + } +} + +func TestBootstrapExplicitPlannerOnlyForABootstrapWithHarness(t *testing.T) { + for name, o := range map[string]InitOptions{ + "no harness": {Planner: "codex"}, + "enable review": {Preferred: "cursor", Planner: "codex", EnableReview: true}, + "role": {Preferred: "cursor", Planner: "codex", RoleHarness: true, Role: "analysis"}, + "rebind": {Rebind: "cursor", Planner: "codex"}, + } { + t.Run(name, func(t *testing.T) { + root, run := repoFixture(t) + dir := filepath.Join(t.TempDir(), "lab") + o.Cwd, o.LabDir, o.Launcher, o.Candidates, o.Runner = root, dir, "/installed/forgecell", allReady(), run + out, err := Prepare(context.Background(), o) + if err == nil || !strings.Contains(err.Error(), "a planner is chosen only for a GitHub bootstrap") { + t.Fatalf("planner with %s: %v", name, err) + } + assertNothingSaved(t, out, dir) + }) + } + // A Lab that already has a Formula takes no bootstrap planner. + root, run := repoFixture(t) + dir := filepath.Join(t.TempDir(), "lab") + out, err := Prepare(context.Background(), InitOptions{Cwd: root, LabDir: dir, Launcher: "/installed/forgecell", Preferred: "cursor", Planner: "codex", Candidates: allReady(), Runner: run}) + if err != nil { + t.Fatal(err) + } + if _, err = Review(dir, out.Proposal.ID, "approve"); err != nil { + t.Fatal(err) + } + if _, err = Prepare(context.Background(), InitOptions{Cwd: root, LabDir: dir, Launcher: "/installed/forgecell", Preferred: "cursor", Planner: "claude-code", Candidates: allReady(), Runner: run}); err == nil || !strings.Contains(err.Error(), "only for a bootstrap") { + t.Fatalf("planner on an approved Lab: %v", err) + } +} + +func TestBootstrapExplicitPlannerDefaultMatchesTheEngine(t *testing.T) { + for name, tc := range map[string]struct { + candidates []Candidate + want string + }{ + "claude first": {allReady(), "claude-code"}, + "codex second": {[]Candidate{bootstrapCandidate("claude-code", "/bin/claude", "login-required"), bootstrapCandidate("codex", "/bin/codex", "ready")}, "codex"}, + "none ready": {[]Candidate{bootstrapCandidate("claude-code", "", "missing"), bootstrapCandidate("cursor", "/bin/cursor-agent", "ready")}, ""}, + "none at all": {nil, ""}, + "relative paths": {[]Candidate{bootstrapCandidate("claude-code", "bin/claude", "ready")}, ""}, + } { + t.Run(name, func(t *testing.T) { + if got := DefaultBootstrapPlanner(tc.candidates, "/installed/forgecell"); got != tc.want { + t.Fatalf("default planner %q, want %q", got, tc.want) + } + }) + } + if DefaultBootstrapPlanner(allReady(), "") != "" { + t.Fatal("a default planner without a launcher") + } + for id, want := range map[string]string{"claude-code": "supported", "codex": "supported", "cursor": "unsupported"} { + if got := PlanningCapability(id); got.State != want || got.Reason == "" { + t.Fatalf("%s planning capability %+v, want %s", id, got, want) + } + } + if got := PlanningCapability("cursor").Reason; got != harness.AdapterCapabilities("cursor").Analysis.Reason { + t.Fatalf("cursor reason %q", got) + } +} diff --git a/lab/internal/onboarding/init.go b/lab/internal/onboarding/init.go index ee24162..b793ea0 100644 --- a/lab/internal/onboarding/init.go +++ b/lab/internal/onboarding/init.go @@ -33,6 +33,10 @@ type InitOptions struct { // RoleHarness marks an explicit --harness ID --role ROLE: Preferred is proposed // as the harnesses. override only, declaring the role when it is absent. RoleHarness bool + // Planner chooses the harnesses.analysis binding of a bootstrap whose coding + // harness cannot do read-only ticket analysis; empty keeps the first ready + // planner (claude-code, then codex). + Planner string // RefreshBaseline proposes a baseline-only change: requiredChecks re-derived // from the current pull-request workflows of the approved Formula's repository. RefreshBaseline bool @@ -447,20 +451,68 @@ func bootstrapPlanner(candidates []Candidate, coding string, command []string, l if analysis.State != "unsupported" { return nil, nil, nil } + if planner, plannerCommand := firstBootstrapPlanner(candidates, launcher, lab); planner != nil { + return planner, plannerCommand, nil + } + return nil, nil, fmt.Errorf("cannot propose a bootstrap Formula that codes with %s: it cannot be used for read-only ticket analysis: %s; planning needs a ready Claude Code or Codex harness and neither is ready; install or sign in to Claude Code or Codex, then rerun forgecell init; no proposal saved", coding, analysis.Reason) +} + +// firstBootstrapPlanner is the first ready, discovered bootstrap planner whose +// adapter can do read-only ticket analysis, with its native command, or nil. +func firstBootstrapPlanner(candidates []Candidate, launcher, lab string) (*Candidate, []string) { for _, id := range bootstrapPlanners { for i := range candidates { if candidates[i].ID != id || candidates[i].Readiness != "ready" || candidates[i].Executable == "" { continue } if planner, err := roleHarnessTarget(candidates, id, launcher, formula.AnalysisHarness, lab); err == nil { - return &candidates[i], planner, nil + return &candidates[i], planner } } } - return nil, nil, fmt.Errorf("cannot propose a bootstrap Formula that codes with %s: it cannot be used for read-only ticket analysis: %s; planning needs a ready Claude Code or Codex harness and neither is ready; install or sign in to Claude Code or Codex, then rerun forgecell init; no proposal saved", coding, analysis.Reason) + return nil, nil +} + +// DefaultBootstrapPlanner is the harness a bootstrap that codes with a harness +// unable to plan binds harnesses.analysis to when no Planner is chosen, or "" +// when none is ready. It executes nothing. +func DefaultBootstrapPlanner(candidates []Candidate, launcher string) string { + if planner, _ := firstBootstrapPlanner(candidates, launcher, ""); planner != nil { + return planner.ID + } + return "" +} + +// PlanningCapability is the static support of the native adapter id for +// read-only ticket analysis. It executes nothing. +func PlanningCapability(id string) harness.Capability { + return roleCapability(formula.AnalysisHarness, formula.Binding{Binding: id, Command: []string{"", "__adapter", id, ""}}) +} + +// explicitBootstrapPlanner is the chosen Planner of a bootstrap whose coding +// binding cannot do read-only ticket analysis. It refuses a coding binding that +// can plan itself, and a planner that is not ready or cannot plan, quoting the +// adapter's reason. It executes nothing. +func explicitBootstrapPlanner(candidates []Candidate, coding string, command []string, id, launcher, lab string) (*Candidate, []string, error) { + if analysis := roleCapability(formula.AnalysisHarness, formula.Binding{Binding: coding, Command: command}); analysis.State != "unsupported" { + return nil, nil, fmt.Errorf("%s can do read-only ticket analysis itself, so a bootstrap that codes with it takes no separate planner; no proposal saved", coding) + } + plannerCommand, err := roleHarnessTarget(candidates, id, launcher, formula.AnalysisHarness, lab) + if err != nil { + return nil, nil, err + } + for i := range candidates { + if candidates[i].ID == id { + return &candidates[i], plannerCommand, nil + } + } + return nil, nil, fmt.Errorf("cannot bind harnesses.analysis to %q: no such harness was discovered; no proposal saved", id) } func Prepare(ctx context.Context, o InitOptions) (out Outcome, err error) { + if o.Planner != "" && (o.pluginInitRequested() || o.Preferred == "" || o.Rebind != "" || o.RoleHarness || o.EnableReview || o.RefreshBaseline || o.ModelSet || o.Role != "" || o.MetaHarness != "" || o.MetaCommand != nil) { + return out, fmt.Errorf("a planner is chosen only for a GitHub bootstrap with --harness ID; do not combine it with plugins, --rebind, --role, --enable-review, --refresh-baseline, --model, --meta-harness or --meta-command; no proposal saved") + } if o.pluginInitRequested() { return preparePluginInit(ctx, o) } @@ -546,6 +598,9 @@ func Prepare(ctx context.Context, o InitOptions) (out Outcome, err error) { if o.RoleHarness { return out, fmt.Errorf("a proposal is already pending; review, approve or dismiss it with init --lab %s --review/--approve/--dismiss %s before --harness --role", formula.ShellWord(o.LabDir), pending.ID) } + if o.Planner != "" { + return out, fmt.Errorf("a proposal is already pending; review, approve or dismiss it with init --lab %s --review/--approve/--dismiss %s before choosing a planner", formula.ShellWord(o.LabDir), pending.ID) + } f, e := formula.Parse([]byte(pending.YAML)) if e != nil { return out, e @@ -587,6 +642,9 @@ func Prepare(ctx context.Context, o InitOptions) (out Outcome, err error) { if o.RoleHarness && active.Formula.ID == "" { return out, fmt.Errorf("--harness --role needs an approved Formula; run init --lab %s and approve the bootstrap proposal first", formula.ShellWord(o.LabDir)) } + if o.Planner != "" && active.Formula.ID != "" { + return out, fmt.Errorf("a planner is chosen only for a bootstrap; this Lab already has a Formula, so run forgecell init --harness ID --role analysis after approval; no proposal saved") + } if active.Formula.ID != "" { approved, e := formula.Load(o.LabDir, "") if e != nil || !approved.Approved { @@ -691,7 +749,11 @@ func Prepare(ctx context.Context, o InitOptions) (out Outcome, err error) { // with a planning harness, or refuses before anything is saved. var planner *Candidate var plannerCommand []string - if !active.Approved { + if !active.Approved && o.Planner != "" { + if planner, plannerCommand, err = explicitBootstrapPlanner(out.Candidates, selected.ID, command, o.Planner, o.Launcher, o.LabDir); err != nil { + return out, err + } + } else if !active.Approved { if planner, plannerCommand, err = bootstrapPlanner(out.Candidates, selected.ID, command, o.Launcher, o.LabDir); err != nil { return out, err } @@ -755,7 +817,11 @@ func Prepare(ctx context.Context, o InitOptions) (out Outcome, err error) { out.Note = "Bootstrap Assay is read-only. Proposed checks are evidence, not executed commands. Approving binds the coding harness; runs still stop at human review." if planner != nil { recipe["harnesses"] = map[string]any{string(formula.AnalysisHarness): map[string]any{"binding": planner.ID, "command": plannerCommand}} - out.Note += fmt.Sprintf(" %s cannot do read-only ticket analysis, so harnesses.analysis binds planning to %s, the first ready harness that can (claude-code, then codex); %s still codes. To plan with another harness after approval, run forgecell init --harness ID --role analysis.", selected.ID, planner.ID, selected.ID) + chosen := "the first ready harness that can (claude-code, then codex)" + if o.Planner != "" { + chosen = "the planner you chose" + } + out.Note += fmt.Sprintf(" %s cannot do read-only ticket analysis, so harnesses.analysis binds planning to %s, %s; %s still codes. To plan with another harness after approval, run forgecell init --harness ID --role analysis.", selected.ID, planner.ID, chosen, selected.ID) } if len(baseline) > 0 { recipe["requiredChecks"] = baseline diff --git a/lab/internal/ui/setup.go b/lab/internal/ui/setup.go index 262093e..685276d 100644 --- a/lab/internal/ui/setup.go +++ b/lab/internal/ui/setup.go @@ -28,7 +28,7 @@ import ( // GET /api/setup → the step, read from the Lab's own state // POST /api/setup/check {} → onboarding.ReadRepository and Discover (read-only) // POST /api/setup/choose {"lab"} → one of the matching Labs, by index -// POST /api/setup/create {"harness"} → onboarding.Prepare, as forgecell init --harness ID +// POST /api/setup/create {"harness","planner"?} → onboarding.Prepare, as forgecell init --harness ID // POST /api/setup/approve {"proposal","digest"} → onboarding.Review approve, as init --approve // POST /api/setup/review {} → onboarding.Prepare EnableReview, a second proposal // @@ -146,6 +146,9 @@ type SetupView struct { Repository *SetupRepository `json:"repository,omitempty"` Agents []SetupAgent `json:"agents,omitempty"` Proposal *SetupProposal `json:"proposal,omitempty"` + // Planner is the planning agent a bootstrap pairs with a coding agent that + // cannot plan when none is chosen, as the engine picks it; empty when none is ready. + Planner string `json:"planner,omitempty"` // ReviewAtom is present, available or unavailable once the Formula is approved. ReviewAtom string `json:"reviewAtom,omitempty"` Problem string `json:"problem,omitempty"` @@ -180,6 +183,10 @@ type SetupAgent struct { Installed string `json:"installed"` SignIn string `json:"signIn"` Detail string `json:"detail"` + // Plans reports whether its adapter can do read-only ticket analysis; one + // that cannot codes only, and PlanReason is the adapter's reason. + Plans bool `json:"plans"` + PlanReason string `json:"planReason,omitempty"` } // SetupProposal is the waiting Formula proposal in plain words. ID and Digest @@ -190,9 +197,12 @@ type SetupProposal struct { // Kind is bootstrap, enable-review or change. Kind string `json:"kind"` // Resume marks an approval that started and must be finished. - Resume bool `json:"resume"` - Formula string `json:"formula"` - Binding string `json:"binding"` + Resume bool `json:"resume"` + Formula string `json:"formula"` + Binding string `json:"binding"` + // Planning and Coding are the bindings intake and coding resolve to. + Planning string `json:"planning,omitempty"` + Coding string `json:"coding,omitempty"` Atoms []SetupAtom `json:"atoms"` Checks []SetupCheck `json:"checks"` Gates []SetupGate `json:"gates"` @@ -337,6 +347,7 @@ func (h *handler) setupView() SetupView { repository := s.repository v.Repository = &repository v.Agents = agentViews(s.candidates) + v.Planner = setupDefaultPlanner(s.candidates, s.o.Launcher) } st, err := readSetupLab(s.lab) switch { @@ -389,6 +400,8 @@ type chooseRequest struct { type createRequest struct { Harness string `json:"harness"` + // Planner optionally chooses the planning agent of a coding agent that cannot plan. + Planner string `json:"planner"` } type approveRequest struct { @@ -506,6 +519,9 @@ func (h *handler) setupCreate(ctx context.Context, w http.ResponseWriter, req cr h.refuseGate(w, http.StatusBadRequest, "invalid", "choose claude-code, codex or cursor") return false } + if req.Planner != "" && !h.setupPlannerChoice(w, req) { + return false + } lab, ok := h.setupLabDir(w) if !ok { return false @@ -546,11 +562,18 @@ func (h *handler) setupCreate(ctx context.Context, w http.ResponseWriter, req cr h.refuseWith(w, http.StatusConflict, "agent", "that coding agent is not ready now; nothing was written", "Sign in with its own CLI, then Check Again") return false } + if req.Planner != "" { + j := slices.IndexFunc(candidates, func(c onboarding.Candidate) bool { return c.ID == req.Planner }) + if j < 0 || candidates[j].Readiness != "ready" || candidates[j].Executable == "" { + h.refuseWith(w, http.StatusConflict, "planner", "that planning agent is not ready now; nothing was written", "Sign in to "+setupHarnessNames[req.Planner]+" with its own CLI, then Check Again, or choose another planning agent") + return false + } + } env := map[string]string{} for k, v := range s.o.Env { env[k] = v } - out, err := s.o.Prepare(ctx, onboarding.InitOptions{Cwd: h.opts.SourceRoot, LabDir: lab, Launcher: s.o.Launcher, Preferred: req.Harness, Env: env, Candidates: candidates}) + out, err := s.o.Prepare(ctx, onboarding.InitOptions{Cwd: h.opts.SourceRoot, LabDir: lab, Launcher: s.o.Launcher, Preferred: req.Harness, Planner: req.Planner, Env: env, Candidates: candidates}) if err != nil { h.refuseWith(w, http.StatusUnprocessableEntity, "refused", "the engine did not create the Lab; nothing else was attempted", setupReason(err)) return false @@ -562,6 +585,45 @@ func (h *handler) setupCreate(ctx context.Context, w http.ResponseWriter, req cr return true } +// setupPlannerChoice checks a chosen planning agent before anything is read or +// probed: one of the offered agents, sent only with a coding agent that cannot +// plan, and able to plan itself. A planner that cannot plan is refused with its +// adapter's reason, redacted. +func (h *handler) setupPlannerChoice(w http.ResponseWriter, req createRequest) bool { + if !slices.Contains(SetupHarnesses, req.Planner) { + h.refuseGate(w, http.StatusBadRequest, "invalid", "choose claude-code or codex as the planning agent") + return false + } + if onboarding.PlanningCapability(req.Harness).State != "unsupported" { + h.refuseGate(w, http.StatusBadRequest, "invalid", setupHarnessNames[req.Harness]+" plans itself; send a planning agent only with a coding agent that cannot plan") + return false + } + if c := onboarding.PlanningCapability(req.Planner); c.State != "supported" { + h.refuseWith(w, http.StatusUnprocessableEntity, "planner", setupHarnessNames[req.Planner]+" cannot plan; nothing was written", setupPlanReason(c.Reason)) + return false + } + return true +} + +// setupPlanReason is an adapter's reason it cannot plan: one bounded, redacted +// line without its final stop. +func setupPlanReason(reason string) string { + if text := strings.TrimRight(setupText(reason), ". "); text != "" { + return text + } + return "Its adapter cannot do read-only ticket analysis" +} + +// setupDefaultPlanner is the planning agent the engine pairs with a coding agent +// that cannot plan when none is chosen; empty when none is ready. +func setupDefaultPlanner(candidates []onboarding.Candidate, launcher string) string { + id := onboarding.DefaultBootstrapPlanner(candidates, launcher) + if !slices.Contains(SetupHarnesses, id) { + return "" + } + return id +} + func (h *handler) setupApprove(w http.ResponseWriter, req approveRequest) bool { s := h.setup if !assayID.MatchString(req.Proposal) || !setupDigest.MatchString(req.Digest) { @@ -635,6 +697,8 @@ var setupReasons = []struct{ part, reason string }{ {"belongs to another repository", "This Lab belongs to another repository; run forgecell ui with --lab-name NAME for a separate Lab"}, {"repository discovery failed for gh", "Run gh auth login, then Check Again"}, {"launcher are required", "Install Forgecell so its launcher is known, then Check Again"}, + {"cannot bind harnesses.analysis", "Choose a ready planning agent, then Check Again"}, + {"planning needs a ready Claude Code or Codex harness", "Install or sign in to Claude Code or Codex for planning, then Check Again"}, } func setupReason(err error) string { @@ -679,6 +743,11 @@ func agentViews(candidates []onboarding.Candidate) []SetupAgent { var out []SetupAgent for _, id := range SetupHarnesses { a := SetupAgent{ID: id, Name: setupHarnessNames[id], Readiness: "missing", Installed: "missing", SignIn: "unknown", Detail: "Not found on this machine. Install it, sign in with its own CLI, then Check Again."} + if plan := onboarding.PlanningCapability(id); plan.State == "supported" { + a.Plans = true + } else { + a.PlanReason = setupPlanReason(plan.Reason) + } if i := slices.IndexFunc(candidates, func(c onboarding.Candidate) bool { return c.ID == id }); i >= 0 { c := candidates[i].CompleteUnknowns() a.Ready = c.Readiness == "ready" @@ -722,6 +791,9 @@ func proposalView(p onboarding.Proposal, resume bool) *SetupProposal { v.Kind = "enable-review" } v.Formula, v.Binding = setupText(f.Name), setupText(f.Harness.Binding) + planning, _ := f.ExecutionBinding(formula.IntakeRole) + coding, _ := f.ExecutionBinding(formula.Coding) + v.Planning, v.Coding = setupText(planning.Binding), setupText(coding.Binding) for i, a := range f.Atoms { if i == maxSetupItems { v.Truncated = true diff --git a/lab/internal/ui/setup_planner_test.go b/lab/internal/ui/setup_planner_test.go new file mode 100644 index 0000000..ef9a1f0 --- /dev/null +++ b/lab/internal/ui/setup_planner_test.go @@ -0,0 +1,243 @@ +package ui + +import ( + "context" + "net/http" + "os" + "strings" + "testing" + + "github.com/DanielNetzer/forgecell/lab/internal/harness" + "github.com/DanielNetzer/forgecell/lab/internal/onboarding" +) + +// plannerCandidate is a probed coding agent with the given readiness. +func plannerCandidate(id, readiness string) onboarding.Candidate { + auth := "verified" + if readiness != "ready" { + auth = readiness + } + return onboarding.Candidate{ID: id, Executable: localLeak + "/bin/" + id, Readiness: readiness, Detail: id + " probe at " + localLeak + " is " + readiness + ".", Installed: harness.Capability{State: "verified"}, Authentication: harness.Capability{State: auth}} +} + +// plannerFormula is the bootstrap Formula the fake Prepare proposes for o: the +// shared one, recoded with o.Preferred, and with harnesses.analysis on the +// chosen planner (or Claude Code, the engine's default) for Cursor. +func plannerFormula(o onboarding.InitOptions) string { + coding := o.Preferred + yaml := strings.ReplaceAll(setupFormula, "claude-code", coding) + if coding != "cursor" { + return yaml + } + planner := o.Planner + if planner == "" { + planner = "claude-code" + } + return strings.Replace(yaml, "atoms:\n", "harnesses:\n analysis: {binding: "+planner+", command: [/cli, __adapter, "+planner+", /"+planner+"]}\natoms:\n", 1) +} + +// plannerHandler serves setup with the fakeSetup seams, except that Prepare +// proposes plannerFormula, so the proposal shows the bindings it was given. +func plannerHandler(t *testing.T, f *fakeSetup) http.Handler { + t.Helper() + o := f.options_() + o.Prepare = func(_ context.Context, io onboarding.InitOptions) (onboarding.Outcome, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.unchanged("before Prepare") + defer func() { f.last = snapshot(f.t, f.lab) }() + f.prepares++ + f.options = append(f.options, io) + p, err := onboarding.SaveProposal(io.LabDir, plannerFormula(io)) + if err != nil { + f.t.Fatal(err) + } + return onboarding.Outcome{Status: "pending", Proposal: &p}, nil + } + return testHandler(t, Options{LabDir: f.lab, SourceRoot: "/src/checkout", Setup: o}) +} + +func (f *fakeSetup) lastOptions(t *testing.T) onboarding.InitOptions { + t.Helper() + f.mu.Lock() + defer f.mu.Unlock() + if len(f.options) == 0 { + t.Fatal("Prepare was not called") + } + return f.options[len(f.options)-1] +} + +// The agent step marks which agents can plan, says why Cursor codes only, and +// serves the engine's default planner: Claude Code, then Codex, else none. +func TestUISetupPlannerDefaultFollowsTheEngine(t *testing.T) { + for name, tc := range map[string]struct { + candidates []onboarding.Candidate + want string + }{ + "claude code first": {[]onboarding.Candidate{plannerCandidate("codex", "ready"), plannerCandidate("claude-code", "ready"), plannerCandidate("cursor", "ready")}, "claude-code"}, + "codex second": {[]onboarding.Candidate{plannerCandidate("claude-code", "login-required"), plannerCandidate("codex", "ready"), plannerCandidate("cursor", "ready")}, "codex"}, + "none ready": {[]onboarding.Candidate{plannerCandidate("claude-code", "login-required"), plannerCandidate("cursor", "ready")}, ""}, + } { + t.Run(name, func(t *testing.T) { + lab := t.TempDir() + f := newFakeSetup(t, lab) + f.candidates = tc.candidates + h := plannerHandler(t, f) + _, v, _ := setupPost(t, h, lab, "check", `{}`) + if v.Planner != tc.want { + t.Fatalf("default planner %q, want %q", v.Planner, tc.want) + } + if len(v.Agents) != 3 || !v.Agents[0].Plans || !v.Agents[1].Plans || v.Agents[2].ID != "cursor" || v.Agents[2].Plans { + t.Fatalf("agents %+v", v.Agents) + } + if v.Agents[0].PlanReason != "" || v.Agents[2].PlanReason == "" || strings.HasSuffix(v.Agents[2].PlanReason, ".") { + t.Fatalf("plan reasons %q %q", v.Agents[0].PlanReason, v.Agents[2].PlanReason) + } + if v := setupGet(t, h, lab); v.Planner != tc.want { + t.Fatalf("reload default planner %q", v.Planner) + } + // Without a choice, Create leaves the planner to the engine's default. + if tc.want == "" { + return + } + code, v, _ := setupPost(t, h, lab, "create", `{"harness":"cursor"}`) + if code != http.StatusOK || v.Proposal == nil { + t.Fatalf("create: %d %+v", code, v) + } + if o := f.lastOptions(t); o.Preferred != "cursor" || o.Planner != "" { + t.Fatalf("Prepare got %+v", o) + } + }) + } +} + +// An explicit Claude Code or Codex planner reaches Prepare, and the review step +// shows both bindings of the proposal: plans with it, codes with Cursor. +func TestUISetupPlannerExplicitChoiceReachesPrepare(t *testing.T) { + for _, planner := range []string{"claude-code", "codex"} { + t.Run(planner, func(t *testing.T) { + lab := t.TempDir() + f := newFakeSetup(t, lab) + f.candidates = []onboarding.Candidate{plannerCandidate("claude-code", "ready"), plannerCandidate("codex", "ready"), plannerCandidate("cursor", "ready")} + h := plannerHandler(t, f) + setupPost(t, h, lab, "check", `{}`) + code, v, refusal := setupPost(t, h, lab, "create", `{"harness":"cursor","planner":"`+planner+`"}`) + if code != http.StatusOK || v.Step != "review" || v.Proposal == nil { + t.Fatalf("create: %d %+v %+v", code, v, refusal) + } + o := f.lastOptions(t) + if o.Preferred != "cursor" || o.Planner != planner || o.LabDir != lab || o.Launcher != "/cli" || len(o.Candidates) != 3 { + t.Fatalf("Prepare got %+v", o) + } + if p := v.Proposal; p.Planning != planner || p.Coding != "cursor" || p.Binding != "cursor" { + t.Fatalf("review bindings: planning %q, coding %q, binding %q", p.Planning, p.Coding, p.Binding) + } + // A reload shows the same bindings, read from the saved proposal. + if again := setupGet(t, h, lab); again.Proposal == nil || again.Proposal.Planning != planner || again.Proposal.Coding != "cursor" || again.Proposal.Digest != v.Proposal.Digest { + t.Fatalf("reload %+v", again.Proposal) + } + }) + } +} + +// A planner that cannot plan, is not ready or is not offered is refused before +// Prepare with the adapter's reason or a fixed next step; nothing is written. +func TestUISetupPlannerRefusesUnreadyOrUnableToPlan(t *testing.T) { + cursorReason := strings.TrimRight(harness.AdapterCapabilities("cursor").Analysis.Reason, ". ") + for name, tc := range map[string]struct { + body string + code int + category string + reason string + probes bool + }{ + "cursor plans": {`{"harness":"cursor","planner":"cursor"}`, http.StatusUnprocessableEntity, "planner", cursorReason, false}, + "codex signed out": {`{"harness":"cursor","planner":"codex"}`, http.StatusConflict, "planner", "Sign in to Codex", true}, + "gone": {`{"harness":"cursor","planner":"claude-code"}`, http.StatusConflict, "planner", "Sign in to Claude Code", true}, + "not offered": {`{"harness":"cursor","planner":"../evil"}`, http.StatusBadRequest, "invalid", "", false}, + "coding agent plans": {`{"harness":"codex","planner":"claude-code"}`, http.StatusBadRequest, "invalid", "", false}, + "planner as an object": {`{"harness":"cursor","planner":{"id":"codex"}}`, http.StatusBadRequest, "invalid", "", false}, + "extra field": {`{"harness":"cursor","planner":"codex","x":1}`, http.StatusBadRequest, "invalid", "", false}, + } { + t.Run(name, func(t *testing.T) { + lab := t.TempDir() + f := newFakeSetup(t, lab) + f.candidates = []onboarding.Candidate{plannerCandidate("codex", "login-required"), plannerCandidate("cursor", "ready")} + if name != "gone" { + f.candidates = append(f.candidates, plannerCandidate("claude-code", "ready")) + } + h := plannerHandler(t, f) + setupPost(t, h, lab, "check", `{}`) + f.mu.Lock() + discovers := f.discovers + f.mu.Unlock() + code, _, refusal := setupPost(t, h, lab, "create", tc.body) + if code != tc.code || refusal.Category != tc.category || !strings.Contains(refusal.Reason, tc.reason) { + t.Fatalf("create %s: %d %+v", tc.body, code, refusal) + } + if tc.reason != "" && strings.Contains(refusal.Reason, localLeak) { + t.Fatalf("refusal leaks probe detail: %+v", refusal) + } + f.mu.Lock() + probed := f.discovers - discovers + f.mu.Unlock() + if want := map[bool]int{true: 1, false: 0}[tc.probes]; probed != want { + t.Fatalf("create probed %d times, want %d", probed, want) + } + if prepares, _ := f.calls(); prepares != 0 { + t.Fatalf("a refused planner reached Prepare %d times", prepares) + } + if entries, _ := os.ReadDir(lab); len(entries) != 0 { + t.Fatalf("a refused planner wrote %v", entries) + } + if v := setupGet(t, h, lab); v.Step != "agent" { + t.Fatalf("after a refused planner: %+v", v) + } + f.unchanged("after a refused planner") + }) + } +} + +// With no planner in the request, Claude Code and Codex setups reach Prepare +// with exactly the options they did before, and plan and code with themselves. +func TestUISetupPlannerUnchangedWithoutAChoice(t *testing.T) { + for _, id := range []string{"claude-code", "codex"} { + t.Run(id, func(t *testing.T) { + lab := t.TempDir() + f := newFakeSetup(t, lab) + f.candidates = []onboarding.Candidate{plannerCandidate("claude-code", "ready"), plannerCandidate("codex", "ready"), plannerCandidate("cursor", "ready")} + h := plannerHandler(t, f) + setupPost(t, h, lab, "check", `{}`) + code, v, _ := setupPost(t, h, lab, "create", `{"harness":"`+id+`"}`) + if code != http.StatusOK || v.Proposal == nil { + t.Fatalf("create: %d %+v", code, v) + } + o := f.lastOptions(t) + want := onboarding.InitOptions{Cwd: "/src/checkout", LabDir: lab, Launcher: "/cli", Preferred: id} + if o.Cwd != want.Cwd || o.LabDir != want.LabDir || o.Launcher != want.Launcher || o.Preferred != want.Preferred || o.Planner != "" || o.RoleHarness || o.Role != "" || o.EnableReview || len(o.Candidates) != 3 || len(o.Env) != 1 { + t.Fatalf("Prepare got %+v", o) + } + if p := v.Proposal; p.Planning != id || p.Coding != id || p.Binding != id { + t.Fatalf("review bindings: planning %q, coding %q, binding %q", p.Planning, p.Coding, p.Binding) + } + }) + } +} + +// The page renders the codes-only note, the planning choice and both review +// bindings as text, and the stylesheet gives them the setup conventions. +func TestUISetupPlannerViewRendersTextOnly(t *testing.T) { + h := testHandler(t, Options{LabDir: t.TempDir()}) + js := do(t, h, authed("/js/setup.js")).Body.String() + for _, want := range []string{"'Codes only: it needs a second agent for planning.'", "name: 'planner'", "planner: chosen.value", "view.planner", "'Plans with '", "' · Codes with '", "choice.hidden = hide; choice.disabled = hide;"} { + if !strings.Contains(js, want) { + t.Errorf("setup.js lacks %q", want) + } + } + css := do(t, h, authed("/css/setup.css")).Body.String() + for _, want := range []string{".setup-codes-only", ".setup-planners", ".setup-option input:focus-visible"} { + if !strings.Contains(css, want) { + t.Errorf("setup.css lacks %s", want) + } + } +} diff --git a/lab/internal/ui/web/css/setup.css b/lab/internal/ui/web/css/setup.css index d255d53..f8b8446 100644 --- a/lab/internal/ui/web/css/setup.css +++ b/lab/internal/ui/web/css/setup.css @@ -38,6 +38,10 @@ body.setup-mode .shell { grid-template-columns: minmax(0, 1fr); } .setup-option input:focus-visible, .setup-toggle input:focus-visible { outline: 2px solid var(--focus); outline-offset: 2px; } .setup-option-text { display: grid; gap: 2px; min-width: 0; } .setup-option-text small { color: var(--muted); } +.setup-codes-only { color: var(--needs-ink); } +.setup-planners { padding-top: 4px; } +.setup-planners legend { padding: 0; margin-bottom: 4px; } +.setup-bindings { margin: 0; } .setup-toggle { display: flex; align-items: flex-start; gap: 8px; min-height: 44px; touch-action: manipulation; } .setup-actions { display: flex; flex-wrap: wrap; gap: 10px; } .setup-actions .btn { touch-action: manipulation; } @@ -62,4 +66,5 @@ body.setup-mode .shell { grid-template-columns: minmax(0, 1fr); } .setup-card, .setup-head, .setup-option { background: Canvas; color: CanvasText; box-shadow: none; border: 1px solid CanvasText; } .setup-option:has(input:checked) { border-color: Highlight; } .setup-error { border: 1px solid CanvasText; } + .setup-codes-only { color: CanvasText; } } diff --git a/lab/internal/ui/web/js/setup.js b/lab/internal/ui/web/js/setup.js index efc8e22..e0467fb 100644 --- a/lab/internal/ui/web/js/setup.js +++ b/lab/internal/ui/web/js/setup.js @@ -18,12 +18,14 @@ const STEPS = [ const STEP_INDEX = { choose: 0, repository: 0, agent: 1, review: 3, done: 4, problem: 0 }; const KNOWN_STEPS = ['choose', 'repository', 'agent', 'review', 'done', 'problem']; const AGENT_IDS = ['claude-code', 'codex', 'cursor']; +const AGENT_NAMES = { 'claude-code': 'Claude Code', codex: 'Codex', cursor: 'Cursor' }; const ASSAY_ID = /^assay-[a-f0-9]{64}$/; const DIGEST = /^[a-f0-9]{64}$/; // setup is this view's state: the last view served, the action in flight, the -// inline error and the form it belongs to, and each card's last rendered key. -const setup = { root: null, view: null, pending: '', error: '', errorFor: '', keys: {}, timer: null, review: false, live: '' }; +// inline error and the form it belongs to, each card's last rendered key, and +// the coding and planning agents picked, kept across card rebuilds. +const setup = { root: null, view: null, pending: '', error: '', errorFor: '', keys: {}, timer: null, review: false, live: '', choice: { harness: '', planner: '' } }; const text = (value, fallback) => (typeof value === 'string' && value ? value : fallback); @@ -189,7 +191,7 @@ function render() { let previous = null; for (const [name, build] of cards) { const id = 'setup-card-' + name; - const key = JSON.stringify([view.step, view.busy, view[name === 'agent' ? 'agents' : name] || null, view.repository || null, view.proposal || null, view.reviewAtom || '', view.problem || '', view.labs || null, setup.pending, setup.error, setup.errorFor, setup.review]); + const key = JSON.stringify([view.step, view.busy, view[name === 'agent' ? 'agents' : name] || null, view.repository || null, view.proposal || null, view.reviewAtom || '', view.problem || '', view.labs || null, view.planner || '', setup.pending, setup.error, setup.errorFor, setup.review]); let node = document.getElementById(id); if (!node || setup.keys[id] !== key) { const focused = node && node.contains(document.activeElement) ? document.activeElement : null; @@ -280,21 +282,64 @@ function repositoryCard(view) { function agentCard(view) { const agents = (Array.isArray(view.agents) ? view.agents : []).filter((a) => a && AGENT_IDS.includes(a.id)); - const firstReady = agents.find((a) => a.ready === true); - const ready = agents.filter((a) => a.ready === true).length; + const readyAgents = agents.filter((a) => a.ready === true); + const ready = readyAgents.length; + const picked = readyAgents.find((a) => a.id === setup.choice.harness) || readyAgents[0]; + // Planners are the ready agents that can plan, offered when the coding agent cannot. + const planners = readyAgents.filter((a) => a.plans === true); + const planner = planners.find((a) => a.id === setup.choice.planner) || planners.find((a) => a.id === view.planner) || planners[0]; + const needsPlanner = (id) => agents.some((a) => a.id === id && a.plans === false); return card('2. Coding agent', el('p', { class: 'setup-note' }, ready ? 'Pick the coding agent that writes changes. You can change it later with a reviewed Formula change.' : 'No coding agent is ready yet. Install one and sign in with its own CLI, then Check Again.'), - form('agent', (f) => { const picked = f.querySelector('input[name="harness"]:checked'); if (picked) submit('create', { harness: picked.value }, 'agent'); }, - el('fieldset', { class: 'setup-options' }, el('legend', { class: 'sr-only' }, 'Coding agents'), + form('agent', (f) => { + const harness = f.querySelector('input[name="harness"]:checked'); + if (!harness) return; + if (!needsPlanner(harness.value)) { submit('create', { harness: harness.value }, 'agent'); return; } + const chosen = f.querySelector('input[name="planner"]:checked'); + if (chosen) { submit('create', { harness: harness.value, planner: chosen.value }, 'agent'); return; } + setup.error = 'Choose a planning agent first. Install or sign in to Claude Code or Codex, then Check Again.'; + setup.errorFor = 'agent'; + render(); + focusError('agent'); + }, + el('fieldset', { class: 'setup-options', onchange: (e) => pickAgent(e.target, needsPlanner) }, el('legend', { class: 'sr-only' }, 'Coding agents'), agents.map((a) => el('label', { class: 'setup-option', for: fieldId('harness-' + a.id), dataset: { ready: a.ready === true ? 'yes' : 'no' } }, - el('input', { type: 'radio', id: fieldId('harness-' + a.id), name: 'harness', value: a.id, required: true, disabled: a.ready !== true, checked: a === firstReady, 'aria-describedby': 'setup-agent-' + a.id }), + el('input', { type: 'radio', id: fieldId('harness-' + a.id), name: 'harness', value: a.id, required: true, disabled: a.ready !== true, checked: a === picked, 'aria-describedby': 'setup-agent-' + a.id + (a.plans === false ? ' setup-agent-plan-' + a.id : '') }), el('span', { class: 'setup-option-text' }, el('strong', null, text(a.name, a.id)), - el('small', { id: 'setup-agent-' + a.id }, agentLine(a)))))), + el('small', { id: 'setup-agent-' + a.id }, agentLine(a)), + a.plans === false ? el('small', { id: 'setup-agent-plan-' + a.id, class: 'setup-codes-only' }, 'Codes only: it needs a second agent for planning.') : null)))), + plannerChoice(planners, planner, !(picked && needsPlanner(picked.id))), el('p', { class: 'setup-hint' }, '3. Create the Lab: Forgecell saves the Lab and proposes its first Formula, as forgecell init --harness does. Nothing is approved yet.'), el('div', { class: 'setup-actions' }, ready ? submitButton('create', 'Create Lab', true) : null))); } +// pickAgent keeps the picked agents and shows the planning choice only while +// the picked coding agent cannot plan. A hidden choice is disabled, so its +// required radios never block the form. +function pickAgent(input, needsPlanner) { + if (!input || input.type !== 'radio') return; + if (input.name === 'planner') { setup.choice.planner = input.value; return; } + if (input.name !== 'harness') return; + setup.choice.harness = input.value; + const choice = document.getElementById(fieldId('planners')); + if (choice) { const hide = !needsPlanner(input.value); choice.hidden = hide; choice.disabled = hide; } +} + +// plannerChoice lists the ready agents that can plan, the served default first +// picked, for a coding agent that codes only. +function plannerChoice(planners, picked, hidden) { + return el('fieldset', { id: fieldId('planners'), class: 'setup-options setup-planners', hidden: hidden, disabled: hidden, onchange: (e) => pickAgent(e.target, () => true) }, + el('legend', { class: 'setup-sub' }, 'Planning agent'), + el('p', { class: 'setup-hint' }, 'This coding agent codes only. Pick the agent that reads each Issue and plans the change; you can change it later with forgecell init --harness ID --role analysis.'), + planners.length ? planners.map((a) => el('label', { class: 'setup-option', for: fieldId('planner-' + a.id) }, + el('input', { type: 'radio', id: fieldId('planner-' + a.id), name: 'planner', value: a.id, required: true, checked: a === picked }), + el('span', { class: 'setup-option-text' }, el('strong', null, 'Plans with ' + text(a.name, a.id)), el('small', null, 'Ready · reads the Issue and proposes the plan you approve')))) + : el('p', { class: 'setup-note setup-warn' }, 'No planning agent is ready. Install or sign in to Claude Code or Codex, then Check Again.')); +} + +const agentName = (id) => AGENT_NAMES[id] || text(id, 'Not recorded'); + const SIGN_IN = { verified: 'Signed in', 'login-required': 'Sign-in needed', unknown: 'Sign-in not verified' }; function agentLine(a) { const parts = [a.ready === true ? 'Ready' : 'Not available', SIGN_IN[a.signIn] || 'Sign-in not verified']; @@ -313,7 +358,8 @@ function reviewCard(view) { : 'Formula ready for your approval. Approving binds the coding agent; every run still stops for your plan approval and your review.'; return card(title, el('p', { class: 'setup-note' }, intro), - el('p', { class: 'setup-formula' }, text(p.formula, 'Formula'), ' · coding agent ', el('code', null, text(p.binding, 'Not recorded'))), + el('p', { class: 'setup-formula' }, text(p.formula, 'Formula')), + el('p', { class: 'setup-bindings' }, 'Plans with ', el('strong', null, agentName(text(p.planning, p.binding))), ' · Codes with ', el('strong', null, agentName(text(p.coding, p.binding)))), el('h3', { class: 'setup-sub' }, 'Atoms, in order'), el('ol', { class: 'setup-atoms' }, atoms.map((a) => el('li', null, text(a.label, 'Atom'), ' ', el('small', null, text(a.type, ''))))), el('h3', { class: 'setup-sub' }, 'Baseline checks from CI ', el('span', { class: 'setup-count' }, String(checks.length))),