From 5e93a019520ab2c352568c3fdde265cefed75bea Mon Sep 17 00:00:00 2001 From: Jory Irving Date: Thu, 9 Jul 2026 21:38:34 -0600 Subject: [PATCH] feat(cli): --revise-from-branch on foreman dispatch for in-place PR refresh issue-fix tasks can already restore a prior attempt and force-with-lease push an update to the same branch via payload.reviseFromBranch, but the flag was unreachable from the CLI. Add --revise-from-branch : when set, both payload.branch and payload.reviseFromBranch are stamped with that ref, so the executor restores the prior attempt and refreshes the open PR instead of cutting a new branch. Requires exactly one issue. Closes #996 Signed-off-by: Jory Irving --- pkg/cli/dispatch.go | 69 ++++++++++++++++++++++++++------ pkg/cli/dispatch_test.go | 85 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 11 deletions(-) diff --git a/pkg/cli/dispatch.go b/pkg/cli/dispatch.go index 64f3bf36..3fe5d1ee 100644 --- a/pkg/cli/dispatch.go +++ b/pkg/cli/dispatch.go @@ -57,16 +57,17 @@ func fprint(w io.Writer, a ...any) { _, _ = fmt.Fprint(w, a...) // dispatchOptions holds the resolved flags for one `foreman dispatch` run. type dispatchOptions struct { - repo string - agents []string - namespace string - timeoutSecs int32 - baseBranch string - branchPrefix string - promptFiles []string - noWait bool - pollInterval time.Duration - dryRun bool + repo string + agents []string + namespace string + timeoutSecs int32 + baseBranch string + branchPrefix string + reviseFromBranch string + promptFiles []string + noWait bool + pollInterval time.Duration + dryRun bool } // taskAssignment pairs an issue with the coder Agent that will run it. @@ -124,7 +125,12 @@ Examples: # Override one issue's prompt; create and detach: llmkube foreman dispatch --repo defilantech/LLMKube --agents coder-metal \ - --prompt-file 813=/tmp/813.md --no-wait 813 854`, + --prompt-file 813=/tmp/813.md --no-wait 813 854 + + # In-place PR refresh: restore an existing branch and push an update on top: + llmkube foreman dispatch --repo defilantech/LLMKube --agents coder-go \ + --revise-from-branch foreman/adhoc/issue-991 \ + --prompt-file 991=feedback.md 991`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runDispatch(cmd.Context(), cmd.OutOrStdout(), args, opts) @@ -137,6 +143,9 @@ Examples: f.Int32Var(&opts.timeoutSecs, "timeout", 1800, "Per-task timeout in seconds") f.StringVar(&opts.baseBranch, "base-branch", "", "Base branch the coder branches from (payload.baseBranch)") f.StringVar(&opts.branchPrefix, "branch-prefix", "", "Prefix for the coder's working branch (payload.branchPrefix)") + f.StringVar(&opts.reviseFromBranch, "revise-from-branch", "", + "Restore this ref as the coder's working branch and push an update to the existing PR "+ + "(sets payload.branch and payload.reviseFromBranch; requires exactly one issue)") f.StringArrayVar(&opts.promptFiles, "prompt-file", nil, "Override an issue's prompt: ISSUE=PATH (repeatable)") f.BoolVar(&opts.noWait, "no-wait", false, "Create the tasks and exit without watching") f.DurationVar(&opts.pollInterval, "poll-interval", 5*time.Second, "How often to poll task status while watching") @@ -162,6 +171,9 @@ func runDispatch(ctx context.Context, out io.Writer, args []string, opts *dispat if err != nil { return err } + if err := validateReviseFromBranch(issues, opts.reviseFromBranch); err != nil { + return err + } overrides, err := parsePromptOverrides(opts.promptFiles) if err != nil { return err @@ -259,6 +271,30 @@ func assignAgents(issues []int, agents []string) []taskAssignment { return out } +// validateReviseFromBranch enforces the single-issue rule for the +// --revise-from-branch flag (#996): a revision targets exactly one branch, +// and the executor path will create one AgenticTask for it. Empty flag is +// a no-op (fresh-branch path). +func validateReviseFromBranch(issues []int, flag string) error { + if flag == "" { + return nil + } + if len(issues) != 1 { + return fmt.Errorf("--revise-from-branch requires exactly one issue, got %d", len(issues)) + } + return nil +} + +// branchStrategyIfRevision pairs BranchStrategy with ReviseFromBranch: the +// rebase strategy is what makes the executor actually restore the prior +// attempt (#1042), so the two must move together. +func branchStrategyIfRevision(ref string) foremanv1alpha1.BranchStrategy { + if ref == "" { + return "" + } + return foremanv1alpha1.BranchStrategyRebase +} + // parsePromptOverrides reads each ISSUE=PATH spec into a map of issue -> // file contents. func parsePromptOverrides(specs []string) (map[int]string, error) { @@ -321,6 +357,17 @@ func buildTask( BaseBranch: opts.baseBranch, BranchPrefix: opts.branchPrefix, Agent: a.Agent, + // Issue-fix revision (#996): stamp Branch and ReviseFromBranch + // to the same ref so the executor restores the prior attempt + // and force-with-lease pushes an update, refreshing the open + // PR in place. BranchStrategy must be "rebase" — under the + // default "reset" (#1042), setupTaskBranch skips the + // ReviseFromBranch restore and the revision becomes a no-op + // fresh cut from base. Both revision fields stay empty on the + // fresh-branch path. + Branch: opts.reviseFromBranch, + ReviseFromBranch: opts.reviseFromBranch, + BranchStrategy: branchStrategyIfRevision(opts.reviseFromBranch), }, }, } diff --git a/pkg/cli/dispatch_test.go b/pkg/cli/dispatch_test.go index b967240a..bdeda8ed 100644 --- a/pkg/cli/dispatch_test.go +++ b/pkg/cli/dispatch_test.go @@ -18,6 +18,7 @@ package cli import ( "context" + "io" "os" "path/filepath" "strings" @@ -305,6 +306,90 @@ func TestWatchTasks_ContextCancelledReturnsPartial(t *testing.T) { } } +func TestBuildTask_ReviseFromBranch(t *testing.T) { + cases := []struct { + name string + flag string + wantBranch string + wantReviseFrom string + wantBranchStrategy foremanv1alpha1.BranchStrategy + }{ + {"unset leaves revision fields empty", "", "", "", ""}, + { + "set stamps branch, reviseFromBranch, and rebase strategy", + "foreman/adhoc/issue-991", + "foreman/adhoc/issue-991", + "foreman/adhoc/issue-991", + foremanv1alpha1.BranchStrategyRebase, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + opts := &dispatchOptions{reviseFromBranch: tc.flag} + task := buildTask("default", "run1", "defilantech/LLMKube", + taskAssignment{Issue: 991, Agent: "coder-go"}, "feedback", opts) + p := task.Spec.Payload + if p.Branch != tc.wantBranch { + t.Errorf("Branch = %q, want %q", p.Branch, tc.wantBranch) + } + if p.ReviseFromBranch != tc.wantReviseFrom { + t.Errorf("ReviseFromBranch = %q, want %q", p.ReviseFromBranch, tc.wantReviseFrom) + } + if p.BranchStrategy != tc.wantBranchStrategy { + t.Errorf("BranchStrategy = %q, want %q (rebase required to restore; #1042 setupTaskBranch gates on this)", + p.BranchStrategy, tc.wantBranchStrategy) + } + }) + } +} + +func TestValidateReviseFromBranch(t *testing.T) { + cases := []struct { + name string + issues []int + flag string + wantErr bool + }{ + {"unset, many issues", []int{1, 2}, "", false}, + {"unset, no issues", nil, "", false}, + {"set, one issue", []int{991}, "foreman/issue-991", false}, + {"set, two issues", []int{991, 992}, "foreman/issue-991", true}, + {"set, no issues", nil, "foreman/issue-991", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateReviseFromBranch(tc.issues, tc.flag) + if (err != nil) != tc.wantErr { + t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) + } + if tc.wantErr && err != nil && !strings.Contains(err.Error(), "exactly one issue") { + t.Errorf("expected error to mention 'exactly one issue', got %q", err) + } + }) + } +} + +// runDispatch exercises its input-validation rules by passing an unset +// GITHUB_TOKEN (forcing the prompt-fetch step to fail late). The point of +// these cases is that the --revise-from-branch rule fires before any +// network call, so a multi-issue + flag invocation errors with the rule +// message, not a fetch error. +func TestRunDispatch_ReviseFromBranchFiresBeforeFetch(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + opts := &dispatchOptions{ + repo: "defilantech/LLMKube", + agents: []string{"coder-go"}, + reviseFromBranch: "foreman/adhoc/issue-991", + } + err := runDispatch(context.Background(), io.Discard, []string{"991", "992"}, opts) + if err == nil { + t.Fatal("expected validation error, got nil") + } + if !strings.Contains(err.Error(), "--revise-from-branch requires exactly one issue") { + t.Errorf("expected validation message, got %q", err) + } +} + func TestPrintTasksYAML(t *testing.T) { opts := &dispatchOptions{timeoutSecs: 1800} task := buildTask("default", "run1", "defilantech/LLMKube",