Skip to content

Commit 163565c

Browse files
ivovivov
authored
refactor(core): Extract CLI parsing from command registry (#17676)
1 parent 64902c9 commit 163565c

File tree

7 files changed

+214
-39
lines changed

7 files changed

+214
-39
lines changed

‎packages/@n8n/backend-common/package.json‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@
2929
"n8n-workflow": "workspace:^",
3030
"picocolors": "catalog:",
3131
"reflect-metadata": "catalog:",
32-
"winston": "3.14.2"
32+
"winston": "3.14.2",
33+
"yargs-parser": "21.1.1"
3334
},
3435
"devDependencies": {
35-
"@n8n/typescript-config": "workspace:*"
36+
"@n8n/typescript-config": "workspace:*",
37+
"@types/yargs-parser": "21.0.0",
38+
"zod": "catalog:"
3639
}
3740
}

‎packages/@n8n/backend-common/src/__tests__/cli-parser.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import { mock } from 'jest-mock-extended';
2+
import z from 'zod';
3+
4+
import { CliParser } from '../cli-parser';
5+
6+
describe('parse', () => {
7+
it('should parse `argv` without flags schema', () => {
8+
const cliParser = new CliParser(mock());
9+
const result = cliParser.parse({ argv: ['node', 'script.js', 'arg1', 'arg2'] });
10+
expect(result).toEqual({ flags: {}, args: ['arg1', 'arg2'] });
11+
});
12+
13+
it('should parse `argv` with flags schema', () => {
14+
const cliParser = new CliParser(mock());
15+
const flagsSchema = z.object({
16+
verbose: z.boolean().optional(),
17+
name: z.string().optional(),
18+
});
19+
20+
const result = cliParser.parse({
21+
argv: ['node', 'script.js', '--verbose', '--name', 'test', 'arg1'],
22+
flagsSchema,
23+
});
24+
25+
expect(result).toEqual({
26+
flags: { verbose: true, name: 'test' },
27+
args: ['arg1'],
28+
});
29+
});
30+
31+
it('should ignore flags not defined in schema', () => {
32+
const cliParser = new CliParser(mock());
33+
const flagsSchema = z.object({
34+
name: z.string().optional(),
35+
// ignored is absent
36+
});
37+
38+
const result = cliParser.parse({
39+
argv: ['node', 'script.js', '--name', 'test', '--ignored', 'value', 'arg1'],
40+
flagsSchema,
41+
});
42+
43+
expect(result).toEqual({
44+
flags: {
45+
name: 'test',
46+
// ignored is absent
47+
},
48+
args: ['arg1'],
49+
});
50+
});
51+
52+
it('should handle a numeric value for `--id` flag', () => {
53+
const cliParser = new CliParser(mock());
54+
const result = cliParser.parse({
55+
argv: ['node', 'script.js', '--id', '123', 'arg1'],
56+
flagsSchema: z.object({
57+
id: z.string(),
58+
}),
59+
});
60+
61+
expect(result).toEqual({
62+
flags: { id: '123' },
63+
args: ['arg1'],
64+
});
65+
});
66+
67+
it('should handle positional arguments', () => {
68+
const cliParser = new CliParser(mock());
69+
const result = cliParser.parse({
70+
argv: ['node', 'script.js', '123', 'true'],
71+
});
72+
73+
expect(result.args).toEqual(['123', 'true']);
74+
expect(typeof result.args[0]).toBe('string');
75+
expect(typeof result.args[1]).toBe('string');
76+
});
77+
78+
it('should handle required flags with aliases', () => {
79+
const cliParser = new CliParser(mock());
80+
const flagsSchema = z.object({
81+
name: z.string(),
82+
});
83+
84+
// @ts-expect-error zod was monkey-patched to support aliases
85+
flagsSchema.shape.name._def._alias = 'n';
86+
87+
const result = cliParser.parse({
88+
argv: ['node', 'script.js', '-n', 'test', 'arg1'],
89+
flagsSchema,
90+
});
91+
92+
expect(result).toEqual({
93+
flags: { name: 'test' },
94+
args: ['arg1'],
95+
});
96+
});
97+
98+
it('should handle optional flags with aliases', () => {
99+
const cliParser = new CliParser(mock());
100+
const flagsSchema = z.object({
101+
name: z.optional(z.string()),
102+
});
103+
104+
// @ts-expect-error zod was monkey-patched to support aliases
105+
flagsSchema.shape.name._def.innerType._def._alias = 'n';
106+
107+
const result = cliParser.parse({
108+
argv: ['node', 'script.js', '-n', 'test', 'arg1'],
109+
flagsSchema,
110+
});
111+
112+
expect(result).toEqual({
113+
flags: { name: 'test' },
114+
args: ['arg1'],
115+
});
116+
});
117+
});

