Skip to content

Commit f916773

Browse files
committed
fix(sdk): apply Stricli flag defaults in SDK invoke path
The typed SDK (createSentrySDK()) 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. This caused CLI-1W1 and CLI-1W0: serializeTimeRange() received undefined instead of a TimeRange object because flags.period was never parsed from its '7d'/'90d' default. The fix adds applyFlagDefaults() which replicates Stricli's default application logic: - For kind:'parsed' flags with string defaults, calls flag.parse() - For boolean/enum/counter flags, uses the raw default value - Strips undefined values from caller-provided flags resolveCommand() now also returns the command's flag definitions alongside the handler, so buildInvoker can apply defaults before calling the command func. Fixes CLI-1W1, CLI-1W0
1 parent d9bcd70 commit f916773

2 files changed

Lines changed: 308 additions & 18 deletions

File tree

src/lib/sdk-invoke.ts

Lines changed: 124 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,90 @@ 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 or parsing fails.
133+
*/
134+
function resolveFlagDefault(def: FlagDef): unknown {
135+
if (!("default" in def) || def.default === undefined) {
136+
return;
91137
}
92-
return loaderFn();
138+
if (
139+
def.kind === "parsed" &&
140+
typeof def.default === "string" &&
141+
typeof def.parse === "function"
142+
) {
143+
try {
144+
return def.parse(def.default);
145+
} catch {
146+
// If parsing the default fails, skip — the command will handle the error
147+
return;
148+
}
149+
}
150+
return def.default;
151+
}
152+
153+
/**
154+
* Apply Stricli flag defaults for any missing or undefined flag values.
155+
*
156+
* The SDK invoke path bypasses Stricli's `buildArgumentScanner`, so parsed
157+
* flags with defaults (e.g., `period: { kind: "parsed", parse: parsePeriod,
158+
* default: "7d" }`) never have their `parse` function called on the default
159+
* string. This function replicates that behavior:
160+
*
161+
* - For `kind: "parsed"` flags with a string `default`, calls `flag.parse(flag.default)`
162+
* (same as Stricli's `parseInput` path in `parseInputsForFlag`).
163+
* - For `kind: "boolean"` / `kind: "enum"` flags with a `default`, uses the raw value.
164+
* - Skips flags already set (non-`undefined`) by the caller.
165+
*
166+
* @param flags - The flags object from the SDK caller (may have `undefined` values).
167+
* @param flagDefs - The command's Stricli flag definitions.
168+
* @returns A new flags object with defaults applied.
169+
*/
170+
export function applyFlagDefaults(
171+
flags: Record<string, unknown>,
172+
flagDefs: Record<string, FlagDef>
173+
): Record<string, unknown> {
174+
const result: Record<string, unknown> = {};
175+
// Copy caller-provided flags, skipping undefined values so they
176+
// don't shadow the defaults we're about to apply.
177+
for (const [key, value] of Object.entries(flags)) {
178+
if (value !== undefined) {
179+
result[key] = value;
180+
}
181+
}
182+
// Apply defaults for any flag not already set by the caller
183+
for (const [name, def] of Object.entries(flagDefs)) {
184+
if (name in result) {
185+
continue;
186+
}
187+
const resolved = resolveFlagDefault(def);
188+
if (resolved !== undefined) {
189+
result[name] = resolved;
190+
}
191+
}
192+
return result;
93193
}
94194

95195
// biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI escape sequences use ESC (0x1b)
@@ -412,26 +512,32 @@ export function buildInvoker(options?: SentryOptions) {
412512
): Promise<T> | AsyncIterable<T> {
413513
if (meta?.streaming) {
414514
return executeWithStream<T>(options, async (ctx, span) => {
415-
const func = await resolveCommand(commandPath);
515+
const { handler, flagDefs } = await resolveCommand(commandPath);
416516
if (span) {
417517
const { setCommandSpanName } = await import("./telemetry.js");
418518
setCommandSpanName(span, commandPath.join("."));
419519
}
420-
await func.call(
520+
const resolvedFlags = applyFlagDefaults(flags, flagDefs);
521+
await handler.call(
421522
ctx.context,
422-
{ ...flags, json: true },
523+
{ ...resolvedFlags, json: true },
423524
...positionalArgs
424525
);
425526
});
426527
}
427528

428529
return executeWithCapture<T>(options, async (ctx, span) => {
429-
const func = await resolveCommand(commandPath);
530+
const { handler, flagDefs } = await resolveCommand(commandPath);
430531
if (span) {
431532
const { setCommandSpanName } = await import("./telemetry.js");
432533
setCommandSpanName(span, commandPath.join("."));
433534
}
434-
await func.call(ctx.context, { ...flags, json: true }, ...positionalArgs);
535+
const resolvedFlags = applyFlagDefaults(flags, flagDefs);
536+
await handler.call(
537+
ctx.context,
538+
{ ...resolvedFlags, json: true },
539+
...positionalArgs
540+
);
435541
});
436542
};
437543
}

