Skip to content

Commit 4dfcf2d

Browse files
committed
fix: typecheck errors, CLI precedence over GCL_ env vars, completion support
- Fix TS2559 by widening injectGclVariableEnvVars parameter type - Fix TS2551 by using type assertion for yargs internal getOptions() - Use yargs parsed.defaulted to correctly detect CLI-explicit values, ensuring CLI always takes precedence over GCL_ env vars even when the CLI value matches the option default - Inject GCL env vars in completion callback so GCL_CWD etc. work during tab completion - Add 12 unit tests for injectGclEnvVars
1 parent c7fe62d commit 4dfcf2d

3 files changed

Lines changed: 92 additions & 4 deletions

File tree

src/argv.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ async function gitRootPath () {
2222
return stdout;
2323
}
2424

25-
export function injectGclVariableEnvVars (argv: {variable?: string[]}, env: Record<string, string | undefined>): void {
25+
export function injectGclVariableEnvVars (argv: {variable?: string[]; [key: string]: any}, env: Record<string, string | undefined>): void {
2626
const prefix = "GCL_VARIABLE_";
2727
for (const [envKey, envValue] of Object.entries(env)) {
2828
if (!envKey.startsWith(prefix) || envValue == null) continue;
@@ -47,6 +47,7 @@ export function injectGclEnvVars (
4747
argv: Record<string, any>,
4848
yargsOptions: YargsOptionsMeta,
4949
env: Record<string, string | undefined>,
50+
defaulted: Record<string, boolean> = {},
5051
): void {
5152
const arrays = new Set(yargsOptions.array.map(String));
5253
const booleans = new Set(yargsOptions.boolean.map(String));
@@ -65,7 +66,7 @@ export function injectGclEnvVars (
6566
continue;
6667
}
6768

68-
if (argv[key] !== undefined && argv[key] !== yargsOptions.default[key]) continue;
69+
if (argv[key] !== undefined && !defaulted[key]) continue;
6970

7071
if (booleans.has(key)) argv[key] = envValue === "true" || envValue === "1";
7172
else if (numbers.has(key)) argv[key] = Number(envValue);

src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ process.on("SIGUSR2", async () => {
4141
.command({
4242
handler: async (argv) => {
4343
try {
44+
const defaulted: Record<string, boolean> = (yparser as any).parsed?.defaulted ?? {};
4445
injectGclVariableEnvVars(argv, process.env);
45-
injectGclEnvVars(argv, yparser.getOptions(), process.env);
46+
injectGclEnvVars(argv, (yparser as any).getOptions(), process.env, defaulted);
4647
await handler(argv, new WriteStreamsProcess(), jobs);
4748
const failedJobs = Executor.getFailed(jobs);
4849
process.exit(failedJobs.length > 0 ? 1 : 0);
@@ -365,6 +366,9 @@ process.on("SIGUSR2", async () => {
365366
if (current.startsWith("-")) {
366367
completionFilter();
367368
} else {
369+
const completionDefaulted: Record<string, boolean> = (yparser as any).parsed?.defaulted ?? {};
370+
injectGclVariableEnvVars(yargsArgv, process.env);
371+
injectGclEnvVars(yargsArgv, (yparser as any).getOptions(), process.env, completionDefaulted);
368372
Argv.build({...yargsArgv, autoCompleting: true})
369373
.then(argv => state.getPipelineIid(argv.cwd, argv.stateDir).then(pipelineIid => ({argv, pipelineIid})))
370374
.then(({argv, pipelineIid}) => Parser.create(argv, new WriteStreamsMock(), pipelineIid, []))

tests/test-cases/gcl-variable-env/integration.test.ts

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {injectGclVariableEnvVars} from "../../../src/argv.js";
1+
import {injectGclVariableEnvVars, injectGclEnvVars} from "../../../src/argv.js";
22
import {execFile} from "child_process";
33
import {promisify} from "util";
44

@@ -75,6 +75,89 @@ describe("injectGclVariableEnvVars unit tests", () => {
7575
});
7676
});
7777

78+
describe("injectGclEnvVars unit tests", () => {
79+
const baseOptions = {
80+
array: ["volume"],
81+
boolean: ["quiet"],
82+
number: ["concurrency"],
83+
default: {quiet: false, concurrency: 0, cwd: "."},
84+
key: {quiet: true, concurrency: true, cwd: true, volume: true, _: true, $0: true, "some-kebab": true},
85+
};
86+
87+
test("injects string env var", () => {
88+
const argv: Record<string, any> = {cwd: ".", quiet: false};
89+
injectGclEnvVars(argv, baseOptions, {"GCL_CWD": "/tmp/test"}, {cwd: true, quiet: true});
90+
expect(argv.cwd).toBe("/tmp/test");
91+
});
92+
93+
test("injects boolean env var", () => {
94+
const argv: Record<string, any> = {quiet: false};
95+
injectGclEnvVars(argv, baseOptions, {"GCL_QUIET": "true"}, {quiet: true});
96+
expect(argv.quiet).toBe(true);
97+
});
98+
99+
test("injects boolean env var from '1'", () => {
100+
const argv: Record<string, any> = {quiet: false};
101+
injectGclEnvVars(argv, baseOptions, {"GCL_QUIET": "1"}, {quiet: true});
102+
expect(argv.quiet).toBe(true);
103+
});
104+
105+
test("injects number env var", () => {
106+
const argv: Record<string, any> = {concurrency: 0};
107+
injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {concurrency: true});
108+
expect(argv.concurrency).toBe(4);
109+
});
110+
111+
test("splits array env var on semicolons", () => {
112+
const argv: Record<string, any> = {volume: []};
113+
injectGclEnvVars(argv, baseOptions, {"GCL_VOLUME": "/a:/b;/c:/d"}, {volume: true});
114+
expect(argv.volume).toEqual(["/a:/b", "/c:/d"]);
115+
});
116+
117+
test("merges array env var with CLI values", () => {
118+
const argv: Record<string, any> = {volume: ["/cli:/path"]};
119+
injectGclEnvVars(argv, baseOptions, {"GCL_VOLUME": "/env:/path"}, {});
120+
expect(argv.volume).toEqual(["/env:/path", "/cli:/path"]);
121+
});
122+
123+
test("CLI explicit value takes precedence over env", () => {
124+
const argv: Record<string, any> = {concurrency: 8};
125+
injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {});
126+
expect(argv.concurrency).toBe(8);
127+
});
128+
129+
test("CLI explicit value takes precedence even when matching default", () => {
130+
const argv: Record<string, any> = {concurrency: 0};
131+
injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {});
132+
expect(argv.concurrency).toBe(0);
133+
});
134+
135+
test("env overrides when value was defaulted", () => {
136+
const argv: Record<string, any> = {concurrency: 0};
137+
injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {concurrency: true});
138+
expect(argv.concurrency).toBe(4);
139+
});
140+
141+
test("skips keys with hyphens", () => {
142+
const argv: Record<string, any> = {};
143+
injectGclEnvVars(argv, baseOptions, {"GCL_SOME_KEBAB": "val"}, {});
144+
expect(argv["some-kebab"]).toBeUndefined();
145+
});
146+
147+
test("skips _ and $0 keys", () => {
148+
const argv: Record<string, any> = {_: [], $0: "bin"};
149+
injectGclEnvVars(argv, baseOptions, {"GCL__": "x", "GCL_$0": "y"}, {});
150+
expect(argv._).toEqual([]);
151+
expect(argv.$0).toBe("bin");
152+
});
153+
154+
test("skips undefined env values", () => {
155+
const argv: Record<string, any> = {quiet: false};
156+
injectGclEnvVars(argv, baseOptions, {"GCL_QUIET": undefined}, {quiet: true});
157+
expect(argv.quiet).toBe(false);
158+
});
159+
});
160+
78161
test("GCL_VARIABLE_* env vars are injected into job output via CLI", async () => {
79162
const {stdout} = await execFileAsync("bun", ["src/index.ts", "test-job", "--cwd", "tests/test-cases/gcl-variable-env"], {
80163
env: {

0 commit comments

Comments
 (0)