fix(pulls+issues): first-PR dead ends die — forms keep your input, templates render, single-branch guidance #5547
3 changed files+180−32
Addedsrc/__tests__/first-pr-form-honesty.test.ts+59−0View fileUnifiedSplit
@@ -0,0 +1,59 @@
1/**
2 * Pins the 2026-08-27 first-PR dead-end fixes (flow audit #3/#6 + the
3 * empty-dropdown finding):
4 *
5 * - pulls.tsx: POST validation failures re-render the form with the
6 * typed values (renderNewPrForm) — never a ?error= redirect that
7 * wipes the user's title and description.
8 * - pulls.tsx: single-branch repos get guidance, not an empty head
9 * <Select> that dead-ends into a cryptic error.
10 * - pulls.tsx + issues.tsx: the PR/issue templates that were loaded and
11 * silently discarded now reach the body TextArea.
12 *
13 * Source-level pins (the render paths need a live DB); each asserts the
14 * specific wiring whose absence WAS the bug, not mere symbol presence.
15 */
16
17import { describe, it, expect } from "bun:test";
18import { readFileSync } from "fs";
19import { join } from "path";
20
21const pullsSrc = readFileSync(
22 join(import.meta.dir, "../routes/pulls.tsx"),
23 "utf8"
24);
25const issuesSrc = readFileSync(
26 join(import.meta.dir, "../routes/issues.tsx"),
27 "utf8"
28);
29
30describe("new-PR form", () => {
31 it("no validation failure redirects to a blank form any more", () => {
32 expect(pullsSrc).not.toContain("pulls/new?error=");
33 });
34
35 it("validation failures re-render with the typed values", () => {
36 // The POST handler must thread title/prBody/base/head back into the
37 // shared renderer.
38 expect(pullsSrc).toContain("renderNewPrForm(c, {");
39 expect(pullsSrc).toMatch(/typed\s*=\s*\{\s*title,\s*prBody,/);
40 });
41
42 it("the PR template reaches the body TextArea", () => {
43 expect(pullsSrc).toMatch(/prBody \?\? template \?\? ""/);
44 expect(pullsSrc).toMatch(/value=\{bodyValue\}/);
45 });
46
47 it("single-branch repos get guidance instead of an empty head dropdown", () => {
48 expect(pullsSrc).toContain("headChoices.length === 0");
49 expect(pullsSrc).toContain("git checkout -b my-feature");
50 });
51});
52
53describe("new-issue form", () => {
54 it("the issue template reaches the body TextArea", () => {
55 // The load call existed for years; the value wiring is what was missing.
56 expect(issuesSrc).toContain("loadIssueTemplate");
57 expect(issuesSrc).toMatch(/value=\{template \?\? ""\}/);
58 });
59});
Modifiedsrc/routes/issues.tsx+4−0View fileUnifiedSplit
@@ -1320,10 +1320,14 @@ issueRoutes.get(
13201320 />
13211321 </FormGroup>
13221322 <FormGroup>
1323 {/* Prefilled from .github/ISSUE_TEMPLATE.md — the template
1324 was loaded above and then silently discarded for as long
1325 as this form has existed (flow audit #3). */}
13231326 <TextArea
13241327 name="body"
13251328 rows={12}
13261329 placeholder="Leave a comment... (Markdown supported)"
1330 value={template ?? ""}
13271331 mono
13281332 />
13291333 </FormGroup>
Modifiedsrc/routes/pulls.tsx+117−32View fileUnifiedSplit
@@ -3899,29 +3899,84 @@ pulls.get("/:owner/:repo/pulls/insights", softAuth, requireRepoAccess("read"), a
38993899 );
39003900});
39013901
3902// New PR form
3903pulls.get(
3904 "/:owner/:repo/pulls/new",
3905 softAuth,
3906 requireAuth,
3907 requireRepoAccess("write"),
3908 async (c) => {
3909 const { owner: ownerName, repo: repoName } = c.req.param();
3910 const user = c.get("user")!;
3911 const branches = await listBranches(ownerName, repoName);
3912 const error = c.req.query("error");
3913 const defaultBase = branches.includes("main") ? "main" : branches[0] || "";
3914 const template = await loadPrTemplate(ownerName, repoName);
3915
3902/**
3903 * Shared renderer for the new-PR form.
3904 *
3905 * Exists so every POST validation failure re-renders the form WITH the
3906 * values the user typed (title, description, branch picks) — the old
3907 * `?error=` redirect threw all of it away on every mistake, the single
3908 * most common friction the flow audit found. Also fixes two more of its
3909 * findings in one place: the PR template used to be loaded and silently
3910 * discarded (now it prefills the description when the user hasn't typed
3911 * one), and a single-branch repo used to render an EMPTY head dropdown —
3912 * a dead end that submitted into a cryptic error. Now it renders honest
3913 * guidance: a PR needs a second branch, here are the commands.
3914 */
3915async function renderNewPrForm(
3916 c: any,
3917 opts: {
3918 error?: string;
3919 title?: string;
3920 prBody?: string;
3921 base?: string;
3922 head?: string;
3923 } = {}
3924): Promise<Response> {
3925 const { owner: ownerName, repo: repoName } = c.req.param();
3926 const user = c.get("user")!;
3927 const branches = await listBranches(ownerName, repoName);
3928 const defaultBase =
3929 opts.base && branches.includes(opts.base)
3930 ? opts.base
3931 : branches.includes("main")
3932 ? "main"
3933 : branches[0] || "";
3934 const headChoices = branches.filter((b) => b !== defaultBase);
3935 const selectedHead =
3936 opts.head && headChoices.includes(opts.head)
3937 ? opts.head
3938 : headChoices[0] || "";
3939 // Prefill from .github/PULL_REQUEST_TEMPLATE.md only when the user has
3940 // not typed a body of their own.
3941 const template =
3942 opts.prBody === undefined
3943 ? await loadPrTemplate(ownerName, repoName)
3944 : null;
3945 const bodyValue = opts.prBody ?? template ?? "";
3946 const error = opts.error;
3947
3948 if (headChoices.length === 0) {
39163949 return c.html(
39173950 <Layout title={`New PR — ${ownerName}/${repoName}`} user={user}>
39183951 <RepoHeader owner={ownerName} repo={repoName} />
39193952 <PrNav owner={ownerName} repo={repoName} active="pulls" />
39203953 <Container maxWidth={800}>
39213954 <h2 style="margin-bottom:16px">Open a pull request</h2>
3922 {error && (
3923 <Alert variant="error">{decodeURIComponent(error)}</Alert>
3955 <Alert variant="info">
3956 {branches.length === 0
3957 ? "This repository has no branches yet — push some commits first, then create a branch to compare."
3958 : `A pull request compares two branches, and this repository has only ${defaultBase}. Create a branch, push it, then come back:`}
3959 </Alert>
3960 {branches.length > 0 && (
3961 <pre style="background:var(--bg-inset);border:1px solid var(--border);border-radius:8px;padding:14px 16px;font-size:13px;overflow-x:auto;margin:12px 0">
3962 {`git checkout -b my-feature\n# ...commit your changes...\ngit push -u origin my-feature`}
3963 </pre>
39243964 )}
3965 <p style="margin-top:12px">
3966 <a href={`/${ownerName}/${repoName}`}>← Back to repository</a>
3967 </p>
3968 </Container>
3969 </Layout>
3970 );
3971 }
3972
3973 return c.html(
3974 <Layout title={`New PR — ${ownerName}/${repoName}`} user={user}>
3975 <RepoHeader owner={ownerName} repo={repoName} />
3976 <PrNav owner={ownerName} repo={repoName} active="pulls" />
3977 <Container maxWidth={800}>
3978 <h2 style="margin-bottom:16px">Open a pull request</h2>
3979 {error && <Alert variant="error">{error}</Alert>}
39253980 <Form method="post" action={`/${ownerName}/${repoName}/pulls/new`}>
39263981 <Flex gap={12} align="center" style="margin-bottom: 16px">
39273982 <Select name="base">
@@ -3933,12 +3988,11 @@ pulls.get(
39333988 </Select>
39343989 <Text muted>←</Text>
39353990 <Select name="head">
3936 {branches
3937 .filter((b) => b !== defaultBase)
3938 .concat(defaultBase === branches[0] ? [] : [branches[0]])
3939 .map((b) => (
3940 <option value={b}>{b}</option>
3941 ))}
3991 {headChoices.map((b) => (
3992 <option value={b} selected={b === selectedHead}>
3993 {b}
3994 </option>
3995 ))}
39423996 </Select>
39433997 </Flex>
39443998 <FormGroup>
@@ -3946,6 +4000,7 @@ pulls.get(
39464000 name="title"
39474001 required
39484002 placeholder="Title"
4003 value={opts.title}
39494004 style="font-size:16px;padding:10px 14px"
39504005 aria-label="Pull request title"
39514006 />
@@ -3956,6 +4011,7 @@ pulls.get(
39564011 id="pr-body"
39574012 rows={8}
39584013 placeholder="Description (Markdown supported)"
4014 value={bodyValue}
39594015 mono
39604016 />
39614017 </FormGroup>
@@ -3985,7 +4041,23 @@ pulls.get(
39854041 />
39864042 </Container>
39874043 </Layout>
3988 );
4044 );
4045}
4046
4047// New PR form
4048pulls.get(
4049 "/:owner/:repo/pulls/new",
4050 softAuth,
4051 requireAuth,
4052 requireRepoAccess("write"),
4053 async (c) => {
4054 // `?error=` and `?head=` are kept for old links; new error paths
4055 // re-render directly with preserved values instead of redirecting.
4056 const error = c.req.query("error");
4057 return renderNewPrForm(c, {
4058 error: error ? decodeURIComponent(error) : undefined,
4059 head: c.req.query("head") || undefined,
4060 });
39894061 }
39904062);
39914063
@@ -4084,16 +4156,28 @@ pulls.post(
40844156 const baseBranch = String(body.base || "main");
40854157 const headBranch = String(body.head || "");
40864158
4159 // Validation failures re-render the form with everything the user
4160 // typed — the old ?error= redirects landed on a BLANK form, so a
4161 // one-word mistake cost the whole description.
4162 const typed = {
4163 title,
4164 prBody,
4165 base: baseBranch,
4166 head: headBranch,
4167 };
4168
40874169 if (!title || !headBranch) {
4088 return c.redirect(
4089 `/${ownerName}/${repoName}/pulls/new?error=Title+and+branches+are+required`
4090 );
4170 return renderNewPrForm(c, {
4171 ...typed,
4172 error: "Title and branches are required.",
4173 });
40914174 }
40924175
40934176 if (baseBranch === headBranch) {
4094 return c.redirect(
4095 `/${ownerName}/${repoName}/pulls/new?error=Base+and+head+branches+must+be+different`
4096 );
4177 return renderNewPrForm(c, {
4178 ...typed,
4179 error: "Base and head branches must be different.",
4180 });
40974181 }
40984182
40994183 // Same guard as the JSON create path above: these are request-body values
@@ -4101,9 +4185,10 @@ pulls.post(
41014185 // as an option rather than a ref. Rejecting here also keeps the stored PR
41024186 // row from carrying an option-like branch into every later diff and merge.
41034187 if (!isSafeRef(baseBranch) || !isSafeRef(headBranch)) {
4104 return c.redirect(
4105 `/${ownerName}/${repoName}/pulls/new?error=Invalid+branch+name`
4106 );
4188 return renderNewPrForm(c, {
4189 ...typed,
4190 error: "Invalid branch name.",
4191 });
41074192 }
41084193
41094194 const resolved = await resolveRepo(ownerName, repoName);
41104195
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts