feat(pr): draft PRs by default, auto-undraft in pr-patrol (#1794)

OAGr · claude · web-flow · commit 1fac8c522fb1 · 2026-03-05T18:37:28.000-08:00
* fix(ci): increase scheduled-maintenance max-turns and timeout

Previous run (2026-03-05T07:59:11Z) failed with error_max_turns after
31 turns — Claude ran out with max-turns=30 on a weekly sweep. Also
increase timeout-minutes from 30 to 60 to give sufficient wall-clock
headroom for weekly/monthly cadences.

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;

* feat(pr): draft PRs by default, auto-undraft in pr-patrol

- `crux pr create` now creates draft PRs by default (use --no-draft to override)
- New `crux pr ready` command validates eligibility (CI green, no conflicts,
  no unresolved threads, no unchecked items) before converting draft to ready
- PR Patrol auto-undrafts draft PRs labeled `ready-to-merge` when all other
  eligibility checks pass, then merges in the same cycle
- PR Patrol skips draft PRs from the fix queue (no point fixing drafts)
- Added `isDraft` to GraphQL PR query and `GqlPrNode` interface
- `is-draft` added as a new `MergeBlockReason`
- 4 new tests covering draft eligibility checks

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

* fix: address review findings — is-draft self-block, REST fallback, failed undraft tracking

- Filter out 'is-draft' from block reasons in `crux pr ready` (it's the
  whole point of the command, not a blocker)
- Replace REST-then-GraphQL fallback with direct GraphQL for undrafting
  (REST API doesn't support undrafting, so the REST call always fails)
- Track which PRs were successfully undrafted before attempting merge
  (prevents merge attempts on PRs where undraft API call failed)

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

* fix(pr): address CodeRabbit review feedback

- Validate --pr option: return error on malformed values instead of
  silently falling back to branch autodetection
- Keep ci-pending as a blocker for `crux pr ready` to match PR Patrol
  auto-undraft behavior (only is-draft is filtered out)
- Simulate undraft in dry-run mode so merge phase also reports what
  would happen

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

---------

Co-authored-by: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/crux/commands/pr.ts b/crux/commands/pr.ts
@@ -197,7 +197,8 @@ async function detect(_args: string[], options: CommandOptions): Promise<Command
  *   --body="PR body"        Body as inline string (vulnerable to shell expansion).
  *   --body-file=<path>      Body from a file (safe for markdown with backticks).
  *   --base=main             Base branch (default: main).
- *   --draft                 Create as draft PR.
+ *   --draft                 Create as draft PR (default: true).
+ *   --no-draft              Create as ready PR (not draft).
  *
  * If a PR already exists for this branch, reports it instead of creating a duplicate.
  */
@@ -210,7 +211,8 @@ async function create(_args: string[], options: CommandOptions): Promise<Command
   const bodyFile = (options.bodyFile ?? options['body-file']) as string | undefined;
   let body = options.body as string | undefined;
   const base = (options.base as string) || 'main';
-  const draft = Boolean(options.draft);
+  // Default to draft unless --no-draft is explicitly passed
+  const draft = options.noDraft === true || options['no-draft'] === true ? false : true;
 
   // --body-file takes precedence (avoids shell expansion of backticks in markdown)
   if (bodyFile) {
@@ -602,12 +604,108 @@ async function resolveConflictsCmd(_args: string[], options: CommandOptions): Pr
   return { output, exitCode: failed > 0 ? 1 : 0 };
 }
 
+/**
+ * Mark the current branch's PR as ready for review.
+ *
+ * Validates eligibility (CI green, no conflicts, no unresolved threads,
+ * no unchecked checkboxes) before converting from draft to ready.
+ *
+ * Options:
+ *   --pr=N    Target a specific PR number instead of auto-detecting.
+ *   --force   Skip eligibility checks and mark as ready anyway.
+ */
+async function ready(_args: string[], options: CommandOptions): Promise<CommandResult> {
+  const log = createLogger(Boolean(options.ci));
+  const c = log.colors;
+  const force = Boolean(options.force);
+
+  let prNum: number | null = null;
+  if (options.pr !== undefined) {
+    const rawPr = String(options.pr);
+    const parsed = parseInt(rawPr, 10);
+    if (!Number.isInteger(parsed) || parsed <= 0) {
+      return {
+        output: `${c.red}Invalid --pr value: ${rawPr}${c.reset}\n`,
+        exitCode: 1,
+      };
+    }
+    prNum = parsed;
+  }
+
+  if (!prNum) {
+    const branch = currentBranch();
+    const prs = await githubApi<GitHubPR[]>(
+      `/repos/${REPO}/pulls?head=quantified-uncertainty:${branch}&state=open`
+    );
+    if (!prs.length) {
+      return {
+        output: `${c.red}No open PR found for branch ${currentBranch()}.${c.reset}\n`,
+        exitCode: 1,
+      };
+    }
+    prNum = prs[0].number;
+  }
+
+  // Fetch full PR details via GraphQL for eligibility checks
+  const { checkMergeEligibility, fetchSinglePr } = await import('../pr-patrol/index.ts');
+  const prNode = await fetchSinglePr(prNum);
+
+  if (!prNode) {
+    return {
+      output: `${c.red}Could not fetch PR #${prNum} details.${c.reset}\n`,
+      exitCode: 1,
+    };
+  }
+
+  // Check eligibility (unless --force)
+  if (!force) {
+    const eligibility = checkMergeEligibility(prNode);
+    // Ignore only is-draft: this command exists to remove that block.
+    // CI must still be green — matching PR Patrol auto-undraft behavior.
+    const blockReasons = eligibility.blockReasons.filter(
+      (r: string) => r !== 'is-draft',
+    );
+
+    if (blockReasons.length > 0) {
+      let output = `${c.red}✗ PR #${prNum} is not eligible to be marked ready:${c.reset}\n`;
+      for (const reason of blockReasons) {
+        output += `  - ${reason}\n`;
+      }
+      output += `\n  Use --force to mark ready anyway.\n`;
+      return { output, exitCode: 1 };
+    }
+  }
+
+  // Convert from draft to ready via GraphQL mutation
+  // (GitHub REST API doesn't support undrafting — only GraphQL works)
+  try {
+    const { githubGraphQL } = await import('../lib/github.ts');
+    const prData = await githubApi<{ node_id: string }>(`/repos/${REPO}/pulls/${prNum}`);
+    await githubGraphQL(
+      `mutation($id: ID!) { markPullRequestReadyForReview(input: { pullRequestId: $id }) { pullRequest { isDraft } } }`,
+      { id: prData.node_id },
+    );
+  } catch (e: unknown) {
+    const msg = e instanceof Error ? e.message : String(e);
+    return {
+      output: `${c.red}Failed to mark PR #${prNum} as ready: ${msg}${c.reset}\n`,
+      exitCode: 1,
+    };
+  }
+
+  let output = `${c.green}✓${c.reset} PR #${prNum} marked as ready for review.\n`;
+  if (force) output += `  ${c.yellow}(eligibility checks skipped with --force)${c.reset}\n`;
+
+  return { output, exitCode: 0 };
+}
+
 // ---------------------------------------------------------------------------
 // Domain entry point (required by crux.mjs dispatch)
 // ---------------------------------------------------------------------------
 
 export const commands = {
   create,
+  ready,
   detect,
   'fix-body': fixBody,
   'rebase-all': rebaseAll,
@@ -620,7 +718,8 @@ export function getHelp(): string {
 PR Domain — GitHub Pull Request utilities
 
 Commands:
-  create                        Create a PR for the current branch (corruption-safe).
+  create                        Create a draft PR for the current branch (corruption-safe).
+  ready [--pr=N]                Mark PR as ready (validates eligibility, removes draft status).
   detect                        Detect open PR for current branch (returns URL + number).
   fix-body [--pr=N]             Detect and repair literal \\n in the current branch's PR body.
   rebase-all                    Rebase all open non-draft PRs onto main (used by CI).
@@ -632,11 +731,15 @@ Options (create):
   --body="..."        PR body (inline — avoid for multi-line bodies; use --body-file or stdin).
   --body-file=<path>  PR body from file (safe for markdown with backticks).
   --base=main         Base branch (default: main).
-  --draft             Create as draft PR.
+  --no-draft          Create as ready PR (default: draft).
   --allow-empty-body  Allow creating PR without a description (not recommended).
   --skip-test-plan    Skip test plan validation (not recommended).
   (stdin)             If --body and --body-file are absent and stdin is a pipe, body is read from stdin.
 
+Options (ready):
+  --pr=N              Target a specific PR number instead of auto-detecting.
+  --force             Skip eligibility checks and mark as ready anyway.
+
 Options (detect):
   --ci                JSON output.
 
diff --git a/crux/pr-patrol/index.test.ts b/crux/pr-patrol/index.test.ts
@@ -14,6 +14,7 @@ function makePrNode(overrides: Partial<GqlPrNode> = {}): GqlPrNode {
     title: 'Test PR',
     headRefName: 'claude/test',
     mergeable: 'MERGEABLE',
+    isDraft: false,
     createdAt: '2026-01-01T00:00:00Z',
     updatedAt: '2026-03-05T00:00:00Z',
     body: '## Summary\n\n- [x] Task done\n\n## Test plan\n\n- [x] Tests pass\n\nCloses #1',
@@ -283,6 +284,20 @@ describe('checkMergeEligibility', () => {
     expect(result.blockReasons).toContain('claude-working');
   });
 
+  it('blocks when PR is a draft', () => {
+    const result = checkMergeEligibility(
+      makePrNode({ isDraft: true }),
+    );
+    expect(result.eligible).toBe(false);
+    expect(result.blockReasons).toContain('is-draft');
+  });
+
+  it('does NOT block when PR is not a draft', () => {
+    const result = checkMergeEligibility(makePrNode({ isDraft: false }));
+    expect(result.eligible).toBe(true);
+    expect(result.blockReasons).not.toContain('is-draft');
+  });
+
   it('returns multiple block reasons when several checks fail', () => {
     const result = checkMergeEligibility(
       makePrNode({
@@ -349,6 +364,23 @@ describe('findMergeCandidates', () => {
     expect(result.find((c) => c.number === 1)?.eligible).toBe(true);
     expect(result.find((c) => c.number === 2)?.eligible).toBe(false);
   });
+
+  it('marks draft PRs as blocked with is-draft reason', () => {
+    const draftPr = makePrNode({ number: 1, isDraft: true });
+    const result = findMergeCandidates([draftPr]);
+    expect(result).toHaveLength(1);
+    expect(result[0].eligible).toBe(false);
+    expect(result[0].blockReasons).toContain('is-draft');
+  });
+
+  it('draft PR with only is-draft block is undraft-eligible', () => {
+    const draftPr = makePrNode({ number: 1, isDraft: true });
+    const result = findMergeCandidates([draftPr]);
+    const undraftEligible = result.filter(
+      (c) => !c.eligible && c.blockReasons.length === 1 && c.blockReasons[0] === 'is-draft',
+    );
+    expect(undraftEligible).toHaveLength(1);
+  });
 });
 
 // ── detectIssues (regression test after refactor) ────────────────────────────
diff --git a/crux/pr-patrol/index.ts b/crux/pr-patrol/index.ts
@@ -58,7 +58,8 @@ export type MergeBlockReason =
   | 'ci-pending'
   | 'unresolved-threads'
   | 'unchecked-items'
-  | 'claude-working';
+  | 'claude-working'
+  | 'is-draft';
 
 export interface MergeCandidate {
   number: number;
@@ -486,7 +487,7 @@ const PR_QUERY = `query($owner: String!, $name: String!) {
   repository(owner: $owner, name: $name) {
     pullRequests(first: 50, states: [OPEN], orderBy: {field: UPDATED_AT, direction: DESC}) {
       nodes {
-        number title headRefName mergeable createdAt updatedAt body
+        number title headRefName mergeable isDraft createdAt updatedAt body
         labels(first: 20) { nodes { name } }
         commits(last: 1) { nodes { commit { statusCheckRollup {
           contexts(first: 50) { nodes {
@@ -525,6 +526,7 @@ export interface GqlPrNode {
   title: string;
   headRefName: string;
   mergeable: string;
+  isDraft: boolean;
   createdAt: string;
   updatedAt: string;
   body: string | null;
@@ -625,6 +627,42 @@ export async function fetchOpenPrs(config: PatrolConfig): Promise<GqlPrNode[]> {
   return prs;
 }
 
+const SINGLE_PR_QUERY = `query($owner: String!, $name: String!, $number: Int!) {
+  repository(owner: $owner, name: $name) {
+    pullRequest(number: $number) {
+      number title headRefName mergeable isDraft createdAt updatedAt body
+      labels(first: 20) { nodes { name } }
+      commits(last: 1) { nodes { commit { statusCheckRollup {
+        contexts(first: 50) { nodes {
+          ... on CheckRun { conclusion }
+          ... on StatusContext { state }
+        }}
+      }}}}
+      reviewThreads(first: 50) { nodes {
+        isResolved isOutdated path line startLine
+        comments(first: 3) { nodes {
+          author { login }
+          body
+        }}
+      }}
+    }
+  }
+}`;
+
+/** Fetch a single PR by number. Used by `crux pr ready` for eligibility checks. */
+export async function fetchSinglePr(prNumber: number): Promise<GqlPrNode | null> {
+  const [owner, name] = REPO.split('/');
+  try {
+    const data = await githubGraphQL<{
+      repository: { pullRequest: GqlPrNode | null };
+    }>(SINGLE_PR_QUERY, { owner, name, number: prNumber });
+    return data.repository.pullRequest;
+  } catch (e) {
+    log(`Warning: could not fetch PR #${prNumber}: ${e instanceof Error ? e.message : String(e)}`);
+    return null;
+  }
+}
+
 function detectAllPrIssuesFromNodes(
   prs: GqlPrNode[],
   config: PatrolConfig,
@@ -635,6 +673,8 @@ function detectAllPrIssuesFromNodes(
     .filter((pr) => {
       const labels = pr.labels.nodes.map((l) => l.name);
       if (labels.includes('claude-working')) return false;
+      // Skip draft PRs — they're not ready for automated fixes
+      if (pr.isDraft) return false;
       return true;
     })
     .map((pr) => {
@@ -688,6 +728,10 @@ export function checkMergeEligibility(pr: GqlPrNode): MergeCandidate {
   const blockReasons: MergeBlockReason[] = [];
   const labels = pr.labels.nodes.map((l) => l.name);
 
+  if (pr.isDraft) {
+    blockReasons.push('is-draft');
+  }
+
   if (labels.includes('claude-working')) {
     blockReasons.push('claude-working');
   }
@@ -759,6 +803,41 @@ export function findMergeCandidates(prs: GqlPrNode[]): MergeCandidate[] {
     );
 }
 
+// ── Undraft execution ────────────────────────────────────────────────────────
+
+async function undraftPr(prNum: number, config: PatrolConfig): Promise<boolean> {
+  log(`→ Undrafting PR #${prNum} (all eligibility checks pass)`);
+
+  try {
+    // GitHub REST API doesn't support undrafting — must use GraphQL mutation
+    const prData = await githubApi<{ node_id: string }>(
+      `/repos/${config.repo}/pulls/${prNum}`,
+    );
+    await githubGraphQL(
+      `mutation($id: ID!) { markPullRequestReadyForReview(input: { pullRequestId: $id }) { pullRequest { isDraft } } }`,
+      { id: prData.node_id },
+    );
+
+    log(`✓ PR #${prNum} marked as ready for review`);
+    appendJsonl(JSONL_FILE, {
+      type: 'undraft_result',
+      pr_num: prNum,
+      outcome: 'undrafted',
+    });
+    return true;
+  } catch (e) {
+    const msg = e instanceof Error ? e.message : String(e);
+    log(`✗ Failed to undraft PR #${prNum}: ${msg}`);
+    appendJsonl(JSONL_FILE, {
+      type: 'undraft_result',
+      pr_num: prNum,
+      outcome: 'error',
+      reason: msg,
+    });
+    return false;
+  }
+}
+
 // ── Merge execution ─────────────────────────────────────────────────────────
 
 async function mergePr(
@@ -1298,9 +1377,34 @@ async function runCheckCycle(
     }
   }
 
-  // ── Merge phase ────────────────────────────────────────────────────
+  // ── Undraft phase ──────────────────────────────────────────────────
+  // Auto-undraft draft PRs that are otherwise eligible for merge.
+  // A PR is auto-undrafted when its only block reason is 'is-draft'.
 
-  const mergeCandidates = findMergeCandidates(allPrs);
+  const draftCandidates = findMergeCandidates(allPrs).filter(
+    (c) => !c.eligible && c.blockReasons.length === 1 && c.blockReasons[0] === 'is-draft',
+  );
+
+  const undraftedNumbers = new Set<number>();
+  for (const candidate of draftCandidates) {
+    if (config.dryRun) {
+      log(`  [DRY RUN] Would undraft PR #${candidate.number} (all other checks pass)`);
+      undraftedNumbers.add(candidate.number);
+    } else {
+      const success = await undraftPr(candidate.number, config);
+      if (success) undraftedNumbers.add(candidate.number);
+    }
+  }
+
+  // ── Merge phase ────────────────────────────────────────────────────
+  // Re-evaluate after undrafting (only successfully undrafted PRs become eligible)
+  const mergeCandidates = findMergeCandidates(allPrs).map((c) => {
+    if (undraftedNumbers.has(c.number)) {
+      const updated = { ...c, blockReasons: c.blockReasons.filter((r) => r !== 'is-draft') };
+      return { ...updated, eligible: updated.blockReasons.length === 0 };
+    }
+    return c;
+  });
   const eligibleForMerge = mergeCandidates.filter((c) => c.eligible);
   const blockedForMerge = mergeCandidates.filter((c) => !c.eligible);
   let mergedPr: number | null = null;
