Skip to content

Commit 8876def

Browse files
colbymchenryclaude
andauthored
fix(nestjs): propagate RouterModule.register prefixes to controller routes (colbymchenry#459) (colbymchenry#460)
NestJS's RouterModule lets apps compose modular route prefixes across files (`RouterModule.register([{ path: 'admin', module: AdminModule, children: [...] }])` in `app.module.ts` sets the prefix for controllers declared in another file's `@Controller()`). The per-file `extract()` only sees one file at a time, so a `UsersController` indexed in isolation showed up as `GET /` instead of `GET /admin/users`. Add an optional cross-file `postExtract(context)` hook to FrameworkResolver, called by the orchestrator once after each `indexAll` and after every incremental `sync` that touched files. The nestjs implementation: * walks every `*.module.{ts,js}` for `RouterModule.{register,forRoot,forChild}([...])` and recursively resolves `children` into `Module → /full/prefix`, * walks `@Module({ controllers: [...] })` for `Controller → Module`, * matches each route node against its controller's class line range (multi-controller files keep getting attributed correctly), and * rewrites `name` while preserving `id` (route→handler edges intact) and `qualifiedName` (still encodes the *original* in-file path, which keeps the pass idempotent on a re-sync — `app.module.ts` edits propagate to controllers in unchanged files without double-prefixing). End-to-end validated against the exact reproduction in colbymchenry#459 (admin children users) — all four routes (`GET /admin`, `GET /admin/users`, `GET /admin/users/:id`, `POST /admin/users`) resolve correctly, edits to the RouterModule tree re-propagate on the next sync, and route→handler edges in `codegraph context` are preserved. Closes colbymchenry#459 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 180ba78 commit 8876def

6 files changed

Lines changed: 650 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99

1010
## [Unreleased]
1111

12+
### Fixed
13+
- **NestJS: `RouterModule.register([...])` route prefixes now propagate to controller routes.** Previously a controller declared inside a module wired through NestJS's `RouterModule` (a common pattern for modular apps with nested route prefixes) was indexed with its raw `@Controller(...) + @Get(...)` path — so `UsersController` under `RouterModule.register([{ path: 'admin', module: AdminModule, children: [{ path: 'users', module: UsersModule }] }])` showed up as `GET /` instead of `GET /admin/users`. The new cross-file pass walks every `*.module.{ts,js}` for `RouterModule.register/forRoot/forChild([...])` (recursive `children`) and `@Module({ controllers: [...] })`, then prepends the correct prefix to each affected route — including non-empty `@Controller` paths and method-level params (`/admin/users/:id`). The route node's `id` is preserved across the update so existing route→handler edges stay intact, and the pass is idempotent so incremental sync recovers when `app.module.ts` itself is edited. Closes #459.
14+
1215
### Added
1316
- **Installer targets for Gemini CLI and the Antigravity IDE.** `codegraph install` (and the interactive prompt) now detect and configure two more agents out of the box:
1417
- **Gemini CLI** (also covers the rebranded Antigravity CLI) — writes `mcpServers.codegraph` to `~/.gemini/settings.json` (global) or `./.gemini/settings.json` (local), and the codegraph usage block into `~/.gemini/GEMINI.md` / `./GEMINI.md`. Existing top-level settings (e.g. `security.auth`) and sibling MCP servers are preserved.

__tests__/frameworks.test.ts

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,271 @@ describe('nestjsResolver.resolve', () => {
528528
});
529529
});
530530