‎packages/@n8n/backend-common/src/cli-parser.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { Service } from '@n8n/di';
2+
import argvParser from 'yargs-parser';
3+
import type { z } from 'zod';
4+
5+
import { Logger } from './logging';
6+
7+
type CliInput<Flags extends z.ZodRawShape> = {
8+
argv: string[];
9+
flagsSchema?: z.ZodObject<Flags>;
10+
description?: string;
11+
examples?: string[];
12+
};
13+
14+
type ParsedArgs<Flags = Record<string, unknown>> = {
15+
flags: Flags;
16+
args: string[];
17+
};
18+
19+
@Service()
20+
export class CliParser {
21+
constructor(private readonly logger: Logger) {}
22+
23+
parse<Flags extends z.ZodRawShape>(
24+
input: CliInput<Flags>,
25+
): ParsedArgs<z.infer<z.ZodObject<Flags>>> {
26+
// eslint-disable-next-line id-denylist
27+
const { _: rest, ...rawFlags } = argvParser(input.argv, { string: ['id'] });
28+
29+
let flags = {} as z.infer<z.ZodObject<Flags>>;
30+
if (input.flagsSchema) {
31+
for (const key in input.flagsSchema.shape) {
32+
const flagSchema = input.flagsSchema.shape[key];
33+
let schemaDef = flagSchema._def as z.ZodTypeDef & {
34+
typeName: string;
35+
innerType?: z.ZodType;
36+
_alias?: string;
37+
};
38+
39+
if (schemaDef.typeName === 'ZodOptional' && schemaDef.innerType) {
40+
schemaDef = schemaDef.innerType._def as typeof schemaDef;
41+
}
42+
43+
const alias = schemaDef._alias;
44+
if (alias?.length && !(key in rawFlags) && rawFlags[alias]) {
45+
rawFlags[key] = rawFlags[alias] as unknown;
46+
}
47+
}
48+
49+
flags = input.flagsSchema.parse(rawFlags);
50+
}
51+
52+
const args = rest.map(String).slice(2);
53+
54+
this.logger.debug('Received CLI command', {
55+
execPath: rest[0],
56+
scriptPath: rest[1],
57+
args,
58+
flags,
59+
});
60+
61+
return { flags, args };
62+
}
63+
}

‎packages/@n8n/backend-common/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ export { Logger } from './logging/logger';
77
export { ModuleRegistry } from './modules/module-registry';
88
export { ModulesConfig, ModuleName } from './modules/modules.config';
99
export { isContainedWithin, safeJoinPath } from './utils/path-util';
10+
export { CliParser } from './cli-parser';

