diff --git a/src/lib/sdk-invoke.ts b/src/lib/sdk-invoke.ts index e1c365cc9..e9137a3d3 100644 --- a/src/lib/sdk-invoke.ts +++ b/src/lib/sdk-invoke.ts @@ -59,17 +59,43 @@ function buildIsolatedEnv( /** Type alias for command handler functions loaded from Stricli's route tree. */ type CommandHandler = (...args: unknown[]) => unknown; -/** Cached command loaders, keyed by joined path (e.g., "org.list") */ -const commandCache = new Map Promise>(); +/** + * Stricli flag definition shape — subset needed for default application. + * + * The SDK invoke path bypasses Stricli's `buildArgumentScanner`, so parsed + * flags with defaults never have their `parse` function called on the default + * string. We capture the flag definitions here so {@link applyFlagDefaults} + * can replicate that behavior at invocation time. + */ +export type FlagDef = { + kind: string; + default?: unknown; + optional?: boolean; + parse?: (value: string) => unknown; + variadic?: boolean; +}; + +/** Resolved command: handler function + flag definitions for default application. */ +type ResolvedCommand = { + handler: CommandHandler; + flagDefs: Record; +}; + +/** Cached command entries, keyed by joined path (e.g., "org.list"). */ +const commandCache = new Map< + string, + { loader: () => Promise; flagDefs: Record } +>(); /** * Resolve a command from the route tree by path segments. + * Returns both the handler function and the command's flag definitions. * Result is cached — route tree is only walked once per command. */ -async function resolveCommand(path: string[]): Promise { +async function resolveCommand(path: string[]): Promise { const key = path.join("."); - let loaderFn = commandCache.get(key); - if (!loaderFn) { + let cached = commandCache.get(key); + if (!cached) { const { routes } = await import("../app.js"); // Walk route tree: routes → sub-route → command // biome-ignore lint/suspicious/noExplicitAny: Stricli's RoutingTarget union requires runtime duck-typing @@ -80,16 +106,87 @@ async function resolveCommand(path: string[]): Promise { throw new Error(`SDK: command not found: ${path.join(" ")}`); } } - // target is now a Command — cache its loader + // target is now a Command — extract flag definitions and cache loader const command = target; - loaderFn = () => - command.loader().then( - // biome-ignore lint/suspicious/noExplicitAny: Stricli CommandModule shape has a default export - (m: any) => (typeof m === "function" ? m : m.default) - ); - commandCache.set(key, loaderFn); + const flagDefs: Record = command.parameters?.flags ?? {}; + cached = { + loader: () => + command.loader().then( + // biome-ignore lint/suspicious/noExplicitAny: Stricli CommandModule shape has a default export + (m: any) => (typeof m === "function" ? m : m.default) + ), + flagDefs, + }; + commandCache.set(key, cached); + } + return { handler: await cached.loader(), flagDefs: cached.flagDefs }; +} + +/** + * Resolve the default value for a single flag definition. + * + * For `kind: "parsed"` flags with a string default, calls `flag.parse(flag.default)` + * to replicate Stricli's `parseInput` behavior. For all other flag kinds (boolean, + * enum, counter), returns the raw default value. + * + * @returns The resolved default, or `undefined` if no default is defined. + * @throws Re-throws if `flag.parse(flag.default)` fails — a parse function + * that rejects its own default is a command definition bug, not a runtime error. + */ +function resolveFlagDefault(def: FlagDef): unknown { + if (!("default" in def) || def.default === undefined) { + return; + } + if ( + def.kind === "parsed" && + typeof def.default === "string" && + typeof def.parse === "function" + ) { + return def.parse(def.default); } - return loaderFn(); + return def.default; +} + +/** + * Apply Stricli flag defaults for any missing or undefined flag values. + * + * The SDK invoke path bypasses Stricli's `buildArgumentScanner`, so parsed + * flags with defaults (e.g., `period: { kind: "parsed", parse: parsePeriod, + * default: "7d" }`) never have their `parse` function called on the default + * string. This function replicates that behavior: + * + * - For `kind: "parsed"` flags with a string `default`, calls `flag.parse(flag.default)` + * (same as Stricli's `parseInput` path in `parseInputsForFlag`). + * - For `kind: "boolean"` / `kind: "enum"` flags with a `default`, uses the raw value. + * - Skips flags already set (non-`undefined`) by the caller. + * + * @param flags - The flags object from the SDK caller (may have `undefined` values). + * @param flagDefs - The command's Stricli flag definitions. + * @returns A new flags object with defaults applied. + */ +export function applyFlagDefaults( + flags: Record, + flagDefs: Record +): Record { + const result: Record = {}; + // Copy caller-provided flags, skipping undefined values so they + // don't shadow the defaults we're about to apply. + for (const [key, value] of Object.entries(flags)) { + if (value !== undefined) { + result[key] = value; + } + } + // Apply defaults for any flag not already set by the caller + for (const [name, def] of Object.entries(flagDefs)) { + if (name in result) { + continue; + } + const resolved = resolveFlagDefault(def); + if (resolved !== undefined) { + result[name] = resolved; + } + } + return result; } // biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI escape sequences use ESC (0x1b) @@ -412,26 +509,32 @@ export function buildInvoker(options?: SentryOptions) { ): Promise | AsyncIterable { if (meta?.streaming) { return executeWithStream(options, async (ctx, span) => { - const func = await resolveCommand(commandPath); + const { handler, flagDefs } = await resolveCommand(commandPath); if (span) { const { setCommandSpanName } = await import("./telemetry.js"); setCommandSpanName(span, commandPath.join(".")); } - await func.call( + const resolvedFlags = applyFlagDefaults(flags, flagDefs); + await handler.call( ctx.context, - { ...flags, json: true }, + { ...resolvedFlags, json: true }, ...positionalArgs ); }); } return executeWithCapture(options, async (ctx, span) => { - const func = await resolveCommand(commandPath); + const { handler, flagDefs } = await resolveCommand(commandPath); if (span) { const { setCommandSpanName } = await import("./telemetry.js"); setCommandSpanName(span, commandPath.join(".")); } - await func.call(ctx.context, { ...flags, json: true }, ...positionalArgs); + const resolvedFlags = applyFlagDefaults(flags, flagDefs); + await handler.call( + ctx.context, + { ...resolvedFlags, json: true }, + ...positionalArgs + ); }); }; } diff --git a/test/lib/sdk-invoke.test.ts b/test/lib/sdk-invoke.test.ts new file mode 100644 index 000000000..55c7d0a78 --- /dev/null +++ b/test/lib/sdk-invoke.test.ts @@ -0,0 +1,183 @@ +/** + * Unit tests for the SDK invoke layer. + * + * Focuses on `applyFlagDefaults` which replicates Stricli's default + * application for the SDK direct-invoke path (bypasses Stricli parsing). + */ + +import { describe, expect, test } from "vitest"; +import { applyFlagDefaults, type FlagDef } from "../../src/lib/sdk-invoke.js"; + +// --------------------------------------------------------------------------- +// applyFlagDefaults — parsed flags with defaults +// --------------------------------------------------------------------------- + +describe("applyFlagDefaults: parsed flags with defaults", () => { + test("calls parse on string default when flag is missing", () => { + const flagDefs: Record = { + period: { + kind: "parsed", + default: "7d", + parse: (v: string) => ({ type: "relative", period: v }), + }, + }; + const result = applyFlagDefaults({}, flagDefs); + expect(result.period).toEqual({ type: "relative", period: "7d" }); + }); + + test("calls parse on string default when flag value is undefined", () => { + const flagDefs: Record = { + period: { + kind: "parsed", + default: "90d", + parse: (v: string) => ({ type: "relative", period: v }), + }, + }; + const result = applyFlagDefaults({ period: undefined }, flagDefs); + expect(result.period).toEqual({ type: "relative", period: "90d" }); + }); + + test("preserves caller-provided value over default", () => { + const flagDefs: Record = { + period: { + kind: "parsed", + default: "7d", + parse: (v: string) => ({ type: "relative", period: v }), + }, + }; + const callerValue = { type: "relative", period: "24h" }; + const result = applyFlagDefaults({ period: callerValue }, flagDefs); + expect(result.period).toBe(callerValue); + }); + + test("handles parse function that returns a number", () => { + const flagDefs: Record = { + limit: { + kind: "parsed", + default: "25", + parse: (v: string) => Number(v), + }, + }; + const result = applyFlagDefaults({}, flagDefs); + expect(result.limit).toBe(25); + }); + + test("re-throws if parse function rejects its own default", () => { + const flagDefs: Record = { + period: { + kind: "parsed", + default: "invalid", + parse: () => { + throw new Error("parse error"); + }, + }, + }; + expect(() => applyFlagDefaults({}, flagDefs)).toThrow("parse error"); + }); +}); + +// --------------------------------------------------------------------------- +// applyFlagDefaults — boolean flags +// --------------------------------------------------------------------------- + +describe("applyFlagDefaults: boolean flags", () => { + test("applies raw boolean default", () => { + const flagDefs: Record = { + json: { kind: "boolean", default: false }, + fresh: { kind: "boolean", default: false }, + }; + const result = applyFlagDefaults({}, flagDefs); + expect(result.json).toBe(false); + expect(result.fresh).toBe(false); + }); + + test("preserves caller-provided boolean over default", () => { + const flagDefs: Record = { + fresh: { kind: "boolean", default: false }, + }; + const result = applyFlagDefaults({ fresh: true }, flagDefs); + expect(result.fresh).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// applyFlagDefaults — enum flags +// --------------------------------------------------------------------------- + +describe("applyFlagDefaults: enum flags", () => { + test("applies raw enum default", () => { + const flagDefs: Record = { + sort: { kind: "enum", default: "date" }, + }; + const result = applyFlagDefaults({}, flagDefs); + expect(result.sort).toBe("date"); + }); +}); + +// --------------------------------------------------------------------------- +// applyFlagDefaults — optional flags without defaults +// --------------------------------------------------------------------------- + +describe("applyFlagDefaults: optional flags without defaults", () => { + test("does not inject value for optional flag with no default", () => { + const flagDefs: Record = { + query: { kind: "parsed", optional: true }, + }; + const result = applyFlagDefaults({}, flagDefs); + expect("query" in result).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// applyFlagDefaults — strips undefined, preserves other falsy values +// --------------------------------------------------------------------------- + +describe("applyFlagDefaults: undefined stripping", () => { + test("strips undefined values from input flags", () => { + const flagDefs: Record = {}; + const result = applyFlagDefaults( + { a: undefined, b: null, c: 0, d: "", e: false }, + flagDefs + ); + expect("a" in result).toBe(false); + expect(result.b).toBeNull(); + expect(result.c).toBe(0); + expect(result.d).toBe(""); + expect(result.e).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// applyFlagDefaults — multiple flags combined +// --------------------------------------------------------------------------- + +describe("applyFlagDefaults: combined scenario", () => { + test("applies defaults for missing flags while preserving provided ones", () => { + const flagDefs: Record = { + period: { + kind: "parsed", + default: "7d", + parse: (v: string) => ({ type: "relative", period: v }), + }, + limit: { + kind: "parsed", + default: "25", + parse: (v: string) => Number(v), + }, + sort: { kind: "enum", default: "date" }, + fresh: { kind: "boolean", default: false }, + query: { kind: "parsed", optional: true }, + }; + const result = applyFlagDefaults({ limit: 50, query: undefined }, flagDefs); + // period: default applied via parse + expect(result.period).toEqual({ type: "relative", period: "7d" }); + // limit: caller value preserved + expect(result.limit).toBe(50); + // sort: default applied (raw) + expect(result.sort).toBe("date"); + // fresh: default applied (raw) + expect(result.fresh).toBe(false); + // query: undefined stripped, no default → absent + expect("query" in result).toBe(false); + }); +});