531+
describe('nestjsResolver.postExtract — RouterModule', () => {
532+
function mkClass(name: string, filePath: string, startLine: number, endLine: number): Node {
533+
return {
534+
id: `class:${filePath}:${startLine}:${name}`,
535+
kind: 'class',
536+
name,
537+
qualifiedName: `${filePath}::${name}`,
538+
filePath,
539+
language: 'typescript',
540+
startLine,
541+
endLine,
542+
startColumn: 0,
543+
endColumn: 0,
544+
updatedAt: 0,
545+
};
546+
}
547+
548+
function mkRoute(
549+
filePath: string,
550+
line: number,
551+
method: string,
552+
path: string,
553+
nameOverride?: string
554+
): Node {
555+
return {
556+
id: `route:${filePath}:${line}:${method}:${path}`,
557+
kind: 'route',
558+
name: nameOverride ?? `${method} ${path}`,
559+
qualifiedName: `${filePath}::${method}:${path}`,
560+
filePath,
561+
language: 'typescript',
562+
startLine: line,
563+
endLine: line,
564+
startColumn: 0,
565+
endColumn: 0,
566+
updatedAt: 0,
567+
};
568+
}
569+
570+
function makeContext(opts: {
571+
files?: Record<string, string>;
572+
nodes?: Node[];
573+
}) {
574+
const files = opts.files ?? {};
575+
const all = opts.nodes ?? [];
576+
return {
577+
getNodesInFile: (fp: string) => all.filter((n) => n.filePath === fp),
578+
getNodesByName: (name: string) => all.filter((n) => n.name === name),
579+
getNodesByQualifiedName: () => [],
580+
getNodesByKind: (kind: Node['kind']) => all.filter((n) => n.kind === kind),
581+
fileExists: (fp: string) => files[fp] !== undefined,
582+
readFile: (fp: string) => files[fp] ?? null,
583+
getProjectRoot: () => '/test',
584+
getAllFiles: () => Object.keys(files),
585+
getNodesByLowerName: () => [],
586+
getImportMappings: () => [],
587+
} as any;
588+
}
589+
590+
it('prepends RouterModule prefix to a controller route (top-level register)', () => {
591+
const ctx = makeContext({
592+
files: {
593+
'src/app.module.ts': `
594+
@Module({
595+
imports: [
596+
RouterModule.register([
597+
{ path: 'admin', module: AdminModule },
598+
]),
599+
],
600+
})
601+
export class AppModule {}
602+
603+
@Module({ controllers: [AdminController] })
604+
export class AdminModule {}
605+
`,
606+
},
607+
nodes: [
608+
mkClass('AdminController', 'src/admin/admin.controller.ts', 1, 10),
609+
mkRoute('src/admin/admin.controller.ts', 3, 'GET', '/'),
610+
],
611+
});
612+
613+
const updates = nestjsResolver.postExtract!(ctx);
614+
expect(updates).toHaveLength(1);
615+
expect(updates[0]!.name).toBe('GET /admin');
616+
// id and qualifiedName must be preserved so existing route→handler edges
617+
// stay intact and the pass remains idempotent on a second run.
618+
expect(updates[0]!.id).toBe('route:src/admin/admin.controller.ts:3:GET:/');
619+
expect(updates[0]!.qualifiedName).toBe('src/admin/admin.controller.ts::GET:/');
620+
});
621+
622+
it('resolves nested children — the issue #459 example', () => {
623+
const ctx = makeContext({
624+
files: {
625+
'src/app.module.ts': `
626+
@Module({
627+
imports: [
628+
AdminModule,
629+
UsersModule,
630+
RouterModule.register([
631+
{
632+
path: 'admin',
633+
module: AdminModule,
634+
children: [
635+
{ path: 'users', module: UsersModule },
636+
],
637+
},
638+
]),
639+
],
640+
})
641+
export class AppModule {}
642+
`,
643+
'src/users/users.module.ts': `
644+
@Module({ controllers: [UsersController] })
645+
export class UsersModule {}
646+
`,
647+
},
648+
nodes: [
649+
mkClass('UsersController', 'src/users/users.controller.ts', 1, 10),
650+
mkRoute('src/users/users.controller.ts', 3, 'GET', '/'),
651+
],
652+
});
653+
654+
const updates = nestjsResolver.postExtract!(ctx);
655+
expect(updates).toHaveLength(1);
656+
expect(updates[0]!.name).toBe('GET /admin/users');
657+
});
658+
659+
it('joins module prefix with a non-empty @Controller path and method params', () => {
660+
const ctx = makeContext({
661+
files: {
662+
'src/app.module.ts': `
663+
RouterModule.register([{ path: 'admin', module: UsersModule }])
664+
665+
@Module({ controllers: [UsersController] })
666+
export class UsersModule {}
667+
`,
668+
},
669+
nodes: [
670+
mkClass('UsersController', 'src/users.controller.ts', 1, 10),
671+
// Existing extract emitted GET /users/:id from @Controller('users') + @Get(':id')
672+
mkRoute('src/users.controller.ts', 3, 'GET', '/users/:id'),
673+
],
674+
});
675+
676+
const updates = nestjsResolver.postExtract!(ctx);
677+
expect(updates).toHaveLength(1);
678+
expect(updates[0]!.name).toBe('GET /admin/users/:id');
679+
});
680+
681+
it('is idempotent — a second run returns no updates', () => {
682+
// Simulate the state after one round of postExtract: name is already
683+
// 'GET /admin', but qualifiedName still encodes the original 'GET:/'.
684+
const ctx = makeContext({
685+
files: {
686+
'src/app.module.ts': `
687+
RouterModule.register([{ path: 'admin', module: UsersModule }])
688+
@Module({ controllers: [UsersController] })
689+
export class UsersModule {}
690+
`,
691+
},
692+
nodes: [
693+
mkClass('UsersController', 'src/users.controller.ts', 1, 10),
694+
mkRoute('src/users.controller.ts', 3, 'GET', '/', 'GET /admin'),
695+
],
696+
});
697+
698+
const updates = nestjsResolver.postExtract!(ctx);
699+
expect(updates).toHaveLength(0);
700+
});
701+
702+
it('is a no-op when the project does not use RouterModule', () => {
703+
const ctx = makeContext({
704+
files: {
705+
'src/app.module.ts': `
706+
@Module({ controllers: [UsersController] })
707+
export class AppModule {}
708+
`,
709+
},
710+
nodes: [
711+
mkClass('UsersController', 'src/users.controller.ts', 1, 10),
712+
mkRoute('src/users.controller.ts', 3, 'GET', '/'),
713+
],
714+
});
715+
716+
const updates = nestjsResolver.postExtract!(ctx);
717+
expect(updates).toHaveLength(0);
718+
});
719+
720+
it('attributes routes to the right controller when one file has two', () => {
721+
// Two controllers in one file, declared in two different modules with
722+
// two different module prefixes. The route's startLine has to match the
723+
// class scope, not just the file path.
724+
const ctx = makeContext({
725+
files: {
726+
'src/app.module.ts': `
727+
RouterModule.register([
728+
{ path: 'p1', module: AModule },
729+
{ path: 'p2', module: BModule },
730+
])
731+
@Module({ controllers: [AController] }) export class AModule {}
732+
@Module({ controllers: [BController] }) export class BModule {}
733+
`,
734+
},
735+
nodes: [
736+
mkClass('AController', 'src/multi.controller.ts', 1, 5),
737+
mkClass('BController', 'src/multi.controller.ts', 7, 12),
738+
mkRoute('src/multi.controller.ts', 3, 'GET', '/a/x'),
739+
mkRoute('src/multi.controller.ts', 9, 'GET', '/b/y'),
740+
],
741+
});
742+
743+
const updates = nestjsResolver.postExtract!(ctx);
744+
expect(updates).toHaveLength(2);
745+
const byId = new Map(updates.map((u) => [u.id, u.name]));
746+
expect(byId.get('route:src/multi.controller.ts:3:GET:/a/x')).toBe('GET /p1/a/x');
747+
expect(byId.get('route:src/multi.controller.ts:9:GET:/b/y')).toBe('GET /p2/b/y');
748+
});
749+
750+
it('merges RouterModule registrations spread across multiple module files', () => {
751+
const ctx = makeContext({
752+
files: {
753+
'src/app.module.ts': `
754+
RouterModule.register([{ path: 'a', module: AModule }])
755+
@Module({ controllers: [AController] }) export class AModule {}
756+
`,
757+
'src/feature.module.ts': `
758+
RouterModule.forChild([{ path: 'b', module: BModule }])
759+
@Module({ controllers: [BController] }) export class BModule {}
760+
`,
761+
},
762+
nodes: [
763+
mkClass('AController', 'src/a.controller.ts', 1, 5),
764+
mkClass('BController', 'src/b.controller.ts', 1, 5),
765+
mkRoute('src/a.controller.ts', 3, 'GET', '/'),
766+
mkRoute('src/b.controller.ts', 3, 'GET', '/'),
767+
],
768+
});
769+
770+
const updates = nestjsResolver.postExtract!(ctx);
771+
expect(updates).toHaveLength(2);
772+
const byId = new Map(updates.map((u) => [u.id, u.name]));
773+
expect(byId.get('route:src/a.controller.ts:3:GET:/')).toBe('GET /a');
774+
expect(byId.get('route:src/b.controller.ts:3:GET:/')).toBe('GET /b');
775+
});
776+
777+
it('silently skips controllers whose class node is not in the graph', () => {
778+
// RouterModule declares a prefix for a module, but the @Module that
779+
// would link it to a controller is missing — common during partial
780+
// re-extraction. Must not throw.
781+
const ctx = makeContext({
782+
files: {
783+
'src/app.module.ts': `
784+
RouterModule.register([{ path: 'orphans', module: GhostModule }])
785+
@Module({ controllers: [GhostController] }) export class GhostModule {}
786+
`,
787+
},
788+
nodes: [], // no class or route nodes
789+
});
790+
791+
const updates = nestjsResolver.postExtract!(ctx);
792+
expect(updates).toHaveLength(0);
793+
});
794+
});
795+
531796
import { laravelResolver } from '../src/resolution/frameworks/laravel';
532797

533798
describe('laravelResolver.extract', () => {

src/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,9 @@ export class CodeGraph {
336336
// chance to see the actual project before resolution runs.
337337
if (result.success && result.filesIndexed > 0) {
338338
this.resolver.initialize();
339+
// Cross-file finalization (e.g. NestJS RouterModule prefixes). Runs
340+
// before resolution so updated names show up in subsequent reads.
341+
this.resolver.runPostExtract();
339342
}
340343

341344
// Resolve references to create call/import/extends edges
@@ -406,6 +409,14 @@ export class CodeGraph {
406409
try {
407410
const result = await this.orchestrator.sync(options.onProgress);
408411

412+
// Cross-file finalization (e.g. NestJS RouterModule prefixes). Run on
413+
// every sync that touched files so edits to `app.module.ts` propagate
414+
// to controllers in unchanged files. The pass is idempotent and cheap
415+
// (regex over *.module.ts only).
416+
if (result.filesAdded > 0 || result.filesModified > 0) {
417+
this.resolver.runPostExtract();
418+
}
419+
409420
// Resolve references if files were updated
410421
if (result.filesAdded > 0 || result.filesModified > 0) {
411422
if (result.changedFilePaths) {

0 commit comments

Comments
 (0)