test/lib/sdk-invoke.test.ts

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
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 "bun:test";
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("skips default if parse throws", () => {
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+
const result = applyFlagDefaults({}, flagDefs);
76+
expect(result.period).toBeUndefined();
77+
});
78+
});
79+
80+
// ---------------------------------------------------------------------------
81+
// applyFlagDefaults — boolean flags
82+
// ---------------------------------------------------------------------------
83+
84+
describe("applyFlagDefaults: boolean flags", () => {
85+
test("applies raw boolean default", () => {
86+
const flagDefs: Record<string, FlagDef> = {
87+
json: { kind: "boolean", default: false },
88+
fresh: { kind: "boolean", default: false },
89+
};
90+
const result = applyFlagDefaults({}, flagDefs);
91+
expect(result.json).toBe(false);
92+
expect(result.fresh).toBe(false);
93+
});
94+
95+
test("preserves caller-provided boolean over default", () => {
96+
const flagDefs: Record<string, FlagDef> = {
97+
fresh: { kind: "boolean", default: false },
98+
};
99+
const result = applyFlagDefaults({ fresh: true }, flagDefs);
100+
expect(result.fresh).toBe(true);
101+
});
102+
});
103+
104+
// ---------------------------------------------------------------------------
105+
// applyFlagDefaults — enum flags
106+
// ---------------------------------------------------------------------------
107+
108+
describe("applyFlagDefaults: enum flags", () => {
109+
test("applies raw enum default", () => {
110+
const flagDefs: Record<string, FlagDef> = {
111+
sort: { kind: "enum", default: "date" },
112+
};
113+
const result = applyFlagDefaults({}, flagDefs);
114+
expect(result.sort).toBe("date");
115+
});
116+
});
117+
118+
// ---------------------------------------------------------------------------
119+
// applyFlagDefaults — optional flags without defaults
120+
// ---------------------------------------------------------------------------
121+
122+
describe("applyFlagDefaults: optional flags without defaults", () => {
123+
test("does not inject value for optional flag with no default", () => {
124+
const flagDefs: Record<string, FlagDef> = {
125+
query: { kind: "parsed", optional: true },
126+
};
127+
const result = applyFlagDefaults({}, flagDefs);
128+
expect("query" in result).toBe(false);
129+
});
130+
});
131+
132+
// ---------------------------------------------------------------------------
133+
// applyFlagDefaults — strips undefined, preserves other falsy values
134+
// ---------------------------------------------------------------------------
135+
136+
describe("applyFlagDefaults: undefined stripping", () => {
137+
test("strips undefined values from input flags", () => {
138+
const flagDefs: Record<string, FlagDef> = {};
139+
const result = applyFlagDefaults(
140+
{ a: undefined, b: null, c: 0, d: "", e: false },
141+
flagDefs
142+
);
143+
expect("a" in result).toBe(false);
144+
expect(result.b).toBeNull();
145+
expect(result.c).toBe(0);
146+
expect(result.d).toBe("");
147+
expect(result.e).toBe(false);
148+
});
149+
});
150+
151+
// ---------------------------------------------------------------------------
152+
// applyFlagDefaults — multiple flags combined
153+
// ---------------------------------------------------------------------------
154+
155+
describe("applyFlagDefaults: combined scenario", () => {
156+
test("applies defaults for missing flags while preserving provided ones", () => {
157+
const flagDefs: Record<string, FlagDef> = {
158+
period: {
159+
kind: "parsed",
160+
default: "7d",
161+
parse: (v: string) => ({ type: "relative", period: v }),
162+
},
163+
limit: {
164+
kind: "parsed",
165+
default: "25",
166+
parse: (v: string) => Number(v),
167+
},
168+
sort: { kind: "enum", default: "date" },
169+
fresh: { kind: "boolean", default: false },
170+
query: { kind: "parsed", optional: true },
171+
};
172+
const result = applyFlagDefaults({ limit: 50, query: undefined }, flagDefs);
173+
// period: default applied via parse
174+
expect(result.period).toEqual({ type: "relative", period: "7d" });
175+
// limit: caller value preserved
176+
expect(result.limit).toBe(50);
177+
// sort: default applied (raw)
178+
expect(result.sort).toBe("date");
179+
// fresh: default applied (raw)
180+
expect(result.fresh).toBe(false);
181+
// query: undefined stripped, no default → absent
182+
expect("query" in result).toBe(false);
183+
});
184+
});

0 commit comments

Comments
 (0)