From bd8f92c0953fc5a5c77a34f1084b65389d24c328 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 5 Mar 2026 13:26:36 +0000 Subject: [PATCH] fix(logger): inject --verbose and --log-level as proper Stricli flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix three bugs in the logging system introduced in #338: 1. `--verbose` rejected by all commands except `api` — Stricli throws "No flag registered for --verbose" because extractLogLevelFromArgs intentionally left it in argv for the api command. 2. `--log-level=debug` (equals form) not handled — argv.indexOf only matched the space-separated form, passing the flag through to Stricli. 3. `SENTRY_LOG_LEVEL` env var has no visible effect — consola withTag() creates independent instances that snapshot the level at creation time. Module-level scoped loggers never saw later setLogLevel() calls. Instead of pre-parsing flags from argv (bypassing Stricli), define them as proper hidden Stricli flags. buildCommand in src/lib/command.ts now wraps Stricli to inject hidden --log-level (enum) and --verbose (boolean) flags into every command, intercept them, apply setLogLevel(), then strip them before calling the original func. When a command already defines its own --verbose (e.g. api uses it for HTTP output), the injected one is skipped — the command own value still triggers debug-level logging as a side-effect. setLogLevel() now propagates to all withTag() children via a registry, fixing the env var and flag having no effect on scoped loggers. Document SENTRY_LOG_LEVEL, --log-level, and --verbose in configuration docs. --- AGENTS.md | 2 +- biome.jsonc | 28 ++ docs/src/content/docs/commands/index.md | 2 + docs/src/content/docs/configuration.md | 39 ++ plugins/sentry-cli/skills/sentry-cli/SKILL.md | 9 + script/generate-skill.ts | 28 +- src/bin.ts | 20 +- src/commands/help.ts | 3 +- src/lib/command.ts | 177 +++++++-- src/lib/logger.ts | 99 ++--- test/lib/command.test.ts | 351 +++++++++++++++++- test/lib/logger.property.test.ts | 108 +----- test/lib/logger.test.ts | 123 ++---- 13 files changed, 711 insertions(+), 278 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f9e9a5d70..df8126803 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -686,7 +686,7 @@ mock.module("./some-module", () => ({ * **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: List commands with cursor pagination use \`buildPaginationContextKey(type, identifier, flags)\` for composite context keys and \`parseCursorFlag(value)\` accepting \`"last"\` magic value. Critical: \`resolveCursor()\` must be called inside the \`org-all\` override closure, not before \`dispatchOrgScopedList\` — otherwise cursor validation errors fire before the correct mode-specific error. -* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` so it's a no-op without active transaction. When returning \`withTracingSpan(...)\` directly, drop \`async\` and use \`Promise.resolve(null)\` for early returns. User-visible fallbacks should use \`log.warn()\` not \`log.debug()\` — debug is invisible at default level. Also: several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\`. Affected: trace/list, trace/view, log/view, api.ts, help.ts. +* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` so it's a no-op without active transaction. When returning \`withTracingSpan(...)\` directly, drop \`async\` and use \`Promise.resolve(null)\` for early returns. User-visible fallbacks should use \`log.warn()\` not \`log.debug()\` — debug is invisible at default level. ### Preference diff --git a/biome.jsonc b/biome.jsonc index a010d3024..7f55cf5ac 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -7,6 +7,23 @@ "javascript": { "globals": ["Bun"] }, + "linter": { + "rules": { + "style": { + "noRestrictedImports": { + "level": "error", + "options": { + "paths": { + "@stricli/core": { + "importNames": ["buildCommand"], + "message": "Import buildCommand from '../lib/command.js' instead. The wrapper injects telemetry, --log-level, and --verbose." + } + } + } + } + } + } + }, "overrides": [ { "includes": ["test/**/*.ts"], @@ -47,6 +64,17 @@ } } } + }, + { + // command.ts is the canonical wrapper — it must import buildCommand from @stricli/core + "includes": ["src/lib/command.ts"], + "linter": { + "rules": { + "style": { + "noRestrictedImports": "off" + } + } + } } ] } diff --git a/docs/src/content/docs/commands/index.md b/docs/src/content/docs/commands/index.md index 9decfeb0d..20452e259 100644 --- a/docs/src/content/docs/commands/index.md +++ b/docs/src/content/docs/commands/index.md @@ -25,6 +25,8 @@ All commands support the following global options: - `--help` - Show help for the command - `--version` - Show CLI version +- `--log-level ` - Set log verbosity (`error`, `warn`, `log`, `info`, `debug`, `trace`). Overrides `SENTRY_LOG_LEVEL` +- `--verbose` - Shorthand for `--log-level debug` ## JSON Output diff --git a/docs/src/content/docs/configuration.md b/docs/src/content/docs/configuration.md index 9a367877b..7e0efec24 100644 --- a/docs/src/content/docs/configuration.md +++ b/docs/src/content/docs/configuration.md @@ -89,6 +89,18 @@ Disable CLI telemetry (error tracking for the CLI itself). The CLI sends anonymi export SENTRY_CLI_NO_TELEMETRY=1 ``` +### `SENTRY_LOG_LEVEL` + +Controls the verbosity of diagnostic output. Defaults to `info`. + +Valid values: `error`, `warn`, `log`, `info`, `debug`, `trace` + +```bash +export SENTRY_LOG_LEVEL=debug +``` + +Equivalent to passing `--log-level debug` on the command line. CLI flags take precedence over the environment variable. + ### `SENTRY_CLI_NO_UPDATE_CHECK` Disable the automatic update check that runs periodically in the background. @@ -97,6 +109,33 @@ Disable the automatic update check that runs periodically in the background. export SENTRY_CLI_NO_UPDATE_CHECK=1 ``` +## Global Options + +These flags are accepted by every command. They are not shown in individual command `--help` output, but are always available. + +### `--log-level ` + +Set the log verbosity level. Accepts: `error`, `warn`, `log`, `info`, `debug`, `trace`. + +```bash +sentry issue list --log-level debug +sentry --log-level=trace cli upgrade +``` + +Overrides `SENTRY_LOG_LEVEL` when both are set. + +### `--verbose` + +Shorthand for `--log-level debug`. Enables debug-level diagnostic output. + +```bash +sentry issue list --verbose +``` + +:::note +The `sentry api` command also uses `--verbose` to show full HTTP request/response details. When used with `sentry api`, it serves both purposes (debug logging + HTTP output). +::: + ## Credential Storage Credentials are stored in a SQLite database at `~/.sentry/` (or the path set by `SENTRY_CONFIG_DIR`) with restricted file permissions (mode 600) for security. The database also caches: diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 1ced85af1..ad61a784d 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -721,6 +721,15 @@ Show the currently authenticated user **Flags:** - `--json - Output as JSON` +## Global Options + +All commands support the following global options: + +- `--help` - Show help for the command +- `--version` - Show CLI version +- `--log-level ` - Set log verbosity (`error`, `warn`, `log`, `info`, `debug`, `trace`). Overrides `SENTRY_LOG_LEVEL` +- `--verbose` - Shorthand for `--log-level debug` + ## Output Formats ### JSON Output diff --git a/script/generate-skill.ts b/script/generate-skill.ts index 41707f718..6d248d739 100644 --- a/script/generate-skill.ts +++ b/script/generate-skill.ts @@ -75,6 +75,7 @@ type FlagDef = { optional?: boolean; variadic?: boolean; placeholder?: string; + hidden?: boolean; }; // ───────────────────────────────────────────────────────────────────────────── @@ -376,6 +377,7 @@ async function loadCommandExamples( * Load supplementary content from commands/index.md */ async function loadCommandsOverview(): Promise<{ + globalOptions: string; jsonOutput: string; webFlag: string; } | null> { @@ -385,10 +387,12 @@ async function loadCommandsOverview(): Promise<{ return null; } + const globalSection = extractSection(content, "Global Options"); const jsonSection = extractSection(content, "JSON Output"); const webSection = extractSection(content, "Opening in Browser"); return { + globalOptions: globalSection || "", jsonOutput: jsonSection || "", webFlag: webSection || "", }; @@ -423,6 +427,7 @@ type FlagInfo = { default?: unknown; optional: boolean; variadic: boolean; + hidden: boolean; }; type RouteInfo = { @@ -468,6 +473,7 @@ function extractFlags(flags: Record | undefined): FlagInfo[] { default: def.default, optional: def.optional ?? def.kind === "boolean", variadic: def.variadic ?? false, + hidden: def.hidden ?? false, })); } @@ -616,9 +622,9 @@ function generateCommandDoc(cmd: CommandInfo): string { lines.push(""); lines.push(cmd.brief); - // Flags section + // Flags section — exclude help flags and hidden global flags (e.g. --log-level, --verbose) const visibleFlags = cmd.flags.filter( - (f) => f.name !== "help" && f.name !== "helpAll" + (f) => !f.hidden && f.name !== "help" && f.name !== "helpAll" ); if (visibleFlags.length > 0) { @@ -707,12 +713,22 @@ function generateCommandsSection(routeInfos: RouteInfo[]): string { } /** - * Generate the Output Formats section from docs + * Generate supplementary sections (Global Options, Output Formats) from docs */ -async function generateOutputFormatsSection(): Promise { +async function generateSupplementarySections(): Promise { const overview = await loadCommandsOverview(); const lines: string[] = []; + + // Global Options section + if (overview?.globalOptions) { + lines.push("## Global Options"); + lines.push(""); + lines.push(overview.globalOptions); + lines.push(""); + } + + // Output Formats section lines.push("## Output Formats"); lines.push(""); @@ -747,7 +763,7 @@ async function generateOutputFormatsSection(): Promise { async function generateSkillMarkdown(routeMap: RouteMap): Promise { const routeInfos = await extractRoutes(routeMap); const prerequisites = await loadPrerequisites(); - const outputFormats = await generateOutputFormatsSection(); + const supplementary = await generateSupplementarySections(); const sections = [ generateFrontMatter(), @@ -759,7 +775,7 @@ async function generateSkillMarkdown(routeMap: RouteMap): Promise { prerequisites, "", generateCommandsSection(routeInfos), - outputFormats, + supplementary, "", ]; diff --git a/src/bin.ts b/src/bin.ts index c29bb3786..5766d2888 100755 --- a/src/bin.ts +++ b/src/bin.ts @@ -5,11 +5,7 @@ import { buildContext } from "./context.js"; import { AuthError, formatError, getExitCode } from "./lib/errors.js"; import { error } from "./lib/formatters/colors.js"; import { runInteractiveLogin } from "./lib/interactive-login.js"; -import { - extractLogLevelFromArgs, - getEnvLogLevel, - setLogLevel, -} from "./lib/logger.js"; +import { getEnvLogLevel, setLogLevel } from "./lib/logger.js"; import { withTelemetry } from "./lib/telemetry.js"; import { startCleanupOldBinary } from "./lib/upgrade.js"; import { @@ -94,22 +90,14 @@ async function main(): Promise { const args = process.argv.slice(2); - // Apply SENTRY_LOG_LEVEL env var first (lazy read, not at module load time). - // CLI flags below override this if present. + // Apply SENTRY_LOG_LEVEL env var early (lazy read, not at module load time). + // CLI flags (--log-level, --verbose) are handled by Stricli via + // buildCommand and take priority when present. const envLogLevel = getEnvLogLevel(); if (envLogLevel !== null) { setLogLevel(envLogLevel); } - // Extract global log-level flags before Stricli parses args. - // --log-level is consumed (removed); --verbose is read but left in place - // because some commands (e.g., `api`) define their own --verbose flag. - // CLI flags take priority over SENTRY_LOG_LEVEL env var. - const logLevel = extractLogLevelFromArgs(args); - if (logLevel !== null) { - setLogLevel(logLevel); - } - const suppressNotification = shouldSuppressNotification(args); // Start background update check (non-blocking) diff --git a/src/commands/help.ts b/src/commands/help.ts index 5ec8dc094..7fce649ec 100644 --- a/src/commands/help.ts +++ b/src/commands/help.ts @@ -6,8 +6,9 @@ * - `sentry help `: Shows Stricli's detailed help (--helpAll) for that command */ -import { buildCommand, run } from "@stricli/core"; +import { run } from "@stricli/core"; import type { SentryContext } from "../context.js"; +import { buildCommand } from "../lib/command.js"; import { printCustomHelp } from "../lib/help.js"; export const helpCommand = buildCommand({ diff --git a/src/lib/command.ts b/src/lib/command.ts index bf7f6169c..4878d34e6 100644 --- a/src/lib/command.ts +++ b/src/lib/command.ts @@ -1,16 +1,27 @@ /** - * Command Builder with Telemetry + * Command builder with telemetry and global logging flag injection. * - * Wraps Stricli's buildCommand to automatically capture flag usage for telemetry. + * Provides `buildCommand` — the standard command builder for all Sentry CLI + * commands. It wraps Stricli's `buildCommand` with: * - * ALL commands MUST import `buildCommand` from this module, NOT from `@stricli/core`. - * Importing directly from `@stricli/core` silently bypasses flag/arg telemetry capture. + * 1. **Automatic flag/arg telemetry** — captures flag values and positional + * arguments as Sentry span context for observability. * - * Correct: import { buildCommand } from "../../lib/command.js"; - * Incorrect: import { buildCommand } from "@stricli/core"; // skips telemetry! + * 2. **Hidden global logging flags** — injects `--log-level` and `--verbose` + * into every command's parameters. These are intercepted before the original + * `func` runs: the logger level is set, and the injected flags are stripped + * so the original function never sees them. If a command already defines its + * own `--verbose` flag (e.g. `api` uses it for HTTP output), the injected + * one is skipped and the command's own value is used for both purposes. + * + * ALL commands MUST use `buildCommand` from this module, NOT from + * `@stricli/core`. Importing directly from Stricli silently bypasses + * telemetry and global flag handling. * - * Exception: `help.ts` may import from `@stricli/core` because it also needs `run`, - * and the help command has no meaningful flags to capture. + * ``` + * Correct: import { buildCommand } from "../../lib/command.js"; + * Incorrect: import { buildCommand } from "@stricli/core"; // skips everything! + * ``` */ import { @@ -20,6 +31,12 @@ import { buildCommand as stricliCommand, numberParser as stricliNumberParser, } from "@stricli/core"; +import { + LOG_LEVEL_NAMES, + type LogLevelName, + parseLogLevel, + setLogLevel, +} from "./logger.js"; import { setArgsContext, setFlagContext } from "./telemetry.js"; /** @@ -54,23 +71,81 @@ type LocalCommandBuilderArguments< readonly func: CommandFunction; }; +// --------------------------------------------------------------------------- +// Global logging flags +// --------------------------------------------------------------------------- + /** - * Build a command with automatic flag telemetry. + * Hidden `--log-level` flag injected into every command by {@link buildCommand}. * - * This is a drop-in replacement for Stricli's buildCommand that wraps the - * command function to automatically capture flag values as Sentry tags. + * Accepts one of the valid log level names. Hidden so it doesn't clutter + * individual command `--help` output — it's documented at the CLI level. + */ +export const LOG_LEVEL_FLAG = { + kind: "enum" as const, + values: LOG_LEVEL_NAMES as unknown as LogLevelName[], + brief: "Set log verbosity level", + optional: true as const, + hidden: true as const, +} as const; + +/** + * Hidden `--verbose` flag injected into every command by {@link buildCommand}. + * Equivalent to `--log-level debug`. + */ +export const VERBOSE_FLAG = { + kind: "boolean" as const, + brief: "Enable verbose (debug-level) logging output", + default: false, + hidden: true as const, +} as const; + +/** The flag key for the injected --log-level flag (always stripped) */ +const LOG_LEVEL_KEY = "log-level"; + +/** + * Apply logging flags parsed by Stricli. * - * Usage is identical to Stricli's buildCommand - just change the import: - * ```ts - * // Before: - * import { buildCommand } from "@stricli/core"; + * `--log-level` takes priority over `--verbose`. If neither is specified, + * the level is left as-is (env var or default). * - * // After: - * import { buildCommand } from "../../lib/command.js"; - * ``` + * @param logLevel - Value of the `--log-level` flag, if provided + * @param verbose - Value of the `--verbose` flag + */ +export function applyLoggingFlags( + logLevel: LogLevelName | undefined, + verbose: boolean +): void { + if (logLevel) { + setLogLevel(parseLogLevel(logLevel)); + } else if (verbose) { + setLogLevel(parseLogLevel("debug")); + } +} + +// --------------------------------------------------------------------------- +// buildCommand — the single entry point for all Sentry CLI commands +// --------------------------------------------------------------------------- + +/** + * Build a Sentry CLI command with telemetry and global logging flags. + * + * This is the **only** command builder that should be used. It: + * 1. Injects hidden `--log-level` and `--verbose` flags into the parameters + * 2. Intercepts them before the original `func` runs to call `setLogLevel()` + * 3. Strips injected flags so the original function never sees them + * 4. Captures flag values and positional arguments as Sentry telemetry context + * + * When a command already defines its own `verbose` flag (e.g. the `api` command + * uses `--verbose` for HTTP request/response output), the injected `VERBOSE_FLAG` + * is skipped. The command's own `verbose` value is still used for log-level + * side-effects, and it is **not** stripped — the original func receives it as usual. * - * @param builderArgs - Same arguments as Stricli's buildCommand - * @returns A Command with automatic flag telemetry + * Flag keys use kebab-case because Stricli uses the literal object key as + * the CLI flag name (e.g. `"log-level"` → `--log-level`). + * + * @param builderArgs - Same shape as Stricli's buildCommand arguments + * @returns A fully-wrapped Stricli Command */ export function buildCommand< const FLAGS extends BaseFlags = NonNullable, @@ -81,27 +156,69 @@ export function buildCommand< ): Command { const originalFunc = builderArgs.func; - // Wrap the function to capture flags and args before execution - const wrappedFunc = function ( - this: CONTEXT, - flags: FLAGS, - ...args: ARGS - ): ReturnType { + // Merge logging flags into the command's flag definitions. + // Quoted keys produce kebab-case CLI flags: "log-level" → --log-level + const existingParams = (builderArgs.parameters ?? {}) as Record< + string, + unknown + >; + const existingFlags = (existingParams.flags ?? {}) as Record; + + // If the command already defines --verbose (e.g. api command), don't override it. + const commandOwnsVerbose = "verbose" in existingFlags; + + const mergedFlags: Record = { + ...existingFlags, + [LOG_LEVEL_KEY]: LOG_LEVEL_FLAG, + }; + if (!commandOwnsVerbose) { + mergedFlags.verbose = VERBOSE_FLAG; + } + const mergedParams = { ...existingParams, flags: mergedFlags }; + + // Wrap func to intercept logging flags, capture telemetry, then call original + // biome-ignore lint/suspicious/noExplicitAny: Stricli's CommandFunction type is complex + const wrappedFunc = function (this: CONTEXT, flags: any, ...args: any[]) { + // Apply logging side-effects from whichever flags are present. + // The command's own --verbose (if any) also triggers debug-level logging. + const logLevel = flags[LOG_LEVEL_KEY] as LogLevelName | undefined; + const verbose = flags.verbose as boolean; + applyLoggingFlags(logLevel, verbose); + + // Strip only the flags WE injected — never strip command-owned flags. + // --log-level is always ours. --verbose is only stripped when we injected it. + const cleanFlags: Record = {}; + for (const [key, value] of Object.entries( + flags as Record + )) { + if (key === LOG_LEVEL_KEY) { + continue; + } + if (key === "verbose" && !commandOwnsVerbose) { + continue; + } + cleanFlags[key] = value; + } + // Capture flag values as telemetry tags - setFlagContext(flags as Record); + setFlagContext(cleanFlags); // Capture positional arguments as context if (args.length > 0) { setArgsContext(args); } - // Call the original function with the same context and arguments - return originalFunc.call(this, flags, ...args); + return originalFunc.call( + this, + cleanFlags as FLAGS, + ...(args as unknown as ARGS) + ); } as typeof originalFunc; - // Build the command with the wrapped function + // Build the command with the wrapped function via Stricli return stricliCommand({ ...builderArgs, + parameters: mergedParams, func: wrappedFunc, // biome-ignore lint/suspicious/noExplicitAny: Stricli types are complex unions } as any); diff --git a/src/lib/logger.ts b/src/lib/logger.ts index 153070e6b..c6b60bcad 100644 --- a/src/lib/logger.ts +++ b/src/lib/logger.ts @@ -44,9 +44,19 @@ * - `NO_COLOR` — Disables color output (respected by consola natively) * - `FORCE_COLOR` — Forces color output even in non-TTY * + * ## withTag level propagation + * + * Consola's `withTag()` creates an independent instance that snapshots the + * parent's level at creation time. Scoped loggers created at module load + * time (e.g. `const log = logger.withTag("upgrade")`) would miss later + * `setLogLevel()` calls. To fix this, the logger's `withTag` method is + * wrapped to register all children in a registry, and `setLogLevel()` + * propagates the level to every registered child. + * * @module */ +import type { ConsolaInstance } from "consola"; import { createConsola } from "consola"; /** @@ -118,6 +128,16 @@ export function getEnvLogLevel(): number | null { return null; } +/** + * Registry of all scoped loggers created via `logger.withTag()` at any depth. + * + * When `setLogLevel()` is called, every registered descendant is updated so that + * module-level scoped loggers (created at import time, before the CLI parses + * `--log-level` or `SENTRY_LOG_LEVEL`) see the new level. Grandchildren and + * deeper descendants are tracked via recursive `patchWithTag()`. + */ +const scopedLoggers: ConsolaInstance[] = []; + /** * Global CLI logger instance. * @@ -141,6 +161,27 @@ export const logger = createConsola({ // Sentry reporter is added after Sentry.init() via attachSentryReporter(). }); +/** + * Patch a consola instance's `withTag` so every child (and grandchild) + * is registered in {@link scopedLoggers} for {@link setLogLevel} propagation. + */ +function patchWithTag(instance: ConsolaInstance): void { + const original = instance.withTag.bind(instance); + instance.withTag = (tag: string): ConsolaInstance => { + const child = original(tag); + scopedLoggers.push(child); + patchWithTag(child); + return child; + }; +} + +// Patch the root logger so all descendants are tracked. +// Consola's withTag() creates a completely independent instance that +// snapshots the level at creation time — children never see later +// setLogLevel() calls. By registering them here, setLogLevel() can +// propagate the new level to all descendants. +patchWithTag(logger); + /** Whether the Sentry reporter has already been attached */ let sentryReporterAttached = false; @@ -182,58 +223,22 @@ export function attachSentryReporter(): void { } /** - * Set the logger level at runtime. + * Set the logger level at runtime and propagate to all scoped children. + * + * Called from: + * - bin.ts to apply `SENTRY_LOG_LEVEL` env var early + * - `buildCommand` wrapper when `--log-level` or `--verbose` flags are parsed * - * Called from the global `--verbose` / `--log-level` flag handler in bin.ts - * after pre-parsing argv. + * Propagation is necessary because consola's `withTag()` creates independent + * instances that snapshot the parent's level at creation time. Module-level + * scoped loggers (e.g. `const log = logger.withTag("upgrade")`) would never + * see a later level change without this. * * @param level - consola numeric level (0-5) */ export function setLogLevel(level: number): void { logger.level = level; -} - -/** - * Parse `--verbose` and `--log-level ` from raw argv. - * - * These are "global" flags that must be processed before Stricli sees the args, - * because Stricli has no concept of global flags. - * - * `--log-level` is consumed (removed from argv) because it's exclusively a - * logger flag — no command defines it. `--verbose` is NOT consumed because - * some commands (e.g., `api`) define their own `--verbose` flag with - * command-specific semantics. - * - * Priority: `--log-level` wins over `--verbose` if both are specified. - * `--verbose` is equivalent to `--log-level debug`. - * - * @param argv - Raw process.argv.slice(2), mutated in place - * @returns The resolved log level, or null if no flag was specified - */ -export function extractLogLevelFromArgs(argv: string[]): number | null { - let resolvedLevel: number | null = null; - - const debugLevel = LOG_LEVEL_NAMES.indexOf("debug"); - - // Check --verbose but leave it in argv — commands like `api` have their own --verbose flag - const verboseIdx = argv.indexOf("--verbose"); - if (verboseIdx !== -1) { - resolvedLevel = debugLevel; + for (const child of scopedLoggers) { + child.level = level; } - - // Process --log-level (overrides --verbose, consumed from argv) - const logLevelIdx = argv.indexOf("--log-level"); - if (logLevelIdx !== -1) { - const levelValue = argv[logLevelIdx + 1]; - if (levelValue && !levelValue.startsWith("-")) { - resolvedLevel = parseLogLevel(levelValue); - argv.splice(logLevelIdx, 2); - } else { - // --log-level without value — remove just the flag, use debug as sensible default - resolvedLevel = debugLevel; - argv.splice(logLevelIdx, 1); - } - } - - return resolvedLevel; } diff --git a/test/lib/command.test.ts b/test/lib/command.test.ts index e2c13768a..9999f7af4 100644 --- a/test/lib/command.test.ts +++ b/test/lib/command.test.ts @@ -15,7 +15,14 @@ import { type CommandContext, run, } from "@stricli/core"; -import { buildCommand, numberParser } from "../../src/lib/command.js"; +import { + applyLoggingFlags, + buildCommand, + LOG_LEVEL_FLAG, + numberParser, + VERBOSE_FLAG, +} from "../../src/lib/command.js"; +import { LOG_LEVEL_NAMES, logger, setLogLevel } from "../../src/lib/logger.js"; /** Minimal context for test commands */ type TestContext = CommandContext & { @@ -254,3 +261,345 @@ describe("buildCommand telemetry integration", () => { expect(setTagSpy).toHaveBeenCalledWith("flag.delay", "1"); }); }); + +// --------------------------------------------------------------------------- +// Global logging flag definitions +// --------------------------------------------------------------------------- + +describe("LOG_LEVEL_FLAG", () => { + test("is an enum flag with all log level names", () => { + expect(LOG_LEVEL_FLAG.kind).toBe("enum"); + expect(LOG_LEVEL_FLAG.values).toEqual([...LOG_LEVEL_NAMES]); + }); + + test("is optional and hidden", () => { + expect(LOG_LEVEL_FLAG.optional).toBe(true); + expect(LOG_LEVEL_FLAG.hidden).toBe(true); + }); +}); + +describe("VERBOSE_FLAG", () => { + test("is a boolean flag defaulting to false", () => { + expect(VERBOSE_FLAG.kind).toBe("boolean"); + expect(VERBOSE_FLAG.default).toBe(false); + }); + + test("is hidden", () => { + expect(VERBOSE_FLAG.hidden).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// applyLoggingFlags +// --------------------------------------------------------------------------- + +describe("applyLoggingFlags", () => { + let originalLevel: number; + + beforeEach(() => { + originalLevel = logger.level; + }); + + afterEach(() => { + setLogLevel(originalLevel); + }); + + test("sets level from logLevel flag", () => { + applyLoggingFlags("debug", false); + expect(logger.level).toBe(4); + }); + + test("sets level from verbose flag", () => { + applyLoggingFlags(undefined, true); + expect(logger.level).toBe(4); // debug + }); + + test("logLevel takes priority over verbose", () => { + applyLoggingFlags("error", true); + expect(logger.level).toBe(0); // error, not debug + }); + + test("does nothing when both are unset/false", () => { + const before = logger.level; + applyLoggingFlags(undefined, false); + expect(logger.level).toBe(before); + }); + + test("accepts all valid level names", () => { + for (const name of LOG_LEVEL_NAMES) { + applyLoggingFlags(name, false); + expect(logger.level).toBe(LOG_LEVEL_NAMES.indexOf(name)); + } + }); +}); + +// --------------------------------------------------------------------------- +// buildCommand +// --------------------------------------------------------------------------- + +describe("buildCommand", () => { + test("builds a valid command object", () => { + const command = buildCommand({ + docs: { brief: "Test command" }, + parameters: { + flags: { + json: { kind: "boolean", brief: "JSON output", default: false }, + }, + }, + func(_flags: { json: boolean }) { + // no-op + }, + }); + expect(command).toBeDefined(); + }); + + test("accepts --verbose and --log-level flags without error", async () => { + let calledFlags: Record | null = null; + + const command = buildCommand<{ json: boolean }, [], TestContext>({ + docs: { brief: "Test" }, + parameters: { + flags: { + json: { kind: "boolean", brief: "JSON output", default: false }, + }, + }, + func(this: TestContext, flags: { json: boolean }) { + calledFlags = flags as unknown as Record; + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + // Should NOT throw "No flag registered for --verbose" + await run(app, ["test", "--verbose", "--json"], { + process, + } as TestContext); + + // Original func receives json flag but NOT verbose/log-level + expect(calledFlags).toBeDefined(); + expect(calledFlags!.json).toBe(true); + expect(calledFlags!.verbose).toBeUndefined(); + expect(calledFlags!["log-level"]).toBeUndefined(); + }); + + test("--verbose sets logger to debug level", async () => { + const originalLevel = logger.level; + try { + const command = buildCommand, [], TestContext>({ + docs: { brief: "Test" }, + parameters: {}, + func() { + // no-op + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + await run(app, ["test", "--verbose"], { process } as TestContext); + + expect(logger.level).toBe(4); // debug + } finally { + setLogLevel(originalLevel); + } + }); + + test("--log-level sets logger to specified level", async () => { + const originalLevel = logger.level; + try { + const command = buildCommand, [], TestContext>({ + docs: { brief: "Test" }, + parameters: {}, + func() { + // no-op + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + await run(app, ["test", "--log-level", "trace"], { + process, + } as TestContext); + + expect(logger.level).toBe(5); // trace + } finally { + setLogLevel(originalLevel); + } + }); + + test("--log-level=value (equals form) works", async () => { + const originalLevel = logger.level; + try { + const command = buildCommand, [], TestContext>({ + docs: { brief: "Test" }, + parameters: {}, + func() { + // no-op + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + await run(app, ["test", "--log-level=error"], { + process, + } as TestContext); + + expect(logger.level).toBe(0); // error + } finally { + setLogLevel(originalLevel); + } + }); + + test("strips logging flags from func's flags parameter", async () => { + let receivedFlags: Record | null = null; + + const command = buildCommand<{ limit: number }, [], TestContext>({ + docs: { brief: "Test" }, + parameters: { + flags: { + limit: { + kind: "parsed", + parse: numberParser, + brief: "Limit", + default: "10", + }, + }, + }, + func(this: TestContext, flags: { limit: number }) { + receivedFlags = flags as unknown as Record; + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + await run( + app, + ["test", "--verbose", "--log-level", "debug", "--limit", "50"], + { process } as TestContext + ); + + expect(receivedFlags).toBeDefined(); + expect(receivedFlags!.limit).toBe(50); + // Logging flags must be stripped + expect(receivedFlags!.verbose).toBeUndefined(); + expect(receivedFlags!["log-level"]).toBeUndefined(); + }); + + test("preserves command's own --verbose flag when already defined", async () => { + const originalLevel = logger.level; + let receivedFlags: Record | null = null; + + try { + // Simulates the api command: defines its own --verbose with HTTP semantics + const command = buildCommand< + { verbose: boolean; silent: boolean }, + [], + TestContext + >({ + docs: { brief: "Test" }, + parameters: { + flags: { + verbose: { + kind: "boolean", + brief: "Show HTTP details", + default: false, + }, + silent: { + kind: "boolean", + brief: "Suppress output", + default: false, + }, + }, + }, + func(this: TestContext, flags: { verbose: boolean; silent: boolean }) { + receivedFlags = flags as unknown as Record; + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + await run(app, ["test", "--verbose", "--log-level", "trace"], { + process, + } as TestContext); + + // Command's own --verbose is passed through (not stripped) + expect(receivedFlags).toBeDefined(); + expect(receivedFlags!.verbose).toBe(true); + expect(receivedFlags!.silent).toBe(false); + // --log-level is always stripped (it's ours) + expect(receivedFlags!["log-level"]).toBeUndefined(); + // --verbose also sets debug-level logging as a side-effect + // but --log-level=trace takes priority + expect(logger.level).toBe(5); // trace + } finally { + setLogLevel(originalLevel); + } + }); + + test("command's own --verbose sets debug log level as side-effect", async () => { + const originalLevel = logger.level; + + try { + const command = buildCommand<{ verbose: boolean }, [], TestContext>({ + docs: { brief: "Test" }, + parameters: { + flags: { + verbose: { + kind: "boolean", + brief: "Show HTTP details", + default: false, + }, + }, + }, + func() { + // no-op + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const { process } = createTestProcess(); + + await run(app, ["test", "--verbose"], { + process, + } as TestContext); + + // Even though verbose is command-owned, it triggers debug-level logging + expect(logger.level).toBe(4); // debug + } finally { + setLogLevel(originalLevel); + } + }); +}); diff --git a/test/lib/logger.property.test.ts b/test/lib/logger.property.test.ts index 6612a8d81..3f0ab5c16 100644 --- a/test/lib/logger.property.test.ts +++ b/test/lib/logger.property.test.ts @@ -4,22 +4,20 @@ * Uses fast-check to verify invariants that should hold for any valid input. */ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { - array, - constant, constantFrom, assert as fcAssert, integer, - oneof, property, string, } from "fast-check"; import { - extractLogLevelFromArgs, LOG_LEVEL_NAMES, type LogLevelName, + logger, parseLogLevel, + setLogLevel, } from "../../src/lib/logger.js"; import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; @@ -35,11 +33,6 @@ const invalidLevelName = string().filter( s.length > 0 ); -/** Generate a random CLI arg that is NOT --verbose or --log-level */ -const nonLogArg = string().filter( - (s) => s !== "--verbose" && s !== "--log-level" && s.length > 0 -); - describe("property: parseLogLevel", () => { test("always returns a number between 0 and 5 for valid names", () => { fcAssert( @@ -99,97 +92,26 @@ describe("property: parseLogLevel", () => { }); }); -describe("property: extractLogLevelFromArgs", () => { - test("returns null and doesn't modify args with no log flags", () => { - fcAssert( - property(array(nonLogArg, { maxLength: 10 }), (args) => { - const original = [...args]; - const result = extractLogLevelFromArgs(args); - expect(result).toBeNull(); - expect(args).toEqual(original); - }), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); - - test("--verbose always produces level 4 (debug) and stays in argv", () => { - fcAssert( - property( - array(nonLogArg, { maxLength: 5 }), - integer({ min: 0, max: 5 }), - (otherArgs, insertIdx) => { - const args = [...otherArgs]; - const pos = Math.min(insertIdx, args.length); - args.splice(pos, 0, "--verbose"); - const original = [...args]; +describe("property: setLogLevel propagation", () => { + let originalLevel: number; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(4); - // --verbose is NOT consumed — commands like `api` have their own --verbose - expect(args).toEqual(original); - } - ), - { numRuns: DEFAULT_NUM_RUNS } - ); + beforeEach(() => { + originalLevel = logger.level; }); - test("--log-level with valid name always produces correct level", () => { - fcAssert( - property( - array(nonLogArg, { maxLength: 5 }), - validLevelName, - integer({ min: 0, max: 5 }), - (otherArgs, levelName, insertIdx) => { - const args = [...otherArgs]; - const pos = Math.min(insertIdx, args.length); - args.splice(pos, 0, "--log-level", levelName); - - const result = extractLogLevelFromArgs(args); - expect(result).toBe(parseLogLevel(levelName)); - expect(args).not.toContain("--log-level"); - expect(args).not.toContain(levelName); - } - ), - { numRuns: DEFAULT_NUM_RUNS } - ); + afterEach(() => { + setLogLevel(originalLevel); }); - test("--log-level overrides --verbose", () => { + test("any level set on parent propagates to withTag children", () => { fcAssert( - property(validLevelName, (levelName) => { - const args = ["--verbose", "--log-level", levelName, "cmd"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(parseLogLevel(levelName)); - // --log-level consumed, --verbose stays - expect(args).toEqual(["--verbose", "cmd"]); + property(integer({ min: 0, max: 5 }), (level) => { + const child = logger.withTag("prop-test"); + setLogLevel(level); + expect(logger.level).toBe(level); + expect(child.level).toBe(level); }), { numRuns: DEFAULT_NUM_RUNS } ); }); - - test("--verbose preserves all args, --log-level removes exactly 2 elements", () => { - fcAssert( - property( - array(nonLogArg, { minLength: 1, maxLength: 5 }), - oneof(constant("verbose"), constant("log-level")), - validLevelName, - (otherArgs, flagType, levelName) => { - const originalNonLogArgs = [...otherArgs]; - - if (flagType === "verbose") { - // --verbose is NOT consumed from argv - const args = ["--verbose", ...otherArgs]; - extractLogLevelFromArgs(args); - expect(args).toEqual(["--verbose", ...originalNonLogArgs]); - } else { - // --log-level + value ARE consumed from argv - const args = ["--log-level", levelName, ...otherArgs]; - extractLogLevelFromArgs(args); - expect(args).toEqual(originalNonLogArgs); - } - } - ), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); }); diff --git a/test/lib/logger.test.ts b/test/lib/logger.test.ts index 2895b0765..39d2a9ce6 100644 --- a/test/lib/logger.test.ts +++ b/test/lib/logger.test.ts @@ -1,14 +1,13 @@ /** * Unit tests for the logger module. * - * Tests parseLogLevel, getEnvLogLevel, extractLogLevelFromArgs, setLogLevel, + * Tests parseLogLevel, getEnvLogLevel, setLogLevel (with withTag propagation), * attachSentryReporter, and the logger instance configuration. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { attachSentryReporter, - extractLogLevelFromArgs, getEnvLogLevel, LOG_LEVEL_ENV_VAR, LOG_LEVEL_NAMES, @@ -137,96 +136,49 @@ describe("setLogLevel", () => { setLogLevel(5); expect(logger.level).toBe(5); }); -}); -describe("extractLogLevelFromArgs", () => { - test("returns null when no flags are present", () => { - const args = ["issue", "list", "--json"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBeNull(); - expect(args).toEqual(["issue", "list", "--json"]); - }); + test("propagates level to withTag children", () => { + const child1 = logger.withTag("test-child-1"); + const child2 = logger.withTag("test-child-2"); - test("reads --verbose but leaves it in args for downstream commands", () => { - const args = ["--verbose", "issue", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(4); // debug - // --verbose stays in argv because commands like `api` have their own --verbose flag - expect(args).toEqual(["--verbose", "issue", "list"]); - }); - - test("reads --verbose from middle of args without consuming it", () => { - const args = ["issue", "--verbose", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(4); - expect(args).toEqual(["issue", "--verbose", "list"]); - }); + // Children start at the logger's current level + expect(child1.level).toBe(logger.level); + expect(child2.level).toBe(logger.level); - test("reads --verbose from end of args without consuming it", () => { - const args = ["issue", "list", "--verbose"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(4); - expect(args).toEqual(["issue", "list", "--verbose"]); - }); - - test("extracts --log-level with value", () => { - const args = ["--log-level", "trace", "issue", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(5); // trace - expect(args).toEqual(["issue", "list"]); - }); + // Change the level + setLogLevel(5); + expect(child1.level).toBe(5); + expect(child2.level).toBe(5); - test("extracts --log-level from middle of args", () => { - const args = ["issue", "--log-level", "error", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(0); // error - expect(args).toEqual(["issue", "list"]); + // Change again + setLogLevel(0); + expect(child1.level).toBe(0); + expect(child2.level).toBe(0); }); - test("--log-level overrides --verbose when both present", () => { - const args = ["--verbose", "--log-level", "warn", "issue", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(1); // warn wins over debug - // --log-level is consumed, --verbose stays - expect(args).toEqual(["--verbose", "issue", "list"]); - }); + test("propagates to children created before level change", () => { + // Simulate the real-world scenario: module-level child created at default level, + // then setLogLevel called later by bin.ts + const moduleChild = logger.withTag("upgrade"); + expect(moduleChild.level).toBe(originalLevel); - test("--log-level without value defaults to debug", () => { - const args = ["issue", "--log-level"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(4); // debug - expect(args).toEqual(["issue"]); + setLogLevel(4); // debug + expect(moduleChild.level).toBe(4); }); - test("--log-level followed by another flag defaults to debug", () => { - const args = ["--log-level", "--json", "issue", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBe(4); // debug (--json starts with -) - expect(args).toEqual(["--json", "issue", "list"]); - }); + test("propagates to grandchildren (nested withTag)", () => { + const child = logger.withTag("parent"); + const grandchild = child.withTag("sub-scope"); - test("handles all valid level names via --log-level", () => { - for (const name of LOG_LEVEL_NAMES) { - const args = ["--log-level", name, "cmd"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBeGreaterThanOrEqual(0); - expect(result).toBeLessThanOrEqual(5); - expect(args).toEqual(["cmd"]); - } - }); + expect(grandchild.level).toBe(logger.level); - test("does not consume -v (only --verbose)", () => { - const args = ["-v", "issue", "list"]; - const result = extractLogLevelFromArgs(args); - expect(result).toBeNull(); - expect(args).toEqual(["-v", "issue", "list"]); - }); + setLogLevel(5); // trace + expect(child.level).toBe(5); + expect(grandchild.level).toBe(5); - test("handles empty args array", () => { - const args: string[] = []; - const result = extractLogLevelFromArgs(args); - expect(result).toBeNull(); - expect(args).toEqual([]); + setLogLevel(0); // error + expect(child.level).toBe(0); + expect(grandchild.level).toBe(0); }); }); @@ -247,9 +199,14 @@ describe("logger instance", () => { expect(typeof scoped.debug).toBe("function"); }); - test("default level is info (3)", () => { - // Logger always starts at info (3) — env var is applied lazily by bin.ts - expect(logger.level).toBe(3); + test("level can be set to info (3) via setLogLevel", () => { + const before = logger.level; + try { + setLogLevel(3); + expect(logger.level).toBe(3); + } finally { + setLogLevel(before); + } }); });