Skip to content

Commit d7440ff

Browse files
authored
fix(sdk): apply Stricli flag defaults in SDK invoke path (#1027)
## Problem **CLI-1W1** (3 events) and **CLI-1W0** (1 event) crash at `serializeTimeRange(flags.period)` because `flags.period` is `undefined`. Both issues affect users calling the CLI programmatically via `createSentrySDK()` (the typed SDK). ## Root Cause The typed SDK's invoke path (`src/lib/sdk-invoke.ts`) calls command handlers **directly**, bypassing Stricli's `buildArgumentScanner`. This means parsed flags with defaults (e.g., `period: { kind: 'parsed', default: '7d', parse: parsePeriod }`) never have their `parse` function called on the default string when the SDK caller omits them. The generated SDK methods (`sdk.generated.ts`) pass `period: params?.period`, which evaluates to `undefined` when omitted. All 14+ commands with a `period` flag are affected. ## Fix Adds `applyFlagDefaults()` to the SDK invoke layer, which replicates Stricli's default application logic: - **`kind: "parsed"` flags** with a string `default` → calls `flag.parse(flag.default)` (same as Stricli's `parseInput`) - **Boolean/enum/counter flags** → uses the raw default value - **Strips `undefined` values** from caller-provided flags so they don't shadow defaults - **Skips flags** already set by the caller `resolveCommand()` now returns both the handler function and the command's flag definitions, so `buildInvoker` can apply defaults before calling the command func. This is a systemic fix — protects **all** flags across **all** SDK-invoked commands, not just `period`. ## Testing - 11 new unit tests for `applyFlagDefaults()` covering: parsed flags with defaults, boolean defaults, enum defaults, optional flags without defaults, undefined stripping, caller value preservation, parse failure handling, and combined scenarios. Fixes CLI-1W1, CLI-1W0
1 parent d9bcd70 commit d7440ff

2 files changed

Lines changed: 304 additions & 18 deletions

File tree

src/lib/sdk-invoke.ts

Lines changed: 121 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,43 @@ function buildIsolatedEnv(
5959
/** Type alias for command handler functions loaded from Stricli's route tree. */
6060
type CommandHandler = (...args: unknown[]) => unknown;
6161

62-
/** Cached command loaders, keyed by joined path (e.g., "org.list") */
63-
const commandCache = new Map<string, () => Promise<CommandHandler>>();
62+
/**
63+
* Stricli flag definition shape — subset needed for default application.
64+
*
65+
* The SDK invoke path bypasses Stricli's `buildArgumentScanner`, so parsed
66+
* flags with defaults never have their `parse` function called on the default
67+
* string. We capture the flag definitions here so {@link applyFlagDefaults}
68+
* can replicate that behavior at invocation time.
69+
*/
70+
export type FlagDef = {
71+
kind: string;
72+
default?: unknown;
73+
optional?: boolean;
74+
parse?: (value: string) => unknown;
75+
variadic?: boolean;
76+
};
77+
78+
/** Resolved command: handler function + flag definitions for default application. */
79+
type ResolvedCommand = {
80+
handler: CommandHandler;
81+
flagDefs: Record<string, FlagDef>;
82+
};
83+
84+
/** Cached command entries, keyed by joined path (e.g., "org.list"). */
85+
const commandCache = new Map<
86+
string,
87+
{ loader: () => Promise<CommandHandler>; flagDefs: Record<string, FlagDef> }
88+
>();
6489

6590
/**
6691
* Resolve a command from the route tree by path segments.
92+
* Returns both the handler function and the command's flag definitions.
6793
* Result is cached — route tree is only walked once per command.
6894
*/
69-
async function resolveCommand(path: string[]): Promise<CommandHandler> {
95+
async function resolveCommand(path: string[]): Promise<ResolvedCommand> {
7096
const key = path.join(".");
71-
let loaderFn = commandCache.get(key);
72-
if (!loaderFn) {
97+
let cached = commandCache.get(key);
98+
if (!cached) {
7399
const { routes } = await import("../app.js");
74100
// Walk route tree: routes → sub-route → command
75101
// biome-ignore lint/suspicious/noExplicitAny: Stricli's RoutingTarget union requires runtime duck-typing
@@ -80,16 +106,87 @@ async function resolveCommand(path: string[]): Promise<CommandHandler> {
80106
throw new Error(`SDK: command not found: ${path.join(" ")}`);
81107
}
82108
}
83-
// target is now a Command — cache its loader
109+
// target is now a Command — extract flag definitions and cache loader
84110
const command = target;
85-
loaderFn = () =>
86-
command.loader().then(
87-
// biome-ignore lint/suspicious/noExplicitAny: Stricli CommandModule shape has a default export
88-
(m: any) => (typeof m === "function" ? m : m.default)
89-
);
90-
commandCache.set(key, loaderFn);
111+
const flagDefs: Record<string, FlagDef> = command.parameters?.flags ?? {};
112+
cached = {
113+
loader: () =>
114+
command.loader().then(
115+
// biome-ignore lint/suspicious/noExplicitAny: Stricli CommandModule shape has a default export
116+
(m: any) => (typeof m === "function" ? m : m.default)
117+
),
118+
flagDefs,
119+
};
120+
commandCache.set(key, cached);
121+
}
122+
return { handler: await cached.loader(), flagDefs: cached.flagDefs };
123+
}
124+
125+
/**
126+
* Resolve the default value for a single flag definition.
127+
*
128+
* For `kind: "parsed"` flags with a string default, calls `flag.parse(flag.default)`
129+
* to replicate Stricli's `parseInput` behavior. For all other flag kinds (boolean,
130+
* enum, counter), returns the raw default value.
131+
*
132+
* @returns The resolved default, or `undefined` if no default is defined.
133+
* @throws Re-throws if `flag.parse(flag.default)` fails — a parse function
134+
* that rejects its own default is a command definition bug, not a runtime error.
135+
*/
136+
function resolveFlagDefault(def: FlagDef): unknown {
137+
if (!("default" in def) || def.default === undefined) {
138+
return;
139+
}
140+
if (
141+
def.kind === "parsed" &&
142+
typeof def.default === "string" &&
143+
typeof def.parse === "function"
144+
) {
145+
return def.parse(def.default);
91146
}
92-
return loaderFn();
147+
return def.default;
148+
}
149+
150+
/**
151+
* Apply Stricli flag defaults for any missing or undefined flag values.
152+
*
153+
* The SDK invoke path bypasses Stricli's `buildArgumentScanner`, so parsed
154+
* flags with defaults (e.g., `period: { kind: "parsed", parse: parsePeriod,
155+
* default: "7d" }`) never have their `parse` function called on the default
156+
* string. This function replicates that behavior:
157+
*
158+
* - For `kind: "parsed"` flags with a string `default`, calls `flag.parse(flag.default)`
159+
* (same as Stricli's `parseInput` path in `parseInputsForFlag`).
160+
* - For `kind: "boolean"` / `kind: "enum"` flags with a `default`, uses the raw value.
161+
* - Skips flags already set (non-`undefined`) by the caller.
162+
*
163+
* @param flags - The flags object from the SDK caller (may have `undefined` values).
164+
* @param flagDefs - The command's Stricli flag definitions.
165+
* @returns A new flags object with defaults applied.
166+
*/
167+
export function applyFlagDefaults(
168+
flags: Record<string, unknown>,
169+
flagDefs: Record<string, FlagDef>
170+
): Record<string, unknown> {
171+
const result: Record<string, unknown> = {};
172+
// Copy caller-provided flags, skipping undefined values so they
173+
// don't shadow the defaults we're about to apply.
174+
for (const [key, value] of Object.entries(flags)) {
175+
if (value !== undefined) {
176+
result[key] = value;
177+
}
178+
}
179+
// Apply defaults for any flag not already set by the caller
180+
for (const [name, def] of Object.entries(flagDefs)) {
181+
if (name in result) {
182+
continue;
183+
}
184+
const resolved = resolveFlagDefault(def);
185+
if (resolved !== undefined) {
186+
result[name] = resolved;
187+
}
188+
}
189+
return result;
93190
}
94191

95192
// biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI escape sequences use ESC (0x1b)
@@ -412,26 +509,32 @@ export function buildInvoker(options?: SentryOptions) {
412509
): Promise<T> | AsyncIterable<T> {
413510
if (meta?.streaming) {
414511
return executeWithStream<T>(options, async (ctx, span) => {
415-
const func = await resolveCommand(commandPath);
512+
const { handler, flagDefs } = await resolveCommand(commandPath);
416513
if (span) {
417514
const { setCommandSpanName } = await import("./telemetry.js");
418515
setCommandSpanName(span, commandPath.join("."));
419516
}
420-
await func.call(
517+
const resolvedFlags = applyFlagDefaults(flags, flagDefs);
518+
await handler.call(
421519
ctx.context,
422-
{ ...flags, json: true },
520+
{ ...resolvedFlags, json: true },
423521
...positionalArgs
424522
);
425523
});
426524
}
427525

428526
return executeWithCapture<T>(options, async (ctx, span) => {
429-
const func = await resolveCommand(commandPath);
527+
const { handler, flagDefs } = await resolveCommand(commandPath);
430528
if (span) {
431529
const { setCommandSpanName } = await import("./telemetry.js");
432530
setCommandSpanName(span, commandPath.join("."));
433531
}
434-
await func.call(ctx.context, { ...flags, json: true }, ...positionalArgs);
532+
const resolvedFlags = applyFlagDefaults(flags, flagDefs);
533+
await handler.call(
534+
ctx.context,
535+
{ ...resolvedFlags, json: true },
536+
...positionalArgs
537+
);
435538
});
436539
};
437540
}

test/lib/sdk-invoke.test.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/**
2+
* Unit tests for the SDK invoke layer.
3+
*
4+
* Focuses on `applyFlagDefaults` which replicates Stricli's default
5+
* application for the SDK direct-invoke path (bypasses Stricli parsing).
6+
*/
7+
8+
import { describe, expect, test } from "vitest";
9+
import { applyFlagDefaults, type FlagDef } from "../../src/lib/sdk-invoke.js";
10+
11+
// ---------------------------------------------------------------------------
12+
// applyFlagDefaults — parsed flags with defaults
13+
// ---------------------------------------------------------------------------
14+
15+
describe("applyFlagDefaults: parsed flags with defaults", () => {
16+
test("calls parse on string default when flag is missing", () => {
17+
const flagDefs: Record<string, FlagDef> = {
18+
period: {
19+
kind: "parsed",
20+
default: "7d",
21+
parse: (v: string) => ({ type: "relative", period: v }),
22+
},
23+
};
24+
const result = applyFlagDefaults({}, flagDefs);
25+
expect(result.period).toEqual({ type: "relative", period: "7d" });
26+
});
27+
28+
test("calls parse on string default when flag value is undefined", () => {
29+
const flagDefs: Record<string, FlagDef> = {
30+
period: {
31+
kind: "parsed",
32+
default: "90d",
33+
parse: (v: string) => ({ type: "relative", period: v }),
34+
},
35+
};
36+
const result = applyFlagDefaults({ period: undefined }, flagDefs);
37+
expect(result.period).toEqual({ type: "relative", period: "90d" });
38+
});
39+
40+
test("preserves caller-provided value over default", () => {
41+
const flagDefs: Record<string, FlagDef> = {
42+
period: {
43+
kind: "parsed",
44+
default: "7d",
45+
parse: (v: string) => ({ type: "relative", period: v }),
46+
},
47+
};
48+
const callerValue = { type: "relative", period: "24h" };
49+
const result = applyFlagDefaults({ period: callerValue }, flagDefs);
50+
expect(result.period).toBe(callerValue);
51+
});
52+
53+
test("handles parse function that returns a number", () => {
54+
const flagDefs: Record<string, FlagDef> = {
55+
limit: {
56+
kind: "parsed",
57+
default: "25",
58+
parse: (v: string) => Number(v),
59+
},
60+
};
61+
const result = applyFlagDefaults({}, flagDefs);
62+
expect(result.limit).toBe(25);
63+
});
64+
65+
test("re-throws if parse function rejects its own default", () => {
66+
const flagDefs: Record<string, FlagDef> = {
67+
period: {
68+
kind: "parsed",
69+
default: "invalid",
70+
parse: () => {
71+
throw new Error("parse error");
72+
},
73+
},
74+
};
75+
expect(() => applyFlagDefaults({}, flagDefs)).toThrow("parse error");
76+
});
77+
});
78+
79+
// ---------------------------------------------------------------------------
80+
// applyFlagDefaults — boolean flags
81+
// ---------------------------------------------------------------------------
82+
83+
describe("applyFlagDefaults: boolean flags", () => {
84+
test("applies raw boolean default", () => {
85+
const flagDefs: Record<string, FlagDef> = {
86+
json: { kind: "boolean", default: false },
87+
fresh: { kind: "boolean", default: false },
88+
};
89+
const result = applyFlagDefaults({}, flagDefs);
90+
expect(result.json).toBe(false);
91+
expect(result.fresh).toBe(false);
92+
});
93+
94+
test("preserves caller-provided boolean over default", () => {
95+
const flagDefs: Record<string, FlagDef> = {
96+
fresh: { kind: "boolean", default: false },
97+
};
98+
const result = applyFlagDefaults({ fresh: true }, flagDefs);
99+
expect(result.fresh).toBe(true);
100+
});
101+
});
102+
103+
// ---------------------------------------------------------------------------
104+
// applyFlagDefaults — enum flags
105+
// ---------------------------------------------------------------------------
106+
107+
describe("applyFlagDefaults: enum flags", () => {
108+
test("applies raw enum default", () => {
109+
const flagDefs: Record<string, FlagDef> = {
110+
sort: { kind: "enum", default: "date" },
111+
};
112+
const result = applyFlagDefaults({}, flagDefs);
113+
expect(result.sort).toBe("date");
114+
});
115+
});
116+
117+
// ---------------------------------------------------------------------------
118+
// applyFlagDefaults — optional flags without defaults
119+
// ---------------------------------------------------------------------------
120+
121+
describe("applyFlagDefaults: optional flags without defaults", () => {
122+
test("does not inject value for optional flag with no default", () => {
123+
const flagDefs: Record<string, FlagDef> = {
124+
query: { kind: "parsed", optional: true },
125+
};
126+
const result = applyFlagDefaults({}, flagDefs);
127+
expect("query" in result).toBe(false);
128+
});
129+
});
130+
131+
// ---------------------------------------------------------------------------
132+
// applyFlagDefaults — strips undefined, preserves other falsy values
133+
// ---------------------------------------------------------------------------
134+
135+
describe("applyFlagDefaults: undefined stripping", () => {
136+
test("strips undefined values from input flags", () => {
137+
const flagDefs: Record<string, FlagDef> = {};
138+
const result = applyFlagDefaults(
139+
{ a: undefined, b: null, c: 0, d: "", e: false },
140+
flagDefs
141+
);
142+
expect("a" in result).toBe(false);
143+
expect(result.b).toBeNull();
144+
expect(result.c).toBe(0);
145+
expect(result.d).toBe("");
146+
expect(result.e).toBe(false);
147+
});
148+
});
149+
150+
// ---------------------------------------------------------------------------
151+
// applyFlagDefaults — multiple flags combined
152+
// ---------------------------------------------------------------------------
153+
154+
describe("applyFlagDefaults: combined scenario", () => {
155+
test("applies defaults for missing flags while preserving provided ones", () => {
156+
const flagDefs: Record<string, FlagDef> = {
157+
period: {
158+
kind: "parsed",
159+
default: "7d",
160+
parse: (v: string) => ({ type: "relative", period: v }),
161+
},
162+
limit: {
163+
kind: "parsed",
164+
default: "25",
165+
parse: (v: string) => Number(v),
166+
},
167+
sort: { kind: "enum", default: "date" },
168+
fresh: { kind: "boolean", default: false },
169+
query: { kind: "parsed", optional: true },
170+
};
171+
const result = applyFlagDefaults({ limit: 50, query: undefined }, flagDefs);
172+
// period: default applied via parse
173+
expect(result.period).toEqual({ type: "relative", period: "7d" });
174+
// limit: caller value preserved
175+
expect(result.limit).toBe(50);
176+
// sort: default applied (raw)
177+
expect(result.sort).toBe("date");
178+
// fresh: default applied (raw)
179+
expect(result.fresh).toBe(false);
180+
// query: undefined stripped, no default → absent
181+
expect("query" in result).toBe(false);
182+
});
183+
});

0 commit comments

Comments
 (0)