From fb0b0d9c207640d6849244dd91be0dc5bfa6a102 Mon Sep 17 00:00:00 2001 From: Peter Schilling Date: Mon, 31 Aug 2026 17:04:20 -0700 Subject: [PATCH] Follow-up to #266: make the pull request template actually reach GitHub #266 adds `-T/--template` and a `pr_template` config option, which are the right surface -- the names and the precedence are kept exactly as the contributor designed them. The mechanism cannot work, though, and I could not find a variant of it that does. The command already passes `--body `, and #266 appends `--template` next to it. `gh` refuses that pair outright: `--template` is not supported when using `--body` or `--body-file` So every use of the new flag fails, and setting `pr_template` in config breaks `issue pr` on every invocation rather than only when the flag is passed. Dropping `--body` to make room for `--template` -- the obvious repair -- is worse. `gh` only consults a template when it is running interactively; without a body a non-TTY caller gets must provide `--title` and `--body` (or `--fill` ...) when not running interactively and no pull request at all. That would trade a broken flag for a command broken in CI, scripts, and agents. Handing `gh` a temporary file that already contains the template fails the same way, because the problem is the missing `--body`, not the file's contents. So the template is read here and folded into the body we already send, with the issue URL appended after it. The URL is what Linear matches on to attach the pull request to its issue, so it has to survive; putting it last leaves the template's prose as the first thing a reviewer reads. Every existing flag keeps working, because the argv shape is unchanged. Reading the file ourselves means we own its failures, and per CLAUDE.md an explicitly requested template that cannot be used is an error rather than a silent fallback to a URL-only body -- otherwise the user gets a pull request quietly missing the content they asked for. Missing paths, directories, non-regular files and unreadable files all produce a message naming the path. NUL bytes are rejected too: `Deno.readTextFile` does not refuse binary input, it substitutes U+FFFD and keeps the NULs, which `Deno.Command` then rejects with a bare TypeError that never mentions the file. One deliberate surface change: #266's description suggests `-T ""` to override a configured default. That worked only because an empty string happened to be falsy. It is now an explicit `--no-template` flag, and `-T ""` errors with a suggestion pointing at it. The generated skill docs under skills/ are left alone; they are produced from an installed binary out of band and are already stale on trunk. --- CHANGELOG.md | 11 +- README.md | 19 +-- src/commands/issue/issue-pull-request.ts | 107 ++++++++++++++++- src/config.ts | 71 ++++++++++- .../issue-pull-request.test.ts.snap | 27 +++++ .../commands/issue/issue-pull-request.test.ts | 113 ++++++++++++++++++ test/config.test.ts | 82 +++++++++++++ 7 files changed, 406 insertions(+), 24 deletions(-) create mode 100644 test/commands/issue/__snapshots__/issue-pull-request.test.ts.snap create mode 100644 test/commands/issue/issue-pull-request.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 89b35323..d9801515 100644 --- a/CHANGELOG.md +++ b/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 ` 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 diff --git a/README.md b/README.md index 2907af55..5679c9b0 100644 --- a/README.md +++ b/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): diff --git a/src/commands/issue/issue-pull-request.ts b/src/commands/issue/issue-pull-request.ts index abeddcae..87dd2b23 100644 --- a/src/commands/issue/issue-pull-request.ts +++ b/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 { + 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,8 +111,12 @@ export const pullRequestCommand = new Command() "The branch that contains commits for your pull request", ) .option( - "-T, --template ", - "Optional template filename for the pull request body", + "-T, --template ", + "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( @@ -38,8 +124,16 @@ export const pullRequestCommand = new Command() { 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", diff --git a/src/config.ts b/src/config.ts index b2b2832b..865b49f3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,5 +1,5 @@ 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" @@ -7,6 +7,11 @@ import { ValidationError } from "./utils/errors.ts" let globalConfig: Record = {} let projectConfig: Record = {} +// 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( 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 diff --git a/test/commands/issue/__snapshots__/issue-pull-request.test.ts.snap b/test/commands/issue/__snapshots__/issue-pull-request.test.ts.snap new file mode 100644 index 00000000..99652627 --- /dev/null +++ b/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 - The branch into which you want your code merged + --draft - Create the pull request as a draft + -t, --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: +"" +`; diff --git a/test/commands/issue/issue-pull-request.test.ts b/test/commands/issue/issue-pull-request.test.ts new file mode 100644 index 00000000..6d6efa46 --- /dev/null +++ b/test/commands/issue/issue-pull-request.test.ts @@ -0,0 +1,113 @@ +import { snapshotTest } from "@cliffy/testing" +import { assertEquals, assertRejects } from "@std/assert" +import { + composePullRequestBody, + pullRequestCommand, + readPullRequestTemplate, +} from "../../../src/commands/issue/issue-pull-request.ts" +import { ValidationError } from "../../../src/utils/errors.ts" +import { commonDenoArgs } from "../../utils/test-helpers.ts" + +// The help output is the contract for the two new flags. +await snapshotTest({ + name: "Issue Pull Request Command - Help Text", + meta: import.meta, + colors: false, + args: ["--help"], + denoArgs: commonDenoArgs, + async fn() { + await pullRequestCommand.parse() + }, +}) + +// `gh pr create` rejects `--template` next to `--body`, and consults a template +// only when interactive -- so the template has to end up inside the body we +// already send. These assert the shape of that body. +Deno.test("composePullRequestBody - appends the issue URL after the template", () => { + assertEquals( + composePullRequestBody("## Summary\n\n## Testing", "https://linear.app/x"), + "## Summary\n\n## Testing\n\nhttps://linear.app/x", + ) +}) + +Deno.test("composePullRequestBody - collapses the template's trailing whitespace", () => { + // Template files almost always end in a newline; without the trim the URL + // would drift further down the body with every blank line in the file. + assertEquals( + composePullRequestBody("## Summary\n\n\n", "https://linear.app/x"), + "## Summary\n\nhttps://linear.app/x", + ) +}) + +Deno.test("composePullRequestBody - an empty template yields the URL alone", () => { + assertEquals( + composePullRequestBody(" \n", "https://linear.app/x"), + "https://linear.app/x", + ) +}) + +Deno.test("readPullRequestTemplate - reads a regular file verbatim", async () => { + const dir = await Deno.makeTempDir() + try { + const path = `${dir}/tmpl.md` + await Deno.writeTextFile(path, "## Summary\n") + assertEquals(await readPullRequestTemplate(path), "## Summary\n") + } finally { + await Deno.remove(dir, { recursive: true }) + } +}) + +// An explicitly requested template that cannot be used must fail loudly rather +// than quietly falling back to a URL-only body: the user would get a pull +// request silently missing the content they asked for. +Deno.test("readPullRequestTemplate - rejects a missing file", async () => { + const dir = await Deno.makeTempDir() + try { + await assertRejects( + () => readPullRequestTemplate(`${dir}/absent.md`), + ValidationError, + "does not exist", + ) + } finally { + await Deno.remove(dir, { recursive: true }) + } +}) + +Deno.test("readPullRequestTemplate - rejects a directory", async () => { + const dir = await Deno.makeTempDir() + try { + await assertRejects( + () => readPullRequestTemplate(dir), + ValidationError, + "is a directory, not a file", + ) + } finally { + await Deno.remove(dir, { recursive: true }) + } +}) + +Deno.test("readPullRequestTemplate - rejects an empty path", async () => { + await assertRejects( + () => readPullRequestTemplate(" "), + ValidationError, + "the path is empty", + ) +}) + +// Deno.readTextFile does not reject binary input; it substitutes U+FFFD and +// keeps NUL bytes, which Deno.Command later rejects with a bare TypeError that +// never names the file. +Deno.test("readPullRequestTemplate - rejects a file containing NUL bytes", async () => { + const dir = await Deno.makeTempDir() + try { + const path = `${dir}/binary.md` + await Deno.writeFile(path, new Uint8Array([0x23, 0x00, 0x41])) + await assertRejects( + () => readPullRequestTemplate(path), + ValidationError, + "is not a text file", + ) + } finally { + await Deno.remove(dir, { recursive: true }) + } +}) diff --git a/test/config.test.ts b/test/config.test.ts index 164aee4e..1f224d7c 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -4,6 +4,7 @@ import { getOption, getOptionWithSource, resolveIssueSort, + resolvePrTemplate, } from "../src/config.ts" import { ValidationError } from "../src/utils/errors.ts" @@ -1109,6 +1110,87 @@ Deno.test("getOptionWithSource - LINEAR_IGNORE_ENV_FILE skips .env loading entir } }) +// getOption() silently drops a value that fails to parse, which would create a +// pull request quietly missing the configured template. resolvePrTemplate must +// error instead -- explicit input works or errors, it never falls back. +Deno.test("resolvePrTemplate - rejects an empty explicit value", () => { + assertThrows( + () => resolvePrTemplate(""), + ValidationError, + "Invalid pull request template", + ) +}) + +Deno.test("resolvePrTemplate - false means --no-template and yields no path", () => { + assertEquals(resolvePrTemplate(false), undefined) +}) + +Deno.test("resolvePrTemplate - an explicit path is left relative to the cwd", () => { + // Only config-file values are rebased; a path typed on the command line means + // what it means in the shell the user typed it in. + assertEquals(resolvePrTemplate("docs/pr.md"), "docs/pr.md") +}) + +// A path written in a config file is relative to that file. The config loader +// finds <repo-root>/.linear.toml from any subdirectory, so resolving its value +// against the working directory instead would make a project-wide setting work +// at the repo root and fail everywhere below it. +Deno.test("optionBaseDir - a project config value resolves against the config file, not the cwd", async () => { + const repoDir = await Deno.makeTempDir() + const home = await Deno.makeTempDir() + try { + await Deno.writeTextFile( + `${repoDir}/.linear.toml`, + 'pr_template = ".github/pull_request_template.md"\n', + ) + const nested = `${repoDir}/packages/app` + await Deno.mkdir(nested, { recursive: true }) + // The repo-root config paths are only searched inside a git work tree. + await initGitRepo(repoDir) + + const configUrl = new URL("../src/config.ts", import.meta.url) + const denoJsonPath = fromFileUrl(new URL("../deno.json", import.meta.url)) + const homeDir = Deno.env.get("HOME") + const denoDir = Deno.env.get("DENO_DIR") ?? + (homeDir == null ? undefined : `${homeDir}/.cache/deno`) + const command = new Deno.Command(Deno.execPath(), { + args: [ + "eval", + `--config=${denoJsonPath}`, + `import { getOptionWithSource, optionBaseDir } from "${configUrl}"; + const r = getOptionWithSource("pr_template"); + console.log(JSON.stringify({ source: r?.source ?? null, base: optionBaseDir(r.source) ?? null }));`, + ], + cwd: nested, + clearEnv: true, + env: { + HOME: home, + XDG_CONFIG_HOME: `${home}/.config`, + PATH: Deno.env.get("PATH") ?? "", + NO_COLOR: "1", + ...(denoDir == null ? {} : { DENO_DIR: denoDir }), + }, + stdout: "piped", + stderr: "piped", + }) + const { stdout, stderr } = await command.output() + const out = new TextDecoder().decode(stdout).trim() + if (out === "") { + throw new Error( + `subprocess produced no output: ${new TextDecoder().decode(stderr)}`, + ) + } + const result = JSON.parse(out) + + assertEquals(result.source, "project-config") + // The base is the directory holding the config file, not the nested cwd. + assertEquals(result.base, repoDir) + } finally { + await Deno.remove(repoDir, { recursive: true }) + await Deno.remove(home, { recursive: true }) + } +}) + Deno.test("getOptionWithSource - invalid project value shadows valid global value", async () => { // A present-but-invalid higher-precedence value must block fallback to a // lower-precedence source, matching the pre-split spread-merge behavior.