diff --git a/cmd/entire/cli/runner_apply.go b/cmd/entire/cli/runner_apply.go index f357a8c03b..ff91908bc7 100644 --- a/cmd/entire/cli/runner_apply.go +++ b/cmd/entire/cli/runner_apply.go @@ -2,14 +2,234 @@ package cli import ( "bytes" + "context" "encoding/json" "errors" "fmt" + "io" + "maps" + "path/filepath" "regexp" + "slices" "sort" "strings" + + "github.com/entireio/cli/cmd/entire/cli/entiredir" + "github.com/entireio/cli/cmd/entire/cli/paths" + + "github.com/sergi/go-diff/diffmatchpatch" ) +// tunedRunner is one accepted rewrite: the runner, its new file bytes, and the +// new template on its own. The template is kept because --dry-run diffs the +// template rather than the JSON file. +type tunedRunner struct { + runner tuneRunner + newRaw []byte + template string +} + +// runTuning runs the prompt through an already-resolved summary provider +// (prompt -> text) and turns the runner-id -> template map it returns into +// accepted rewrites. Rejections — out of scope, invalid template, unpatchable +// file — are reported on errW and counted rather than returned, so one bad +// runner does not sink the rest. Nothing is written here: the caller chooses +// between applying the changes and previewing them. +// +// The provider is resolved by the caller, before it gathers repository signal: +// resolution can fail outright or stop to ask which provider to use, and +// neither belongs after the seconds the gather costs. +func runTuning(ctx context.Context, errW io.Writer, provider *checkpointSummaryProvider, runners []tuneRunner, prompt, debugDir string) (changes []tunedRunner, skipped int, err error) { + // provider.TextGenerator is the plain prompt->text generator, and is + // guaranteed non-nil: its constructor fails when the agent has none. + // provider.Generator is deliberately not used — that is a + // summarize.Generator, which turns a transcript into a checkpoint Summary. + stop := startSpinner(errW, fmt.Sprintf("Tuning %d runner(s) with %s", len(runners), provider.DisplayName)) + out, err := provider.TextGenerator.GenerateText(ctx, prompt, provider.Model) + stop(err == nil) + if err != nil { + return nil, 0, fmt.Errorf("agent run failed: %w", err) + } + if debugDir != "" { + writeTuneDebug(errW, debugDir, "response.txt", out) + } + + templates, err := parseTuneOutput(out) + if err != nil { + return nil, 0, err + } + changes, skipped = classifyTuneProposals(errW, runners, templates) + return changes, skipped, nil +} + +// classifyTuneProposals turns the model's runner-id -> template map into the +// rewrites that will actually be used, reporting each rejection on errW and +// counting it. It is separate from the provider call so the accept/reject rules +// can be tested against canned proposals. +// +// A rejection is never fatal: one unusable proposal must not cost the other +// runners their tailoring. +func classifyTuneProposals(errW io.Writer, runners []tuneRunner, templates map[string]string) (changes []tunedRunner, skipped int) { + byID := make(map[string]tuneRunner, len(runners)) + for _, r := range runners { + byID[normalizeRunnerID(r.ID)] = r + } + + // Sorted so the skip/note messages and the preview diff come out in a stable + // order rather than in Go's randomized map order. + for _, id := range slices.Sorted(maps.Keys(templates)) { + tmpl := templates[id] + r, ok := byID[normalizeRunnerID(id)] + if !ok { + fmt.Fprintf(errW, "skip %q: not a runner in scope\n", id) + skipped++ + continue + } + if err := validateNewTemplate(r.Template, tmpl); err != nil { + fmt.Fprintf(errW, "skip %s: %v\n", r.ID, err) + skipped++ + continue + } + if dropped := droppedPlaceholders(r.Template, tmpl); len(dropped) > 0 { + fmt.Fprintf(errW, "note: %s no longer references %v\n", r.ID, dropped) + } + newRaw, err := replaceRunnerTemplate(r.Raw, tmpl) + if err != nil { + fmt.Fprintf(errW, "skip %s: %v\n", r.ID, err) + skipped++ + continue + } + if bytes.Equal(newRaw, r.Raw) { + continue // model returned the current template verbatim — benign no-op + } + changes = append(changes, tunedRunner{runner: r, newRaw: newRaw, template: tmpl}) + } + return changes, skipped +} + +// applyTunedRunners writes each accepted rewrite over its runner file. +// createdIDs are runners this invocation just scaffolded from defaults; any of +// those left un-tailored is flagged so it is not committed as if it were +// repo-specific. +func applyTunedRunners(w, errW io.Writer, repoRoot string, changes []tunedRunner, skipped int, createdIDs []string) error { + root, err := entiredir.OpenAt(repoRoot) + if err != nil { + return fmt.Errorf("open %s: %w", paths.EntireDir, err) + } + + tailored := make(map[string]bool, len(changes)) + for _, c := range changes { + if err := entiredir.WriteFile(root, c.runner.Name, c.newRaw, 0o644); err != nil { + return fmt.Errorf("writing %s: %w", c.runner.Path, err) + } + fmt.Fprintf(w, "updated %s\n", filepath.Base(c.runner.Path)) + tailored[normalizeRunnerID(c.runner.ID)] = true + } + + switch { + case len(changes) > 0: + fmt.Fprintf(w, "\nUpdated %d runner(s). Review with: git diff %s\n", + len(changes), filepath.Join(paths.EntireDir, runnersName)) + case len(createdIDs) == 0 && skipped > 0: + // Existing runners, model proposed templates, all rejected — a failed run. + // (When onboarding just created the set, an un-tailored runner is reported + // below as a generic default instead, which is more actionable.) + return fmt.Errorf("model proposed %d template(s) but all were rejected or out of scope (see messages above)", skipped) + case len(createdIDs) == 0: + fmt.Fprintln(w, "No runner changes proposed.") + } + + // Runners onboarding scaffolded but tailoring did not change remain the + // generic defaults. Those are working minimal prompts (valid output + // contract), so they are committable as-is — just note which are generic. + if untailored := untailoredRunners(createdIDs, tailored); len(untailored) > 0 { + fmt.Fprintf(errW, "\n%d runner(s) kept as working defaults (generic, not tailored to this repo): %s\n", + len(untailored), strings.Join(untailored, ", ")) + fmt.Fprintln(errW, "They are functional as-is; re-run `entire runner setup -y` to tailor them.") + } + return nil +} + +// previewTunedRunners prints what tailoring would change and writes nothing. +// It diffs each runner's prompt template rather than its JSON file: the +// template is the only field that changes and it is stored as one long JSON +// string, so a file-level diff would be a single unreadable line. +func previewTunedRunners(w, errW io.Writer, inScope int, changes []tunedRunner, skipped int) { + for _, c := range changes { + fmt.Fprintf(w, "=== %s — prompt.template would change ===\n", c.runner.ID) + fmt.Fprint(w, renderTemplateDiff(c.runner.Template, c.template)) + fmt.Fprintln(w) + } + + switch { + case len(changes) > 0: + fmt.Fprintf(errW, "%d of %d runner(s) would change", len(changes), inScope) + if skipped > 0 { + fmt.Fprintf(errW, ", %d proposal(s) rejected", skipped) + } + fmt.Fprintln(errW, ". Nothing was written — re-run with --yes to apply.") + case skipped > 0: + fmt.Fprintf(errW, "No runner would change: all %d proposal(s) were rejected or out of scope (see messages above).\n", skipped) + default: + fmt.Fprintln(errW, "No runner changes proposed.") + } +} + +// diffContextLines is how many unchanged template lines to keep either side of +// a change in the --dry-run preview. A tailored template is mostly rewritten, +// so the point of the collapse is the long unchanged tail (the output-JSON +// contract), not economy on the changed part. +const diffContextLines = 3 + +// renderTemplateDiff renders a line-level diff of two prompt templates, with +// unchanged runs longer than twice the context collapsed to a count. +// diffmatchpatch is character-oriented, so the templates are folded to one +// char per line first (the DiffLinesToChars/DiffCharsToLines pattern, as in +// strategy/manual_commit_attribution.go). +func renderTemplateDiff(oldText, newText string) string { + dmp := diffmatchpatch.New() + a, b, lines := dmp.DiffLinesToChars(oldText, newText) + diffs := dmp.DiffCharsToLines(dmp.DiffMain(a, b, false), lines) + + var out strings.Builder + for _, d := range diffs { + switch d.Type { + case diffmatchpatch.DiffInsert: + writeDiffLines(&out, "+", splitLines(d.Text)) + case diffmatchpatch.DiffDelete: + writeDiffLines(&out, "-", splitLines(d.Text)) + case diffmatchpatch.DiffEqual: + ls := splitLines(d.Text) + if len(ls) <= 2*diffContextLines { + writeDiffLines(&out, " ", ls) + continue + } + writeDiffLines(&out, " ", ls[:diffContextLines]) + fmt.Fprintf(&out, "@@ %d unchanged line(s) @@\n", len(ls)-2*diffContextLines) + writeDiffLines(&out, " ", ls[len(ls)-diffContextLines:]) + } + } + return out.String() +} + +func writeDiffLines(out *strings.Builder, prefix string, lines []string) { + for _, line := range lines { + out.WriteString(prefix) + out.WriteString(line) + out.WriteByte('\n') + } +} + +// splitLines splits a diff chunk into lines, dropping the empty element a +// trailing newline produces so it is not rendered as a blank diff line. The +// empty-string guard matters: Split("") is [""], which would render one blank. +func splitLines(s string) []string { + if s == "" { + return nil + } + return strings.Split(strings.TrimSuffix(s, "\n"), "\n") +} + // parseTuneOutput extracts the runner-id -> new-template map the tuning model // is instructed to emit as a single JSON object. The model may wrap the object // in prose or code fences, so we slice from the first "{" to the last "}". An diff --git a/cmd/entire/cli/runner_init.go b/cmd/entire/cli/runner_init.go index 24abeebda1..96ecbce739 100644 --- a/cmd/entire/cli/runner_init.go +++ b/cmd/entire/cli/runner_init.go @@ -1,14 +1,13 @@ package cli import ( - "errors" + "context" "fmt" "io" "path/filepath" "strings" "github.com/entireio/cli/cmd/entire/cli/entiredir" - "github.com/entireio/cli/cmd/entire/cli/interactive" "github.com/entireio/cli/cmd/entire/cli/osroot" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/runnerdefaults" @@ -16,36 +15,24 @@ import ( "charm.land/huh/v2" ) -// ensureRunnersPresent scaffolds the default runner set when a repo has none -// yet, so `tune` doubles as onboarding. It returns the IDs it created (nil when -// runners already existed) so the caller can flag any that tuning then leaves -// un-tailored. It is a no-op when runners already exist, and errors when the -// user declined or creation failed. Writing is gated on confirmation -// (interactive prompt, or the --yes flag for non-interactive runs). -func ensureRunnersPresent(w, errW io.Writer, repoRoot string, assumeYes bool) (created []string, err error) { +// createDefaultRunners writes the default runner set, so setup doubles as +// onboarding. It returns the IDs it created, so the caller can flag any that +// tailoring then leaves un-tailored. +// +// Two things it deliberately does not do. It does not ask — consent belongs to +// the mode decision in resolveRunnerSetupMode, so one answer covers every +// prompt the command can raise, and runnerSetupMode.writesRunnerFiles is what +// gates the call. And it does not re-check whether runners already exist: the +// caller has that answer already, and deriving it twice gave the invariant two +// homes that could drift. +func createDefaultRunners(w io.Writer, repoRoot string) (created []string, err error) { dir := runnersDir(repoRoot) - if runnerConfigsExist(repoRoot) { - return nil, nil - } defaults, err := runnerdefaults.Files() if err != nil { return nil, fmt.Errorf("loading default runners: %w", err) } - if !assumeYes { - if !interactive.CanPromptInteractively() { - return nil, fmt.Errorf("no runner configs found under %s; re-run with --yes to create the default set (%d runners)", dir, len(defaults)) - } - confirmed, err := confirmCreateRunners(len(defaults)) - if err != nil { - return nil, err - } - if !confirmed { - return nil, errors.New("no runner configs created (declined)") - } - } - root, err := entiredir.OpenAt(repoRoot) if err != nil { return nil, fmt.Errorf("creating %s: %w", dir, err) @@ -61,7 +48,6 @@ func ensureRunnersPresent(w, errW io.Writer, repoRoot string, assumeYes bool) (c fmt.Fprintf(w, "created %s\n", filepath.Join(paths.EntireDir, runnersName, f.Name)) created = append(created, strings.TrimSuffix(f.Name, ".json")) } - fmt.Fprintf(errW, "Created %d default runner(s); tailoring them to this repo…\n", len(defaults)) return created, nil } @@ -85,36 +71,42 @@ func runnerConfigsExist(repoRoot string) bool { return false } -func confirmCreateRunners(n int) (bool, error) { - var ok bool - form := NewAccessibleForm( - huh.NewGroup( - huh.NewConfirm(). - Title(fmt.Sprintf("No trail runners found. Create the default set (%d runners) in .entire/runners/?", n)). - Description("Written from the built-in defaults, then tailored to this repo."). - Value(&ok), - ), - ) - if err := form.Run(); err != nil { - return false, fmt.Errorf("runner-creation prompt cancelled: %w", err) +// chooseRunnerSetupAction asks a terminal what setup should do. The choice is +// the user's own framing of the command — generic defaults, or defaults +// tailored to this repo — and it is asked once, whether or not the repo already +// has runners, so that the answer settles both creating and tailoring. +// Cancelling the form yields setupModeNone, which authorizes nothing. +func chooseRunnerSetupAction(ctx context.Context, errW io.Writer, haveRunners bool) (runnerSetupMode, error) { + title := "Runners are already configured for this repo. Tailor them to this repo now?" + adaptLabel := "Tailor them to this repo" + keepLabel := "Leave them as they are" + if !haveRunners { + defaults, err := runnerdefaults.Files() + if err != nil { + return setupModeNone, fmt.Errorf("loading default runners: %w", err) + } + title = fmt.Sprintf("No trail runners found. What should setup do? (%d runners)", len(defaults)) + adaptLabel = "Create the defaults and tailor them to this repo" + keepLabel = "Create the generic defaults only" } - return ok, nil -} -// confirmTuneExisting asks whether to re-tailor runners that already exist — -// the re-run case where there is nothing to scaffold. -func confirmTuneExisting() (bool, error) { - var ok bool + mode := setupModeAdapt form := NewAccessibleForm( huh.NewGroup( - huh.NewConfirm(). - Title("Runners are already configured for this repo. Tune them to this repo now?"). - Description("Re-tailors the runner prompts using fresh repository signal."). - Value(&ok), + huh.NewSelect[runnerSetupMode](). + Title(title). + Description("Tailoring rewrites each runner's prompt from this repo's docs, history and past findings — one call to your configured summary provider."). + Options( + huh.NewOption(adaptLabel, setupModeAdapt), + huh.NewOption(keepLabel, setupModeDefaults), + ). + Value(&mode), ), ) - if err := form.Run(); err != nil { - return false, fmt.Errorf("runner-tune prompt cancelled: %w", err) + // RunWithContext, not Run: the command's context must be able to abort the + // form, and it is what puts context.Canceled in reach of the handler. + if err := form.RunWithContext(ctx); err != nil { + return setupModeNone, handleFormCancellation(errW, "Runner setup", err) } - return ok, nil + return mode, nil } diff --git a/cmd/entire/cli/runner_init_test.go b/cmd/entire/cli/runner_init_test.go index a76d9b11f8..1c502dbdba 100644 --- a/cmd/entire/cli/runner_init_test.go +++ b/cmd/entire/cli/runner_init_test.go @@ -75,15 +75,15 @@ func TestWriteTuneDebug(t *testing.T) { } } -func TestEnsureRunnersPresent_CreatesDefaultsWhenEmpty(t *testing.T) { +func TestCreateDefaultRunners_WritesTheWholeSet(t *testing.T) { t.Parallel() repoRoot := t.TempDir() - var out, errOut bytes.Buffer + var out bytes.Buffer - created, err := ensureRunnersPresent(&out, &errOut, repoRoot, true /* assumeYes */) + created, err := createDefaultRunners(&out, repoRoot) if err != nil { - t.Fatalf("ensureRunnersPresent: %v", err) + t.Fatalf("createDefaultRunners: %v", err) } if len(created) < 6 { t.Fatalf("expected >=6 created runner IDs, got %d: %v", len(created), created) @@ -106,32 +106,29 @@ func TestEnsureRunnersPresent_CreatesDefaultsWhenEmpty(t *testing.T) { } } -func TestEnsureRunnersPresent_NoopWhenRunnersExist(t *testing.T) { +// TestRunnerConfigsExist_GatesTheScaffold covers what used to be +// createDefaultRunners' own no-op check: the caller asks this predicate and +// only writes when it says the repo has none, so the invariant has one home. +func TestRunnerConfigsExist_GatesTheScaffold(t *testing.T) { t.Parallel() repoRoot := t.TempDir() + if runnerConfigsExist(repoRoot) { + t.Error("an empty repo should report no runner configs") + } + dir := filepath.Join(repoRoot, ".entire", "runners") if err := os.MkdirAll(dir, 0o755); err != nil { t.Fatal(err) } + if runnerConfigsExist(repoRoot) { + t.Error("an empty runners directory should report no runner configs") + } if err := os.WriteFile(filepath.Join(dir, "trail-risk.json"), []byte(`{"id":"trail-risk","prompt":{"template":"x"}}`), 0o644); err != nil { t.Fatal(err) } - - created, err := ensureRunnersPresent(&bytes.Buffer{}, &bytes.Buffer{}, repoRoot, true) - if err != nil { - t.Fatalf("ensureRunnersPresent: %v", err) - } - if len(created) != 0 { - t.Errorf("expected no created runners when they already exist, got %v", created) - } - // No defaults should have been scaffolded over the existing runner. - after, err := filepath.Glob(filepath.Join(dir, "*.json")) - if err != nil { - t.Fatal(err) - } - if len(after) != 1 { - t.Errorf("expected the existing single runner untouched, got %d files", len(after)) + if !runnerConfigsExist(repoRoot) { + t.Error("a repo with a runner config should report that it has one") } } diff --git a/cmd/entire/cli/runner_prompt.go b/cmd/entire/cli/runner_prompt.go index 6e018ae4a0..aafa1c9788 100644 --- a/cmd/entire/cli/runner_prompt.go +++ b/cmd/entire/cli/runner_prompt.go @@ -11,6 +11,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/entiredir" "github.com/entireio/cli/cmd/entire/cli/osroot" "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/runnerdefaults" ) // runnersName is the runner-config directory relative to the .entire root, which @@ -50,41 +51,66 @@ func loadTuneRunners(repoRoot, filter string) ([]tuneRunner, error) { return nil, fmt.Errorf("reading %s: %w", dir, err) } - var runners []tuneRunner + var files []runnerdefaults.File for _, e := range entries { if e.IsDir() || !strings.HasSuffix(e.Name(), ".json") { continue } - name := runnersName + "/" + e.Name() - path := filepath.Join(dir, e.Name()) - raw, err := entiredir.ReadFile(root, name) + raw, err := entiredir.ReadFile(root, runnersName+"/"+e.Name()) if err != nil { - return nil, fmt.Errorf("reading %s: %w", path, err) + return nil, fmt.Errorf("reading %s: %w", filepath.Join(dir, e.Name()), err) } + files = append(files, runnerdefaults.File{Name: e.Name(), Data: raw}) + } + return parseTuneRunners(files, dir, filter) +} + +// defaultTuneRunners returns the embedded default set as tuneRunner values +// without touching the repo. It backs --dry-run in a repo that has no runners +// yet: the preview is of tailoring the set setup would have created, and a dry +// run must not create it. Name and Path describe where each file would go, so +// messages read the same as the on-disk case; nothing writes them. +func defaultTuneRunners(repoRoot, filter string) ([]tuneRunner, error) { + files, err := runnerdefaults.Files() + if err != nil { + return nil, fmt.Errorf("loading default runners: %w", err) + } + return parseTuneRunners(files, runnersDir(repoRoot), filter) +} + +// parseTuneRunners turns raw runner files into tuneRunner values: it reads each +// id and current template, applies the id filter, and sorts by id. dir is the +// directory the files belong to, used for the file paths in messages. +func parseTuneRunners(files []runnerdefaults.File, dir, filter string) ([]tuneRunner, error) { + var runners []tuneRunner + for _, f := range files { + path := filepath.Join(dir, f.Name) var doc struct { ID string `json:"id"` Prompt struct { Template string `json:"template"` } `json:"prompt"` } - if err := json.Unmarshal(raw, &doc); err != nil { + if err := json.Unmarshal(f.Data, &doc); err != nil { return nil, fmt.Errorf("parsing %s: %w", path, err) } if doc.ID == "" { - doc.ID = strings.TrimSuffix(e.Name(), ".json") + doc.ID = strings.TrimSuffix(f.Name, ".json") } runners = append(runners, tuneRunner{ ID: doc.ID, - Name: name, + Name: runnersName + "/" + f.Name, Path: path, - Raw: raw, + Raw: f.Data, Template: doc.Prompt.Template, }) } if filter != "" { + // A fresh slice, not runners[:0]: compacting in place would keep every + // unselected runner's Raw bytes alive through the shared array. want := normalizeRunnerID(filter) - filtered := runners[:0] + var filtered []tuneRunner for _, r := range runners { if normalizeRunnerID(r.ID) == want { filtered = append(filtered, r) diff --git a/cmd/entire/cli/runner_prompt_test.go b/cmd/entire/cli/runner_prompt_test.go index 2fa8732cbf..92c9154a41 100644 --- a/cmd/entire/cli/runner_prompt_test.go +++ b/cmd/entire/cli/runner_prompt_test.go @@ -80,14 +80,29 @@ func TestLoadTuneRunners_Errors(t *testing.T) { } } +// The limit is validated once a mode that gathers has been chosen, so this +// names one (--print-prompt needs no provider) and runs in an isolated repo. +// --defaults-only deliberately does NOT reach this check: it reads no signal. func TestRunRunnerSetupRejectsNonPositiveLimit(t *testing.T) { - t.Parallel() + repoRoot := newRunnerSetupRepo(t) for _, limit := range []int{0, -1} { - err := runRunnerSetup(context.Background(), io.Discard, io.Discard, runnerSetupOptions{limit: limit}) + err := runRunnerSetup(context.Background(), io.Discard, io.Discard, runnerSetupOptions{ + printPrompt: true, + limit: limit, + }) if err == nil || err.Error() != runnerSetupLimitErrorMessage { t.Fatalf("limit %d error = %v, want limit validation error", limit, err) } } + if err := runRunnerSetup(context.Background(), io.Discard, io.Discard, runnerSetupOptions{ + defaultsOnly: true, + limit: 0, + }); err != nil { + t.Errorf("--defaults-only reads no signal, so --limit must not gate it: %v", err) + } + if written := runnerFiles(t, repoRoot); len(written) != wantDefaultCount(t) { + t.Errorf("--defaults-only wrote %d runner file(s), want the full default set", len(written)) + } } func TestParseTuneSources(t *testing.T) { diff --git a/cmd/entire/cli/runner_setup.go b/cmd/entire/cli/runner_setup.go index 9b19921ee5..f673af5b31 100644 --- a/cmd/entire/cli/runner_setup.go +++ b/cmd/entire/cli/runner_setup.go @@ -1,17 +1,13 @@ package cli import ( - "bytes" "context" "errors" "fmt" "io" "os" "path/filepath" - "strings" - "github.com/entireio/cli/cmd/entire/cli/agent" - "github.com/entireio/cli/cmd/entire/cli/entiredir" "github.com/entireio/cli/cmd/entire/cli/interactive" "github.com/entireio/cli/cmd/entire/cli/paths" @@ -20,11 +16,78 @@ import ( const runnerSetupLimitErrorMessage = "limit must be greater than 0" +// runnerSetupMode is what one `runner setup` invocation does. The modes differ +// in how far they go, not in whether they ask: consent is settled once, when +// the mode is resolved, which is what makes --yes answer every question the +// command can raise. It previously answered only the create-defaults +// confirmation and left the tailoring one to prompt anyway. +type runnerSetupMode string + +const ( + // setupModeAdapt creates the defaults when the repo has none, then tailors + // every runner in scope to this repo, in place. The full action, so it is + // what --yes and the interactive default select. + setupModeAdapt runnerSetupMode = "adapt" + // setupModeDefaults creates the generic defaults and stops. No provider call. + setupModeDefaults runnerSetupMode = "defaults" + // setupModePrintPrompt creates the defaults as needed and prints the + // tailoring prompt for the caller's own agent instead of running a provider. + setupModePrintPrompt runnerSetupMode = "print-prompt" + // setupModeDryRun tailors and prints the result as a diff, writing nothing — + // not even the defaults, so in a fresh repo it previews the embedded set. + setupModeDryRun runnerSetupMode = "dry-run" + // setupModeNone is the zero value: no mode was chosen, because the picker + // was cancelled. It authorizes nothing, which is why both predicates below + // answer false for it. + setupModeNone runnerSetupMode = "" +) + +// writesRunnerFiles reports whether this mode may create or rewrite files under +// .entire/runners. It is the consent question, asked positively and answered by +// a total switch, so an unhandled mode writes nothing rather than everything — +// the zero value used to fall through to the most invasive path. `exhaustive` +// makes a new mode a build failure here rather than a silent write. +func (m runnerSetupMode) writesRunnerFiles() bool { + switch m { + case setupModeAdapt, setupModeDefaults, setupModePrintPrompt: + return true + case setupModeDryRun, setupModeNone: + return false + } + return false +} + +// gathersSignal reports whether this mode reads repository signal, and so +// whether --sources and --limit apply to it at all. +func (m runnerSetupMode) gathersSignal() bool { + switch m { + case setupModeAdapt, setupModePrintPrompt, setupModeDryRun: + return true + case setupModeDefaults, setupModeNone: + return false + } + return false +} + +// needsProvider reports whether this mode calls the summary provider, so the +// caller can resolve it before spending seconds gathering signal for it. +func (m runnerSetupMode) needsProvider() bool { + switch m { + case setupModeAdapt, setupModeDryRun: + return true + case setupModeDefaults, setupModePrintPrompt, setupModeNone: + return false + } + return false +} + type runnerSetupOptions struct { runner string // optional: limit to one runner (id, with or without "trail-") - run bool // headless apply vs. print prompt - assumeYes bool // skip the create-defaults confirmation - debugDir string // if set, dump prompt.txt (+ response.txt on --run) here + assumeYes bool // -y: answer every prompt, i.e. create the defaults and tailor them + defaultsOnly bool + printPrompt bool + dryRun bool + debugDir string // if set, dump prompt.txt (+ response.txt when a provider ran) here sources []string limit int insecureHTTP bool @@ -32,11 +95,8 @@ type runnerSetupOptions struct { func newRunnerSetupCmd() *cobra.Command { var ( - run bool - assumeYes bool - debugDir string - sources []string - limit int + opts runnerSetupOptions + deprecatedRun bool ) cmd := &cobra.Command{ @@ -45,93 +105,130 @@ func newRunnerSetupCmd() *cobra.Command { Long: `Set up the .entire/runners/*.json evaluators for this repository. Runners (risk, confidence, drift, security, review, …) score and review a -branch's changes. The shipped templates are generic; "setup" tailors them to +branch's changes. The shipped defaults are generic; setup can tailor them to THIS repo using gathered signal — its docs and structure, merged PRs and issues, checkpoint churn hotspots, and past trail findings. -- In a repo with no runners, setup creates the default set first (use --yes to - skip the confirmation), then tailors them. -- Run again in a repo that already has runners and setup offers to re-tune them. +In a terminal with no flags, setup asks whether you want the generic defaults +or defaults tailored to this repo. Otherwise name the action up front: + + -y, --yes create the defaults if missing, then tailor them in place + --defaults-only create the generic defaults and stop + --print-prompt print the tailoring prompt for your own agent to run + --dry-run show the tailoring as a diff and write nothing -By default setup prints the tailoring prompt to stdout, ready to paste into -your agent. With --run it executes the prompt headlessly through your -configured summary provider and rewrites the runner files in place (review with -git diff). +--yes and --dry-run each call your configured summary provider once. Review a +tailoring with git diff .entire/runners. If is given (e.g. "risk" or "trail-risk"), only that runner is tuned.`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runner := "" if len(args) == 1 { - runner = args[0] + opts.runner = args[0] + } + if deprecatedRun { + opts.assumeYes = true } - return runRunnerSetup(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr(), runnerSetupOptions{ - runner: runner, - run: run, - assumeYes: assumeYes, - debugDir: debugDir, - sources: sources, - limit: limit, - insecureHTTP: runnerInsecureHTTP(cmd), - }) + opts.insecureHTTP = runnerInsecureHTTP(cmd) + return runRunnerSetup(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr(), opts) }, } - cmd.Flags().BoolVar(&run, "run", false, - "Run the configured summary provider headlessly to rewrite the runner files in place (default: print the prompt)") - cmd.Flags().StringSliceVar(&sources, "sources", nil, + cmd.Flags().BoolVarP(&opts.assumeYes, "yes", "y", false, + "Create the default runners if missing and tailor them to this repo, without asking") + cmd.Flags().BoolVar(&opts.defaultsOnly, "defaults-only", false, + "Create the generic default runners and stop (no tailoring, no provider call)") + cmd.Flags().BoolVar(&opts.printPrompt, "print-prompt", false, + "Print the tailoring prompt for your own agent instead of running a provider") + cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, + "Show the tailoring as a diff and write nothing") + cmd.Flags().StringSliceVar(&opts.sources, "sources", nil, "Comma-separated data sources to gather: repo, prs, checkpoints, trails, all (default: all)") - cmd.Flags().IntVar(&limit, "limit", 20, "How many recent PRs/issues/trails to sample") - cmd.Flags().BoolVarP(&assumeYes, "yes", "y", false, - "Skip the confirmation when creating the default runner set in a repo that has none") - cmd.Flags().StringVar(&debugDir, "debug-dir", "", - "Write the assembled prompt (prompt.txt) and, with --run, the raw model response (response.txt) to this directory for debugging") + cmd.Flags().IntVar(&opts.limit, "limit", 20, "How many recent PRs/issues/trails to sample") + cmd.Flags().StringVar(&opts.debugDir, "debug-dir", "", + "Write the assembled prompt (prompt.txt) and, when a provider runs, its raw response (response.txt) to this directory for debugging") + + // --run was the flag that made setup finish its job; tailoring is now the + // default action, so it survives only as an alias for the flag that means it. + cmd.Flags().BoolVar(&deprecatedRun, "run", false, "Deprecated alias for --yes") + if err := cmd.Flags().MarkDeprecated("run", "use --yes (tailoring is now the default action)"); err != nil { + panic(fmt.Sprintf("deprecate run flag: %v", err)) + } + + cmd.MarkFlagsMutuallyExclusive("defaults-only", "print-prompt", "dry-run") return cmd } func runRunnerSetup(ctx context.Context, w, errW io.Writer, opts runnerSetupOptions) error { - if opts.limit <= 0 { - return errors.New(runnerSetupLimitErrorMessage) - } - src, err := parseTuneSources(opts.sources) - if err != nil { - return err - } - repoRoot, err := paths.WorktreeRoot(ctx) if err != nil { return fmt.Errorf("not a git repository: %w", err) } - // A repo with no runners gets the default set scaffolded (on confirmation), - // which setup then tailors below. - created, err := ensureRunnersPresent(w, errW, repoRoot, opts.assumeYes) + haveRunners := runnerConfigsExist(repoRoot) + mode, err := resolveRunnerSetupMode(ctx, errW, opts, haveRunners) if err != nil { return err } + if mode == setupModeNone { + return nil // picker cancelled; handleFormCancellation already said so + } - // Re-run on an already-configured repo: setup is done, so offer to re-tune - // rather than silently re-emitting. --run is taken as an explicit yes. - if len(created) == 0 && !opts.run { - if !interactive.CanPromptInteractively() { - fmt.Fprintln(errW, "Runners already configured. Re-run with --run to tailor them headlessly.") - return nil + // --sources and --limit only steer the gather, so they are validated for + // the modes that gather and not for --defaults-only, which reads neither. + // Before the scaffold, though: a usage error must not leave files behind. + var src tuneSources + if mode.gathersSignal() { + if src, err = parseTuneSources(opts.sources); err != nil { + return err + } + if opts.limit <= 0 { + return errors.New(runnerSetupLimitErrorMessage) } - proceed, err := confirmTuneExisting() - if err != nil { + } + + // Choosing a mode was the consent for creating the runner files. + var created []string + if mode.writesRunnerFiles() && !haveRunners { + if created, err = createDefaultRunners(w, repoRoot); err != nil { return err } - if !proceed { + } + + if mode == setupModeDefaults { + if len(created) == 0 { fmt.Fprintln(errW, "Runners already configured. Nothing to do.") return nil } + reportCreatedDefaults(errW, len(created), mode) + return nil } - runners, err := loadTuneRunners(repoRoot, opts.runner) + // Resolved before the gather: resolution can fail outright, or stop to ask + // which provider to use, and neither belongs after seconds of waiting. + var provider *checkpointSummaryProvider + if mode.needsProvider() { + if provider, err = resolveCheckpointSummaryProvider(ctx, errW); err != nil { + return err + } + } + + // A dry run creates nothing, so in a repo with no runners it previews the + // tailoring of the embedded set it would otherwise have written. That is + // the one place the preview's source differs, so it says so here. + loadRunners := loadTuneRunners + if !mode.writesRunnerFiles() && !haveRunners { + loadRunners = defaultTuneRunners + fmt.Fprintf(errW, "This repo has no runners yet and %s creates none, so the preview is against the embedded defaults.\n", mode) + } + runners, err := loadRunners(repoRoot, opts.runner) if err != nil { return err } + if len(created) > 0 && mode == setupModeAdapt { + reportCreatedDefaults(errW, len(created), mode) + } stopGather := startSpinner(errW, "Gathering repository signal") brief := gatherTuningContext(ctx, errW, repoRoot, src, opts.limit, opts.insecureHTTP) @@ -142,16 +239,65 @@ func runRunnerSetup(ctx context.Context, w, errW io.Writer, opts runnerSetupOpti writeTuneDebug(errW, opts.debugDir, "prompt.txt", prompt) } - if !opts.run { + if mode == setupModePrintPrompt { fmt.Fprintln(w, prompt) if len(created) > 0 { - fmt.Fprintf(errW, "\nCreated %d working default runner(s) (untracked). They are functional as-is; paste the prompt above into your agent to tailor them to this repo.\n", len(created)) + reportCreatedDefaults(errW, len(created), mode) } - fmt.Fprintf(errW, "\n%d runner(s) in scope. Paste the prompt above into your agent, or re-run with --run to apply headlessly.\n", len(runners)) + fmt.Fprintf(errW, "\n%d runner(s) in scope. Paste the prompt above into your agent, or re-run with --yes to apply it headlessly.\n", len(runners)) return nil } - return applyTuneWithAgent(ctx, w, errW, repoRoot, runners, prompt, created, opts.debugDir) + changes, skipped, err := runTuning(ctx, errW, provider, runners, prompt, opts.debugDir) + if err != nil { + return err + } + switch mode { + case setupModeDryRun: + previewTunedRunners(w, errW, len(runners), changes, skipped) + return nil + case setupModeAdapt: + return applyTunedRunners(w, errW, repoRoot, changes, skipped, created) + case setupModeDefaults, setupModePrintPrompt, setupModeNone: + // All returned above. Reaching here means an early return was removed. + } + return fmt.Errorf("unhandled runner setup mode %q", mode) +} + +// reportCreatedDefaults says the same thing about a fresh scaffold in every +// mode — one sentence, one place, so the three call sites cannot drift apart +// the way "default" / "generic default" / "working default" already had. +func reportCreatedDefaults(errW io.Writer, n int, mode runnerSetupMode) { + next := "run `entire runner setup -y` to tailor them to this repo" + switch mode { + case setupModeAdapt: + next = "tailoring them to this repo now…" + case setupModePrintPrompt: + next = "paste the prompt above into your agent to tailor them to this repo" + case setupModeDefaults, setupModeDryRun, setupModeNone: + } + fmt.Fprintf(errW, "\nCreated %d default runner(s) (untracked, functional as-is); %s\n", n, next) +} + +// resolveRunnerSetupMode settles what this invocation will do, and is the only +// place consent is taken. An explicit mode flag wins, --yes means the full +// action, a terminal is asked, and a non-interactive caller that named nothing +// is told which flag to pass rather than being given half the job. +func resolveRunnerSetupMode(ctx context.Context, errW io.Writer, opts runnerSetupOptions, haveRunners bool) (runnerSetupMode, error) { + switch { + case opts.dryRun: + return setupModeDryRun, nil + case opts.printPrompt: + return setupModePrintPrompt, nil + case opts.defaultsOnly: + return setupModeDefaults, nil + case opts.assumeYes: + return setupModeAdapt, nil + case interactive.CanPromptInteractively(): + return chooseRunnerSetupAction(ctx, errW, haveRunners) + default: + return setupModeNone, errors.New("no terminal to ask what setup should do: pass --yes (create the default runners and tailor them), --defaults-only, --print-prompt, or --dry-run — see --help") + } } // writeTuneDebug best-effort writes content to / for debugging, @@ -168,108 +314,3 @@ func writeTuneDebug(errW io.Writer, dir, name, content string) { } fmt.Fprintf(errW, "debug: wrote %s\n", path) } - -// applyTuneWithAgent runs the prompt through the configured summary provider -// (prompt -> text), parses the runner-id -> template map it returns, and -// surgically rewrites each runner file's prompt.template in place. createdIDs -// are runners onboarding just scaffolded from defaults; any of those left -// un-tailored is flagged so it isn't committed as if it were repo-specific. -func applyTuneWithAgent(ctx context.Context, w, errW io.Writer, repoRoot string, runners []tuneRunner, prompt string, createdIDs []string, debugDir string) error { - root, err := entiredir.OpenAt(repoRoot) - if err != nil { - return fmt.Errorf("open %s: %w", paths.EntireDir, err) - } - - // Reuse the summary-provider resolution (selection + persistence), but pull - // the raw TextGenerator rather than provider.Generator: the latter is a - // summarize.Generator that turns a transcript Input into a checkpoint - // Summary, whereas we need plain prompt->text generation here. - provider, err := resolveCheckpointSummaryProvider(ctx, errW) - if err != nil { - return err - } - ag, err := agent.Get(provider.Name) - if err != nil { - return fmt.Errorf("loading provider %s: %w", provider.Name, err) - } - textGen, ok := agent.AsTextGenerator(ag) - if !ok { - return fmt.Errorf("provider %s cannot generate text", provider.Name) - } - - stop := startSpinner(errW, fmt.Sprintf("Tuning %d runner(s) with %s", len(runners), provider.DisplayName)) - out, err := textGen.GenerateText(ctx, prompt, provider.Model) - stop(err == nil) - if err != nil { - return fmt.Errorf("agent run failed: %w", err) - } - if debugDir != "" { - writeTuneDebug(errW, debugDir, "response.txt", out) - } - - templates, err := parseTuneOutput(out) - if err != nil { - return err - } - - byID := make(map[string]tuneRunner, len(runners)) - for _, r := range runners { - byID[normalizeRunnerID(r.ID)] = r - } - - updated, skipped := 0, 0 - tailored := make(map[string]bool) - for id, tmpl := range templates { - r, ok := byID[normalizeRunnerID(id)] - if !ok { - fmt.Fprintf(errW, "skip %q: not a runner in scope\n", id) - skipped++ - continue - } - if err := validateNewTemplate(r.Template, tmpl); err != nil { - fmt.Fprintf(errW, "skip %s: %v\n", r.ID, err) - skipped++ - continue - } - if dropped := droppedPlaceholders(r.Template, tmpl); len(dropped) > 0 { - fmt.Fprintf(errW, "note: %s no longer references %v\n", r.ID, dropped) - } - newRaw, err := replaceRunnerTemplate(r.Raw, tmpl) - if err != nil { - fmt.Fprintf(errW, "skip %s: %v\n", r.ID, err) - skipped++ - continue - } - if bytes.Equal(newRaw, r.Raw) { - continue // model returned the current template verbatim — benign no-op - } - if err := entiredir.WriteFile(root, r.Name, newRaw, 0o644); err != nil { - return fmt.Errorf("writing %s: %w", r.Path, err) - } - fmt.Fprintf(w, "updated %s\n", filepath.Base(r.Path)) - tailored[normalizeRunnerID(r.ID)] = true - updated++ - } - - switch { - case updated > 0: - fmt.Fprintf(w, "\nUpdated %d runner(s). Review with: git diff .entire/runners\n", updated) - case len(createdIDs) == 0 && skipped > 0: - // Existing runners, model proposed templates, all rejected — a failed run. - // (When onboarding just created the set, an un-tailored runner is reported - // below as a generic default instead, which is more actionable.) - return fmt.Errorf("model proposed %d template(s) but all were rejected or out of scope (see messages above)", skipped) - case len(createdIDs) == 0: - fmt.Fprintln(w, "No runner changes proposed.") - } - - // Runners onboarding scaffolded but tuning didn't tailor remain the generic - // defaults. Those are working minimal prompts (valid output contract), so - // they're committable as-is — just note which are still generic. - if untailored := untailoredRunners(createdIDs, tailored); len(untailored) > 0 { - fmt.Fprintf(errW, "\n%d runner(s) kept as working defaults (generic, not tailored to this repo): %s\n", - len(untailored), strings.Join(untailored, ", ")) - fmt.Fprintln(errW, "They are functional as-is; re-run `entire runner setup --run` to tailor them.") - } - return nil -} diff --git a/cmd/entire/cli/runner_setup_test.go b/cmd/entire/cli/runner_setup_test.go new file mode 100644 index 0000000000..23e6999450 --- /dev/null +++ b/cmd/entire/cli/runner_setup_test.go @@ -0,0 +1,435 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/types" + "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/runnerdefaults" + "github.com/entireio/cli/cmd/entire/cli/settings" + "github.com/entireio/cli/cmd/entire/cli/testutil" +) + +// TestResolveRunnerSetupMode_FlagPrecedence pins the mode each flag combination +// selects. The point of the table is that -y resolves to the FULL action: +// tailoring used to need its own flag, so -y left the job half done. +func TestResolveRunnerSetupMode_FlagPrecedence(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opts runnerSetupOptions + want runnerSetupMode + }{ + {"yes means create and tailor", runnerSetupOptions{assumeYes: true}, setupModeAdapt}, + {"defaults-only stops after creating", runnerSetupOptions{defaultsOnly: true}, setupModeDefaults}, + {"print-prompt emits the prompt", runnerSetupOptions{printPrompt: true}, setupModePrintPrompt}, + {"dry-run previews", runnerSetupOptions{dryRun: true}, setupModeDryRun}, + // An explicit mode outranks -y, so `-y --dry-run` still writes nothing. + {"dry-run outranks yes", runnerSetupOptions{assumeYes: true, dryRun: true}, setupModeDryRun}, + {"defaults-only outranks yes", runnerSetupOptions{assumeYes: true, defaultsOnly: true}, setupModeDefaults}, + {"print-prompt outranks yes", runnerSetupOptions{assumeYes: true, printPrompt: true}, setupModePrintPrompt}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := resolveRunnerSetupMode(context.Background(), io.Discard, tt.opts, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("mode = %q, want %q", got, tt.want) + } + }) + } +} + +// TestResolveRunnerSetupMode_NonInteractiveNeedsAnAction covers the case that +// used to hand back a wall of prompt text nobody asked for: with no terminal +// and no flag, setup names the choices instead of picking one. +func TestResolveRunnerSetupMode_NonInteractiveNeedsAnAction(t *testing.T) { + t.Parallel() + + // interactive.CanPromptInteractively() is false under `go test`. + _, err := resolveRunnerSetupMode(context.Background(), io.Discard, runnerSetupOptions{}, false) + if err == nil { + t.Fatal("expected an error when no terminal and no action flag") + } + for _, flag := range []string{"--yes", "--defaults-only", "--print-prompt", "--dry-run"} { + if !strings.Contains(err.Error(), flag) { + t.Errorf("error should name %s, got: %v", flag, err) + } + } +} + +// TestRunRunnerSetup_DryRunCreatesNothing is the invariant that separates +// --dry-run from every other mode: it must not scaffold the defaults, so a +// preview of a fresh repo leaves that repo untouched. +func TestRunRunnerSetup_DryRunCreatesNothing(t *testing.T) { + repoRoot := newRunnerSetupRepo(t) + stubUnavailableSummaryProvider(t) + + var out, errOut bytes.Buffer + err := runRunnerSetup(context.Background(), &out, &errOut, runnerSetupOptions{ + dryRun: true, + sources: []string{"repo"}, // local signal only: no gh, no API + limit: 1, + }) + // The stubbed provider is unresolvable, so tailoring cannot run — which is + // exactly where this test wants to stop. What matters is what is on disk. + if err == nil { + t.Fatal("expected the stubbed provider to fail the tailoring step") + } + if _, statErr := os.Stat(runnersDir(repoRoot)); !os.IsNotExist(statErr) { + t.Errorf("--dry-run created .entire/runners (stat err = %v); it must write nothing", statErr) + } +} + +// TestRunRunnerSetup_YesCreatesDefaultsBeforeTailoring is the counterpart: -y +// scaffolds without asking, and does so before the provider is involved, so a +// provider failure still leaves a working generic set behind. +func TestRunRunnerSetup_YesCreatesDefaultsBeforeTailoring(t *testing.T) { + repoRoot := newRunnerSetupRepo(t) + stubUnavailableSummaryProvider(t) + + var out, errOut bytes.Buffer + if err := runRunnerSetup(context.Background(), &out, &errOut, runnerSetupOptions{ + assumeYes: true, + sources: []string{"repo"}, + limit: 1, + }); err == nil { + t.Fatal("expected the stubbed provider to fail the tailoring step") + } + + if written := runnerFiles(t, repoRoot); len(written) != wantDefaultCount(t) { + t.Errorf("-y wrote %d runner file(s), want the full default set (%d)", len(written), wantDefaultCount(t)) + } +} + +// TestRunRunnerSetup_DefaultsOnlyMakesNoProviderCall pins that --defaults-only +// is the offline mode: it returns successfully even though the configured +// provider cannot be resolved, because it never asks for one. +func TestRunRunnerSetup_DefaultsOnlyMakesNoProviderCall(t *testing.T) { + repoRoot := newRunnerSetupRepo(t) + stubUnavailableSummaryProvider(t) + + var out, errOut bytes.Buffer + if err := runRunnerSetup(context.Background(), &out, &errOut, runnerSetupOptions{ + defaultsOnly: true, + limit: 1, + }); err != nil { + t.Fatalf("--defaults-only should not need a provider: %v", err) + } + + if written := runnerFiles(t, repoRoot); len(written) != wantDefaultCount(t) { + t.Errorf("--defaults-only wrote %d runner file(s), want the full default set (%d)", len(written), wantDefaultCount(t)) + } + // And it says how to tailor them, rather than leaving them looking finished. + if !strings.Contains(errOut.String(), "-y") { + t.Errorf("expected a pointer at tailoring, got %q", errOut.String()) + } +} + +// TestRunRunnerSetup_DefaultsOnlyIsANoopWhenConfigured covers the re-run case: +// there is nothing to create and --defaults-only asks for nothing else. +func TestRunRunnerSetup_DefaultsOnlyIsANoopWhenConfigured(t *testing.T) { + repoRoot := newRunnerSetupRepo(t) + if err := os.MkdirAll(runnersDir(repoRoot), 0o755); err != nil { + t.Fatal(err) + } + writeRunner(t, runnersDir(repoRoot), "trail-risk", "existing") + + var out, errOut bytes.Buffer + if err := runRunnerSetup(context.Background(), &out, &errOut, runnerSetupOptions{ + defaultsOnly: true, + limit: 1, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(errOut.String(), "Nothing to do") { + t.Errorf("expected a no-op notice, got %q", errOut.String()) + } + if after := runnerFiles(t, repoRoot); len(after) != 1 { + t.Errorf("expected the existing runner left alone, got %d files", len(after)) + } +} + +func TestDefaultTuneRunners(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() // deliberately empty: the embedded set needs no repo + runners, err := defaultTuneRunners(repoRoot, "") + if err != nil { + t.Fatalf("defaultTuneRunners: %v", err) + } + if len(runners) != wantDefaultCount(t) { + t.Fatalf("expected the %d embedded runners, got %d", wantDefaultCount(t), len(runners)) + } + for _, r := range runners { + if r.ID == "" || r.Template == "" || len(r.Raw) == 0 { + t.Errorf("%s: incomplete tuneRunner (template_empty=%v raw=%d)", r.ID, r.Template == "", len(r.Raw)) + } + // Paths describe where the file WOULD go, so messages match the on-disk case. + if want := filepath.Join(repoRoot, ".entire", "runners"); filepath.Dir(r.Path) != want { + t.Errorf("%s: path %q not under %q", r.ID, r.Path, want) + } + } + if _, err := os.Stat(filepath.Join(repoRoot, ".entire")); !os.IsNotExist(err) { + t.Errorf("defaultTuneRunners touched the repo (stat err = %v)", err) + } + + // The filter accepts an id with or without the "trail-" prefix. + for _, filter := range []string{"risk", "trail-risk"} { + one, err := defaultTuneRunners(repoRoot, filter) + if err != nil { + t.Fatalf("filter %q: %v", filter, err) + } + if len(one) != 1 || normalizeRunnerID(one[0].ID) != "risk" { + t.Errorf("filter %q selected %d runner(s): %+v", filter, len(one), one) + } + } + if _, err := defaultTuneRunners(repoRoot, "nope"); err == nil { + t.Error("expected an error when the filter matches nothing") + } +} + +func TestRenderTemplateDiff(t *testing.T) { + t.Parallel() + + oldText := "keep one\nkeep two\ndrop me\nkeep three\n" + newText := "keep one\nkeep two\nadd me\nkeep three\n" + got := renderTemplateDiff(oldText, newText) + + for _, want := range []string{"-drop me", "+add me", " keep one"} { + if !strings.Contains(got, want) { + t.Errorf("diff missing %q:\n%s", want, got) + } + } +} + +// TestRenderTemplateDiff_CollapsesLongUnchangedRuns keeps the preview from +// becoming the very wall of text --dry-run exists to avoid: a tailored template +// shares a long unchanged tail (the output-JSON contract) with the original. +func TestRenderTemplateDiff_CollapsesLongUnchangedRuns(t *testing.T) { + t.Parallel() + + tail := strings.Repeat("unchanged tail\n", 40) + got := renderTemplateDiff("old head\n"+tail, "new head\n"+tail) + + if !strings.Contains(got, "@@ ") { + t.Errorf("expected a collapse marker for the unchanged tail:\n%s", got) + } + if n := strings.Count(got, "unchanged tail"); n > 2*diffContextLines { + t.Errorf("kept %d unchanged lines, want at most %d", n, 2*diffContextLines) + } + if !strings.Contains(got, "-old head") || !strings.Contains(got, "+new head") { + t.Errorf("collapse dropped the actual change:\n%s", got) + } +} + +func TestPreviewTunedRunners_ReportsWithoutWriting(t *testing.T) { + t.Parallel() + + runners, err := defaultTuneRunners(t.TempDir(), "risk") + if err != nil { + t.Fatal(err) + } + changes := []tunedRunner{{ + runner: runners[0], + newRaw: []byte("{}"), // never written in a preview + template: "a tailored template\n", + }} + + var out, errOut bytes.Buffer + previewTunedRunners(&out, &errOut, len(runners), changes, 1 /* skipped */) + + if !strings.Contains(out.String(), "+a tailored template") { + t.Errorf("expected the tailored template as an added line:\n%s", out.String()) + } + for _, want := range []string{"Nothing was written", "--yes", "1 proposal(s) rejected"} { + if !strings.Contains(errOut.String(), want) { + t.Errorf("summary missing %q:\n%s", want, errOut.String()) + } + } +} + +// newRunnerSetupRepo makes an isolated git repo and chdirs into it, so +// paths.WorktreeRoot resolves there rather than in the developer's checkout. +func newRunnerSetupRepo(t *testing.T) string { + t.Helper() + repoRoot := t.TempDir() + testutil.InitRepo(t, repoRoot) + testutil.WriteFile(t, repoRoot, "README.md", "# fixture\n") + testutil.GitAdd(t, repoRoot, "README.md") + testutil.GitCommit(t, repoRoot, "init") + // The worktree-root cache is keyed on cwd, so reset it either side of the + // chdir like the other git fixtures in this package do. + paths.ClearWorktreeRootCache() + t.Chdir(repoRoot) + t.Cleanup(paths.ClearWorktreeRootCache) + // t.TempDir is a symlink on macOS (/var -> /private/var); git reports the + // resolved path, so resolve here too or the glob checks look at a ghost. + resolved, err := filepath.EvalSymlinks(repoRoot) + if err != nil { + t.Fatalf("resolving repo root: %v", err) + } + return resolved +} + +// stubUnavailableSummaryProvider points summary generation at a provider that +// cannot resolve, so a test reaching the tailoring step fails there instead of +// making a real model call with whatever agent the developer has installed. +// Discovery is stubbed out as well: an unknown provider name otherwise reaches +// external.DiscoverAndRegisterAlways, which globs the real $PATH for +// entire-agent-* plugins and execs each match, making the test depend on the +// developer's machine. +func stubUnavailableSummaryProvider(t *testing.T) { + t.Helper() + originalLoad, originalGet := loadSummarySettings, getSummaryAgent + originalDiscover, originalDiscoverAlways := discoverSummaryProviders, discoverSummaryProvidersAlways + t.Cleanup(func() { + loadSummarySettings = originalLoad + getSummaryAgent = originalGet + discoverSummaryProviders = originalDiscover + discoverSummaryProvidersAlways = originalDiscoverAlways + }) + loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { + return &settings.EntireSettings{ + SummaryGeneration: &settings.SummaryGenerationSettings{Provider: "not-a-real-agent"}, + }, nil + } + getSummaryAgent = func(name types.AgentName) (agent.Agent, error) { + return nil, fmt.Errorf("stub: no agent %s", name) + } + // No-ops, not assertions: an unresolvable provider name legitimately reaches + // discovery (discoverSummaryProviderIfMissing calls it when the agent lookup + // fails). Replacing it is what keeps the test off the real $PATH. + discoverSummaryProviders = func(context.Context) {} + discoverSummaryProvidersAlways = func(context.Context) {} +} + +// runnerFiles lists the runner configs on disk, for the several tests whose +// whole assertion is whether a mode wrote the set or left the repo alone. +func runnerFiles(t *testing.T, repoRoot string) []string { + t.Helper() + files, err := filepath.Glob(filepath.Join(runnersDir(repoRoot), "*.json")) + if err != nil { + t.Fatalf("globbing runner files: %v", err) + } + return files +} + +// wantDefaultCount is the size of the embedded default set, derived rather +// than hardcoded so adding or removing a default does not need a test edit. +func wantDefaultCount(t *testing.T) int { + t.Helper() + files, err := runnerdefaults.Files() + if err != nil { + t.Fatalf("runnerdefaults.Files: %v", err) + } + return len(files) +} + +// TestClassifyTuneProposals covers the accept/reject rules against canned +// proposals, so the outcome of a tailoring run is pinned without a provider +// call. Every rejection is per-runner: the accepted one must survive them. +func TestClassifyTuneProposals(t *testing.T) { + t.Parallel() + + runners, err := defaultTuneRunners(t.TempDir(), "") + if err != nil { + t.Fatal(err) + } + byID := make(map[string]tuneRunner, len(runners)) + for _, r := range runners { + byID[normalizeRunnerID(r.ID)] = r + } + risk, drift := byID["risk"], byID["drift"] + + // A rewrite may drop a placeholder but not invent one, so reuse the risk + // template's own placeholders in the accepted proposal. + accepted := "Tailored for this repo.\n" + risk.Template + + var errOut bytes.Buffer + changes, skipped := classifyTuneProposals(&errOut, runners, map[string]string{ + "trail-risk": accepted, + "trail-drift": drift.Template, // verbatim: a benign no-op + "trail-nonsense": "whatever", // not a runner in scope + "trail-security": "score {{invented_thing}}", // invents a placeholder + }) + + if len(changes) != 1 || normalizeRunnerID(changes[0].runner.ID) != "risk" { + t.Fatalf("expected only trail-risk to change, got %d: %+v", len(changes), changes) + } + if changes[0].template != accepted { + t.Error("accepted change did not carry the proposed template") + } + // The verbatim proposal is dropped silently; the other two are counted. + if skipped != 2 { + t.Errorf("skipped = %d, want 2 (out-of-scope + invented placeholder)", skipped) + } + for _, want := range []string{`skip "trail-nonsense"`, "trail-security", "{{invented_thing}}"} { + if !strings.Contains(errOut.String(), want) { + t.Errorf("rejection messages missing %q:\n%s", want, errOut.String()) + } + } + if strings.Contains(errOut.String(), "trail-drift") { + t.Errorf("a verbatim proposal should be a silent no-op:\n%s", errOut.String()) + } + + // The new file bytes must be valid JSON with only the template changed. + var doc struct { + ID string `json:"id"` + Output struct { + ResultType string `json:"result_type"` + } `json:"output"` + Prompt struct { + Template string `json:"template"` + } `json:"prompt"` + } + if err := json.Unmarshal(changes[0].newRaw, &doc); err != nil { + t.Fatalf("rewritten runner is not valid JSON: %v", err) + } + if doc.Prompt.Template != accepted { + t.Error("rewritten file does not carry the new template") + } + if doc.ID != risk.ID || doc.Output.ResultType == "" { + t.Errorf("rewrite lost structural fields: id=%q result_type=%q", doc.ID, doc.Output.ResultType) + } +} + +// TestRunRunnerSetup_BadGatherFlagsWriteNothing pins that a usage error leaves +// the repo alone. The gather flags are validated for the modes that read them, +// but ahead of the scaffold — validating after it meant `-y --limit 0` on a +// fresh repo created .entire/runners and then failed. +func TestRunRunnerSetup_BadGatherFlagsWriteNothing(t *testing.T) { + repoRoot := newRunnerSetupRepo(t) + stubUnavailableSummaryProvider(t) + + for _, tc := range []struct { + name string + opts runnerSetupOptions + }{ + {"yes with a bad limit", runnerSetupOptions{assumeYes: true, limit: 0}}, + {"yes with bad sources", runnerSetupOptions{assumeYes: true, limit: 1, sources: []string{"nope"}}}, + {"print-prompt with a bad limit", runnerSetupOptions{printPrompt: true, limit: -1}}, + } { + t.Run(tc.name, func(t *testing.T) { + if err := runRunnerSetup(context.Background(), io.Discard, io.Discard, tc.opts); err == nil { + t.Fatal("expected a usage error") + } + if written := runnerFiles(t, repoRoot); len(written) != 0 { + t.Errorf("a usage error wrote %d runner file(s); it must write none", len(written)) + } + }) + } +}