Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions apps/website/docs/guide/02-commands/09-subcommand-hierarchy.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,46 @@ This produces:
- `/ops status`
- `/ops deploy`

## Non-Leaf Command Files

A hierarchical node with children is a non-leaf node. When a directory
such as `[admin]` contains child subcommands or groups, its
`command.ts` file can define command metadata and configuration, but it
must not export executable handlers such as `chatInput`, `message`, or
`autocomplete`.

Executable handlers belong in leaf nodes, such as `[logs]/command.ts`
or `logs.subcommand.ts`.

Incorrect:

```text title="Directory Structure"
src/app/commands/
└── [admin]/
├── command.ts # Incorrect: exports chatInput while admin has children
└── [logs]/
└── command.ts
```

Correct:

```text title="Directory Structure"
src/app/commands/
└── [admin]/
├── command.ts # Correct: metadata/config only
└── [logs]/
└── command.ts # Correct: executable handlers live here
```

You can also use a shorthand leaf:

```text title="Directory Structure"
src/app/commands/
└── [admin]/
├── command.ts # metadata/config only
└── logs.subcommand.ts
```

## Middleware Scope In Hierarchical Trees

Hierarchical commands use the same middleware model as flat commands:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ This transforms into:
- `/settings notifications enable`
- `/settings notifications disable`

`[command]/command.ts` may represent a parent command node. If that
command directory contains child subcommands or groups, its
`command.ts` file must only define metadata or configuration and must
not export executable handlers such as `chatInput`, `message`, or
`autocomplete`. Place those handlers in leaf command files, such as
`profile.subcommand.ts` or `[disable]/command.ts`.

Middleware inside these trees follows the same same-directory rule:

- `+middleware.ts` applies only to leaves defined in that directory
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, describe, expect, test } from 'vitest';
import { afterEach, describe, expect, test, vi } from 'vitest';
import {
Client,
Collection,
Expand All @@ -9,6 +9,7 @@ import {
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import { CommandKit } from '../../commandkit';
import { Logger } from '../../logger/Logger';
import { AppCommandHandler } from './AppCommandHandler';
import { CommandsRouter } from '../router';

Expand Down Expand Up @@ -254,4 +255,90 @@ export async function message() {}
await client.destroy();
}
});

test('warns and ignores executable handlers from non-leaf nodes', async () => {
const warn = vi.spyOn(Logger, 'warn').mockImplementation(() => {});

const { client, handler } = await createHandlerWithCommands([
[
'[admin]/command.mjs',
`
export const command = {
description: "Admin metadata",
dm_permission: false
};

export async function chatInput() {}
export async function message() {}
export async function autocomplete() {}
`,
],
[
'[admin]/[logs]/command.mjs',
`
export const command = {
description: "Admin logs"
};

export async function chatInput() {}
export async function message() {}
`,
],
]);

try {
expect(warn).toHaveBeenCalledWith(
expect.arrayContaining([
'Ignoring ',
' exported by ',
' hierarchical node ',
'. Move ',
', ',
', or ',
' handlers to a ',
' command node.',
]),
expect.stringContaining('executable handlers'),
expect.stringContaining('[non-leaf]'),
expect.stringContaining('[admin]'),
expect.stringContaining('chatInput'),
expect.stringContaining('message'),
expect.stringContaining('autocomplete'),
expect.stringContaining('[leaf]'),
);

expect(warn.mock.calls[0]).toEqual(
expect.arrayContaining([
expect.arrayContaining([
' hierarchical node ',
'. Move ',
' command node.',
]),
expect.stringContaining('[admin]'),
]),
);

const hierarchicalNodes = handler.getHierarchicalNodesArray();
const adminNode = hierarchicalNodes.find((command) => {
return command.command.name === 'admin';
});

expect(adminNode?.data.command.description).toBe('Admin metadata');
expect(adminNode?.data.command.dm_permission).toBe(false);
expect(adminNode?.data.chatInput).toBeUndefined();
expect(adminNode?.data.message).toBeUndefined();
expect(adminNode?.data.autocomplete).toBeUndefined();

const runtimeRouteKeys = handler
.getRuntimeCommandsArray()
.map((command) => {
return (command.data.command as Record<string, unknown>).__routeKey;
});

expect(runtimeRouteKeys).toEqual(['admin.logs']);
} finally {
warn.mockRestore();
await client.destroy();
}
});
});
13 changes: 9 additions & 4 deletions packages/commandkit/src/app/handlers/AppCommandHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,9 @@ export class AppCommandHandler {
commandFileData.message ||
commandFileData.autocomplete
);
const sanitizedCommandFileData: AppCommandNative = {
...commandFileData,
};