‎packages/cli/src/__tests__/command-registry.test.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Logger, ModuleRegistry } from '@n8n/backend-common';
2+
import { CliParser } from '@n8n/backend-common';
23
import { CommandMetadata } from '@n8n/decorators';
34
import { Container } from '@n8n/di';
45
import { mock } from 'jest-mock-extended';
@@ -17,6 +18,7 @@ describe('CommandRegistry', () => {
1718
const logger = mock<Logger>();
1819
let originalProcessArgv: string[];
1920
let mockProcessExit: jest.SpyInstance;
21+
const cliParser = new CliParser(logger);
2022

2123
class TestCommand {
2224
flags: any;
@@ -64,7 +66,7 @@ describe('CommandRegistry', () => {
6466
it('should execute the specified command', async () => {
6567
process.argv = ['node', 'n8n', 'test-command'];
6668

67-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
69+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
6870
await commandRegistry.execute();
6971

7072
expect(moduleRegistry.loadModules).toHaveBeenCalled();
@@ -83,7 +85,7 @@ describe('CommandRegistry', () => {
8385
const commandInstance = Container.get(commandClass);
8486
commandInstance.run = jest.fn().mockRejectedValue(error);
8587

86-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
88+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
8789
await commandRegistry.execute();
8890

8991
expect(commandInstance.catch).toHaveBeenCalledWith(error);
@@ -93,7 +95,7 @@ describe('CommandRegistry', () => {
9395
it('should parse and apply command flags', async () => {
9496
process.argv = ['node', 'n8n', 'test-command', '--flag1', 'value1', '--flag2', '-s', '123'];
9597

96-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
98+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
9799
await commandRegistry.execute();
98100

99101
const commandInstance = Container.get(commandMetadata.get('test-command')!.class);
@@ -107,7 +109,7 @@ describe('CommandRegistry', () => {
107109
it('should handle alias flags', async () => {
108110
process.argv = ['node', 'n8n', 'test-command', '--flag1', 'value1', '-s', '123'];
109111

110-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
112+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
111113
await commandRegistry.execute();
112114

113115
const commandInstance = Container.get(commandMetadata.get('test-command')!.class);
@@ -120,7 +122,7 @@ describe('CommandRegistry', () => {
120122
it('should exit with error when command not found', async () => {
121123
process.argv = ['node', 'n8n', 'non-existent-command'];
122124

123-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
125+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
124126
await commandRegistry.execute();
125127

126128
expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('not found'));
@@ -130,7 +132,7 @@ describe('CommandRegistry', () => {
130132
it('should display help when --help flag is used', async () => {
131133
process.argv = ['node', 'n8n', 'test-command', '--help'];
132134

133-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
135+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
134136
await commandRegistry.execute();
135137

136138
expect(logger.info).toHaveBeenCalledWith(expect.stringContaining('USAGE'));
@@ -143,7 +145,7 @@ describe('CommandRegistry', () => {
143145
it('should list all commands when global help is requested', async () => {
144146
process.argv = ['node', 'n8n', '--help'];
145147

146-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
148+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
147149
await commandRegistry.execute();
148150

149151
expect(logger.info).toHaveBeenCalledWith(expect.stringContaining('Available commands'));
@@ -153,7 +155,7 @@ describe('CommandRegistry', () => {
153155
it('should display proper command usage with printCommandUsage', () => {
154156
process.argv = ['node', 'n8n', 'test-command'];
155157

156-
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger);
158+
commandRegistry = new CommandRegistry(commandMetadata, moduleRegistry, logger, cliParser);
157159
const commandEntry = commandMetadata.get('test-command')!;
158160
commandRegistry.printCommandUsage(commandEntry);
159161

‎packages/cli/src/command-registry.ts‎

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { Logger, ModuleRegistry } from '@n8n/backend-common';
1+
import { CliParser, Logger, ModuleRegistry } from '@n8n/backend-common';
22
import { CommandMetadata, type CommandEntry } from '@n8n/decorators';
33
import { Container, Service } from '@n8n/di';
44
import glob from 'fast-glob';
55
import picocolors from 'picocolors';
6-
import argvParser from 'yargs-parser';
76
import { z } from 'zod';
87
import './zod-alias-support';
98

@@ -15,16 +14,12 @@ import './zod-alias-support';
1514
export class CommandRegistry {
1615
private commandName: string;
1716

18-
private readonly argv: argvParser.Arguments;
19-
2017
constructor(
2118
private readonly commandMetadata: CommandMetadata,
2219
private readonly moduleRegistry: ModuleRegistry,
2320
private readonly logger: Logger,
21+
private readonly cliParser: CliParser,
2422
) {
25-
// yargs-parser was resolving number like strings to numbers, which is not what we want
26-
// eslint-disable-next-line id-denylist
27-
this.argv = argvParser(process.argv.slice(2), { string: ['id'] });
2823
this.commandName = process.argv[2] ?? 'start';
2924
}
3025

@@ -53,13 +48,18 @@ export class CommandRegistry {
5348
return process.exit(1);
5449
}
5550

56-
if (this.argv.help || this.argv.h) {
51+
if (process.argv.includes('--help') || process.argv.includes('-h')) {
5752
this.printCommandUsage(commandEntry);
5853
return process.exit(0);
5954
}
6055

56+
const { flags } = this.cliParser.parse({
57+
argv: process.argv,
58+
flagsSchema: commandEntry.flagsSchema,
59+
});
60+
6161
const command = Container.get(commandEntry.class);
62-
command.flags = this.parseFlags(commandEntry);
62+
command.flags = flags;
6363

6464
let error: Error | undefined = undefined;
6565
try {
@@ -73,26 +73,6 @@ export class CommandRegistry {
7373
}
7474
}
7575

76-
parseFlags(commandEntry: CommandEntry) {
77-
if (!commandEntry.flagsSchema) return {};
78-
const { _, ...argv } = this.argv;
79-
Object.entries(commandEntry.flagsSchema.shape).forEach(([key, flagSchema]) => {
80-
let schemaDef = flagSchema._def as z.ZodTypeDef & {
81-
typeName: string;
82-
innerType?: z.ZodType;
83-
};
84-
if (schemaDef.typeName === 'ZodOptional' && schemaDef.innerType) {
85-
schemaDef = schemaDef.innerType._def as typeof schemaDef;
86-
}
87-
const alias = schemaDef._alias;
88-
if (alias?.length && !(key in argv) && argv[alias]) {
89-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
90-
argv[key] = argv[alias];
91-
}
92-
});
93-
return commandEntry.flagsSchema.parse(argv);
94-
}
95-
9676
async listAllCommands() {
9777
// Import all command files to register all the non-module commands
9878
const commandFiles = await glob('./commands/**/*.js', {

‎pnpm-lock.yaml‎

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)