From 8d1947bd3f50f785d0e6dba0280c1f223d0cb625 Mon Sep 17 00:00:00 2001 From: Peter Schilling Date: Wed, 12 Aug 2026 13:44:41 -0700 Subject: [PATCH] Suppress default team note when the team is project configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit issue query prints "Note: using default team ..." whenever the team scope falls back to the configured default. The note exists to flag ambient defaults (a global config file or a shell-exported LINEAR_TEAM_ID) silently narrowing a query, but it also fired when the team came from the project's own linear.toml or .env — explicit, directory-scoped configuration where the reminder is just noise on every query. Track where each option was resolved from (cli, env, project-env, project-config, global-config) by keeping global and project config separate and recording which env keys were applied from a project .env. issue query now consults the source and only prints the note for ambient sources. getOption keeps its interface, delegating to the new getOptionWithSource, which also removes its type casts. --- src/commands/issue/issue-query.ts | 34 ++++-- src/config.ts | 114 ++++++++++++++----- src/utils/linear.ts | 22 +++- test/commands/issue/issue-query.test.ts | 119 +++++++++++++++++++- test/config.test.ts | 140 +++++++++++++++++++++++- 5 files changed, 386 insertions(+), 43 deletions(-) diff --git a/src/commands/issue/issue-query.ts b/src/commands/issue/issue-query.ts index 69da30d9..63cb9a8c 100644 --- a/src/commands/issue/issue-query.ts +++ b/src/commands/issue/issue-query.ts @@ -1,7 +1,7 @@ import { Command, EnumType } from "@cliffy/command" import { unicodeWidth } from "@std/cli" import { rgb24 } from "@std/fmt/colors" -import { resolveIssueSort } from "../../config.ts" +import { type OptionSource, resolveIssueSort } from "../../config.ts" import { colorCycleShort, type CycleDisplayInfo, @@ -17,7 +17,7 @@ import { getProjectIdByName, getProjectOptionsByName, getTeamIdByKey, - getTeamKey, + getTeamKeyWithSource, isIssueBlocked, isLinearUuid, resolveMilestoneId, @@ -242,7 +242,7 @@ export const queryCommand = new Command() resolvedTeamKeys = teamKeys isMultiTeam = teamKeys.length > 1 } else { - const defaultTeam = getTeamKey() + const defaultTeam = getTeamKeyWithSource() if (!defaultTeam) { throw new ValidationError( "No default team configured and no team scope provided", @@ -252,10 +252,12 @@ export const queryCommand = new Command() }, ) } - console.error( - `Note: using default team ${defaultTeam}. Pass --team or --all-teams to be explicit.`, - ) - resolvedTeamKeys = [defaultTeam] + if (shouldShowDefaultTeamNote(defaultTeam.source)) { + console.error( + `Note: using default team ${defaultTeam.key}. Pass --team or --all-teams to be explicit.`, + ) + } + resolvedTeamKeys = [defaultTeam.key] } // --- Resolve entity IDs --- @@ -407,6 +409,24 @@ export const queryCommand = new Command() } }) +/** + * The default-team note warns that an ambient default (global config file or + * shell-exported env var) silently scoped the query. A team configured by the + * project itself — a linear.toml in the directory/repo, or a project .env — + * is explicit local intent, so the note would be noise. + */ +export function shouldShowDefaultTeamNote(source: OptionSource): boolean { + switch (source) { + case "cli": + case "project-env": + case "project-config": + return false + case "env": + case "global-config": + return true + } +} + async function outputPaged( outputLines: string[], usePager: boolean, diff --git a/src/config.ts b/src/config.ts index d3cb09f1..1abfaa1d 100644 --- a/src/config.ts +++ b/src/config.ts @@ -4,7 +4,12 @@ import { load } from "@std/dotenv" import * as v from "valibot" import { ValidationError } from "./utils/errors.ts" -let config: Record = {} +let globalConfig: Record = {} +let projectConfig: Record = {} + +// Env keys that loadEnvFiles() actually wrote from a project .env file, as +// opposed to values that were already present in the process environment. +const dotenvAppliedKeys = new Set() async function loadConfigFromPath( path: string, @@ -56,18 +61,18 @@ async function loadConfig() { // Load global config first (lowest priority) for (const path of globalConfigPaths) { - const globalConfig = await loadConfigFromPath(path) - if (globalConfig) { - config = globalConfig + const loaded = await loadConfigFromPath(path) + if (loaded) { + globalConfig = loaded break } } - // Load project config and merge on top (project overrides global) + // Load project config (higher priority; shadows global per option) for (const path of projectConfigPaths) { - const projectConfig = await loadConfigFromPath(path) - if (projectConfig) { - config = { ...config, ...projectConfig } + const loaded = await loadConfigFromPath(path) + if (loaded) { + projectConfig = loaded break } } @@ -106,6 +111,7 @@ async function loadEnvFiles() { // Use same precedence as dotenv if (Deno.env.get(key) !== undefined) continue Deno.env.set(key, value) + dotenvAppliedKeys.add(key) } } } @@ -136,8 +142,9 @@ export const ISSUE_SORT_VALUES = ["manual", "priority"] as const export type IssueSort = (typeof ISSUE_SORT_VALUES)[number] export const DEFAULT_ISSUE_SORT: IssueSort = "priority" -// Options schema -const OptionsSchema = v.object({ +// Per-option schemas, indexable by option name so parsed values keep their +// option-specific output types without casts. +const OptionSchemas = { team_id: v.optional(v.string()), api_key: v.optional(v.string()), workspace: v.optional(v.string()), @@ -149,27 +156,78 @@ const OptionsSchema = v.object({ hyperlink_format: v.optional(v.string()), attachment_dir: v.optional(v.string()), auto_download_attachments: v.optional(BooleanLike), -}) +} + +export type OptionName = keyof typeof OptionSchemas +type OptionValue = v.InferOutput< + (typeof OptionSchemas)[T] +> +export type Options = { [K in OptionName]: OptionValue } -export type Options = v.InferOutput -export type OptionName = keyof Options +/** Where a resolved option value came from. */ +export type OptionSource = + | "cli" + | "env" // pre-existing process environment variable + | "project-env" // LINEAR_* applied from a project .env file + | "project-config" // linear.toml / .linear.toml in cwd or git root + | "global-config" // XDG / ~/.config / APPDATA linear.toml -function getRawOption(optionName: OptionName, cliValue?: string): unknown { - return cliValue ?? - Deno.env.get("LINEAR_" + optionName.toUpperCase()) ?? - config[optionName] +export interface ResolvedOption { + value: T + source: OptionSource } -export function getOption( +function resolveRawOption( + optionName: OptionName, + cliValue?: string, +): { raw: unknown; source: OptionSource } | undefined { + if (cliValue != null) { + return { raw: cliValue, source: "cli" } + } + const envKey = "LINEAR_" + optionName.toUpperCase() + const envValue = Deno.env.get(envKey) + if (envValue != null) { + return { + raw: envValue, + source: dotenvAppliedKeys.has(envKey) ? "project-env" : "env", + } + } + // Check key presence rather than value nullishness so a present-but-invalid + // higher-precedence value still shadows lower-precedence values, matching + // the previous spread-merge behavior. + if (Object.hasOwn(projectConfig, optionName)) { + return { raw: projectConfig[optionName], source: "project-config" } + } + if (Object.hasOwn(globalConfig, optionName)) { + return { raw: globalConfig[optionName], source: "global-config" } + } + return undefined +} + +export function getOptionWithSource( optionName: T, cliValue?: string, -): Options[T] { - const raw = getRawOption(optionName, cliValue) - const result = v.safeParse(OptionsSchema, { [optionName]: raw }) - if (result.success) { - return result.output[optionName] as Options[T] +): ResolvedOption>> | undefined { + const resolved = resolveRawOption(optionName, cliValue) + if (resolved == null) { + return undefined + } + const parsed = v.safeParse(OptionSchemas[optionName], resolved.raw) + if (!parsed.success) { + return undefined + } + const value = parsed.output + if (value == null) { + return undefined } - return undefined as Options[T] + return { value, source: resolved.source } +} + +export function getOption( + optionName: T, + cliValue?: string, +): OptionValue | undefined { + return getOptionWithSource(optionName, cliValue)?.value } /** @@ -179,12 +237,12 @@ export function getOption( * silently falling back to the default. */ export function resolveIssueSort(cliValue?: string): IssueSort { - const raw = getRawOption("issue_sort", cliValue) - if (raw == null) return DEFAULT_ISSUE_SORT - const parsed = v.safeParse(v.picklist(ISSUE_SORT_VALUES), raw) + const resolved = resolveRawOption("issue_sort", cliValue) + if (resolved == null || resolved.raw == null) return DEFAULT_ISSUE_SORT + const parsed = v.safeParse(v.picklist(ISSUE_SORT_VALUES), resolved.raw) if (!parsed.success) { throw new ValidationError( - `Invalid issue sort: ${JSON.stringify(raw)}`, + `Invalid issue sort: ${JSON.stringify(resolved.raw)}`, { suggestion: `Use one of: ${ ISSUE_SORT_VALUES.join(", ") diff --git a/src/utils/linear.ts b/src/utils/linear.ts index 6d376b67..83141d04 100644 --- a/src/utils/linear.ts +++ b/src/utils/linear.ts @@ -16,7 +16,11 @@ import type { SearchIssuesQuery, } from "../__codegen__/graphql.ts" import { Select } from "@cliffy/prompt" -import { getOption, resolveIssueSort } from "../config.ts" +import { + getOptionWithSource, + type OptionSource, + resolveIssueSort, +} from "../config.ts" import { CliError, NotFoundError, ValidationError } from "./errors.ts" import { getGraphQLClient } from "./graphql.ts" import { normalizeIssueIdentifier } from "./issue-identifier.ts" @@ -74,12 +78,18 @@ export function formatIssueIdentifier(providedId: string): string { return normalizeIssueIdentifier(providedId) ?? providedId.toUpperCase() } -export function getTeamKey(): string | undefined { - const teamId = getOption("team_id") - if (teamId) { - return teamId.toUpperCase() +export function getTeamKeyWithSource(): + | { key: string; source: OptionSource } + | undefined { + const resolved = getOptionWithSource("team_id") + if (resolved == null || resolved.value === "") { + return undefined } - return undefined + return { key: resolved.value.toUpperCase(), source: resolved.source } +} + +export function getTeamKey(): string | undefined { + return getTeamKeyWithSource()?.key } /** diff --git a/test/commands/issue/issue-query.test.ts b/test/commands/issue/issue-query.test.ts index 73f5b530..47af1ff2 100644 --- a/test/commands/issue/issue-query.test.ts +++ b/test/commands/issue/issue-query.test.ts @@ -2,7 +2,11 @@ import { snapshotTest } from "@cliffy/testing" import { assertEquals } from "@std/assert" import { getColorEnabled, setColorEnabled } from "@std/fmt/colors" import { stub } from "@std/testing/mock" -import { queryCommand } from "../../../src/commands/issue/issue-query.ts" +import { + queryCommand, + shouldShowDefaultTeamNote, +} from "../../../src/commands/issue/issue-query.ts" +import type { OptionSource } from "../../../src/config.ts" import { commonDenoArgs, setupMockLinearServer, @@ -592,3 +596,116 @@ Deno.test("Issue Query Command - Hides Cycle Column When Cycles Disabled", async await cleanup() } }) + +// --- default-team note policy --- + +const emptyIssuesResponse = { + queryName: "GetIssuesForQuery", + response: { + data: { + issues: { + nodes: [], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }, +} + +Deno.test("Issue Query Command - shows note when default team comes from env var", async () => { + // Set LINEAR_TEAM_ID manually (not via setupMockLinearServer) so this test + // owns restoring any pre-existing value; the helper's cleanup only deletes. + const priorTeamId = Deno.env.get("LINEAR_TEAM_ID") + const { cleanup } = await setupMockLinearServer([emptyIssuesResponse], { + NO_COLOR: "true", + }) + Deno.env.set("LINEAR_TEAM_ID", "ENG") + + const errorLogs: string[] = [] + const errorStub = stub(console, "error", (...args: unknown[]) => { + errorLogs.push(args.map(String).join(" ")) + }) + const logStub = stub(console, "log", () => {}) + + try { + await queryCommand.parse(["--no-pager"]) + } finally { + errorStub.restore() + logStub.restore() + if (priorTeamId == null) { + Deno.env.delete("LINEAR_TEAM_ID") + } else { + Deno.env.set("LINEAR_TEAM_ID", priorTeamId) + } + await cleanup() + } + + assertEquals( + errorLogs.some((l) => + l.includes( + "Note: using default team ENG. Pass --team or --all-teams to be explicit.", + ) + ), + true, + ) +}) + +Deno.test("Issue Query Command - suppresses note when default team comes from project config", async () => { + // With LINEAR_TEAM_ID absent, the default team falls through to the repo's + // own root .linear.toml (loaded at module init), i.e. a project-config + // source. This test intentionally depends on that file setting team_id. + const priorTeamId = Deno.env.get("LINEAR_TEAM_ID") + const { cleanup } = await setupMockLinearServer([emptyIssuesResponse], { + NO_COLOR: "true", + }) + Deno.env.delete("LINEAR_TEAM_ID") + + const errorLogs: string[] = [] + const errorStub = stub(console, "error", (...args: unknown[]) => { + errorLogs.push(args.map(String).join(" ")) + }) + const logStub = stub(console, "log", () => {}) + + try { + await queryCommand.parse(["--no-pager"]) + } finally { + errorStub.restore() + logStub.restore() + if (priorTeamId != null) { + Deno.env.set("LINEAR_TEAM_ID", priorTeamId) + } + await cleanup() + } + + assertEquals( + errorLogs.some((l) => l.includes("using default team")), + false, + `unexpected note in stderr: ${errorLogs.join("\n")}`, + ) +}) + +Deno.test("shouldShowDefaultTeamNote - full source matrix", () => { + // Record makes this table exhaustive at the type + // level: adding a new OptionSource member without an expectation here is a + // compile error. + const expectations: Record = { + "cli": false, + "project-env": false, + "project-config": false, + "env": true, + "global-config": true, + } + const sources: OptionSource[] = [ + "cli", + "project-env", + "project-config", + "env", + "global-config", + ] + for (const source of sources) { + assertEquals( + shouldShowDefaultTeamNote(source), + expectations[source], + source, + ) + } +}) diff --git a/test/config.test.ts b/test/config.test.ts index 1e6fc33b..eb9084a1 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -1,6 +1,10 @@ import { assertEquals, assertThrows } from "@std/assert" import { fromFileUrl } from "@std/path" -import { getOption, resolveIssueSort } from "../src/config.ts" +import { + getOption, + getOptionWithSource, + resolveIssueSort, +} from "../src/config.ts" import { ValidationError } from "../src/utils/errors.ts" // Note: These tests use the cliValue parameter (highest precedence) @@ -723,3 +727,137 @@ Deno.test("resolveIssueSort - defaults to priority when nothing is configured", await Deno.remove(tempDir, { recursive: true }) } }) + +// --- getOptionWithSource provenance --- +// These subprocesses use clearEnv so a developer's or CI's LINEAR_TEAM_ID +// cannot leak in; module-init config loading depends precisely on the +// environment at startup. On Windows, APPDATA is pointed at the same +// directory as XDG_CONFIG_HOME so one global config file covers both +// platforms' lookup paths. + +async function runTeamSourceSubprocess(options: { + cwd: string + home: string + extraEnv?: Record +}): Promise { + 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 } from "${configUrl}"; console.log(JSON.stringify(getOptionWithSource("team_id") ?? null));`, + ], + cwd: options.cwd, + clearEnv: true, + env: { + HOME: options.home, + XDG_CONFIG_HOME: `${options.home}/.config`, + PATH: Deno.env.get("PATH") ?? "", + ...(denoDir == null ? {} : { DENO_DIR: denoDir }), + ...(Deno.build.os === "windows" + ? { + SystemRoot: Deno.env.get("SystemRoot") ?? "", + APPDATA: `${options.home}/.config`, + } + : {}), + ...options.extraEnv, + }, + stdout: "piped", + stderr: "piped", + }) + const { stdout, stderr } = await command.output() + const errorOutput = new TextDecoder().decode(stderr) + if (errorOutput) { + console.error("Subprocess stderr:", errorOutput) + } + return JSON.parse(new TextDecoder().decode(stdout).trim()) +} + +Deno.test("getOptionWithSource - project config file yields project-config source", async () => { + const projectDir = await Deno.makeTempDir() + const home = await Deno.makeTempDir() + try { + await Deno.writeTextFile(`${projectDir}/.linear.toml`, 'team_id = "ENG"\n') + const result = await runTeamSourceSubprocess({ cwd: projectDir, home }) + assertEquals(result, { value: "ENG", source: "project-config" }) + } finally { + await Deno.remove(projectDir, { recursive: true }) + await Deno.remove(home, { recursive: true }) + } +}) + +Deno.test("getOptionWithSource - global config file yields global-config source", async () => { + const workDir = await Deno.makeTempDir() + const home = await Deno.makeTempDir() + try { + await Deno.mkdir(`${home}/.config/linear`, { recursive: true }) + await Deno.writeTextFile( + `${home}/.config/linear/linear.toml`, + 'team_id = "ENG"\n', + ) + const result = await runTeamSourceSubprocess({ cwd: workDir, home }) + assertEquals(result, { value: "ENG", source: "global-config" }) + } finally { + await Deno.remove(workDir, { recursive: true }) + await Deno.remove(home, { recursive: true }) + } +}) + +Deno.test("getOptionWithSource - project .env yields project-env source", async () => { + const projectDir = await Deno.makeTempDir() + const home = await Deno.makeTempDir() + try { + await Deno.writeTextFile(`${projectDir}/.env`, "LINEAR_TEAM_ID=ENG\n") + const result = await runTeamSourceSubprocess({ cwd: projectDir, home }) + assertEquals(result, { value: "ENG", source: "project-env" }) + } finally { + await Deno.remove(projectDir, { recursive: true }) + await Deno.remove(home, { recursive: true }) + } +}) + +Deno.test("getOptionWithSource - process env wins over project .env and is classified env", async () => { + const projectDir = await Deno.makeTempDir() + const home = await Deno.makeTempDir() + try { + await Deno.writeTextFile(`${projectDir}/.env`, "LINEAR_TEAM_ID=ENG\n") + const result = await runTeamSourceSubprocess({ + cwd: projectDir, + home, + extraEnv: { LINEAR_TEAM_ID: "OPS" }, + }) + assertEquals(result, { value: "OPS", source: "env" }) + } finally { + await Deno.remove(projectDir, { 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. + const projectDir = await Deno.makeTempDir() + const home = await Deno.makeTempDir() + try { + await Deno.mkdir(`${home}/.config/linear`, { recursive: true }) + await Deno.writeTextFile( + `${home}/.config/linear/linear.toml`, + 'team_id = "ENG"\n', + ) + await Deno.writeTextFile(`${projectDir}/.linear.toml`, "team_id = 5\n") + const result = await runTeamSourceSubprocess({ cwd: projectDir, home }) + assertEquals(result, null) + } finally { + await Deno.remove(projectDir, { recursive: true }) + await Deno.remove(home, { recursive: true }) + } +}) + +Deno.test("getOptionWithSource - cli value yields cli source", () => { + const result = getOptionWithSource("team_id", "eng") + assertEquals(result, { value: "eng", source: "cli" }) +})