diff --git a/docs/src/content/docs/reference/global-config.md b/docs/src/content/docs/reference/global-config.md index 65530ad63..78606a7c6 100644 --- a/docs/src/content/docs/reference/global-config.md +++ b/docs/src/content/docs/reference/global-config.md @@ -42,6 +42,8 @@ log_level: info session_reuse: true +draft_pr: false + auto_fix: rebase: 3 review: 0 @@ -218,6 +220,19 @@ agent_args_override: For Codex, `service_tier` and `model_reasoning_effort` tune different things: `service_tier` selects the speed or priority lane, while `model_reasoning_effort` selects reasoning depth. no-mistakes reloads global config while setting up each run, so edits made before `no-mistakes axi run` apply to that run. For repeatable profiles, use separately initialized `NM_HOME` directories; each has its own `config.yaml` and no-mistakes state. +### draft_pr + +Create newly opened GitHub pull requests as drafts. + +| | | +| --- | --- | +| Type | `bool` | +| Default | `false` | + +Set `draft_pr: true` to add `--draft` to `gh pr create`. It applies only when no-mistakes creates a new GitHub pull request. Existing pull requests are updated without changing their draft or ready-for-review state. + +A repository-level [`draft_pr`](/no-mistakes/reference/repo-config/#draft_pr) setting overrides this global value when present. GitLab and Azure DevOps creation remain unchanged because no-mistakes does not map this GitHub-specific setting onto provider-specific draft semantics. + ### ci_timeout How long the CI step monitors an open PR, including provider CI status and on GitHub, GitLab, or Azure DevOps PR mergeability, before giving up. diff --git a/docs/src/content/docs/reference/repo-config.md b/docs/src/content/docs/reference/repo-config.md index 452571cf2..e1e48f0a3 100644 --- a/docs/src/content/docs/reference/repo-config.md +++ b/docs/src/content/docs/reference/repo-config.md @@ -12,7 +12,7 @@ The daemon also reads `document.instructions`, `review.path_instructions`, `disa If the default branch cannot be fetched and resolved to a readable commit, or its present `.no-mistakes.yaml` cannot be read and parsed, the run aborts before launching an agent. A readable default-branch tree with no `.no-mistakes.yaml` is valid and uses defaults. Commit the gate-control settings you want to your default branch. -Non-executing fields (`ignore_patterns`, `auto_fix`, `commit`, `intent`, `test`) are still read from the pushed branch, except `test.evidence.branch`, which names a git ref the daemon pushes to. +Non-executing fields (`ignore_patterns`, `draft_pr`, `auto_fix`, `commit`, `intent`, `test`) are still read from the pushed branch, except `test.evidence.branch`, which names a git ref the daemon pushes to. If you genuinely want per-branch `commands` and `agent` (for example, a single-developer repo where you trust your own feature branches), opt in with [`allow_repo_commands: true`](#allow_repo_commands) in this same file on your default branch. This re-enables the previous behavior with eyes open. The switch is read only from the trusted default-branch copy, so a contributor cannot self-enable it from a pushed branch. ::: @@ -32,6 +32,9 @@ ignore_patterns: - "*.generated.go" - "vendor/**" +# Override the global setting for this branch. GitHub only. +draft_pr: true + # Optional documentation ownership policy, read only from the trusted default branch. document: instructions: | @@ -128,6 +131,17 @@ Opt in to honoring the code-executing selection fields (`commands.{test,lint,for This field is itself read **only from the trusted default-branch copy** of `.no-mistakes.yaml`, never from the pushed SHA, so a contributor cannot self-enable it by setting it on a feature branch. By default the daemon reads `commands` and `agent` from your default branch (e.g. `origin/main`) so a pushed SHA cannot inject shell or pick the launched agent on the daemon host. This opt-in covers those two fields only; `document.instructions`, `review.path_instructions`, and `disable_project_settings` stay trusted-only either way. Leave this `false` for any repo that accepts contributions. Set it to `true` only for a single-developer environment where you trust every branch you push (for example, a personal repo gated by your own daemon). +### draft_pr + +Override the global [`draft_pr`](/no-mistakes/reference/global-config/#draft_pr) setting for this repository or branch. + +| | | +| --- | --- | +| Type | `bool` | +| Default | Inherits from global config (`false` when unset) | + +With `draft_pr: true`, no-mistakes passes `--draft` when it creates a new GitHub pull request. It never converts a pull request after creation and never changes the readiness state of an existing pull request during updates. This setting affects GitHub only. GitLab and Azure DevOps creation remain unchanged. + ### disable_project_settings Suppress project-level agent settings and instructions for every gate-agent start and resumed session. diff --git a/internal/config/config.go b/internal/config/config.go index e944e6915..667445b25 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -99,7 +99,10 @@ type GlobalConfig struct { // findings prescribed the fixes it certifies. Default true; set // session_reuse: false to force every invocation cold. SessionReuse bool `yaml:"-"` - AutoFix AutoFixRaw + // DraftPR controls whether newly created GitHub pull requests start as + // drafts. A nil value lets a repository-level setting take precedence. + DraftPR *bool `yaml:"-"` + AutoFix AutoFixRaw // CI is the operator's own CI-step floor. It is the only place the rerun // budget can be set for a repository whose default branch this machine's // user does not control (the common case when contributing to someone @@ -128,6 +131,7 @@ type globalConfigRaw struct { StepQuietWarning string `yaml:"step_quiet_warning"` LogLevel string `yaml:"log_level"` SessionReuse *bool `yaml:"session_reuse"` + DraftPR *bool `yaml:"draft_pr"` AutoFix AutoFixRaw `yaml:"auto_fix"` CI CIRaw `yaml:"ci"` Commit CommitRaw `yaml:"commit"` @@ -142,6 +146,9 @@ type RepoConfig struct { Agents []types.AgentName `yaml:"-"` Commands Commands `yaml:"commands"` IgnorePatterns []string `yaml:"ignore_patterns"` + // DraftPR controls whether GitHub PRs newly created for this branch are + // drafts. A non-nil repository value overrides the global setting. + DraftPR *bool `yaml:"draft_pr"` // AllowRepoCommands opts in to honoring the code-executing selection // fields (commands.{test,lint,format} and agent) from a contributor's // pushed branch instead of the trusted default-branch copy. It is read @@ -321,6 +328,7 @@ func (c *RepoConfig) UnmarshalYAML(value *yaml.Node) error { Agent agentList `yaml:"agent"` Commands Commands `yaml:"commands"` IgnorePatterns []string `yaml:"ignore_patterns"` + DraftPR *bool `yaml:"draft_pr"` AllowRepoCommands bool `yaml:"allow_repo_commands"` AutoFix AutoFixRaw `yaml:"auto_fix"` CI CIRaw `yaml:"ci"` @@ -340,6 +348,7 @@ func (c *RepoConfig) UnmarshalYAML(value *yaml.Node) error { c.Agents = copyAgents(raw.Agent) c.Commands = raw.Commands c.IgnorePatterns = raw.IgnorePatterns + c.DraftPR = raw.DraftPR c.AllowRepoCommands = raw.AllowRepoCommands c.AutoFix = raw.AutoFix c.CI = raw.CI @@ -415,6 +424,7 @@ type Config struct { StepQuietWarning time.Duration LogLevel string SessionReuse bool + DraftPR bool Eval Eval Commands Commands IgnorePatterns []string @@ -673,6 +683,10 @@ daemon_connect_timeout: "3s" # force every agent invocation cold. session_reuse: true +# Create newly opened GitHub pull requests as drafts. Existing pull requests +# are only updated and never have their readiness state changed. +draft_pr: false + # Log level for daemon output # Options: debug, info, warn, error log_level: info @@ -1335,6 +1349,9 @@ func LoadGlobalFromBytes(data []byte) (*GlobalConfig, error) { if raw.SessionReuse != nil { cfg.SessionReuse = *raw.SessionReuse } + if raw.DraftPR != nil { + cfg.DraftPR = raw.DraftPR + } if raw.AutoFix.CI == nil { raw.AutoFix.CI = raw.AutoFix.Babysit } @@ -1890,6 +1907,14 @@ func Merge(global *GlobalConfig, repo *RepoConfig) *Config { commit.FixMessage = *repo.Commit.FixMessage } + draftPR := false + if global.DraftPR != nil { + draftPR = *global.DraftPR + } + if repo.DraftPR != nil { + draftPR = *repo.DraftPR + } + cfg := &Config{ Agent: global.Agent, Agents: copyAgents(global.Agents), @@ -1901,6 +1926,7 @@ func Merge(global *GlobalConfig, repo *RepoConfig) *Config { StepQuietWarning: global.StepQuietWarning, LogLevel: global.LogLevel, SessionReuse: global.SessionReuse, + DraftPR: draftPR, // Eval is global-only by design (see GlobalConfig.Eval), so it is // copied straight through with no repository override step. Eval: global.Eval, diff --git a/internal/config/config_global_test.go b/internal/config/config_global_test.go index 624d242d8..75e517f77 100644 --- a/internal/config/config_global_test.go +++ b/internal/config/config_global_test.go @@ -54,6 +54,7 @@ func TestEnsureDefaultGlobalConfig_CreatesFile(t *testing.T) { "step_quiet_warning:", "daemon_connect_timeout:", "log_level: info", + "draft_pr: false", "# agent_path_override:", "# commit:", `# fix_message: "no-mistakes({{.Step}}): {{.Summary}}"`, diff --git a/internal/config/config_merge_test.go b/internal/config/config_merge_test.go index 56f76f1b5..ba9ab12f5 100644 --- a/internal/config/config_merge_test.go +++ b/internal/config/config_merge_test.go @@ -167,6 +167,57 @@ func TestMerge_AutoFixRepoOverridesGlobal(t *testing.T) { } } +func TestMerge_DraftPRDefaultsFalseAndRepoOverridesGlobal(t *testing.T) { + tests := []struct { + name string + globalYAML string + repoYAML string + want bool + }{ + { + name: "default false", + globalYAML: "agent: claude\n", + repoYAML: "commands: {}\n", + want: false, + }, + { + name: "global true", + globalYAML: "draft_pr: true\n", + repoYAML: "commands: {}\n", + want: true, + }, + { + name: "repo true overrides global false", + globalYAML: "draft_pr: false\n", + repoYAML: "draft_pr: true\n", + want: true, + }, + { + name: "repo false overrides global true", + globalYAML: "draft_pr: true\n", + repoYAML: "draft_pr: false\n", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + global, err := LoadGlobalFromBytes([]byte(tt.globalYAML)) + if err != nil { + t.Fatalf("LoadGlobalFromBytes() error = %v", err) + } + repo, err := LoadRepoFromBytes([]byte(tt.repoYAML)) + if err != nil { + t.Fatalf("LoadRepoFromBytes() error = %v", err) + } + + if got := Merge(global, repo).DraftPR; got != tt.want { + t.Errorf("DraftPR = %v, want %v", got, tt.want) + } + }) + } +} + func TestAutoFixLimit(t *testing.T) { cfg := &Config{ AutoFix: AutoFix{Lint: 5, Test: 2, Review: 0, Document: 1, CI: 3, Rebase: 4}, diff --git a/internal/pipeline/steps/host.go b/internal/pipeline/steps/host.go index 2ec48c8c7..41fa9cb91 100644 --- a/internal/pipeline/steps/host.go +++ b/internal/pipeline/steps/host.go @@ -45,7 +45,7 @@ func buildHost(sctx *pipeline.StepContext, provider scm.Provider) (scm.Host, str // the plain slug (without host prefix) is correct here. forkRepo = github.RepoSlug(sctx.Repo.ForkURL) } - return github.NewWithFork(cmdFactory, func() bool { return stepCLIAvailable(sctx, provider) }, host, repo, forkRepo), "" + return github.NewWithFork(cmdFactory, func() bool { return stepCLIAvailable(sctx, provider) }, host, repo, forkRepo).WithDraftPR(sctx.Config.DraftPR), "" case scm.ProviderGitLab: if sctx.Repo.ForkURL != "" { // Fork MR routing for GitLab is intentionally not half-wired. diff --git a/internal/pipeline/steps/pr_test.go b/internal/pipeline/steps/pr_test.go index 41a90de18..221498cb6 100644 --- a/internal/pipeline/steps/pr_test.go +++ b/internal/pipeline/steps/pr_test.go @@ -55,6 +55,7 @@ func TestPRStep_UpdatesExistingPR(t *testing.T) { ag := &mockAgent{name: "test"} sctx := newTestContextWithDBRecords(t, ag, dir, baseSHA, headSHA, config.Commands{}) sctx.Env = env + sctx.Config.DraftPR = true reviewStep, err := sctx.DB.InsertStepResult(sctx.Run.ID, types.StepReview) if err != nil { t.Fatal(err) @@ -81,6 +82,9 @@ func TestPRStep_UpdatesExistingPR(t *testing.T) { if !strings.Contains(ghLog, "pr edit") { t.Errorf("expected gh pr edit to be called, got:\n%s", ghLog) } + if strings.Contains(ghLog, "--draft") { + t.Errorf("existing PR update must not change draft readiness, got:\n%s", ghLog) + } if !strings.Contains(ghLog, "--body") { t.Errorf("expected --body flag in gh pr edit, got:\n%s", ghLog) } @@ -327,6 +331,7 @@ func TestPRStep_GitHubForkCreatesParentPRWithForkHead(t *testing.T) { } sctx := newTestContextWithDBRecords(t, ag, dir, baseSHA, headSHA, config.Commands{}) sctx.Env = env + sctx.Config.DraftPR = true sctx.Repo.UpstreamURL = "https://github.com/parent-owner/no-mistakes.git" sctx.Repo.ForkURL = "https://github.com/fork-owner/no-mistakes.git" sctx.Run.Branch = "refs/heads/feature" @@ -347,7 +352,7 @@ func TestPRStep_GitHubForkCreatesParentPRWithForkHead(t *testing.T) { if strings.Contains(ghLog, "pr list --head fork-owner:feature") { t.Fatalf("PR lookup used unsupported owner-qualified --head, got:\n%s", ghLog) } - if !strings.Contains(ghLog, "pr create --head fork-owner:feature --base main --repo parent-owner/no-mistakes") { + if !strings.Contains(ghLog, "pr create --head fork-owner:feature --base main --repo parent-owner/no-mistakes --draft") { t.Fatalf("expected PR create to target parent repo with fork owner head, got:\n%s", ghLog) } if strings.Contains(ghLog, "--repo fork-owner/no-mistakes") { diff --git a/internal/scm/github/github.go b/internal/scm/github/github.go index d0de176ff..be7fb8937 100644 --- a/internal/scm/github/github.go +++ b/internal/scm/github/github.go @@ -24,6 +24,7 @@ type Host struct { host string // repo's GitHub hostname; scopes the auth check repo string // "owner/name" slug for --repo; empty when unknown forkOwner string // fork owner for cross-repository PR heads + draftPR bool } // New builds a Host. cliAvailable reports whether the gh binary is @@ -55,6 +56,13 @@ func NewWithFork(cmd CmdFactory, cliAvailable func() bool, host, repo, forkRepo return h } +// WithDraftPR configures this host to create new pull requests as drafts. +// Existing PR updates deliberately leave readiness unchanged. +func (h *Host) WithDraftPR(enabled bool) *Host { + h.draftPR = enabled + return h +} + // RepoSlug extracts the "owner/name" identifier from a GitHub remote or PR // URL. Longer paths such as PR links are reduced to their leading two segments. func RepoSlug(remoteURL string) string { @@ -221,6 +229,9 @@ func (h *Host) CreatePR(ctx context.Context, branch, base string, content scm.PR "--head", h.headRef(branch), "--base", base, }, h.repoArgs()...) + if h.draftPR { + args = append(args, "--draft") + } args = append(args, "--title", content.Title, "--body-file", "-") cmd := h.cmd(ctx, "gh", args...) cmd.Stdin = strings.NewReader(content.Body) diff --git a/internal/scm/github/github_test.go b/internal/scm/github/github_test.go index 7da630403..fe8477b5b 100644 --- a/internal/scm/github/github_test.go +++ b/internal/scm/github/github_test.go @@ -153,6 +153,40 @@ func TestCreatePRStreamsBodyThroughStdin(t *testing.T) { } } +func TestCreatePRDraftAddsFlagOnlyToNewGitHubPR(t *testing.T) { + t.Parallel() + + var recorded [][]string + host := New(recordingCmdFactory("https://github.com/test/repo/pull/42\n", &recorded), nil, "", "test/repo").WithDraftPR(true) + if _, err := host.CreatePR(context.Background(), "feature/draft", "main", scm.PRContent{Title: "feat: draft", Body: "body"}); err != nil { + t.Fatalf("CreatePR() error = %v", err) + } + if len(recorded) != 1 { + t.Fatalf("expected one gh invocation, got %d: %v", len(recorded), recorded) + } + want := []string{"gh", "pr", "create", "--head", "feature/draft", "--base", "main", "--repo", "test/repo", "--draft", "--title", "feat: draft", "--body-file", "-"} + if got := recorded[0]; strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("create argv = %v, want %v", got, want) + } +} + +func TestCreatePRDraftUsesForkHead(t *testing.T) { + t.Parallel() + + var recorded [][]string + host := NewWithFork(recordingCmdFactory("https://github.com/parent/repo/pull/42\n", &recorded), nil, "", "parent/repo", "fork-owner/repo").WithDraftPR(true) + if _, err := host.CreatePR(context.Background(), "feature/draft", "main", scm.PRContent{Title: "feat: draft", Body: "body"}); err != nil { + t.Fatalf("CreatePR() error = %v", err) + } + if len(recorded) != 1 { + t.Fatalf("expected one gh invocation, got %d: %v", len(recorded), recorded) + } + want := []string{"gh", "pr", "create", "--head", "fork-owner:feature/draft", "--base", "main", "--repo", "parent/repo", "--draft", "--title", "feat: draft", "--body-file", "-"} + if got := recorded[0]; strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("fork create argv = %v, want %v", got, want) + } +} + func TestUpdatePRStreamsBodyThroughStdin(t *testing.T) { t.Parallel()