Skip to content

feat(mapper): preserve node route mount prefixes#98

Open
rohitjavvadi wants to merge 2 commits into
openclaw:mainfrom
rohitjavvadi:feat/node-mounted-route-prefixes
Open

feat(mapper): preserve node route mount prefixes#98
rohitjavvadi wants to merge 2 commits into
openclaw:mainfrom
rohitjavvadi:feat/node-mounted-route-prefixes

Conversation

@rohitjavvadi
Copy link
Copy Markdown
Contributor

@rohitjavvadi rohitjavvadi commented May 20, 2026

Summary

This updates the Node mapper so routes declared inside mounted Express routers/apps and Hono sub-apps keep their literal mount prefix. A route declared inside a router mounted at /api is now reported under /api instead of as an unmounted child route.

Dynamic mounts stay conservative. If a router is only mounted through an unknown prefix, the mapper suppresses the raw child route instead of guessing a path that may be wrong.

Verification

  • Added mapper coverage for Express literal mounts, nested mounts, pathless mounted routers, prefix arrays, and Hono route prefixes
  • Added coverage that dynamic-only mounts do not emit misleading raw child routes
  • Ran focused mapper tests and the full test suite
  • Ran typecheck, lint, format check, diff check, and build

@rohitjavvadi rohitjavvadi requested a review from a team as a code owner May 20, 2026 17:22
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a69a00fa9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +376 to +379
const pathlessExpressMount =
method === "use" &&
(isPathlessExpressMount(args) || isSingleExpressChildMount(args, parent, targets));
const prefixes = literalPrefixes ?? (pathlessExpressMount ? ["/"] : null);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle pathless use(mw, router) mounts before suppression

When app.use() is called with middleware followed by a router (for example app.use(mw, apiRouter)), isPathlessExpressMount only treats the call as pathless if the first argument matches a narrow middleware-name heuristic. If the middleware identifier is generic (e.g. mw), this branch marks the child router as unknownMountedTargets, and routePathsForTarget drops all of that router’s routes. This is a regression in route discovery for common Express patterns that use short middleware variable names.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5c3f77a: generic middleware identifiers are now treated as pathless only when they resolve to function-like declarations, so app.use(mw, router) preserves mounted child routes while dynamic mw prefixes remain suppressed. Added regressions for sync/async generic middleware and dynamic mw prefixes.

Comment thread src/mappers/node-routes.ts Outdated
Comment on lines +546 to +547
function isMountPrefix(path: string): boolean {
return path.startsWith("/");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Accept wildcard mount prefixes instead of discarding routes

isMountPrefix rejects literal "*" mount paths, so valid mounts like app.use('*', router) are treated as unknown and can cause all child routes to be suppressed. Since wildcard mounts are legitimate Express mount paths, this drops discoverable routes that previously appeared.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5c3f77a: literal * mount prefixes are now accepted and normalized to slash-prefixed wildcard route output, with regression coverage for app.use("*", router).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant