Follow-up to #266: make the pull request template actually reach GitHub

CHANGELOG.md

@@ -2,22 +2,17 @@
 
 ## [Unreleased]
 
-### Fixed
-
-- `issue mine`, `issue query`, `issue start`, and `team states` now group statuses in the same order as the Linear app: by workflow state type, then by the team's configured position within that type. Issue listings previously ran the order backwards (canceled and done first), and every status list sorted on raw position alone, which stranded a late-positioned status such as an "In Review" at position 1002 after "Duplicate" instead of beside "In Progress"
-
-### Changed
-
-- when `--limit` truncates an issue listing, the retained issues are now the most actionable rather than the most recently closed. The Linear API cannot sort by a team's configured positions, so it still selects which issues are fetched; that selection changed from closed-first to open-first. A status this build does not recognize sorts after all known ones
-
 ### Added
 
+- `issue pr` accepts `--template/-T <file>` to start the pull request body from a template file, with a `pr_template` config option (`LINEAR_PR_TEMPLATE`) as a per-project default and `--no-template` to skip that default for one invocation. The Linear issue URL is appended after the template, so the pull request stays linked to its issue
 - issue comment list --json now exposes stable author identity: `user.id`, `externalUser.id`, and a `botActor` object (`id`, `name`, `type`, `subType`) for comments posted by integrations. Display names are editable and can collide across a workspace — an external user's display name can even match a real member's — so programs consuming the JSON previously had nothing reliable to attribute a comment with
 - issue comment list --json now includes `editedAt`, which is set only when a comment's author revised it. `updatedAt` also moves for unrelated backend churn, so it could not answer "has this been changed since it was written?"
 - `LINEAR_IGNORE_ENV_FILE=1` skips `.env` loading entirely, for repositories whose `.env` is not dotenv-shaped
 
 ### Changed
 
+- `issue mine`, `issue query`, `issue start`, and `team states` now group statuses in the same order as the Linear app: by workflow state type, then by the team's configured position within that type. Issue listings previously ran the order backwards (canceled and done first), and every status list sorted on raw position alone, which stranded a late-positioned status such as an "In Review" at position 1002 after "Duplicate" instead of beside "In Progress"
+- when `--limit` truncates an issue listing, the retained issues are now the most actionable rather than the most recently closed. The Linear API cannot sort by a team's configured positions, so it still selects which issues are fetched; that selection changed from closed-first to open-first. A status this build does not recognize sorts after all known ones
 - an unquoted `$VAR` reference in a `LINEAR_`/`GH_`/`GITHUB_` value is now skipped with a warning rather than expanded. Expansion of an unset variable silently produced the string `"undefined"`, and a self-referential one hung. Quoted values are unaffected, since dotenv never expanded those
 
 ### Fixed

README.md

@@ -274,15 +274,16 @@ linear completions     # generate shell completions
 
 the CLI supports configuration via environment variables or a `.linear.toml` config file. environment variables take precedence over config file values.
 
-| option          | env var                           | toml key                   | example                            | description                                           |
-| --------------- | --------------------------------- | -------------------------- | ---------------------------------- | ----------------------------------------------------- |
-| Team ID         | `LINEAR_TEAM_ID`                  | `team_id`                  | `"ENG"`                            | default team for operations                           |
-| Workspace       | `LINEAR_WORKSPACE`                | `workspace`                | `"mycompany"`                      | workspace slug for web/app URLs                       |
-| Issue sort      | `LINEAR_ISSUE_SORT`               | `issue_sort`               | `"priority"` or `"manual"`         | how to sort issue lists                               |
-| Ask project     | `LINEAR_ISSUE_CREATE_ASK_PROJECT` | `issue_create_ask_project` | `true` or `false`                  | ask for a project during interactive `issue create`   |
-| Assign self     | `LINEAR_ISSUE_CREATE_ASSIGN_SELF` | `issue_create_assign_self` | `"always"`, `"auto"`, or `"never"` | control default self-assignment during issue creation |
-| VCS             | `LINEAR_VCS`                      | `vcs`                      | `"git"` or `"jj"`                  | version control system (default: git)                 |
-| Download images | `LINEAR_DOWNLOAD_IMAGES`          | `download_images`          | `true` or `false`                  | download images when viewing issues                   |
+| option          | env var                           | toml key                   | example                              | description                                                                                      |
+| --------------- | --------------------------------- | -------------------------- | ------------------------------------ | ------------------------------------------------------------------------------------------------ |
+| Team ID         | `LINEAR_TEAM_ID`                  | `team_id`                  | `"ENG"`                              | default team for operations                                                                      |
+| Workspace       | `LINEAR_WORKSPACE`                | `workspace`                | `"mycompany"`                        | workspace slug for web/app URLs                                                                  |
+| Issue sort      | `LINEAR_ISSUE_SORT`               | `issue_sort`               | `"priority"` or `"manual"`           | how to sort issue lists                                                                          |
+| Ask project     | `LINEAR_ISSUE_CREATE_ASK_PROJECT` | `issue_create_ask_project` | `true` or `false`                    | ask for a project during interactive `issue create`                                              |
+| Assign self     | `LINEAR_ISSUE_CREATE_ASSIGN_SELF` | `issue_create_assign_self` | `"always"`, `"auto"`, or `"never"`   | control default self-assignment during issue creation                                            |
+| VCS             | `LINEAR_VCS`                      | `vcs`                      | `"git"` or `"jj"`                    | version control system (default: git)                                                            |
+| Download images | `LINEAR_DOWNLOAD_IMAGES`          | `download_images`          | `true` or `false`                    | download images when viewing issues                                                              |
+| PR template     | `LINEAR_PR_TEMPLATE`              | `pr_template`              | `".github/pull_request_template.md"` | template file for `issue pr` bodies (the Linear issue URL is appended; `--no-template` skips it) |
 
 the config file can be placed at (checked in order, first found is used):
 

src/commands/issue/issue-pull-request.ts

@@ -2,7 +2,89 @@ import { Command } from "@cliffy/command"
 import { fetchIssueDetails, getIssueIdentifier } from "../../utils/linear.ts"
 import { shouldShowSpinner } from "../../utils/hyperlink.ts"
 import { CliError, handleError, ValidationError } from "../../utils/errors.ts"
-import { getOption } from "../../config.ts"
+import { resolvePrTemplate } from "../../config.ts"
+
+/**
+ * Compose the pull request body from a template and the Linear issue URL.
+ *
+ * `gh pr create` refuses `--template` alongside `--body` ("`--template` is not
+ * supported when using `--body` or `--body-file`"), and dropping `--body` to
+ * pass `--template` instead is worse: `gh` only consults a template when it is
+ * running interactively, so a non-TTY caller gets "must provide `--title` and
+ * `--body` ... when not running interactively" and no pull request at all. So
+ * the template is read here and folded into the body we already send.
+ *
+ * The issue URL goes last: it is what Linear matches on to attach the pull
+ * request to the issue, and keeping it out of the way leaves the template's own
+ * prose as the first thing a reviewer reads.
+ */
+export function composePullRequestBody(
+  templateContents: string,
+  issueUrl: string,
+): string {
+  const template = templateContents.trimEnd()
+  return template === "" ? issueUrl : `${template}\n\n${issueUrl}`
+}
+
+/**
+ * Read a pull request template, rejecting anything that would not produce a
+ * usable body. An explicitly requested template that cannot be used is an
+ * error, never a silent fallback to the plain URL body -- the caller asked for
+ * it, so failing quietly would ship a pull request missing the content they
+ * expected.
+ */
+export async function readPullRequestTemplate(path: string): Promise<string> {
+  const unusable = (reason: string) =>
+    new ValidationError(`Cannot read pull request template: ${reason}`, {
+      suggestion:
+        "Pass a readable file to --template, fix the pr_template config option, or use --no-template to skip the template.",
+    })
+
+  if (path.trim() === "") {
+    throw unusable("the path is empty")
+  }
+
+  let info: Deno.FileInfo
+  try {
+    info = await Deno.stat(path)
+  } catch (error) {
+    if (error instanceof Deno.errors.NotFound) {
+      throw unusable(`"${path}" does not exist`)
+    }
+    throw unusable(
+      `"${path}" could not be read: ${
+        error instanceof Error ? error.message : String(error)
+      }`,
+    )
+  }
+  if (info.isDirectory) {
+    throw unusable(`"${path}" is a directory, not a file`)
+  }
+  if (!info.isFile) {
+    throw unusable(`"${path}" is not a regular file`)
+  }
+
+  let contents: string
+  try {
+    contents = await Deno.readTextFile(path)
+  } catch (error) {
+    throw unusable(
+      `"${path}" could not be read: ${
+        error instanceof Error ? error.message : String(error)
+      }`,
+    )
+  }
+
+  // Deno.readTextFile does not reject binary input -- it substitutes U+FFFD and
+  // keeps any NUL bytes, which Deno.Command then rejects with a bare
+  // "nul byte found in provided data" TypeError. Catch it here with a message
+  // that names the file.
+  if (contents.includes("\0")) {
+    throw unusable(`"${path}" is not a text file`)
+  }
+
+  return contents
+}
 
 export const pullRequestCommand = new Command()
   .name("pull-request")
