Add CLI module deduplication and improve conflict reporting

Individual CLI modules now silently take precedence over array-sourced
modules (e.g. from cli-defaults) when their commands overlap. Conflict
errors between non-array modules include both package names for easier
debugging.

Commands reference their parent module instead of storing a raw package
name string. createCliModule now validates that packageJson has a name.

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Made-with: Cursor

packages/cli-defaults/report.api.md

@@ -0,0 +1,13 @@
+## API Report File for "@backstage/cli-defaults"
+
+> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
+
+```ts
+import { CliModule } from '@backstage/cli-node';
+
+// @public
+const _default: CliModule[];
+export default _default;
+
+// (No @packageDocumentation comment for this package)
+```

packages/cli-defaults/src/index.ts

@@ -25,6 +25,11 @@ import newModule from '@backstage/cli-module-new';
 import testJest from '@backstage/cli-module-test-jest';
 import translations from '@backstage/cli-module-translations';
 
+/**
+ * The default set of CLI modules for the Backstage CLI.
+ *
+ * @public
+ */
 export default [
   auth,
   build,

packages/cli-internal/src/InternalCommandNode.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { CliCommand } from '@backstage/cli-node';
+import { CliCommand, CliModule } from '@backstage/cli-node';
 import { OpaqueType } from '@internal/opaque';
 
 /** @internal */
@@ -47,6 +47,7 @@ export const OpaqueCommandLeafNode = OpaqueType.create<{
     readonly version: 'v1';
     readonly name: string;
     readonly command: CliCommand;
+    readonly module?: CliModule;
   };
 }>({
   type: '@backstage/CommandLeafNode',

packages/cli-node/src/cli-module/createCliModule.ts

@@ -55,6 +55,12 @@ export function createCliModule(options: {
     addCommand: (command: CliCommand) => void;
   }) => Promise<void>;
 }): CliModule {
+  if (!options.packageJson.name) {
+    throw new Error(
+      'The packageJson provided to createCliModule must have a name',
+    );
+  }
+
   const commands: CliCommand[] = [];
   const commandsPromise = options
     .init({ addCommand: command => commands.push(command) })

packages/cli/src/wiring/CliInitializer.test.ts

@@ -19,6 +19,24 @@ import { createCliModule } from './factory';
 
 process.exit = jest.fn() as any;
 
+describe('createCliModule', () => {
+  it('should throw if packageJson has no name', () => {
+    expect(() =>
+      createCliModule({
+        packageJson: { name: '' },
+        init: async () => {},
+      }),
+    ).toThrow('The packageJson provided to createCliModule must have a name');
+
+    expect(() =>
+      createCliModule({
+        packageJson: {} as any,
+        init: async () => {},
+      }),
+    ).toThrow('The packageJson provided to createCliModule must have a name');
+  });
+});
+
 describe('CliInitializer', () => {
   beforeEach(() => {
     jest.resetAllMocks();
@@ -239,4 +257,120 @@ describe('CliInitializer', () => {
     await initializer.run();
     expect(process.exit).toHaveBeenCalledWith(0);
   });
+
+  it('should silently override array-sourced module with conflicting individual module while keeping siblings', async () => {
+    expect.assertions(3);
+    process.argv = ['node', 'cli', 'test'];
+    const initializer = new CliInitializer();
+
+    const individualModule = createCliModule({
+      packageJson: { name: '@backstage/individual' },
+      init: async reg =>
+        reg.addCommand({
+          path: ['test'],
+          description: 'individual test',
+          execute: ({ args }) => {
+            expect(args).toEqual([]);
+            return Promise.resolve();
+          },
+        }),
+    });
+
+    const conflictingArrayModule = createCliModule({
+      packageJson: { name: '@backstage/array-conflict' },
+      init: async reg =>
+        reg.addCommand({
+          path: ['test'],
+          description: 'array test (should be skipped)',
+          execute: () => Promise.resolve(),
+        }),
+    });
+
+    const nonConflictingArrayModule = createCliModule({
+      packageJson: { name: '@backstage/array-sibling' },
+      init: async reg =>
+        reg.addCommand({
+          path: ['other'],
+          description: 'other command',
+          execute: () => Promise.resolve(),
+        }),
+    });
+
+    initializer.add(individualModule);
+    initializer.add([conflictingArrayModule, nonConflictingArrayModule]);
+
+    await initializer.run();
+    expect(process.exit).toHaveBeenCalledWith(0);
+
+    // Verify the sibling command is available by running it
+    process.argv = ['node', 'cli', 'other'];
+    const initializer2 = new CliInitializer();
+    initializer2.add(individualModule);
+    initializer2.add([conflictingArrayModule, nonConflictingArrayModule]);
+    await initializer2.run();
+    expect(process.exit).toHaveBeenCalledTimes(2);
+  });
+
+  it('should error with package names when two individual modules conflict', async () => {
+    const initializer = new CliInitializer();
+
+    initializer.add(
+      createCliModule({
+        packageJson: { name: '@backstage/module-a' },
+        init: async reg =>
+          reg.addCommand({
+            path: ['conflicting'],
+            description: 'from module A',
+            execute: () => Promise.resolve(),
+          }),
+      }),
+    );
+
+    initializer.add(
+      createCliModule({
+        packageJson: { name: '@backstage/module-b' },
+        init: async reg =>
+          reg.addCommand({
+            path: ['conflicting'],
+            description: 'from module B',
+            execute: () => Promise.resolve(),
+          }),
+      }),
+    );
+
+    await expect(initializer.run()).rejects.toThrow(
+      'Command "conflicting" from "@backstage/module-b" conflicts with an existing command from "@backstage/module-a"',
+    );
+  });
+
+  it('should error with package names when two array-sourced modules conflict', async () => {
+    const initializer = new CliInitializer();
+
+    const moduleA = createCliModule({
+      packageJson: { name: '@backstage/array-a' },
+      init: async reg =>
+        reg.addCommand({
+          path: ['shared'],
+          description: 'from array A',
+          execute: () => Promise.resolve(),
+        }),
+    });
+
+    const moduleB = createCliModule({
+      packageJson: { name: '@backstage/array-b' },
+      init: async reg =>
+        reg.addCommand({
+          path: ['shared'],
+          description: 'from array B',
+          execute: () => Promise.resolve(),
+        }),
+    });
+
+    initializer.add([moduleA]);
+    initializer.add([moduleB]);
+
+    await expect(initializer.run()).rejects.toThrow(
+      'Command "shared" from "@backstage/array-b" conflicts with an existing command from "@backstage/array-a"',
+    );
+  });
 });

packages/cli/src/wiring/CliInitializer.ts

@@ -22,7 +22,6 @@ import {
 } from '@internal/cli';
 import type { CommandNode } from '@internal/cli';
 import type { CliModule } from '@backstage/cli-node';
-import { CommandRegistry } from './CommandRegistry';
 import { Command } from 'commander';
 import { version } from './version';
 import chalk from 'chalk';
@@ -44,28 +43,42 @@ type UninitializedFeature =
   | CliModule[]
   | Promise<{ default: CliModule | CliModule[] }>;
 
+interface TaggedFeature {
+  feature: CliModule;
+  fromArray: boolean;
+}
+
 export class CliInitializer {
   private graph = new CommandGraph();
-  private commandRegistry = new CommandRegistry(this.graph);
-  #uninitiazedFeatures: Promise<CliModule | CliModule[]>[] = [];
+  #uninitiazedFeatures: Promise<TaggedFeature[]>[] = [];
 
   add(feature: UninitializedFeature) {
     if (isPromise(feature)) {
       this.#uninitiazedFeatures.push(
-        feature.then(f => unwrapFeature(f.default)),
+        feature.then(f => {
+          const unwrapped = unwrapFeature(f.default);
+          if (Array.isArray(unwrapped)) {
+            return unwrapped.map(m => ({ feature: m, fromArray: true }));
+          }
+          return [{ feature: unwrapped, fromArray: false }];
+        }),
       );
     } else if (Array.isArray(feature)) {
-      this.#uninitiazedFeatures.push(Promise.resolve(feature));
+      this.#uninitiazedFeatures.push(
+        Promise.resolve(feature.map(m => ({ feature: m, fromArray: true }))),
+      );
     } else {
-      this.#uninitiazedFeatures.push(Promise.resolve(feature));
+      this.#uninitiazedFeatures.push(
+        Promise.resolve([{ feature, fromArray: false }]),
+      );
     }
   }
 
   async #register(feature: CliModule) {
     if (OpaqueCliModule.isType(feature)) {
-      const internal = OpaqueCliModule.toInternal(feature);
-      for (const command of await internal.commands) {
-        this.commandRegistry.addCommand(command);
+      for (const command of await OpaqueCliModule.toInternal(feature)
+        .commands) {
+        this.graph.add(command, feature);
       }
     } else {
       throw new Error(`Unsupported feature type: ${(feature as any).$$type}`);
@@ -73,15 +86,29 @@ export class CliInitializer {
   }
 
   async #doInit() {
-    const resolved = await Promise.all(this.#uninitiazedFeatures);
-    for (const featureOrArray of resolved) {
-      const features = Array.isArray(featureOrArray)
-        ? featureOrArray
-        : [featureOrArray];
-      for (const feature of features) {
-        await this.#register(feature);
+    const resolvedGroups = await Promise.all(this.#uninitiazedFeatures);
+    const allFeatures = resolvedGroups.flat();
+
+    // Collect command paths from individually-added modules
+    const individualPaths = new Set<string>();
+    for (const { feature, fromArray } of allFeatures) {
+      if (!fromArray && OpaqueCliModule.isType(feature)) {
+        const cmds = await OpaqueCliModule.toInternal(feature).commands;
+        for (const cmd of cmds) {
+          individualPaths.add(cmd.path.join(' '));
+        }
       }
     }
+
+    for (const { feature, fromArray } of allFeatures) {
+      if (fromArray && OpaqueCliModule.isType(feature)) {
+        const cmds = await OpaqueCliModule.toInternal(feature).commands;
+        if (cmds.some(cmd => individualPaths.has(cmd.path.join(' ')))) {
+          continue;
+        }
+      }
+      await this.#register(feature);
+    }
   }
 
   /**

packages/cli/src/wiring/CommandGraph.ts

@@ -17,8 +17,9 @@ import {
   CommandNode,
   OpaqueCommandTreeNode,
   OpaqueCommandLeafNode,
+  OpaqueCliModule,
 } from '@internal/cli';
-import { CliCommand } from './types';
+import { CliCommand, CliModule } from './types';
 
 /**
  * A sparse graph of commands.
@@ -30,7 +31,7 @@ export class CommandGraph {
    * Adds a command to the graph. The graph is sparse, so we use the path to determine the nodes
    *    to traverse. Only leaf nodes should have a command/action.
    */
-  add(command: CliCommand) {
+  add(command: CliCommand, module?: CliModule) {
     const path = command.path;
     let current = this.graph;
     for (let i = 0; i < path.length - 1; i++) {
@@ -50,7 +51,11 @@ export class CommandGraph {
         current.push(next);
       } else if (OpaqueCommandLeafNode.isType(next)) {
         throw new Error(
-          `Command already exists at path: "${path.slice(0, i).join(' ')}"`,
+          formatConflictError(
+            path,
+            module,
+            OpaqueCommandLeafNode.toInternal(next).module,
+          ),
         );
       }
       current = OpaqueCommandTreeNode.toInternal(next).children;
@@ -64,13 +69,18 @@ export class CommandGraph {
     });
     if (last && OpaqueCommandLeafNode.isType(last)) {
       throw new Error(
-        `Command already exists at path: "${path.slice(0, -1).join(' ')}"`,
+        formatConflictError(
+          path,
+          module,
+          OpaqueCommandLeafNode.toInternal(last).module,
+        ),
       );
     } else {
       current.push(
         OpaqueCommandLeafNode.createInstance('v1', {
           name: lastName,
           command,
+          module,
         }),
       );
     }
@@ -119,3 +129,30 @@ export class CommandGraph {
     return current;
   }
 }
+
+function getModuleName(module?: CliModule): string | undefined {
+  if (module && OpaqueCliModule.isType(module)) {
+    return OpaqueCliModule.toInternal(module).packageName;
+  }
+  return undefined;
+}
+
+function formatConflictError(
+  path: string[],
+  newModule?: CliModule,
+  existingModule?: CliModule,
+): string {
+  const cmd = path.join(' ');
+  const newPkg = getModuleName(newModule);
+  const existingPkg = getModuleName(existingModule);
+  if (newPkg && existingPkg) {
+    return `Command "${cmd}" from "${newPkg}" conflicts with an existing command from "${existingPkg}"`;
+  }
+  if (newPkg) {
+    return `Command "${cmd}" from "${newPkg}" conflicts with an existing command`;
+  }
+  if (existingPkg) {
+    return `Command "${cmd}" conflicts with an existing command from "${existingPkg}"`;
+  }
+  return `Command "${cmd}" conflicts with an existing command`;
+}