if (!isRootHierarchyLeaf && hasContextMenuHandlers) {
throw new Error(
Expand All @@ -1315,17 +1318,19 @@ export class AppCommandHandler {
}

if (!node.executable && hasExecutableSlashHandlers) {
throw new Error(
`Invalid export for hierarchical node ${routeKey}: non-leaf hierarchical nodes cannot export executable slash/prefix handlers`,
);
Logger.warn`Ignoring ${colors.yellow('executable handlers')} exported by ${colors.cyan('[non-leaf]')} hierarchical node ${colors.magenta(`[${routeKey}]`)}. Move ${colors.yellow('chatInput')}, ${colors.yellow('message')}, or ${colors.yellow('autocomplete')} handlers to a ${colors.green('[leaf]')} command node.`;

delete sanitizedCommandFileData.chatInput;
delete sanitizedCommandFileData.message;
delete sanitizedCommandFileData.autocomplete;
}

const loadedCommand: LoadedCommand = {
discordId: null,
command,
metadata: resolvedMetadata,
data: {
...commandFileData,
...sanitizedCommandFileData,
metadata: resolvedMetadata,
command: {
...commandJson,
Expand Down
24 changes: 24 additions & 0 deletions packages/commandkit/src/app/router/CommandsRouter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,29 @@ describe('CommandsRouter', () => {
]);
});

test('warns when a non-leaf command node has a definition file', async () => {
const entrypoint = await createCommandsFixture([
['[admin]/command.ts'],
['[admin]/[logs]/command.ts'],
]);

const router = new CommandsRouter({ entrypoint });
const result = await router.scan();
const diagnostics = result.diagnostics ?? [];
const diagnostic = diagnostics.find((diag) => {
return diag.code === 'NON_LEAF_NODE_MAY_HAVE_HANDLERS';
});

expect(Object.keys(result.compiledRoutes ?? {})).toEqual(['admin.logs']);
expect(diagnostic).toMatchObject({
message:
'Hierarchical node "admin" has child nodes. Ensure its command.ts file does not export chatInput, message, or autocomplete handlers. Executable handlers must be defined on leaf nodes.',
});
expect(normalizePath(diagnostic?.path ?? '')).toBe(
normalizePath(join(entrypoint, '[admin]/command.ts')),
);
});

test('emits diagnostics for invalid hierarchical structures', async () => {
const entrypoint = await createCommandsFixture([
['{oops}/group.ts'],
Expand All @@ -172,6 +195,7 @@ describe('CommandsRouter', () => {
const diagnosticCodes = (result.diagnostics ?? []).map((diag) => diag.code);

expect(diagnosticCodes).toContain('ROOT_GROUP_NOT_ALLOWED');
expect(diagnosticCodes).toContain('NON_LEAF_NODE_MAY_HAVE_HANDLERS');
expect(diagnosticCodes).toContain('DUPLICATE_SIBLING_TOKEN');
expect(diagnosticCodes).toContain('MIXED_ROOT_CHILDREN');
expect(diagnosticCodes).toContain('MISSING_COMMAND_DEFINITION');
Expand Down
8 changes: 8 additions & 0 deletions packages/commandkit/src/app/router/CommandsRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,14 @@ export class CommandsRouter {
node.executable =
!!node.definitionPath && node.kind !== 'group' && !hasChildren;

if (hasChildren && node.definitionPath && node.kind !== 'group') {
this.addDiagnostic(
'NON_LEAF_NODE_MAY_HAVE_HANDLERS',
`Hierarchical node "${node.route.join('.')}" has child nodes. Ensure its command.ts file does not export chatInput, message, or autocomplete handlers. Executable handlers must be defined on leaf nodes.`,
node.definitionPath,
);
}

if (node.kind === 'subcommand' && hasChildren) {
this.addDiagnostic(
'SUBCOMMAND_CANNOT_HAVE_CHILDREN',
Expand Down