Skip to content

Commit 083968b

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 083968b

2 files changed

Lines changed: 311 additions & 18 deletions

File tree

src/lib/sdk-invoke.ts

Lines changed: 127 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@ import type { Span } from "@sentry/core";
1919
import type { Writer } from "../types/index.js";
2020
import { type AsyncChannel, createAsyncChannel } from "./async-channel.js";
2121
import { setEnv } from "./env.js";
22+
import { logger } from "./logger.js";
2223
import { SentryError, type SentryOptions } from "./sdk-types.js";
2324

25+
const log = logger.withTag("sdk-invoke");
26+
2427
/** CLI flag names/aliases that trigger infinite streaming output. */
2528
const STREAMING_FLAG_NAMES = new Set(["--refresh", "--follow", "-f"]);
2629

@@ -59,17 +62,43 @@ function buildIsolatedEnv(
5962
/** Type alias for command handler functions loaded from Stricli's route tree. */
6063
type CommandHandler = (...args: unknown[]) => unknown;
6164

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

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

95198
// biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI escape sequences use ESC (0x1b)
@@ -412,26 +515,32 @@ export function buildInvoker(options?: SentryOptions) {
412515
): Promise<T> | AsyncIterable<T> {
413516
if (meta?.streaming) {
414517
return executeWithStream<T>(options, async (ctx, span) => {
415-
const func = await resolveCommand(commandPath);
518+
const { handler, flagDefs } = await resolveCommand(commandPath);
416519
if (span) {
417520
const { setCommandSpanName } = await import("./telemetry.js");
418521
setCommandSpanName(span, commandPath.join("."));
419522
}
420-
await func.call(
523+
const resolvedFlags = applyFlagDefaults(flags, flagDefs);
524+
await handler.call(
421525
ctx.context,
422-
{ ...flags, json: true },
526+
{ ...resolvedFlags, json: true },
423527
...positionalArgs
424528
);
425529
});
426530
}
427531

428532
return executeWithCapture<T>(options, async (ctx, span) => {
429-
const func = await resolveCommand(commandPath);
533+
const { handler, flagDefs } = await resolveCommand(commandPath);
430534
if (span) {
431535
const { setCommandSpanName } = await import("./telemetry.js");
432536
setCommandSpanName(span, commandPath.join("."));
433537
}
434-
await func.call(ctx.context, { ...flags, json: true }, ...positionalArgs);
538+
const resolvedFlags = applyFlagDefaults(flags, flagDefs);
539+
await handler.call(
540+
ctx.context,
541+
{ ...resolvedFlags, json: true },
542+
...positionalArgs
543+
);
435544
});
436545
};
437546
}

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 "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("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)