@@ -29,17 +111,29 @@ export const pullRequestCommand = new Command()
     "The branch that contains commits for your pull request",
   )
   .option(
-    "-T, --template <template:string>",
-    "Optional template filename for the pull request body",
+    "-T, --template <file:string>",
+    "Start the pull request body from this template file (the Linear issue URL is appended)",
+  )
+  .option(
+    "--no-template",
+    "Ignore the pr_template config option for this pull request",
   )
   .arguments("[issueId:string]")
   .action(
     async (
       { base, draft, title: customTitle, web, head, template },
       issueId,
     ) => {
-      template = template ?? getOption("pr_template")
       try {
+        // `--no-template` arrives as false and opts out even when the config
+        // option is set; otherwise an explicit path wins over the default. A
+        // path from a config file resolves against that file, so a project-wide
+        // default keeps working from a subdirectory.
+        const templatePath = resolvePrTemplate(template)
+        const templateContents = templatePath == null
+          ? undefined
+          : await readPullRequestTemplate(templatePath)
+
         const resolvedId = await getIssueIdentifier(issueId)
         if (!resolvedId) {
           throw new ValidationError(
@@ -59,12 +153,13 @@ export const pullRequestCommand = new Command()
             "--title",
             `${resolvedId} ${customTitle ?? title}`,
             "--body",
-            url,
+            templateContents == null
+              ? url
+              : composePullRequestBody(templateContents, url),
             ...(base ? ["--base", base] : []),
             ...(head ? ["--head", head] : []),
             ...(draft ? ["--draft"] : []),
             ...(web ? ["--web"] : []),
-            ...(template && template.length ? ["--template", template] : []),
           ],
           stdin: "inherit",
           stdout: "inherit",

src/config.ts

@@ -1,12 +1,17 @@
 import { parse } from "@std/toml"
-import { join, resolve } from "@std/path"
+import { dirname, join, resolve } from "@std/path"
 import { parse as parseDotenv } from "@std/dotenv"
 import { gray, yellow } from "@std/fmt/colors"
 import * as v from "valibot"
 import { ValidationError } from "./utils/errors.ts"
 
 let globalConfig: Record<string, unknown> = {}
 let projectConfig: Record<string, unknown> = {}
+// Which file each of the above came from, so a relative path written in a
+// config file can be resolved against that file rather than the working
+// directory. See optionBaseDir().
+let globalConfigPath: string | null = null
+let projectConfigPath: string | null = null
 
 // Env keys that loadEnvFiles() actually wrote from a project .env file, as
 // opposed to values that were already present in the process environment.
@@ -65,6 +70,7 @@ async function loadConfig() {
     const loaded = await loadConfigFromPath(path)
     if (loaded) {
       globalConfig = loaded
+      globalConfigPath = path
       break
     }
   }
@@ -74,6 +80,7 @@ async function loadConfig() {
     const loaded = await loadConfigFromPath(path)
     if (loaded) {
       projectConfig = loaded
+      projectConfigPath = path
       break
     }
   }
@@ -448,6 +455,31 @@ function resolveRawOption(
   return undefined
 }
 
+/**
+ * The directory a relative path from `source` should resolve against, or
+ * undefined to use the working directory.
+ *
+ * A path written in a config file is relative to that file. Resolving it
+ * against the working directory instead would make a project-wide setting such
+ * as `pr_template = ".github/pull_request_template.md"` work at the repository
+ * root and fail in every subdirectory, even though the very same config file is
+ * the one that supplied it. Values given at invocation time -- a CLI flag or an
+ * environment variable -- stay relative to the working directory, which is what
+ * a shell user expects.
+ */
+export function optionBaseDir(source: OptionSource): string | undefined {
+  switch (source) {
+    case "project-config":
+      return projectConfigPath == null ? undefined : dirname(projectConfigPath)
+    case "global-config":
+      return globalConfigPath == null ? undefined : dirname(globalConfigPath)
+    case "cli":
+    case "env":
+    case "project-env":
+      return undefined
+  }
+}
+
 export function getOptionWithSource<T extends OptionName>(
   optionName: T,
   cliValue?: string,
@@ -497,6 +529,43 @@ export function resolveIssueSort(cliValue?: string): IssueSort {
   return parsed.output
 }
 
+/**
+ * Resolve the pull request template path from `--template`, LINEAR_PR_TEMPLATE,
+ * or the `pr_template` config option, with `false` meaning `--no-template`.
+ *
+ * Follows resolveIssueSort() rather than getOption(): getOption() silently
+ * returns undefined for a value that fails to parse, which would create a pull
+ * request quietly missing the template the user configured. An explicitly
+ * configured value must work or error.
+ *
+ * A path from a config file is resolved against that file's directory; see
+ * optionBaseDir().
+ */
+export function resolvePrTemplate(
+  cliValue?: string | false,
+): string | undefined {
+  if (cliValue === false) return undefined
+  const resolved = resolveRawOption("pr_template", cliValue)
+  if (resolved == null || resolved.raw == null) return undefined
+  const parsed = v.safeParse(
+    v.pipe(v.string(), v.trim(), v.nonEmpty()),
+    resolved.raw,
+  )
+  if (!parsed.success) {
+    throw new ValidationError(
+      `Invalid pull request template: ${JSON.stringify(resolved.raw)}`,
+      {
+        suggestion:
+          "Set a non-empty file path via --template, the pr_template config option, or LINEAR_PR_TEMPLATE; use --no-template to skip the template.",
+      },
+    )
+  }
+  const base = optionBaseDir(resolved.source)
+  // resolve() returns an absolute path unchanged, so an absolute value is
+  // honoured as written.
+  return base == null ? parsed.output : resolve(base, parsed.output)
+}
+
 // CLI workspace set via --workspace flag
 let cliWorkspace: string | undefined
 

test/commands/issue/__snapshots__/issue-pull-request.test.ts.snap

@@ -0,0 +1,27 @@
+export const snapshot = {};
+
+snapshot[`Issue Pull Request Command - Help Text 1`] = `
+stdout:
+"
+Usage: pull-request [issueId]
+
+Description:
+
+  Create a GitHub pull request with issue details
+
+Options:
+
+  -h, --help                - Show this help.                                                               
+  --base          <branch>  - The branch into which you want your code merged                               
+  --draft                   - Create the pull request as a draft                                            
+  -t, --title     <title>   - Optional title for the pull request (Linear issue ID will be prefixed)        
+  --web                     - Open the pull request in the browser after creating it                        
+  --head          <branch>  - The branch that contains commits for your pull request                        
+  -T, --template  <file>    - Start the pull request body from this template file (the Linear issue URL is  
+                              appended)                                                                     
+  --no-template             - Ignore the pr_template config option for this pull request                    
+
+"
+stderr:
+""
+`;